Skip to content

Fix nested elements in SVG #57

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

Jump to bottom
Merged
merged 3 commits into from
Feb 4, 2022
Merged

Fix nested elements in SVG #57

merged 3 commits into from
Feb 4, 2022

Conversation

@asvanberg
Copy link
Contributor

@asvanberg asvanberg commented Feb 4, 2022

Fixes #56
easafe
easafe requested changes Feb 4, 2022
View reviewed changes
Copy link
Owner

@easafe easafe left a comment

Choose a reason for hiding this comment

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

Nice work @asvanberg . Could you add some tests as well?
@asvanberg
Copy link
Contributor Author

asvanberg commented Feb 4, 2022

No idea how to write a test for this. Scanning over the both Main.purs and Main.js gives nothing helpful about nodeType so nothing to learn from. The only tests related to SVG seems to be the rendered HTML (which was fine before this change already).
@easafe
Copy link
Owner

easafe commented Feb 4, 2022

The tests are super messy, yeah. But for this kind of thing you can, say, mount the SVG snippet from the open issue and check that the elements were indeed created (e.g. https://github.com/easafe/purescript-flame/blob/master/test/Main.purs#L319 or https://github.com/easafe/purescript-flame/blob/master/test/Main.purs#L463)
@asvanberg asvanberg requested a review from easafe February 4, 2022 17:44
@easafe easafe merged commit 28936cd into easafe:master Feb 4, 2022
@asvanberg asvanberg deleted the nested-elements-in-svg branch February 4, 2022 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants

@asvanberg @easafe