fix: revert string offset ViewTimeline acceleration#3659
Conversation
String offsets (e.g. ['start start', 'end end']) passed to useScroll's offset option should not be accelerated via ViewTimeline. The string-to- ProgressIntersection parsing added in 6bae74e causes incorrect animation behaviour when the target element uses position: sticky inside a transformed parent, because the ViewTimeline 'contain' range does not produce identical results to JS-based scroll tracking in that DOM structure. This removes parseStringOffset() and the string branch from normaliseOffset(), restoring pre-12.37.0 behaviour where string offsets always fall back to JS main-thread scroll tracking. Array-based ProgressIntersection offsets (the original preset format) continue to be accelerated as before. Fixes motiondivision#3658
There was a problem hiding this comment.
Pull request overview
This PR restores pre-v12.37.0 behavior for useScroll string-based offsets by preventing them from mapping to ViewTimeline named ranges, ensuring string offsets fall back to JS-based scroll tracking (avoiding the "contain" ViewTimeline regression described in #3658).
Changes:
- Remove string-offset parsing/mapping from
offsetToViewTimelineRangeby only acceptingProgressIntersectionarray offsets. - Update unit tests to expect
undefined(non-accelerated) for string-based offsets.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/framer-motion/src/render/dom/scroll/utils/offset-to-range.ts | Removes string offset normalization so only preset ProgressIntersection arrays can map to ViewTimeline ranges. |
| packages/framer-motion/src/render/dom/scroll/utils/tests/offset-to-range.test.ts | Updates expectations to confirm string offsets no longer produce a ViewTimeline range (thus won’t accelerate). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| expect( | ||
| offsetToViewTimelineRange(["start start", "end end"]) | ||
| ).toEqual({ | ||
| rangeStart: "contain 0%", | ||
| rangeEnd: "contain 100%", | ||
| }) | ||
| }) | ||
|
|
||
| it("returns undefined for other string offsets", () => { | ||
| ).toBeUndefined() |
Greptile SummaryThis PR reverts the string-offset ViewTimeline acceleration introduced in v12.37.0 ( Key changes:
Confidence Score: 5/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["offsetToViewTimelineRange(offset)"] --> B{offset is undefined?}
B -- Yes --> C["Return contain 0% / contain 100%\n(default accelerated)"]
B -- No --> D["normaliseOffset(offset)"]
D --> E{All items\nare Arrays?}
E -- No\n(string offsets now\nalways fall here) --> F["Return undefined"]
E -- Yes --> G["Match against presets\n(Enter, Exit, Any, All)"]
G --> H{Matches a\npreset?}
H -- Yes --> I["Return named ViewTimeline range\ne.g. entry 0% / entry 100%"]
H -- No --> F
F --> J["Caller falls back to\nJS main-thread scroll tracking"]
|
|
The CI failure in This PR only modifies The failing Cypress test is timing-sensitive: it drags an element past constraints, releases, waits a hardcoded 2 seconds for the spring animation to settle, then asserts exact pixel positions. This is a known flaky pattern on slower CI runners. |
Problem
Since v12.37.0 (commit
6bae74e— "Add string offset to ViewTimeline"),useScrollwith string-based offsets such as['start start', 'end end']produces incorrect animation behaviour.When a scroll-driven animation target uses
position: stickyinside aheight: 200vhcontainer with a transformed parent, the ViewTimeline"contain"range does not produce identical results to the JS-based scroll tracking that was used before 12.37.0. Specifically:Reproduction
See issue #3658 for a CodeSandbox reproduction and a video demonstrating the bug.
Root Cause
The
parseStringOffset()function added in6bae74econverts string offsets like"start start"toProgressIntersectionarrays, which enablesoffsetToViewTimelineRange()to return a ViewTimeline range (e.g."contain 0%" / "contain 100%"). This causescanAccelerateScroll()inuse-scroll.tsto returntrue, switching from JS main-thread scroll tracking to hardware-accelerated WAAPI animation.The WAAPI ViewTimeline animation with
"contain"range behaves differently from the JS-based scroll tracking for this DOM structure, producing the visual regression.Notably, the existing Cypress E2E test in
scroll-view-timeline.tsstill asserts "Does NOT accelerate target with string offset", which was never updated after 12.37.0 — confirming the acceleration of string offsets was not fully tested.Fix
This PR removes
parseStringOffset()and thestringToProgressmapping fromoffset-to-range.ts, and makesnormaliseOffset()returnundefinedfor any non-array offset item. This restores the pre-12.37.0 behaviour where string offsets always fall back to JS main-thread scroll tracking.Array-based
ProgressIntersectionoffsets (the original preset format) continue to be accelerated as before — this change only affects string offsets.Changes
offset-to-range.ts: RemovedparseStringOffset(),stringToProgress, and string handling innormaliseOffset()(−20 lines)offset-to-range.test.ts: Updated string offset tests to expectundefinedinstead of range objects (+5/−58 lines)All 37 existing unit tests pass (offset-to-range, use-scroll, use-transform).
Fixes #3658