Product Gallery Thumbnails: Fix overflow issues and improve responsiv…

…eness (#11665) * Product Gallery Thumbnails: Refactor sizing in the editor and the front end * Product Gallery Thumbnails: Change default vertical alignment to top and better control the width of the thumbnails * Product Gallery Thumbnails: Restrict the bottom position thumbnails width based on the total number of thumbnails set * Product Gallery: Remove hardcoded width for Thumbnails and the Large Image and update the width inside of the Dialog * Product Gallery Thumbnails: Introduce thumbnails scaling based on the number of thumbnails * Product Gallery Thumbnails: Fix editor thumbnails scaling * Product Gallery Thumbnails: Remove unused column gap variable * Product Gallery Thumbnails: Fix styling for vertical images * Product Gallery: Remove the unused editor.scss file * Product Gallery: Fix the placement of the Thumbnails block in the block template * Product Gallery Dialog: Reset changes to the dialog * update @wordpress/e2e-test-utils-playwright package * don't update node version * remove waitForSiteEditorFinishLoading function * use visitSiteEditor util * Product Gallery Thumbnails: Add code comments * Product Gallery Thumbnails E2E: Fix the test checking the default position of the thumbnails * Product Gallery E2E: Fix the test checking if the cropping setting works correctly * Product Gallery Thumbnails: Hide the Thumbnails block if there aren't at least 2 thumbnails to display --------- Co-authored-by: Paulo Arromba <[email protected]> Co-authored-by: Luigi Teschio <[email protected]>
woocommerce · Nov 24, 2023 · bf2ce9e · bf2ce9e
1 parent 9d234fc
commit bf2ce9e
Show file tree

Hide file tree

Showing 10 changed files with 183 additions and 71 deletions.
diff --git a/assets/js/blocks/product-gallery/edit.tsx b/assets/js/blocks/product-gallery/edit.tsx
@@ -25,7 +25,13 @@ import type { ProductGalleryAttributes } from './types';
 const TEMPLATE: InnerBlockTemplate[] = [
 	[
 		'core/group',
-		{ layout: { type: 'flex', flexWrap: 'nowrap' } },
+		{
+			layout: {
+				type: 'flex',
+				flexWrap: 'nowrap',
+				verticalAlignment: 'top',
+			},
+		},
 		[
 			[
 				'woocommerce/product-gallery-thumbnails',
@@ -38,6 +44,10 @@ const TEMPLATE: InnerBlockTemplate[] = [
 						type: 'flex',
 						orientation: 'vertical',
 						justifyContent: 'center',
+						verticalAlignment: 'top',
+					},
+					style: {
+						layout: { selfStretch: 'fixed', flexSize: '100%' },
 					},
 					...getInnerBlocksLockAttributes( 'lock' ),
 				},

diff --git a/assets/js/blocks/product-gallery/editor.scss b/assets/js/blocks/product-gallery/editor.scss
diff --git a/assets/js/blocks/product-gallery/inner-blocks/product-gallery-large-image/editor.scss b/assets/js/blocks/product-gallery/inner-blocks/product-gallery-large-image/editor.scss
@@ -1,4 +1,6 @@
 .wc-block-editor-product-gallery-large-image {
+	width: 100%;
+
 	img {
 		max-width: 100%;
 		margin: 0 auto;

diff --git a/...s/blocks/product-gallery/inner-blocks/product-gallery-thumbnails/block-settings/index.tsx b/...s/blocks/product-gallery/inner-blocks/product-gallery-thumbnails/block-settings/index.tsx
@@ -51,7 +51,7 @@ export const ProductGalleryThumbnailsBlockSettings = ( {
 	context,
 }: ProductGalleryThumbnailsSettingsProps ) => {
 	const maxNumberOfThumbnails = 8;
-	const minNumberOfThumbnails = 2;
+	const minNumberOfThumbnails = 3;
 	const { productGalleryClientId } = context;
 	// @ts-expect-error @wordpress/block-editor/store types not provided
 	const { updateBlockAttributes } = useDispatch( blockEditorStore );
@@ -110,7 +110,7 @@ export const ProductGalleryThumbnailsBlockSettings = ( {
 						} )
 					}
 					help={ __(
-						'Choose how many thumbnails (2-8) will display. If more images exist, a “View all” button will display.',
+						'Choose how many thumbnails (3-8) will display. If more images exist, a “View all” button will display.',
 						'woo-gutenberg-products-block'
 					) }
 					max={ maxNumberOfThumbnails }

diff --git a/assets/js/blocks/product-gallery/inner-blocks/product-gallery-thumbnails/edit.tsx b/assets/js/blocks/product-gallery/inner-blocks/product-gallery-thumbnails/edit.tsx
@@ -25,26 +25,29 @@ interface EditProps
 
 export const Edit = ( { attributes, setAttributes, context }: EditProps ) => {
 	const blockProps = useBlockProps( {
-		className: 'wc-block-product-gallery-thumbnails',
+		className: classNames(
+			'wc-block-product-gallery-thumbnails',
+			`wc-block-product-gallery-thumbnails--number-of-thumbnails-${ context.thumbnailsNumberOfThumbnails }`,
+			`wc-block-product-gallery-thumbnails--position-${ context.thumbnailsPosition }`
+		),
 	} );
 
 	const Placeholder = () => {
 		return context.thumbnailsPosition !== ThumbnailsPosition.OFF ? (
-			<div
-				className={ classNames(
-					'wc-block-editor-product-gallery-thumbnails',
-					`wc-block-editor-product-gallery-thumbnails--${ context.thumbnailsPosition }`
-				) }
-			>
+			<div className="wc-block-editor-product-gallery-thumbnails">
 				{ [
 					...Array( context.thumbnailsNumberOfThumbnails ).keys(),
 				].map( ( index ) => {
 					return (
-						<img
+						<div
+							className="wc-block-product-gallery-thumbnails__thumbnail"
 							key={ index }
-							src={ `${ WC_BLOCKS_IMAGE_URL }block-placeholders/product-image-gallery.svg` }
-							alt="Placeholder"
-						/>
+						>
+							<img
+								src={ `${ WC_BLOCKS_IMAGE_URL }block-placeholders/product-image-gallery.svg` }
+								alt="Placeholder"
+							/>
+						</div>
 					);
 				} ) }
 			</div>

diff --git a/assets/js/blocks/product-gallery/inner-blocks/product-gallery-thumbnails/editor.scss b/assets/js/blocks/product-gallery/inner-blocks/product-gallery-thumbnails/editor.scss
@@ -1,17 +1,30 @@
-.wc-block-product-gallery-thumbnails {
-	width: fit-content;
-	.wc-block-editor-product-gallery-thumbnails {
-		display: flex;
-		flex-flow: column nowrap;
+$thumbnails: ".wc-block-editor-product-gallery-thumbnails";
+$thumbnails-gap: 15px;
 
-		&.wc-block-editor-product-gallery-thumbnails--bottom {
-			flex-flow: row nowrap;
-		}
+#{$thumbnails} {
+	display: flex;
 
-		img {
-			width: 100px;
-			height: 100px;
-			margin: 5px;
-		}
+	.wc-block-product-gallery-thumbnails--position-bottom & {
+		flex-direction: row;
+		gap: 0 15px;
+	}
+
+	.wc-block-product-gallery-thumbnails:not(.wc-block-product-gallery-thumbnails--position-bottom) & {
+		flex-direction: column;
+		gap: 15px 0;
+	}
+}
+
+@for $i from 3 through 8 {
+	// Calculate the total width occupied by the gaps between thumbnails.
+	$gap-width: $thumbnails-gap * ($i - 1);
+
+	// Calculate the border width, which is multiplied by 2 to account for both sides of each thumbnail.
+	$border-width: ($i * 1px * 2);
+
+	$additional-space: $i * 1px;
+
+	.wc-block-product-gallery-thumbnails--number-of-thumbnails-#{$i}:not(.wc-block-product-gallery-thumbnails--position-bottom) {
+		flex-basis: calc((100% - #{$gap-width} - #{$border-width} - #{$additional-space}) / #{$i});
 	}
 }
diff --git a/assets/js/blocks/product-gallery/style.scss b/assets/js/blocks/product-gallery/style.scss
@@ -10,6 +10,8 @@ $gallery-next-previous-inside-image: "#{$gallery}:not([data-next-previous-button
 
 $outside-image-offset: 30px;
 $outside-image-max-width: calc(100% - (2 * $outside-image-offset));
+$thumbnails-gap: 15px;
+$default-number-of-thumbnails: 3;
 
 // Product Gallery
 #{$gallery} {
@@ -55,6 +57,7 @@ $outside-image-max-width: calc(100% - (2 * $outside-image-offset));
 	height: fit-content;
 	position: relative;
 	overflow: hidden;
+	flex-grow: 1;
 
 	// Restrict the width of the Large Image when the Next/Previous buttons are outside the image.
 	#{$gallery-next-previous-outside-image} & .wc-block-product-gallery-large-image__image-element {
@@ -64,10 +67,13 @@ $outside-image-max-width: calc(100% - (2 * $outside-image-offset));
 	}
 
 	.wc-block-product-gallery-large-image__wrapper {
+		aspect-ratio: 1 / 1;
 		flex-shrink: 0;
 		max-width: 100%;
 		overflow: hidden;
 		width: 100%;
+		display: flex;
+		align-items: center;
 	}
 
 	.wc-block-product-gallery-large-image__container {
@@ -93,7 +99,8 @@ $outside-image-max-width: calc(100% - (2 * $outside-image-offset));
 		margin: 0 auto;
 		z-index: 1;
 		transition: all 0.1s linear;
-		width: auto;
+		aspect-ratio: 1 / 1;
+		object-fit: contain;
 
 		// Keep the order in this way. The hoverZoom class should override the full-screen-on-click class when both are applied.
 		&.wc-block-woocommerce-product-gallery-large-image__image--full-screen-on-click {
@@ -234,20 +241,82 @@ $outside-image-max-width: calc(100% - (2 * $outside-image-offset));
 
 // Thumbnails
 #{$thumbnails} {
+	display: flex;
+
 	img {
 		cursor: pointer;
+		height: auto;
+		width: auto;
+		max-width: 100%;
 	}
 
-	.is-vertical & {
-		display: flex;
+	#{$gallery}[data-thumbnails-position='bottom'] & {
 		flex-direction: row;
+		gap: 0 15px;
+	}
+
+	#{$gallery}:not([data-thumbnails-position='bottom']) & {
+		flex-direction: column;
+		gap: 15px 0;
+
+		// Calculate the total width occupied by the gaps between thumbnails.
+		$gap-width: $thumbnails-gap * ($default-number-of-thumbnails - 1);
+
+		// Calculate the border width, which is multiplied by 2 to account for both sides of each thumbnail.
+		$border-width: #{$default-number-of-thumbnails * 1px * 2};
+
+		// Calculate the width of each thumbnail by accounting for the gap, border, and additional space.
+		flex-basis: calc((100% - #{$gap-width} - #{$border-width} - 4px) / #{$default-number-of-thumbnails});
+	}
+
+	@for $i from 3 through 8 {
+		#{$gallery}[data-thumbnails-number-of-thumbnails='#{$i}']:not([data-thumbnails-position='bottom']) & {
+			// Calculate the total width occupied by the gaps between thumbnails.
+			$gap-width: $thumbnails-gap * ($i - 1);
+
+			// Calculate the border width, which is multiplied by 2 to account for both sides of each thumbnail.
+			$border-width: $i * 1px * 2;
+
+			flex-basis: calc((100% - #{$gap-width} - #{$border-width}) / $i);
+		}
 	}
 
 	.wc-block-product-gallery-thumbnails__thumbnail {
-		width: 100px;
-		height: 100px;
-		margin: 5px;
+		border: 1px solid rgba($color: #000, $alpha: 0.1);
+		height: auto;
+		width: auto;
+		display: flex;
+		justify-content: center;
+		align-items: center;
+		aspect-ratio: 1 / 1;
 		position: relative;
+		flex-basis: 0;
+		flex-grow: 1;
+
+		img {
+			aspect-ratio: 1 / 1;
+			object-fit: contain;
+		}
+
+		&::before {
+			content: "";
+			display: block;
+			padding-top: 100%;
+		}
+
+		@for $i from 3 through 8 {
+			#{$gallery}[data-thumbnails-number-of-thumbnails='#{$i}'][data-thumbnails-position="bottom"] & {
+				// Calculate the total width occupied by the gaps between thumbnails.
+				$gap-width: $thumbnails-gap * ($i - 1);
+
+				// Calculate the border width, which is multiplied by 2 to account for both sides of each thumbnail.
+				$border-width: $i * 1px * 2;
+
+				$thumbnail-width: calc((100% - #{$gap-width} - #{$border-width}) / $i);
+
+				flex: 0 0 $thumbnail-width;
+			}
+		}
 
 	}
 

diff --git a/src/BlockTypes/ProductGalleryThumbnails.php b/src/BlockTypes/ProductGalleryThumbnails.php
@@ -133,8 +133,8 @@ protected function render( $attributes, $content, $block ) {
 
 			if ( $product ) {
 				$post_thumbnail_id      = $product->get_image_id();
-				$product_gallery_images = ProductGalleryUtils::get_product_gallery_images( $post_id, 'thumbnail', array(), 'wc-block-product-gallery-thumbnails__thumbnail' );
-				if ( $product_gallery_images && $post_thumbnail_id ) {
+				$product_gallery_images = ProductGalleryUtils::get_product_gallery_images( $post_id, 'full', array(), 'wc-block-product-gallery-thumbnails__thumbnail' );
+				if ( $product_gallery_images && count( $product_gallery_images ) > 1 && $post_thumbnail_id ) {
 					$html                 = '';
 					$number_of_thumbnails = isset( $block->context['thumbnailsNumberOfThumbnails'] ) ? $block->context['thumbnailsNumberOfThumbnails'] : 3;
 					$mode                 = $block->context['mode'] ?? '';

diff --git a/...ks/product-gallery-thumbnails/product-gallery-thumbnails.block_theme.side_effects.spec.ts b/...ks/product-gallery-thumbnails/product-gallery-thumbnails.block_theme.side_effects.spec.ts
@@ -52,6 +52,8 @@ test.describe( `${ blockData.name }`, () => {
 		page,
 		editor,
 		pageObject,
+		editorUtils,
+		frontendUtils,
 	} ) => {
 		await editor.insertBlock( {
 			name: 'woocommerce/product-gallery',
@@ -60,22 +62,38 @@ test.describe( `${ blockData.name }`, () => {
 		const thumbnailsBlock = await pageObject.getThumbnailsBlock( {
 			page: 'editor',
 		} );
-		const largeImageBlock = await pageObject.getMainImageBlock( {
-			page: 'editor',
-		} );
-
-		const thumbnailsBlockBoundingRect = await thumbnailsBlock.boundingBox();
-		const largeImageBlockBoundingRect = await largeImageBlock.boundingBox();
 
 		await expect( thumbnailsBlock ).toBeVisible();
-		// Check the default position: on the left of the large image
-		await expect( thumbnailsBlockBoundingRect?.y ).toBeGreaterThan(
-			largeImageBlockBoundingRect?.y as number
+
+		// We should refactor this.
+		// eslint-disable-next-line playwright/no-wait-for-timeout
+		await page.waitForTimeout( 500 );
+
+		// Test the default (left) position of thumbnails by cross-checking:
+		// - The Gallery block has the classes "is-layout-flex" and "is-nowrap".
+		// - The Thumbnails block has a lower index than the Large Image block.
+
+		const groupBlock = (
+			await editorUtils.getBlockByTypeWithParent(
+				'core/group',
+				'woocommerce/product-gallery'
+			)
+		 ).first();
+
+		const groupBlockClassAttribute = await groupBlock.getAttribute(
+			'class'
 		);
-		await expect( thumbnailsBlockBoundingRect?.x ).toBeLessThan(
-			largeImageBlockBoundingRect?.x as number
+		expect( groupBlockClassAttribute ).toContain( 'is-layout-flex' );
+		expect( groupBlockClassAttribute ).toContain( 'is-nowrap' );
+
+		const isThumbnailsBlockEarlier = await editorUtils.isBlockEarlierThan(
+			groupBlock,
+			'woocommerce/product-gallery-thumbnails',
+			'core/group'
 		);
 
+		expect( isThumbnailsBlockEarlier ).toBe( true );
+
 		await Promise.all( [
 			editor.saveSiteEditorEntities(),
 			page.waitForResponse( ( response ) =>
@@ -87,27 +105,27 @@ test.describe( `${ blockData.name }`, () => {
 			waitUntil: 'commit',
 		} );
 
-		const thumbnailsBlockFrontend = await pageObject.getThumbnailsBlock( {
-			page: 'frontend',
-		} );
-
-		const largeImageBlockFrontend = await pageObject.getMainImageBlock( {
-			page: 'frontend',
-		} );
+		const groupBlockFrontend = (
+			await frontendUtils.getBlockByClassWithParent(
+				'wp-block-group',
+				'woocommerce/product-gallery'
+			)
+		 ).first();
+
+		const groupBlockFrontendClassAttribute =
+			await groupBlockFrontend.getAttribute( 'class' );
+		expect( groupBlockFrontendClassAttribute ).toContain(
+			'is-layout-flex'
+		);
+		expect( groupBlockFrontendClassAttribute ).toContain( 'is-nowrap' );
 
-		const thumbnailsBlockFrontendBoundingRect =
-			await thumbnailsBlockFrontend.boundingBox();
-		const largeImageBlockFrontendBoundingRect =
-			await largeImageBlockFrontend.boundingBox();
+		const isThumbnailsFrontendBlockEarlier =
+			await frontendUtils.isBlockEarlierThanGroupBlock(
+				groupBlockFrontend,
+				'woocommerce/product-gallery-thumbnails'
+			);
 
-		await expect( thumbnailsBlockFrontend ).toBeVisible();
-		// Check the default position: on the left of the large image
-		await expect( thumbnailsBlockFrontendBoundingRect?.y ).toBeGreaterThan(
-			largeImageBlockFrontendBoundingRect?.y as number
-		);
-		await expect( thumbnailsBlockFrontendBoundingRect?.x ).toBeLessThan(
-			largeImageBlockFrontendBoundingRect?.x as number
-		);
+		expect( isThumbnailsFrontendBlockEarlier ).toBe( true );
 	} );
 
 	test.describe( `${ blockData.name } Settings`, () => {

diff --git a/tests/e2e/tests/product-gallery/product-gallery.block_theme.side_effects.spec.ts b/tests/e2e/tests/product-gallery/product-gallery.block_theme.side_effects.spec.ts
@@ -327,6 +327,8 @@ test.describe( `${ blockData.name }`, () => {
 		const width = image?.width;
 
 		// Allow 1 pixel of difference.
-		expect( width === height + 1 || width === height - 1 ).toBeTruthy();
+		expect(
+			width === height + 1 || width === height - 1 || width === height
+		).toBeTruthy();
 	} );
 } );