Skip to content

Commit

Permalink
Merge pull request #2505 from ampproject/fix/gallery-carousel-layout
Browse files Browse the repository at this point in the history 
Fix layout of Gallery as amp-carousel
  • Loading branch information
@westonruter
westonruter authored Jun 5, 2019
2 parents b22a32d + 6c885b0 commit e1c78fb
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 18 deletions.
9 changes: 7 additions & 2 deletions includes/embeds/class-amp-gallery-embed.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,9 @@ public function render( $args ) {
return '';
}

$max_width = 0;
$max_height = 0;

$images = array();
foreach ( $args['images'] as $props ) {
$image_atts = array(
Expand All @@ -212,6 +215,8 @@ public function render( $args ) {
'height' => $props['height'],
'layout' => 'responsive',
);
$max_width = max( $max_width, $props['width'] );
$max_height = max( $max_height, $props['height'] );
if ( ! empty( $args['lightbox'] ) ) {
$image_atts['lightbox'] = '';
$image_atts['on'] = 'tap:' . AMP_Img_Sanitizer::AMP_IMAGE_LIGHTBOX_ID;
Expand Down Expand Up @@ -239,8 +244,8 @@ public function render( $args ) {
return AMP_HTML_Utils::build_tag(
'amp-carousel',
array(
'width' => $this->args['width'],
'height' => $this->args['height'],
'width' => $max_width,
'height' => $max_height,
'type' => 'slides',
'layout' => 'responsive',
),
Expand Down
26 changes: 15 additions & 11 deletions includes/sanitizers/class-amp-gallery-block-sanitizer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,13 +128,16 @@ public function sanitize() {
continue;
}

list( $width, $height ) = $this->get_carousel_dimensions( $node );

$amp_carousel = AMP_DOM_Utils::create_node(
$this->dom,
'amp-carousel',
array(
'height' => $this->get_carousel_height( $node ),
'width' => $width,
'height' => $height,
'type' => 'slides',
'layout' => 'fixed-height',
'layout' => 'responsive',
)
);
foreach ( $images as $image ) {
Expand All @@ -150,15 +153,20 @@ public function sanitize() {
* Get carousel height by containing images.
*
* @param DOMElement $element The UL element.
* @return int Height.
* @return array {
* Dimensions.
*
* @type int $width Width.
* @type int $height Height.
* }
*/
protected function get_carousel_height( $element ) {
protected function get_carousel_dimensions( $element ) {
$images = $element->getElementsByTagName( 'amp-img' );
$num_images = $images->length;
$max_height = 0;
$max_width = 0;
if ( 0 === $num_images ) {
return self::FALLBACK_HEIGHT;
return array( self::FALLBACK_WIDTH, self::FALLBACK_HEIGHT );
}
foreach ( $images as $image ) {
/**
Expand All @@ -170,17 +178,13 @@ protected function get_carousel_height( $element ) {
if ( is_numeric( $image_height ) ) {
$max_height = max( $max_height, $image_height );
}
$image_width = $image->getAttribute( 'height' );
$image_width = $image->getAttribute( 'width' );
if ( is_numeric( $image_width ) ) {
$max_width = max( $max_width, $image_width );
}
}

if ( ! empty( $this->args['content_max_width'] ) && $max_height > 0 && $max_width > $this->args['content_max_width'] ) {
$max_height = ( $max_width * $this->args['content_max_width'] ) / $max_height;
}

return ! $max_height ? self::FALLBACK_HEIGHT : $max_height;
return array( $max_width, $max_height );
}

/**
Expand Down
10 changes: 5 additions & 5 deletions tests/test-class-amp-gallery-block-sanitizer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public function get_data() {

'data_amp_with_carousel' => array(
'<ul class="wp-block-gallery" data-amp-carousel="true"><li class="blocks-gallery-item"><figure><a href="http://example.com"><amp-img src="http://example.com/img.png" width="600" height="400"></amp-img></a></figure></li></ul>',
'<amp-carousel height="400" type="slides" layout="fixed-height"><a href="http://example.com"><amp-img src="http://example.com/img.png" width="600" height="400"></amp-img></a></amp-carousel>',
'<amp-carousel width="600" height="400" type="slides" layout="responsive"><a href="http://example.com"><amp-img src="http://example.com/img.png" width="600" height="400"></amp-img></a></amp-carousel>',
),

'data_amp_with_lightbox' => array(
Expand All @@ -48,8 +48,8 @@ public function get_data() {
),

'data_amp_with_lightbox_and_carousel' => array(
'<ul class="wp-block-gallery" data-amp-lightbox="true" data-amp-carousel="true"><li class="blocks-gallery-item"><figure><a href="http://example.com"><amp-img src="http://example.com/img.png" width="600" height="400"></amp-img></a></figure></li></ul>',
'<amp-carousel height="400" type="slides" layout="fixed-height"><amp-img src="http://example.com/img.png" width="600" height="400" data-amp-lightbox="" on="tap:amp-image-lightbox" role="button" tabindex="0"></amp-img></amp-carousel><amp-image-lightbox id="amp-image-lightbox" layout="nodisplay" data-close-button-aria-label="Close"></amp-image-lightbox>',
'<ul class="wp-block-gallery" data-amp-lightbox="true" data-amp-carousel="true"><li class="blocks-gallery-item"><figure><a href="http://example.com"><amp-img src="http://example.com/img.png" width="1234" height="567"></amp-img></a></figure></li></ul>',
'<amp-carousel width="1234" height="567" type="slides" layout="responsive"><amp-img src="http://example.com/img.png" width="1234" height="567" data-amp-lightbox="" on="tap:amp-image-lightbox" role="button" tabindex="0"></amp-img></amp-carousel><amp-image-lightbox id="amp-image-lightbox" layout="nodisplay" data-close-button-aria-label="Close"></amp-image-lightbox>',
),
);
}
Expand Down Expand Up @@ -93,12 +93,12 @@ public function get_reader_mode_data() {

'data_amp_with_lightbox' => array(
'<ul class="wp-block-gallery" data-amp-lightbox="true"><li class="blocks-gallery-item"><figure><a href="http://example.com"><amp-img src="http://example.com/img.png" width="600" height="400"></amp-img></a></figure></li></ul>',
'<amp-carousel height="400" type="slides" layout="fixed-height"><amp-img src="http://example.com/img.png" width="600" height="400" data-amp-lightbox="" on="tap:amp-image-lightbox" role="button" tabindex="0"></amp-img></amp-carousel><amp-image-lightbox id="amp-image-lightbox" layout="nodisplay" data-close-button-aria-label="Close"></amp-image-lightbox>',
'<amp-carousel width="600" height="400" type="slides" layout="responsive"><amp-img src="http://example.com/img.png" width="600" height="400" data-amp-lightbox="" on="tap:amp-image-lightbox" role="button" tabindex="0"></amp-img></amp-carousel><amp-image-lightbox id="amp-image-lightbox" layout="nodisplay" data-close-button-aria-label="Close"></amp-image-lightbox>',
),

'data_amp_with_lightbox_and_carousel' => array(
'<ul class="wp-block-gallery" data-amp-lightbox="true" data-amp-carousel="true"><li class="blocks-gallery-item"><figure><a href="http://example.com"><amp-img src="http://example.com/img.png" width="600" height="400"></amp-img></a></figure></li></ul>',
'<amp-carousel height="400" type="slides" layout="fixed-height"><amp-img src="http://example.com/img.png" width="600" height="400" data-amp-lightbox="" on="tap:amp-image-lightbox" role="button" tabindex="0"></amp-img></amp-carousel><amp-image-lightbox id="amp-image-lightbox" layout="nodisplay" data-close-button-aria-label="Close"></amp-image-lightbox>',
'<amp-carousel width="600" height="400" type="slides" layout="responsive"><amp-img src="http://example.com/img.png" width="600" height="400" data-amp-lightbox="" on="tap:amp-image-lightbox" role="button" tabindex="0"></amp-img></amp-carousel><amp-image-lightbox id="amp-image-lightbox" layout="nodisplay" data-close-button-aria-label="Close"></amp-image-lightbox>',
),
);
}
Expand Down

0 comments on commit e1c78fb

Please sign in to comment.