Handle nested svg and foreignObject elements #184
Conversation
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
jleen
commented
Mar 8, 2025
| expect( | ||
| <svg id="foo" className="wrapper" viewBox="25 25 50 50"> | ||
| <circle id="baz" className="circle" cx="50" cy="50" r="20" /> | ||
| <svg id="bar" className="inner" viewBox="1 2 3 4"> |
Author
There was a problem hiding this comment.
This is the first case that’s fixed by my PR: an svg within an svg. Formerly, the inner svg was having its attributes erased due to double conversion.
jleen
commented
Mar 8, 2025
Author
jleen
left a comment
jleen
left a comment
There was a problem hiding this comment.
Thanks for your useful library. This little change fixes a couple of bugs that I encountered.
| <circle id="ding" className="ball" cx="40" cy="50" r="60" /> | ||
| </svg> | ||
| <foreignObject id="xyzzy" className="abc"> | ||
| <a href="quux"/> |
Author
There was a problem hiding this comment.
This is the second case that I’m fixing: an HTML tag within a foreign object was also having its props erased.
| return data; | ||
| }; | ||
|
|
||
| const tagFromSel = (sel: String | undefined): String | undefined => { |
Author
There was a problem hiding this comment.
I added this utility method so that considerSVG can easily determine what its child tags are. I couldn’t think of a simpler way to do this, but I’m open to suggestions.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
snabbdom-jsx wasn’t correctly handling svg tags inside of svg tags. considerSVG was double-converting them, leading to an invalid DOM. Similarly, foreignObject inside of svg was getting converted incorrectly. This patch adds unit tests for both of these cases, and adds a check in considerSVG to avoid incorrect conversion.