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
Change classMap to set classes correctly for SVGs #932
Conversation
The assignment to `element.className` in `classMap` was causing the error `setting getter-only property "className"`. According to [the SVG specification](https://www.w3.org/TR/SVG11/types.html#InterfaceSVGStylable), `className` is a read-only property. This differs from [`className` on Element](https://developer.mozilla.org/en-US/docs/Web/API/Element/className), which can be set.
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.
Thanks!
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.
Thanks for the work!
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.
We have to go back to el.setAttribute('class', value)
due to IE11. Sorry!
Fixed! For posterity, here is the Can I Use page for
Which matters here because this pull request's specific intent is to fix support for SVG elements. |
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.
Sorry for the back and forth. Thanks for doing the work, though!
Ping @justinfagnani, this is good to merge. |
Thanks @AdamVig and @jridgewell! @AdamVig could you update the changelog for this? Otherwise, looks good. |
@justinfagnani Sure, done. |
This reverts commit dbb3312.
@AdamVig unfortunately, IE11 doesn't support Looks like Stencil ran into the same problem here: ionic-team/stencil#1079 |
This reverts commit dbb3312.
Only test uses classList. The actual code already uses setAttribute, so is needed to update only the tests |
No, the problem is in the What's happening is that this PR fixes the first error with classMap on SVG, and so exposes the second, which is IE-only. |
The assignment to `element.className` in `classMap` was causing the error `setting getter-only property "className"`. According to [the SVG specification](https://www.w3.org/TR/SVG11/types.html#InterfaceSVGStylable), `className` is a read-only property. This differs from [`className` on Element](https://developer.mozilla.org/en-US/docs/Web/API/Element/className), which can be set.
This reverts commit dbb3312.
The assignment to
element.className
inclassMap
was causing the errorsetting getter-only property "className"
.According to the SVG specification,
className
is a read-only property. This differs fromclassName
on Element, which can be set.Fixes #930.