Open jeffgolenski opened 11 months ago
Good issue, would definitely agree that if we are going to have lightbox enabled in galleries, it should either be a nice gallery there as well, chevrons left and right plus arrow key navigation. Or, just be disabled. I could swear there was already an issue for this, but I didn't find one linked from #51132. CC: @michalczaplinski
I don't recall there being an issue for this yet, so thank you @jeffgolenski!
I've linked this issue in Tracking issue: Image Lightbox #51132
+1 on this - very common in all other platforms (webflow, sqs, etc.) would love to see this added soon. Thanks all.
Just to clarify behavior, this seems desirable for multiple images within a gallery, but not necessarily desirable for all images in a post. E.g. if I have multiple galleries (which might be separated by paragraphs of text), I'd expect (or like the ability to configure) the lightbox to let someone flip through the images in a single gallery, but stop them when they get to the end of that gallery so they know to continue reading. This is how the baguetteBox.js lightbox plugin I've been using on my site operates.
Related: https://github.com/WordPress/gutenberg/issues/37652 (this one isn't linked on https://github.com/WordPress/gutenberg/issues/51132 yet, maybe that's the one @jasmussen was thinking of earlier? It probably got lost since it was opened long before the 6.4 lightbox was implemented.)
+1
Another +1 here. A wee bit disappointed that there is no hint of this for 6.6. The single image lightbox works nicely... if gallery support is added, that would be a great thing. Thanks!
I wanted to reference https://github.com/WordPress/gutenberg/issues/56587 as having mockups relevant to this issue, notably this one:
As far as a design for how this could look, at least the experience of it, IMO those designs can work.
As far as the block editor UI for it, I would recommend keeping things as simple as possible. Right now the way you set lightbox on individual images is in the link dialog:
Choose "expand on click". The motivation for this is that it's a click behavior, just like visiting a link is, and opening a lightbox is mutually exclusive to zooming.
It's honestly the same for galleries and gallery lightboxes, so would a good meaningful first step be to add "Expand on click", and the improved dropdown appearance (which features icons) to this dropdown?
It does open a question we have to answer eventually: what happens in a 5 image gallery if all but the third image is set to "expand on click"? My instinct: lightbox arrowkeys would navigate images 1 and 2, then you'd exit, and then you could again swap between 4 and 5, then exit. Of course not ideal, but nevertheless reflecting what you can do.
CC: @mikachan
what happens in a 5 image gallery if all but the third image is set to "expand on click"? My instinct: lightbox arrowkeys would navigate images 1 and 2, then you'd exit, and then you could again swap between 4 and 5, then exit. Of course not ideal, but nevertheless reflecting what you can do.
Can't we remove/hide the "Expand on click" option of the Image block when the image is inside of a Gallery block, and add a global "Expand on click" option to the Gallery block itself?
Can't we remove/hide the "Expand on click" option of the Image block when the image is inside of a Gallery block, and add a global "Expand on click" option to the Gallery block itself?
It veers into exotic use-cases that, if we have to handle them all, might end up adding a lot of complexity.
The above is one of the reasons the gallery block currently has a dropdown for applying properties across all child blocks, and then fires a snackbar when it's done so. E.g. link to and resolution. These aren't properties of the gallery block itself.
Simply to get things moving, if it was possible to simply detect if more than one block in a sequence are set to open in the same lightbox, we'd avoid the complexity of handling the above edge-cases, and would then be able to evaluate whether we need to do more in the future.
I don't object to the idea of starting simple to see how it works and then we'll see from there 🙂
But for the record:
Individual images in a gallery still need to be possible to link to arbitrary URLs.
This is fine, it could keep working just as it does now.
What if you have three images, two of which are set to expand on click, then you multi-select all of them and transform into a gallery; would we need to change their markup in that same transform action?
I don't think anything needs to be changed. "Expand on click" is just one attribute and nothing is added to the markup. All the Interactivity API directives are added dynamically in PHP. I don't think there's any problem with preserving that attribute as it is, and ignoring it when necessary.
So, simply, if the Image is inside a Gallery block, the Image block rendering would ignore its own attribute and consider the global "Expand on click" Gallery attribute instead.
Something like this (not real variable names):
$gallery_expand = $block['context']['galllery_expand_on_click'];
$image_expand = $attributes['expand_on_click'];
if ( $image_expand && ! $gallery_expand ) {
// If the Image is not inside a gallery block and has expand on click...
} elseif ( $gallery_expand ) {
// If the Image is inside a gallery which has expand on click...
}
What if you, using the list view, move an image block from one gallery to another?
That would still work fine because the Image is not modified at all.
Actually, if you have one Image with "Expand on click" and you move it in and out of a Gallery block, it will preserve its "Expand on click" attribute when it's moved outside.
The only thing that would need to happen in the Editor UI is that the "Expand on click" button is hidden when the Image block is inside a Gallery.
Does that make sense?
Again, not opposed to starting simple 🙂
Could work. But would that mean if I have a single image inside the gallery that links to https://wordpress.org, but the gallery itself is set to expand on click, then that hyperlink is simply ignored, but still in the DOM? This is probably the main headache, as screen readers would say one thing, but the click behavior would say another. Let me know if I misunderstood.
We can delete the hyperlink dynamically in PHP during the Image rendering if the $gallery_expand
attribute is set to true
, so that the HTML sent to the browser is correct but the hyperlink is still there if you move the Image block outside of the Gallery block.
We are entering the WordPress' HTML API era where things that were really difficult before are about to become trivial 😄
Gotcha.
From my current vantage point, it sounds like a better user-experience. It also sound like it may still be best to do in two steps; no smarts for first implementation, then the gallery-wide toggle as a followup step. But that's purely an instinct, and ultimately I'll defer to a developer on what is simplest to implement! Thanks for chatting through this.
no smarts for first implementation, then the gallery-wide toggle as a followup step
That makes sense. Thanks Joen!
There are now atleast 3 few issues/PR's that are associated with each other. Gallery: Add lightbox support https://github.com/WordPress/gutenberg/pull/62906 (PR)
Move gallery link controls to the block toolbar https://github.com/WordPress/gutenberg/pull/62762 (PR)
and this issue to have multiple blocks set to open in a lightbox that should connect to create a cohesive carousel.
We really need to break this down into smaller steps. From how I see it.
Thanks to all who have offered their thoughts and to @akasunil and @madhusudhand for starting work on this!
As suggested by @paaljoachim, I do think we should take a step back and consider the various possible scenarios of gallery support for the lightbox in combination with individual image settings and the configurations in theme.json
.
@t-hamano has made a key observation in this issue:
However, we can't simply add the "Expand on click" option to the Gallery block, because we need to check whether the theme supports lightbox. And the current Image block has complex logic to decide whether to show this setting or not. Try searching for "lightbox" in the packages/block-library/src/image/image.js file.
Perhaps the Gallery block needs equivalent logic, and I think it will be necessary to thoroughly verify it in a separate PR to make it happen.
Here I'll go over the scenarios as far as I can see (anyone feel free to elaborate or correct anything).
First I'll review some background just to make sure we're on the same page.
Many theme and plugin authors have their own lightbox implementations that conflict with the lightbox in core — they'll want to disable the lightbox and hide the UI so as not to confuse their end users.
In other cases, site builders who want to use the core lightbox may choose to enable it, yet still hide the UI to streamline the editing experience. Yet other users may prefer to have the lightbox enabled by default, but show the UI so they can disable it on a case by case basis.
For that reason, one is currently allowed to define lightbox settings for the image as follows in theme.json
— allowEditing
controls whether the UI appears, and enabled
determines whether the lightbox is enabled by default:
{
"$schema": "https://schemas.wp.org/trunk/theme.json",
"version": 2,
"settings": {
"blocks": {
"core/image": {
"lightbox": {
"allowEditing": true | false,
"enabled": true | false
}
}
}
}
}
So the possible scenarios for the lightbox functionality are:
For each scenario then, we should consider how the gallery's Expand on click
setting relates. For now, I'll just go over the simplest approach outlined by @jasmussen above — adding the Expand on click
option to the Link to
dropdown.
Scenarios 1, 2, and 3 are fairly straightforward — we can simply respect the individual image settings, and if more than one image has the lightbox enabled, we then show the chevrons to allow flipping from one image to the other.
That just leaves scenario 4. The intention here is to disable the lightbox entirely, which means that I believe it would also make sense in this scenario to hide the Expand on click
option from the gallery dropdown.
There are also edge cases to consider. What if a user enables Expand on click
for a gallery using one theme, then changes their theme to one that has the lightbox UI and functionality disabled? Do we respect the original setting, or simply remember it in case they later switch to a theme that uses the core lightbox again?
There may be other scenarios, particularly as we explore the gallery lightbox settings potentially overriding the image settings.
Anyway, I hope this can help bring some clarity and alignment to the discussion and begin to unearth some of the potential edge cases.
For anyone interested in learning more, there are a few lightbox edge cases regarding image block-level overrides and how they interact with theme.json
covered in https://github.com/WordPress/gutenberg/pull/59890. These may have an effect on the gallery or offer food for thought on scenarios to consider depending on the approach we end up taking.
Just let me know if anything is unclear! I'm also happy to elaborate further or answer any questions on the above 🙏
@artemiomorales Thank you for the detailed explanation. ❤️
That just leaves scenario 4. The intention here is to disable the lightbox entirely, which means that I believe it would also make sense in this scenario to hide the Expand on click option from the gallery dropdown.
I Agree. Gallery Expand on click
should also be controlled by the theme.json
setting when it is being introduced as part of #62762 @akasunil It should be similar to how image currently behaves.
There are also edge cases to consider. What if a user enables Expand on click for a gallery using one theme, then changes their theme to one that has the lightbox UI and functionality disabled? Do we respect the original setting, or simply remember it in case they later switch to a theme that uses the core lightbox again?
Since user settings override, theme.json
, switching a theme in this case would still keep the lightbox functionality, and I think it is expected to be preserved. Following is how an image behaves.
allowEditing
or go to theme.json of the current theme and turn it off.Once disabled, it is no longer available because of the new theme setting.
A gallery should have the option with same behaviour.
What do you think?
Gallery
Expand on click
should also be controlled by thetheme.json
setting when it is being introduced as part of #62762 @akasunil It should be similar to how image currently behaves.
I agree, we might have to follow the same logic that image block have for light-box, I'm planning to create a separate PR for this, which would give us better idea of use-cases where it fails.
Thank you @artemiomorales I appreciate your thorough explanation.
A gallery should have the option with same behaviour.
What do you think?
That sounds good to me.
Something does tell me that adding the lightbox to galleries will increase the potential for conflicts with existing 3rd party lightbox solutions, namely when folks decide to override the gallery lightbox using a block-level setting.
In any case, we can keep an eye out and determine later if a different behavior would be more appropriate.
A gallery should have the option with same behaviour.
What do you think?
I have opened this PR https://github.com/WordPress/gutenberg/pull/64014 to implement lightbox setting on gallery.
What problem does this address?
Currently, if I have several full sized images within a post set to open in a lightbox, it's very tedious to view the next image. Each image in a post must be clicked or tapped, then closed in order for viewer to get to the next one. It's not a fluid experience for the viewer.
If I have multiple images in a post set to open in the lightbox, a viewer should be able to tap on one to open the lightbox, and then be able to fuildly navigate to each image within the lightbox, like a carousel.
What is your proposed solution?
Example: In my post I have many images set to open in the lightbox. If I tap on the first image, the lightbox opens, but to view the others I have to close it and tap the other images to re-open the lightbox.
cc @mtias @jasmussen