fix(Carousel): SSR Layout shift issues (#991)

Telefonica · Jan 10, 2024 · aba1f65 · aba1f65 · github-actions · Jan 10, 2024
1 parent f7254c7
commit aba1f65
Show file tree

Hide file tree

Showing 5 changed files with 137 additions and 48 deletions.
diff --git a/src/__acceptance_tests__/carousel-ssr-acceptance-test.tsx b/src/__acceptance_tests__/carousel-ssr-acceptance-test.tsx
@@ -5,11 +5,12 @@ test('ssr carousel mobile', async () => {
 });
 
 test('ssr carousel tablet', async () => {
-    // TODO WEB-1672: fix hydration mismatches and enable the check
+    // checkHidrationVisualMismatch is disabled because for some reason there are some small visual differences in TABLET
+    // after hidration. Also, in some cases the carousel is scrolled to the next page and this causes a visual mismatch in
+    // the bullets. I've not found a way to know why this happens.
     await openSSRPage({name: 'carousel', device: 'TABLET', checkHidrationVisualMismatch: false});
 });
 
 test('ssr carousel desktop', async () => {
-    // TODO WEB-1672: fix hydration mismatches and enable the check
-    await openSSRPage({name: 'carousel', device: 'DESKTOP', checkHidrationVisualMismatch: false});
+    await openSSRPage({name: 'carousel', device: 'DESKTOP'});
 });
diff --git a/src/__acceptance_tests__/mosaic-ssr-acceptance-test.tsx b/src/__acceptance_tests__/mosaic-ssr-acceptance-test.tsx
@@ -1,6 +1,5 @@
 import {openSSRPage} from '../test-utils';
 
-// TODO WEB-1672: fix hydration visual mismatches and enable the check. These mismatches are caused by carousel when the itemsPerPage === items.length
 test('ssr VerticalMosaic and HorizontalMosaic', async () => {
-    await openSSRPage({name: 'mosaic', checkHidrationVisualMismatch: false});
+    await openSSRPage({name: 'mosaic'});
 });
diff --git a/src/carousel.css.ts b/src/carousel.css.ts
@@ -38,6 +38,40 @@ const bulletActiveBase = style([
     },
 ]);
 
