Image block: Fix responsive sizing in lightbox (#51823)

* Rewrite logic to calculate image dimensions based on aspect ratio

* Add padding for lightbox images using fade animation

* Use window inner dimensions to account for mobile address bar

On mobile, the address bar is sometimes visible in browsers,
which the CSS vh (viewport height) value does not account for.
This causes calculations based on vh to render incorrectly
if the address bar is ever visible - in this case, placing
the lightbox image off center.

To address this, I changed the lightbox calculations to be
based on window.innerHeight and window.innerWidth instead.

* Revise to deactivate responsive image on lightbox close

* Use built-in directive for mouseover event

* Add safe area inset to close button positioning

* Revert "Use built-in directive for mouseover event"

This reverts commit a4c9a22ccd17fc92f5dd99796b52605654ce82e9.

* Update tests

packages/block-library/src/image/style.scss

@@ -187,8 +187,8 @@
 
 	.close-button {
 		position: absolute;
-		top: 12.5px;
-		right: 12.5px;
+		top: calc(env(safe-area-inset-top) + 12.5px);
+		right: calc(env(safe-area-inset-right) + 12.5px);
 		padding: 0;
 		cursor: pointer;
 		z-index: 5000000;
@@ -231,6 +231,18 @@
 	}
 
 	&.fade {
+		.wp-block-image {
+			padding: 40px 0;
+
+			@media screen and (min-width: 480px) {
+				padding: 40px;
+			}
+
+			@media screen and (min-width: 1920px) {
+				padding: 40px 80px;
+			}
+		}
+
 		&.active {
 			visibility: visible;
 			animation: both turn-on-visibility 0.25s;

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

@@ -143,7 +143,8 @@ store( {
 					return context.core.image.lightboxEnabled ? 'dialog' : '';
 				},
 				responsiveImgSrc: ( { context } ) => {
-					return context.core.image.activateLargeImage
+					return context.core.image.activateLargeImage &&
+						context.core.image.hideAnimationEnabled
 						? ''
 						: context.core.image.imageCurrentSrc;
 				},
@@ -206,48 +207,48 @@ function setZoomStyles( imgDom, context, event ) {
 	let targetWidth = context.core.image.targetWidth;
 	let targetHeight = context.core.image.targetHeight;
 
-	const verticalPadding = 40;
+	// Since the lightbox image has `position:absolute`, it
+	// ignores its parent's padding, so we need to set padding here
+	// to calculate dimensions and positioning.
 
-	// As per the design, let's allow the image to stretch
-	// to the full width of its containing figure, but for the height,
-	// constrain it with a fixed padding
-	const containerWidth = context.core.image.figureRef.clientWidth;
+	// As per the design, let's constrain the height with fixed padding
+	const containerOuterHeight = window.innerHeight;
+	const verticalPadding = 40;
+	const containerInnerHeight = containerOuterHeight - verticalPadding * 2;
 
-	// The lightbox image has `positione:absolute` and
-	// ignores its parent's padding, so let's set the padding here,
-	// to be used when calculating the image width and positioning
+	// Let's set a variable horizontal padding based on the container width
+	const containerOuterWidth = window.innerWidth;
 	let horizontalPadding = 0;
-	if ( containerWidth > 480 ) {
+	if ( containerOuterWidth > 480 ) {
 		horizontalPadding = 40;
-	} else if ( containerWidth > 1920 ) {
+	} else if ( containerOuterWidth > 1920 ) {
 		horizontalPadding = 80;
 	}
-
-	const containerHeight =
-		context.core.image.figureRef.clientHeight - verticalPadding * 2;
+	const containerInnerWidth = containerOuterWidth - horizontalPadding * 2;
 
 	// Check difference between the image and figure dimensions
 	const widthOverflow = Math.abs(
-		Math.min( containerWidth - targetWidth, 0 )
+		Math.min( containerInnerWidth - targetWidth, 0 )
 	);
 	const heightOverflow = Math.abs(
-		Math.min( containerHeight - targetHeight, 0 )
+		Math.min( containerInnerHeight - targetHeight, 0 )
 	);
 
-	// If image is larger than its container any dimension, resize along its largest axis.
-	// For vertically oriented devices, always maximize the width.
+	// If the image is larger than the container, let's resize
+	// it along the greater axis relative to the container
 	if ( widthOverflow > 0 || heightOverflow > 0 ) {
-		if (
-			widthOverflow >= heightOverflow ||
-			containerHeight >= containerWidth
-		) {
-			targetWidth = containerWidth - horizontalPadding * 2;
+		const containerInnerAspectRatio =
+			containerInnerWidth / containerInnerHeight;
+		const imageAspectRatio = targetWidth / targetHeight;
+
+		if ( imageAspectRatio > containerInnerAspectRatio ) {
+			targetWidth = containerInnerWidth;
 			targetHeight =
-				imgDom.naturalHeight * ( targetWidth / imgDom.naturalWidth );
+				( targetWidth * imgDom.naturalHeight ) / imgDom.naturalWidth;
 		} else {
-			targetHeight = containerHeight;
+			targetHeight = containerInnerHeight;
 			targetWidth =
-				imgDom.naturalWidth * ( targetHeight / imgDom.naturalHeight );
+				( targetHeight * imgDom.naturalWidth ) / imgDom.naturalHeight;
 		}
 	}
 
@@ -261,16 +262,16 @@ function setZoomStyles( imgDom, context, event ) {
 
 	// Get values used to center the image
 	let targetLeft = 0;
-	if ( targetWidth >= containerWidth ) {
+	if ( targetWidth >= containerInnerWidth ) {
 		targetLeft = horizontalPadding;
 	} else {
-		targetLeft = ( containerWidth - targetWidth ) / 2;
+		targetLeft = ( containerOuterWidth - targetWidth ) / 2;
 	}
 	let targetTop = 0;
-	if ( targetHeight >= containerHeight ) {
+	if ( targetHeight >= containerInnerHeight ) {
 		targetTop = verticalPadding;
 	} else {
-		targetTop = ( containerHeight - targetHeight ) / 2 + verticalPadding;
+		targetTop = ( containerOuterHeight - targetHeight ) / 2;
 	}
 
 	const root = document.documentElement;

test/e2e/specs/editor/blocks/image.spec.js

@@ -853,7 +853,10 @@ test.describe( 'Image - interactivity', () => {
 
 			await page.getByRole( 'button', { name: 'Enlarge image' } ).click();
 
-			await expect( responsiveImage ).toHaveAttribute( 'src', '' );
+			await expect( responsiveImage ).toHaveAttribute(
+				'src',
+				new RegExp( filename )
+			);
 			await expect( enlargedImage ).toHaveAttribute(
 				'src',
 				new RegExp( filename )
@@ -866,6 +869,12 @@ test.describe( 'Image - interactivity', () => {
 			} );
 			await closeButton.click();
 
+			await expect( responsiveImage ).toHaveAttribute( 'src', '' );
+			await expect( enlargedImage ).toHaveAttribute(
+				'src',
+				new RegExp( filename )
+			);
+
 			await expect( lightbox ).toBeHidden();
 		} );
 
@@ -1093,6 +1102,11 @@ test.describe( 'Image - interactivity', () => {
 
 		await page.getByRole( 'button', { name: 'Enlarge image' } ).click();
 
+		await expect( responsiveImage ).toHaveAttribute( 'src', imgUrl );
+		await expect( enlargedImage ).toHaveAttribute( 'src', imgUrl );
+
+		await page.getByRole( 'button', { name: 'Close' } ).click();
+
 		await expect( responsiveImage ).toHaveAttribute( 'src', '' );
 		await expect( enlargedImage ).toHaveAttribute( 'src', imgUrl );
 	} );