Skip to content
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

Closed
liviopv opened this issue Aug 24, 2023 · 20 comments · Fixed by #36565
Closed

Carousel: conflict with Gutenberg Lightbox behavior #32668

liviopv opened this issue Aug 24, 2023 · 20 comments · Fixed by #36565
Assignees
Labels
Customer Report Issues or PRs that were reported via Happiness. Previously known as "Happiness Request". [Feature] Carousel A fullscreen modal appearing when clicking on an image in a gallery or tiled gallery. [Feature Group] Editor Experience Features related to Gutenberg integration on WordPress.com. [Focus] Compatibility Ensuring our products play well with third-parties [Platform] Atomic [Platform] Simple [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Pri] Normal Triaged [Type] Bug When a feature is broken and / or not performing as intended

Comments

@liviopv
Copy link

liviopv commented Aug 24, 2023

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

  1. Turn on Carousel
  2. Add a core Gallery Block
  3. In the Image Block inside the Gallery Block, select "Lightbox" in Advanced > Behaviors
  4. Open the post/page and click on the image - it will show Carousel first, and after a second click it will show GB's lightbox

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

@liviopv liviopv added [Type] Bug When a feature is broken and / or not performing as intended [Feature] Carousel A fullscreen modal appearing when clicking on an image in a gallery or tiled gallery. [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ Needs triage Ticket needs to be triaged [Product] Jetpack labels Aug 24, 2023
@cuemarie
Copy link

📌 REPRODUCTION RESULTS

  • Tested on Simple – Replicated
  • Tested on Atomic – Replicated

📌 FINDINGS/SCREENSHOTS/VIDEO
Replicated on GB 16.4.0 (on simple) and GB 16.5.0 (on AT). I believe the Image Block > Lightbox feature was released in an early 16.x update, so unclear if this is a new issue or just one that hasn't come up quite yet.

Atomic

CbTTxT.mp4

📌 ACTIONS

@cuemarie cuemarie removed the Needs triage Ticket needs to be triaged label Aug 24, 2023
@jeherve jeherve added the [Focus] Compatibility Ensuring our products play well with third-parties label Aug 29, 2023
@jeherve
Copy link
Member

jeherve commented Aug 29, 2023

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 WP_Block_Supports class here, or maybe on the filter added here:
https://github.com/WordPress/gutenberg/blob/676f25961bfab695bacb5dd7ebb10443e49bce23/lib/block-supports/behaviors.php#L28-L35

@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?

@Poliuk
Copy link

Poliuk commented Aug 29, 2023

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.

@Poliuk
Copy link

Poliuk commented Aug 29, 2023

@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.

@jeherve
Copy link
Member

jeherve commented Aug 29, 2023

Do you know if Jetpack is disabling any other Core feature that the plugin re-implements?

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.

Would Jetpack users be able to disable Jetpack's Carousel if they want to use the Core Lightbox instead?

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 render_block_core/image isn't enough because the UI remains in the block editor. Do you have any suggestions there?
Would there be a killswitch that I could use?

@Poliuk
Copy link

Poliuk commented Aug 29, 2023

Yes, folks can disable the feature if they're not interested in it or if they're already using another Lightbox feature or plugin (like Gutenberg).

Perfect then. My last suggestion would be to let the JetPack users know that the default Lightbox is being disabled.

hooking into render_block_core/image isn't enough because the UI remains in the block editor. Do you have any suggestions there? Would there be a killswitch that I could use?

I'll have to defer to @michalczaplinski or @artemiomorales for this one.

@cuemarie
Copy link

cuemarie commented Oct 10, 2023

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:

When images added to a Gallery block have "Expand on Click" enabled, users need to click twice to exit to the page/post: first click to exit the carousel, and a second click to exit the lightbox.

📌 ACTIONS

  • With that in mind, I upped the priority from Low to Normal here for now.

Any updates to know about this, beyond the above discussion?

@cuemarie cuemarie added [Pri] Normal [Feature Group] Editor Experience Features related to Gutenberg integration on WordPress.com. and removed [Pri] Low labels Oct 10, 2023
@mrfoxtalbot
Copy link

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!

Copy link
Contributor

Support References

This comment is automatically generated. Please do not edit it.

  • 7545923-zen

@github-actions github-actions bot added the Customer Report Issues or PRs that were reported via Happiness. Previously known as "Happiness Request". label Jan 16, 2024
@jeherve
Copy link
Member

jeherve commented Jan 16, 2024

@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!

@michalczaplinski
Copy link

michalczaplinski commented Jan 16, 2024

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 theme.json:

	"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

@jeherve
Copy link
Member

jeherve commented Jan 16, 2024

@michalczaplinski Thank you!
Can it be done from outside theme.json, like from another plugin, instead? We're hoping to disable the feature from inside the Jetpack plugin.

@michalczaplinski
Copy link

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 lightbox setting seems to get overwritten when I try to console log it around here so maybe there is another bug lurking or perhaps I'm missing something.

@mrfoxtalbot
Copy link

Thank you so much, @jeherve and @michalczaplinski for looking into this.

@michalczaplinski
Copy link

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 );

@annezazu
Copy link

I just ran into this on my nomad.blog simple site on WPCOM:

Screen.Recording.2024-03-12.at.9.14.09.AM.mov

Why aren't we finding ways to work with Core here and to ensure they work well together?

@simison
Copy link
Member

simison commented Mar 12, 2024

I'm afraid there is no "easy" and declarative way to disable the lightbox.

@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.

@jeherve
Copy link
Member

jeherve commented Mar 12, 2024

Related discussion: p1710260245886189-slack-CBG1CP4EN

Related Gutenberg issue: WordPress/gutenberg#59797

jeherve added a commit that referenced this issue Mar 25, 2024
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.
jeherve added a commit that referenced this issue Mar 28, 2024
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.
@djcowan
Copy link

djcowan commented Apr 10, 2024

Apologies for dumping this here randomly.
Carousel functionality may conflict with Wordpress 6.5 "Expand on click"
Reference gutenberg/issues/55407
Not sure if I shoud create a new issue and on which repository it belongs

@jeherve
Copy link
Member

jeherve commented Apr 10, 2024

@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!

TimBroddin pushed a commit that referenced this issue Apr 10, 2024
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Customer Report Issues or PRs that were reported via Happiness. Previously known as "Happiness Request". [Feature] Carousel A fullscreen modal appearing when clicking on an image in a gallery or tiled gallery. [Feature Group] Editor Experience Features related to Gutenberg integration on WordPress.com. [Focus] Compatibility Ensuring our products play well with third-parties [Platform] Atomic [Platform] Simple [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Pri] Normal Triaged [Type] Bug When a feature is broken and / or not performing as intended
Development

Successfully merging a pull request may close this issue.

9 participants