Skip to content
Open
Changes from 1 commit
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
129 changes: 115 additions & 14 deletions packages/ui/src/components/Carousel/Carousel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@
const [activeItem, setActiveItem] = useState(0);
const [isDragging, setIsDragging] = useState(false);
const [isHovering, setIsHovering] = useState(false);
const [isAnimating, setIsAnimating] = useState(false);

const didMountRef = useRef(false);

Expand All @@ -102,31 +103,96 @@
[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);

// Scroll to the clone (last element for backward, first-after-last for forward)
if (isWrappingForward) {
// Clone of first slide is at the end (index = totalItems + 1 in the extended list)
// But we use scrollLeft directly
container.scrollTo({
left: container.clientWidth * (totalItems + 1),
behavior: "smooth",
});
} else {
// Clone of last slide is at position 0
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; // real first slide at index 1
} else {
container.scrollLeft = container.clientWidth * totalItems; // real last slide
}
container.style.scrollBehavior = "";
setIsAnimating(false);
};

setTimeout(onTransitionDone, 500);
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 156 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:156:19 ❯ components/Carousel/Carousel.tsx:191:100 ❯ components/Carousel/Carousel.test.tsx:14:6

Check failure on line 156 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:156:19 ❯ components/Carousel/Carousel.tsx:191:100 ❯ components/Carousel/Carousel.test.tsx:14:6

Check failure on line 156 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:156:19 ❯ components/Carousel/Carousel.tsx:191:100 ❯ components/Carousel/Carousel.test.tsx:14:6

Check failure on line 156 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:156:19 ❯ components/Carousel/Carousel.tsx:191:100 ❯ components/Carousel/Carousel.test.tsx:14:6

Check failure on line 156 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:156:19 ❯ components/Carousel/Carousel.tsx:191:100 ❯ 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(() => {
if (carouselContainer.current && !isDragging && carouselContainer.current.scrollLeft !== 0) {
setActiveItem(Math.round(carouselContainer.current.scrollLeft / carouselContainer.current.clientWidth));
const container = carouselContainer.current;
if (container && items && items.length > 0) {
container.style.scrollBehavior = "auto";
container.scrollLeft = container.clientWidth * 1;
container.style.scrollBehavior = "";
}
}, [isDragging]);
}, [items]);

useEffect(() => {
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, 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 @@ -138,9 +204,44 @@

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 +257,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