fix(icon): icon element not removed when svgIcon is reset

Fixes the underlying SVG element not being removed when the `svgIcon` binding is set to undefined. Fixes angular#6495.
crisbeto · Aug 16, 2017 · 3cb73da · 3cb73da
1 parent b5fc68b
commit 3cb73da
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 16 deletions.
diff --git a/src/lib/icon/icon.spec.ts b/src/lib/icon/icon.spec.ts
@@ -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', () => {
@@ -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>'})

diff --git a/src/lib/icon/icon.ts b/src/lib/icon/icon.ts
@@ -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()) {
@@ -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;
 
@@ -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() {