Bug fix: Vertical images in experimental lightbox

[artemiomorales]

What?

It revises the CSS to fix the display of vertical images in the experimental lightbox.

Why?

Address #51125
Vertical images should be contained within the viewport when being viewed in the lightbox.

How?

It revises the CSS to make sure the image doesn't expand beyond the appropriate borders of the lightbox. It also improves the style for the caption.

Testing Instructions

1. Ensure the Interactivity API and Behaviors UI setting is enabled in Gutenberg > Experiments in the admin.
2. Create a new post and an image whose height is larger than its width; set the image's resolution to large or full size.
3. Activate the image's lightbox via Advanced > Behaviors > Lightbox.
4. Publish and view the post.
5. Click on the image; see that the image is fully visible and contained within the lightbox.

Testing Instructions for Keyboard

N/A

Screenshots or screencast

[SantosGuillamot]

Do we need this? Is not 0 the default value?

[artemiomorales]

Yes, we need this, as the default value is actually auto. Without it, the image will expand take up the minimum size of its content and overflow into its parent container, breaking the flexbox layout.

[SantosGuillamot]

Oh, I read here that the default value is true, and removing those lines didn't seem to affect. That's why I thought they weren't necessary.

[artemiomorales]

Interesting — MDN says that the initial value is auto, and the behavior I saw was that of auto as far as I can tell.

[SantosGuillamot]

Approving with a small comment. I've tested it and it is definitely an improvement.

I have one question: right now, if you have a really big image with the lightbox (for example if you set it to full width or align wide), it gets smaller and it doesn't cover as much space as possible when you click on it. Is that the expected behavior or should it get the max-height?

[artemiomorales]

I have one question: right now, if you have a really big image with the lightbox (for example if you set it to full width or align wide), it gets smaller and it doesn't cover as much space as possible when you click on it. Is that the expected behavior or should it get the max-height?

@SantosGuillamot I believe it depended on the size of the image — if it was wide enough to cover the full screen, then it would have done so and been capped at the size of the container.

That being said, based on your comment, I decided to add another adjustment so that horizontal images expand to fit the full width of the container. This will stretch and distort thumbnails since we're not yet putting the full resolution image in the lightbox (related issue here), but is maybe more in line with expected behavior of bigger horizontal images for now.

Do you think this is a good change, or should we revert to the way it was working before?

[SantosGuillamot]

I will review it in more detail, but I'd like to clarify first what is the expected behavior of the lightbox.

Should it always display the largest available size of image as explained here? My guess, from a total lack of knowledge, is that it should. We shouldn't make the image bigger than expected I assume, which is currently happening with this implementation. I may be mistaken but I believe it should behave as the attachment page but without going to a new link. Is that correct?

[SantosGuillamot]

I was thinking a bit more about this and I believe this can be fixed by removing the sizes property from the overlay image. This way, it should get the highest size possible and it will fit into the page.

What about going back to the previous implementation and doing something like this:

// Add directive to remove the sizes limitation.
$m = new WP_HTML_Tag_Processor( $content );
$m->next_tag( 'img' );
$m->set_attribute( 'sizes', '' );
$modal_content = $m->get_updated_html();

I quickly tested and it seems to work fine. I would probably remove the padding we are adding.

The main problem with this approach is that it would download the biggest image possible, so maybe it is not right. It would be great if we download the lightbox image when it is clicked/hovered.

Additionally, while comparing it with the media pages I wondered, should the lightbox include the caption? 🤔

[SantosGuillamot]

Sorry for the spam 😄 I've been thinking a bit more.

The main problem with this approach is that it would download the biggest image possible, so maybe it is not right. It would be great if we download the lightbox image when it is clicked/hovered.

This could be solved using directives that only change the attribute when the lightbox is initialized. At this moment, it could be done this way:

In PHP

$m = new WP_HTML_Tag_Processor( $content );
$m->next_tag( 'img' );
$m->set_attribute( 'data-wp-effect', 'effects.core.image.initLightboxImage' );
$modal_content = $m->get_updated_html();

