Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions .changeset/carousel-circular-transition.md
Original file line number Diff line number Diff line change
@@ -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
148 changes: 134 additions & 14 deletions packages/ui/src/components/Carousel/Carousel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@

export interface DefaultLeftRightControlProps extends ComponentProps<"div">, ThemingProps<CarouselTheme> {}

const SCROLL_ANIMATION_DURATION_MS = 500;

export const Carousel = forwardRef<HTMLDivElement, CarouselProps>((props, ref) => {
const provider = useThemeProvider();
const theme = useResolveTheme(
Expand Down Expand Up @@ -89,8 +91,11 @@
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<ReturnType<typeof setTimeout> | null>(null);

const items = useMemo(
() =>
Expand All @@ -102,31 +107,102 @@
[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",
});
}
Comment on lines +134 to +143
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Tests fail because JSDOM lacks scrollTo implementation.

The static analysis shows multiple test failures with TypeError: container.scrollTo is not a function. JSDOM (used by Vitest/Jest) doesn't implement scrollTo on elements by default.

Add a mock in your test setup or at the top of the test file:

🧪 Proposed fix: Mock scrollTo in tests

Add to your test setup file or before tests:

beforeAll(() => {
  Element.prototype.scrollTo = vi.fn();
});

Or if you need to verify scroll positions:

beforeEach(() => {
  Element.prototype.scrollTo = vi.fn(function(this: Element, options?: ScrollToOptions) {
    if (options?.left !== undefined) {
      (this as any).scrollLeft = options.left;
    }
  });
});

Also applies to: 160-166

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/ui/src/components/Carousel/Carousel.tsx` around lines 134 - 143,
Tests fail because JSDOM doesn't implement element.scrollTo; update the test
setup (or top of the Carousel tests) to mock scrollTo so calls like
container.scrollTo(...) in Carousel (the logic that sets left to
container.clientWidth * (totalItems + 1) or 0) won't throw; add a beforeAll or
beforeEach that sets Element.prototype.scrollTo = vi.fn() and optionally
implement the mock to set this.scrollLeft when options.left is provided so
assertions about scroll position still work.


// 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);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
} else {
// Normal navigation - account for the prepended clone
container.scrollTo({

Check failure on line 162 in packages/ui/src/components/Carousel/Carousel.tsx

View workflow job for this annotation

GitHub Actions / 🔬 Test

components/Carousel/Carousel.test.tsx > Components / Carousel > should change slide via click on control

TypeError: container.scrollTo is not a function ❯ components/Carousel/Carousel.tsx:162:19 ❯ components/Carousel/Carousel.tsx:199:71 ❯ components/Carousel/Carousel.test.tsx:14:6

Check failure on line 162 in packages/ui/src/components/Carousel/Carousel.tsx

View workflow job for this annotation

GitHub Actions / 🔬 Test

components/Carousel/Carousel.test.tsx > Components / Carousel > should render custom controls

TypeError: container.scrollTo is not a function ❯ components/Carousel/Carousel.tsx:162:19 ❯ components/Carousel/Carousel.tsx:199:71 ❯ components/Carousel/Carousel.test.tsx:14:6

Check failure on line 162 in packages/ui/src/components/Carousel/Carousel.tsx

View workflow job for this annotation

GitHub Actions / 🔬 Test

components/Carousel/Carousel.test.tsx > Components / Carousel > should change slide via click on indicator

TypeError: container.scrollTo is not a function ❯ components/Carousel/Carousel.tsx:162:19 ❯ components/Carousel/Carousel.tsx:199:71 ❯ components/Carousel/Carousel.test.tsx:14:6

Check failure on line 162 in packages/ui/src/components/Carousel/Carousel.tsx

View workflow job for this annotation

GitHub Actions / 🔬 Test

components/Carousel/Carousel.test.tsx > Components / Carousel > should render without indicators

TypeError: container.scrollTo is not a function ❯ components/Carousel/Carousel.tsx:162:19 ❯ components/Carousel/Carousel.tsx:199:71 ❯ components/Carousel/Carousel.test.tsx:14:6

Check failure on line 162 in packages/ui/src/components/Carousel/Carousel.tsx

View workflow job for this annotation

GitHub Actions / 🔬 Test

components/Carousel/Carousel.test.tsx > Components / Carousel > should render and show first item

TypeError: container.scrollTo is not a function ❯ components/Carousel/Carousel.tsx:162:19 ❯ components/Carousel/Carousel.tsx:199:71 ❯ components/Carousel/Carousel.test.tsx:14:6
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) {
Expand All @@ -136,11 +212,55 @@
}
}, [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 (
<div
ref={ref}
Expand All @@ -156,16 +276,16 @@
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) => (
<div
key={index}
className={theme.item.wrapper[draggable ? "on" : "off"]}
data-active={activeItem === index}
data-active={activeItem === (index - 1 + itemCount) % itemCount}
data-testid="carousel-item"
>
{item}
Expand Down
Loading