-
Notifications
You must be signed in to change notification settings - Fork 383
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
Merge AMP Customizer with main Customizer #796
Comments
In paired mode (the only mode presently), there can be an AMP toggle that appears with the device preview buttons, where you can decide whether or not you want to preview the AMP version or not, if it is available. When in canonical mode (#668) then this button would not be needed because every URL would already be in AMP mode. Alternatively, we could always automatically show the AMP version when the Mobile device is being previewed, though since AMP is not specific to mobile anymore, this is something that should be previewable regardless of the device type. |
Acceptance criteria:
|
Explored what the 'Preview AMP' toggle could look like, can be seen in the figma file here. Including a screenshot for reference. The top section shows the preview AMP toggle active, the second shows it disabled. The third one shows what would happen if a user navigates from an article page to the homepage where AMP is not supported – an alert would appear for 5 - 8 seconds and then fade out. This alert would reappear if the user hovers the toggle while on a page that isn't AMP compatible. |
I like how the toggle style is similar to what is in Gutenberg: I also like how the tooltip has the notification as part of the tooltip. We should, however, make sure that all of this fits on one line. I think the Preview toggle should be positioned directly tho the left of the Desktop icon, with a faint separator between them to indicate it's a different kind of device preview toggle. I'm hoping we can hide the “Preview AMP” label, and make it part of a tooltip? The “Hide Controls” label could be hidden if we must. |
I made a couple tweaks to the ACs 6 & 12. I added |
@ThierryA @westonruter Comments re: use case |
If I go to Customize my site and land on a Non-AMPed page, the AMP toggle button displays correctly (it's in the off state with a notice about navigating to an AMP compatible page). If I click on the notice to go to an AMP compatible page, and then click on the logo to go back to the homepage the toggle shows up in a ON state but is disabled. Shouldn't the toggle now be set to an OFF state? Video example: https://v.usetapes.com/FtlcBJY15l |
@jwold This is the infamous question behind https://core.trac.wordpress.org/ticket/40432 In other words, should the AMP panel be contextual based on whether AMP is is shown in the preview? If that is the case, then it would be harder to discover the AMP panel. |
possibly solvable by having the notification saying that "AMP is disabled for page" include something like "note: any published revisions will not appear until AMP enabled" or something like that? |
No, because AMP is still enabled but it is unavailable. So as soon as you land on a page or post again, then you'll get the AMP version not the non-AMPed version. |
This is a general UX problem for the Customizer: modifying settings which aren't displayed in the preview. For widgets the approach has been to hide the Customizer section entirely, but this introduces problems with discovery. An alternative approach to hiding can be seen in the Header Media section in Twenty Seventeen. Only the video control is shown when you are previewing the homepage. If you are not previewing the homepage then the video control is hidden and a persistent notification is shown in it's place. Would such a persistent notification be better than the momentary tooltip? |
How about replacing the AMP settings with a admin notice (within the panel) if the user is on a Non-AMP page or post? |
Yes, re-using https://core.trac.wordpress.org/ticket/38794 For example: |
[Edited] Refer to the latest [acceptance criteria] #796 (comment)
Currently if you want to Customizer your AMP template you have to go to Appearance > AMP in the admin, and this will open the Customizer in a blank slate with only the AMP panel registered. The latest post is loaded in to the preview, and navigation of the site is disabled. There are a couple problems with this:
So I propose that we merge the AMP panel into the regular Customizer. We can allow the user to navigate around the entirety of their site in the Customizer preview, but then depending on whether or not the AMP panel is expanded then automatically toggle the preview URL to include the AMP version if it is available. If AMP is not available for a given URL then the Customizer can show a notification in the AMP panel or any of its sub-sections that AMP is not available, and that they should navigate to a URL that is AMP compatible. The notification could have a link to take them to the most recent post in the preview.
There can still be an Appearance > AMP link in the admin menu, but it can just deep-link to the AMP panel and supply the
url
of the most recent post as the preview to start with.I consider this to be a dependency for adding page support (#176), because if you can't navigate to pages (or other custom post types) in the Customizer, then you can't customize their appearance.
The text was updated successfully, but these errors were encountered: