diff --git a/.changeset/carousel-circular-transition.md b/.changeset/carousel-circular-transition.md new file mode 100644 index 0000000000..fbc7eb7335 --- /dev/null +++ b/.changeset/carousel-circular-transition.md @@ -0,0 +1,12 @@ +--- +"flowbite-react": patch +--- + +fix(carousel): seamless circular transition between last and first slides + +### Changes + +- [x] implement circular carousel using clone-based technique for smooth wrap-around +- [x] extract animation duration to named constant +- [x] prevent memory leak with timeout cleanup on unmount +- [x] guard initialization to prevent reset on dynamic children updates diff --git a/packages/ui/src/components/Carousel/Carousel.test.tsx b/packages/ui/src/components/Carousel/Carousel.test.tsx index 0cb726989e..b5450982e1 100644 --- a/packages/ui/src/components/Carousel/Carousel.test.tsx +++ b/packages/ui/src/components/Carousel/Carousel.test.tsx @@ -7,6 +7,10 @@ import { Carousel } from "./Carousel"; beforeEach(() => { vi.useFakeTimers(); vi.spyOn(global, "setTimeout"); + // Mock scrollTo since jsdom does not implement it + Element.prototype.scrollTo = vi.fn(); + // Mock clientWidth so scroll-based index calculations work correctly in jsdom + Object.defineProperty(HTMLElement.prototype, "clientWidth", { configurable: true, value: 100 }); }); afterEach(() => { @@ -19,8 +23,9 @@ describe("Components / Carousel", () => { it("should render and show first item", () => { render(); - expect(carouselItems()[0]).toHaveAttribute("data-active", "true"); - expect(carouselItems()[1]).toHaveAttribute("data-active", "false"); + // With cloned slides the real items start at index 1 + expect(realCarouselItems()[0]).toHaveAttribute("data-active", "true"); + expect(realCarouselItems()[1]).toHaveAttribute("data-active", "false"); expect(carouselIndicators()).toHaveLength(5); expect(carouselIndicators()[0]).toHaveClass(activeIndicatorClasses); expect(carouselIndicators()[1]).toHaveClass(nonActiveIndicatorClasses); @@ -38,14 +43,14 @@ describe("Components / Carousel", () => { render(); expect(carouselIndicators()[0]).toHaveClass(activeIndicatorClasses); - expect(carouselItems()[0]).toHaveAttribute("data-active", "true"); + expect(realCarouselItems()[0]).toHaveAttribute("data-active", "true"); act(() => { fireEvent.click(carouselIndicators()[3]); }); - expect(carouselItems()[0]).toHaveAttribute("data-active", "false"); - expect(carouselItems()[3]).toHaveAttribute("data-active", "true"); + expect(realCarouselItems()[0]).toHaveAttribute("data-active", "false"); + expect(realCarouselItems()[3]).toHaveAttribute("data-active", "true"); expect(carouselIndicators()[0]).not.toHaveClass(activeIndicatorClasses); expect(carouselIndicators()[3]).toHaveClass(activeIndicatorClasses); }); @@ -61,14 +66,14 @@ describe("Components / Carousel", () => { render(); expect(carouselIndicators()[0]).toHaveClass(activeIndicatorClasses); - expect(carouselItems()[0]).toHaveAttribute("data-active", "true"); + expect(realCarouselItems()[0]).toHaveAttribute("data-active", "true"); act(() => { fireEvent.click(carouselRightControl()); }); - expect(carouselItems()[0]).toHaveAttribute("data-active", "false"); - expect(carouselItems()[1]).toHaveAttribute("data-active", "true"); + expect(realCarouselItems()[0]).toHaveAttribute("data-active", "false"); + expect(realCarouselItems()[1]).toHaveAttribute("data-active", "true"); expect(carouselIndicators()[0]).not.toHaveClass(activeIndicatorClasses); expect(carouselIndicators()[1]).toHaveClass(activeIndicatorClasses); }); @@ -76,8 +81,8 @@ describe("Components / Carousel", () => { it("should transition to the next item after about 3 s by default", () => { render(); - expect(carouselItems()[0]).toHaveAttribute("data-active", "true"); - expect(carouselItems()[1]).toHaveAttribute("data-active", "false"); + expect(realCarouselItems()[0]).toHaveAttribute("data-active", "true"); + expect(realCarouselItems()[1]).toHaveAttribute("data-active", "false"); expect(carouselIndicators()[0]).toHaveClass(activeIndicatorClasses); expect(carouselIndicators()[1]).toHaveClass(nonActiveIndicatorClasses); @@ -85,8 +90,8 @@ describe("Components / Carousel", () => { vi.advanceTimersByTime(3000); }); - expect(carouselItems()[0]).toHaveAttribute("data-active", "false"); - expect(carouselItems()[1]).toHaveAttribute("data-active", "true"); + expect(realCarouselItems()[0]).toHaveAttribute("data-active", "false"); + expect(realCarouselItems()[1]).toHaveAttribute("data-active", "true"); expect(carouselIndicators()[0]).toHaveClass(nonActiveIndicatorClasses); expect(carouselIndicators()[1]).toHaveClass(activeIndicatorClasses); }); @@ -94,8 +99,8 @@ describe("Components / Carousel", () => { it("should transition to the next item after `slideInterval` when it is provided", () => { render(); - expect(carouselItems()[0]).toHaveAttribute("data-active", "true"); - expect(carouselItems()[1]).toHaveAttribute("data-active", "false"); + expect(realCarouselItems()[0]).toHaveAttribute("data-active", "true"); + expect(realCarouselItems()[1]).toHaveAttribute("data-active", "false"); expect(carouselIndicators()[0]).toHaveClass(activeIndicatorClasses); expect(carouselIndicators()[1]).toHaveClass(nonActiveIndicatorClasses); @@ -103,8 +108,8 @@ describe("Components / Carousel", () => { vi.advanceTimersByTime(9000); }); - expect(carouselItems()[0]).toHaveAttribute("data-active", "false"); - expect(carouselItems()[1]).toHaveAttribute("data-active", "true"); + expect(realCarouselItems()[0]).toHaveAttribute("data-active", "false"); + expect(realCarouselItems()[1]).toHaveAttribute("data-active", "true"); expect(carouselIndicators()[0]).toHaveClass(nonActiveIndicatorClasses); expect(carouselIndicators()[1]).toHaveClass(activeIndicatorClasses); }); @@ -112,8 +117,8 @@ describe("Components / Carousel", () => { it("should not automatically transition to the next item when `slide={false}`", () => { render(); - expect(carouselItems()[0]).toHaveAttribute("data-active", "true"); - expect(carouselItems()[1]).toHaveAttribute("data-active", "false"); + expect(realCarouselItems()[0]).toHaveAttribute("data-active", "true"); + expect(realCarouselItems()[1]).toHaveAttribute("data-active", "false"); expect(carouselIndicators()[0]).toHaveClass(activeIndicatorClasses); expect(carouselIndicators()[1]).toHaveClass(nonActiveIndicatorClasses); @@ -121,8 +126,8 @@ describe("Components / Carousel", () => { vi.advanceTimersByTime(3000); }); - expect(carouselItems()[0]).toHaveAttribute("data-active", "true"); - expect(carouselItems()[1]).toHaveAttribute("data-active", "false"); + expect(realCarouselItems()[0]).toHaveAttribute("data-active", "true"); + expect(realCarouselItems()[1]).toHaveAttribute("data-active", "false"); expect(carouselIndicators()[0]).toHaveClass(activeIndicatorClasses); expect(carouselIndicators()[1]).toHaveClass(nonActiveIndicatorClasses); }); @@ -142,6 +147,11 @@ const TestCarousel = (props: CarouselProps) => ( ); const carouselItems = () => screen.getAllByTestId("carousel-item"); +// Real items exclude the prepended and appended clones +const realCarouselItems = () => { + const items = carouselItems(); + return items.slice(1, items.length - 1); +}; const carouselIndicators = () => screen.getAllByTestId("carousel-indicator"); const carouselLeftControl = () => screen.getByTestId("carousel-left-control"); const carouselRightControl = () => screen.getByTestId("carousel-right-control"); diff --git a/packages/ui/src/components/Carousel/Carousel.tsx b/packages/ui/src/components/Carousel/Carousel.tsx index a2710d5d82..19d0a3f560 100644 --- a/packages/ui/src/components/Carousel/Carousel.tsx +++ b/packages/ui/src/components/Carousel/Carousel.tsx @@ -62,6 +62,8 @@ export interface CarouselProps extends ComponentProps<"div">, ThemingProps, ThemingProps {} +const SCROLL_ANIMATION_DURATION_MS = 500; + export const Carousel = forwardRef((props, ref) => { const provider = useThemeProvider(); const theme = useResolveTheme( @@ -89,8 +91,11 @@ export const Carousel = forwardRef((props, ref) = const [activeItem, setActiveItem] = useState(0); const [isDragging, setIsDragging] = useState(false); const [isHovering, setIsHovering] = useState(false); + const [isAnimating, setIsAnimating] = useState(false); const didMountRef = useRef(false); + const initializedRef = useRef(false); + const transitionTimeoutRef = useRef | null>(null); const items = useMemo( () => @@ -102,31 +107,102 @@ export const Carousel = forwardRef((props, ref) = [children, theme.item.base], ); + const itemCount = items?.length ?? 0; + const navigateTo = useCallback( (item: number) => () => { - if (!items) return; - item = (item + items.length) % items.length; - if (carouselContainer.current) { - carouselContainer.current.scrollLeft = carouselContainer.current.clientWidth * item; + if (!items || isAnimating) return; + + const container = carouselContainer.current; + if (!container) return; + + const totalItems = items.length; + const targetItem = ((item % totalItems) + totalItems) % totalItems; + + const isWrappingForward = activeItem === totalItems - 1 && item >= totalItems; + const isWrappingBackward = activeItem === 0 && item < 0; + + if (isWrappingForward || isWrappingBackward) { + setIsAnimating(true); + + if (transitionTimeoutRef.current) { + clearTimeout(transitionTimeoutRef.current); + } + + // Scroll to the clone (last element for backward, first-after-last for forward) + if (isWrappingForward) { + container.scrollTo({ + left: container.clientWidth * (totalItems + 1), + behavior: "smooth", + }); + } else { + container.scrollTo({ + left: 0, + behavior: "smooth", + }); + } + + // After the smooth scroll animation, jump instantly to the real slide + const onTransitionDone = () => { + container.style.scrollBehavior = "auto"; + if (isWrappingForward) { + container.scrollLeft = container.clientWidth * 1; + } else { + container.scrollLeft = container.clientWidth * totalItems; + } + container.style.scrollBehavior = ""; + setIsAnimating(false); + transitionTimeoutRef.current = null; + }; + + transitionTimeoutRef.current = setTimeout(onTransitionDone, SCROLL_ANIMATION_DURATION_MS); + setActiveItem(targetItem); + } else { + // Normal navigation - account for the prepended clone + container.scrollTo({ + left: container.clientWidth * (targetItem + 1), + behavior: "smooth", + }); + setActiveItem(targetItem); } - setActiveItem(item); }, - [items], + [items, activeItem, isAnimating], ); + // Initialize scroll position to first real slide (past the prepended clone) + useEffect(() => { + const container = carouselContainer.current; + if (container && items && items.length > 0 && !initializedRef.current) { + initializedRef.current = true; + container.style.scrollBehavior = "auto"; + container.scrollLeft = container.clientWidth * 1; + container.style.scrollBehavior = ""; + } + }, [items]); + useEffect(() => { - if (carouselContainer.current && !isDragging && carouselContainer.current.scrollLeft !== 0) { - setActiveItem(Math.round(carouselContainer.current.scrollLeft / carouselContainer.current.clientWidth)); + if (carouselContainer.current && !isDragging && !isAnimating) { + const container = carouselContainer.current; + const rawIndex = Math.round(container.scrollLeft / container.clientWidth); + // Account for the prepended clone: real items start at index 1 + const totalItems = items?.length ?? 0; + if (totalItems > 0) { + const realIndex = (((rawIndex - 1) % totalItems) + totalItems) % totalItems; + setActiveItem(realIndex); + } } - }, [isDragging]); + }, [isDragging, isAnimating, items]); useEffect(() => { if (slide && !(pauseOnHover && isHovering)) { - const intervalId = setInterval(() => !isDragging && navigateTo(activeItem + 1)(), slideInterval ?? 3000); + const intervalId = setInterval( + () => !isDragging && !isAnimating && navigateTo(activeItem + 1)(), + slideInterval ?? 3000, + ); return () => clearInterval(intervalId); } - }, [activeItem, isDragging, navigateTo, slide, slideInterval, pauseOnHover, isHovering]); + }, [activeItem, isDragging, isAnimating, navigateTo, slide, slideInterval, pauseOnHover, isHovering]); useEffect(() => { if (didMountRef.current) { @@ -136,11 +212,55 @@ export const Carousel = forwardRef((props, ref) = } }, [onSlideChange, activeItem]); + // Cleanup transition timeout on unmount + useEffect(() => { + return () => { + if (transitionTimeoutRef.current) { + clearTimeout(transitionTimeoutRef.current); + } + }; + }, []); + const handleDragging = (dragging: boolean) => () => setIsDragging(dragging); + // Handle drag end: snap to nearest real slide and fix wrap-around + const handleDragEnd = useCallback(() => { + setIsDragging(false); + const container = carouselContainer.current; + if (!container || !items) return; + + const totalItems = items.length; + const rawIndex = Math.round(container.scrollLeft / container.clientWidth); + + // If scrolled to the prepended clone (index 0), jump to real last slide + if (rawIndex <= 0) { + container.style.scrollBehavior = "auto"; + container.scrollLeft = container.clientWidth * totalItems; + container.style.scrollBehavior = ""; + setActiveItem(totalItems - 1); + } + // If scrolled to the appended clone (index totalItems + 1), jump to real first slide + else if (rawIndex >= totalItems + 1) { + container.style.scrollBehavior = "auto"; + container.scrollLeft = container.clientWidth * 1; + container.style.scrollBehavior = ""; + setActiveItem(0); + } else { + setActiveItem(rawIndex - 1); + } + }, [items]); + const setHoveringTrue = useCallback(() => setIsHovering(true), []); const setHoveringFalse = useCallback(() => setIsHovering(false), []); + // Build the extended items array: [cloneLast, ...items, cloneFirst] + const extendedItems = useMemo(() => { + if (!items || items.length === 0) return items; + const lastClone = cloneElement(items[items.length - 1], { key: "clone-last" }); + const firstClone = cloneElement(items[0], { key: "clone-first" }); + return [lastClone, ...items, firstClone]; + }, [items]); + return (
((props, ref) = className={twMerge(theme.scrollContainer.base, (isDeviceMobile || !isDragging) && theme.scrollContainer.snap)} draggingClassName="cursor-grab" innerRef={carouselContainer} - onEndScroll={handleDragging(false)} + onEndScroll={handleDragEnd} onStartScroll={handleDragging(draggable)} vertical={false} horizontal={draggable} > - {items?.map((item, index) => ( + {extendedItems?.map((item, index) => (
{item}