Skip to content

Commit 0795432

Browse files
crisbetotinayuangao
authored andcommitted
fix(icon): error when toggling icon with binding in IE11 (#6102)
* fix(icon): error when toggling icon with binding in IE11 * Fixes an error that was being logged by IE11 for icons that are toggled via `ngIf` and have a binding expression. * Some minor readability improvements in the icon component. Fixes #6093. * fix: address feedback
1 parent 49a0d7b commit 0795432

File tree

2 files changed

+43
-20
lines changed

2 files changed

+43
-20
lines changed

src/lib/icon/icon.spec.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ describe('MdIcon', () => {
4747
IconWithCustomFontCss,
4848
IconFromSvgName,
4949
IconWithAriaHiddenFalse,
50+
IconWithBindingAndNgIf,
5051
],
5152
providers: [
5253
MockBackend,
@@ -313,6 +314,23 @@ describe('MdIcon', () => {
313314
verifyPathChildElement(svgElement, 'left');
314315
expect(svgElement.getAttribute('viewBox')).toBeFalsy();
315316
});
317+
318+
it('should not throw when toggling an icon that has a binding in IE11', () => {
319+
mdIconRegistry.addSvgIcon('fluffy', trust('cat.svg'));
320+
321+
const fixture = TestBed.createComponent(IconWithBindingAndNgIf);
322+
323+
fixture.detectChanges();
324+
325+
expect(() => {
326+
fixture.componentInstance.showIcon = false;
327+
fixture.detectChanges();
328+
329+
fixture.componentInstance.showIcon = true;
330+
fixture.detectChanges();
331+
}).not.toThrow();
332+
});
333+
316334
});
317335

318336
describe('custom fonts', () => {
@@ -405,3 +423,9 @@ class IconFromSvgName {
405423

406424
@Component({template: '<md-icon aria-hidden="false">face</md-icon>'})
407425
class IconWithAriaHiddenFalse { }
426+
427+
@Component({template: `<md-icon [svgIcon]="iconName" *ngIf="showIcon">{{iconName}}</md-icon>`})
428+
class IconWithBindingAndNgIf {
429+
iconName = 'fluffy';
430+
showIcon = true;
431+
}

src/lib/icon/icon.ts

Lines changed: 19 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import {
1414
OnChanges,
1515
OnInit,
1616
Renderer2,
17-
SimpleChange,
17+
SimpleChanges,
1818
ViewEncapsulation,
1919
Attribute,
2020
} from '@angular/core';
@@ -33,12 +33,6 @@ export const _MdIconMixinBase = mixinColor(MdIconBase);
3333

3434
/**
3535
* Component to display an icon. It can be used in the following ways:
36-
* - Specify the svgSrc input to load an SVG icon from a URL. The SVG content is directly inlined
37-
* as a child of the <md-icon> component, so that CSS styles can easily be applied to it.
38-
* The URL is loaded via an XMLHttpRequest, so it must be on the same domain as the page or its
39-
* server must be configured to allow cross-domain requests.
40-
* Example:
41-
* <md-icon svgSrc="assets/arrow.svg"></md-icon>
4236
*
4337
* - Specify the svgIcon input to load an SVG icon from a URL previously registered with the
4438
* addSvgIcon, addSvgIconInNamespace, addSvgIconSet, or addSvgIconSetInNamespace methods of
@@ -134,17 +128,16 @@ export class MdIcon extends _MdIconMixinBase implements OnChanges, OnInit, CanCo
134128
}
135129
}
136130

137-
ngOnChanges(changes: {[propertyName: string]: SimpleChange}) {
138-
const changedInputs = Object.keys(changes);
131+
ngOnChanges(changes: SimpleChanges) {
139132
// Only update the inline SVG icon if the inputs changed, to avoid unnecessary DOM operations.
140-
if (changedInputs.indexOf('svgIcon') != -1 || changedInputs.indexOf('svgSrc') != -1) {
141-
if (this.svgIcon) {
142-
const [namespace, iconName] = this._splitIconName(this.svgIcon);
143-
first.call(this._mdIconRegistry.getNamedSvgIcon(iconName, namespace)).subscribe(
144-
svg => this._setSvgElement(svg),
145-
(err: Error) => console.log(`Error retrieving icon: ${err.message}`));
146-
}
133+
if (changes.svgIcon && this.svgIcon) {
134+
const [namespace, iconName] = this._splitIconName(this.svgIcon);
135+
136+
first.call(this._mdIconRegistry.getNamedSvgIcon(iconName, namespace)).subscribe(
137+
svg => this._setSvgElement(svg),
138+
(err: Error) => console.log(`Error retrieving icon: ${err.message}`));
147139
}
140+
148141
if (this._usingFontIcon()) {
149142
this._updateFontIconClasses();
150143
}
@@ -164,21 +157,27 @@ export class MdIcon extends _MdIconMixinBase implements OnChanges, OnInit, CanCo
164157

165158
private _setSvgElement(svg: SVGElement) {
166159
const layoutElement = this._elementRef.nativeElement;
167-
// Remove existing child nodes and add the new SVG element.
168-
// We would use renderer.detachView(Array.from(layoutElement.childNodes)) here,
169-
// but it fails in IE11: https://github.com/angular/angular/issues/6327
170-
layoutElement.innerHTML = '';
160+
const childCount = layoutElement.childNodes.length;
161+
162+
// Remove existing child nodes and add the new SVG element. Note that we can't
163+
// use innerHTML, because IE will throw if the element has a data binding.
164+
for (let i = 0; i < childCount; i++) {
165+
this._renderer.removeChild(layoutElement, layoutElement.childNodes[i]);
166+
}
167+
171168
this._renderer.appendChild(layoutElement, svg);
172169
}
173170

174171
private _updateFontIconClasses() {
175172
if (!this._usingFontIcon()) {
176173
return;
177174
}
175+
178176
const elem = this._elementRef.nativeElement;
179177
const fontSetClass = this.fontSet ?
180178
this._mdIconRegistry.classNameForFontAlias(this.fontSet) :
181179
this._mdIconRegistry.getDefaultFontSetClass();
180+
182181
if (fontSetClass != this._previousFontSetClass) {
183182
if (this._previousFontSetClass) {
184183
this._renderer.removeClass(elem, this._previousFontSetClass);

0 commit comments

Comments
 (0)