Skip to content

Commit

Permalink
fix: carousel not scroll syncing with state & add interaction tests
Browse files Browse the repository at this point in the history
  • Loading branch information
anuraghazra committed Nov 5, 2024
1 parent 2b027bf commit 982187b
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 11 deletions.
6 changes: 5 additions & 1 deletion packages/blade/src/components/Carousel/Carousel.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { isReactNative } from '~utils';
import { List, ListItem } from '~components/List';
import { Link } from '~components/Link';
import { useTheme } from '~components/BladeProvider';
import { Button } from '~components/Button';

const Page = (): React.ReactElement => {
return (
Expand Down Expand Up @@ -475,6 +476,9 @@ export const Controlled: StoryFn<typeof CarouselComponent> = (props) => {
Setting <Code>activeSlide</Code> & <Code>onChange</Code> you can control the active slide
and use the carousel in a controlled way. Here the active slide is {activeSlide}
</Text>
<Button marginY="spacing.4" size="small" onClick={() => setActiveSlide(2)}>
Go to slide #3
</Button>
<CarouselExample
{...props}
activeSlide={activeSlide}
Expand All @@ -488,7 +492,7 @@ export const Controlled: StoryFn<typeof CarouselComponent> = (props) => {
};

Controlled.args = {
visibleItems: 2,
visibleItems: 1,
};
Controlled.argTypes = {
shouldAddStartEndSpacing: {
Expand Down
32 changes: 22 additions & 10 deletions packages/blade/src/components/Carousel/Carousel.web.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import { useTheme } from '~components/BladeProvider';
import { getStyledProps } from '~components/Box/styledProps';
import { useControllableState } from '~utils/useControllable';
import { useIsomorphicLayoutEffect } from '~utils/useIsomorphicLayoutEffect';
import { useDidUpdate } from '~utils/useDidUpdate';

type ControlsProp = Required<
Pick<
Expand Down Expand Up @@ -259,7 +260,8 @@ const Carousel = ({
const [startEndMargin, setStartEndMargin] = React.useState(0);
const containerRef = React.useRef<HTMLDivElement>(null);
const isMobile = platform === 'onMobile';
const id = useId('carousel');
const id = useId();
const carouselId = `carousel-${id}`;

useVerifyAllowedChildren({
componentName: 'Carousel',
Expand Down Expand Up @@ -307,20 +309,20 @@ const Carousel = ({
if (!isResponsive && !shouldAddStartEndSpacing) return;
if (!containerRef.current) return;

const carouselItemId = getCarouselItemId(id, 0);
const carouselItemId = getCarouselItemId(carouselId, 0);
const carouselItem = containerRef.current.querySelector(carouselItemId);
if (!carouselItem) return;

const carouselItemLeft = carouselItem.getBoundingClientRect().left ?? 0;
const carouselContainerLeft = containerRef.current.getBoundingClientRect().left ?? 0;

setStartEndMargin(carouselItemLeft - carouselContainerLeft);
}, [id, isResponsive, shouldAddStartEndSpacing]);
}, [carouselId, isResponsive, shouldAddStartEndSpacing]);

const goToSlideIndex = (slideIndex: number, shouldAnimate = true) => {
const scrollToSlide = (slideIndex: number, shouldAnimate = true) => {
if (!containerRef.current) return;

const carouselItemId = getCarouselItemId(id, slideIndex * _visibleItems);
const carouselItemId = getCarouselItemId(carouselId, slideIndex * _visibleItems);
const carouselItem = containerRef.current.querySelector(carouselItemId);
if (!carouselItem) return;

Expand All @@ -333,6 +335,9 @@ const Carousel = ({
left: left - startEndMargin,
behavior: shouldAnimate ? 'smooth' : 'auto',
});
};

const goToSlideIndex = (slideIndex: number) => {
setActiveSlide(() => slideIndex);
setActiveIndicator(slideIndex);
};
Expand Down Expand Up @@ -443,8 +448,15 @@ const Carousel = ({

// set initial active slide on mount
useIsomorphicLayoutEffect(() => {
goToSlideIndex(activeSlide, false);
}, []);
if (!id) return;
goToSlideIndex(activeSlide);
scrollToSlide(activeSlide, false);
}, [id]);

// Scroll the carousel to the active slide
useDidUpdate(() => {
scrollToSlide(activeSlide);
}, [activeSlide]);

const carouselContext = React.useMemo<CarouselContextProps>(() => {
return {
Expand All @@ -453,14 +465,14 @@ const Carousel = ({
carouselItemWidth,
carouselContainerRef: containerRef,
setActiveIndicator,
carouselId: id,
carouselId,
totalNumberOfSlides,
activeSlide,
startEndMargin,
shouldAddStartEndSpacing,
};
}, [
id,
carouselId,
startEndMargin,
isResponsive,
_visibleItems,
Expand Down Expand Up @@ -530,7 +542,7 @@ const Carousel = ({
/>
) : null}
<CarouselBody
idPrefix={id}
idPrefix={carouselId}
startEndMargin={startEndMargin}
totalSlides={totalNumberOfSlides}
shouldAddStartEndSpacing={shouldAddStartEndSpacing}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import type { CarouselProps } from '../';
import { Carousel as CarouselComponent } from '../';
import { CarouselExample } from '../Carousel.stories';
import { Box } from '~components/Box';
import { Text } from '~components/Typography';
import { Button } from '~components/Button';

const sleep = (ms: number): Promise<void> => new Promise((resolve) => setTimeout(resolve, ms));

Expand Down Expand Up @@ -191,6 +193,53 @@ TestOnChangeParentUpdate.play = async ({ canvasElement }) => {
await expect(multipleOnChange).toBeCalledTimes(2);
};

const controlledOnChange = jest.fn();
export const TestControlledCarousel: StoryFn<typeof CarouselComponent> = (
props,
): React.ReactElement => {
const [activeIndex, setActiveIndex] = React.useState(3);

return (
<Box>
<Text>Current slide: {activeIndex}</Text>
<Button
onClick={() => {
setActiveIndex(5);
}}
>
Change slide
</Button>
<BasicCarousel
{...props}
visibleItems={1}
activeSlide={activeIndex}
onChange={(index) => {
console.log('index', index);
setActiveIndex(index);
controlledOnChange(index);
}}
/>
</Box>
);
};

TestControlledCarousel.play = async ({ canvasElement }) => {
const { getByText, getByRole } = within(canvasElement);
await sleep(1000);
await expect(controlledOnChange).not.toBeCalled();
await expect(getByText('Current slide: 3')).toBeInTheDocument();
const goToBtn = getByRole('button', { name: 'Change slide' });
await userEvent.click(goToBtn);
await expect(getByText('Current slide: 5')).toBeInTheDocument();
await sleep(1000);
await expect(controlledOnChange).not.toBeCalled();
const nextButton = getByRole('button', { name: 'Next Slide' });
await userEvent.click(nextButton);
await sleep(1000);
await expect(controlledOnChange).toBeCalledWith(6);
await expect(controlledOnChange).toBeCalledTimes(1);
};

export default {
title: 'Components/Interaction Tests/Carousel',
component: CarouselComponent,
Expand Down

0 comments on commit 982187b

Please sign in to comment.