fix(icon): handle icons as <symbol> nodes #4699
Conversation
Fixes icons not being rendered if they're defined as a `<symbol>` inside the source file. Fixes angular#4680.
googlebot
added
the
cla: yes
crisbeto
force-pushed
the
4680/symbol-icons
branch
from
23ce96a to
798336f
Compare
crisbeto
force-pushed
the
4680/symbol-icons
branch
from
798336f to
511591e
Compare
| return this._setSvgAttributes(iconNode.cloneNode(true) as SVGElement); | ||
| } | ||
|
|
||
| // If the node is a <symbol>, it won't be rendered so we have to convert it into <svg>. Note |
There was a problem hiding this comment.
I'm not sure we want this behavior. If an SVG has a <symbol> element, we should interpret it as not rendering out on purpose, where the <use> element should be used to render something. We shouldn't be making assumptions about why someone is using <symbol> in their SVG.
There was a problem hiding this comment.
We're not doing this for all symbol elements, but rather for the one that the user is referring to via an id. I think it's safe to assume that if there's a <symbol id="something"> and the user is referring to it via iconSet:something, it would be expected that the symbol would be rendered.
There was a problem hiding this comment.
That's fair, I suppose. Are you able to reference <symbol> elements in <use> elements in a completely different <svg> parent? I'm wondering if it would make sense to just use <use> for this, though it might be overkill.
There was a problem hiding this comment.
I initially went for the use element, but the problem is that, if your page has a base tag (which all Angular apps have), Firefox requires you to include the current URL before the hash. E.g. if I was using an icon on /about, the use element would be <use href="/about#icon-id"/>. The href also has to be maintained on route changes.
jelbourn
added
pr: lgtm
action: merge
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Fixes icons not being rendered if they're defined as a
<symbol>inside the source file.Fixes #4680.