From 3a43283e9247f27996f6dc94efeb82c29bc37978 Mon Sep 17 00:00:00 2001
From: Ricardo Artemio Morales <ric.morales22@gmail.com>
Date: Tue, 15 Aug 2023 15:21:52 -0400
Subject: [PATCH 01/13] Replace overflow: hidden with JavaScript callback to
 prevent scrolling

---
 lib/block-supports/behaviors.php            |  1 -
 packages/block-library/src/image/style.scss |  4 --
 packages/block-library/src/image/view.js    | 70 ++++++++++-----------
 3 files changed, 34 insertions(+), 41 deletions(-)

diff --git a/lib/block-supports/behaviors.php b/lib/block-supports/behaviors.php
index cf668ed22d8875..1b73b5d37b0b59 100644
--- a/lib/block-supports/behaviors.php
+++ b/lib/block-supports/behaviors.php
@@ -233,7 +233,6 @@ function gutenberg_render_behaviors_support_lightbox( $block_content, $block ) {
             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" style="fill: $close_button_color" class="close-button" data-wp-on--click="actions.core.image.hideLightbox">
diff --git a/packages/block-library/src/image/style.scss b/packages/block-library/src/image/style.scss
index c7eec224f65878..60f1126286dd39 100644
--- a/packages/block-library/src/image/style.scss
+++ b/packages/block-library/src/image/style.scss
@@ -297,10 +297,6 @@
 	}
 }
 
