From 969d1e5d1584ee3974b03a1041e4800840b8efc7 Mon Sep 17 00:00:00 2001
From: Ricardo Artemio Morales <ric.morales22@gmail.com>
Date: Tue, 17 Oct 2023 14:57:13 -0500
Subject: [PATCH 1/7] Improve focus management

This commit removes the logic to set focus differently
based on event.pointerType and instead sets focus on the
dialog itself when the lightbox opens, and on the lightbox
trigger when the lightbox closes.
---
 packages/block-library/src/image/index.php |  2 ++
 packages/block-library/src/image/view.js   | 28 +++++++---------------
 2 files changed, 11 insertions(+), 19 deletions(-)

diff --git a/packages/block-library/src/image/index.php b/packages/block-library/src/image/index.php
index dd2e72aebe5ef2..261e07c1d1f2cf 100644
--- a/packages/block-library/src/image/index.php
+++ b/packages/block-library/src/image/index.php
@@ -235,6 +235,7 @@ function block_core_image_render_lightbox( $block_content, $block ) {
 	$button =
 		$img[0]
 		. '<button
+			class="lightbox-trigger"
 			type="button"
 			aria-haspopup="dialog"
 			aria-label="' . esc_attr( $aria_label ) . '"
@@ -322,6 +323,7 @@ function block_core_image_render_lightbox( $block_content, $block ) {
             data-wp-on--touchmove="actions.core.image.handleTouchMove"
             data-wp-on--touchend="actions.core.image.handleTouchEnd"
             data-wp-on--click="actions.core.image.hideLightbox"
+			tabindex="0"
             >
                 <button type="button" aria-label="$close_button_label" style="fill: $close_button_color" class="close-button" data-wp-on--click="actions.core.image.hideLightbox">
                     $close_button_icon
diff --git a/packages/block-library/src/image/view.js b/packages/block-library/src/image/view.js
index 30d1259637e3d9..44936bfb428542 100644
--- a/packages/block-library/src/image/view.js
+++ b/packages/block-library/src/image/view.js
@@ -135,7 +135,7 @@ store(
 							false
 						);
 					},
-					hideLightbox: async ( { context, event } ) => {
+					hideLightbox: async ( { context } ) => {
 						context.core.image.hideAnimationEnabled = true;
 						if ( context.core.image.lightboxEnabled ) {
 							// We want to wait until the close animation is completed
@@ -152,16 +152,9 @@ store(
 							}, 450 );
 
 							context.core.image.lightboxEnabled = false;
-
-							// We want to avoid drawing attention to the button
-							// after the lightbox closes for mouse and touch users.
-							// Note that the `event.pointerType` property returns
-							// as an empty string if a keyboard fired the event.
-							if ( event.pointerType === '' ) {
-								context.core.image.lastFocusedElement.focus( {
-									preventScroll: true,
-								} );
-							}
+							context.core.image.lightboxTriggerRef.focus( {
+								preventScroll: true,
+							} );
 						}
 					},
 					handleKeydown: ( { context, actions, event } ) => {
@@ -266,6 +259,10 @@ store(
 				image: {
 					initOriginImage: ( { context, ref } ) => {
 						context.core.image.imageRef = ref;
+						context.core.image.lightboxTriggerRef =
+							ref.parentElement.querySelector(
+								'.lightbox-trigger'
+							);
 						if ( ref.complete ) {
 							context.core.image.imageLoaded = true;
 							context.core.image.imageCurrentSrc = ref.currentSrc;
@@ -282,14 +279,7 @@ store(
 									focusableElements.length - 1
 								];
 
-							// We want to avoid drawing unnecessary attention to the close
-							// button for mouse and touch users. Note that even if opening
-							// the lightbox via keyboard, the event fired is of type
-							// `pointerEvent`, so we need to rely on the `event.pointerType`
-							// property, which returns an empty string for keyboard events.
-							if ( context.core.image.pointerType === '' ) {
-								ref.querySelector( '.close-button' ).focus();
-							}
+							ref.focus();
 						}
 					},
 					setButtonStyles: ( { context, ref } ) => {

From d0d49ca256ddf3bfddbb736d60228a140b943ea0 Mon Sep 17 00:00:00 2001
From: Ricardo Artemio Morales <ric.morales22@gmail.com>
Date: Tue, 17 Oct 2023 15:50:38 -0500
Subject: [PATCH 2/7] Add delay before focusing when closing lightbox

---
 packages/block-library/src/image/view.js | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/packages/block-library/src/image/view.js b/packages/block-library/src/image/view.js
index 44936bfb428542..435cc6d28f2eb8 100644
--- a/packages/block-library/src/image/view.js
+++ b/packages/block-library/src/image/view.js
@@ -149,12 +149,15 @@ store(
 									'scroll',
 									scrollCallback
 								);
+								// If we don't delay before changing the focus,
+								// the focus ring will appear on Firefox before
+								// the image has finished animating, which looks broken.
+								context.core.image.lightboxTriggerRef.focus( {
+									preventScroll: true,
+								} );
 							}, 450 );
 
 							context.core.image.lightboxEnabled = false;
-							context.core.image.lightboxTriggerRef.focus( {
-								preventScroll: true,
-							} );
 						}
 					},
 					handleKeydown: ( { context, actions, event } ) => {

From 40b7b5772a93190243961089113bcd83761f0ed3 Mon Sep 17 00:00:00 2001
From: Ricardo Artemio Morales <ric.morales22@gmail.com>
Date: Wed, 18 Oct 2023 12:09:24 -0500
Subject: [PATCH 3/7] Put focus back on close button when opening lightbox

It turns out that placing focus on the modal was causing
inconsistent behavior in Safari, so I've put the focus back
on the close button when the lightbox opens, which
performs predictably.

I've also added a tabindex to the image, which prevents
the focus ring from erroneously showing when opening the lightbox
with a mouse in Chrome and Firefox.
---
 packages/block-library/src/image/index.php | 2 +-
 packages/block-library/src/image/view.js   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/packages/block-library/src/image/index.php b/packages/block-library/src/image/index.php
index 261e07c1d1f2cf..7c46b8077c7491 100644
--- a/packages/block-library/src/image/index.php
+++ b/packages/block-library/src/image/index.php
@@ -218,6 +218,7 @@ function block_core_image_render_lightbox( $block_content, $block ) {
 		)
 	);
 	$w->next_tag( 'img' );
+	$w->set_attribute( 'tabindex', '-1' );
 	$w->set_attribute( 'data-wp-init', 'effects.core.image.initOriginImage' );
 	$w->set_attribute( 'data-wp-on--load', 'actions.core.image.handleLoad' );
 	$w->set_attribute( 'data-wp-effect', 'effects.core.image.setButtonStyles' );
@@ -323,7 +324,6 @@ class="lightbox-trigger"
             data-wp-on--touchmove="actions.core.image.handleTouchMove"
             data-wp-on--touchend="actions.core.image.handleTouchEnd"
             data-wp-on--click="actions.core.image.hideLightbox"
-			tabindex="0"
             >
                 <button type="button" aria-label="$close_button_label" style="fill: $close_button_color" class="close-button" data-wp-on--click="actions.core.image.hideLightbox">
                     $close_button_icon
diff --git a/packages/block-library/src/image/view.js b/packages/block-library/src/image/view.js
index 435cc6d28f2eb8..ad79e04ff68c1e 100644
--- a/packages/block-library/src/image/view.js
+++ b/packages/block-library/src/image/view.js
@@ -282,7 +282,7 @@ store(
 									focusableElements.length - 1
 								];
 
-							ref.focus();
+							ref.querySelector( '.close-button' ).focus();
 						}
 					},
 					setButtonStyles: ( { context, ref } ) => {

From ab653a4acea9e1b6b8a20d0fe303f1f2ecc22d1e Mon Sep 17 00:00:00 2001
From: Andrea Fercia <a.fercia@gmail.com>
Date: Thu, 19 Oct 2023 12:37:25 +0200
Subject: [PATCH 4/7] Move focus to the dialog when opening the lightbox.

---
 packages/block-library/src/image/index.php | 1 +
 packages/block-library/src/image/view.js   | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/packages/block-library/src/image/index.php b/packages/block-library/src/image/index.php
index 7c46b8077c7491..2abef6f3b51d22 100644
--- a/packages/block-library/src/image/index.php
+++ b/packages/block-library/src/image/index.php
@@ -324,6 +324,7 @@ class="lightbox-trigger"
             data-wp-on--touchmove="actions.core.image.handleTouchMove"
             data-wp-on--touchend="actions.core.image.handleTouchEnd"
             data-wp-on--click="actions.core.image.hideLightbox"
+            tabindex="-1"
             >
                 <button type="button" aria-label="$close_button_label" style="fill: $close_button_color" class="close-button" data-wp-on--click="actions.core.image.hideLightbox">
                     $close_button_icon
diff --git a/packages/block-library/src/image/view.js b/packages/block-library/src/image/view.js
index ad79e04ff68c1e..331c0e79c731fc 100644
--- a/packages/block-library/src/image/view.js
+++ b/packages/block-library/src/image/view.js
@@ -282,7 +282,8 @@ store(
 									focusableElements.length - 1
 								];
 
-							ref.querySelector( '.close-button' ).focus();
+							// Move focus to the dialog when opening it.
+							ref.focus();
 						}
 					},
 					setButtonStyles: ( { context, ref } ) => {

From a438bd30a8ee20ff791361fb91fc9ccff940f958 Mon Sep 17 00:00:00 2001
From: Andrea Fercia <a.fercia@gmail.com>
Date: Thu, 19 Oct 2023 12:38:54 +0200
Subject: [PATCH 5/7] Fix SVG markup.

---
 packages/block-library/src/image/index.php | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/block-library/src/image/index.php b/packages/block-library/src/image/index.php
index 2abef6f3b51d22..ba167e0e97e467 100644
--- a/packages/block-library/src/image/index.php
+++ b/packages/block-library/src/image/index.php
@@ -245,7 +245,7 @@ class="lightbox-trigger"
 			data-wp-style--top="context.core.image.imageButtonTop"
 			style="background: #000"
 		>
-			<svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24" fill="none">
+			<svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24" fill="none" aria-hidden="true" focusable="false">
 				<path d="M9 5H5V9" stroke="#FFFFFF" stroke-width="1.5"/>
 				<path d="M15 19L19 19L19 15" stroke="#FFFFFF" stroke-width="1.5"/>
 				<path d="M15 5H19V9" stroke="#FFFFFF" stroke-width="1.5"/>

From af376c1d1c47b8c3af352210604d4329b0407c42 Mon Sep 17 00:00:00 2001
From: Andrea Fercia <a.fercia@gmail.com>
Date: Thu, 19 Oct 2023 14:22:57 +0200
Subject: [PATCH 6/7] Consistent indentation with spaces.

---
 packages/block-library/src/image/index.php | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/block-library/src/image/index.php b/packages/block-library/src/image/index.php
index ba167e0e97e467..821de4fb5ae248 100644
--- a/packages/block-library/src/image/index.php
+++ b/packages/block-library/src/image/index.php
@@ -330,7 +330,7 @@ class="lightbox-trigger"
                     $close_button_icon
                 </button>
                 <div class="lightbox-image-container">$initial_image_content</div>
-				<div class="lightbox-image-container">$enlarged_image_content</div>
+                <div class="lightbox-image-container">$enlarged_image_content</div>
                 <div class="scrim" style="background-color: $background_color" aria-hidden="true"></div>
         </div>
 HTML;

From 6c1090ad592637367f390cfc2dce3b56aa66776c Mon Sep 17 00:00:00 2001
From: Ricardo Artemio Morales <ric.morales22@gmail.com>
Date: Thu, 19 Oct 2023 12:40:40 -0500
Subject: [PATCH 7/7] Remove unnecessary tabindex

---
 packages/block-library/src/image/index.php | 1 -
 1 file changed, 1 deletion(-)

diff --git a/packages/block-library/src/image/index.php b/packages/block-library/src/image/index.php
index 821de4fb5ae248..69e6941b1403df 100644
--- a/packages/block-library/src/image/index.php
+++ b/packages/block-library/src/image/index.php
@@ -218,7 +218,6 @@ function block_core_image_render_lightbox( $block_content, $block ) {
 		)
 	);
 	$w->next_tag( 'img' );
-	$w->set_attribute( 'tabindex', '-1' );
 	$w->set_attribute( 'data-wp-init', 'effects.core.image.initOriginImage' );
 	$w->set_attribute( 'data-wp-on--load', 'actions.core.image.handleLoad' );
 	$w->set_attribute( 'data-wp-effect', 'effects.core.image.setButtonStyles' );