Skip to content

fix(drag): refresh root scroll before measuring ref constraints (#2829)#3722

Merged
mattgperry merged 1 commit into
mainfrom
worktree-fix-issue-2829
May 12, 2026
Merged

fix(drag): refresh root scroll before measuring ref constraints (#2829)#3722
mattgperry merged 1 commit into
mainfrom
worktree-fix-issue-2829

Conversation

@mattgperry
Copy link
Copy Markdown
Collaborator

Summary

  • When dragConstraints is a ref to a viewport-sized element (e.g. position: absolute; inset: 0) and the document is already scrolled when the drag mounts, the constraints are computed with a stale scroll offset. The draggable area shrinks by roughly the scroll amount — the user reported they could no longer drag to the bottom of the viewport after refreshing on a scrolled page.
  • Root cause: drag setup runs in a layout effect, calls projection.root.updateScroll() once, and then schedules measureDragConstraints via frame.read. If scroll changes in between (browser restoring scroll on refresh, or an ancestor's layout effect scrolling), the cached root scroll is wrong when the constraint element's viewport box is translated to page coords.
  • Fix: clear and re-measure projection.root.scroll inside resolveRefConstraints so the page-coord translation uses the live scroll offset. projection.updateScroll's per-animationId cache made the obvious "just call updateScroll again" a no-op, so the cached value is cleared first.

Test plan

  • New Cypress test drag-ref-constraints-absolute-scrolled reproduces the bug: pre-scroll the window in a layout effect, drag a motion.div inside a position: absolute; inset: 0 ref constraint, assert the box reaches the visible bottom of the viewport. Fails without the fix (expected 200 to be close to 500), passes with it.
  • All existing drag Cypress tests pass (92/92) on React 18.
  • New test passes on React 19.
  • yarn test passes (793 unit tests).

Fixes #2829

🤖 Generated with Claude Code

The scroll captured by the drag feature on mount can be stale by the
time `frame.read` reads the constraint element's bounding box — for
example when the browser restores scroll on refresh, or when an
ancestor's layout effect scrolls after this element mounts. The
constraint then translates to wrong page coordinates and the
draggable area shrinks by the scroll offset.

Clear and re-measure the root scroll inside `resolveRefConstraints`
so the constraint's page box matches the live scroll position.

Fixes #2829

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 12, 2026

Greptile Summary

This PR fixes a stale scroll offset bug (#2829) where dragConstraints pointing to a viewport-sized position: absolute; inset: 0 element would incorrectly shrink the draggable area when the page was already scrolled on mount. The fix clears projection.root.scroll and re-calls updateScroll() inside resolveRefConstraints() to bypass the per-animationId cache and force a live measurement before converting the constraint element's viewport box to page coordinates.

  • VisualElementDragControls.ts: Inserts a targeted projection.root.scroll = undefined + updateScroll() call at the top of resolveRefConstraints(). The approach is correct — DocumentProjectionNode always has layoutScroll: true so updateScroll will re-read document.documentElement.scrollTop after the cache is cleared.
  • New Cypress test + fixture: Reproduces the bug by pre-scrolling the window in a useLayoutEffect, then asserting the draggable reaches the visible constraint bottom (rect.bottom ≈ 500). The test logic is sound; a .wait(300) before asserting the scroll is time-based and may be flaky on slow CI runners.

Confidence Score: 4/5

Safe to merge; the core change is a small, targeted mutation of a shared singleton's cached scroll value that is immediately re-populated, and is guarded by a null check consistent with the surrounding code.

The logic fix in VisualElementDragControls.ts is correct and well-reasoned. The only concern is the .wait(300) in the new Cypress test, which is time-dependent and could produce intermittent failures on slow CI runners rather than a deterministic signal.

packages/framer-motion/cypress/integration/drag-ref-constraints-absolute-scrolled.ts — the time-based wait warrants a second look.

Important Files Changed

Filename Overview
packages/framer-motion/src/gestures/drag/VisualElementDragControls.ts Core fix: clears and re-measures projection.root.scroll inside resolveRefConstraints() before computing page-coordinate constraints, so a stale cached scroll offset no longer shrinks the draggable area on a pre-scrolled page.
packages/framer-motion/cypress/integration/drag-ref-constraints-absolute-scrolled.ts New Cypress regression test for #2829; covers the pre-scrolled page scenario correctly, but contains a time-based .wait(300) that is potentially flaky on slow CI runners.
dev/react/src/tests/drag-ref-constraints-absolute-scrolled.tsx New test fixture page that renders a viewport-sized absolute constraint element and scrolls the window in a layout effect, faithfully reproducing the #2829 scenario.

Sequence Diagram

sequenceDiagram
    participant LE as useLayoutEffect (test page)
    participant DC as VisualElementDragControls
    participant PR as projection.root (DocumentProjectionNode)
    participant MP as measurePageBox

    LE->>LE: window.scrollTo(0, 300) — scroll restored
    Note over DC: drag mount (layout effect)
    DC->>PR: "updateScroll() — caches scroll=0 (stale, animationId cached)"
    DC->>DC: frame.read(measureDragConstraints)
    Note over DC: next frame — measureDragConstraints fires
    DC->>DC: resolveRefConstraints()
    DC->>PR: "scroll = undefined (bust cache)"
    DC->>PR: "updateScroll() — re-reads live scrollY=300"
    DC->>MP: measurePageBox(constraintsElement, projection.root)
    MP-->>DC: constraintsBox with correct page coords
    DC-->>DC: constraints computed correctly
Loading

Reviews (1): Last reviewed commit: "fix(drag): Refresh root scroll before me..." | Re-trigger Greptile

Comment on lines +14 to +15
.window()
.then((win) => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Time-based wait may cause flaky test on slow CI runners

The .wait(300) on line 14 relies on a fixed timeout to let the useLayoutEffect scroll settle before asserting win.scrollY > 0. On an overloaded CI machine the effect may not have run yet, while on a fast machine the wait is unnecessarily slow. A more robust approach would be to poll until the scroll condition is true — e.g., wrap the expect(win.scrollY).to.be.greaterThan(0) check in a cy.waitUntil or use a Cypress retry via .should(() => { ... }) attached directly to cy.window().

@mattgperry mattgperry merged commit 5973dfb into main May 12, 2026
8 checks passed
@mattgperry mattgperry deleted the worktree-fix-issue-2829 branch May 12, 2026 09:14
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.

[BUG] Draggable area incorrectly reduced after page scroll and refresh

1 participant