-
Notifications
You must be signed in to change notification settings - Fork 384
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
Implement support for off cache AMP #5378
Comments
We may want to split this into two separate issues: one that implements just the low-level support for opting out of AMP Cache (but leaving it enabled by default), and then a second issue where we decide if certain templates should be off-cache by default and a UI for controlling it. The first issue can be tackled ASAP in a 2.0.x release, whereas the UI can be part of 2.1. |
One possible approach is having something like the "Template Support" capability, or may be expanding that, to include the ability of opting out from cache at the template or content type level. And this will be complemented with the ability of opting out from the cache at a fine granularity level, along the lines of the options you suggested (Enabled, Enabled with/without AMP cache). |
Yeah; I was thinking to have two check boxes, one for enabling and one for the cache, with the latter conditionally enabled when the former is checked. |
How would the checkboxes be labelled? |
One possibility would be along the lines of labels indicating (1) AMP content generation, and (2) opt in/out from the cache:
And we can update the heading of the section to encompass the dual function. |
Yeah. Let's explore some options for the UI to figure out something that minimizes clutter. The way it is right now with the one checkbox appears nice and tidy as the checkboxes kinda serve as bullet points. So if we add a second checkbox or use a select dropdown, then that bullet-point feeling will be lost: Maybe we need a new icon that can represent the three states of (for lack of better icons, consider these emoji placeholders):
If we have icons for those three states, then perhaps instead of what we have right now: We could replace the simple checkbox with the icon representing the current state. When you click the icon, then a dropdown could appear which could let you choose from among the 3 options. Once selected and the dropdown is closed, the icon could be updated to reflect the chosen option. So something like this: I think this might be a tidy way to avoid clutter. |
User who also has requested this: https://wordpress.org/support/topic/disable-amp-cache-on-certain-pages/ |
I don't think that UI allows for enough information density. Basically, every checkbox you see in this screenshot needs to be able to have this multi-state selection: Some kind of dropdown that is revealed by an icon, where the icon represents the current state seems the most promising so far. |
Now that I think about it more, perhaps we should only have AMP Cache enabled by default for the Oh, this brings up something else just realized. Before I said that the tri-state toggle would apply to everywhere that a checkbox appears now. However, this doesn't really work when you consider the One option to deal with this would be to keep the Content Types as being a simple boolean checkbox, but then to add the template subpoints under "Singular" (for amp-wp/includes/class-amp-theme-support.php Lines 875 to 887 in 5902120
Something I realize we're not doing now is omitting the post type archive for post types that have AMP disabled. In the same way, if we list out each post type under Singular template, then we should probably omit those templates which don't have corresponding post type enabled. |
@westonruter is this what you were talking about this morning? |
@jwold lol, yes! I wasn't aware that this is standard behavior in Chrome now. So one option would be to use a regular This gets us part of the way. What is not possible, however, is the inclusion of icons in This would work when the toggle is just for "AMP Enabled", like on the edit post screen: However, I need to correct something I said this morning. In the context of the Template Hierarchy, the selected option would not be shown the control is not selected. Instead, it should be the icon representing the status followed by the template name (e.g. Singular). I suppose this is why I tried to mock it up like: So in this case, throw out what I was thinking of entirely in regards to the selected option maintaining its vertical position when you tap on the control. The line item would be the template label (e.g. Singular) and then preceding that would be an icon indicating what its current support status is. Clicking on the icon should then reveal a dropdown to show what the currently selected icon means, as well as what other options are available. (In my mock I used a The goal is to keep the template hierarchy primarily showing the names of the templates and/or post types. Then to add these new settings by replacing the ☑️ with another icon which is a UI element to select from these other options. The hope is that this prevents the UI from becoming too cluttered. |
Regarding AMP cache defaults, you should take into consideration why a lot of site builders use AMP: they want to profit from the additional exposure that Google Search offers to AMP results => provided they are served via the Google AMP Cache. If you remove caching by default for most of the site, a lot of people will wait for these benefits to kick in only to find out at some point that the site is seemingly "misconfigured", and thus wastes the "AMP benefit" on the homepage and the evergreen pages. |
Regarding the borderless select, we need to find a compromise between sleek look and accessibility/discoverability. If something doesn't look like a control, no one will click on it to find out what it does... |
That's true, and the intention behind off-cache AMP is that any pages that opt-out of the AMP cache will still look the same in search results. They'll still have the AMP badge. The only difference is they won't get the prerendering, so tapping on a result will be slower to load since the page has to be loaded and rendered from origin, rather than having been loaded and pre-rendered in the background from the AMP Cache. |
Yes, but without the AMP cache, they will not appear in the different forms of carousels on mobile search results. See https://developers.google.com/search/docs/guides/about-amp |
I believe that will soon be outdated, but we will need to wait until off-cache AMP is launched in a couple weeks at AMP Fest to be sure. In any case, we can move forward with just a filter that allows a site owner to turn off AMP Cache at a programmatic level (with a filter). We just should make sure that this is done in a way that we anticipate a UI being added as well. So if that filter is added, for example, we may need to revert to only showing a simple checkbox since at that point we can no longer be sure that the UI toggles will have any effect (as the filter would presumably override them, unless we make the filter only change the default). |
Since our call I've been looking at some concepts that I'd love to discus in our product sync tomorrow. First, here are some initial sketches: The important factor here is for users to be able to understand that they are changing the states for different content types. So, for instance, for the Option D feels like the simplest, where the Here's a 📹 video of the prototype: https://v.usetapes.com/N97gx3310z |
Screenshot from prototype: I like the popover with the list. That's good. The only thing is that feels somewhat busy. What would it look like if only showed the selected state as opposed to the selected icon and the two that are not selected? I like that you're styling the UI as a button which will encourage users to hover/tap to discover the options. We'll still need to look into the iconography, as turning off AMP Cache shouldn't be
The last icon could be some illustration combining the AMP icon with a 🌐 to represent the AMP Cache. (Heh, better than 💵.) |
We'll also need to re-think the “Serve all templates as AMP” toggle since it wouldn't make sense anymore. In fact, this could also solve the label problem. What if above the template list we had three labelled buttons for the three states with their icons. Clicking those buttons would be doing bulk actions to set all templates to have the state. In this way, it would be similar to the bulk actions to change the state of multiple validation errors as discussed in #5427 and #5431. In this way we could freely show just the selected icon for each item rather than the selected one and the others being grayed out. |
Here's a refinement on the previous idea. It simplifies the user experience quite a bit, reducing the amount of icons and making the button action more clear.
This may still be valuable. I can't quite envision how that would look though. Figma prototype: https://d.pr/70XnlU |
I like that dropdown.
The template hierarchy could have a horizontal line above it, and then above that the dropdown could be prefixed by a label like "Update all templates:", or something. |
Should the "update all" button also apply to content types, or just to template hierarchy options? |
Only the template hierarchy like we have the single toggle today. I'm not sure yet if we will have the tri-state toggle for the content types. Those may get the binary checkbox still. |
I like the dropdown, but I would prefer to have the Label first then the dropdown. |
An issue I see with moving the dropdown after the label is that the label widths are variable, as each has a different template name. If the dropdowns were on the right then they wouldn't be neatly liked up vertically making it more difficult to repeatedly click down the list of templates with a mouse. An alternative could be to put the dropdowns vertically in a column to the right, but then we may need to add zebra striping to make sure users are clear which template is for which dropdown. |
@westonruter here's how I envision the "update all" button could work. And a note on the labeling, I agree that having the button before the label works better, but we also could setup a grid system of sorts as Weston described. We could try something like this: |
From Weston: "All archives should by default be not on AMP cache. Singular posts should be on AMP cache by default." Singular pages -- not sure yet. |
The filter that is used for off-cache AMP should also be designed to also resolve filtering whether AMP is disabled for arbitrary queries (see #2817). |
On that note, see #2817 (comment) for implications of filter. In particular, the plugin currently has return apply_filters( 'amp_template_supported', $supported, $query ); So instead of this, the return apply_filters( 'amp_template_supported', $supported, $template_id ); But that would limit flexibility for the filtering, since the reality is that the template hierarchy that we currently expose in the UI is a subset of all the template hierarchy. Nevertheless, it's true that users can add their own templates to the list via the The
|
Note that off-cache AMP will essentially come as a side effect for sites in Standard mode using Bento with L1/L2 sandboxing levels. |
This will still be importance in a Bento world, since we can have a site off cache depending on its sandbox level. It’s currently blocked upstream in the AMP project. |
See latest comment on upstream issue: ampproject/amphtml#29296 (comment) |
Feature description
AMP will soon support valid AMP pages opting out of being served from an AMP Cache. See ampproject/amphtml#29296. This will be an important ability to fix various issues caused by caching, including analytics and serving stale pages (ampproject/amphtml#24326), the latter of which is particularly important for e-commerce.
Opting out of AMP Cache by default seems also important for non-singular templates, or any post/page that has frequent updates. For example, the homepage of a site (and any other archive pages) should probably not be served by the AMP Cache by default until there is a reliable way to ensure the AMP Cache is flushed (see #598). We could also have a list of post types from popular e-commerce plugins that we disable by default for AMP Cache.
👉 This is blocked from being closed until the feature lands in upstream. However, work can begin on implementing support as well as figuring out what the right defaults are. We also need to determine what UI (if any) is added for this. For example, there may need to be a new advanced section for AMP Cache handling which contains whether AMP Cache is enabled/disabled overall. Nevertheless, there could be fine-grained controls over whether AMP Cache is enabled, down to the post type and template level. If so, then this could be combined with the Supported Templates section somehow, where the current on/off toggles are replaced with a multi-value dropdown:
This could also extend to the "AMP Enabled" toggle on individual posts and pages.
Relation to Bento AMP
This is also closely related to how we might need to adjust terminology for Bento AMP. When themes and plugins can use AMP components on non-AMP pages, it's not accurate to say that “AMP” can be disabled. It's more whether or not “AMP validation” is disabled. In this way, the AMP plugin could continue to inject AMP components into the responses for non-AMP pages, such as in oEmbed handlers or in converting
img
intoamp-img
to get the speed benefits that AMP provides (althoughamp-img
specifically is on a deprecation path now that lazy-loaded images are part of the platform, per ampproject/amphtml#29786).There are also implications here for AMP Optimizer. if we serve AMP components on non-AMP pages, we should make sure they get server-side rendered (SSR) as much as possible since the AMP Boilerplate will not be present. Currently AMP Optimizer takes complete AMP documents in DOM as input. The oEmbed handlers, however, only manipulate HTML strings. So either the oEmbed handlers need to create transformed AMP manually, or we need to constrict DOM fragments and pass them into the Optimizer for processing.
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
amp="no-caching"
in thehtml
attribute.Implementation brief
amp_dev_mode_enabled
filter, introduce anamp_cache_enabled
filter. The default value should betrue
for singular queries, andfalse
for archives (and perhaps other select post types).AMP_Dev_Mode_Sanitizer
, introduce anAMP_Off_Cache_Sanitizer
which is added to the list ofamp_content_sanitizers
whenamp_cache_enabled
is filtered to befalse
. This sanitizer will set theamp
attribute on thehtml
element to beoff-cache
(tentatively).amp_dev_mode_enabled
filter. So if a sitehas_filter('amp_dev_mode_enabled')
then any cache options must be disabled.QA testing instructions
Demo
Changelog entry
The text was updated successfully, but these errors were encountered: