Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Address issues from testing with WordPress 5.0 #1627

Merged
merged 7 commits into from
Nov 20, 2018
Merged
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