Fix carousel not looping if dragging past akaalroop.com#64
Fix carousel not looping if dragging past akaalroop.com#64Spacexplorer11 merged 2 commits intomainfrom
Conversation
Signed-off-by: Akaalroop Singh <akaal@akaalroop.com>
WalkthroughThe carousel's pointer-drag handling now computes a singleSetWidth (half the scrollWidth), updates scrollPosition by the drag delta, and conditionally wraps the position by subtracting/adding singleSetWidth when crossing positive/negative boundaries. An animation guard prevents starting a new auto-scroll if an animation is already running. Changes
Sequence Diagram(s)(omitted — changes are confined to a single component's internal flow and do not introduce multi-component interactions requiring a sequence diagram) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 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 |
Deploying akaalroop-com with
|
| Latest commit: |
47d7848
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://2f3fd8c2.akaalroop-com.pages.dev |
| Branch Preview URL: | https://carousel-issue-spa-112.akaalroop-com.pages.dev |
There was a problem hiding this comment.
Pull request overview
This PR updates the home page project carousel drag behavior to keep the “infinite” loop working when the user drags past the duplicated-content boundary.
Changes:
- Compute a
singleSetWidthduring drag and adjustscrollPositionwhen it exceeds the loop bounds. - Apply the transform update after the boundary adjustment so the carousel stays visually continuous.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/routes/+page.svelte (1)
57-75:⚠️ Potential issue | 🟠 MajorPossible duplicate
requestAnimationFrameloops on repeatedshowCarouseltoggles.
$effecton line 110 callsstartSmoothScroll()every timeshowCarouselbecomes true, andstartSmoothScrollunconditionally starts a newrequestAnimationFrameloop. IfshowCarouseltoggles more than once (or the effect re-runs), multiple independent animation loops will stack, each decrementingscrollPositionand overwriting the transform — leading to progressively faster scrolling.Consider guarding against starting a second loop when one is already running.
Proposed guard
function startSmoothScroll() { + if (animationFrameId) return; // already running const animate = () => {
🤖 Fix all issues with AI agents
In `@src/routes/`+page.svelte:
- Around line 87-103: handlePointerMove lacks the null guard present in
startSmoothScroll and can throw when carouselContainer is not bound; add an
early return if carouselContainer is falsy at the top of handlePointerMove. Also
replace the single-step wrap logic that adjusts scrollPosition using +/-
singleSetWidth with a robust modulo-based wrap so scrollPosition is normalized
into the expected range (relative to singleSetWidth) even if delta exceeds one
cycle; use the same variables referenced here (startX, scrollStart,
scrollPosition, singleSetWidth) to compute the wrapped translateX value before
applying carouselContainer.style.transform.
Signed-off-by: Akaalroop Singh <akaal@akaalroop.com>
|
@coderabbitai do you approve? |
|
✅ Actions performedComments resolved and changes approved. |
|
Fixes #63 |
Summary by CodeRabbit