Skip to content

[LiveComponent] Fix svg re-render #557

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

Merged
merged 1 commit into from
Nov 28, 2022
Merged

[LiveComponent] Fix svg re-render #557

merged 1 commit into from
Nov 28, 2022

Conversation

norkunas
Copy link
Contributor

Q A
Bug fix? no
New feature? no
Tickets #556
License MIT

@weaverryan
Copy link
Member

Thanks for the reproducer test - super useful. I can't think, immediately, what the problem might be - maybe something in morphdom? Unless someone beats me to it (which would be great!) I'll check it out when I have some time.

@norkunas
Copy link
Contributor Author

Thought about morphdom too, but they have svg tests,so not sure how this could happen currently..

@norkunas norkunas force-pushed the svg branch 3 times, most recently from a1e6ba9 to dbc93e8 Compare November 23, 2022 08:23
@norkunas norkunas changed the title [LiveComponent] Add failing test for SVG updates after re-render [LiveComponent] Fix svg re-render Nov 23, 2022
@norkunas
Copy link
Contributor Author

Found the root cause and added a fix.

if (
!(fromEl instanceof HTMLElement || fromEl instanceof SVGElement) ||
!(toEl instanceof HTMLElement || toEl instanceof SVGElement)
) {
Copy link
Member

Choose a reason for hiding this comment

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

Nice find. SVG's not being part of HTMLElement is such a pain in the butt :). Thanks for the test case to prevent it from slipping through in the future.

@weaverryan
Copy link
Member

Thanks Tomas!

@weaverryan weaverryan merged commit e27d318 into symfony:2.x Nov 28, 2022
@norkunas norkunas deleted the svg branch November 28, 2022 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants