Skip to content

Commit

Permalink
fix(icon): error when toggling icon with binding in IE11 (#6102)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
crisbeto authored and tinayuangao committed Aug 2, 2017
1 parent 49a0d7b commit 0795432
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 20 deletions.
24 changes: 24 additions & 0 deletions src/lib/icon/icon.spec.ts
Expand Up @@ -47,6 +47,7 @@ describe('MdIcon', () => {
IconWithCustomFontCss,
IconFromSvgName,
IconWithAriaHiddenFalse,
IconWithBindingAndNgIf,
],
providers: [
MockBackend,
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -405,3 +423,9 @@ class IconFromSvgName {

@Component({template: '<md-icon aria-hidden="false">face</md-icon>'})
class IconWithAriaHiddenFalse { }

@Component({template: `<md-icon [svgIcon]="iconName" *ngIf="showIcon">{{iconName}}</md-icon>`})
class IconWithBindingAndNgIf {
iconName = 'fluffy';
showIcon = true;
}
39 changes: 19 additions & 20 deletions src/lib/icon/icon.ts
Expand Up @@ -14,7 +14,7 @@ import {
OnChanges,
OnInit,
Renderer2,
SimpleChange,
SimpleChanges,
ViewEncapsulation,
Attribute,
} from '@angular/core';
Expand All @@ -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 <md-icon> 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:
* <md-icon svgSrc="assets/arrow.svg"></md-icon>
*
* - Specify the svgIcon input to load an SVG icon from a URL previously registered with the
* addSvgIcon, addSvgIconInNamespace, addSvgIconSet, or addSvgIconSetInNamespace methods of
Expand Down Expand Up @@ -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();
}
Expand All @@ -164,21 +157,27 @@ 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);
}

private _updateFontIconClasses() {
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);
Expand Down

0 comments on commit 0795432

Please sign in to comment.