From bf2ce9e6b5b931f99113dddef9feddd31ba5af61 Mon Sep 17 00:00:00 2001 From: Daniel Dudzic Date: Fri, 24 Nov 2023 21:31:02 +0100 Subject: [PATCH] Product Gallery Thumbnails: Fix overflow issues and improve responsiveness (#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 <17236129+wavvves@users.noreply.github.com> Co-authored-by: Luigi Teschio --- assets/js/blocks/product-gallery/edit.tsx | 12 ++- assets/js/blocks/product-gallery/editor.scss | 5 -- .../product-gallery-large-image/editor.scss | 2 + .../block-settings/index.tsx | 4 +- .../product-gallery-thumbnails/edit.tsx | 25 +++--- .../product-gallery-thumbnails/editor.scss | 39 ++++++--- assets/js/blocks/product-gallery/style.scss | 81 +++++++++++++++++-- src/BlockTypes/ProductGalleryThumbnails.php | 4 +- ...humbnails.block_theme.side_effects.spec.ts | 78 +++++++++++------- ...t-gallery.block_theme.side_effects.spec.ts | 4 +- 10 files changed, 183 insertions(+), 71 deletions(-) delete mode 100644 assets/js/blocks/product-gallery/editor.scss diff --git a/assets/js/blocks/product-gallery/edit.tsx b/assets/js/blocks/product-gallery/edit.tsx index 9264f5b07ec..6aeeb77d1f0 100644 --- 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 deleted file mode 100644 index 13116d7fc2d..00000000000 --- a/assets/js/blocks/product-gallery/editor.scss +++ /dev/null @@ -1,5 +0,0 @@ -.wc-block-product-gallery { - .block-editor-inner-blocks { - width: fit-content; - } -} 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 index 65b0e5b7bf6..1c3198620cc 100644 --- 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/assets/js/blocks/product-gallery/inner-blocks/product-gallery-thumbnails/block-settings/index.tsx b/assets/js/blocks/product-gallery/inner-blocks/product-gallery-thumbnails/block-settings/index.tsx index cdf4e133cfb..640497103c4 100644 --- a/assets/js/blocks/product-gallery/inner-blocks/product-gallery-thumbnails/block-settings/index.tsx +++ b/assets/js/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 index d62da00ad1a..b925d005f05 100644 --- 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 ? ( -
+
{ [ ...Array( context.thumbnailsNumberOfThumbnails ).keys(), ].map( ( index ) => { return ( - Placeholder + > + Placeholder +
); } ) }
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 index 006c477d7f1..a789d44745b 100644 --- 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 index f73e0d735cd..191bc88bf4b 100644 --- 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 index 3704b0f93a4..13d9f4a0e87 100644 --- 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/tests/e2e/tests/product-gallery/inner-blocks/product-gallery-thumbnails/product-gallery-thumbnails.block_theme.side_effects.spec.ts b/tests/e2e/tests/product-gallery/inner-blocks/product-gallery-thumbnails/product-gallery-thumbnails.block_theme.side_effects.spec.ts index 1b93c340685..f749265e177 100644 --- a/tests/e2e/tests/product-gallery/inner-blocks/product-gallery-thumbnails/product-gallery-thumbnails.block_theme.side_effects.spec.ts +++ b/tests/e2e/tests/product-gallery/inner-blocks/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 index aff6c83330c..e4a051a4276 100644 --- 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(); } ); } );