feat(home): scroll gravity toward completed acts#208
Conversation
Settle-assist for the pinned story (owner: transitions and completes felt equal). When scrolling rests within a tunable catch radius (0.05 progress) of an act's hold plateau (0.40 / 0.64 / 0.95), the last stretch eases to the plateau over 550ms. scrollend where supported, debounced scroll as fallback; any wheel/touch/key input cancels instantly; dead zone prevents re-trigger loops; reduced motion disables it. Verified empirically in Chromium: 0.37→0.400, 0.67→0.640, mid-transition 0.52 stays.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds an act-gravity module with hold snapping and settle behavior, wires it into the pinned scroll story, and expands tests for the hook and motion mock. ChangesScroll Gravity Feature
Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
kilbot has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
🚀 Preview: https://wcpos-k1kopu0b9-wcpos.vercel.app |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 767b15e137
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
kilbot has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Fixed the remaining actionable review threads in commit
Skipped threads: none. Verification:
Post-push unresolved inline thread count before resolving: 2. |
|
🚀 Preview: https://wcpos-a3zsgyzaa-wcpos.vercel.app |
|
Review-thread follow-up for current head
Skipped threads: none; fresh unresolved inline review-thread inventory returned 0 rows. CI / validation status:
Remaining unresolved inline review threads after final GraphQL inventory: 0. |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/components/home/scroll-story/act-gravity.test.ts (2)
72-94: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftOnly cancellation is tested; the actual settle mechanics are not.
The single
useActGravitytest verifies debounce cancellation on user input, but nothing here exercises the "pull toward plateau" behavior itself (hold selection →animate/window.scrollToinvocation), thescrollend-available path, or theprefers-reduced-motionearly-return. Given this is the PR's core new behavior, consider adding tests that stubwindow.scrollTo/Element.prototype.getBoundingClientRectand assert a settle is triggered when scrolling stops near a hold (and skipped when reduced-motion is set).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/home/scroll-story/act-gravity.test.ts` around lines 72 - 94, The current useActGravity coverage only checks debounce cancellation, so add tests that exercise the actual settle behavior in GravityHarness/useActGravity by stubbing window.scrollTo and Element.prototype.getBoundingClientRect and asserting a hold selection triggers the “pull toward plateau” path when scrolling stops; also add coverage for the scrollend-supported branch and the prefers-reduced-motion early return so the main behavior and skip paths are both verified.
30-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDead-zone test relies on an unexported magic number.
DEAD_ZONE(0.004) isn't exported fromact-gravity.ts, so line 33'shold + 0.002is a magic number that silently decouples from the real constant if it ever changes.♻️ Suggested fix
import { ACT_HOLDS, CATCH_RADIUS, + DEAD_ZONE, nearestHold, useActGravity, } from './act-gravity'- expect(nearestHold(hold + 0.002)).toBeNull() + expect(nearestHold(hold + DEAD_ZONE / 2)).toBeNull()(Requires exporting
DEAD_ZONEfromact-gravity.ts.)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/home/scroll-story/act-gravity.test.ts` around lines 30 - 35, The dead-zone test is hardcoding a value that should stay in sync with the source constant. Export DEAD_ZONE from act-gravity.ts, then update act-gravity.test.ts to reference that symbol instead of using the unexported magic number in the plateau assertion. Keep the test aligned with nearestHold and ACT_HOLDS so future changes to the dead zone only need one update.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/components/home/scroll-story/act-gravity.test.ts`:
- Around line 72-94: The current useActGravity coverage only checks debounce
cancellation, so add tests that exercise the actual settle behavior in
GravityHarness/useActGravity by stubbing window.scrollTo and
Element.prototype.getBoundingClientRect and asserting a hold selection triggers
the “pull toward plateau” path when scrolling stops; also add coverage for the
scrollend-supported branch and the prefers-reduced-motion early return so the
main behavior and skip paths are both verified.
- Around line 30-35: The dead-zone test is hardcoding a value that should stay
in sync with the source constant. Export DEAD_ZONE from act-gravity.ts, then
update act-gravity.test.ts to reference that symbol instead of using the
unexported magic number in the plateau assertion. Keep the test aligned with
nearestHold and ACT_HOLDS so future changes to the dead zone only need one
update.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 6c48e6b4-20dc-485c-a3d7-33f571f18a85
📒 Files selected for processing (4)
src/components/home/scroll-story/act-gravity.test.tssrc/components/home/scroll-story/act-gravity.tssrc/components/home/scroll-story/scroll-story.test.tsxsrc/components/home/scroll-story/scroll-story.tsx
There was a problem hiding this comment.
kilbot has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
🚀 Preview: https://wcpos-1zhqqdxzv-wcpos.vercel.app |
|
Fixed the current actionable CodeRabbit review-body feedback in commit
Skipped threads: none; fresh unresolved inline review-thread inventory returned 0 rows. Validation:
Post-push status: merge state is clean, remote Test/E2E/CodeQL/Deploy Preview checks are passing, CodeRabbit status is success, and remaining unresolved inline review threads after final GraphQL inventory: 0. |
Owner ask: completed acts should have more "weight" than transitions when scrolling the story. This is the scrollytelling settle-assist pattern (GSAP ScrollTrigger's
snap; CSS proximity snapping is the platform cousin but its radius isn't tunable) — hand-rolled onmotion'sanimate, no new dependency.Behaviour
0.40,0.64,0.95— act 1 is the top of the page), the remaining distance eases there over 550ms with a soft curvescrollendwhere supported with a debounced-scroll fallback; disabled underprefers-reduced-motionEvidence
Real-Chromium probe: rest at 0.37 → settles 0.400; rest at 0.67 → settles 0.640; rest at 0.52 (transition) → stays 0.520. Unit tests cover the catch/dead-zone/nearest-hold maths. Full suite green, lint clean.
Note:
ACT_HOLDS/CATCH_RADIUSinact-gravity.tsare the tuning knobs; if #204 (shader) merges first this rebases trivially (disjoint regions of scroll-story.tsx).Summary by CodeRabbit
New Features
Bug Fixes
Tests