feat(carousel): add swipe deck with snap, indicators, and parallax#432
Open
leazi99 wants to merge 2 commits into
Open
feat(carousel): add swipe deck with snap, indicators, and parallax#432leazi99 wants to merge 2 commits into
leazi99 wants to merge 2 commits into
Conversation
leazi99
had a problem deploying
to
preview-deployment
GitHub Actions
Failure
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a new SwipeDeck React component with snap-aligned, horizontally scrollable cards, viewport-center active-index tracking, parallax transforms, pointer-drag scrolling, optional nav arrows and dots, plus a Storybook story and MDX documentation. (49 words) Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@animata/carousel/swipe-deck.tsx`:
- Around line 254-280: dragState.current.moved is set in the pointer move
handler but never read, causing clicks to always trigger
scrollToIndex(nearestIndex(...)) on pointer release; update onPointerUp and
onPointerCancel to check dragState.current.moved and only call
scrollToIndex(nearestIndex(container)) when moved is true (suppress snapping on
pure clicks), and reset dragState.current.moved appropriately when
starting/ending a pointer interaction (or remove the flag if you prefer not to
change behavior). Ensure the checks reference dragState.current.moved, the
handlers onPointerUp/onPointerCancel, and the scrollToIndex(nearestIndex(...))
invocation.
- Around line 310-325: The indicator buttons don't expose which item is active
to assistive tech; update the buttons generated in the items.map inside the
showIndicators/hasMultipleCards block to include aria-current on the active dot
(use activeIndex to determine the current button) so screen readers announce the
active card; keep existing onClick/aria-label behavior and className logic
(references: items.map, scrollToIndex, activeIndex, cn).
- Around line 220-260: The touch swipe is disabled because the element forces
vertical-only touch handling (the "touch-pan-y" class and style touchAction:
"pan-y pinch-zoom") and the onPointerDown handler ignores non-mouse pointers; to
fix, remove or change the touchAction/class that prevents horizontal pans
(remove "touch-pan-y" and/or change touchAction to allow horizontal panning,
e.g. include "pan-x" or just omit) and update the onPointerDown logic in the
pointer handler to not early-return for touch/pen (allow pointerId capture and
dragState initialization for event.pointerType === "touch" or all pointer types
while keeping the existing mouse behavior), referencing the component DOM ref
deckRef, the dragState.current fields, isDragging state and the onPointerMove
container scroll logic so touch devices can either use native horizontal scroll
or the JS drag fallback.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 596402f1-d4a2-416a-91d1-0c779256b900
📒 Files selected for processing (3)
animata/carousel/swipe-deck.stories.tsxanimata/carousel/swipe-deck.tsxcontent/docs/carousel/swipe-deck.mdx
Comment on lines
+254
to
+280
| const deltaX = event.clientX - dragState.current.startX; | ||
| if (Math.abs(deltaX) > 3) { | ||
| dragState.current.moved = true; | ||
| } | ||
|
|
||
| container.scrollLeft = dragState.current.startScrollLeft - deltaX; | ||
| }} | ||
| onPointerUp={(event) => { | ||
| const container = deckRef.current; | ||
| if (!container || dragState.current.pointerId !== event.pointerId) { | ||
| return; | ||
| } | ||
|
|
||
| container.releasePointerCapture(event.pointerId); | ||
| setIsDragging(false); | ||
| scrollToIndex(nearestIndex(container)); | ||
| }} | ||
| onPointerCancel={(event) => { | ||
| const container = deckRef.current; | ||
| if (!container || dragState.current.pointerId !== event.pointerId) { | ||
| return; | ||
| } | ||
|
|
||
| container.releasePointerCapture(event.pointerId); | ||
| setIsDragging(false); | ||
| scrollToIndex(nearestIndex(container)); | ||
| }} |
There was a problem hiding this comment.
dragState.current.moved is set but never read.
Line 256 tracks whether the pointer moved past a 3px threshold, but neither onPointerUp nor onPointerCancel consult it. Consequently, a plain mouse click anywhere in the deck always triggers a smooth scrollToIndex(nearestIndex(...)) on release, which can produce an unexpected micro-scroll even when the user didn't intend to drag. Either use the flag to suppress the snap on pure clicks, or drop it to remove dead state.
♻️ Proposed fix
onPointerUp={(event) => {
const container = deckRef.current;
if (!container || dragState.current.pointerId !== event.pointerId) {
return;
}
container.releasePointerCapture(event.pointerId);
setIsDragging(false);
- scrollToIndex(nearestIndex(container));
+ if (dragState.current.moved) {
+ scrollToIndex(nearestIndex(container));
+ }
+ dragState.current.pointerId = -1;
}}
onPointerCancel={(event) => {
const container = deckRef.current;
if (!container || dragState.current.pointerId !== event.pointerId) {
return;
}
container.releasePointerCapture(event.pointerId);
setIsDragging(false);
- scrollToIndex(nearestIndex(container));
+ if (dragState.current.moved) {
+ scrollToIndex(nearestIndex(container));
+ }
+ dragState.current.pointerId = -1;
}}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const deltaX = event.clientX - dragState.current.startX; | |
| if (Math.abs(deltaX) > 3) { | |
| dragState.current.moved = true; | |
| } | |
| container.scrollLeft = dragState.current.startScrollLeft - deltaX; | |
| }} | |
| onPointerUp={(event) => { | |
| const container = deckRef.current; | |
| if (!container || dragState.current.pointerId !== event.pointerId) { | |
| return; | |
| } | |
| container.releasePointerCapture(event.pointerId); | |
| setIsDragging(false); | |
| scrollToIndex(nearestIndex(container)); | |
| }} | |
| onPointerCancel={(event) => { | |
| const container = deckRef.current; | |
| if (!container || dragState.current.pointerId !== event.pointerId) { | |
| return; | |
| } | |
| container.releasePointerCapture(event.pointerId); | |
| setIsDragging(false); | |
| scrollToIndex(nearestIndex(container)); | |
| }} | |
| const deltaX = event.clientX - dragState.current.startX; | |
| if (Math.abs(deltaX) > 3) { | |
| dragState.current.moved = true; | |
| } | |
| container.scrollLeft = dragState.current.startScrollLeft - deltaX; | |
| }} | |
| onPointerUp={(event) => { | |
| const container = deckRef.current; | |
| if (!container || dragState.current.pointerId !== event.pointerId) { | |
| return; | |
| } | |
| container.releasePointerCapture(event.pointerId); | |
| setIsDragging(false); | |
| if (dragState.current.moved) { | |
| scrollToIndex(nearestIndex(container)); | |
| } | |
| dragState.current.pointerId = -1; | |
| }} | |
| onPointerCancel={(event) => { | |
| const container = deckRef.current; | |
| if (!container || dragState.current.pointerId !== event.pointerId) { | |
| return; | |
| } | |
| container.releasePointerCapture(event.pointerId); | |
| setIsDragging(false); | |
| if (dragState.current.moved) { | |
| scrollToIndex(nearestIndex(container)); | |
| } | |
| dragState.current.pointerId = -1; | |
| }} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@animata/carousel/swipe-deck.tsx` around lines 254 - 280,
dragState.current.moved is set in the pointer move handler but never read,
causing clicks to always trigger scrollToIndex(nearestIndex(...)) on pointer
release; update onPointerUp and onPointerCancel to check dragState.current.moved
and only call scrollToIndex(nearestIndex(container)) when moved is true
(suppress snapping on pure clicks), and reset dragState.current.moved
appropriately when starting/ending a pointer interaction (or remove the flag if
you prefer not to change behavior). Ensure the checks reference
dragState.current.moved, the handlers onPointerUp/onPointerCancel, and the
scrollToIndex(nearestIndex(...)) invocation.
Comment on lines
+310
to
+325
| {showIndicators && hasMultipleCards && ( | ||
| <div className="flex items-center justify-center gap-2"> | ||
| {items.map((item, index) => ( | ||
| <button | ||
| key={item.id} | ||
| type="button" | ||
| onClick={() => scrollToIndex(index)} | ||
| aria-label={`Go to card ${index + 1}`} | ||
| className={cn( | ||
| "h-2.5 rounded-full transition-all duration-300", | ||
| activeIndex === index ? "w-8 bg-teal-600" : "w-2.5 bg-stone-300 hover:bg-stone-400", | ||
| )} | ||
| /> | ||
| ))} | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
Expose active indicator to assistive tech with aria-current.
The dot buttons only differ visually for the active card (width + color). Screen reader users get no indication which card is current. Adding aria-current makes the active state announceable.
♿ Proposed fix
{items.map((item, index) => (
<button
key={item.id}
type="button"
onClick={() => scrollToIndex(index)}
aria-label={`Go to card ${index + 1}`}
+ aria-current={activeIndex === index ? "true" : undefined}
className={cn(
"h-2.5 rounded-full transition-all duration-300",
activeIndex === index ? "w-8 bg-teal-600" : "w-2.5 bg-stone-300 hover:bg-stone-400",
)}
/>
))}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| {showIndicators && hasMultipleCards && ( | |
| <div className="flex items-center justify-center gap-2"> | |
| {items.map((item, index) => ( | |
| <button | |
| key={item.id} | |
| type="button" | |
| onClick={() => scrollToIndex(index)} | |
| aria-label={`Go to card ${index + 1}`} | |
| className={cn( | |
| "h-2.5 rounded-full transition-all duration-300", | |
| activeIndex === index ? "w-8 bg-teal-600" : "w-2.5 bg-stone-300 hover:bg-stone-400", | |
| )} | |
| /> | |
| ))} | |
| </div> | |
| )} | |
| {showIndicators && hasMultipleCards && ( | |
| <div className="flex items-center justify-center gap-2"> | |
| {items.map((item, index) => ( | |
| <button | |
| key={item.id} | |
| type="button" | |
| onClick={() => scrollToIndex(index)} | |
| aria-label={`Go to card ${index + 1}`} | |
| aria-current={activeIndex === index ? "true" : undefined} | |
| className={cn( | |
| "h-2.5 rounded-full transition-all duration-300", | |
| activeIndex === index ? "w-8 bg-teal-600" : "w-2.5 bg-stone-300 hover:bg-stone-400", | |
| )} | |
| /> | |
| ))} | |
| </div> | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@animata/carousel/swipe-deck.tsx` around lines 310 - 325, The indicator
buttons don't expose which item is active to assistive tech; update the buttons
generated in the items.map inside the showIndicators/hasMultipleCards block to
include aria-current on the active dot (use activeIndex to determine the current
button) so screen readers announce the active card; keep existing
onClick/aria-label behavior and className logic (references: items.map,
scrollToIndex, activeIndex, cn).
There was a problem hiding this comment.
Pull request overview
Adds a new “Swipe Deck” carousel component to the animata/carousel collection, along with Storybook coverage and a published docs page for the carousel documentation section.
Changes:
- Introduces
SwipeDeckcomponent with snap scrolling, arrow controls, dot indicators, and parallax-on-scroll. - Adds a Storybook story showcasing multiple usage examples/items.
- Adds a docs MDX page and marks it as published.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
content/docs/carousel/swipe-deck.mdx |
New docs page for Swipe Deck (installation + preview + credits). |
animata/carousel/swipe-deck.tsx |
Implements the Swipe Deck carousel component (scroll snap + drag + parallax + controls). |
animata/carousel/swipe-deck.stories.tsx |
Adds Storybook story for the new component with sample items. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| )} | ||
| style={{ touchAction: "pan-y pinch-zoom" }} | ||
| onPointerDown={(event) => { | ||
| if (event.pointerType !== "mouse") { |
Comment on lines
+254
to
+257
| const deltaX = event.clientX - dragState.current.startX; | ||
| if (Math.abs(deltaX) > 3) { | ||
| dragState.current.moved = true; | ||
| } |
|
|
||
| Open the newly created file and paste the following code: | ||
|
|
||
| ```tsx file=<rootDir>/animata/carousel/swipe-deck.tsx |
leazi99
had a problem deploying
to
preview-deployment
GitHub Actions
Failure
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary\n- add a new Swipe Deck carousel component with touch and mouse swipe support\n- implement scroll snap behavior, arrow controls, dot indicators, and swipe parallax\n- add Storybook story for onboarding, featured content, and recommendation card examples\n- add docs page and publish it in carousel docs\n\n## Testing\n- yarn biome check animata/carousel/swipe-deck.tsx animata/carousel/swipe-deck.stories.tsx\n
Summary by CodeRabbit
New Features
Documentation