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

Use data-hero-candidate attribute in DetermineHeroImages #5934

Merged
merged 25 commits into from
Mar 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
8b84aaa
Switch to data-hero-candidate branch for amp-toolbox
schlessera Feb 27, 2021
d727a36
Use data-hero-candidate attribute in DetermineHeroImages transformer
schlessera Feb 27, 2021
77d5f99
Refactor transformer to get rid of code duplication
schlessera Feb 27, 2021
c490443
Adapt tests
schlessera Feb 27, 2021
48b9524
Update Composer lock file - 2020-03-08
schlessera Mar 8, 2021
b126bc3
Update amp-toolbox-php dependency to v0.2.0
schlessera Mar 18, 2021
b685b0e
Use switch instead of variable methods
schlessera Mar 18, 2021
7d96d8a
Improve XPath to target custom header images
schlessera Mar 18, 2021
cdaa8c4
Use new Encoding::AMP constant
schlessera Mar 18, 2021
223b008
Ignore PHPStan error for PHP7+ isBuiltIn() method
schlessera Mar 18, 2021
3d6ff8b
Merge branch 'fix/5824-use-data-hero-candidate-attribute' of https://…
schlessera Mar 18, 2021
e13d93d
Remove DetermineHeroImages from default transformers
schlessera Mar 18, 2021
1d99a1e
Revert "Ignore PHPStan error for PHP7+ isBuiltIn() method"
schlessera Mar 18, 2021
02d1b90
Use centralized default options for Dom\Document
schlessera Mar 18, 2021
68b961c
Keep matching the custom header class
schlessera Mar 18, 2021
025a6f0
Change the way the amp carousel test is set up
schlessera Mar 18, 2021
b5e45f1
Fix assignment alignment for phpcs; add phpdoc type for static analysis
westonruter Mar 19, 2021
cadc98f
Fix detecting hero image candidate in Cover Block
westonruter Mar 19, 2021
2cac676
Only consider cover block in initial position of first entry content
westonruter Mar 19, 2021
834cca5
Test detection of cover block inside initial group block
westonruter Mar 19, 2021
e42f1b3
Add prerendering for image blocks in initial position if first entry …
westonruter Mar 19, 2021
8513aba
Enable DetermineHeroImages transformer by default in core themes
westonruter Mar 19, 2021
25ec074
Remove obsolete logic for candidates as array
westonruter Mar 19, 2021
65b0e05
Add covers annotations for DetermineHeroImages tests
schlessera Mar 19, 2021
bcb50e5
Use constant for config key
schlessera Mar 19, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
"ext-json": "*",
"ext-libxml": "*",
"ext-spl": "*",
"ampproject/amp-toolbox": "^0.1",
"ampproject/amp-toolbox": "^0.2",
"cweagans/composer-patches": "1.7.0",
"fasterimage/fasterimage": "1.5.0",
"sabberworm/php-css-parser": "dev-master#bfdd976"
Expand Down
138 changes: 70 additions & 68 deletions composer.lock

Large diffs are not rendered by default.

5 changes: 2 additions & 3 deletions includes/class-amp-theme-support.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

use AmpProject\Amp;
use AmpProject\AmpWP\Dom\Options;
use AmpProject\AmpWP\ExtraThemeAndPluginHeaders;
use AmpProject\AmpWP\Option;
use AmpProject\AmpWP\QueryVar;
Expand Down Expand Up @@ -1867,7 +1868,7 @@ public static function prepare_response( $response, $args = [] ) {
*/
do_action( 'amp_server_timing_start', 'amp_dom_parse', '', [], true );

$dom = Document::fromHtml( $response );
$dom = Document::fromHtml( $response, Options::DEFAULTS );

if ( AMP_Validation_Manager::$is_validate_request ) {
AMP_Validation_Manager::remove_illegal_source_stack_comments( $dom );
Expand Down Expand Up @@ -2117,8 +2118,6 @@ private static function get_optimizer_configuration( $args ) {
Optimizer\Transformer\TransformedIdentifier::class,
]
);
} else {
array_unshift( $transformers, Transformer\DetermineHeroImages::class );
}

array_unshift( $transformers, Transformer\AmpSchemaOrgMetadata::class );
Expand Down
35 changes: 34 additions & 1 deletion includes/sanitizers/class-amp-core-theme-sanitizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@
* @since 1.0
*/

use AmpProject\AmpWP\Transformer\DetermineHeroImages;
use AmpProject\Attribute;
use AmpProject\Dom\Document;
use AmpProject\Optimizer\Configuration;
use AmpProject\Role;

/**
Expand Down Expand Up @@ -99,6 +101,7 @@ protected static function get_theme_features_config( $theme_slug ) {
'amend_twentytwentyone_sub_menu_toggles' => [],
'add_twentytwentyone_mobile_modal' => [],
'add_twentytwentyone_sub_menu_fix' => [],
'enable_determine_hero_images_transformer' => [],
];

// Dark mode button toggle is only supported in the Customizer for now.
Expand Down Expand Up @@ -139,6 +142,7 @@ protected static function get_theme_features_config( $theme_slug ) {
'add_img_display_block_fix' => [],
'add_twentytwenty_custom_logo_fix' => [],
'add_twentytwenty_current_page_awareness' => [],
'enable_determine_hero_images_transformer' => [],
];

$theme = wp_get_theme( 'twentytwenty' );
Expand Down Expand Up @@ -166,6 +170,7 @@ protected static function get_theme_features_config( $theme_slug ) {
],
'add_twentynineteen_masthead_styles' => [],
'adjust_twentynineteen_images' => [],
'enable_determine_hero_images_transformer' => [],
];

// Twenty Seventeen.
Expand Down Expand Up @@ -201,6 +206,7 @@ protected static function get_theme_features_config( $theme_slug ) {
],
'set_twentyseventeen_quotes_icon' => [],
'add_twentyseventeen_attachment_image_attributes' => [],
'enable_determine_hero_images_transformer' => [],
];

// Twenty Sixteen.
Expand All @@ -223,6 +229,7 @@ protected static function get_theme_features_config( $theme_slug ) {
'sub_menu_button_toggle_class' => 'toggled-on',
'no_js_submenu_visible' => true,
],
'enable_determine_hero_images_transformer' => [],
];

// Twenty Fifteen.
Expand All @@ -243,6 +250,7 @@ protected static function get_theme_features_config( $theme_slug ) {
'sub_menu_button_toggle_class' => 'toggle-on',
'no_js_submenu_visible' => true,
],
'enable_determine_hero_images_transformer' => [],
];

// Twenty Fourteen.
Expand All @@ -259,6 +267,7 @@ protected static function get_theme_features_config( $theme_slug ) {
'add_twentyfourteen_masthead_styles' => [],
'add_twentyfourteen_slider_carousel' => [],
'add_twentyfourteen_search' => [],
'enable_determine_hero_images_transformer' => [],
];

// Twenty Thirteen.
Expand All @@ -271,6 +280,7 @@ protected static function get_theme_features_config( $theme_slug ) {
'add_nav_menu_toggle' => [],
'add_nav_sub_menu_buttons' => [],
'add_nav_menu_styles' => [],
'enable_determine_hero_images_transformer' => [],
];

// Twenty Twelve.
Expand All @@ -280,6 +290,7 @@ protected static function get_theme_features_config( $theme_slug ) {
'twentytwelve-navigation',
],
'add_nav_menu_styles' => [],
'enable_determine_hero_images_transformer' => [],
];

// Twenty Eleven.
Expand All @@ -289,11 +300,14 @@ protected static function get_theme_features_config( $theme_slug ) {
'//style[ @id = "twentyeleven-header-css" ]',
'//link[ @id = "dark-css" ]',
],
'enable_determine_hero_images_transformer' => [],
];

// Twenty Ten.
case 'twentyten':
return [];
return [
'enable_determine_hero_images_transformer' => [],
];

default:
return null;
Expand Down Expand Up @@ -533,6 +547,25 @@ public static function add_buffering_hooks( $args = [] ) {
}
}

/**
* Enable transformer that identifies hero image candidates for prerendering.
*
* Note that this transformer will likely be enabled by default in the future. It is only enabled by default for
* core themes in the immediate term since it is known to work well with this set of themes.
*
* @since 2.1
*/
public static function enable_determine_hero_images_transformer() {
add_filter(
'amp_optimizer_config',
static function ( $config ) {
array_unshift( $config[ Configuration::KEY_TRANSFORMERS ], DetermineHeroImages::class );
return $config;
},
9
);
}

/**
* Add filter to output the quote icons in front of the article content.
*
Expand Down
2 changes: 1 addition & 1 deletion includes/sanitizers/class-amp-meta-sanitizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ protected function create_charset_element() {
$this->dom,
Tag::META,
[
Attribute::CHARSET => Document::AMP_ENCODING,
Attribute::CHARSET => Document\Encoding::AMP,
]
);
}
Expand Down
20 changes: 16 additions & 4 deletions includes/utils/class-amp-dom-utils.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
* @package AMP
*/

use AmpProject\AmpWP\Dom\Options;
use AmpProject\Dom\Document;
use AmpProject\Dom\Element;
use AmpProject\Tag;

/**
Expand Down Expand Up @@ -65,7 +67,14 @@ class AMP_DOM_Utils {
*/
public static function get_dom( $document, $encoding = null ) {
_deprecated_function( __METHOD__, '1.5.0', 'AmpProject\Dom\Document::fromHtml()' );
return Document::fromHtml( $document, $encoding );

$options = Options::DEFAULTS;

if ( null !== $encoding ) {
$options[ Document\Option::ENCODING ] = $encoding;
}

return Document::fromHtml( $document, $options );
}

/**
Expand Down Expand Up @@ -172,7 +181,10 @@ public static function get_dom_from_content( $content, $encoding = null ) {
*/
$document = "<html><head></head><body>{$content}</body></html>";

return Document::fromHtml( $document, $encoding );
$options = Options::DEFAULTS;
$options[ Document\Option::ENCODING ] = $encoding;

return Document::fromHtml( $document, $options );
}

/**
Expand Down Expand Up @@ -375,8 +387,8 @@ public static function has_class( DOMElement $element, $class ) {
*
* @deprecated Use AmpProject\Dom\Document::getElementId() instead.
*
* @param DOMElement $element Element to get the ID for.
* @param string $prefix Optional. Defaults to 'amp-wp-id'.
* @param DOMElement|Element $element Element to get the ID for.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than having to do this for static analysis, the getElementId method could indicate the $element is a DOMElement or an Element.

* @param string $prefix Optional. Defaults to 'amp-wp-id'.
* @return string ID to use.
*/
public static function get_element_id( $element, $prefix = 'amp-wp-id' ) {
Expand Down
22 changes: 22 additions & 0 deletions src/Dom/Options.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<?php
/**
* Class Options.
*
* @package AmpProject\AmpWP
*/

namespace AmpProject\AmpWP\Dom;

use AmpProject\Dom\Document;

interface Options {

/**
* Default options to use for the Dom.
*
* @var array
*/
const DEFAULTS = [
Document\Option::AMP_BIND_SYNTAX => Document\Option::AMP_BIND_SYNTAX_DATA_ATTRIBUTE,
];
}
Loading