From 92a76e791bb305fe1ce00708ee34645f08055490 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Wed, 27 Jun 2018 22:51:32 +0200 Subject: [PATCH] fix(icon): clearing all content when inserting a new SVG Currently when we insert a new SVG icon, we clear the entire content of the `mat-icon` in order to get rid of the previous icon. This is problematic, because it ends up removing other elements like `mat-badge`. These changes switch to removing all non-element nodes and all SVGs. Fixes #11151. --- src/lib/icon/icon.spec.ts | 26 ++++++++++++++++++++++++++ src/lib/icon/icon.ts | 13 +++++++++---- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/src/lib/icon/icon.spec.ts b/src/lib/icon/icon.spec.ts index b12c4ea68ed8..d8df8da37346 100644 --- a/src/lib/icon/icon.spec.ts +++ b/src/lib/icon/icon.spec.ts @@ -51,6 +51,7 @@ describe('MatIcon', () => { IconWithAriaHiddenFalse, IconWithBindingAndNgIf, InlineIcon, + SvgIconWithUserContent, ] }); @@ -397,6 +398,26 @@ describe('MatIcon', () => { expect(icon.querySelector('svg')).toBeFalsy(); }); + + it('should keep non-SVG user content inside the icon element', fakeAsync(() => { + iconRegistry.addSvgIcon('fido', trustUrl('dog.svg')); + + const fixture = TestBed.createComponent(SvgIconWithUserContent); + const testComponent = fixture.componentInstance; + const iconElement = fixture.debugElement.nativeElement.querySelector('mat-icon'); + + testComponent.iconName = 'fido'; + fixture.detectChanges(); + http.expectOne('dog.svg').flush(FAKE_SVGS.dog); + + const userDiv = iconElement.querySelector('div'); + + expect(userDiv).toBeTruthy(); + expect(iconElement.textContent.trim()).toContain('Hello'); + + tick(); + })); + }); describe('Icons from HTML string', () => { @@ -706,3 +727,8 @@ class IconWithBindingAndNgIf { class InlineIcon { inline = false; } + +@Component({template: `
Hello
`}) +class SvgIconWithUserContent { + iconName: string | undefined = ''; +} diff --git a/src/lib/icon/icon.ts b/src/lib/icon/icon.ts index d07002ed220d..fc69c274f326 100644 --- a/src/lib/icon/icon.ts +++ b/src/lib/icon/icon.ts @@ -74,7 +74,6 @@ export const _MatIconMixinBase = mixinColor(MatIconBase); changeDetection: ChangeDetectionStrategy.OnPush, }) export class MatIcon extends _MatIconMixinBase implements OnChanges, OnInit, CanColor { - /** * Whether the icon should be inlined, automatically sizing the icon to match the font size of * the element the icon is contained in. @@ -199,10 +198,16 @@ export class MatIcon extends _MatIconMixinBase implements OnChanges, OnInit, Can const layoutElement: HTMLElement = this._elementRef.nativeElement; const childCount = layoutElement.childNodes.length; - // Remove existing child nodes and add the new SVG element. Note that we can't - // use innerHTML, because IE will throw if the element has a data binding. + // Remove existing non-element child nodes and SVGs, and add the new SVG element. Note that + // we can't use innerHTML, because IE will throw if the element has a data binding. for (let i = 0; i < childCount; i++) { - layoutElement.removeChild(layoutElement.childNodes[i]); + const child = layoutElement.childNodes[i]; + + // 1 corresponds to Node.ELEMENT_NODE. We remove all non-element nodes in order to get rid + // of any loose text nodes, as well as any SVG elements in order to remove any old icons. + if (child.nodeType !== 1 || child.nodeName.toLowerCase() === 'svg') { + layoutElement.removeChild(child); + } } }