-
Notifications
You must be signed in to change notification settings - Fork 47.9k
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
Add xmlns to svg attributes. #6471
Conversation
For some reason I thought we already had this… maybe it was a part of the SVG change that got backed out. I like this in concept but let's do 2 things
|
Actually, I guess just leave it as in the SVG config since there's also |
Should we add |
@salzhrani updated the pull request. |
rebased and squashed. |
@salzhrani updated the pull request. |
As a React newbie, I found weird that React strips off the xmlns attribute while it claims full svg support. |
+1 for adding support for this. In the mean time if anyone is interested in a workaround (which I needed to be able to support IE9-11) this is what I came up with: // explicitly build the SVG to be rendered here so we don't lose the NS
const stringifiedSvg = `<svg xmlns="http://www.w3.org/2000/svg" class="svgClass">
<use xlink:href="#linkToSymbolId"></use>
<svg/>`
return <div dangerouslySetInnerHTML={{__html: stringifiedSvg}}/> |
I created a package that can add xmlns and doctype, to enable server side SVG rendering: svgx. |
+1 on this. Status update? |
Same question here, this is blocking my upgrade to react 15.0. Is anything blocking this PR? |
Coming back around to look at this again now. |
There is also #6532. |
I saw, was going to look into the NS change there. Otherwise I think I like this one better. The capitalization of xml{NS,ns} is annoying, both ways. |
Ok, going to take this as is (mostly, 1 comment incoming). I like |
@@ -271,6 +271,8 @@ var ATTRS = { | |||
yChannelSelector: 'yChannelSelector', | |||
z: 0, | |||
zoomAndPan: 'zoomAndPan', | |||
xmlns: 0, | |||
xmlnsXlink: 'xml:xlink', |
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.
2 things:
- This value should be
'xmlns:xlink'
right? - Can you move these up so they're with the other
xml*
attributes? Doesn't have to be perfect, we have a few others minorly out of order so we'll do a:sort
sometime later.
integrated feedback and squashed. |
(cherry picked from commit 9a80d42)
No description provided.