Skip to content

Commit 425eb7e

Browse files
crisbetojelbourn
authored andcommitted
fix(icon): not copying attributes from symbol nodes (#16896)
Currently we have some special handling for the cases where an icon resolves to a `symbol` node, because cloning it directly won't display anything. Our workaround is set up by creating a blank SVG and copying over all of the children of the `symbol` into it, however this doesn't account for any attributes on the `symbol` (e.g. its `viewBox`). These changes also copy over all the attributes. Fixes #16892.
1 parent 6009211 commit 425eb7e

File tree

3 files changed

+34
-0
lines changed

3 files changed

+34
-0
lines changed

src/material/icon/fake-svgs.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,13 @@ export const FAKE_SVGS = {
4545
</defs>
4646
</svg>
4747
`,
48+
farmSet5: `
49+
<svg>
50+
<symbol id="duck" viewBox="0 0 13 37">
51+
<path id="quack" name="quack"></path>
52+
</symbol>
53+
</svg>
54+
`,
4855
arrows: `
4956
<svg>
5057
<defs>

src/material/icon/icon-registry.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -525,6 +525,16 @@ export class MatIconRegistry implements OnDestroy {
525525
*/
526526
private _toSvgElement(element: Element): SVGElement {
527527
const svg = this._svgElementFromString('<svg></svg>');
528+
const attributes = element.attributes;
529+
530+
// Copy over all the attributes from the `symbol` to the new SVG, except the id.
531+
for (let i = 0; i < attributes.length; i++) {
532+
const {name, value} = attributes[i];
533+
534+
if (name !== 'id') {
535+
svg.setAttribute(name, value);
536+
}
537+
}
528538

529539
for (let i = 0; i < element.childNodes.length; i++) {
530540
if (element.childNodes[i].nodeType === this._document.ELEMENT_NODE) {

src/material/icon/icon.spec.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -433,6 +433,23 @@ describe('MatIcon', () => {
433433
expect((firstChild as HTMLElement).getAttribute('name')).toBe('quack');
434434
});
435435

436+
it('should copy over the attributes when unwrapping <symbol> nodes', () => {
437+
iconRegistry.addSvgIconSetInNamespace('farm', trustUrl('farm-set-5.svg'));
438+
439+
const fixture = TestBed.createComponent(IconFromSvgName);
440+
const testComponent = fixture.componentInstance;
441+
const matIconElement = fixture.debugElement.nativeElement.querySelector('mat-icon');
442+
443+
testComponent.iconName = 'farm:duck';
444+
fixture.detectChanges();
445+
http.expectOne('farm-set-5.svg').flush(FAKE_SVGS.farmSet5);
446+
447+
const svgElement = verifyAndGetSingleSvgChild(matIconElement);
448+
expect(svgElement.getAttribute('viewBox')).toBe('0 0 13 37');
449+
expect(svgElement.getAttribute('id')).toBeFalsy();
450+
expect(svgElement.querySelector('symbol')).toBeFalsy();
451+
});
452+
436453
it('should not wrap <svg> elements in icon sets in another svg tag', () => {
437454
iconRegistry.addSvgIconSet(trustUrl('arrow-set.svg'));
438455

0 commit comments

Comments
 (0)