-
Notifications
You must be signed in to change notification settings - Fork 815
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
Carousel: conflict with Gutenberg Lightbox behavior #32668
Comments
📌 REPRODUCTION RESULTS
📌 FINDINGS/SCREENSHOTS/VIDEO Atomic CbTTxT.mp4📌 ACTIONS
|
TIL about the Lightbox feature in Gutenberg! It was introduced in WordPress/gutenberg#50373 I'm not sure what should be our approach with this. Looking at Gutenberg's implementation, I would personally think that Jetpack's Carousel feature is more interesting, it offers more features and works better with multiple images. Maybe we should opt to disable Gutenberg's implementation when the Carousel feature is enabled. I think we may be able to rely on the @artemiomorales I see you worked on the initial implementation of the Lightbox. I'd be curious to have your take on this; do you think third-party plugins should be disabling Gutenberg's Lightbox when they offer a similar feature? |
That's a good question :) It'd be good to know if this is the first time Jetpack has faced something like this. @jeherve Do you know if Jetpack is disabling any other Core feature that the plugin re-implements? I think that your suggestion of disabling the Lightbox Core Feature when installing a plugin that implements a similar feature makes sense. I think Plugins are meant to extend, modify, and sometimes overwrite Core functionalities. |
@jeherve Another thing that just came to my mind. Would Jetpack users be able to disable Jetpack's Carousel if they want to use the default Lightbox instead? I don't know how granular the Jetpack plugin is, but there might be users interested in other Jetpack features who still prefer to use the default Lightbox. |
We've gone both ways in the past; at times we've deprecated our feature and migrated folks to a core feature. Other times we've favored our implementation.
Yes, folks can disable the feature if they're not interested in it, or if they're already using another Ligthbox feature or plugin (like Gutenberg). I'm now looking at disabling the Gutenberg feature, but I cannot find a straightforward way of doing so; hooking into |
Perfect then. My last suggestion would be to let the JetPack users know that the default Lightbox is being disabled.
I'll have to defer to @michalczaplinski or @artemiomorales for this one. |
Hey folks! I'm still seeing reports come in from users about the overlap between Jetpack's Carousel and the Gutenberg Lightbox features that are rolling out. The latest (Automattic/wp-calypso#82790) notes:
📌 ACTIONS
Any updates to know about this, beyond the above discussion? |
I just replied to a ticket from a user struggling with this in 7545923-zd-a8c The native lightbox is still a relatively new feature so I would also favor continuing with Jetpack's lightbox for the time being. And, if we want to stick to Jetpack's lightbox, would it be possible to just deactivate the "Expand on click" feature for ALL NEW Image blocks by default? This would ensure that newly added galleries would work as expected without affecting the existing ones. This might feel like a minor glitch but it affects a lot of users, both on dotcom and self-hosted. Do you think we could bump the priority, @jeherve? Thank you! |
Support References This comment is automatically generated. Please do not edit it.
|
@michalczaplinski @artemiomorales Looping you in again about a Gutenberg lightbox killswitch. How would I go about removing the Gutenberg lightbox interface and functionality? Thank you! |
Hi @jeherve ! You can disable the lightbox (both the interface in the editor as well as the functionality on the frontend) by adding the following setting to the "settings": {
"blocks": {
"core/image": {
"lightbox": {
"enabled": false,
"allowEditing": false
}
}
},
} There is more information in https://developer.wordpress.org/themes/global-settings-and-styles/settings/lightbox/#lightbox-settings |
@michalczaplinski Thank you! |
I'm afraid there is no "easy" and declarative way to disable the lightbox. I've just looked into it and you should be able to get away with using a block filter: /**
* WordPress dependencies
*/
import { addFilter } from '@wordpress/hooks';
function removeLightbox( settings, name ) {
if ( name !== 'core/image' ) {
return settings;
}
return {
...settings,
lightbox: {
allowEditing: false,
},
};
}
addFilter(
'blocks.registerBlockType',
'jetpack/lightbox/remove',
removeLightbox,
50 // Adjust the priority to run after any other filters that might override this.
); However, I've just tested the code above and the value for the |
Thank you so much, @jeherve and @michalczaplinski for looking into this. |
I also thought that using the PHP filter might do the trick but to no avail 😕: function filter_metadata_registration( $settings, $metadata ) {
if ($metadata['name'] === 'core/image') {
return array_merge($metadata, array(
"lightbox" => array(
"allowEditing" => false
)
));
}
return $metadata;
};
add_filter( 'block_type_metadata_settings', 'filter_metadata_registration', 5, 2 ); |
I just ran into this on my nomad.blog simple site on WPCOM: Screen.Recording.2024-03-12.at.9.14.09.AM.movWhy aren't we finding ways to work with Core here and to ensure they work well together? |
@michalczaplinski I'd be interested in ways to disable the Gutenberg lightbox (alongside Jetpack lightbox) for posts Jetpack sends as emails to subscribers. JS functionality simply doesn't work in emails obviously, but there's button/link added to the markup that we now need to hide via CSS, while ideally we'd just disable markup getting added entirely via PHP. We'd disable only at runtime when rendering the email. |
Related discussion: p1710260245886189-slack-CBG1CP4EN Related Gutenberg issue: WordPress/gutenberg#59797 |
Fixes #32668 It's best to keep only one lightbox option, to avoid any confusion. Since Jetpack's Carousel feature currently offers more features, let's keep ours in favor of Core's for now. In the future, when core's lightbox option becomes more robust, we can consider deprecating Jetpack's Carousel feature altogether.
Fixes #32668 It's best to keep only one lightbox option, to avoid any confusion. Since Jetpack's Carousel feature currently offers more features, let's keep ours in favor of Core's for now. In the future, when core's lightbox option becomes more robust, we can consider deprecating Jetpack's Carousel feature altogether.
Apologies for dumping this here randomly. |
@djcowan Thanks for the warning. Right now, Jetpack's Carousel feature, when enabled, disables the Lightbox feature provided by WordPress Core. We've done that to avoid conflicts between the 2 features. However, it sounds like you may have found a conflict anyway. Do you think you could create a new issue here with more details explaining how to reproduce the issue, so we can take a closer look? Thank you! |
Fixes #32668 It's best to keep only one lightbox option, to avoid any confusion. Since Jetpack's Carousel feature currently offers more features, let's keep ours in favor of Core's for now. In the future, when core's lightbox option becomes more robust, we can consider deprecating Jetpack's Carousel feature altogether.
Impacted plugin
Jetpack
Quick summary
With the release of Lightbox for Image Blocks in the Gutenberg plugin, there's a conflict between Jetpack's Carousel and the core Lightbox behavior when using a core Gallery Block.
Somewhat similar to #19496 and #17484
Steps to reproduce
A clear and concise description of what you expected to happen.
In core blocks, if I select the Lightbox behavior, it should use that instead of Carousel, as that action is more specific, since it's defined at block-level
What actually happened
Carousel was shown in the first click, and core's Lightbox after trying to exit, creating a confusing experience
Impact
Some (< 50%)
Available workarounds?
Yes, easy to implement
Platform (Simple and/or Atomic)
Simple, Atomic, Self-hosted
Logs or notes
WordPress 6.3, Gutenberg 16.5
The text was updated successfully, but these errors were encountered: