Skip to content

Commit

Permalink
Merge pull request #6198 from ampproject/fix/reader-theme-customizer-…
Browse files Browse the repository at this point in the history 
…preview-url-performance

Defer obtaining URL for Customizer preview until AMP Customizer is accessed
  • Loading branch information
@westonruter
westonruter authored May 13, 2021
2 parents 3111d98 + 5cc5658 commit 1c7728c
Show file tree
Hide file tree
Showing 7 changed files with 290 additions and 38 deletions.
28 changes: 25 additions & 3 deletions includes/admin/class-amp-template-customizer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

use AmpProject\AmpWP\Admin\ReaderThemes;
use AmpProject\AmpWP\Option;
use AmpProject\AmpWP\QueryVar;
use AmpProject\AmpWP\ReaderThemeLoader;
use AmpProject\AmpWP\Services;

Expand Down Expand Up @@ -71,8 +72,8 @@ protected function __construct( WP_Customize_Manager $wp_customize, ReaderThemeL
* @since 0.4
* @access public
*
* @param WP_Customize_Manager $wp_customize Customizer instance.
* @param ReaderThemeLoader $reader_theme_loader Reader theme loader.
* @param WP_Customize_Manager $wp_customize Customizer instance.
* @param ReaderThemeLoader|null $reader_theme_loader Reader theme loader.
* @return AMP_Template_Customizer Instance.
*/
public static function init( WP_Customize_Manager $wp_customize, ReaderThemeLoader $reader_theme_loader = null ) {
Expand All @@ -85,6 +86,8 @@ public static function init( WP_Customize_Manager $wp_customize, ReaderThemeLoad
$has_reader_theme = ( ReaderThemes::DEFAULT_READER_THEME !== AMP_Options_Manager::get_option( Option::READER_THEME ) ); // @todo Verify that the theme actually exists.

if ( $is_reader_mode ) {
add_action( 'customize_controls_init', [ $self, 'set_reader_preview_url' ] );

if ( $has_reader_theme ) {
add_action( 'customize_save_after', [ $self, 'store_modified_theme_mod_setting_timestamps' ] );
}
Expand Down Expand Up @@ -123,6 +126,18 @@ public static function init( WP_Customize_Manager $wp_customize, ReaderThemeLoad
return $self;
}

/**
* Set the preview URL when using a Reader theme if the AMP preview permalink was requested and no URL was provided.
*/
public function set_reader_preview_url() {
if ( isset( $_GET[ QueryVar::AMP_PREVIEW ] ) && ! isset( $_GET['url'] ) ) { // phpcs:ignore WordPress.Security.NonceVerification.Recommended
$url = amp_admin_get_preview_permalink();
if ( $url ) {
$this->wp_customize->set_preview_url( $url );
}
}
}

/**
* Force changes to header video to cause refresh since there are various JS dependencies that prevent selective refresh from working properly.
*
Expand Down Expand Up @@ -243,7 +258,14 @@ public function add_dark_mode_toggler_button_notice() {
* @since 0.4
*/
public function init_legacy_preview() {
add_action( 'amp_post_template_head', 'wp_no_robots' );
// This is only needed in WP<5.7 because in 5.7 the `wp_robots()` function runs at the `amp_post_template_head`
// action and in `WP_Customize_Manager::customize_preview_init()` the `wp_robots_no_robots()` function is added
// to the `wp_robots` filter.
if ( version_compare( strtok( get_bloginfo( 'version' ), '-' ), '5.7', '<' ) ) {
// There is code coverage for this, but code coverage is only collected for the latest WP version.
add_action( 'amp_post_template_head', 'wp_no_robots' ); // @codeCoverageIgnore
}

add_action( 'wp_enqueue_scripts', [ $this, 'enqueue_legacy_preview_scripts' ] );
add_action( 'amp_customizer_enqueue_preview_scripts', [ $this, 'enqueue_legacy_preview_scripts' ] );

Expand Down
37 changes: 4 additions & 33 deletions includes/admin/functions.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

use AmpProject\AmpWP\Option;
use AmpProject\AmpWP\QueryVar;

/**
* Sets up the AMP template editor for the Customizer.
Expand Down Expand Up @@ -38,14 +39,15 @@ function amp_admin_get_preview_permalink() {
/**
* Filter the post type to retrieve the latest for use in the AMP template customizer.
*
* @todo This filter doesn't actually do anything at present. Instead of array_unique() below, an array_intersect() should have been used.
* @param string $post_type Post type slug. Default 'post'.
*/
$post_type = (string) apply_filters( 'amp_customizer_post_type', 'post' );

// Make sure the desired post type is actually supported, and if so, prefer it.
$supported_post_types = AMP_Post_Type_Support::get_supported_post_types();
if ( in_array( $post_type, $supported_post_types, true ) ) {
$supported_post_types = array_unique( array_merge( [ $post_type ], $supported_post_types ) );
$supported_post_types = array_values( array_unique( array_merge( [ $post_type ], $supported_post_types ) ) );
}

// Bail if there are no supported post types.
Expand Down Expand Up @@ -98,7 +100,7 @@ function amp_get_customizer_url() {
}

$args = [
'url' => amp_admin_get_preview_permalink(),
QueryVar::AMP_PREVIEW => '1',
];
if ( $is_legacy ) {
$args['autofocus[panel]'] = AMP_Template_Customizer::PANEL_ID;
Expand Down Expand Up @@ -130,37 +132,6 @@ function amp_add_customizer_link() {
);
}

/**
* Add custom analytics.
*
* This is currently only used for legacy AMP post templates.
*
* @since 0.5
* @see amp_get_analytics()
* @internal
*
* @param array $analytics Analytics.
* @return array Analytics.
*/
function amp_add_custom_analytics( $analytics = [] ) {
$analytics = amp_get_analytics( $analytics );

/**
* Add amp-analytics tags.
*
* This filter allows you to easily insert any amp-analytics tags without needing much heavy lifting.
* This filter should be used to alter entries for legacy AMP templates.
*
* @since 0.4
*
* @param array $analytics An associative array of the analytics entries we want to output. Each array entry must have a unique key, and the value should be an array with the following keys: `type`, `attributes`, `script_data`. See readme for more details.
* @param WP_Post $post The current post.
*/
$analytics = apply_filters( 'amp_post_template_analytics', $analytics, get_queried_object() );

return $analytics;
}

/**
* Bootstrap AMP Editor core blocks.
*
Expand Down
31 changes: 31 additions & 0 deletions includes/amp-post-template-functions.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,37 @@ function amp_post_template_add_styles( $amp_template ) {
}
}

/**
* Add custom analytics.
*
* This is currently only used for legacy AMP post templates.
*
* @since 0.5
* @see amp_get_analytics()
* @internal
*
* @param array $analytics Analytics.
* @return array Analytics.
*/
function amp_add_custom_analytics( $analytics = [] ) {
$analytics = amp_get_analytics( $analytics );

/**
* Add amp-analytics tags.
*
* This filter allows you to easily insert any amp-analytics tags without needing much heavy lifting.
* This filter should be used to alter entries for legacy AMP templates.
*
* @since 0.4
*
* @param array $analytics An associative array of the analytics entries we want to output. Each array entry must have a unique key, and the value should be an array with the following keys: `type`, `attributes`, `script_data`. See readme for more details.
* @param WP_Post $post The current post.
*/
$analytics = apply_filters( 'amp_post_template_analytics', $analytics, get_queried_object() );

return $analytics;
}

/**
* Print analytics data.
*
Expand Down
7 changes: 7 additions & 0 deletions src/QueryVar.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,4 +70,11 @@ interface QueryVar {
* @var string
*/
const VERBOSE_SERVER_TIMING = 'amp_verbose_server_timing';

/**
* Query parameter provided to customize.php to indicate that the preview should be loaded with an AMP URL.
*
* @var string
*/
const AMP_PREVIEW = 'amp_preview';
}
45 changes: 45 additions & 0 deletions tests/php/test-amp-post-template-functions.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,51 @@ function () use ( $template ) {
$this->assertStringContains( 'body{color:blue', $output );
}

/** @covers ::amp_add_custom_analytics() */
public function test_amp_add_custom_analytics() {
$post_id = self::factory()->post->create();
$this->go_to( get_permalink( $post_id ) );

$uuid1 = 'a6a13d59-a5bb-40a8-af3f-657d12d523bd';
$uuid2 = 'b6a13d59-a5bb-40a8-af3f-657d12d523bd';

add_filter(
'amp_analytics_entries',
static function () use ( $uuid1 ) {
return [
$uuid1 => [
'type' => 'foo',
'config_data' => [
'bar' => true,
],
],
];
}
);

add_filter(
'amp_post_template_analytics',
function ( $analytics, WP_Post $queried_oject ) use ( $uuid2, $post_id ) {
$this->assertEquals( $post_id, $queried_oject->ID );

$analytics[ $uuid2 ] = [
'type' => 'baz',
'config_data' => [
'quux' => true,
],
];

return $analytics;
},
10,
2
);

$analytics = amp_add_custom_analytics();
$this->assertArrayHasKey( $uuid1, $analytics );
$this->assertArrayHasKey( $uuid2, $analytics );
}

/** @covers ::amp_post_template_add_analytics_data() */
public function test_amp_post_template_add_analytics_data() {
$this->assertEmpty( get_echo( 'amp_post_template_add_analytics_data' ) );
Expand Down
41 changes: 39 additions & 2 deletions tests/php/test-class-amp-template-customizer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

use AmpProject\AmpWP\Option;
use AmpProject\AmpWP\QueryVar;
use AmpProject\AmpWP\ReaderThemeLoader;
use AmpProject\AmpWP\Tests\Helpers\AssertContainsCompatibility;
use AmpProject\AmpWP\Tests\Helpers\LoadsCoreThemes;
Expand Down Expand Up @@ -69,6 +70,7 @@ public function test_init_canonical() {
}

$instance = AMP_Template_Customizer::init( $wp_customize );
$this->assertFalse( has_action( 'customize_controls_init', [ $instance, 'set_reader_preview_url' ] ) );
$this->assertEquals( 0, did_action( 'amp_customizer_init' ) );
$this->assertFalse( has_action( 'customize_controls_enqueue_scripts', [ $instance, 'add_customizer_scripts' ] ) );

Expand All @@ -93,6 +95,7 @@ public function test_init_transitional() {
}

$instance = AMP_Template_Customizer::init( $wp_customize );
$this->assertFalse( has_action( 'customize_controls_init', [ $instance, 'set_reader_preview_url' ] ) );
$this->assertEquals( 0, did_action( 'amp_customizer_init' ) );
$this->assertFalse( has_action( 'customize_save_after', [ $instance, 'store_modified_theme_mod_setting_timestamps' ] ) );
$this->assertFalse( has_action( 'customize_controls_enqueue_scripts', [ $instance, 'add_customizer_scripts' ] ) );
Expand Down Expand Up @@ -127,6 +130,7 @@ public function test_init_legacy_reader() {
}

$instance = AMP_Template_Customizer::init( $wp_customize );
$this->assertEquals( 10, has_action( 'customize_controls_init', [ $instance, 'set_reader_preview_url' ] ) );
$this->assertFalse( has_action( 'customize_save_after', [ $instance, 'store_modified_theme_mod_setting_timestamps' ] ) );
$this->assertFalse( has_action( 'customize_controls_enqueue_scripts', [ $instance, 'add_customizer_scripts' ] ) );
$this->assertEquals( 1, did_action( 'amp_customizer_init' ) );
Expand Down Expand Up @@ -181,6 +185,7 @@ public function test_init_reader_theme_with_amp() {
}

$instance = AMP_Template_Customizer::init( $wp_customize );
$this->assertEquals( 10, has_action( 'customize_controls_init', [ $instance, 'set_reader_preview_url' ] ) );
$this->assertEquals( 10, has_action( 'customize_save_after', [ $instance, 'store_modified_theme_mod_setting_timestamps' ] ) );
$this->assertEquals( 10, has_action( 'customize_controls_enqueue_scripts', [ $instance, 'add_customizer_scripts' ] ) );
$this->assertEquals( 10, has_action( 'customize_controls_print_footer_scripts', [ $instance, 'render_setting_import_section_template' ] ) );
Expand All @@ -200,6 +205,30 @@ public function test_init_reader_theme_with_amp() {
$this->assertNull( $wp_customize->get_section( 'static_front_page' ) );
}

/** @covers AMP_Template_Customizer::set_reader_preview_url() */
public function test_set_reader_preview_url() {
$post_id = self::factory()->post->create();
$wp_customize = $this->get_customize_manager();
$instance = AMP_Template_Customizer::init( $wp_customize );

$this->assertEquals( amp_get_permalink( $post_id ), amp_admin_get_preview_permalink() );
$this->assertEquals( home_url( '/' ), $wp_customize->get_preview_url() );

$instance->set_reader_preview_url();
$this->assertEquals( home_url( '/' ), $wp_customize->get_preview_url() );

$_GET[ QueryVar::AMP_PREVIEW ] = '1';
$instance->set_reader_preview_url();
$this->assertNotEquals( home_url( '/' ), $wp_customize->get_preview_url() );
$this->assertEquals( amp_admin_get_preview_permalink(), $wp_customize->get_preview_url() );

$wp_customize->set_preview_url( home_url( '/foo/' ) );
$_GET[ QueryVar::AMP_PREVIEW ] = '1';
$_GET['url'] = home_url( '/foo/' );
$instance->set_reader_preview_url();
$this->assertEquals( home_url( '/foo/' ), $wp_customize->get_preview_url() );
}

/**
* @covers AMP_Template_Customizer::init()
* @covers AMP_Template_Customizer::add_dark_mode_toggler_button_notice()
Expand Down Expand Up @@ -279,7 +308,11 @@ public function test_init_legacy_preview_controls() {
$wp_customize = $this->get_customize_manager();
$instance = AMP_Template_Customizer::init( $wp_customize );
$instance->init_legacy_preview();
$this->assertEquals( 10, has_action( 'amp_post_template_head', 'wp_no_robots' ) );
if ( version_compare( strtok( get_bloginfo( 'version' ), '-' ), '5.7', '<' ) ) {
$this->assertEquals( 10, has_action( 'amp_post_template_head', 'wp_no_robots' ) );
} else {
$this->assertFalse( has_action( 'amp_post_template_head', 'wp_no_robots' ) );
}
$this->assertEquals( 10, has_action( 'amp_customizer_enqueue_preview_scripts', [ $instance, 'enqueue_legacy_preview_scripts' ] ) );
$this->assertNull( $wp_customize->get_messenger_channel() );
}
Expand All @@ -290,7 +323,11 @@ public function test_init_legacy_preview_iframe() {
$wp_customize->start_previewing_theme();
$instance = AMP_Template_Customizer::init( $wp_customize );
$instance->init_legacy_preview();
$this->assertEquals( 10, has_action( 'amp_post_template_head', 'wp_no_robots' ) );
if ( version_compare( strtok( get_bloginfo( 'version' ), '-' ), '5.7', '<' ) ) {
$this->assertEquals( 10, has_action( 'amp_post_template_head', 'wp_no_robots' ) );
} else {
$this->assertFalse( has_action( 'amp_post_template_head', 'wp_no_robots' ) );
}
$this->assertEquals( 10, has_action( 'amp_customizer_enqueue_preview_scripts', [ $instance, 'enqueue_legacy_preview_scripts' ] ) );
$this->assertEquals( '123', $wp_customize->get_messenger_channel() );

Expand Down
Loading

0 comments on commit 1c7728c

Please sign in to comment.