Defer obtaining URL for Customizer preview until AMP Customizer is accessed #6198
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Plugin builds for 5cc5658 are ready 🛎️!
|
Codecov Report
@@ Coverage Diff @@
## develop #6198 +/- ##
=============================================
+ Coverage 75.10% 75.23% +0.13%
- Complexity 5865 5870 +5
=============================================
Files 234 234
Lines 17747 17754 +7
=============================================
+ Hits 13329 13358 +29
+ Misses 4418 4396 -22
Flags with carried forward coverage won't be shown. Click here to find out more.
|
westonruter
commented
May 12, 2021
westonruter
commented
May 13, 2021
Comment on lines
+128
to
+158
| /** | ||
| * 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; | ||
| } | ||
|
|
There was a problem hiding this comment.
This was simply moved from includes/admin/functions.php
.
westonruter
deleted the
fix/reader-theme-customizer-preview-url-performance
branch
westonruter
added a commit
that referenced
this pull request
May 13, 2021
|
QA Passed With Reader mode enabled, I verified with Query Monitor that the query to find the latest AMP page is no longer being run:
There is no
Clicking on the customizer link does indeed display latest AMP page in the preview:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This addresses a support forum topic:
The issue here is the AMP Customizer link which is added under the Appearance menu when Reader mode is selected:
In order to have the Customizer open with an AMP page in the preview, we do a query to find an AMP page and then supply it in the
urlparameter tocustomize.php. However, we don't have to supply this URL at this point. We can defer lookup up the URL until the Customizer is actually accessed. We can omit theurlparameter and instead supply a custom one likeamp_preview. When that is supplied when accessingcustomize.phpand Reader mode is active, in thecustomize_controls_initaction we can at that point do the lookup for an AMP page and pass it toWP_Customize_Manager::set_preview_url(). In this way the preview URL will be determined just in time.This only applies in Reader mode to the aforementioned AMP admin menu item under Appearance and also to the “Customize Reader Theme” link on the AMP Settings screen:
This PR also fixes a deprecation notice which was something I missed in #5793.
Checklist