Image: Add lightbox using directives. #50373

artemiomorales · 2023-05-05T11:10:57Z

Updated testing instructions and screengrabs on 24 May 2023.
Updated testing instructions with new Gutenberg Experiments setting on 30 May 2023.

What?

This pull request is a replacement for #49621, which was closed by Github's UI due to the target branch being merged to trunk. We can continue the conversation here.

This is part of the experiments section with the interactivity API in revising core blocks.
It uses directives from the Directives hydration system

Why?

It would enable native support for image lightboxes, a commonly used feature in many websites, without the use of jQuery or other external libraries.

How?

It adds directives on click to images, presenting a larger version of the image along with an overlay, which can be hidden with an additional click, by scrolling, or pressing the escape key.

Testing Instructions

in the admin, enable Gutenberg > Experiments > Interactivity API and Behaviors UI
Add an image to a post
Test enabling and disabling the lightbox using image's Advanced panel
View the post
Click on the image

Testing Instructions for Keyboard

Tab to the image
Press Enter to open the lightbox
Test pressing Escape to close the lightbox; also test pressing Enter on the close button

Notes

This is an early implementation; the idea is for themes to be able to enable or disable the lightbox behavior, for child themes to be able to override that behavior, and for users to be able to toggle lightboxes for all images sitewide or at the block level.
This is part of a broader initiative to enable adding Interactivity API behaviors to components using the editor.
Currently, we can just toggle the lightbox for a particular image at the block level under the Advanced panel (see below)

Screenshots or screencast

Editor

lightbox-dropdown.mp4

Frontend

lightbox-fade.mp4

gziolo · 2023-05-10T09:39:39Z

I paired with Artemio yesterday, and I wanted to reshare some high-level comments:

I think the lightbox should be disabled by default for a start, the user would need to explicitly enable it, or with the work on Block Behaviors (Behaviors UI #49972), it’s the theme author who could enable it for all Image blocks with a single setting.
We should consider putting the lighbox behind the feature flag that needs to be enabled on the Gutenberg plugin settings page. We could reuse the same flag as the Navigation block (Experimental: Navigation block with the Interactivity API #50041). The reason is that they both use Interactivity API, so it would be beneficial to have a shared way to control whether the experience is present on the site or not.

Otherwise, I think that this PR is shaping up nicely, and we should start testing the logic to plan for landing it in the Gutenberg plugin.

artemiomorales · 2023-05-11T07:10:17Z

@gziolo I've just pushed up the tests.
The two tests potentially missing are 1.) tabbing to the lightbox and 2.) having multiple lightboxes on the page at once.

I'm not sure if the first is necessary and haven't figured out a way to tab through the page elements and compare locators to determine if the lightbox button is selected.

The second would be nice to cover, and I'll get to it when I can.

For the moment, syncing this with WIP: Behaviors UI seems more important than either of these tests, so I'm going to start looking at that.

gziolo · 2023-05-11T07:47:29Z

packages/block-library/src/image/interactivity.js

