Merge pull request #3659 from ampproject/add/gallery-shortcode-captions

Enable captions on Gallery shortcodes

Author: westonruter

bin/build-plugin-zip.sh

@@ -40,7 +40,7 @@ cd "$BUILD_DIR"
 
 # Run the build.
 status "Installing dependencies... 📦"
-composer install
+composer install -o
 PUPPETEER_SKIP_CHROMIUM_DOWNLOAD=true npm install
 
 status "Generating build... ⚙️"

composer.json

@@ -44,5 +44,10 @@
         "PHP-CSS-Parser: Fix parsing CSS selectors which contain commas <https://github.com/sabberworm/PHP-CSS-Parser/pull/138>": "https://github.com/sabberworm/PHP-CSS-Parser/commit/fa139f65c5b098ae652c970b25e6eb03fc495eb4.diff"
       }
     }
+  },
+  "autoload": {
+    "psr-4": {
+      "Amp\\AmpWP\\": "src/"
+    }
   }
 }

contributing/engineering.md

@@ -308,7 +308,7 @@ This will create an `amp.zip` in the plugin directory which you can install. The
 1. Create changelog draft on [Wiki page](https://github.com/ampproject/amp-wp/wiki/Release-Changelog-Draft).
 1. Check out the branch intended for release (`develop` for major, `x.y` for minor) and pull latest commits.
 1. Bump plugin versions in `amp.php` (×2: the metadata block in the header and also the `AMP__VERSION` constant).
-1. Do `npm install && composer selfupdate && composer install`.
+1. Do `npm install && composer selfupdate && composer install -o`.
 1. Do `npm run build` and install the `amp.zip` onto a normal WordPress install running a stable release build; do smoke test to ensure it works.
 1. [Draft new release](https://github.com/ampproject/amp-wp/releases/new) on GitHub targeting the required branch (`develop` for major, `x.y` for minor).
     1. Use the new plugin version as the tag (e.g. `1.2-beta3` or `1.2.1-RC1`)
@@ -334,7 +334,7 @@ Contributors who want to make a new release, follow these steps:
 1. For major release, draft blog post about the new release.
 1. For minor releases, make sure all merged commits in `develop` have been also merged onto release branch.
 1. Check out the branch intended for release (`develop` for major, `x.y` for minor) and pull latest commits.
-1. Do `npm install && composer selfupdate && composer install`.
+1. Do `npm install && composer selfupdate && composer install -o`.
 1. Bump plugin versions in `amp.php` (×2: the metadata block in the header and also the `AMP__VERSION` constant). Verify via `npx grunt shell:verify_matching_versions`. Ensure patch version number is supplied for major releases, so `1.2-RC1` should bump to `1.2.0`.
 1. Do `npm run build` and install the `amp.zip` onto a normal WordPress install running a stable release build; do smoke test to ensure it works.
 1. Optionally do sanity check by comparing the `build` directory with the previously-deployed plugin on WordPress.org for example: `svn export https://plugins.svn.wordpress.org/amp/trunk /tmp/amp-trunk; diff /tmp/amp-trunk/ ./build/` (instead of straight `diff`, it's best to use a GUI like `idea diff`, `phpstorm diff`, or `opendiff`).

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

@@ -5,6 +5,9 @@
  * @package AMP
  */
 
+use Amp\AmpWP\Component\DOMElementList;
+use Amp\AmpWP\Component\Carousel;
+
 /**
  * Class AMP_Gallery_Embed_Handler
  *
@@ -129,25 +132,14 @@ public function shortcode( $attr ) {
 				'width'  => $width,
 				'height' => $height,
 				'alt'    => trim( wp_strip_all_tags( get_post_meta( $attachment_id, '_wp_attachment_image_alt', true ) ) ), // Logic from wp_get_attachment_image().
+				'id'     => $attachment_id,
 			];
 		}
 
 		$args = [
-			'images' => $urls,
+			'images'   => $urls,
+			'lightbox' => ! empty( $atts['lightbox'] ),
 		];
-		if ( ! empty( $atts['lightbox'] ) ) {
-			$args['lightbox'] = true;
-			$lightbox_tag     = AMP_HTML_Utils::build_tag(
-				'amp-image-lightbox',
-				[
-					'id'                           => AMP_Base_Sanitizer::AMP_IMAGE_LIGHTBOX_ID,
-					'layout'                       => 'nodisplay',
-					'data-close-button-aria-label' => __( 'Close', 'amp' ),
-				]
-			);
-			/* We need to add lightbox tag, too. @todo Could there be a better alternative for this? */
-			return $this->render( $args ) . $lightbox_tag;
-		}
 
 		return $this->render( $args );
 	}
@@ -211,6 +203,7 @@ public function maybe_override_gallery( $html, $attributes ) {
 	 * @return string Rendered.
 	 */
 	public function render( $args ) {
+		$dom                        = new DOMDocument();
 		$this->did_convert_elements = true;
 
 		$args = wp_parse_args(
@@ -224,11 +217,7 @@ public function render( $args ) {
 			return '';
 		}
 
-		$max_aspect_ratio = 0;
-		$carousel_width   = 0;
-		$carousel_height  = 0;
-
-		$images = [];
+		$images = new DOMElementList();
 		foreach ( $args['images'] as $props ) {
 			$image_atts = [
 				'src'    => $props['url'],
@@ -241,47 +230,38 @@ public function render( $args ) {
 				$image_atts['srcset'] = $props['srcset'];
 			}
 
-			$this_aspect_ratio = $props['width'] / $props['height'];
-			if ( $this_aspect_ratio > $max_aspect_ratio ) {
-				$max_aspect_ratio = $this_aspect_ratio;
-				$carousel_width   = $props['width'];
-				$carousel_height  = $props['height'];
-			}
-
 			if ( ! empty( $args['lightbox'] ) ) {
 				$image_atts['lightbox'] = '';
-				$image_atts['on']       = 'tap:' . AMP_Img_Sanitizer::AMP_IMAGE_LIGHTBOX_ID;
-				$image_atts['role']     = 'button';
-				$image_atts['tabindex'] = 0;
 			}
-			$image = AMP_HTML_Utils::build_tag(
+			$image = AMP_DOM_Utils::create_node(
+				$dom,
 				'amp-img',
 				$image_atts
 			);
 
 			if ( ! empty( $props['href'] ) ) {
-				$image = AMP_HTML_Utils::build_tag(
+				$previous_image = $image;
+				$image          = AMP_DOM_Utils::create_node(
+					$dom,
 					'a',
 					[
 						'href' => $props['href'],
-					],
-					$image
+					]
 				);
+				$image->appendChild( $previous_image );
 			}
 
-			$images[] = $image;
+			$caption = isset( $props['id'] ) ? wp_get_attachment_caption( $props['id'] ) : '';
+			$images  = $images->add( $image, $caption );
 		}
 
-		return AMP_HTML_Utils::build_tag(
-			'amp-carousel',
-			[
-				'width'  => $carousel_width,
-				'height' => $carousel_height,
-				'type'   => 'slides',
-				'layout' => 'responsive',
-			],
-			implode( PHP_EOL, $images )
-		);
+		$amp_carousel  = new Carousel( $dom, $images );
+		$carousel_node = $amp_carousel->get_dom_element();
+
+		// Prevent an error in get_content_from_dom_node() when it calls $node->parentNode->insertBefore().
+		$dom->appendChild( $carousel_node );
+
+		return AMP_DOM_Utils::get_content_from_dom_node( $dom, $carousel_node );
 	}
 
 	/**

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

@@ -5,31 +5,16 @@
  * @package AMP
  */
 
+use Amp\AmpWP\Component\DOMElementList;
+use Amp\AmpWP\Component\Carousel;
+
 /**
  * Class AMP_Gallery_Block_Sanitizer
  *
  * Modifies gallery block to match the block's AMP-specific configuration.
  */
 class AMP_Gallery_Block_Sanitizer extends AMP_Base_Sanitizer {
 
-	/**
-	 * Value used for width of amp-carousel.
-	 *
-	 * @since 1.0
-	 *
-	 * @const int
-	 */
-	const FALLBACK_WIDTH = 600;
-
-	/**
-	 * Value used for height of amp-carousel.
-	 *
-	 * @since 1.0
-	 *
-	 * @const int
-	 */
-	const FALLBACK_HEIGHT = 480;
-
 	/**
 	 * Tag.
 	 *
@@ -93,12 +78,6 @@ public function sanitize() {
 		}
 
 		foreach ( $nodes as $node ) {
-			/**
-			 * Element
-			 *
-			 * @var DOMElement $node
-			 */
-
 			// In WordPress 5.3, the Gallery block's <ul> is wrapped in a <figure class="wp-block-gallery">, so look for that node also.
 			$gallery_node = isset( $node->parentNode ) && AMP_DOM_Utils::has_class( $node->parentNode, self::$class ) ? $node->parentNode : $node;
 			$attributes   = AMP_DOM_Utils::get_node_attributes_as_assoc_array( $gallery_node );
@@ -128,128 +107,33 @@ public function sanitize() {
 				continue;
 			}
 
-			$images = [];
+			$images = new DOMElementList();
 
 			// If it's not AMP lightbox, look for links first.
 			if ( ! $is_amp_lightbox ) {
 				foreach ( $node->getElementsByTagName( 'a' ) as $element ) {
-					$images[] = $element;
+					$images = $images->add( $element, $this->possibly_get_caption_text( $element ) );
 				}
 			}
 
 			// If not linking to anything then look for <amp-img>.
-			if ( empty( $images ) ) {
+			if ( 0 === count( $images ) ) {
 				foreach ( $node->getElementsByTagName( 'amp-img' ) as $element ) {
-					$images[] = $element;
+					$images = $images->add( $element, $this->possibly_get_caption_text( $element ) );
 				}
 			}
 
 			// Skip if no images found.
-			if ( empty( $images ) ) {
+			if ( 0 === count( $images ) ) {
 				continue;
 			}
 
-			list( $width, $height ) = $this->get_carousel_dimensions( $node );
-
-			$amp_carousel = AMP_DOM_Utils::create_node(
-				$this->dom,
-				'amp-carousel',
-				[
-					'width'  => $width,
-					'height' => $height,
-					'type'   => 'slides',
-					'layout' => 'responsive',
-				]
-			);
-
-			foreach ( $images as $image ) {
-				$slide = AMP_DOM_Utils::create_node(
-					$this->dom,
-					'div',
-					[ 'class' => 'slide' ]
-				);
-
-				// Ensure the image fills the entire <amp-carousel>, so the possible caption looks right.
-				if ( 'amp-img' === $image->tagName ) {
-					$image->setAttribute( 'layout', 'fill' );
-					$image->setAttribute( 'object-fit', 'cover' );
-				} elseif ( isset( $image->firstChild->tagName ) && 'amp-img' === $image->firstChild->tagName ) {
-					// If the <amp-img> is wrapped in an <a>.
-					$image->firstChild->setAttribute( 'layout', 'fill' );
-					$image->firstChild->setAttribute( 'object-fit', 'cover' );
-				}
-
-				$possible_caption_text = $this->possibly_get_caption_text( $image );
-				$slide->appendChild( $image );
-
-				// Wrap the caption in a <div> and <span>, and append it to the slide.
-				if ( $possible_caption_text ) {
-					$caption_wrapper = AMP_DOM_Utils::create_node(
-						$this->dom,
-						'div',
-						[ 'class' => 'amp-wp-gallery-caption' ]
-					);
-					$caption_span    = AMP_DOM_Utils::create_node( $this->dom, 'span', [] );
-					$text_node       = $this->dom->createTextNode( $possible_caption_text );
-
-					$caption_span->appendChild( $text_node );
-					$caption_wrapper->appendChild( $caption_span );
-					$slide->appendChild( $caption_wrapper );
-				}
-
-				$amp_carousel->appendChild( $slide );
-			}
-
-			$gallery_node->parentNode->replaceChild( $amp_carousel, $gallery_node );
+			$amp_carousel = new Carousel( $this->dom, $images );
+			$gallery_node->parentNode->replaceChild( $amp_carousel->get_dom_element(), $gallery_node );
 		}
 		$this->did_convert_elements = true;
 	}
 
-	/**
-	 * Get carousel height by containing images.
-	 *
-	 * @param DOMElement $element The UL element.
-	 * @return array {
-	 *     Dimensions.
-	 *
-	 *     @type int $width  Width.
-	 *     @type int $height Height.
-	 * }
-	 */
-	protected function get_carousel_dimensions( $element ) {
-		/**
-		 * Elements.
-		 *
-		 * @var DOMElement $image
-		 */
-		$images     = $element->getElementsByTagName( 'amp-img' );
-		$num_images = $images->length;
-
-		$max_aspect_ratio = 0;
-		$carousel_width   = 0;
-		$carousel_height  = 0;
-
-		if ( 0 === $num_images ) {
-			return [ self::FALLBACK_WIDTH, self::FALLBACK_HEIGHT ];
-		}
-		foreach ( $images as $image ) {
-			if ( ! is_numeric( $image->getAttribute( 'width' ) ) || ! is_numeric( $image->getAttribute( 'height' ) ) ) {
-				continue;
-			}
-			$width  = (float) $image->getAttribute( 'width' );
-			$height = (float) $image->getAttribute( 'height' );
-
-			$this_aspect_ratio = $width / $height;
-			if ( $this_aspect_ratio > $max_aspect_ratio ) {
-				$max_aspect_ratio = $this_aspect_ratio;
-				$carousel_width   = $width;
-				$carousel_height  = $height;
-			}
-		}
-
-		return [ $carousel_width, $carousel_height ];
-	}
-
 	/**
 	 * Set lightbox related attributes to <amp-img> within gallery.
 	 *
@@ -280,7 +164,7 @@ protected function add_lightbox_attributes_to_image_nodes( $element ) {
 	 * Gets the caption of an image, if it exists.
 	 *
 	 * @param DOMElement $element The element for which to search for a caption.
-	 * @return string|null The caption for the image, or null.
+	 * @return string The caption for the image, or ''.
 	 */
 	public function possibly_get_caption_text( $element ) {
 		$caption_tag = 'figcaption';
@@ -293,6 +177,6 @@ public function possibly_get_caption_text( $element ) {
 			return $element->parentNode->nextSibling->textContent;
 		}
 
-		return null;
+		return '';
 	}
 }

phpcs.xml

@@ -22,6 +22,7 @@
 
 	<rule ref="WordPress.Files.FileName.InvalidClassFileName">
 		<exclude-pattern>tests/*</exclude-pattern>
+		<exclude-pattern>src/*</exclude-pattern>
 		<exclude-pattern>includes/admin/class-amp-customizer.php</exclude-pattern>
 		<exclude-pattern>includes/embeds/class-amp-dailymotion-embed.php</exclude-pattern>
 		<exclude-pattern>includes/embeds/class-amp-facebook-embed.php</exclude-pattern>
@@ -46,6 +47,7 @@
 		<exclude-pattern>includes/actions/class-amp-paired-post-actions.php</exclude-pattern>
 	</rule>
 	<rule ref="WordPress.Files.FileName.NotHyphenatedLowercase">
+		<exclude-pattern>src/*</exclude-pattern>
 		<exclude-pattern>includes/templates/single-amp_story.php</exclude-pattern>
 	</rule>
 	<rule ref="WordPress.NamingConventions.ValidVariableName">
@@ -79,6 +81,11 @@
 		</properties>
 	</rule>
 
+	<!-- Enforce PascalCase file names in src/ only. -->
+	<rule ref="Squiz.Classes.ClassFileName">
+		<include-pattern>src/*</include-pattern>
+	</rule>
+
 	<!-- Include sniffs for PHP cross-version compatibility. -->
 	<config name="testVersion" value="5.4-"/>
 	<rule ref="PHPCompatibilityWP">