And the initLightboxImage could be like this:

initLightboxImage: ( { context, ref } ) => {
	if ( context.core.image.initialized ) {
		ref.setAttribute( 'sizes', '' );
	}
},

---

Ideally, I believe we should use a selector that reads the current ref, but that isn't possible right now. We could do something more declarative like this:

In PHP

$m = new WP_HTML_Tag_Processor( $content );
$m->next_tag( 'img' );
$m->set_attribute( 'data-wp-bind--sizes', 'selectors.core.image.imageSizes' );
$modal_content = $m->get_updated_html();

And the selectors could be like this:

imagesSizes: ( { context, ref } ) => {
    if ( context.core.image.initialized ) {
        return '';
    } else {
        return ref.sizes;
    }
}					

[artemiomorales]

@SantosGuillamot Thanks for the ideas! The issue I see with this approach is that it doesn't fully solve the issue, as the src and srcset attributes vary depending on which resolution is selected. Here's the markup for the same image with full size, large, medium, and thumbnail resolutions:

[SantosGuillamot]

The issue I see with this approach is that it doesn't fully solve the issue, as the src and srcset attributes vary depending on which resolution is selected.

You're right the values may differ depending on the settings.

Going back to my previous question: What is the purpose of the lightbox? How is this supposed to work with the lightbox? With the current implementation, if I am not mistaken, if I add a low resolution image it creates a big pixelated image if the width is higher than the height.

I have other ideas to make it work like the image media page but, is that what we want? Do we just want to show the same image resolution but with an overlay? Or is it okay pixelating an image?

---

EDIT: Sharing here an idea that could work like the media pages, but I'd like to answer the questions before talking about it and exploring deeper the potential solutions:

Adding the attachment URL to the context in PHP and change the lightbox src depending on it. Adding a selector to check if it has been initialized and don't load the image until then.

// Add directive to expand modal image if appropriate.
$m = new WP_HTML_Tag_Processor( $content );
$m->next_tag( 'img' );
$m->set_attribute( 'data-wp-context', '{ "core": { "image": { "imageSrc": "' . wp_get_attachment_url($attributes['id']) . '"} } }' );
$m->set_attribute( 'data-wp-bind--src', 'selectors.core.image.imageSrc' );
$modal_content = $m->get_updated_html();

And in JS:

imageSrc: ( { context } ) => {
	return context.core.image.initialized
		? context.core.image.imageSrc
		: '';
},

[artemiomorales]

@SantosGuillamot My expectation when I open a lightbox is that it should display the highest resolution version of the image possible appropriate for my screen

If the image is a thumbnail in the content, I believe the lightbox shouldn't display a thumbnail-sized image, but should instead show an image that occupies the full width of the screen at high resolution.

I don't believe it's alright to pixelate the image; what I pushed up was meant to be a point of discussion and maybe a stopgap before fixing the issue for real.

[artemiomorales]

Additionally, while comparing it with the media pages I wondered, should the lightbox include the caption? 🤔

@SantosGuillamot If I recall correctly, there may be accessibility reasons for having it there — here I'm pinging in @andrewhayward for insight

[artemiomorales]

@SantosGuillamot and I spoke this morning and have an implementation that we think will fix this issue as well as address #51171 and #51170.

When opened, the lightbox now loads the image's original attachment URL as the src, which means it gets the largest available file, addressing #51171. Importantly, this src is set dynamically so the image is not loaded unnecessarily by the browser, and it will not stretch the image either.

Removing the caption and padding allows the image to be displayed as large as possible, addressing #51170.

In effect, with these changes, the lightbox acts similarly to the media page, which seems to be a good guideline for expected behavior from the lightbox.

Pinging @jasmussen @SaxonF @jameskoster for any additional feedback.

[artemiomorales]

With the new close button design, I think it makes sense to move these changes to #51206 and continue the discussion regarding all these design fixes there at the same time. Just let me know if you have any questions!