-html.wp-has-lightbox-open {
-	overflow: hidden;
-}
-
 @keyframes turn-on-visibility {
 	0% {
 		opacity: 0;
diff --git a/packages/block-library/src/image/view.js b/packages/block-library/src/image/view.js
index 53a2d1f7d567fc..693fedf97eca55 100644
--- a/packages/block-library/src/image/view.js
+++ b/packages/block-library/src/image/view.js
@@ -17,6 +17,14 @@ const focusableSelectors = [
 	'[tabindex]:not([tabindex^="-"])',
 ];
 
+let scrollCallback;
+
+function handleScroll( event, context, actions ) {
+	event.preventDefault();
+	window.scrollTo( 0, context.core.image.lastScrollTop );
+	actions.core.image.hideLightbox( { context, event } );
+}
+
 store(
 	{
 		state: {
@@ -30,7 +38,7 @@ store(
 		actions: {
 			core: {
 				image: {
-					showLightbox: ( { context, event } ) => {
+					showLightbox: ( { context, event, actions } ) => {
 						// We can't initialize the lightbox until the reference
 						// image is loaded, otherwise the UX is broken.
 						if ( ! context.core.image.imageLoaded ) {
@@ -43,55 +51,45 @@ store(
 
 						context.core.image.lightboxEnabled = true;
 						setStyles( context, event );
-						// Hide overflow only when the animation is in progress,
-						// otherwise the removal of the scrollbars will draw attention
-						// to itself and look like an error
-						document.documentElement.classList.add(
-							'wp-has-lightbox-open'
+
+						context.core.image.lastScrollTop =
+							window.pageYOffset ||
+							document.documentElement.scrollTop;
+
+						scrollCallback = handleScroll.bind(
+							null,
+							event,
+							context,
+							actions
+						);
+						window.addEventListener(
+							'scroll',
+							scrollCallback,
+							false
 						);
 					},
-					hideLightbox: async ( { context, event } ) => {
+					hideLightbox: async ( { context } ) => {
 						context.core.image.hideAnimationEnabled = true;
 						if ( context.core.image.lightboxEnabled ) {
 							// If scrolling, wait a moment before closing the lightbox.
 							if (
 								context.core.image.lightboxAnimation === 'fade'
 							) {
-								context.core.image.scrollDelta += event.deltaY;
-								if (
-									event.type === 'mousewheel' &&
-									Math.abs(
-										window.scrollY -
-											context.core.image.scrollDelta
-									) < 10
-								) {
-									return;
-								}
+								window.removeEventListener(
+									'scroll',
+									scrollCallback
+								);
 							} else if (
 								context.core.image.lightboxAnimation === 'zoom'
 							) {
-								// Disable scroll until the zoom animation ends.
-								// Get the current page scroll position
-								const scrollTop =
-									window.pageYOffset ||
-									document.documentElement.scrollTop;
-								const scrollLeft =
-									window.pageXOffset ||
-									document.documentElement.scrollLeft;
-								// if any scroll is attempted, set this to the previous value.
-								window.onscroll = function () {
-									window.scrollTo( scrollLeft, scrollTop );
-								};
 								// Enable scrolling after the animation finishes
 								setTimeout( function () {
-									window.onscroll = function () {};
-								}, 400 );
+									window.removeEventListener(
+										'scroll',
+										scrollCallback
+									);
+								}, 450 );
 							}
-
-							document.documentElement.classList.remove(
-								'wp-has-lightbox-open'
-							);
-
 							context.core.image.lightboxEnabled = false;
 							context.core.image.lastFocusedElement.focus( {
 								preventScroll: true,

From c438e155664dac18e00f665e3f72e26c4388992b Mon Sep 17 00:00:00 2001
From: Ricardo Artemio Morales <ric.morales22@gmail.com>
Date: Wed, 16 Aug 2023 10:36:34 -0400
Subject: [PATCH 02/13] Disable scroll callback on mobile; add comments; fix
 scrim styles

The page jumps around when trying to override the scroll behavior
on mobile, so I disabled it altogether. I've also added comments
to clarify this and other decisions made around the scroll handling.

While testing, I realized that the scrim was completely opaque during
the zoom animation, which does not match the design, so I added a new
animation specifically for the scrim to fix that.
---
 lib/block-supports/behaviors.php            |  1 +
 packages/block-library/src/image/style.scss | 40 ++++++++++---
 packages/block-library/src/image/view.js    | 64 +++++++++++++++------
 3 files changed, 81 insertions(+), 24 deletions(-)

diff --git a/lib/block-supports/behaviors.php b/lib/block-supports/behaviors.php
index 1b73b5d37b0b59..eb3f3dd2148383 100644
--- a/lib/block-supports/behaviors.php
+++ b/lib/block-supports/behaviors.php
@@ -233,6 +233,7 @@ function gutenberg_render_behaviors_support_lightbox( $block_content, $block ) {
             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--touchmove="actions.core.image.handleTouchMove"
             data-wp-on--click="actions.core.image.hideLightbox"
             >
                 <button type="button" aria-label="$close_button_label" style="fill: $close_button_color" class="close-button" data-wp-on--click="actions.core.image.hideLightbox">
diff --git a/packages/block-library/src/image/style.scss b/packages/block-library/src/image/style.scss
index 60f1126286dd39..44a0b00669b6c0 100644
--- a/packages/block-library/src/image/style.scss
+++ b/packages/block-library/src/image/style.scss
@@ -247,16 +247,16 @@
 	// or faster than the scrim to give a sense of depth.
 	&.active {
 		visibility: visible;
-		animation: both turn-on-visibility 0.25s;
+		animation: both turn-on-visibility-opaque 0.25s;
 		img {
-			animation: both turn-on-visibility 0.35s;
+			animation: both turn-on-visibility-opaque 0.35s;
 		}
 	}
 	&.hideanimationenabled {
 		&:not(.active) {
-			animation: both turn-off-visibility 0.35s;
+			animation: both turn-off-visibility-opaque 0.35s;
 			img {
-				animation: both turn-off-visibility 0.25s;
+				animation: both turn-off-visibility-opaque 0.25s;
 			}
 		}
 	}
@@ -275,7 +275,7 @@
 					}
 				}
 				.scrim {
-					animation: turn-on-visibility 0.4s forwards;
+					animation: turn-on-visibility-transparent 0.4s forwards;
 				}
 			}
 			&.hideanimationenabled {
@@ -289,7 +289,7 @@
 						}
 					}
 					.scrim {
-						animation: turn-off-visibility 0.4s forwards;
+						animation: turn-off-visibility-transparent 0.4s forwards;
 					}
 				}
 			}
@@ -297,7 +297,7 @@
 	}
 }
 
-@keyframes turn-on-visibility {
+@keyframes turn-on-visibility-opaque {
 	0% {
 		opacity: 0;
 	}
@@ -306,7 +306,7 @@
 	}
 }
 
-@keyframes turn-off-visibility {
+@keyframes turn-off-visibility-opaque {
 	0% {
 		opacity: 1;
 		visibility: visible;
@@ -321,6 +321,30 @@
 	}
 }
 
+@keyframes turn-on-visibility-transparent {
+	0% {
+		opacity: 0;
+	}
+	100% {
+		opacity: 0.9;
+	}
+}
+
+@keyframes turn-off-visibility-transparent {
+	0% {
+		opacity: 0.9;
+		visibility: visible;
+	}
+	99% {
+		opacity: 0;
+		visibility: visible;
+	}
+	100% {
+		opacity: 0;
+		visibility: hidden;
+	}
+}
+
 @keyframes lightbox-zoom-in {
 	0% {
 		transform: translate(calc(-50vw + var(--wp--lightbox-initial-left-position)), calc(-50vh + var(--wp--lightbox-initial-top-position))) scale(var(--wp--lightbox-scale));
diff --git a/packages/block-library/src/image/view.js b/packages/block-library/src/image/view.js
index 693fedf97eca55..e2e9d7c3fffced 100644
--- a/packages/block-library/src/image/view.js
+++ b/packages/block-library/src/image/view.js
@@ -17,8 +17,24 @@ const focusableSelectors = [
 	'[tabindex]:not([tabindex^="-"])',
 ];
 
+// I've defined the scroll handling outside of the store
+// to prevent confusion as to how this logic is called.
+// This logic is not referenced directly by the directives
+// in the markup because the scroll event we need to listen
+// to is triggered on the window; so we define it outside of
+// the store to signal that the behavior here is different.
+// If we find a compelling reason to move it to the store,
+// feel free to do so.
 let scrollCallback;
 
+// Using overflow: hidden to prevent scrolling while the lightbox
+// is open causes the content to shift and prevents the zoom
+// animation from working in some cases because we're unable to
+// account for the layout shift when doing the animation calculations.
+// Instead, here we use JavaScript to prevent and reset the scrolling
+// behavior. In the future, we may be able to use CSS or overflow: hidden
+// instead to not rely on JavaScript, but this seems to be the best approach
+// for now that provides the best visual experience.
 function handleScroll( event, context, actions ) {
 	event.preventDefault();
 	window.scrollTo( 0, context.core.image.lastScrollTop );
@@ -56,12 +72,21 @@ store(
 							window.pageYOffset ||
 							document.documentElement.scrollTop;
 
+						// We define and bind the scroll callback here so that
+						// we can pass the context and actions as arguments.
+						// We may be able to change this in the future if we
+						// define the scroll callback in the store instead, but
+						// this approach seems to tbe clearest for now.
 						scrollCallback = handleScroll.bind(
 							null,
-							event,
 							context,
 							actions
 						);
+
+						// We need to add a scroll event listener to the window
+						// here because we are unable to otherwise access it via
+						// the Interactivity API directives. If we add a native way
+						// to access the window, we can remove this.
 						window.addEventListener(
 							'scroll',
 							scrollCallback,
@@ -71,25 +96,20 @@ store(
 					hideLightbox: async ( { context } ) => {
 						context.core.image.hideAnimationEnabled = true;
 						if ( context.core.image.lightboxEnabled ) {
-							// If scrolling, wait a moment before closing the lightbox.
-							if (
-								context.core.image.lightboxAnimation === 'fade'
-							) {
+							// The lightbox will close once it detects a scroll event,
+							// but we want to wait until the close animation is completed
+							// before allowing a user to scroll again. The duration of this
+							// animation is defined in the styles.scss and depends on if the
+							// animation is 'zoom' or 'fade', but in any case we should wait
+							// a few milliseconds longer than the duration, otherwise a user
+							// may scroll too soon and cause the animation to look sloppy.
+							setTimeout( function () {
 								window.removeEventListener(
 									'scroll',
 									scrollCallback
 								);
-							} else if (
-								context.core.image.lightboxAnimation === 'zoom'
-							) {
-								// Enable scrolling after the animation finishes
-								setTimeout( function () {
-									window.removeEventListener(
-										'scroll',
-										scrollCallback
-									);
-								}, 450 );
-							}
+							}, 450 );
+
 							context.core.image.lightboxEnabled = false;
 							context.core.image.lastFocusedElement.focus( {
 								preventScroll: true,
@@ -137,6 +157,18 @@ store(
 							ref,
 						} );
 					},
+					handleTouchMove: ( { context, event } ) => {
+						// On mobile devices, we want to prevent triggering the
+						// scroll event because otherwise the page jumps around as
+						// we reset the scroll position. This also means that the
+						// lightbox will not close on touch move, and will require
+						// that a user perform a simple tap to close instead. This
+						// may be changed in the future if we find a better alternative
+						// to override or reset the scroll position during swipe actions.
+						if ( context.core.image.lightboxEnabled ) {
+							event.preventDefault();
+						}
+					},
 				},
 			},
 		},

From 416803677008590eff672f743bd168c1b8152d0e Mon Sep 17 00:00:00 2001
From: Ricardo Artemio Morales <ric.morales22@gmail.com>
Date: Wed, 16 Aug 2023 10:48:15 -0400
Subject: [PATCH 03/13] Add handling for horizontally oriented pages

---
 packages/block-library/src/image/view.js | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/packages/block-library/src/image/view.js b/packages/block-library/src/image/view.js
index e2e9d7c3fffced..e855aefd1502be 100644
--- a/packages/block-library/src/image/view.js
+++ b/packages/block-library/src/image/view.js
@@ -37,7 +37,10 @@ let scrollCallback;
 // for now that provides the best visual experience.
 function handleScroll( event, context, actions ) {
 	event.preventDefault();
-	window.scrollTo( 0, context.core.image.lastScrollTop );
+	window.scrollTo(
+		context.core.image.lastScrollLeft,
+		context.core.image.lastScrollTop
+	);
 	actions.core.image.hideLightbox( { context, event } );
 }
 
@@ -72,6 +75,12 @@ store(
 							window.pageYOffset ||
 							document.documentElement.scrollTop;
 
+						// In most cases, this value will be 0, but this is included
+						// in case a user has created a page with horizontal scrolling.
+						context.core.image.lastScrollLeft =
+							window.pageXOffset ||
+							document.documentElement.scrollLeft;
+
 						// We define and bind the scroll callback here so that
 						// we can pass the context and actions as arguments.
 						// We may be able to change this in the future if we

From 564db4ca212741b9a3f3947a1cee60c6a8b1498c Mon Sep 17 00:00:00 2001
From: Ricardo Artemio Morales <ric.morales22@gmail.com>
Date: Wed, 16 Aug 2023 15:14:12 -0400
Subject: [PATCH 04/13] Move close button so that it's further from scrollbar
 on desktop

---
 packages/block-library/src/image/style.scss | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/packages/block-library/src/image/style.scss b/packages/block-library/src/image/style.scss
index 44a0b00669b6c0..2552a94a198f27 100644
--- a/packages/block-library/src/image/style.scss
+++ b/packages/block-library/src/image/style.scss
@@ -186,8 +186,8 @@
 
 	.close-button {
 		position: absolute;
-		top: calc(env(safe-area-inset-top) + 12.5px);
-		right: calc(env(safe-area-inset-right) + 12.5px);
+		top: calc(env(safe-area-inset-top) + 20px);
+		right: calc(env(safe-area-inset-right) + 20px);
 		padding: 0;
 		cursor: pointer;
 		z-index: 5000000;

From 2f7af76feeace90bbd1cbfc6de350124f6c2b42f Mon Sep 17 00:00:00 2001
From: Ricardo Artemio Morales <ric.morales22@gmail.com>
Date: Wed, 20 Sep 2023 16:54:14 -0400
Subject: [PATCH 05/13] Fix call to handleScroll() and add touch callback to
 new render method

---
 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 996dbe6abf60df..739f2e77f032cb 100644
--- a/packages/block-library/src/image/index.php
+++ b/packages/block-library/src/image/index.php
@@ -272,7 +272,7 @@ function block_core_image_render_lightbox( $block_content, $block ) {
             aria-modal="false"
             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--touchmove="actions.core.image.handleTouchMove"
             data-wp-on--click="actions.core.image.hideLightbox"
             >
                 <button type="button" aria-label="$close_button_label" style="fill: $close_button_color" class="close-button" data-wp-on--click="actions.core.image.hideLightbox">
diff --git a/packages/block-library/src/image/view.js b/packages/block-library/src/image/view.js
index e855aefd1502be..cb09ff8562f0e4 100644
--- a/packages/block-library/src/image/view.js
+++ b/packages/block-library/src/image/view.js
@@ -35,7 +35,7 @@ let scrollCallback;
 // behavior. In the future, we may be able to use CSS or overflow: hidden
 // instead to not rely on JavaScript, but this seems to be the best approach
 // for now that provides the best visual experience.
-function handleScroll( event, context, actions ) {
+function handleScroll( context, actions, event ) {
 	event.preventDefault();
 	window.scrollTo(
 		context.core.image.lastScrollLeft,

From 2c94042d8de7cb51422d589cdacfa7c912d3e7ca Mon Sep 17 00:00:00 2001
From: Ricardo Artemio Morales <ric.morales22@gmail.com>
Date: Tue, 26 Sep 2023 20:57:25 -0500
Subject: [PATCH 06/13] Improve lightbox experience on mobile

To ensure pinch to zoom works as expected when the lightbox
is open on mobile, I added logic to disable the scroll override
when touch is detected. Without this, the scroll override kicks
in whenever one tries to pinch to zoom, and it breaks the experience.

I also revised the styles for the scrim to make it opaque, as having
content visible outside of the lightbox is distracting when
pinching to zoom on a mobile device, and I think having a consistent
lightbox across devices will make for the best user experience.
---
 packages/block-library/src/image/index.php  |  2 +
 packages/block-library/src/image/style.scss | 40 ++++------------
 packages/block-library/src/image/view.js    | 51 +++++++++++++--------
 3 files changed, 43 insertions(+), 50 deletions(-)

diff --git a/packages/block-library/src/image/index.php b/packages/block-library/src/image/index.php
index 739f2e77f032cb..d6cb0688144964 100644
--- a/packages/block-library/src/image/index.php
+++ b/packages/block-library/src/image/index.php
@@ -272,7 +272,9 @@ function block_core_image_render_lightbox( $block_content, $block ) {
             aria-modal="false"
             data-wp-effect="effects.core.image.initLightbox"
             data-wp-on--keydown="actions.core.image.handleKeydown"
+			data-wp-on--touchstart="actions.core.image.handleTouchStart"
             data-wp-on--touchmove="actions.core.image.handleTouchMove"
+			data-wp-on--touchend="actions.core.image.handleTouchEnd"
             data-wp-on--click="actions.core.image.hideLightbox"
             >
                 <button type="button" aria-label="$close_button_label" style="fill: $close_button_color" class="close-button" data-wp-on--click="actions.core.image.hideLightbox">
diff --git a/packages/block-library/src/image/style.scss b/packages/block-library/src/image/style.scss
index 2552a94a198f27..5c3552fd80c2ee 100644
--- a/packages/block-library/src/image/style.scss
+++ b/packages/block-library/src/image/style.scss
@@ -247,16 +247,16 @@
 	// or faster than the scrim to give a sense of depth.
 	&.active {
 		visibility: visible;
-		animation: both turn-on-visibility-opaque 0.25s;
+		animation: both turn-on-visibility 0.25s;
 		img {
-			animation: both turn-on-visibility-opaque 0.35s;
+			animation: both turn-on-visibility 0.35s;
 		}
 	}
 	&.hideanimationenabled {
 		&:not(.active) {
-			animation: both turn-off-visibility-opaque 0.35s;
+			animation: both turn-off-visibility 0.35s;
 			img {
-				animation: both turn-off-visibility-opaque 0.25s;
+				animation: both turn-off-visibility 0.25s;
 			}
 		}
 	}
@@ -275,7 +275,7 @@
 					}
 				}
 				.scrim {
-					animation: turn-on-visibility-transparent 0.4s forwards;
+					animation: turn-on-visibility 0.4s forwards;
 				}
 			}
 			&.hideanimationenabled {
@@ -289,7 +289,7 @@
 						}
 					}
 					.scrim {
-						animation: turn-off-visibility-transparent 0.4s forwards;
+						animation: turn-off-visibility 0.4s forwards;
 					}
 				}
 			}
@@ -297,7 +297,7 @@
 	}
 }
 
-@keyframes turn-on-visibility-opaque {
+@keyframes turn-on-visibility {
 	0% {
 		opacity: 0;
 	}
@@ -306,7 +306,7 @@
 	}
 }
 
-@keyframes turn-off-visibility-opaque {
+@keyframes turn-off-visibility {
 	0% {
 		opacity: 1;
 		visibility: visible;
@@ -321,30 +321,6 @@
 	}
 }
 
-@keyframes turn-on-visibility-transparent {
-	0% {
-		opacity: 0;
-	}
-	100% {
-		opacity: 0.9;
-	}
-}
-
-@keyframes turn-off-visibility-transparent {
-	0% {
-		opacity: 0.9;
-		visibility: visible;
-	}
-	99% {
-		opacity: 0;
-		visibility: visible;
-	}
-	100% {
-		opacity: 0;
-		visibility: hidden;
-	}
-}
-
 @keyframes lightbox-zoom-in {
 	0% {
 		transform: translate(calc(-50vw + var(--wp--lightbox-initial-left-position)), calc(-50vh + var(--wp--lightbox-initial-top-position))) scale(var(--wp--lightbox-scale));
diff --git a/packages/block-library/src/image/view.js b/packages/block-library/src/image/view.js
index cb09ff8562f0e4..473080ec99497d 100644
--- a/packages/block-library/src/image/view.js
+++ b/packages/block-library/src/image/view.js
@@ -27,6 +27,11 @@ const focusableSelectors = [
 // feel free to do so.
 let scrollCallback;
 
+// We need to track whether a user is touching the screen so we
+// can handle the scroll override differently on mobile devices.
+let isTouching = false;
+let lastTouchTime = 0;
+
 // Using overflow: hidden to prevent scrolling while the lightbox
 // is open causes the content to shift and prevents the zoom
 // animation from working in some cases because we're unable to
@@ -35,13 +40,18 @@ let scrollCallback;
 // behavior. In the future, we may be able to use CSS or overflow: hidden
 // instead to not rely on JavaScript, but this seems to be the best approach
 // for now that provides the best visual experience.
-function handleScroll( context, actions, event ) {
-	event.preventDefault();
-	window.scrollTo(
-		context.core.image.lastScrollLeft,
-		context.core.image.lastScrollTop
-	);
-	actions.core.image.hideLightbox( { context, event } );
+function handleScroll( context ) {
+	// We can't override the scroll behavior on mobile devices
+	// because doing so breaks the pinch to zoom functionality, and we
+	// want to allow users to zoom in further on the high-res image.
+	if ( ! isTouching && Date.now() - lastTouchTime > 450 ) {
+		// We are unable to use event.preventDefault() to prevent scrolling
+		// because the scroll event can't be canceled, so we reset the position instead.
+		window.scrollTo(
+			context.core.image.lastScrollLeft,
+			context.core.image.lastScrollTop
+		);
+	}
 }
 
 store(
@@ -57,7 +67,7 @@ store(
 		actions: {
 			core: {
 				image: {
-					showLightbox: ( { context, event, actions } ) => {
+					showLightbox: ( { context, event } ) => {
 						// We can't initialize the lightbox until the reference
 						// image is loaded, otherwise the UX is broken.
 						if ( ! context.core.image.imageLoaded ) {
@@ -81,16 +91,12 @@ store(
 							window.pageXOffset ||
 							document.documentElement.scrollLeft;
 
-						// We define and bind the scroll callback here so that
-						// we can pass the context and actions as arguments.
+						// We define and bind the scroll callback here so
+						// that we can pass the context and as an argument.
 						// We may be able to change this in the future if we
 						// define the scroll callback in the store instead, but
 						// this approach seems to tbe clearest for now.
-						scrollCallback = handleScroll.bind(
-							null,
-							context,
-							actions
-						);
+						scrollCallback = handleScroll.bind( null, context );
 
 						// We need to add a scroll event listener to the window
 						// here because we are unable to otherwise access it via
@@ -166,18 +172,27 @@ store(
 							ref,
 						} );
 					},
+					handleTouchStart: () => {
+						isTouching = true;
+					},
 					handleTouchMove: ( { context, event } ) => {
 						// On mobile devices, we want to prevent triggering the
 						// scroll event because otherwise the page jumps around as
-						// we reset the scroll position. This also means that the
-						// lightbox will not close on touch move, and will require
-						// that a user perform a simple tap to close instead. This
+						// we reset the scroll position. This also means that closing
+						// the lightbox requires that a user perform a simple tap. This
 						// may be changed in the future if we find a better alternative
 						// to override or reset the scroll position during swipe actions.
 						if ( context.core.image.lightboxEnabled ) {
 							event.preventDefault();
 						}
 					},
+					handleTouchEnd: () => {
+						// We need to wait a few milliseconds before resetting
+						// to ensure that pinch to zoom works consistently
+						// on mobile devices when the lightbox is open.
+						lastTouchTime = Date.now();
+						isTouching = false;
+					},
 				},
 			},
 		},

From aa2b49528aeead04334fdb5a0a7a7a1b8356a34f Mon Sep 17 00:00:00 2001
From: Ricardo Artemio Morales <ric.morales22@gmail.com>
Date: Tue, 26 Sep 2023 21:10:11 -0500
Subject: [PATCH 07/13] Fix spacing

---
 packages/block-library/src/image/index.php | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/packages/block-library/src/image/index.php b/packages/block-library/src/image/index.php
index d6cb0688144964..54aa8fca6d5f42 100644
--- a/packages/block-library/src/image/index.php
+++ b/packages/block-library/src/image/index.php
@@ -272,9 +272,9 @@ function block_core_image_render_lightbox( $block_content, $block ) {
             aria-modal="false"
             data-wp-effect="effects.core.image.initLightbox"
             data-wp-on--keydown="actions.core.image.handleKeydown"
-			data-wp-on--touchstart="actions.core.image.handleTouchStart"
+            data-wp-on--touchstart="actions.core.image.handleTouchStart"
             data-wp-on--touchmove="actions.core.image.handleTouchMove"
-			data-wp-on--touchend="actions.core.image.handleTouchEnd"
+            data-wp-on--touchend="actions.core.image.handleTouchEnd"
             data-wp-on--click="actions.core.image.hideLightbox"
             >
                 <button type="button" aria-label="$close_button_label" style="fill: $close_button_color" class="close-button" data-wp-on--click="actions.core.image.hideLightbox">

From 2e38ab051201878cc3cd3c7d3438a19e910e9caf Mon Sep 17 00:00:00 2001
From: Ricardo Artemio Morales <ric.morales22@gmail.com>
Date: Tue, 26 Sep 2023 21:10:50 -0500
Subject: [PATCH 08/13] Add touch directives to block supports

---
 lib/block-supports/behaviors.php | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/block-supports/behaviors.php b/lib/block-supports/behaviors.php
index eb3f3dd2148383..e5800d2f7f25f0 100644
--- a/lib/block-supports/behaviors.php
+++ b/lib/block-supports/behaviors.php
@@ -233,7 +233,9 @@ function gutenberg_render_behaviors_support_lightbox( $block_content, $block ) {
             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--touchmove="actions.core.image.handleTouchMove"
+			data-wp-on--touchstart="actions.core.image.handleTouchStart"
+            data-wp-on--touchmove="actions.core.image.handleTouchMove"
+            data-wp-on--touchend="actions.core.image.handleTouchEnd"
             data-wp-on--click="actions.core.image.hideLightbox"
             >
                 <button type="button" aria-label="$close_button_label" style="fill: $close_button_color" class="close-button" data-wp-on--click="actions.core.image.hideLightbox">

From 59d32e84b95992522d22040e3801ebe5bfa62d02 Mon Sep 17 00:00:00 2001
From: Ricardo Artemio Morales <ric.morales22@gmail.com>
Date: Tue, 26 Sep 2023 21:12:20 -0500
Subject: [PATCH 09/13] Fix spacing in block supports

---
 lib/block-supports/behaviors.php | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/block-supports/behaviors.php b/lib/block-supports/behaviors.php
index e5800d2f7f25f0..6f442d7b0d2d7c 100644
--- a/lib/block-supports/behaviors.php
+++ b/lib/block-supports/behaviors.php
@@ -233,7 +233,7 @@ function gutenberg_render_behaviors_support_lightbox( $block_content, $block ) {
             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--touchstart="actions.core.image.handleTouchStart"
+            data-wp-on--touchstart="actions.core.image.handleTouchStart"
             data-wp-on--touchmove="actions.core.image.handleTouchMove"
             data-wp-on--touchend="actions.core.image.handleTouchEnd"
             data-wp-on--click="actions.core.image.hideLightbox"

From 057a164b36ed3a6c86be95f3bc2f043bc3a18520 Mon Sep 17 00:00:00 2001
From: Ricardo Artemio Morales <ric.morales22@gmail.com>
Date: Thu, 28 Sep 2023 08:52:07 -0500
Subject: [PATCH 10/13] Rename attribute for clarity

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

diff --git a/packages/block-library/src/image/view.js b/packages/block-library/src/image/view.js
index 473080ec99497d..14680dc37111c1 100644
--- a/packages/block-library/src/image/view.js
+++ b/packages/block-library/src/image/view.js
@@ -48,8 +48,8 @@ function handleScroll( context ) {
 		// We are unable to use event.preventDefault() to prevent scrolling
 		// because the scroll event can't be canceled, so we reset the position instead.
 		window.scrollTo(
-			context.core.image.lastScrollLeft,
-			context.core.image.lastScrollTop
+			context.core.image.scrollLeftReset,
+			context.core.image.scrollTopReset
 		);
 	}
 }
@@ -81,13 +81,13 @@ store(
 						context.core.image.lightboxEnabled = true;
 						setStyles( context, event );
 
-						context.core.image.lastScrollTop =
+						context.core.image.scrollTopReset =
 							window.pageYOffset ||
 							document.documentElement.scrollTop;
 
 						// In most cases, this value will be 0, but this is included
 						// in case a user has created a page with horizontal scrolling.
-						context.core.image.lastScrollLeft =
+						context.core.image.scrollLeftReset =
 							window.pageXOffset ||
 							document.documentElement.scrollLeft;
 

From dc7a8b43e50ab20201bd170d369780003cce34b5 Mon Sep 17 00:00:00 2001
From: Ricardo Artemio Morales <ric.morales22@gmail.com>
Date: Thu, 28 Sep 2023 08:53:09 -0500
Subject: [PATCH 11/13] Update comment

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

diff --git a/packages/block-library/src/image/view.js b/packages/block-library/src/image/view.js
index 14680dc37111c1..edef9981b0a6c3 100644
--- a/packages/block-library/src/image/view.js
+++ b/packages/block-library/src/image/view.js
@@ -111,8 +111,7 @@ store(
 					hideLightbox: async ( { context } ) => {
 						context.core.image.hideAnimationEnabled = true;
 						if ( context.core.image.lightboxEnabled ) {
-							// The lightbox will close once it detects a scroll event,
-							// but we want to wait until the close animation is completed
+							// We want to wait until the close animation is completed
 							// before allowing a user to scroll again. The duration of this
 							// animation is defined in the styles.scss and depends on if the
 							// animation is 'zoom' or 'fade', but in any case we should wait

From 734096af52ae997416f18640a41c38ecdee8ff92 Mon Sep 17 00:00:00 2001
From: Ricardo Artemio Morales <ric.morales22@gmail.com>
Date: Thu, 28 Sep 2023 09:37:46 -0500
Subject: [PATCH 12/13] Update comments

---
 packages/block-library/src/image/view.js | 96 +++++++++++++++++-------
 1 file changed, 68 insertions(+), 28 deletions(-)

diff --git a/packages/block-library/src/image/view.js b/packages/block-library/src/image/view.js
index edef9981b0a6c3..f7ef1afa8c9386 100644
--- a/packages/block-library/src/image/view.js
+++ b/packages/block-library/src/image/view.js
@@ -17,29 +17,55 @@ const focusableSelectors = [
 	'[tabindex]:not([tabindex^="-"])',
 ];
 
-// I've defined the scroll handling outside of the store
-// to prevent confusion as to how this logic is called.
-// This logic is not referenced directly by the directives
-// in the markup because the scroll event we need to listen
-// to is triggered on the window; so we define it outside of
-// the store to signal that the behavior here is different.
-// If we find a compelling reason to move it to the store,
-// feel free to do so.
+/*
+ * Stores a context-bound scroll handler.
+ *
+ * This callback could be defined inline inside of the store
+ * object but it's created externally to avoid confusion about
+ * how its logic is called. This logic is not referenced directly
+ * by the directives in the markup because the scroll event we
+ * need to listen to is triggered on the window; so by defining it
+ * outside of the store, we signal that the behavior here is different.
+ * If we find a compelling reason to move it to the store, feel free.
+ *
+ * @type {Function}
+ */
 let scrollCallback;
 
-// We need to track whether a user is touching the screen so we
-// can handle the scroll override differently on mobile devices.
+/*
+ * Tracks whether user is touching screen; used to
+ * differentiate behavior for touch and mouse input.
+ *
+ * @type {boolean}
+ */
 let isTouching = false;
+
+/*
+ * Tracks the last time the screen was touched; used to
+ * differentiate behavior for touch and mouse input.
+ *
+ * @type {number}
+ */
 let lastTouchTime = 0;
 
-// Using overflow: hidden to prevent scrolling while the lightbox
-// is open causes the content to shift and prevents the zoom
-// animation from working in some cases because we're unable to
-// account for the layout shift when doing the animation calculations.
-// Instead, here we use JavaScript to prevent and reset the scrolling
-// behavior. In the future, we may be able to use CSS or overflow: hidden
-// instead to not rely on JavaScript, but this seems to be the best approach
-// for now that provides the best visual experience.
+/*
+ * Lightbox page-scroll handler: prevents scrolling.
+ *
+ * This handler is added to prevent scrolling behaviors that
+ * trigger content shift while the lightbox is open.
+ *
+ * It would be better to accomplish this through CSS alone, but
+ * using overflow: hidden is currently the only way to do so, and
+ * that causes the layout to shift and prevents the zoom animation
+ * from working in some cases because we're unable to account for
+ * the layout shift when doing the animation calculations. Instead,
+ * here we use JavaScript to prevent and reset the scrolling
+ * behavior. In the future, we may be able to use CSS or overflow: hidden
+ * instead to not rely on JavaScript, but this seems to be the best approach
+ * for now that provides the best visual experience.
+ *
+ * @param {Object} context Interactivity page context?
+ */
 function handleScroll( context ) {
 	// We can't override the scroll behavior on mobile devices
 	// because doing so breaks the pinch to zoom functionality, and we
@@ -319,6 +345,13 @@ store(
 	}
 );
 
+/*
+ * Computes styles for the lightbox and adds them to the document.
+ *
+ * @function
+ * @param {Object} context - An Interactivity API context
+ * @param {Object} event - A triggering event
+ */
 function setStyles( context, event ) {
 	// The reference img element lies adjacent
 	// to the event target button in the DOM.
@@ -475,18 +508,25 @@ function setStyles( context, event ) {
 	// transformation. In any case, adding 1 pixel to the container width and height solves
 	// the problem, though this can be removed if the issue is fixed in the future.
 	styleTag.innerHTML = `
-		:root {
-			--wp--lightbox-initial-top-position: ${ screenPosY }px;
-			--wp--lightbox-initial-left-position: ${ screenPosX }px;
-			--wp--lightbox-container-width: ${ containerWidth + 1 }px;
-			--wp--lightbox-container-height: ${ containerHeight + 1 }px;
-			--wp--lightbox-image-width: ${ lightboxImgWidth }px;
-			--wp--lightbox-image-height: ${ lightboxImgHeight }px;
-			--wp--lightbox-scale: ${ containerScale };
-		}
-	`;
+			:root {
+				--wp--lightbox-initial-top-position: ${ screenPosY }px;
+				--wp--lightbox-initial-left-position: ${ screenPosX }px;
+				--wp--lightbox-container-width: ${ containerWidth + 1 }px;
+				--wp--lightbox-container-height: ${ containerHeight + 1 }px;
+				--wp--lightbox-image-width: ${ lightboxImgWidth }px;
+				--wp--lightbox-image-height: ${ lightboxImgHeight }px;
+				--wp--lightbox-scale: ${ containerScale };
+			}
+		`;
 }
 
+/*
+ * Debounces a function call.
+ *
+ * @function
+ * @param {Function} func - A function to be called
+ * @param {number} wait - The time to wait before calling the function
+ */
 function debounce( func, wait = 50 ) {
 	let timeout;
 	return () => {

From 82eb2bab89f989f29e032f11fd14e118b775ef96 Mon Sep 17 00:00:00 2001
From: Ricardo Artemio Morales <ric.morales22@gmail.com>
Date: Thu, 28 Sep 2023 11:26:14 -0500
Subject: [PATCH 13/13] Fix spacing

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

diff --git a/packages/block-library/src/image/view.js b/packages/block-library/src/image/view.js
index f7ef1afa8c9386..13f20c9cd7cb68 100644
--- a/packages/block-library/src/image/view.js
+++ b/packages/block-library/src/image/view.js
@@ -508,16 +508,16 @@ function setStyles( context, event ) {
 	// transformation. In any case, adding 1 pixel to the container width and height solves
 	// the problem, though this can be removed if the issue is fixed in the future.
 	styleTag.innerHTML = `
-			:root {
-				--wp--lightbox-initial-top-position: ${ screenPosY }px;
-				--wp--lightbox-initial-left-position: ${ screenPosX }px;
-				--wp--lightbox-container-width: ${ containerWidth + 1 }px;
-				--wp--lightbox-container-height: ${ containerHeight + 1 }px;
-				--wp--lightbox-image-width: ${ lightboxImgWidth }px;
-				--wp--lightbox-image-height: ${ lightboxImgHeight }px;
-				--wp--lightbox-scale: ${ containerScale };
-			}
-		`;
+		:root {
+			--wp--lightbox-initial-top-position: ${ screenPosY }px;
+			--wp--lightbox-initial-left-position: ${ screenPosX }px;
+			--wp--lightbox-container-width: ${ containerWidth + 1 }px;
+			--wp--lightbox-container-height: ${ containerHeight + 1 }px;
+			--wp--lightbox-image-width: ${ lightboxImgWidth }px;
+			--wp--lightbox-image-height: ${ lightboxImgHeight }px;
+			--wp--lightbox-scale: ${ containerScale };
+		}
+	`;
 }
 
 /*