From 07954325a21dc45ef1975f889a3fda7d7233541b Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Thu, 3 Aug 2017 01:14:36 +0200 Subject: [PATCH] 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 --- src/lib/icon/icon.spec.ts | 24 ++++++++++++++++++++++++ src/lib/icon/icon.ts | 39 +++++++++++++++++++-------------------- 2 files changed, 43 insertions(+), 20 deletions(-) diff --git a/src/lib/icon/icon.spec.ts b/src/lib/icon/icon.spec.ts index 1ae567cf825f..7e3adb65945a 100644 --- a/src/lib/icon/icon.spec.ts +++ b/src/lib/icon/icon.spec.ts @@ -47,6 +47,7 @@ describe('MdIcon', () => { IconWithCustomFontCss, IconFromSvgName, IconWithAriaHiddenFalse, + IconWithBindingAndNgIf, ], providers: [ MockBackend, @@ -313,6 +314,23 @@ describe('MdIcon', () => { verifyPathChildElement(svgElement, 'left'); expect(svgElement.getAttribute('viewBox')).toBeFalsy(); }); + + it('should not throw when toggling an icon that has a binding in IE11', () => { + mdIconRegistry.addSvgIcon('fluffy', trust('cat.svg')); + + const fixture = TestBed.createComponent(IconWithBindingAndNgIf); + + fixture.detectChanges(); + + expect(() => { + fixture.componentInstance.showIcon = false; + fixture.detectChanges(); + + fixture.componentInstance.showIcon = true; + fixture.detectChanges(); + }).not.toThrow(); + }); + }); describe('custom fonts', () => { @@ -405,3 +423,9 @@ class IconFromSvgName { @Component({template: 'face'}) class IconWithAriaHiddenFalse { } + +@Component({template: `{{iconName}}`}) +class IconWithBindingAndNgIf { + iconName = 'fluffy'; + showIcon = true; +} diff --git a/src/lib/icon/icon.ts b/src/lib/icon/icon.ts index a110851269e1..0e6832c90b0b 100644 --- a/src/lib/icon/icon.ts +++ b/src/lib/icon/icon.ts @@ -14,7 +14,7 @@ import { OnChanges, OnInit, Renderer2, - SimpleChange, + SimpleChanges, ViewEncapsulation, Attribute, } from '@angular/core'; @@ -33,12 +33,6 @@ export const _MdIconMixinBase = mixinColor(MdIconBase); /** * Component to display an icon. It can be used in the following ways: - * - Specify the svgSrc input to load an SVG icon from a URL. The SVG content is directly inlined - * as a child of the component, so that CSS styles can easily be applied to it. - * The URL is loaded via an XMLHttpRequest, so it must be on the same domain as the page or its - * server must be configured to allow cross-domain requests. - * Example: - * * * - Specify the svgIcon input to load an SVG icon from a URL previously registered with the * addSvgIcon, addSvgIconInNamespace, addSvgIconSet, or addSvgIconSetInNamespace methods of @@ -134,17 +128,16 @@ export class MdIcon extends _MdIconMixinBase implements OnChanges, OnInit, CanCo } } - ngOnChanges(changes: {[propertyName: string]: SimpleChange}) { - const changedInputs = Object.keys(changes); + ngOnChanges(changes: SimpleChanges) { // Only update the inline SVG icon if the inputs changed, to avoid unnecessary DOM operations. - if (changedInputs.indexOf('svgIcon') != -1 || changedInputs.indexOf('svgSrc') != -1) { - if (this.svgIcon) { - const [namespace, iconName] = this._splitIconName(this.svgIcon); - first.call(this._mdIconRegistry.getNamedSvgIcon(iconName, namespace)).subscribe( - svg => this._setSvgElement(svg), - (err: Error) => console.log(`Error retrieving icon: ${err.message}`)); - } + if (changes.svgIcon && this.svgIcon) { + const [namespace, iconName] = this._splitIconName(this.svgIcon); + + first.call(this._mdIconRegistry.getNamedSvgIcon(iconName, namespace)).subscribe( + svg => this._setSvgElement(svg), + (err: Error) => console.log(`Error retrieving icon: ${err.message}`)); } + if (this._usingFontIcon()) { this._updateFontIconClasses(); } @@ -164,10 +157,14 @@ export class MdIcon extends _MdIconMixinBase implements OnChanges, OnInit, CanCo private _setSvgElement(svg: SVGElement) { const layoutElement = this._elementRef.nativeElement; - // Remove existing child nodes and add the new SVG element. - // We would use renderer.detachView(Array.from(layoutElement.childNodes)) here, - // but it fails in IE11: https://github.com/angular/angular/issues/6327 - layoutElement.innerHTML = ''; + 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. + for (let i = 0; i < childCount; i++) { + this._renderer.removeChild(layoutElement, layoutElement.childNodes[i]); + } + this._renderer.appendChild(layoutElement, svg); } @@ -175,10 +172,12 @@ export class MdIcon extends _MdIconMixinBase implements OnChanges, OnInit, CanCo if (!this._usingFontIcon()) { return; } + const elem = this._elementRef.nativeElement; const fontSetClass = this.fontSet ? this._mdIconRegistry.classNameForFontAlias(this.fontSet) : this._mdIconRegistry.getDefaultFontSetClass(); + if (fontSetClass != this._previousFontSetClass) { if (this._previousFontSetClass) { this._renderer.removeClass(elem, this._previousFontSetClass);