Skip to content

fix: revert string offset ViewTimeline acceleration#3659

Open
PanagiotisKaraliolios wants to merge 1 commit intomotiondivision:mainfrom
PanagiotisKaraliolios:fix/revert-string-offset-view-timeline-acceleration
Open

fix: revert string offset ViewTimeline acceleration#3659
PanagiotisKaraliolios wants to merge 1 commit intomotiondivision:mainfrom
PanagiotisKaraliolios:fix/revert-string-offset-view-timeline-acceleration

Conversation

@PanagiotisKaraliolios
Copy link

Problem

Since v12.37.0 (commit 6bae74e — "Add string offset to ViewTimeline"), useScroll with string-based offsets such as ['start start', 'end end'] produces incorrect animation behaviour.

When a scroll-driven animation target uses position: sticky inside a height: 200vh container 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:

  • Some of the animation is already applied when the element first enters the viewport
  • The animation does not reach completion when scrolling past

Reproduction

See issue #3658 for a CodeSandbox reproduction and a video demonstrating the bug.

Root Cause

The parseStringOffset() function added in 6bae74e converts string offsets like "start start" to ProgressIntersection arrays, which enables offsetToViewTimelineRange() to return a ViewTimeline range (e.g. "contain 0%" / "contain 100%"). This causes canAccelerateScroll() in use-scroll.ts to return true, 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.ts still 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 the stringToProgress mapping from offset-to-range.ts, and makes normaliseOffset() return undefined for 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 ProgressIntersection offsets (the original preset format) continue to be accelerated as before — this change only affects string offsets.

Changes

  • offset-to-range.ts: Removed parseStringOffset(), stringToProgress, and string handling in normaliseOffset() (−20 lines)
  • offset-to-range.test.ts: Updated string offset tests to expect undefined instead of range objects (+5/−58 lines)

All 37 existing unit tests pass (offset-to-range, use-scroll, use-transform).

Fixes #3658

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
Copilot AI review requested due to automatic review settings March 18, 2026 20:43
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 offsetToViewTimelineRange by only accepting ProgressIntersection array 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.

Comment on lines 69 to +71
expect(
offsetToViewTimelineRange(["start start", "end end"])
).toEqual({
rangeStart: "contain 0%",
rangeEnd: "contain 100%",
})
})

it("returns undefined for other string offsets", () => {
).toBeUndefined()
@greptile-apps
Copy link

greptile-apps bot commented Mar 18, 2026

Greptile Summary

This PR reverts the string-offset ViewTimeline acceleration introduced in v12.37.0 (6bae74e) to fix a visual regression where useScroll with string-based offsets (e.g. ['start start', 'end end']) produced incorrect animation behaviour for sticky-positioned elements in transformed containers.

Key changes:

  • offset-to-range.ts: Removes parseStringOffset() and the stringToProgress lookup map. normaliseOffset() now returns undefined for any non-array offset item, meaning string offsets can never resolve to a ViewTimelineRange. This restores the pre-12.37.0 behaviour where string offsets always fall back to JS main-thread scroll tracking via canAccelerateScroll() returning false.
  • offset-to-range.test.ts: Updates the string-offset test suite to assert .toBeUndefined(). The previous five separate it() blocks (including a duplicated test name) are collapsed into one block; see inline comment for a minor test isolation note.
  • Array-based ProgressIntersection presets (Enter, Exit, Any, All) continue to be accelerated as before — this change is narrowly scoped to string offsets only.

Confidence Score: 5/5

  • Safe to merge — this is a minimal, focused revert that restores known-good behaviour for string-based scroll offsets with no risk to array-based preset acceleration.
  • The change is a clean surgical removal of code introduced in a prior commit. The fix is consistent with the stated pre-12.37.0 behaviour, all 37 existing unit tests pass, the root cause and reproduction are well-documented, and the diff touches only two files with no logic added — only code removed. The only minor concern is test isolation (multiple expect() calls in one it() block), which does not affect correctness.
  • No files require special attention.

Important Files Changed

Filename Overview
packages/framer-motion/src/render/dom/scroll/utils/offset-to-range.ts Removes parseStringOffset() and stringToProgress map; normaliseOffset() now returns undefined for any non-array offset item, correctly preventing string offsets from being accelerated via ViewTimeline.
packages/framer-motion/src/render/dom/scroll/utils/tests/offset-to-range.test.ts Collapses five separate string-offset it() blocks (including a duplicated one) into a single it() block with multiple expect() calls; all now assert .toBeUndefined(). Minor style note: early assertion failure will skip remaining assertions in the block.

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"]
Loading

Comments Outside Diff (1)

  1. packages/framer-motion/src/render/dom/scroll/utils/__tests__/offset-to-range.test.ts, line 59-75 (link)

    P2 Multiple assertions in one it() block will short-circuit on failure

    All five string-offset cases are now grouped into a single it() block. If the first expect() throws (e.g. because an offset is incorrectly parsed in a future change), Jest will not run the remaining assertions, making it harder to identify which specific case regressed.

    Consider splitting these into individual it() blocks (or using it.each) so every case is reported independently:

    const stringOffsetCases: Parameters<typeof offsetToViewTimelineRange>[0][] = [
        ["start end", "end end"],
        ["start start", "end start"],
        ["end start", "start end"],
        ["start start", "end end"],
        ["start center", "end start"],
    ]
    
    it.each(stringOffsetCases)(
        "returns undefined for string offset %j (not accelerated)",
        (offset) => {
            expect(offsetToViewTimelineRange(offset)).toBeUndefined()
        }
    )

    This is a minor style point — the current implementation is functionally correct.

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Last reviewed commit: "fix: revert string o..."

@PanagiotisKaraliolios
Copy link
Author

The CI failure in drag-nested.ts ("Nested drag with constraints and animation -- Child layout") is unrelated to this PR.

This PR only modifies offset-to-range.ts and its unit test — scroll offset utilities with zero overlap with the drag/constraints/layout system.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

useScroll with "start"/"end" string offsets broken with position: sticky inside transformed parent since 12.37.0

2 participants