fix: apply x/y props on nested Svg elements as translate transform (#1283)#2973
Open
MuhammadSuleman97 wants to merge 1 commit into
Open
fix: apply x/y props on nested Svg elements as translate transform (#1283)#2973MuhammadSuleman97 wants to merge 1 commit into
MuhammadSuleman97 wants to merge 1 commit into
Conversation
When an Svg is nested inside another Svg, the x and y props were not being applied. On the web, nested SVG elements respect x/y for positioning within their parent SVG. This fix extracts x and y from the Svg element props and converts them to a translate transform that gets merged with any existing transform prop, matching the behavior of standard SVG elements. Fixes software-mansion#1283
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds support for positioning the root <Svg> using x/y by converting them into a translated React Native transform, and prevents x/y from being forwarded as native props.
Changes:
- Destructure
x/yfrom extracted props and remove them from the forwarded props object. - Extend transform extraction to optionally append
translateX/translateYbased onx/y. - Stop forwarding
onLayoutin the props object passed to the rendered component.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
184
to
+191
| const gStyle = Object.assign({}, StyleSheet.flatten(style)); | ||
| if (transform) { | ||
| if (transform || x != null || y != null) { | ||
| if (gStyle.transform) { | ||
| props.transform = gStyle.transform; | ||
| gStyle.transform = undefined; | ||
| } | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| props.transform = extractTransformSvgView(props as any); | ||
| const svgTransform = extractTransformSvgView(props as any); |
Comment on lines
+191
to
+198
| const svgTransform = extractTransformSvgView(props as any); | ||
| if (x != null || y != null) { | ||
| const translateX = x != null ? parseFloat(String(x)) : 0; | ||
| const translateY = y != null ? parseFloat(String(y)) : 0; | ||
| const translateTransform = [ | ||
| { translateX }, | ||
| { translateY }, | ||
| ]; |
Comment on lines
140
to
+142
| const props: extractedProps = extracted as extractedProps; | ||
| delete (props as Record<string, unknown>).x; | ||
| delete (props as Record<string, unknown>).y; |
Comment on lines
230
to
234
| strokeLinecap, | ||
| strokeLinejoin, | ||
| strokeMiterlimit, | ||
| onLayout, | ||
| }} | ||
| /> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #1283 — Nested SVG elements do not respect
xandyprops for positioning.Problem
When embedding an
<Svg>inside another<Svg>, setting thexandyprops on the inner SVG had no effect. On the web, nested SVG elements respectx/yfor viewport positioning within their parent SVG.Fix
The
xandyprops are now extracted in the Svg element's render method and converted to atranslatetransform that gets merged with any existingtransformprop.The
xandyprops are also removed from the extracted props before passing to the native component to avoid unknown prop warnings.Test Plan
The inner SVG (blue rect) should be positioned at (50, 100) within the outer SVG.