+export const bulletButton = style([
+    sprinkles({
+        display: 'block',
+        padding: 0,
+        border: 'none',
+        background: 'transparent',
+        paddingLeft: 16,
+    }),
+    {
+        '@media': {
+            [mq.tabletOrSmaller]: {
+                paddingLeft: 8,
+            },
+        },
+    },
+]);
+
+const bulletButtonByBreakpoint = style([bulletButton, {display: 'none'}]);
+
+export const bulletButtonMobile = style([
+    bulletButtonByBreakpoint,
+    {'@media': {[mq.mobile]: {display: 'block'}}},
+]);
+
+export const bulletButtonTablet = style([
+    bulletButtonByBreakpoint,
+    {'@media': {[mq.tablet]: {display: 'block'}}},
+]);
+
+export const bulletButtonDesktop = style([
+    bulletButtonByBreakpoint,
+    {'@media': {[mq.desktopOrBigger]: {display: 'block'}}},
+]);
+
 export const bullet = style([bulletBase, sprinkles({background: skinVars.colors.control})]);
 export const bulletInverse = style([bulletBase, {background: applyAlpha(skinVars.rawColors.inverse, 0.5)}]);
 export const bulletActive = style([
@@ -146,10 +180,16 @@ export const carousel = style([
 
 const responsiveLayoutSideMargin = fallbackVar(responsiveLayoutVars.sideMargin, '0px');
 
-export const carouselWithScroll = style({
-    margin: 0,
+export const carouselWithScrollMobile = style({
     '@media': {
-        [mq.tabletOrSmaller]: {
+        [mq.mobile]: {
+            margin: `0 calc(${responsiveLayoutSideMargin} * -1)`,
+        },
+    },
+});
+export const carouselWithScrollTablet = style({
+    '@media': {
+        [mq.tablet]: {
             margin: `0 calc(${responsiveLayoutSideMargin} * -1)`,
         },
     },
@@ -177,45 +217,58 @@ export const centeredCarousel = style({
 export const carouselItem = style({
     scrollSnapAlign: 'start',
     flexShrink: 0,
-    width: `calc(1 / ${itemsPerPageDesktop} * 100% + ${gap} / ${itemsPerPageDesktop} * 1px)`,
+    width: `calc(1 / ${itemsPerPage} * 100% + ${gap} / ${itemsPerPage} * 1px)`,
     scrollMargin: `calc(-1px * ${gap})`,
     ':first-child': {
-        width: `calc(1 / ${itemsPerPageDesktop} * 100% - (${gap} * (${itemsPerPageDesktop} - 1)) / ${itemsPerPageDesktop} * 1px)`,
+        width: `calc(1 / ${itemsPerPage} * 100% - ${gap} * (${itemsPerPage} - 1) / ${itemsPerPage} * 1px)`,
         scrollMargin: 0,
     },
     '@media': {
+        [mq.desktopOrBigger]: {
+            vars: {
+                [itemsPerPage]: itemsPerPageDesktop,
+            },
+        },
         [mq.mobile]: {
             vars: {
                 [itemsPerPage]: itemsPerPageMobile,
             },
+            selectors: {
+                [`${carouselWithScrollMobile}:not(${centeredCarousel}) &`]: {
+                    width: `calc(1 / ${itemsPerPage} * 100% - (2 * ${mobilePageOffset} + ${gap} * 1px) / ${itemsPerPage})`,
+                },
+                [`${carouselWithScrollMobile}:not(${centeredCarousel}) &:first-child`]: {
+                    paddingLeft: responsiveLayoutSideMargin,
+                    width: `calc(1 / ${itemsPerPage} * 100% - (2 * ${mobilePageOffset} + ${gap} * 1px) / ${itemsPerPage} - ${gap} * 1px + ${responsiveLayoutSideMargin})`,
+                },
+                [`${carouselWithScrollMobile}:not(${centeredCarousel}) &:last-child`]: {
+                    paddingRight: responsiveLayoutSideMargin,
+                    width: `calc(1 / ${itemsPerPage} * 100% - (2 * ${mobilePageOffset} + ${gap} * 1px) / ${itemsPerPage} + ${responsiveLayoutSideMargin})`,
+                },
+            },
         },
         [mq.tablet]: {
             vars: {
                 [itemsPerPage]: itemsPerPageTablet,
             },
-        },
-        [mq.tabletOrSmaller]: {
-            width: `calc(1 / ${itemsPerPage} * 100% + ${gap} / ${itemsPerPage} * 1px)`,
-
-            ':first-child': {
-                width: `calc(1 / ${itemsPerPage} * 100% - ${gap} * (${itemsPerPage} - 1) / ${itemsPerPage} * 1px)`,
-            },
-
-            scrollSnapAlign: 'start',
-            scrollMargin: mobilePageOffset,
-
             selectors: {
-                [`${carouselWithScroll}:not(${centeredCarousel}) &`]: {
+                [`${carouselWithScrollTablet}:not(${centeredCarousel}) &`]: {
                     width: `calc(1 / ${itemsPerPage} * 100% - (2 * ${mobilePageOffset} + ${gap} * 1px) / ${itemsPerPage})`,
                 },
-                [`${carouselWithScroll}:not(${centeredCarousel}) &:first-child`]: {
+                [`${carouselWithScrollTablet}:not(${centeredCarousel}) &:first-child`]: {
                     paddingLeft: responsiveLayoutSideMargin,
                     width: `calc(1 / ${itemsPerPage} * 100% - (2 * ${mobilePageOffset} + ${gap} * 1px) / ${itemsPerPage} - ${gap} * 1px + ${responsiveLayoutSideMargin})`,
                 },
-                [`${carouselWithScroll}:not(${centeredCarousel}) &:last-child`]: {
+                [`${carouselWithScrollTablet}:not(${centeredCarousel}) &:last-child`]: {
                     paddingRight: responsiveLayoutSideMargin,
                     width: `calc(1 / ${itemsPerPage} * 100% - (2 * ${mobilePageOffset} + ${gap} * 1px) / ${itemsPerPage} + ${responsiveLayoutSideMargin})`,
                 },
+            },
+        },
+        [mq.tabletOrSmaller]: {
+            scrollMargin: mobilePageOffset,
+
+            selectors: {
                 [`${centeredCarousel} &`]: {
                     width: '50%',
                     scrollSnapAlign: 'center',
@@ -298,9 +351,22 @@ export const carouselBullets = style([
     sprinkles({
         display: 'flex',
         justifyContent: 'center',
+        paddingTop: 24,
     }),
 ]);
 
+export const noCarouselBulletsDesktop = style({
+    '@media': {[mq.desktopOrBigger]: {display: 'none'}},
+});
+
+export const noCarouselBulletsTablet = style({
+    '@media': {[mq.tablet]: {display: 'none'}},
+});
+
+export const noCarouselBulletsMobile = style({
+    '@media': {[mq.mobile]: {display: 'none'}},
+});
+
 export const slideshow = style([
     hideScrollbar,
     sprinkles({

diff --git a/src/carousel.tsx b/src/carousel.tsx
@@ -4,7 +4,6 @@ import IconChevronLeftRegular from './generated/mistica-icons/icon-chevron-left-
 import IconChevronRightRegular from './generated/mistica-icons/icon-chevron-right-regular';
 import {useIsInViewport, useScreenSize, useTheme} from './hooks';
 import Inline from './inline';
-import Stack from './stack';
 import {BaseTouchable} from './touchable';
 import classNames from 'classnames';
 import {useIsInverseVariant, ThemeVariant} from './theme-variant-context';
@@ -13,13 +12,12 @@ import {isAndroid, isIos, isRunningAcceptanceTest} from './utils/platform';
 import {useDocumentVisibility} from './utils/document-visibility';
 import * as styles from './carousel.css';
 import * as mediaStyles from './image.css';
-import {sprinkles} from './sprinkles.css';
 import {useDesktopContainerType} from './desktop-container-type-context';
 import {VIVO_NEW_SKIN} from './skins/constants';
 import {applyCssVars} from './utils/css';
 
 import type {DesktopContainerType} from './desktop-container-type-context';
-import type {DataAttributes} from './utils/types';
+import type {ByBreakpoint, DataAttributes} from './utils/types';
 
 const useShouldAutoplay = (autoplay: boolean, ref: React.RefObject<HTMLElement>): boolean => {
     const isDocumentVisible = useDocumentVisibility();
@@ -29,7 +27,7 @@ const useShouldAutoplay = (autoplay: boolean, ref: React.RefObject<HTMLElement>)
 
 type PageBulletsProps = {
     currentIndex: number;
-    numPages: number;
+    numPages: number | ByBreakpoint<number>;
     onPress?: (index: number) => void;
 };
 
@@ -45,20 +43,25 @@ export const PageBullets: React.FC<PageBulletsProps> = ({currentIndex, numPages,
         }
     };
 
+    const maxNumPages =
+        typeof numPages === 'number'
+            ? numPages
+            : Math.max(numPages.mobile, numPages.tablet ?? numPages.mobile, numPages.desktop);
+
     return (
-        <Inline
-            space={{mobile: 8, desktop: 16}}
-            alignItems="center"
-            dataAttributes={{'component-name': 'PageBullets'}}
-        >
-            {Array.from({length: numPages}, (_, i: number) => (
+        <Inline space={0} alignItems="center" dataAttributes={{'component-name': 'PageBullets'}}>
+            {Array.from({length: maxNumPages}, (_, i: number) => (
                 <BaseTouchable
-                    className={sprinkles({
-                        display: 'block',
-                        padding: 0,
-                        border: 'none',
-                        background: 'transparent',
-                    })}
+                    className={classNames(
+                        typeof numPages === 'number'
+                            ? styles.bulletButton
+                            : {
+                                  [styles.bulletButtonMobile]: i < numPages.mobile,
+                                  [styles.bulletButtonTablet]: i < (numPages.tablet ?? numPages.mobile),
+                                  [styles.bulletButtonDesktop]: i < numPages.desktop,
+                              }
+                    )}
+                    style={i === 0 ? {paddingLeft: 0} : {}}
                     key={i}
                     maybe
                     onPress={isDesktopOrBigger && onPress ? () => onPress(i) : undefined}
@@ -212,6 +215,9 @@ const BaseCarousel: React.FC<BaseCarouselProps> = ({
 
     const carouselRef = React.useRef<HTMLDivElement>(null);
 
+    const pagesCountMobile = Math.ceil(items.length / Math.max(Math.floor(itemsPerPageConfig.mobile), 1));
+    const pagesCountTablet = Math.ceil(items.length / Math.max(Math.floor(itemsPerPageConfig.tablet), 1));
+    const pagesCountDesktop = Math.ceil(items.length / Math.max(Math.floor(itemsPerPageConfig.desktop), 1));
     const pagesCount = Math.ceil(items.length / itemsPerPageFloor);
     const [{scrollLeft, scrollRight}, setScroll] = React.useState({scrollLeft: 0, scrollRight: 0});
     const [itemScrollPositions, setItemScrollPositions] = React.useState<Array<number>>([]);
@@ -371,16 +377,24 @@ const BaseCarousel: React.FC<BaseCarouselProps> = ({
     if (renderBullets) {
         bullets = renderBullets({numPages: pagesCount, currentIndex: currentPageIndex, onPress: goToPage});
     } else if (withBullets) {
-        bullets = pagesCount > 1 && (
-            <PageBullets numPages={pagesCount} currentIndex={currentPageIndex} onPress={goToPage} />
+        bullets = (
+            <PageBullets
+                numPages={{
+                    mobile: pagesCountMobile,
+                    tablet: pagesCountTablet,
+                    desktop: pagesCountDesktop,
+                }}
+                currentIndex={currentPageIndex}
+                onPress={goToPage}
+            />
         );
     }
 
     const largePageOffset = '64px';
     const vivoNewMobilePageOffset = '36px';
 
     return (
-        <Stack space={24} dataAttributes={{'component-name': 'Carousel', ...dataAttributes}}>
+        <div {...getPrefixedDataAttributes({'component-name': 'Carousel', ...dataAttributes})}>
             <div className={styles.carouselContainer}>
                 <ThemeVariant isInverse={false}>
                     <BaseTouchable
@@ -395,7 +409,8 @@ const BaseCarousel: React.FC<BaseCarouselProps> = ({
                 <div
                     className={classNames(styles.carousel, {
                         [styles.centeredCarousel]: centered,
-                        [styles.carouselWithScroll]: pagesCount > 1,
+                        [styles.carouselWithScrollMobile]: pagesCountMobile > 1,
+                        [styles.carouselWithScrollTablet]: pagesCountTablet > 1,
                     })}
                     style={{
                         ...applyCssVars({
@@ -448,8 +463,16 @@ const BaseCarousel: React.FC<BaseCarouselProps> = ({
                     </BaseTouchable>
                 </ThemeVariant>
             </div>
-            {bullets && <div className={styles.carouselBullets}>{bullets}</div>}
-        </Stack>
+            <div
+                className={classNames(styles.carouselBullets, {
+                    [styles.noCarouselBulletsDesktop]: pagesCountDesktop <= 1,
+                    [styles.noCarouselBulletsTablet]: pagesCountTablet <= 1,
+                    [styles.noCarouselBulletsMobile]: pagesCountMobile <= 1,
+                })}
+            >
+                {bullets}
+            </div>
+        </div>
     );
 };
 

diff --git a/src/test-utils/index.tsx b/src/test-utils/index.tsx
@@ -103,8 +103,8 @@ const DEVICES: DeviceCollection = {
             width: 800,
             height: 600,
             deviceScaleFactor: 1,
-            isMobile: false,
-            hasTouch: false,
+            isMobile: true,
+            hasTouch: true,
             isLandscape: false,
         },
         userAgent: '',