Skip to content

Commit

Permalink
Merge pull request #1627 from Automattic/fix/1604-issues
Browse files Browse the repository at this point in the history
Address issues from testing with WordPress 5.0
  • Loading branch information
westonruter committed Nov 20, 2018
2 parents 7b45933 + 6b7d305 commit 53a4fe4
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 5 deletions.
31 changes: 31 additions & 0 deletions includes/embeds/class-amp-gallery-embed.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ class AMP_Gallery_Embed_Handler extends AMP_Base_Embed_Handler {
*/
public function register_embed() {
add_filter( 'post_gallery', array( $this, 'maybe_override_gallery' ), 10, 2 );
add_action( 'wp_print_styles', array( $this, 'print_styles' ) );
}

/**
Expand Down Expand Up @@ -160,6 +161,15 @@ public function maybe_override_gallery( $html, $attributes ) {
}

return $html;
} elseif ( isset( $attributes['size'] ) && 'thumbnail' === $attributes['size'] ) {
/*
* If the 'gallery' shortcode has a 'size' attribute of 'thumbnail', prevent outputting an <amp-carousel>.
* That will often get thumbnail images around 150 x 150,
* while the <amp-carousel> usually has a width of 600 and a height of 480.
* That often means very low-resolution images.
* So fall back to the normal 'gallery' shortcode callback, gallery_shortcode().
*/
return '';
}
return $this->shortcode( $attributes );
}
Expand Down Expand Up @@ -224,4 +234,25 @@ public function render( $args ) {
implode( PHP_EOL, $images )
);
}

/**
* Prints the Gallery block styling.
*
* It would be better to print this in AMP_Gallery_Block_Sanitizer,
* but by the time that runs, it's too late.
* This rule is copied exactly from block-library/style.css, but the selector here has amp-img >.
* The image sanitizer normally converts the <img> from that original stylesheet <amp-img>,
* but that doesn't have the same effect as applying it to the <img>.
*
* @return void
*/
public function print_styles() {
?>
<style>
.wp-block-gallery.is-cropped .blocks-gallery-item amp-img > img {
object-fit: cover;
}
</style>
<?php
}
}
54 changes: 51 additions & 3 deletions includes/sanitizers/class-amp-core-theme-sanitizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -53,17 +53,20 @@ class AMP_Core_Theme_Sanitizer extends AMP_Base_Sanitizer {
protected static $theme_features = array(
// Twenty Nineteen.
'twentynineteen' => array(
'dequeue_scripts' => array(
'dequeue_scripts' => array(
'twentynineteen-skip-link-focus-fix', // This is part of AMP. See <https://github.com/ampproject/amphtml/issues/18671>.
'twentynineteen-priority-menu',
'twentynineteen-touch-navigation', // @todo There could be an AMP implementation of this, similar to what is implemented on ampproject.org.
),
'remove_actions' => array(
'remove_actions' => array(
'wp_print_footer_scripts' => array(
'twentynineteen_skip_link_focus_fix', // See <https://github.com/WordPress/twentynineteen/pull/47>.
),
),
'add_twentynineteen_masthead_styles' => array(),
'add_twentynineteen_masthead_styles' => array(),
'add_twentynineteen_image_styles' => array(),
'remove_twentynineteen_thumbnail_image_sizes' => array(),

),

// Twenty Seventeen.
Expand Down Expand Up @@ -341,6 +344,26 @@ public static function set_twentyseventeen_quotes_icon() {
} );
}

/**
* Remove the sizes attribute from thumbnail images in Twenty Nineteen.
*
* The AMP runtime sets an inline style on an <amp-img> based on the sizes attribute if it's present.
* For example, <amp-img style="width:calc(50vw)">.
* Removing the 'sizes' attribute isn't ideal, but it looks like it's not possible to override that inline style.
*
* @todo: remove when this is resolved: https://github.com/ampproject/amphtml/issues/17053
* @since 1.0
*/
public static function remove_twentynineteen_thumbnail_image_sizes() {
add_filter( 'wp_get_attachment_image_attributes', function( $attr ) {
if ( isset( $attr['class'] ) && false !== strpos( $attr['class'], 'attachment-post-thumbnail' ) ) {
unset( $attr['sizes'] );
}

return $attr;
}, 11 );
}

/**
* Add filter to adjust the attachment image attributes to ensure attachment pages have a consistent <amp-img> rendering.
*
Expand Down Expand Up @@ -1056,4 +1079,29 @@ public static function add_nav_sub_menu_buttons( $args = array() ) {
return $item_output;
}, 10, 4 );
}

/**
* Output image styles for twentynineteen.
*
* When <img> tags have an 'aligncenter' class, AMP_Img_Sanitizer::handle_centering() wraps theme in <figure class="aligncenter">.
* This ensures that the image inside it is centered.
*
* @since 1.0
*
* @param array $args Arguments.
*/
public static function add_twentynineteen_image_styles( $args = array() ) {
add_action( 'wp_enqueue_scripts', function() use ( $args ) {
ob_start();
?>
<style>
figure.aligncenter {
text-align: center
}
</style>
<?php
$styles = str_replace( array( '<style>', '</style>' ), '', ob_get_clean() );
wp_add_inline_style( get_template() . '-style', $styles );
}, 11 );
}
}
2 changes: 1 addition & 1 deletion readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ Enable Accelerated Mobile Pages (AMP) on your WordPress site.
**Contributors:** [batmoo](https://profiles.wordpress.org/batmoo), [joen](https://profiles.wordpress.org/joen), [automattic](https://profiles.wordpress.org/automattic), [potatomaster](https://profiles.wordpress.org/potatomaster), [albertomedina](https://profiles.wordpress.org/albertomedina), [google](https://profiles.wordpress.org/google), [xwp](https://profiles.wordpress.org/xwp), [westonruter](https://profiles.wordpress.org/westonruter)
**Tags:** [amp](https://wordpress.org/plugins/tags/amp), [mobile](https://wordpress.org/plugins/tags/mobile)
**Requires at least:** 4.7
**Tested up to:** 4.9
**Tested up to:** 5.0
**Stable tag:** 0.7.2
**License:** [GPLv2 or later](http://www.gnu.org/licenses/gpl-2.0.html)
**Requires PHP:** 5.3.6
Expand Down
2 changes: 1 addition & 1 deletion readme.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
Contributors: batmoo, joen, automattic, potatomaster, albertomedina, google, xwp, westonruter
Tags: amp, mobile
Requires at least: 4.7
Tested up to: 4.9
Tested up to: 5.0
Stable tag: 0.7.2
License: GPLv2 or later
License URI: http://www.gnu.org/licenses/gpl-2.0.html
Expand Down

0 comments on commit 53a4fe4

Please sign in to comment.