+							event.preventDefault();
+						}
+
+						if ( escapeKeyPressed || isTabKeyPressed ) {


What's the rationale behind using a Tab key to close the lightbox?

The handling is for the container with the dialog role, so I assumed that you the Escape key would trigger that, or pressing Enter or Space on the close button.

This comes from the original set of recommendations you posted in the initial PR.

Either of these:
a. Tabbing beyond the last link in the lightbox or shift-tabbing before the first link should close the lightbox
b. Tabbing should cycle within the actionable links within the lightbox until user clicks a close link.

Since we only have one actionable item inside the lightbox, the close button, it made sense to me to go with option A rather than create a focus trap.

Also, rather than tabbing to some unspecified next link on the page, I thought it made sense to put focus on the enable lightbox button when the lightbox closes, that way folks don't lose their place.

+1 to @gziolo's comment. Only Esc, Enter, and Space should close the dialog.

andrewhayward · 2023-05-12T16:12:31Z

I wanted to make sure that @joedolson and @afercia's accessibility comments didn't get lost in the move, and mirror their thoughts around modal dialog interaction.

To summarise, and as previously stated, while the interaction might seem simple and straightforward, and the user experience not that of a typical modal dialog, semantically it essentially just that. Which means we should be careful to provide appropriate functionality.

Any trigger that opens the image overlay should (semantically) present as a button, and be labelled appropriately to explain what it does. This means we cannot use the image itself as the trigger, because that should (semantically) present as an image, and have a label reflecting what it actually is ('alt text').

We could hide the trigger button until it receives focus, or the image is hovered, but that brings its own discovery issues and cognitive accessibility questions. Ideally it would be present at all times.

When open, everything behind it should be inert. There should be an obvious way to close the overlay (typically an × button), as well as respecting the esc key. Generally, tapping the overlay background closes it too. Focus should return to the element that opened it; most likely the original trigger button.

I'm not sure how much of that has been covered, but comments in the previous PR would suggest some of it is already tackled. The most important thing to remember is that while it might visually be a pretty lightweight experience, we should still make sure the appropriate controls and affordances are in place to cover different access needs.

joedolson

Thanks for working on this! I've flagged a number of continuing accessibility issues that'll need to be handled before this can move forward.

packages/block-library/src/image/index.php

joedolson · 2023-05-12T20:11:50Z

packages/block-library/src/image/interactivity.js

+							event.preventDefault();
+						}
+
+						if ( escapeKeyPressed || isTabKeyPressed ) {


+1 to @gziolo's comment. Only Esc, Enter, and Space should close the dialog.

joedolson · 2023-05-12T20:16:46Z

packages/block-library/src/image/index.php

+					data-wp-context='{ "core": { "image": { "initialized": false, "lightboxEnabled": false, "lastFocusedElement": null } } }'>
+						<button aria-haspopup="dialog" aria-label="$aria_label" data-wp-on.click="actions.core.image.showLightbox">
+							$content
+						</button>


Image captions can contain links; with this model, those links would be nested inside a button, which is a) not allowed and b) creates really unpredictable user interactions.

This needs some further working out to cover scenarios like captions containing links and images that are linked.

Ok I've updated the implementation so the image is no longer inside of a button, so that should cover the caption scenario.

As for the image being a link, the lightbox functionality will be overriden and its markup won't render if the user configures the image to be a link.

packages/block-library/src/image/index.php

joedolson · 2023-05-12T20:27:55Z

packages/block-library/src/image/interactivity.js

