Skip to content

Commit

Permalink
Merge branch 'amp-stories-redux' of github.com:ampproject/amp-wp into…
Browse files Browse the repository at this point in the history 
… amp-story/1963-call-to-action

* 'amp-stories-redux' of github.com:ampproject/amp-wp:
  Preview style improvements
  Clear select block upon reorder start
  Opt-in to amp-img-auto-sizes experiment when there are converted images
  Remove obsolete add_auto_width_to_figure from #1086
  Add test for sizes attribute being removed from converted img
  Improve aligncenter, alignwide, and alignfull in classic templates
  Rename method to adjust_twentynineteen_images
  Eliminate sizes from amp-img converted from img
  Ensure featured image gets responsive layout in Twenty Nineteen, as it is essentially alignwide
  Give responsive layout to alignwide/alignfull instead of intrinsic layout
  Ensure <img> elements converted to <amp-img> which have layout=intrisinc also get object-fit:contain
  Ensure amp-default style is enqueued before theme's stylesheet
  Eliminate duplicated CSS by including amp-default.css in classic styles
  Eliminate handle_centering for images since apparently unnecessary
  • Loading branch information
@westonruter
westonruter committed Apr 4, 2019
2 parents 256522e + 2afcccd commit 2d8b2e6
Show file tree
Hide file tree
Showing 10 changed files with 92 additions and 231 deletions.
11 changes: 8 additions & 3 deletions assets/css/amp-default.css
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
.amp-wp-unknown-size [src] {
/** Worst case scenario when we can't figure out dimensions for an image. **/
/** Force the image into a box of fixed dimensions and use object-fit to scale. **/

/*
* Prevent cases of amp-img converted from img to appear with stretching by using object-fit to scale.
* See <https://github.com/ampproject/amphtml/issues/21371#issuecomment-475443219>.
* Also use object-fit:contain in worst case scenario when we can't figure out dimensions for an image.
*/
amp-img.amp-wp-enforced-sizes[layout=intrinsic] > img,
.amp-wp-unknown-size > img {
object-fit: contain;
}

Expand Down
7 changes: 6 additions & 1 deletion assets/css/amp-editor-story-blocks.css
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@
}
}

#amp-story-editor {
#amp-story-editor,
.amp-story-page-preview .editor-styles-wrapper {
font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, Oxygen-Sans, Ubuntu, Cantarell, "Helvetica Neue", sans-serif;
}

Expand Down Expand Up @@ -417,6 +418,10 @@ div[data-type="amp/amp-story-page"] .editor-inner-blocks .editor-block-list__lay
margin: 0 10px;
}

.amp-story-reorderer .amp-story-reorderer-item .amp-story-reorderer-item-page .wp-block {
transform: scale(0.48) translateX(-40%) translateY(-40%);
}

