fix: reset progress bar on mobile scroll and pause on coming soon items#2036
Conversation
- Add handleFeatureIndexChange in MainFeaturesSection to reset progress and pause on coming soon items - Add handleDetailIndexChange in DetailsSection with same behavior - Add handleTabIndexChange in FloatingPanelContent to reset progress on scroll - Update mobile carousels to use new handlers with index change check Co-Authored-By: john@hyprnote.com <john@hyprnote.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
✅ Deploy Preview for hyprnote ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for hyprnote-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughIntroduces a pause-aware auto-advance mechanism for carousel components by replacing setter callbacks with onIndexChange handlers. When a feature/detail marked as comingSoon is active, auto-advance timer pauses. Parent components now handle index changes centrally, resetting progress and managing pause state based on the selected item's status. Changes
Sequence DiagramsequenceDiagram
actor User
participant Parent as MainFeaturesSection
participant Mobile as FeaturesMobileCarousel
participant Progress as Auto-advance Loop
User->>Mobile: Scroll/Select Item
Mobile->>Mobile: Compute new index
alt Index changed
Mobile->>Parent: onIndexChange(newIndex)
Parent->>Parent: resetProgress()
Parent->>Parent: Check item.comingSoon
alt Item is coming soon
Parent->>Parent: setPaused(true)
else Item is available
Parent->>Parent: setPaused(false)
end
end
loop Progress animation
Progress->>Parent: Check isPaused
alt isPaused = false
Progress->>Progress: Increment progress
Note over Progress: Normal auto-advance
else isPaused = true
Progress->>Progress: Skip frame
Note over Progress: Hold at current index
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/routes/_view/index.tsx (1)
1208-1216: Critical race condition between programmatic scroll and onScroll handler.The
onScrollhandler fires continuously during smooth scroll animations. When programmatic scrolling occurs (from clicking dots at line 1256 or auto-advance at lines 1135-1139), the following race condition can occur:
setSelectedFeature(targetIndex)is called and smooth scroll begins- During animation,
scrollLeftis at an intermediate positiononScrollfires, computes intermediateindexfromscrollLeft- Calls
onIndexChange(intermediateIndex), which resetsselectedFeature- This interrupts the scroll, causing the carousel to jump to the wrong index
The same issue exists in DetailsSection at lines 1503-1511.
Suggested fixes:
- Use the
scrollendevent instead ofscroll(cleanest, but check browser support):-onScroll={(e) => { +onScrollEnd={(e) => {
- Track programmatic scrolls with a ref and ignore
onScrollduring those:const isProgrammaticScroll = useRef(false); // In scrollToFeature and auto-advance: isProgrammaticScroll.current = true; container.scrollTo({ left: scrollLeft, behavior: "smooth" }); setTimeout(() => isProgrammaticScroll.current = false, 500); // In onScroll: if (isProgrammaticScroll.current) return;
- Debounce the
onScrollhandler to only fire after scrolling settles.
🧹 Nitpick comments (2)
apps/web/src/routes/_view/product/ai-notetaking.tsx (1)
2218-2222: Centralized tab-index change handler correctly resets progressWiring
handleTabIndexChangethroughonIndexChangeso that every index change also zeroesprogressandprogressRefsolves the “stale progress after manual change” problem while keeping the auto‑advance effect logic unchanged. Looks solid.If you find more callers that need the same reset semantics (
scrollToTab,handleTabClick), you could optionally factor the reset logic into a tiny helper (e.g.resetForIndex(index)) to keep those invariants DRY, but not required for this PR.Also applies to: 2281-2284
apps/web/src/routes/_view/index.tsx (1)
1098-1108: Optional: Consider extracting duplicated pause state logic.The logic to set
isPausedbased on thecomingSoonflag is duplicated across multiple handlers. While the duplication is minor (2 lines), you could extract it to reduce repetition:const getIsPaused = (index: number, features: typeof mainFeatures) => { return !!features[index]?.comingSoon; }; // Then in handlers: setIsPaused(getIsPaused(nextIndex, mainFeatures));However, given the simplicity, the current inline approach is also acceptable.
Also applies to: 1149-1155, 1384-1394, 1434-1440, 1442-1448
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/src/routes/_view/index.tsx(10 hunks)apps/web/src/routes/_view/product/ai-notetaking.tsx(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
If there are many classNames with conditional logic, usecn(import from@hypr/utils). It is similar toclsx. Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/web/src/routes/_view/product/ai-notetaking.tsxapps/web/src/routes/_view/index.tsx
🧠 Learnings (1)
📚 Learning: 2025-11-24T16:32:23.055Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:32:23.055Z
Learning: Applies to **/*.{ts,tsx} : Use `motion/react` instead of `framer-motion`.
Applied to files:
apps/web/src/routes/_view/product/ai-notetaking.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
- GitHub Check: ci
- GitHub Check: fmt
- GitHub Check: Devin
🔇 Additional comments (5)
apps/web/src/routes/_view/product/ai-notetaking.tsx (2)
10-10: React import update matches new hook usageAdding
useCallbackhere is correct and used below inhandleTabIndexChange; no issues.
2455-2464: Mobile scroll handler now keeps selectedTab and progress in syncPassing
onIndexChangeand calling it only when the computedindexdiffers fromselectedTabensures that horizontal swipes update the parent state and reset the auto‑advance progress without spamming updates. TheoffsetWidth‑based index calculation is reasonable for full‑width slides.Please sanity‑check this on real devices (especially with momentum scrolling) to confirm we don’t hit any off‑by‑one behavior when a slide is partially visible.
Also applies to: 2475-2477
apps/web/src/routes/_view/index.tsx (3)
1095-1108: LGTM: Pause-aware index change handler is well-structured.The new
handleFeatureIndexChangecallback properly centralizes the logic for index changes by resetting progress and managing pause state based on thecomingSoonflag. TheuseCallbackdependencies are correct.
1110-1147: Auto-advance logic correctly pauses for "coming soon" items.The early return when
isPausedis true properly halts the animation. UsingactiveFeatureIndicesensures auto-advance only navigates between non-comingSoon items, so not updatingisPausedduring auto-advance is correct.Note: Auto-advance is also affected by the race condition flagged in the previous comment regarding programmatic scrolling.
1384-1394: DetailsSection follows the same pattern as MainFeaturesSection.The implementation is consistent with
MainFeaturesSection, properly centralizing index changes throughhandleDetailIndexChangewith pause state management.Note: The same race condition between programmatic scroll and
onScrollhandler applies here (lines 1508-1510), as already flagged for the features carousel.Also applies to: 1508-1510
ComputelessComputer
deleted the
devin/1764563046-fix-mobile-progress-reset
branch
1 participant
fix: reset progress bar on mobile scroll and pause on coming soon items
Summary
Fixes mobile carousel progress bar behavior in the landing page features/details sections and the ai-notetaking floating panel section:
Progress resets when scrolling to other cards - Previously, scrolling to a different card would continue the progress from where it was. Now it resets to 0.
Progress pauses on "coming soon" items - When scrolling to a card marked as
comingSoon, the auto-advance timer pauses until the user scrolls back to a non-coming-soon card.Changes made:
handleFeatureIndexChange/handleDetailIndexChange/handleTabIndexChangecallbacks that reset progress and manage pause stateonScrollhandlers to only trigger when the index actually changes (prevents excessive calls during scroll)isPausedstate toMainFeaturesSection(DetailsSection already had it for tablet hover)Review & Testing Checklist for Human
/) and verify progress resets when changing cards/product/ai-notetakingand test the floating panel mobile carouselNotes
isPausedstate for both mobile scroll pause and tablet hover pause - these shouldn't conflict but worth verifying.Link to Devin run: https://app.devin.ai/sessions/dc377cd439c14488bea6ade1d5604a64
Requested by: john@hyprnote.com (@ComputelessComputer)