-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix MotionValues on SVG transform attribute rendering as [object Object] #3629
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
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 | ||||
|---|---|---|---|---|---|---|
|
|
@@ -120,6 +120,30 @@ describe("SVG", () => { | |||||
| ) | ||||||
| }) | ||||||
|
|
||||||
| test("MotionValue can be used for transform attribute on g element", async () => { | ||||||
| const Component = () => { | ||||||
| const transformValue = useMotionValue("translate(50, 50)") | ||||||
|
|
||||||
| return ( | ||||||
| <svg> | ||||||
| <motion.g transform={transformValue as any}> | ||||||
| <motion.rect width={50} height={50} /> | ||||||
| </motion.g> | ||||||
| </svg> | ||||||
| ) | ||||||
| } | ||||||
|
|
||||||
| const { container } = render(<Component />) | ||||||
|
|
||||||
| await nextFrame() | ||||||
|
|
||||||
| const gElement = container.querySelector("g")! | ||||||
| // The transform should NOT be rendered as "[object Object]" | ||||||
| expect(gElement.getAttribute("transform")).not.toBe("[object Object]") | ||||||
|
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. Weak negative assertion The assertion
Suggested change
This would make the test self-documenting: after the fix, |
||||||
| // It should be applied as a CSS style transform | ||||||
| expect(gElement).toHaveStyle("transform: translate(50, 50)") | ||||||
| }) | ||||||
|
|
||||||
| test("animates viewBox", async () => { | ||||||
| const Component = () => { | ||||||
| return ( | ||||||
|
|
||||||
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.
Over-broad TypeScript cast
Using
as unknownfully erases type information and bypasses all TypeScript safety checks. The root cause is thatgetProps()returnsMotionProps(from framer-motion's own types), but the type fordragSnapToOriginin that interface appears to be narrower than the fullboolean | "x" | "y"union defined inmotion-dom/src/node/types.ts.A more precise cast — or better yet, aligning
MotionPropswith the upstream type — would be safer:If the mismatch is in
MotionProps, the proper long-term fix is to ensureMotionProps.dragSnapToOriginis typed asboolean | "x" | "y", which removes the need for a cast entirely.