Skip to content

Commit c75a562

Browse files
committed
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.
1 parent 4166d16 commit c75a562

File tree

2 files changed

+43
-4
lines changed

2 files changed

+43
-4
lines changed

src/lib/icon/icon.spec.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ describe('MatIcon', () => {
5151
IconWithAriaHiddenFalse,
5252
IconWithBindingAndNgIf,
5353
InlineIcon,
54+
SvgIconWithUserContent,
5455
]
5556
});
5657

@@ -397,6 +398,26 @@ describe('MatIcon', () => {
397398

398399
expect(icon.querySelector('svg')).toBeFalsy();
399400
});
401+
402+
it('should keep non-SVG user content inside the icon element', fakeAsync(() => {
403+
iconRegistry.addSvgIcon('fido', trustUrl('dog.svg'));
404+
405+
const fixture = TestBed.createComponent(SvgIconWithUserContent);
406+
const testComponent = fixture.componentInstance;
407+
const iconElement = fixture.debugElement.nativeElement.querySelector('mat-icon');
408+
409+
testComponent.iconName = 'fido';
410+
fixture.detectChanges();
411+
http.expectOne('dog.svg').flush(FAKE_SVGS.dog);
412+
413+
const userDiv = iconElement.querySelector('div');
414+
415+
expect(userDiv).toBeTruthy();
416+
expect(iconElement.textContent.trim()).toContain('Hello');
417+
418+
tick();
419+
}));
420+
400421
});
401422

402423
describe('Icons from HTML string', () => {
@@ -706,3 +727,8 @@ class IconWithBindingAndNgIf {
706727
class InlineIcon {
707728
inline = false;
708729
}
730+
731+
@Component({template: `<mat-icon [svgIcon]="iconName"><div>Hello</div></mat-icon>`})
732+
class SvgIconWithUserContent {
733+
iconName: string | undefined = '';
734+
}

src/lib/icon/icon.ts

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,13 @@ import {
1717
OnInit,
1818
SimpleChanges,
1919
ViewEncapsulation,
20+
Inject,
21+
Optional,
2022
} from '@angular/core';
2123
import {CanColor, mixinColor} from '@angular/material/core';
2224
import {coerceBooleanProperty} from '@angular/cdk/coercion';
2325
import {MatIconRegistry} from './icon-registry';
26+
import {DOCUMENT} from '@angular/platform-browser';
2427

2528

2629
// Boilerplate for applying mixins to MatIcon.
@@ -74,6 +77,7 @@ export const _MatIconMixinBase = mixinColor(MatIconBase);
7477
changeDetection: ChangeDetectionStrategy.OnPush,
7578
})
7679
export class MatIcon extends _MatIconMixinBase implements OnChanges, OnInit, CanColor {
80+
private _document: Document;
7781

7882
/**
7983
* Whether the icon should be inlined, automatically sizing the icon to match the font size of
@@ -113,9 +117,13 @@ export class MatIcon extends _MatIconMixinBase implements OnChanges, OnInit, Can
113117
constructor(
114118
elementRef: ElementRef,
115119
private _iconRegistry: MatIconRegistry,
116-
@Attribute('aria-hidden') ariaHidden: string) {
120+
@Attribute('aria-hidden') ariaHidden: string,
121+
/** @deletion-target 8.0.0 `document` parameter to be made required. */
122+
@Optional() @Inject(DOCUMENT) document?: any) {
117123
super(elementRef);
118124

125+
this._document = document;
126+
119127
// If the user has not explicitly set aria-hidden, mark the icon as hidden, as this is
120128
// the right thing to do for the majority of icon use-cases.
121129
if (!ariaHidden) {
@@ -198,11 +206,16 @@ export class MatIcon extends _MatIconMixinBase implements OnChanges, OnInit, Can
198206
private _clearSvgElement() {
199207
const layoutElement: HTMLElement = this._elementRef.nativeElement;
200208
const childCount = layoutElement.childNodes.length;
209+
const elementNodeType = this._document ? this._document.ELEMENT_NODE : 1;
201210

202-
// Remove existing child nodes and add the new SVG element. Note that we can't
203-
// use innerHTML, because IE will throw if the element has a data binding.
211+
// Remove existing non-element child nodes and SVGs, and add the new SVG element. Note that
212+
// we can't use innerHTML, because IE will throw if the element has a data binding.
204213
for (let i = 0; i < childCount; i++) {
205-
layoutElement.removeChild(layoutElement.childNodes[i]);
214+
const child = layoutElement.childNodes[i];
215+
216+
if (child.nodeType !== elementNodeType || child.nodeName.toLowerCase() === 'svg') {
217+
layoutElement.removeChild(child);
218+
}
206219
}
207220
}
208221

0 commit comments

Comments
 (0)