Skip to content

Commit

Permalink
fix(icon): icon element not removed when svgIcon is reset (#6502)
Browse files Browse the repository at this point in the history
Fixes #6495.
  • Loading branch information
crisbeto authored and kara committed Aug 21, 2017
1 parent 5f75939 commit 5e3228f
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 16 deletions.
21 changes: 20 additions & 1 deletion src/lib/icon/icon.spec.ts
Expand Up @@ -331,6 +331,25 @@ describe('MdIcon', () => {
}).not.toThrow();
});

it('should remove the SVG element from the DOM when the binding is cleared', () => {
mdIconRegistry.addSvgIconSet(trust('arrow-set.svg'));

let fixture = TestBed.createComponent(IconFromSvgName);

const testComponent = fixture.componentInstance;
const icon = fixture.debugElement.nativeElement.querySelector('md-icon');

testComponent.iconName = 'left-arrow';
fixture.detectChanges();

expect(icon.querySelector('svg')).toBeTruthy();

testComponent.iconName = undefined;
fixture.detectChanges();

expect(icon.querySelector('svg')).toBeFalsy();
});

});

describe('custom fonts', () => {
Expand Down Expand Up @@ -418,7 +437,7 @@ class IconWithCustomFontCss {

@Component({template: `<md-icon [svgIcon]="iconName"></md-icon>`})
class IconFromSvgName {
iconName = '';
iconName: string | undefined = '';
}

@Component({template: '<md-icon aria-hidden="false">face</md-icon>'})
Expand Down
33 changes: 18 additions & 15 deletions src/lib/icon/icon.ts
Expand Up @@ -118,24 +118,24 @@ export class MdIcon extends _MdIconMixinBase implements OnChanges, OnInit, CanCo
}
const parts = iconName.split(':');
switch (parts.length) {
case 1:
// Use default namespace.
return ['', parts[0]];
case 2:
return <[string, string]>parts;
default:
throw Error(`Invalid icon name: "${iconName}"`);
case 1: return ['', parts[0]]; // Use default namespace.
case 2: return <[string, string]>parts;
default: throw Error(`Invalid icon name: "${iconName}"`);
}
}

ngOnChanges(changes: SimpleChanges) {
// Only update the inline SVG icon if the inputs changed, to avoid unnecessary DOM operations.
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 (changes.svgIcon) {
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}`));
} else {
this._clearSvgElement();
}
}

if (this._usingFontIcon()) {
Expand All @@ -156,6 +156,11 @@ export class MdIcon extends _MdIconMixinBase implements OnChanges, OnInit, CanCo
}

private _setSvgElement(svg: SVGElement) {
this._clearSvgElement();
this._renderer.appendChild(this._elementRef.nativeElement, svg);
}

private _clearSvgElement() {
const layoutElement = this._elementRef.nativeElement;
const childCount = layoutElement.childNodes.length;

Expand All @@ -164,8 +169,6 @@ export class MdIcon extends _MdIconMixinBase implements OnChanges, OnInit, CanCo
for (let i = 0; i < childCount; i++) {
this._renderer.removeChild(layoutElement, layoutElement.childNodes[i]);
}

this._renderer.appendChild(layoutElement, svg);
}

private _updateFontIconClasses() {
Expand Down

0 comments on commit 5e3228f

Please sign in to comment.