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.
WordPress · Sep 27, 2023 · 2c94042 · 2c94042
1 parent 2f7af76
commit 2c94042
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 50 deletions.
diff --git 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
@@ -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,15 +289,15 @@
 						}
 					}
 					.scrim {
-						animation: turn-off-visibility-transparent 0.4s forwards;
+						animation: turn-off-visibility 0.4s forwards;
 					}
 				}
 			}
 		}
 	}
 }
 
-@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
@@ -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;
+					},
 				},
 			},
 		},