.amp-story-reorderer .amp-story-reorderer-item .components-drop-zone {
border: none;
border-radius: 0;
Expand Down
6 changes: 5 additions & 1 deletion assets/src/components/story-controls.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,14 @@ export default compose(
};
} ),
withDispatch( ( dispatch ) => {
const { clearSelectedBlock } = dispatch( 'core/block-editor' );
const { startReordering, saveOrder, resetOrder } = dispatch( 'amp/story' );

return {
startReordering,
startReordering: () => {
clearSelectedBlock();
startReordering();
},
saveOrder,
resetOrder,
};
Expand Down
4 changes: 3 additions & 1 deletion includes/amp-helper-functions.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -793,7 +793,9 @@ function amp_get_content_sanitizers( $post = null ) {
'template' => get_template(),
'stylesheet' => get_stylesheet(),
),
'AMP_Img_Sanitizer' => array(),
'AMP_Img_Sanitizer' => array(
'align_wide_support' => current_theme_supports( 'align-wide' ),
),
'AMP_Form_Sanitizer' => array(),
'AMP_Comments_Sanitizer' => array(
'comments_live_list' => ! empty( $theme_support_args['comments_live_list'] ),
Expand Down
2 changes: 1 addition & 1 deletion includes/class-amp-theme-support.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -913,7 +913,7 @@ function() {
);

add_action( 'wp_head', 'amp_add_generator_metadata', 20 );
add_action( 'wp_enqueue_scripts', array( __CLASS__, 'enqueue_assets' ) );
add_action( 'wp_enqueue_scripts', array( __CLASS__, 'enqueue_assets' ), 0 ); // Enqueue before theme's styles.
add_action( 'wp_enqueue_scripts', array( __CLASS__, 'dequeue_customize_preview_scripts' ), 1000 );
add_filter( 'customize_partial_render', array( __CLASS__, 'filter_customize_partial_render' ) );

Expand Down
100 changes: 17 additions & 83 deletions includes/sanitizers/class-amp-core-theme-sanitizer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,20 +53,18 @@ 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_image_styles' => array(),
'remove_twentynineteen_thumbnail_image_sizes' => array(),

'add_twentynineteen_masthead_styles' => array(),
'adjust_twentynineteen_images' => array(),
),

// Twenty Seventeen.
Expand Down Expand Up @@ -402,30 +400,6 @@ function ( $content ) {
);
}

/**
* 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 All @@ -435,38 +409,6 @@ function( $attr ) {
* @link https://github.com/WordPress/wordpress-develop/blob/ddc8f803c6e99118998191fd2ea24124feb53659/src/wp-content/themes/twentyseventeen/functions.php#L545:L554
*/
public static function add_twentyseventeen_attachment_image_attributes() {
add_filter(
'wp_get_attachment_image_attributes',
function ( $attr, $attachment, $size ) {
if (
isset( $attr['class'] )
&&
(
'custom-logo' === $attr['class']
||
false !== strpos( $attr['class'], 'attachment-twentyseventeen-featured-image' )
)
) {
/*
* 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:100%">.
* Removing the 'sizes' attribute is only a workaround, as 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
*/
unset( $attr['sizes'] );
} elseif ( is_attachment() ) {
$sizes = wp_get_attachment_image_sizes( $attachment->ID, $size );
if ( false !== $sizes ) {
$attr['sizes'] = $sizes;
}
}
return $attr;
},
11,
3
);

/*
* The max-height of the `.custom-logo-link img` is defined as being 80px, unless
* there is header media in which case it is 200px. Issues related to vertically-squashed
Expand Down Expand Up @@ -1082,29 +1024,21 @@ function() use ( $args ) {
}

/**
* 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.
* Adjust images in twentynineteen.
*
* @since 1.0
* @since 1.1
*/
public static function add_twentynineteen_image_styles() {
add_action(
'wp_enqueue_scripts',
function() {
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
public static function adjust_twentynineteen_images() {

// Make sure the featured image gets responsive layout.
add_filter(
'wp_get_attachment_image_attributes',
function( $attributes ) {
if ( preg_match( '/(^|\s)(attachment-post-thumbnail)(\s|$)/', $attributes['class'] ) ) {
$attributes['data-amp-layout'] = 'responsive';
}
return $attributes;
}
);
}
}
102 changes: 25 additions & 77 deletions includes/sanitizers/class-amp-img-sanitizer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,21 @@ public function sanitize() {

$this->determine_dimensions( $need_dimensions );
$this->adjust_and_replace_nodes_in_array_map( $need_dimensions );

/*
* Opt-in to amp-img-auto-sizes experiment.
* This is needed because the sizes attribute is removed from all img elements converted to amp-img
* in order to prevent the undesirable setting of the width. This $meta tag can be removed once the
* experiment ends (and the feature has been fully launched).
* See <https://github.com/ampproject/amphtml/issues/21371> and <https://github.com/ampproject/amp-wp/pull/2036>.
*/
$head = $this->dom->getElementsByTagName( 'head' )->item( 0 );
if ( $head ) {
$meta = $this->dom->createElement( 'meta' );
$meta->setAttribute( 'name', 'amp-experiments-opt-in' );
$meta->setAttribute( 'content', 'amp-img-auto-sizes' );
$head->insertBefore( $meta, $head->firstChild );
}
}

/**
Expand Down Expand Up @@ -249,9 +264,17 @@ private function adjust_and_replace_node( $node ) {

$this->add_or_append_attribute( $new_attributes, 'class', 'amp-wp-enforced-sizes' );
if ( empty( $new_attributes['layout'] ) && ! empty( $new_attributes['height'] ) && ! empty( $new_attributes['width'] ) ) {
$new_attributes['layout'] = 'intrinsic';
// Use responsive images when a theme supports wide and full-bleed images.
if ( ! empty( $this->args['align_wide_support'] ) && $node->parentNode && 'figure' === $node->parentNode->nodeName && preg_match( '/(^|\s)(alignwide|alignfull)(\s|$)/', $node->parentNode->getAttribute( 'class' ) ) ) {
$new_attributes['layout'] = 'responsive';
} else {
$new_attributes['layout'] = 'intrinsic';
}
}

// Remove sizes attribute since it causes headaches in AMP and because AMP will generate it for us. See <https://github.com/ampproject/amphtml/issues/21371>.
unset( $new_attributes['sizes'] );

if ( $this->is_gif_url( $new_attributes['src'] ) ) {
$this->did_convert_elements = true;

Expand All @@ -261,15 +284,12 @@ private function adjust_and_replace_node( $node ) {
}

$img_node = AMP_DOM_Utils::create_node( $this->dom, $new_tag, $new_attributes );
$new_node = $this->handle_centering( $img_node );
$node->parentNode->replaceChild( $new_node, $node );
$node->parentNode->replaceChild( $img_node, $node );

// Preserve original node in noscript for no-JS environments.
$noscript = $this->dom->createElement( 'noscript' );
$noscript->appendChild( $node );
$img_node->appendChild( $noscript );

$this->add_auto_width_to_figure( $new_node );
}

/**
Expand Down Expand Up @@ -313,76 +333,4 @@ private function is_gif_url( $url ) {
$path = wp_parse_url( $url, PHP_URL_PATH );
return substr( $path, -strlen( $ext ) ) === $ext;
}

/**
* Handles an issue with the aligncenter class.
*
* If the <amp-img> has the class aligncenter, this strips the class and wraps it in a <figure> to center the image.
* There was an issue where the aligncenter class overrode the "display: inline-block" rule of AMP's layout="intrinsic" attribute.
* So this strips that class, and instead wraps the image in a <figure> to center it.
*
* @since 0.7
* @see https://github.com/ampproject/amp-wp/issues/1104
*
* @param DOMElement $node The <amp-img> node.
* @return DOMElement $node The <amp-img> node, possibly wrapped in a <figure>.
*/
public function handle_centering( $node ) {
$align_class = 'aligncenter';
$classes = $node->getAttribute( 'class' );
$width = $node->getAttribute( 'width' );

// If this doesn't have a width attribute, centering it in the <figure> wrapper won't work.
if ( empty( $width ) || ! in_array( $align_class, preg_split( '/\s+/', trim( $classes ) ), true ) ) {
return $node;
}

// Strip the class, and wrap the <amp-img> in a <figure>.
$classes = trim( str_replace( $align_class, '', $classes ) );
$node->setAttribute( 'class', $classes );
$figure = AMP_DOM_Utils::create_node(
$this->dom,
'figure',
array(
'class' => $align_class,
'style' => "max-width: {$width}px;",
)
);
$figure->appendChild( $node );

return $figure;
}

/**
* Add an inline style to set the `<figure>` element's width to `auto` instead of `fit-content`.
*
* @since 1.0
* @see https://github.com/ampproject/amp-wp/issues/1086
*
* @param DOMElement $node The DOMNode to adjust and replace.
*/
protected function add_auto_width_to_figure( $node ) {
$figure = $node->parentNode;
if ( ! ( $figure instanceof DOMElement ) || 'figure' !== $figure->tagName ) {
return;
}

$class = $figure->getAttribute( 'class' );
// Target only the <figure> with a 'wp-block-image' class attribute.
if ( false === strpos( $class, 'wp-block-image' ) ) {
return;
}

// Target only <figure> without a 'is-resized' class attribute.
if ( false !== strpos( $class, 'is-resized' ) ) {
return;
}

$new_style = 'width: auto;';
if ( $figure->hasAttribute( 'style' ) ) {
$figure->setAttribute( 'style', $new_style . $figure->getAttribute( 'style' ) );
} else {
$figure->setAttribute( 'style', $new_style );
}
}
}
12 changes: 7 additions & 5 deletions templates/style.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@

.aligncenter {
display: block;
text-align: center;
margin-left: auto;
margin-right: auto;
}
Expand All @@ -44,11 +45,7 @@
margin: 0 auto;
}

.amp-wp-unknown-size img {
/** Worst case scenario when we can't figure out dimensions for an image. **/
/** Force the image into a box of fixed dimensions and use object-fit to scale. **/
object-fit: contain;
}
<?php echo file_get_contents( AMP__DIR__ . '/assets/css/amp-default.css' ); // phpcs:ignore WordPress.WP.AlternativeFunctions ?>

/* Template Styles */

Expand Down Expand Up @@ -284,6 +281,11 @@

/* AMP Media */

.alignwide,
.alignfull {
clear: both;
}

amp-carousel {
background: <?php echo sanitize_hex_color( $border_color ); ?>;
margin: 0 -16px 1.5em;
Expand Down
Loading

0 comments on commit 2d8b2e6

Please sign in to comment.