Skip to content

Fix wrong starting position for layout transitions in sticky containers #3657

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mattgperry wants to merge 1 commit into
base: main
Choose a base branch
from
+181 −1
Open

Fix wrong starting position for layout transitions in sticky containers #3657

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 106 additions & 0 deletions dev/react/src/tests/layout-shared-sticky.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
import { motion } from "framer-motion"
import { useRef, useState } from "react"

/**
* Reproduction for #2941: layoutId transitions have wrong starting position
* when elements are inside a sticky container with a top offset.
*/

const MenuButton = ({
label,
active,
onClick,
id,
indicatorRef,
}: {
label: string
active: boolean
onClick: () => void
id: string
indicatorRef: React.RefObject<HTMLDivElement | null>
}) => {
return (
<button
id={id}
onClick={onClick}
style={{
position: "relative",
border: "1px solid #ccc",
borderRadius: 8,
padding: "8px 16px",
background: "transparent",
cursor: "pointer",
}}
>
{label}
{active && (
<motion.div
ref={indicatorRef}
id="indicator"
layoutId="indicator"
style={{
position: "absolute",
inset: 0,
borderRadius: 8,
background: "rgba(255, 200, 0, 0.7)",
}}
transition={{ duration: 10, ease: "linear" }}
onLayoutAnimationStart={() => {
if (indicatorRef.current) {
const rect =
indicatorRef.current.getBoundingClientRect()
indicatorRef.current.dataset.startTop =
String(rect.top)
indicatorRef.current.dataset.scrollY = String(
window.scrollY
)
}
}}
/>
)}
</button>
)
}

export const App = () => {
const [active, setActive] = useState(1)
const indicatorRef = useRef<HTMLDivElement>(null)

return (
<div style={{ display: "flex", width: "100%" }}>
<div>
<div
id="sticky-container"
style={{
width: 120,
display: "flex",
flexDirection: "column",
gap: 8,
position: "sticky",
top: 20,
padding: 8,
}}
>
{[1, 2, 3, 4].map((v) => (
<MenuButton
key={v}
id={`btn-${v}`}
label={v.toString()}
active={v === active}
onClick={() => setActive(v)}
indicatorRef={indicatorRef}
/>
))}
</div>
</div>
<div
id="content"
style={{
height: "500vh",
background: "#eee",
flex: 1,
}}
/>
</div>
)
}
60 changes: 60 additions & 0 deletions packages/framer-motion/cypress/integration/layout-shared-sticky.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
describe("Shared layout: sticky container", () => {
it("Layout animation works inside a sticky container after scrolling", () => {
cy.visit("?test=layout-shared-sticky")
.wait(50)
// Click btn-2 so indicator moves there
.get("#btn-2")
.trigger("click")
.wait(300)
// Scroll down so sticky container kicks in
.window()
.then((win: any) => {
win.scrollTo(0, 2000)
})
.wait(100)
// Click btn-3 to trigger layoutId animation while sticky
.get("#btn-3")
.trigger("click")
.wait(500)
.get("#indicator")
.then(([$indicator]: any) => {
const appDoc = $indicator.ownerDocument
const btn2 = (
appDoc.querySelector("#btn-2") as HTMLElement
).getBoundingClientRect()
const btn3 = (
appDoc.querySelector("#btn-3") as HTMLElement
).getBoundingClientRect()
const indicator = $indicator.getBoundingClientRect()
const scrollY =
appDoc.defaultView.scrollY ||
appDoc.defaultView.pageYOffset

// The indicator is animating from btn-2 to btn-3 (10s linear).
// At 500ms (~5% progress), it should be between the two buttons.
//
// WITHOUT the fix, the starting position is offset by ~scrollY
// (2000px), so the indicator would be far outside the button range.
//
// WITH the fix, the indicator stays within the button area.
const buttonsMinTop = Math.min(btn2.top, btn3.top)
const buttonsMaxTop = Math.max(btn2.top, btn3.top)

// Indicator must be within the button range (with tolerance
// for animation overshoot). The key assertion: it must NOT
// be offset by the scroll amount.
expect(indicator.top).to.be.greaterThan(
buttonsMinTop - 50,
`Indicator (${indicator.top}) should be near buttons ` +
`(${buttonsMinTop}-${buttonsMaxTop}), ` +
`not offset by scroll (${scrollY})`
)
expect(indicator.top).to.be.lessThan(
buttonsMaxTop + 50,
`Indicator (${indicator.top}) should be near buttons ` +
`(${buttonsMinTop}-${buttonsMaxTop}), ` +
`not offset by scroll (${scrollY})`
)
})
})
})
Comment on lines +1 to +60
Copy link

@greptile-apps greptile-apps bot Mar 17, 2026

Choose a reason for hiding this comment

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

P2 Test only covers the fully-engaged sticky state

The Cypress test scrolls to 2 000 px (far past the sticky threshold of top: 20) and then triggers the animation. This validates the primary bug scenario but leaves two important cases untested:

  1. Sticky not yet engaged with page scroll > 0 — e.g., top: 200, scroll to 100 px, then animate. With the current implementation hasStickyAncestor still returns true and the scroll offset is skipped, producing an animation starting position offset by -100 px.
  2. Sticky inside an inner scroll container — the root scroll offset should still be applied.

Adding a test variant at scrollY < sticky.top (before engagement) would catch the regression described in the hasStickyAncestor comments above.

8 changes: 8 additions & 0 deletions packages/motion-dom/src/projection/node/HTMLProjectionNode.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,12 @@ export const HTMLProjectionNode = createProjectionNode<HTMLElement>({
},
checkIsScrollRoot: (instance) =>
Boolean(window.getComputedStyle(instance).position === "fixed"),
hasStickyAncestor: (instance) => {
let el = instance.parentElement
while (el) {
if (window.getComputedStyle(el).position === "sticky") return true
el = el.parentElement
}
return false
},
Comment on lines +28 to +35
Copy link

@greptile-apps greptile-apps bot Mar 17, 2026

Choose a reason for hiding this comment

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

P1 Sticky-not-yet-engaged case skips scroll offset incorrectly

hasStickyAncestor returns true purely based on the CSS position property — it does not check whether the sticky ancestor is currently in its "stuck" (engaged) state. This means that when a page has been scrolled but not enough for the sticky container to engage, the scroll offset compensation is still skipped.

In this scenario the element's getBoundingClientRect().y equals naturalPageY - scrollY (normal-flow behaviour). Skipping translateAxis(box.y, scrollY) makes measurePageBox() return naturalPageY - scrollY instead of the correct naturalPageY, so the animation starts scrollY pixels too high.

To detect whether a sticky ancestor is actually engaged, you could compare the ancestor's getBoundingClientRect().top against its top CSS value:

hasStickyAncestor: (instance) => {
    let el = instance.parentElement
    while (el) {
        const style = window.getComputedStyle(el)
        if (style.position === "sticky") {
            const stickyTop = parseFloat(style.top) || 0
            // If the element is at (or near) its `top` value, it is engaged
            if (el.getBoundingClientRect().top <= stickyTop + 1) return true
        }
        el = el.parentElement
    }
    return false
},

The existing test only exercises the fully-engaged state (scroll to 2 000 px), so this case is not currently covered.

Comment on lines +28 to +35
Copy link

@greptile-apps greptile-apps bot Mar 17, 2026

Choose a reason for hiding this comment

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

P1 Walk does not stop at a scroll-container boundary

position: sticky is resolved relative to the nearest scrollable ancestor, not necessarily the root viewport. If the sticky container is inside an overflow: scroll/auto element, its "stuck" position is relative to that inner container, not the window. In that case the code skips this.root.scroll.offset (the window-level scroll) even though the correction should still be applied for the inner-container case.

Consider stopping the ancestor walk when an element with overflow: scroll or overflow: auto is encountered — that scroll container is the sticky's actual reference frame and the root scroll offset would still be relevant.

hasStickyAncestor: (instance) => {
    let el = instance.parentElement
    while (el) {
        const style = window.getComputedStyle(el)
        const overflow = style.overflowY
        // Stop at a scroll container — sticky is relative to it, not the root
        if (overflow === "scroll" || overflow === "auto") break
        if (style.position === "sticky") return true
        el = el.parentElement
    }
    return false
},

Comment on lines +28 to +35
Copy link

@greptile-apps greptile-apps bot Mar 17, 2026

Choose a reason for hiding this comment

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

P2 getComputedStyle called for every ancestor on each measurement

window.getComputedStyle() forces a style recalculation and is relatively expensive. The function is called for every DOM ancestor in the tree on every measurePageBox() invocation — which also occurs inside removeTransform() for each ancestor node that has a transform (line 1124 of create-projection-node.ts).

For layouts with deep DOM hierarchies or many transformed ancestors this could accumulate meaningful cost at animation-start time. Consider caching the result on the projection node (invalidated on mount/unmount) so the walk is done at most once per animation batch rather than per-measurement.

})
7 changes: 6 additions & 1 deletion packages/motion-dom/src/projection/node/create-projection-node.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ export function createProjectionNode<I>({
defaultParent,
measureScroll,
checkIsScrollRoot,
hasStickyAncestor,
resetTransform,
}: ProjectionNodeConfig<I>) {
return class ProjectionNode implements IProjectionNode<I> {
Expand Down Expand Up @@ -1021,7 +1022,11 @@ export function createProjectionNode<I>({
const box = visualElement.measureViewportBox()

const wasInScrollRoot =
this.scroll?.wasRoot || this.path.some(checkNodeWasScrollRoot)
this.scroll?.wasRoot ||
this.path.some(checkNodeWasScrollRoot) ||
(hasStickyAncestor &&
this.instance &&
hasStickyAncestor(this.instance))

if (!wasInScrollRoot) {
// Remove viewport scroll to give page-relative coordinates
Expand Down
1 change: 1 addition & 0 deletions packages/motion-dom/src/projection/node/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ export interface ProjectionNodeConfig<I> {
) => VoidFunction
measureScroll: (instance: I) => Point
checkIsScrollRoot: (instance: I) => boolean
hasStickyAncestor?: (instance: I) => boolean
resetTransform?: (instance: I, value?: string) => void
}

Expand Down