-
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
Eliminate "AMP Settings" panel from Gallery block #4989
Comments
I was confused why these settings were missing after the last update, I know most people have probably switched to a gallery block but I have reasons to use just the shortcode. So should we just add the settings into the shortcode now like "amp-lightbox=true amp-carousel=false"? I really do wish features weren't removed like this, I found them very useful and I wonder if anyone was actually confused by them. |
You're using the Shortcode block with a If so, then yes, if you want to continue using the The PR that removed the toggles from the Shortcode block is #4775. See this changelog entry:
|
I would favour having a settings page like jetpack where you could toggle the carousel/lightbox options site-wide with it turned off by default. Reason being, it took me longer than I care to admit to find out why a gallery was displaying as a carousel and how to stop it from doing so... 🤣 I do think this functionality belongs in the amp plugin as not everyone uses jetpack or, even if they do, they might already have rolled their own non-amp gallery/lightbox solution and turned those toggles off on jetpack (or any other media plugin that seems to provide their own gallery output these days)? |
Yeah, I could see the default values for whether the galleries are displayed as lightbox or carousel could be in an Advanced section on the AMP settings screen. |
Notes from backlog: Related to AMP compatibility work for Jetpack. If you enable the Jetpack module or light boxes, it will enable Lightbox effect for all galleries. This doesn’t do anything, doesn’t make sense. Whether on or off you still get light boxes. We need a better way of representing this. We could have global setting to enable all or off by default. Jetpack should hook into that so it’s consistent. Should also not say AMP. Should integrate more seamlessly into the block sidebar. |
However, Could we add some label for it similar to HTML ANCHOR/ADDITIONAL CSS CLASS(ES)? |
@westonruter I think we can add a label JFI: Moved this to |
@thelovekesh Good point, yes. Please add a heading for those two toggles. |
Feature description
In the theme of preventing the user from having to know anything about AMP (#4554, #4555, #4556, #4557) to author content, we should also consider eliminating the dedicated “AMP Settings” panel from the Gallery block:
At one level this could just be a matter of removing “AMP” from the panel title. It could also mean moving the toggles under “Advanced”.
We also need to take into account how other plugins extend the Gallery block. Jetpack for example has this setting:
When that is enabled (as of Jetpack 8.7) the effect is as if the two toggles in AMP settings have been turned on. Users will be confused as to what purpose they have when this Jetpack feature is enabled.
Perhaps the AMP-provided settings panel should be made easy for another plugin to turn off (e.g. Jetpack).
We should also consider whether this functionality belongs in the AMP plugin in the first place, and if it should be removed/deprecated in favor of ecosystem plugins (like Jetpack).
Jetpack's implementation is key because it provides a non-AMP version on non-AMP pages. The AMP plugin doesn't currently provide that, which is yet another source of confusion for users. If a site is in a paired AMP setup (Reader/Transitional mode), why should there be AMP-specific settings present in the first place if functionality won't be working in the canonical (non-AMP) version of the page?
We should limit extending blocks with AMP functionality when that functionality can be used on any version of the page. This will be greatly facilitated by Bento AMP. However, in the meantime, we should consider whether this functionality should be in the AMP plugin or relegated to the ecosystem.
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
Implementation brief
QA testing instructions
Demo
Changelog entry
The text was updated successfully, but these errors were encountered: