-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix layout animations not triggering with MotionValue #3651
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| import { motion, useMotionValue } from "framer-motion" | ||
|
|
||
| export const App = () => { | ||
| const width = useMotionValue(100) | ||
|
|
||
| return ( | ||
| <> | ||
| <motion.div | ||
| id="box" | ||
| layout | ||
| style={{ | ||
| width, | ||
| height: 100, | ||
| position: "absolute", | ||
| top: 0, | ||
| left: 0, | ||
| background: "red", | ||
| }} | ||
| transition={{ duration: 0.5, ease: () => 0.5 }} | ||
| /> | ||
| <button | ||
| id="toggle" | ||
| style={{ position: "absolute", top: 200 }} | ||
| onClick={() => width.set(width.get() === 100 ? 300 : 100)} | ||
| > | ||
| Toggle | ||
| </button> | ||
| </> | ||
| ) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| describe("Layout animation with MotionValue", () => { | ||
| it("Triggers layout animation when MotionValue changes a layout-affecting property", () => { | ||
| cy.visit("?test=layout-motion-value") | ||
| .wait(50) | ||
| .get("#box") | ||
| .should(([$box]: any) => { | ||
| const bbox = $box.getBoundingClientRect() | ||
| expect(bbox.width).to.equal(100) | ||
| }) | ||
| .get("#toggle") | ||
| .trigger("click") | ||
| .wait(50) | ||
| .get("#box") | ||
| .should(([$box]: any) => { | ||
| const bbox = $box.getBoundingClientRect() | ||
| // With ease: () => 0.5, layout animation freezes at 50% | ||
| // Width should be 200 (midpoint of 100→300), not 300 (no animation) | ||
| expect(bbox.width).to.equal(200) | ||
| }) | ||
| }) | ||
| }) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -45,6 +45,15 @@ import { | |
| } from "./utils/reduced-motion" | ||
| import { resolveVariantFromProps } from "./utils/resolve-variants" | ||
|
|
||
| const layoutKeys = new Set([ | ||
| "width", | ||
| "height", | ||
| "top", | ||
| "left", | ||
| "right", | ||
| "bottom", | ||
| ]) | ||
|
|
||
| const propEventHandlers = [ | ||
| "AnimationStart", | ||
| "AnimationComplete", | ||
|
|
@@ -567,6 +576,7 @@ export abstract class VisualElement< | |
| } | ||
|
|
||
| const valueIsTransform = transformProps.has(key) | ||
| const valueIsLayout = !valueIsTransform && layoutKeys.has(key) | ||
|
|
||
| if (valueIsTransform && this.onBindTransform) { | ||
| this.onBindTransform() | ||
|
|
@@ -579,8 +589,15 @@ export abstract class VisualElement< | |
|
|
||
| this.props.onUpdate && frame.preRender(this.notifyUpdate) | ||
|
|
||
| if (valueIsTransform && this.projection) { | ||
| this.projection.isTransformDirty = true | ||
| if (this.projection) { | ||
| if (valueIsTransform) { | ||
| this.projection.isTransformDirty = true | ||
| } else if (valueIsLayout) { | ||
| this.projection.willUpdate() | ||
| frame.postRender( | ||
| () => this.projection?.root?.didUpdate() | ||
| ) | ||
|
Comment on lines
+597
to
+599
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The A stable, pre-bound reference on the class (similar to how // as a class field:
private scheduleDidUpdate = () => this.projection?.root?.didUpdate()
// in the onChange handler:
frame.postRender(this.scheduleDidUpdate)This mirrors the existing pattern for |
||
| } | ||
| } | ||
|
|
||
| this.scheduleRender() | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
layoutKeysdoesn't cover all layout-affecting CSS propertiesThe set only includes the six box-position/size properties listed here. Several other CSS properties that a user might drive with a
MotionValuecan equally affect an element's bounding box and therefore fail to trigger layout animations:minWidth/maxWidth/minHeight/maxHeightpadding/paddingTop/paddingRight/paddingBottom/paddingLeftmargin/marginTop/marginRight/marginBottom/marginLeftinset(CSS shorthand for top/right/bottom/left)borderWidthand its long-hand variantsIf this is an intentional trade-off (covering only the most common cases), it would be worth documenting with a comment so future contributors understand the scope.