Skip to content

Commit 73a12fa

Browse files
nikomakelanikomakela
committed
fix(carousel): handleUpdateSlideProps
HH-380. Old implementation related to Carousel's handleUpdateSlideProps function hada flaw, which this commit fixes. This commit also improves the documentation related to carousel context and carousel navigation.
1 parent b615719 commit 73a12fa

5 files changed

Lines changed: 121 additions & 37 deletions

File tree

‎src/core/carousel/Carousel.stories.tsx‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,11 @@ export const CarouselDefault = {
5353
},
5454
};
5555

56-
export const CarouselNoDots = {
56+
export const CarouselWithDots = {
5757
render: Template,
5858
args: {
59-
withDots: false,
59+
withDots: true,
60+
navigateWithDots: true,
6061
},
6162
};
6263

‎src/core/carousel/components/CarouselSlideButton.tsx‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,12 @@ export function CarouselPreviousSlideButton() {
1818
handleUpdateSlideProps,
1919
} = useCarouselContext();
2020

21-
const handlePrevClick = (): void =>
22-
handleUpdateSlideProps(
23-
-(currentSlide === 0 ? numberOfSlides - 1 : currentSlide - 1),
24-
);
21+
const handlePrevClick = (): void => {
22+
// Loop to the last slide if we are on the first, otherwise go to the previous.
23+
const targetSlide =
24+
currentSlide === 0 ? numberOfSlides - 1 : currentSlide - 1;
25+
handleUpdateSlideProps(targetSlide);
26+
};
2527

2628
return (
2729
<button
@@ -48,10 +50,12 @@ export function CarouselNextSlideButton() {
4850
handleUpdateSlideProps,
4951
} = useCarouselContext();
5052

51-
const handleNextClick = (): void =>
53+
const handleNextClick = (): void => {
54+
// Loop to the first slide if we are on the last, otherwise go to the next.
5255
handleUpdateSlideProps(
5356
currentSlide + 1 === numberOfSlides ? 0 : currentSlide + 1,
5457
);
58+
};
5559

5660
return (
5761
<button

‎src/core/carousel/components/CarouselSliderDot.tsx‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,11 @@ function CarouselSlideDot({ slideIndex }: { slideIndex: number }) {
1212

1313
const { t } = useTranslationWithFallback();
1414

15-
const onClickHandler = (slideNumber: number): void => {
16-
if (slideNumber === currentSlide) {
15+
const onClickHandler = (targetSlide: number): void => {
16+
if (targetSlide === currentSlide) {
1717
return;
1818
}
19-
if (slideNumber > currentSlide) {
20-
handleUpdateSlideProps(slideNumber);
21-
} else {
22-
handleUpdateSlideProps(-slideNumber);
23-
}
19+
handleUpdateSlideProps(targetSlide);
2420
};
2521
const dotButtonAriaLabel = t('carouselSliderDotNavLabelText').replace(
2622
'{slideNumber}',

‎src/core/carousel/context/CarouselContextProvider.tsx‎

Lines changed: 24 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -16,33 +16,35 @@ function useCarouselContextState(): Omit<
1616
CarouselContextStateType,
1717
'numberOfItems'
1818
> {
19-
const [isReady] = React.useState<boolean>(
19+
const [isReady] = React.useState<CarouselContextStateType['isReady']>(
2020
initialCarouselContextValues.isReady,
2121
);
22-
const [transformValue, setTransformValue] = React.useState<string>(
23-
initialCarouselContextValues.transformValue,
24-
);
25-
const [numberOfSlides, setNumberOfSlides] = React.useState<number>(
26-
initialCarouselContextValues.numberOfSlides,
27-
);
28-
const [itemsPerSlide, setItemsPerSlide] = React.useState<number>(
29-
initialCarouselContextValues.itemsPerSlide,
30-
);
31-
const [currentSlide, setCurrentSlide] = React.useState<number>(
32-
initialCarouselContextValues.currentSlide,
33-
);
34-
const [width, setWidth] = React.useState<number>(
22+
const [transformValue, setTransformValue] = React.useState<
23+
CarouselContextStateType['transformValue']
24+
>(initialCarouselContextValues.transformValue);
25+
const [numberOfSlides, setNumberOfSlides] = React.useState<
26+
CarouselContextStateType['numberOfSlides']
27+
>(initialCarouselContextValues.numberOfSlides);
28+
const [itemsPerSlide, setItemsPerSlide] = React.useState<
29+
CarouselContextStateType['itemsPerSlide']
30+
>(initialCarouselContextValues.itemsPerSlide);
31+
const [currentSlide, setCurrentSlide] = React.useState<
32+
CarouselContextStateType['currentSlide']
33+
>(initialCarouselContextValues.currentSlide);
34+
const [width, setWidth] = React.useState<CarouselContextStateType['width']>(
3535
initialCarouselContextValues.width,
3636
);
3737

38-
const handleUpdateSlideProps = (value: number) => {
39-
if (value === 0) {
40-
setTransformValue('0px');
41-
} else {
42-
setTransformValue(`${value > currentSlide ? '-' : ''}${value}00%`);
43-
}
44-
setCurrentSlide(Math.abs(value));
45-
};
38+
const handleUpdateSlideProps: CarouselContextStateType['handleUpdateSlideProps'] =
39+
(targetSlide: number): void => {
40+
if (targetSlide === 0) {
41+
setTransformValue('0px');
42+
} else {
43+
// The transform is ALWAYS negative, based purely on the target index.
44+
setTransformValue(`-${targetSlide}00%`);
45+
}
46+
setCurrentSlide(targetSlide);
47+
};
4648

4749
return {
4850
isReady,

‎src/core/carousel/types.ts‎

Lines changed: 82 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,18 +60,99 @@ export type CarouselContextBackwardCompatibilityProps = Pick<
6060
};
6161

6262
export type CarouselContextStateType = {
63+
/**
64+
* A flag indicating whether the carousel has been initialized and is ready.
65+
* Useful for preventing interactions before measurements are complete.
66+
* @type {boolean}
67+
*/
6368
isReady: boolean;
69+
/**
70+
* The CSS `transform` value (e.g., 'translateX(-200%)') applied
71+
* to the slider track to position the current slide in the viewport.
72+
* @type {string}
73+
*/
6474
transformValue: string;
75+
/**
76+
* Sets the CSS `transform` value for the slider track.
77+
* @param {string} value - The new CSS transform string.
78+
* @returns {void}
79+
*/
6580
setTransformValue: (value: string) => void;
81+
/**
82+
* The total number of slides in the carousel.
83+
* @type {number}
84+
*/
6685
numberOfSlides: number;
86+
/**
87+
* Sets the total number of slides.
88+
* @param {number} value - The new total number of slides.
89+
* @returns {void}
90+
*/
6791
setNumberOfSlides: (value: number) => void;
92+
/**
93+
* The number of individual items to display per slide.
94+
* @type {number}
95+
*/
6896
itemsPerSlide: number;
97+
/**
98+
* Sets the number of items to display per slide.
99+
* @param {number} value - The new number of items per slide.
100+
* @returns {void}
101+
*/
69102
setItemsPerSlide: (value: number) => void;
103+
/**
104+
* The zero-based index of the currently active slide.
105+
* @type {number}
106+
*/
70107
currentSlide: number;
108+
/**
109+
* Sets the index of the currently active slide.
110+
* @param {number} value - The new active slide index.
111+
* @returns {void}
112+
*/
71113
setCurrentSlide: (value: number) => void;
114+
/**
115+
* The measured width of the carousel container in pixels.
116+
* @type {number}
117+
*/
72118
width: number;
119+
/**
120+
* Sets the measured width of the carousel container.
121+
* @param {number} value - The new width in pixels.
122+
* @returns {void}
123+
*/
73124
setWidth: (value: number) => void;
74-
handleUpdateSlideProps: (value: number) => void;
125+
/**
126+
* Updates the CSS transform value and the current slide index for a slider component.
127+
*
128+
* @description This function calculates the necessary CSS `transform` value to move
129+
* to a target slide. It sets the transform to '0px' for the first slide (index 0).
130+
* For all other slides, it calculates a percentage-based transform (`value * 100%`).
131+
* The direction of the translation (positive or negative) is determined by comparing
132+
* the target slide `value` with the `currentSlide` state. It then updates the
133+
* state for the transform value and the current slide index.
134+
*
135+
* @param {number} targetSlide - The zero-based index of the target slide.
136+
* @returns {void} This function does not return a value.
137+
*
138+
* @example
139+
* // Assume currentSlide is 1, and we want to move to slide 3:
140+
* handleUpdateSlideProps(3);
141+
* // setTransformValue will be called with '-300%'
142+
* // setCurrentSlide will be called with 3
143+
*
144+
* @example
145+
* // Assume we want to go back to the first slide:
146+
* handleUpdateSlideProps(0);
147+
* // setTransformValue will be called with '0px'
148+
* // setCurrentSlide will be called with 0
149+
*/
150+
handleUpdateSlideProps: (targetSlide: number) => void;
151+
/**
152+
* The total number of individual items provided to the carousel.
153+
* This is used along with `itemsPerSlide` to calculate `numberOfSlides`.
154+
* @type {number}
155+
*/
75156
numberOfItems: number;
76157
};
77158

0 commit comments

Comments
 (0)