Enable captions on Gallery shortcodes #3659

kienstra · 2019-10-29T05:01:17Z

Summary

This allows displaying captions in Gallery shortcodes. It adds a class AMP_Carousel that uses logic in the Gallery block sanitizer from PR #3285.

Fixes #3658
Closes #3810

Checklist

My pull request is addressing an open issue (please create one otherwise).
My code is tested and passes existing tests.
My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

Use the existing logic in AMP_Carousel, and add tests for this. This was extracted from the logic in the Gallery block sanitizer.

@const

It doesn't only have images. Also, change some @const tags to @var.

Instead of using destructuring, simply use 0 and 1 indices.

This is now a method of AMP_Carousel, so it seem unnecessary to have 'carousel' in the function name.

It seems like the $dom property is usually protected or private in classes in the AMP plugin. Also, improve the DocBlock for AMP_Carousel::create_and_get().

I forgot to add $images_and_captions.

kienstra · 2019-10-29T20:00:18Z

includes/utils/class-amp-carousel.php

+	 * }
+	 * @return DOMElement A representation of the <amp-carousel>.
+	 */
+	public function create_and_get( $images_and_captions ) {


It'd be better if $images_and_captions were an associative array of images to their respective captions.

But a DOMElement (the image) isn't allowed as a key of an associative array.

So $images_and_captions now looks like:

[ [ DOMElement, 'Example caption here' ], [ DOMElement, 'Another caption for the next image' ], ]

I'd actually prefer this to be a value object instead. It can be type-hinted, would enforce a given structure and make sure it cannot be used incorrectly.

Normally, this would be a combination of a value object for a single entry, and a collection for multiple entries.

I'm not sure how open @westonruter is for going deeper into objects, though, so a compromise of a single value object for a collection of entries would work as well.

As an example, here's how an object-based representation would look like and behave (comments mostly skipped for brevity):

interface Has_Caption { /** @return string */ public function get_caption(); } class AMP_Image { /** @var DOMElement */ protected $image_node; public function __construct( DOMElement $image_node ) { $this->image_node = $image_node; } /** @return DOMElement */ public function get_image_node() { return $this->image_node; } } final class AMP_Captioned_Image extends AMP_Image implements Has_Caption { /** @var string */ private $caption; public function __construct( DOMElement $image_node, $caption ) { parent::__construct( $image_node ); $this->caption = $caption; } /** @return string */ public function get_caption() { return $this->caption; } } final class AMP_Image_List implements IteratorAggregate { /** @var AMP_Captioned_Image[] */ private $elements; /** * @param DOMElement $image_node * @param string $caption * @return self */ public function add( DOMElement $image_node, $caption = '' ) { $elements[] = empty( $caption ) ? new AMP_Image( $image_node ) : new AMP_Captioned_Image( $image_node, $caption ); return $this; } // This together with the IteratorAggregate turns the object into // a "Traversable", so you can just foreach over it and receive its // elements in the correct type. public function getIterator() { return new ArrayIterator( $this->elements ); } }

Using this would be pretty straight-forward, and most of all, more foolproof:

$images = ( new AMP_ImageList ) ->add( $image_node_1, 'First caption' ) ->add( $image_node_2, 'Second caption' ) ->add( $image_without_caption ); foreach ( $images as $image ) { // Using the node... $node = $image->get_image_node(); // Using the caption... if ( $image instanceof Has_Caption ) { $caption = $image->get_caption(); } }

Also, any complex operations that deal with these image lists can then be encapsulated into the list itself, making the consuming code that much cleaner.

Just a suggestion, but I would love if we would slowly would into more structured code like this, instead of building everything on top of unsemantic arrays and strings.
Note: In most cases, objects will use less memory than arrays in modern versions of PHP, as it is heavily optimized towards OOP since 7.0+.

That's pretty slick. I suppose my only hesitation is overly engineering an API for this which we don't necessarily want others to use externally, until we've given it it enough thought into how it relates to the other pieces. If we just use a simple array as the input, then it is more flexible in the case we decide to go a different direction. But then again, if we don't want people to be using this class yet anyway, then I don't suppose it matters. Just make sure it is marked with @internal.

If we do go this route, then it should definitely fall under the AMP namespace and the autoloader should automatically locate the filename purely based off of the name of the class/interface/trait. If we start creating a ton of little PHP files, we don't want to be having to embiggen the \AMP_Autoloader::$classmap endlessly.

Yes, I agree with marking as @internal for now. We should slowly build up our understanding and infrastructure with regards to an OOP approach of the plugin, and only really advertise an explicit set of interfaces with v2.0.

kienstra · 2019-10-29T20:02:35Z

includes/utils/class-amp-carousel.php

+	 * @return DOMElement A representation of the <amp-carousel>.
+	 */
+	public function create_and_get( $images_and_captions ) {
+		$images = [];


This function is mainly moved from AMP_Gallery_Block_Sanitizer, with few changes.

kienstra · 2019-10-29T20:10:32Z

includes/utils/class-amp-carousel.php

+	 *     @type int $height Height.
+	 * }
+	 */
+	public function get_dimensions( $images ) {


This function is mainly moved from AMP_Gallery_Block_Sanitizer::get_carousel_dimensions(), with few changes.

This was 'associate' before, so correct it.

This should simplify the markup a lot, as it reduces the attributes needed, and it eliminates the <amp-image-lightbox> tag. All that's needed here is a 'lightbox' attribute on an image.

kienstra · 2019-10-29T21:45:09Z

includes/embeds/class-amp-gallery-embed.php

-			);
-			/* We need to add lightbox tag, too. @todo Could there be a better alternative for this? */
-			return $this->render( $args ) . $lightbox_tag;
-		}


This is now using amp-lightbox-gallery as recommended by amphtml, so all that's needed is a lightbox attribute on the <amp-img> or <amp-carousel>. This plugin automatically adds the amp-lightbox-gallery component script.

kienstra · 2019-10-29T21:59:17Z

includes/utils/class-amp-carousel.php

+ *
+ * Gets the markup for an <amp-carousel>.
+ *
+ * @since 1.4.1


Maybe this will be different.

Yes, let's have the changes here by targeting 1.5.0

Sounds good

kienstra · 2019-10-29T22:01:03Z

includes/sanitizers/class-amp-gallery-block-sanitizer.php

@@ -12,24 +12,6 @@
 */
 class AMP_Gallery_Block_Sanitizer extends AMP_Base_Sanitizer {

-	/**


There's no intended change to the Gallery block, only a refactoring of this logic into the class AMP_Carousel.

(Now it's AMP\Carousel)

Now, it's Amp\AmpWp\Component\Carousel

Actually, it's Amp\AmpWP (capital_P_dangit? 😄)

For what it's worth, I do like AmpWP more than AmpWp.

count() will only work for arrays or objects, so this would handle having a falsey $image argument.

kienstra · 2019-10-29T23:01:27Z

Request For Review, But No Hurry

Hi @westonruter and @schlessera, and @pierlon,
Good to see you earlier.

No hurry reviewing this, with the stable release and WCUS coming up. Just wanted to let you know that's it's ready when you have a chance.

This extracts much of the logic from the Gallery block caption PR (#3285) into a class AMP_Carousel that is also used for the Gallery shortcode.

There's no intended change to Gallery blocks, and I didn't see any in my testing. But I asked for regression testing for them in the QA testing instructions.

Like in the Gallery block caption PR (#3285), the Gallery shortcode now looks different if there's a mix of landscape and square images.

I think it's an improvement, but some might disagree.

Thanks!

Before

After

schlessera · 2019-11-08T20:58:40Z

includes/utils/class-amp-carousel.php

+	 * @param array[] $images_and_captions {
+	 *     The images and their respective captions, if any.
+	 *
+	 *     @type DOMElement  $image A representation of an image, in index 0.


Suggested change

* @type DOMElement $image A representation of an image, in index 0.

* @type DOMElement $image A representation of an image, in index 0.

This is a good suggestion, but luckily the entire @param tag was simplified, thanks to your code below.

schlessera · 2019-11-08T21:38:16Z

includes/utils/class-amp-carousel.php

+	 * }
+	 * @return DOMElement A representation of the <amp-carousel>.
+	 */
+	public function create_and_get( $images_and_captions ) {


I'd actually prefer this to be a value object instead. It can be type-hinted, would enforce a given structure and make sure it cannot be used incorrectly.

Normally, this would be a combination of a value object for a single entry, and a collection for multiple entries.

I'm not sure how open @westonruter is for going deeper into objects, though, so a compromise of a single value object for a collection of entries would work as well.

As an example, here's how an object-based representation would look like and behave (comments mostly skipped for brevity):

interface Has_Caption { /** @return string */ public function get_caption(); } class AMP_Image { /** @var DOMElement */ protected $image_node; public function __construct( DOMElement $image_node ) { $this->image_node = $image_node; } /** @return DOMElement */ public function get_image_node() { return $this->image_node; } } final class AMP_Captioned_Image extends AMP_Image implements Has_Caption { /** @var string */ private $caption; public function __construct( DOMElement $image_node, $caption ) { parent::__construct( $image_node ); $this->caption = $caption; } /** @return string */ public function get_caption() { return $this->caption; } } final class AMP_Image_List implements IteratorAggregate { /** @var AMP_Captioned_Image[] */ private $elements; /** * @param DOMElement $image_node * @param string $caption * @return self */ public function add( DOMElement $image_node, $caption = '' ) { $elements[] = empty( $caption ) ? new AMP_Image( $image_node ) : new AMP_Captioned_Image( $image_node, $caption ); return $this; } // This together with the IteratorAggregate turns the object into // a "Traversable", so you can just foreach over it and receive its // elements in the correct type. public function getIterator() { return new ArrayIterator( $this->elements ); } }

Using this would be pretty straight-forward, and most of all, more foolproof:

$images = ( new AMP_ImageList ) ->add( $image_node_1, 'First caption' ) ->add( $image_node_2, 'Second caption' ) ->add( $image_without_caption ); foreach ( $images as $image ) { // Using the node... $node = $image->get_image_node(); // Using the caption... if ( $image instanceof Has_Caption ) { $caption = $image->get_caption(); } }

Also, any complex operations that deal with these image lists can then be encapsulated into the list itself, making the consuming code that much cleaner.

Just a suggestion, but I would love if we would slowly would into more structured code like this, instead of building everything on top of unsemantic arrays and strings.
Note: In most cases, objects will use less memory than arrays in modern versions of PHP, as it is heavily optimized towards OOP since 7.0+.

schlessera · 2019-11-08T21:49:32Z

includes/utils/class-amp-carousel.php

+				$image = $image->firstChild;
+			}
+
+			if ( ! is_numeric( $image->getAttribute( 'width' ) ) || ! is_numeric( $image->getAttribute( 'height' ) ) ) {


Do we need to take 0 values into account here? If these can pass through (not sure that might be the case), then we'll end up with a division by zero fatal in the case of the height.

Good point, it's possible that there'd be a '0' width or height.

93aa3ea continues if there's a 0 or 0.0 width or height.

pierlon

Hi @kienstra, your changes work as described. No errors or warnings occurred while testing 👍

…ry-shortcode-captions * 'develop' of github.com:ampproject/amp-wp: (61 commits) Improve specificity of JS doc Fix identifying sources for validation errors coming child themes (#3708) Fix failing E2E tests (#3707) Remove amp_validate query var from Validated URL 'View' row action Re-factor get_html_attribute_pattern as match_element_attributes Quote variables added to regex pattern Escape instances of unescapeed output in AMP settings screen code (#3703) Replace incorrect usage of esc_url() with esc_url_raw() Remove empty alt attributes Add object-fit=contain to amp-youtube placeholder image Prevent copying empty title/alt in WP<5.2 Extract HTML attribute parsing logic into base class, and use for YouTube embeds Update doc comment for `get_video_id_from_url` Merge separate regexes used to retrieve props into one Replace ambiguous `$fallback` with `$fallback_for_expected` Refactor `get_video_id_from_url` function Replace the fallback with an image placeholder for the iframe Remove todo HTML entity encoding will be handled by the DOM instead Bump `oembed` function deprecation version to 1.5.0 ...

This won't make it in 1.4.1, so target the next major release.

@schlessera

As Alain mentioned, this is much easier to understand than previously accepting an array of $images_and_captions. Props @schlessera.

As Alain mentioned, if there is a height of 0 or 0.0, this could cause an error when calculating the aspect ratio.

Like the previous commit, change other package names here.

westonruter · 2019-11-22T23:18:41Z

src/Component/DOMElementList.php

+	 * @return self
+	 */
+	public function add( DOMElement $element, $caption = '' ) {
+		$this->elements[] = empty( $caption ) ? $element->cloneNode() : new CaptionedSlide( $element->cloneNode(), $caption );


Shouldn't this be a deep clone? Otherwise the noscript > img children won't be included.

Suggested change

$this->elements[] = empty( $caption ) ? $element->cloneNode() : new CaptionedSlide( $element->cloneNode(), $caption );

$this->elements[] = empty( $caption ) ? $element->cloneNode( true ) : new CaptionedSlide( $element->cloneNode( true ), $caption );

Yes, good idea.

That helped, but the tests still failed locally, even changing all of the ->cloneNode() calls to ->cloneNode( true ). I'm working on it.

This looked to cause an error here: https://github.com/ampproject/amp-wp/blob/7b603eaeda0d87c71815f8014c9a5ae09fd3eac4/src/Component/Carousel.php#L152 getAttribute(): Couldn't fetch DOMElement It looks like there was no longer a reference to the DOMElement after cloning it.

kienstra · 2019-11-23T01:34:09Z

Hi @schlessera,

@kienstra Can you make the ImageList immutable? If I remember correctly, you'd only need to clone on add() to achieve that, and always re-assign the return value.

So far, I couldn't find a way to make ImageList (now DOMElementList) immutable, without causing an error.

This looked to cause an error in this unit test on this line.

getAttribute(): Couldn't fetch DOMElement

It looks like after running ->cloneNode( true ), there was no longer a reference to the DOMElement.

kienstra · 2019-11-23T01:50:12Z

Though maybe I'm not clear on what immutable means in this context.

Here's a (now reverted) unit test to check if it was possible to change an attribute when iterating over the DOMElementList.

schlessera · 2019-11-23T09:29:42Z

I'm sorry, @kienstra, my comments were probably misleading.

You cannot really make the DOMNodes reliably immutable, as they are not designed that way.

What I meant was make the "container" ImageList immutable in the sense that when you have an image list, it will stick to the images it has, even if some other code adds an image later on.

So, just cloning the list itself before adding and returning the clone, and always assigning the return value of the add() operation to accept the clone as the new image list.

It should be pretty straight-forward. Anything more is overkill and will still never work reliably.

Thanks to Alain's suggestion, this makes the DOMElementList immutable. The $elements property is now public, but the DOMElements in that were publicly available through iteration, so that isn't a huge change.

This doesn't simply return self anymore, but a clone of it, with an element added.

kienstra · 2019-11-24T18:48:39Z

Hi @schlessera,
Thanks, that's a good idea 😄

3cdd86f clones the list in add() and returns that clone.

For $elements, it should be array, as it's possible for it to have CaptionedImage instances.

schlessera · 2019-11-25T10:57:52Z

src/Component/DOMElementList.php

@@ -25,7 +25,7 @@ final class DOMElementList implements IteratorAggregate, Countable {
 	 *
 	 * @var DOMElement[]
 	 */
-	private $elements = [];
+	public $elements = [];


Why did you switch this to public? That totally defeats the purpose of making it immutable, if anyone can just change the internals...

Suggested change

public $elements = [];

private $elements = [];

My mistake, 38df209 changes it back to private.

I originally changed it because of failing unit tests, but they pass now.

schlessera · 2019-11-25T11:13:52Z

composer.json

@@ -33,6 +33,7 @@
    "ext-zip": "Enables the use of ZipArchive to export AMP Stories."
  },
  "config": {
+    "optimize-autoloader": true,


This should be removed, and a corresponding change should be made to the build process instead.

If an optimized autoloader is forced, then you'll have to constantly rebuild the plugin on filesystem changes even during development.

schlessera · 2019-11-25T11:14:41Z

composer.json

+  },
+  "autoload": {
+    "psr-4": {
+      "Amp\\AmpWP\\": "src/"


Discussed in #3810

src/Component/Carousel.php

schlessera · 2019-11-25T11:20:12Z

src/Component/Carousel.php

+	 *     @type int $height The height of the carousel, at index 1.
+	 * }
+	 */
+	public function get_dimensions() {


Is this supposed to be publicly accessible, i.e. do we want to give access to this to third-party developers as our public interface?

Uf, there's probably not a need for it to be public.

478f71e makes it private

schlessera · 2019-11-25T11:23:08Z

src/Component/Carousel.php

+	 */
+	public function __construct( $dom, DOMElementList $images ) {
+		$this->dom    = $dom;
+		$this->images = $images;


As the carousel can contain other elements than just images, should we rename this property and the corresponding variables into $slides instead to match the CaptionedSlide adjustment?

Ah, good point. It's now changed in 450f9cd

I should have brought this up before: the Carousel class probably makes the most sense for images, as get_dimensions() looks for width and height attributes, and has some conditionals for <amp-img>.

Still, maybe Carousel could be used in the future for something else with width and height attributes, like amp-video.

And as you mentioned, the DOMElementList can now have any element.

I had changed this to be public, but it looks like it doesn't need to be anymore.

Co-Authored-By: Alain Schlesser <[email protected]>

Commit Alain's suggestion to make `$dom` private Co-Authored-By: Alain Schlesser <[email protected]>

Co-Authored-By: Alain Schlesser <[email protected]>

As Alain mentioned, it can now accept any element in DOMElementList.

Following up on 450f9cd, also change references to 'slides'.

… in build It looks like it's not good to have this in composer.json.

There probably isn't a need for this to be in the public API.

@see

This flag is recommended for production only: "Note: You should not enable any of these optimizations in development as they all will cause various problems when adding/removing classes. The performance gains are not worth the trouble in a development setting." @see https://getcomposer.org/doc/articles/autoloader-optimization.md#autoloader-optimization Maybe this should be added to .travis.yml But maybe it could cause issues with the cached builds. It looks like the performance improvement could be very slight, compared to possibly debugging in the future.

westonruter

Gallery block and gallery shortcode now render the same, with captions!

Amends #3659

Enable captions on Gallery shortcodes

1b34cef

Use the existing logic in AMP_Carousel, and add tests for this. This was extracted from the logic in the Gallery block sanitizer.

googlebot added the cla: yes Signed the Google CLA label Oct 29, 2019

kienstra added 5 commits October 28, 2019 23:06

Rename a $images to $images_and_captions for clarity

dde1e46

It doesn't only have images. Also, change some @const tags to @var.

Address a PHPCS issue with array destructuring

a48d974

Instead of using destructuring, simply use 0 and 1 indices.

Rename get_carousel_dimensions() to get_dimensions()

5c7334b

This is now a method of AMP_Carousel, so it seem unnecessary to have 'carousel' in the function name.

Make the $dom property protected, improve a DocBlock

641e008

It seems like the $dom property is usually protected or private in classes in the AMP plugin. Also, improve the DocBlock for AMP_Carousel::create_and_get().

Add the parameter name to the @param tag

cbd925b

I forgot to add $images_and_captions.

kienstra commented Oct 29, 2019

View reviewed changes

kienstra added 2 commits October 29, 2019 14:13

Fix a typo in 'associative'

5e07516

This was 'associate' before, so correct it.

Use <amp-lightbox-gallery> for lightboxes

93246e8

This should simplify the markup a lot, as it reduces the attributes needed, and it eliminates the <amp-image-lightbox> tag. All that's needed here is a 'lightbox' attribute on an image.

kienstra commented Oct 29, 2019

View reviewed changes

Exit if ! $images, before running count( $images )

08404be

count() will only work for arrays or objects, so this would handle having a falsey $image argument.

kienstra mentioned this pull request Oct 29, 2019

Add support for captions to the Gallery shortcode #3658

Closed

kienstra changed the title ~~[WIP] Enable captions on Gallery shortcodes~~ Enable captions on Gallery shortcodes Oct 29, 2019

Merge branch 'develop' into add/gallery-shortcode-captions

0caa844

kienstra requested review from westonruter, schlessera and pierlon October 29, 2019 23:01

Merge branch 'develop' into add/gallery-shortcode-captions

2c32f41

schlessera requested changes Nov 8, 2019

View reviewed changes

pierlon reviewed Nov 10, 2019

View reviewed changes

westonruter and others added 4 commits November 10, 2019 22:25

Change @SInCE tag from 1.4.1 to 1.5.0

82b428a

This won't make it in 1.4.1, so target the next major release.

Commit Alain's classes to represent and collect images

d3ba063

As Alain mentioned, this is much easier to understand than previously accepting an array of $images_and_captions. Props @schlessera.

Account for 0.0 or 0 width or height in dimensions function

93aa3ea

As Alain mentioned, if there is a height of 0 or 0.0, this could cause an error when calculating the aspect ratio.

Change other package names

305e471

Like the previous commit, change other package names here.

westonruter reviewed Nov 22, 2019

View reviewed changes

kienstra added 2 commits November 24, 2019 12:39

Clone the DOMElementList in add(), and return the clone

3cdd86f

Thanks to Alain's suggestion, this makes the DOMElementList immutable. The $elements property is now public, but the DOMElements in that were publicly available through iteration, so that isn't a huge change.

Update the DocBlock of DOMElementList::add()

097d1b8

This doesn't simply return self anymore, but a clone of it, with an element added.

Edit DocBlocks, include correcting var tag

873c41a

For $elements, it should be array, as it's possible for it to have CaptionedImage instances.

westonruter requested a review from schlessera November 25, 2019 01:14

schlessera requested changes Nov 25, 2019

View reviewed changes

kienstra and others added 9 commits November 25, 2019 12:53

Make $elements propert private again

38df209

I had changed this to be public, but it looks like it doesn't need to be anymore.

Commit Alain's suggestion to make the Carousel class final

f48d09b

Co-Authored-By: Alain Schlesser <[email protected]>

Update src/Component/Carousel.php

5a8749c

Commit Alain's suggestion to make `$dom` private Co-Authored-By: Alain Schlesser <[email protected]>

Commit Alain's suggestion to make $images private

0355530

Co-Authored-By: Alain Schlesser <[email protected]>

In Carousel, rename $images to $slides

450f9cd

As Alain mentioned, it can now accept any element in DOMElementList.

Also rename images to slides in Test_Carousel

0ae904e

Following up on 450f9cd, also change references to 'slides'.

Remove optimize-autoloader from composer.json config, will apply this…

4ac1d25

… in build It looks like it's not good to have this in composer.json.

Make get_dimensions() private instead of public

478f71e

There probably isn't a need for this to be in the public API.

schlessera approved these changes Nov 26, 2019

View reviewed changes

westonruter added this to the v1.5 milestone Nov 26, 2019

westonruter approved these changes Nov 26, 2019

View reviewed changes

westonruter merged commit 9c853ca into develop Nov 26, 2019

westonruter deleted the add/gallery-shortcode-captions branch November 26, 2019 18:43

westonruter added a commit that referenced this pull request Dec 12, 2019

Add missing src directory to build

e5be971

Amends #3659

westonruter mentioned this pull request Dec 12, 2019

Add missing src directory to build #3921

Merged

westonruter mentioned this pull request Dec 24, 2019

Fix handling of gallery carousels in paragraphs and fix no-JS fallbacks #4009

Merged

3 tasks

westonruter added the Changelogged label Mar 23, 2020

	* @type DOMElement $image A representation of an image, in index 0.
	* @type DOMElement $image A representation of an image, in index 0.

	$this->elements[] = empty( $caption ) ? $element->cloneNode() : new CaptionedSlide( $element->cloneNode(), $caption );
	$this->elements[] = empty( $caption ) ? $element->cloneNode( true ) : new CaptionedSlide( $element->cloneNode( true ), $caption );

Enable captions on Gallery shortcodes #3659

Enable captions on Gallery shortcodes #3659

Conversation

kienstra commented Oct 29, 2019 • edited by westonruter Loading

Summary

Checklist

kienstra Oct 29, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kienstra Oct 29, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kienstra commented Oct 29, 2019 • edited Loading

Before

After

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pierlon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kienstra Nov 22, 2019 • edited Loading

Choose a reason for hiding this comment

kienstra commented Nov 23, 2019 • edited Loading

kienstra commented Nov 23, 2019

schlessera commented Nov 23, 2019 • edited Loading

kienstra commented Nov 24, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kienstra Nov 25, 2019 • edited Loading

Choose a reason for hiding this comment

westonruter left a comment

Choose a reason for hiding this comment

kienstra commented Oct 29, 2019 •

edited by westonruter

Loading

kienstra Oct 29, 2019 •

edited

Loading

kienstra Oct 29, 2019 •

edited

Loading

kienstra commented Oct 29, 2019 •

edited

Loading

kienstra Nov 22, 2019 •

edited

Loading

kienstra commented Nov 23, 2019 •

edited

Loading

schlessera commented Nov 23, 2019 •

edited

Loading

kienstra Nov 25, 2019 •

edited

Loading