+							( event.type === 'click' &&
+								event.pointerType === '' )
+						) {
+							context.core.image.lastFocusedElement.focus();


In my testing, this is failing for the Enter and Space keys when a screen reader is running; the focus is set correctly using Esc and Tab (although tab shouldn't close the dialog.) This is probably because the screen reader is intercepting keypresses at some level. Tested with NVDA/Chrome and NVDA/Firefox. Focus is being lost.

In which case we don't want to focus the lastFocusedElement when the lightbox is hidden? Couldn't we just move the context.core.image.lastFocusedElement.focus(); outside of the conditional?

I was just testing it and removing the conditional, and just calling context.core.image.lastFocusedElement.focus(), seems to solve the issue with NVDA. It works with Enter and Space as well.

Don't we want to focus the last element when the lightbox is hidden after scrolling or when clicking outside?

Don't we want to focus the last element when the lightbox is hidden after scrolling or when clicking outside?

@SantosGuillamot Thanks for looking into this! I previously didn't take this approach because setting the focus when closing the dialog with a mouse click produced a visible focus outline, which would have been distracting for mouse users. However, in the latest push, I moved the image outside of the button element, which fortunately also resolves the UI clutter, so I've been able to remove this conditional 👍

draganescu · 2023-05-16T16:56:15Z

Hi @andrewhayward - I am no accessibility expert just a dev who reading this bit:

We could hide the trigger button until it receives focus, or the image is hovered, but that brings its own discovery issues and cognitive accessibility questions.

... felt a bit caught off guard. The fact that "hide the trigger button until it receives focus" which we do all over the place via a VisuallyHidden component is problematic for cognitive accessibility needs raises a question to me:

how much of web accessibility is making web content (be it document or application) available to assisitive technology users and how much of the scope of all accessibility needs are the responsibility of the assistive technology itself to handle?

In particular, using VisuallyHidden elements that are semantic and focusable, just not visible, should be something that the assisitive technology used by the user should handle for cognitive accessibility needs and describe the semantics - which exist - in a way that is predictable and actionable including in cognitive accessibility needs contexts. Is this a wrong line of thinking?

andrewhayward · 2023-05-17T12:02:57Z

Thanks for follow-up, @draganescu. Great questions.

How much of web accessibility is making web content (be it document or application) available to assistive technology users and how much of the scope of all accessibility needs are the responsibility of the assistive technology itself to handle?

In particular, using VisuallyHidden elements that are semantic and focusable, just not visible, should be something that the assistive technology used by the user should handle for cognitive accessibility needs and describe the semantics - which exist - in a way that is predictable and actionable including in cognitive accessibility needs contexts. Is this a wrong line of thinking?

Very often when we think about accessibility, we consider the technologies that users might make use of to access content, but fall short of thinking about the users themselves. Cognitive accessibility falls into a murky area between traditional accessibility standards and UX.

There's a lot to go into that would be a whole discussion in its own right, but the biggest takeaway in this context is that not everyone who has particular access needs meets them with assistive technology. So your line of thinking isn't necessarily wrong, just maybe not broad enough either. We cannot know who is accessing content, or how, so we have a responsibility to make it as clear and straightforward as we can, both semantically and visually.

gziolo · 2023-05-17T12:43:39Z

With 6f2d112, there might be slight changes necessary in the webpack config and the PHP file that register the view script. The built file is going to be located in build/block-library/blocks/image/interactivity.min.js.

packages/block-library/src/image/interactivity.js

packages/block-library/src/image/block.json

artemiomorales · 2023-05-11T07:57:09Z

packages/block-library/src/image/interactivity.js

+							event.preventDefault();
+						}
+
+						if ( escapeKeyPressed || isTabKeyPressed ) {


This comes from the original set of recommendations you posted in the initial PR.

Either of these:
a. Tabbing beyond the last link in the lightbox or shift-tabbing before the first link should close the lightbox
b. Tabbing should cycle within the actionable links within the lightbox until user clicks a close link.

Since we only have one actionable item inside the lightbox, the close button, it made sense to me to go with option A rather than create a focus trap.

Also, rather than tabbing to some unspecified next link on the page, I thought it made sense to put focus on the enable lightbox button when the lightbox closes, that way folks don't lose their place.

artemiomorales · 2023-05-18T13:12:04Z

packages/block-library/src/image/interactivity.js

+							( event.type === 'click' &&
+								event.pointerType === '' )
+						) {
+							context.core.image.lastFocusedElement.focus();


Don't we want to focus the last element when the lightbox is hidden after scrolling or when clicking outside?

@SantosGuillamot Thanks for looking into this! I previously didn't take this approach because setting the focus when closing the dialog with a mouse click produced a visible focus outline, which would have been distracting for mouse users. However, in the latest push, I moved the image outside of the button element, which fortunately also resolves the UI clutter, so I've been able to remove this conditional 👍

artemiomorales · 2023-05-19T10:19:23Z

packages/block-library/src/image/index.php

+					data-wp-context='{ "core": { "image": { "initialized": false, "lightboxEnabled": false, "lastFocusedElement": null } } }'>
+						<button aria-haspopup="dialog" aria-label="$aria_label" data-wp-on.click="actions.core.image.showLightbox">
+							$content
+						</button>


Ok I've updated the implementation so the image is no longer inside of a button, so that should cover the caption scenario.

As for the image being a link, the lightbox functionality will be overriden and its markup won't render if the user configures the image to be a link.

artemiomorales · 2023-05-19T10:40:50Z

I've pushed a batch of changes that covers @joedolson's accessibility feedback. I've marked 'resolved' for the two simplest issues; for the rest, I see them as handled on my end but believe this PR is ready for another review.

Any trigger that opens the image overlay should (semantically) present as a button, and be labelled appropriately to explain what it does. This means we cannot use the image itself as the trigger, because that should (semantically) present as an image, and have a label reflecting what it actually is ('alt text').

This feedback from @andrewhayward has also been addressed, namely the image itself is no longer the trigger — instead the button is overlaid on top of the image. The button also has an aria-label, though does not have a visible label in the UI pending additional explorations and discussion with design folks.

Also, I just realized a few of my comments from several days ago never became public due to a snafu on my end with the Github UI. That's now been updated, so @gziolo and @SantosGuillamot should see responses to their questions from a while back. Sorry for the delay on that!

Important: I've also rebased on trunk and the lightbox is now hidden behind an experimental flag. To activate it, enable Gutenberg > Experiments > Core blocks in the admin.

SantosGuillamot · 2023-05-19T11:49:21Z

On another note, I was experimenting with the possibility of using a zooming animation for the lightbox, just in case we want to consider it. The idea was to create a similar effect to the UX of the View Transitions API. This is how the experiment looks like at this moment (I did it before the latest changes where @artemiomorales changed the trigger) :

Desktop

Lightbox.Zoom.in.-.Desktop.mp4

Mobile

Lightbox.Zoom.In.-.Mobile.mp4

Explanation

EDIT: I changed the code to use CSS variables which makes the implementation easier, so this video is a bit outdated.

The code of that experiment can be found in this branch, and I also made a video explaining it a bit.

https://www.loom.com/share/b7138ad47a6a42adb7808f3b07a193e4

If at the end we want to follow this approach, I'm happy to help polish the code 🙂

SantosGuillamot

I left some comments so far, although I'd like to do a deeper review later today. Thanks for taking care of it, Artemio 🙂

packages/block-library/src/image/interactivity.js

packages/block-library/src/image/index.php

packages/block-library/src/image/block.json

packages/block-library/src/image/index.php

carolinan · 2023-05-25T03:28:03Z

packages/block-library/src/image/index.php

+
+	if ( ! empty( $experiments['gutenberg-interactivity-api-core-blocks'] ) && 'none' === $link_destination && $lightbox ) {
+
+		$aria_label = 'Open image lightbox';


Is there a reason for not making this text translatable?

It definitely should be translated 👍

carolinan · 2023-05-25T03:48:47Z

packages/block-library/src/image/index.php

+		return
+			<<<HTML
+				<div class="wp-lightbox-container"
+					data-wp-island


Most of these data attributes are self explanatory, but this "island" part is not, I think this could be improved to be easier to read?

This one is special and I run into an issue with it as I had to learn it enables Interactivity API runtime for the block.

That's a good point.

Island is a term that refers to a part of the site that is hydrated and therefore becomes interactive.

The idea of an "islands architecture" was first coined by Etsy's frontend architect Katie Sylor-Miller in 2019, and expanded on in this post by Jason Miller (Preact's creator). Since then, multiple JavaScript frameworks have adopted it: (1, 2, 3...).

Maybe we can change it to data-wp-interactive. Would that be easier to read, or do you have any other suggestions?

Yes I think data-wp-interactive would be clearer.

Thanks, Carolina 🙂

I opened this issue to discuss it.

For context, the full version of the Interactivity API is still in that repo, but we'll move everything to the Gutenberg repo after the WP 6.3 feature freeze, including the codebase and discussions.

gziolo added [Feature] Interactivity API API to add frontend interactivity to blocks. [Block] Image Affects the Image Block [Type] Enhancement A suggestion for improvement. labels May 5, 2023

SantosGuillamot mentioned this pull request May 8, 2023

Experimental: Navigation block with the Interactivity API #50041

Merged

This was referenced May 8, 2023

Experiment: Add lightbox to Image block using directives #49621

Closed

Tracking Issue - Interactivity API runtime for core blocks #49868

Closed

Add test to verify image appears on frontend #50472

Merged

gziolo reviewed May 11, 2023

View reviewed changes

artemiomorales requested review from alexstine and joedolson May 11, 2023 09:55

michalczaplinski mentioned this pull request May 12, 2023

Behaviors UI #49972

Merged

joedolson requested changes May 12, 2023

View reviewed changes

artemiomorales force-pushed the experiment/image-with-interactivity-api branch from 7b76791 to 6f53c0c Compare May 19, 2023 10:14

artemiomorales commented May 19, 2023

View reviewed changes

SantosGuillamot reviewed May 23, 2023

View reviewed changes

gziolo reviewed May 23, 2023

View reviewed changes

packages/block-library/src/image/index.php Outdated Show resolved Hide resolved

SantosGuillamot mentioned this pull request May 23, 2023

useSignalEffect is not working as expected when using visibility: hidden/visible in CSS animations WordPress/block-interactivity-experiments#237

Closed

artemiomorales force-pushed the experiment/image-with-interactivity-api branch from ef6f311 to 01d95b0 Compare May 23, 2023 12:48

artemiomorales changed the base branch from trunk to behaviors-ui May 23, 2023 12:50

cbravobernal force-pushed the behaviors-ui branch from 47e347d to dd0db77 Compare May 23, 2023 13:25

michalczaplinski added 2 commits May 23, 2023 15:44

Add behaviors to the core theme.json

bd53de2

Add behaviors to json schemas for theme.json

bd43bf6

Merge branch 'trunk' into experiment/image-with-interactivity-api

1eaffe1

SantosGuillamot enabled auto-merge (squash) May 24, 2023 21:05

SantosGuillamot merged commit ac13b94 into WordPress:trunk May 24, 2023
48 checks passed

github-actions bot added this to the Gutenberg 15.9 milestone May 24, 2023

carolinan reviewed May 25, 2023

View reviewed changes

luisherranz deleted the experiment/image-with-interactivity-api branch May 25, 2023 09:16

luisherranz mentioned this pull request May 25, 2023

Rename data-wp-island to data-wp-interactive WordPress/block-interactivity-experiments#238

Closed

afercia mentioned this pull request May 25, 2023

Experiment: Improvements for the Image block lightbox #50959

Closed

artemiomorales mentioned this pull request May 25, 2023

Experiment: Duotone filter in Image block lightbox not working #50973

Closed

cbravobernal changed the title ~~Experiment: Add lightbox to Image block using directives (reopened)~~ Experiment: Add lightbox to Image block using directives May 26, 2023

cbravobernal changed the title ~~Experiment: Add lightbox to Image block using directives~~ Image: Add lightbox using directives. May 26, 2023

artemiomorales mentioned this pull request May 29, 2023

Experiment: Add zoom and animation toggle for image lightbox #51055

Closed

juanfra mentioned this pull request May 29, 2023

Improve image block lightbox escaping #51061

Merged

ramonjd added the Backport to WP 6.6 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label May 29, 2023

artemiomorales mentioned this pull request May 30, 2023

Experiment: Put directives on <figure> element and insert lightbox as its child #51089

Merged

ramonjd added Needs PHP backport Needs PHP backport to Core and removed Backport to WP 6.6 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta labels May 31, 2023

artemiomorales mentioned this pull request May 31, 2023

Add aria-describedby if image has no alt but has caption #43059

Closed

This was referenced May 31, 2023

Tracking issue: Image Lightbox #51132

Open

Bug fix: Vertical images in experimental lightbox #51164

Closed

ramonjd assigned artemiomorales and SantosGuillamot Jun 2, 2023

ramonjd mentioned this pull request Jun 5, 2023

Plugin: Synchronize PHP changes for WordPress 6.3 release #51077

Closed

liviopv mentioned this pull request Jun 12, 2023

Feature Request: Full screen (or big screen) mode for Slideshow block Automattic/jetpack#24537

Open

cbravobernal removed the Needs PHP backport Needs PHP backport to Core label Jun 15, 2023

jeherve mentioned this pull request Aug 29, 2023

Carousel: conflict with Gutenberg Lightbox behavior Automattic/jetpack#32668

Closed

artemiomorales mentioned this pull request Oct 9, 2023

Image block lightbox: do not overlay the image with a transparent button #55097

Closed

AnthonyLedesma mentioned this pull request Nov 16, 2023

ISBAT use Lightbox with wordpress.com - Jetpack godaddy-wordpress/coblocks#2577

Open

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Image: Add lightbox using directives. #50373

Image: Add lightbox using directives. #50373

artemiomorales commented May 5, 2023 •

edited

Loading

gziolo commented May 10, 2023 •

edited

Loading

artemiomorales commented May 11, 2023

gziolo May 11, 2023 •

edited

Loading

artemiomorales May 11, 2023

joedolson May 12, 2023

andrewhayward commented May 12, 2023

joedolson left a comment

joedolson May 12, 2023

joedolson May 12, 2023

artemiomorales May 19, 2023

joedolson May 12, 2023

SantosGuillamot May 18, 2023

SantosGuillamot May 18, 2023

artemiomorales May 18, 2023

draganescu commented May 16, 2023 •

edited

Loading

andrewhayward commented May 17, 2023

gziolo commented May 17, 2023 •

edited

Loading

artemiomorales May 11, 2023

artemiomorales May 18, 2023

artemiomorales May 19, 2023

artemiomorales commented May 19, 2023

SantosGuillamot commented May 19, 2023 •

edited

Loading

SantosGuillamot left a comment

carolinan May 25, 2023

gziolo May 25, 2023

carolinan May 25, 2023

gziolo May 25, 2023

luisherranz May 25, 2023

carolinan May 25, 2023

luisherranz May 25, 2023


		if ( ! empty( $experiments['gutenberg-interactivity-api-core-blocks'] ) && 'none' === $link_destination && $lightbox ) {

		$aria_label = 'Open image lightbox';

Image: Add lightbox using directives. #50373

Image: Add lightbox using directives. #50373

Conversation

artemiomorales commented May 5, 2023 • edited Loading

What?

This pull request is a replacement for #49621, which was closed by Github's UI due to the target branch being merged to trunk. We can continue the conversation here.

Why?

How?

Testing Instructions

Testing Instructions for Keyboard

Notes

Screenshots or screencast

Editor

Frontend

gziolo commented May 10, 2023 • edited Loading

artemiomorales commented May 11, 2023

gziolo May 11, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewhayward commented May 12, 2023

joedolson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

draganescu commented May 16, 2023 • edited Loading

andrewhayward commented May 17, 2023

gziolo commented May 17, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

artemiomorales commented May 19, 2023

SantosGuillamot commented May 19, 2023 • edited Loading

Desktop

Mobile

Explanation

SantosGuillamot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

artemiomorales commented May 5, 2023 •

edited

Loading

gziolo commented May 10, 2023 •

edited

Loading

gziolo May 11, 2023 •

edited

Loading

draganescu commented May 16, 2023 •

edited

Loading

gziolo commented May 17, 2023 •

edited

Loading

SantosGuillamot commented May 19, 2023 •

edited

Loading