Enabling lightbox on images in a Gallery block breaks the gallery layout

[ndiego]

Description

Enabling the new lightbox behavior on Image blocks inside of a Gallery block breaks the gallery's layout.

Step-by-step reproduction instructions

1. Create a Gallery with 2+ images
2. Enable the lightbox behavior for each image
3. Set the Gallery block to have two columns
4. Save and view the front end, notice that the Gallery layout is not correctly applied. This is because there is now an extra div around each figure.

Screenshots, screen recording, code snippet

Editor:

Front end:

Environment info

• WordPress 6.2
• Gutenberg trunk

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

[ndiego]

cc @luisherranz

[luisherranz]

Thanks @ndiego. I was worried that extra div would cause some issues 😄

@artemiomorales, what do you need to get rid of it? Can you place the directives directly in the figure? Or is it needed for CSS?

[artemiomorales]

@luisherranz I think it's possible to get rid of it. But to do so, we'd need to use the Tag Processor to place the directives directly on the figure, then do some hefty parsing and processing of the figure to set the wp-lightbox-overlay as a child, which seems potentially brittle.

Here is the lightbox markup; the figure HTML is contained within the $body_content and $content:

<div class="wp-lightbox-container"
	data-wp-island
	data-wp-context='{ "core": { "image": { "initialized": false, "lightboxEnabled": false } } }'>
		$body_content
		<div data-wp-body="" class="wp-lightbox-overlay"
			data-wp-bind.role="selectors.core.image.roleAttribute"
			aria-label="$dialog_label"
			data-wp-class.initialized="context.core.image.initialized"
			data-wp-class.active="context.core.image.lightboxEnabled"
			data-wp-bind.aria-hidden="!context.core.image.lightboxEnabled"
			data-wp-bind.aria-modal="context.core.image.lightboxEnabled"
			data-wp-effect="effects.core.image.initLightbox"
			data-wp-on.keydown="actions.core.image.handleKeydown"
			data-wp-on.mousewheel="actions.core.image.hideLightbox"
			data-wp-on.click="actions.core.image.hideLightbox"
			>
				<button type="button" aria-label="$close_button_label" class="close-button" data-wp-on.click="actions.core.image.hideLightbox">
					$close_button_icon
				</button>
				$content
				<div class="scrim" style="background-color: $background_color"></div>
		</div>
</div>

And by the time the lightbox logic runs inside of the render_callback, the figure HTML has already been generated. We receive it inside of $content:

function render_block_core_image( $attributes, $content ) {
	$processor = new WP_HTML_Tag_Processor( $content );
	/// ... Lightbox code continues here
}

Options:

1. Is it possible to modify the figure HTML further up the chain? That could be an option.
2. We could roll our own solution to process the HTML using a combination of regexes and Tag Processor logic.
3. We could wait until the HTML Processor is created at some point in the future and/or help develop the HTML Processor with this use case in mind.
4. I believe the intent is to incorporate the lightbox with the gallery block anyway, so perhaps we can accept this as a known limitation for now and then fix it all at once when the time comes.

I'm inclined to take option 4 and even start exploring solutions for the lightbox gallery relatively soon.

Thoughts?

[SantosGuillamot]

Asking from a lack of knowledge: cannot we just use the Tag Processor to add the directives to the figure parsing the $content?

I mean at this point adding something like this:

$content = $processor->get_updated_html();
$w = new WP_HTML_Tag_Processor( $content );
$w->next_tag(
	array(
		'tag_name'   => 'FIGURE',
	)
);
$w->add_class( 'wp-lightbox-container' );
$w->set_attribute( 'data-wp-island', '' );
$w->set_attribute( 'data-wp-context', '{ "core": { "image": { "initialized": false, "lightboxEnabled": false } } }' );
$content = $w->get_updated_html();

Then the HTML returned could remain the same and just remove the parent <div>:

<<<HTML
$body_content
<div data-wp-body="" class="wp-lightbox-overlay"
    data-wp-bind.role="selectors.core.image.roleAttribute"
    aria-label="$dialog_label"
    data-wp-class.initialized="context.core.image.initialized"
    data-wp-class.active="context.core.image.lightboxEnabled"
    data-wp-bind.aria-hidden="!context.core.image.lightboxEnabled"
    data-wp-bind.aria-modal="context.core.image.lightboxEnabled"
    data-wp-effect="effects.core.image.initLightbox"
    data-wp-on.keydown="actions.core.image.handleKeydown"
    data-wp-on.mousewheel="actions.core.image.hideLightbox"
    data-wp-on.click="actions.core.image.hideLightbox"
    >
        <button type="button" aria-label="$close_button_label" class="close-button" data-wp-on.click="actions.core.image.hideLightbox">
            $close_button_icon
        </button>
        $content
        <div class="scrim" style="background-color: $background_color"></div>
</div>
HTML;

Wouldn't an approach similar to this work? I guess it would also need some CSS changes if the parent div is removed.

EDIT: Oh okay, I just realized that we are missing a wrapper and that can be an issue. I think I understand your point now better.

Maybe we can use something like the following and just return $body_content?

$body_content = preg_replace( '/<\/figure>/', $lightbox_html . '</figure>', $body_content );

[gziolo]

I was worried that extra div would cause some issues 😄

It's also worth noting that everything works fine in the block editor because there is no wrapping div. I'm sure there are more features that won't work as expected because existing CSS styles assume that the HTML tag with the wp-block-image class is the most outer element of the block. Example would be the block alignment set to left.

The editor:

Frontend without the lightbox enabled:

Frontend with the lightbox enabled:

---

When you apply more styling it becomes even more broken.

Editor:

Frontend:

---

Full-width align doesn't work as expected, too.

Editor:

Frontend:

[luisherranz]

Maybe we can use something like the following and just return $body_content?

$body_content = preg_replace( '/<\/figure>/', $lightbox_html . '</figure>', $body_content );

This is what I was thinking as well, although I have the impression that we already tried that 🤔

Anyway, is this worth a try, @artemiomorales?

[artemiomorales]

@luisherranz @SantosGuillamot

Sure thing, pull request created here.

It works as far as I can tell. I suppose it should work for most use cases, though would break if users say include the string </figure> in their caption.

[artemiomorales]

Closed by #51089