fix(module:icon): fix icons problems

* update dependency to reduce icons requests * fix class name not changes when `type` changes * add svg data-icon fix(module:form): fix icon style
NG-ZORRO · Oct 23, 2018 · 9fd9e97 · 9fd9e97
1 parent 9f1dafc
commit 9fd9e97
Show file tree

Hide file tree

Showing 16 changed files with 167 additions and 106 deletions.
diff --git a/components/button/nz-button.spec.ts b/components/button/nz-button.spec.ts
@@ -167,7 +167,7 @@ describe('button', () => {
       tick();
       fixture.detectChanges();
       expect(button.nativeElement.classList.contains('ant-btn-loading')).toBe(true);
-      expect(button.nativeElement.firstElementChild.classList.contains('anticon-spin')).toBe(true);
+      expect(button.nativeElement.firstElementChild.firstElementChild.classList.contains('anticon-spin')).toBe(true);
       expect(button.nativeElement.firstElementChild.classList.contains('anticon-loading')).toBe(true);
       expect(button.nativeElement.firstElementChild.localName).toBe('i');
       tick(5000);
@@ -179,15 +179,15 @@ describe('button', () => {
       const button = buttons[ 3 ];
       fixture.detectChanges();
       expect(button.nativeElement.classList.contains('ant-btn-loading')).toBe(false);
-      expect(button.nativeElement.firstElementChild.classList.contains('anticon-apin')).toBe(false);
+      expect(button.nativeElement.firstElementChild.querySelector('svg')).toBe(null);
       expect(button.nativeElement.firstElementChild.classList.contains('anticon-loading')).toBe(false);
       expect(button.nativeElement.firstElementChild.localName).toBe('i');
       button.nativeElement.click();
       fixture.detectChanges();
       tick();
       fixture.detectChanges();
       expect(button.nativeElement.classList.contains('ant-btn-loading')).toBe(true);
-      expect(button.nativeElement.firstElementChild.classList.contains('anticon-spin')).toBe(true);
+      expect(button.nativeElement.firstElementChild.firstElementChild.classList.contains('anticon-spin')).toBe(true);
       expect(button.nativeElement.firstElementChild.classList.contains('anticon-loading')).toBe(true);
       expect(button.nativeElement.firstElementChild.localName).toBe('i');
       tick(5000);

diff --git a/components/cascader/nz-cascader.component.ts b/components/cascader/nz-cascader.component.ts
@@ -173,19 +173,20 @@ export class NzCascaderComponent implements OnInit, OnDestroy, ControlValueAcces
     if (!this.inSearch && willBeInSearch) {
       this.oldActivatedOptions = this.activatedOptions;
       this.activatedOptions = [];
-      this.searchWidthStyle = `${this.input.nativeElement.offsetWidth}px`;
     } else if (this.inSearch && !willBeInSearch) {
       this.activatedOptions = this.oldActivatedOptions;
     }
 
     // 搜索状态变更之后
     this.inSearch = !!willBeInSearch;
     if (this.inSearch) {
+      this.labelRenderText = '';
       this.prepareSearchValue();
     } else {
       if (this.showSearch) {
         this.nzColumns = this.oldColumnsHolder;
       }
+      this.buildDisplayLabel();
       this.searchWidthStyle = '';
     }
     this.setClassMap();

diff --git a/components/cascader/nz-cascader.spec.ts b/components/cascader/nz-cascader.spec.ts
@@ -1377,7 +1377,9 @@ describe('cascader', () => {
     it('should support custom sorter', (done) => {
       testComponent.nzShowSearch = {
         sorter(a: CascaderOption[], b: CascaderOption[], inputValue: string): number {
-          return 1; // all reversed, just to be sure it works
+          const l1 = a[ 0 ].label;
+          const l2 = b[ 0 ].label; // all reversed, just to be sure it works
+          return ('' + l1).localeCompare(l2);
         }
       } as NzShowSearchOptions;
       fixture.detectChanges();
@@ -1914,7 +1916,7 @@ export class NzDemoCascaderDefaultComponent {
 
   fakeChangeOn = (node: any, index: number): boolean => {
     return node.value === 'zhejiang';
-  }
+  };
 
   clearSelection(): void {
     this.cascader.clearSelection();
@@ -1973,7 +1975,7 @@ export class NzDemoCascaderLoadDataComponent {
         }
       }, 500);
     });
-  }
+  };
 
   public addCallTimes(): void {
   }

diff --git a/components/form/nz-form-control.component.html b/components/form/nz-form-control.component.html
@@ -2,7 +2,7 @@
   <span class="ant-form-item-children">
     <ng-content></ng-content>
     <span class="ant-form-item-children-icon">
-      <i *ngIf="nzHasFeedback" nz-icon [type]="iconType"></i>
+      <i *ngIf="nzHasFeedback && iconType" nz-icon [type]="iconType"></i>
     </span>
   </span>
   <ng-content select="nz-form-explain"></ng-content>

diff --git a/components/icon/demo/basic.ts b/components/icon/demo/basic.ts
@@ -18,12 +18,12 @@ import { Component } from '@angular/core';
       <i class="anticon anticon-loading anticon-spin"></i>
     </div>
   `,
-  styles: [ `
+  styles  : [ `
     .icons-list > .anticon {
       margin-right: 6px;
       font-size: 24px;
     }
-  `]
+  ` ]
 })
 export class NzDemoIconBasicComponent {
 }
diff --git a/components/icon/nz-icon.directive.ts b/components/icon/nz-icon.directive.ts
@@ -12,6 +12,8 @@ import {
 import { IconDirective } from '@ant-design/icons-angular';
 import { NzIconService } from './nz-icon.service';
 
+const iconTypeRE = /^anticon\-\w/;
+
 /**
  * This directive extends IconDirective to provide:
  *
@@ -32,78 +34,113 @@ export class NzIconDirective extends IconDirective implements OnInit, OnChanges,
 
   /**
    * In order to make this directive compatible to old API, we had do some ugly stuff here.
-   * Should be removed in next major version.
+   * TODO: Should be removed in next major version.
    */
   private _classChangeHandler(className: string): void {
-    if (!className) { return; }
-
-    const forceSpin = className.indexOf('anticon-spin') > -1;
-    const classArr = className.split(/\s/);
-    let anticonType = classArr.filter(cls => cls !== 'anticon' && cls !== 'anticon-spin' && cls.match(/^anticon\-\w/))[ 0 ];
+    if (className) {
+      const iconType = className
+        .split(/\s/)
+        .filter(cls => cls !== 'anticon' && cls !== 'anticon-spin' && !!cls.match(iconTypeRE))[ 0 ];
 
-    if (!anticonType) { return; }
+      if (!iconType) {
+        return;
+      }
 
-    anticonType = anticonType.replace('anticon-', '');
-    if (anticonType.includes('verticle')) {
-      anticonType = anticonType.replace('verticle', 'vertical');
-      if (!this._iconService.warnedAboutVertical) {
-        console.warn('[NG-ZORRO]', `'verticle' is misspelled, would be corrected in the next major version.`);
-        this._iconService.warnedAboutVertical = true;
+      let parsedIconType = iconType.replace('anticon-', '');
+      if (parsedIconType.includes('verticle')) {
+        parsedIconType = parsedIconType.replace('verticle', 'vertical');
+        this._warnAPI('cross');
       }
-    }
-    if (anticonType.startsWith('cross')) {
-      anticonType = anticonType.replace('cross', 'close');
-      if (!this._iconService.warnedAboutCross) {
-        console.warn('[NG-ZORRO]', `'cross' icon is replaced by 'close' icon.`);
-        this._iconService.warnedAboutCross = true;
+      if (parsedIconType.startsWith('cross')) {
+        parsedIconType = parsedIconType.replace('cross', 'close');
+        this._warnAPI('vertical');
       }
-    }
-    if (!(anticonType.endsWith('-o') || anticonType.endsWith('-fill') || anticonType.endsWith('-twotone'))) {
-      anticonType += '-o';
-    }
 
-    if (this.type !== anticonType) {
-      this.type = anticonType;
-      this._changeIcon().catch(err => {
+      // Only change icon when icon type does change.
+      if (this.type !== parsedIconType) {
+        this.type = parsedIconType;
+        this._changeIcon().catch(err => {
           console.warn('[NG-ZORRO]', `You can find more about this error on http://ng.ant.design/components/icon/en\n`, err);
         });
+      }
     }
   }
 
-  private _warnAPI(): void {
-    if (isDevMode() && !this._iconService.warnedAboutAPI) {
-      console.warn('[NG-ZORRO]', `<i class="anticon"></i> would be deprecated soon. Please use <i nz-icon type=""></i> API.`);
+  /**
+   * In order to make this directive compatible to old API, we had do some ugly stuff here.
+   * TODO: Should be removed in next major version.
+   */
+  private _warnAPI(type: 'old' | 'cross' | 'vertical'): void {
+    if (isDevMode()) {
+      if (type === 'old' && !this._iconService.warnedAboutAPI) {
+        console.warn('[NG-ZORRO]', `<i class="anticon"></i> would be deprecated soon. Please use <i nz-icon type=""></i> API.`);
+        this._iconService.warnedAboutAPI = true;
+      }
+      if (type === 'cross' && !this._iconService.warnedAboutCross) {
+        console.warn('[NG-ZORRO]', `'cross' icon is replaced by 'close' icon.`);
+        this._iconService.warnedAboutCross = true;
+      }
+      if (type === 'vertical' && !this._iconService.warnedAboutVertical) {
+        console.warn('[NG-ZORRO]', `'verticle' is misspelled, would be corrected in the next major version.`);
+        this._iconService.warnedAboutVertical = true;
+      }
     }
-    this._iconService.warnedAboutAPI = true;
   }
 
-  private _addExtraModifications(svg: SVGElement): void {
-    if (this.spin || this.type === 'loading') {
-      this._renderer.addClass(this._el, 'anticon-spin');
+  private _toggleSpin(svg: SVGElement): void {
+    if ((this.spin || this.type === 'loading') && !this._el.classList.contains('anticon-spin')) {
+      this._renderer.addClass(svg, 'anticon-spin');
     } else {
-      this._renderer.removeClass(this._el, 'anticon-spin');
+      this._renderer.removeClass(svg, 'anticon-spin');
+    }
+  }
+
+  private _setClassName(): void {
+    // If there's not an anticon class, usually a new API icon, get the icon class name back.
+    // anticon should be added before other class names.
+    if (this._el && typeof this.type === 'string') {
+      const iconClassNameArr = this._el.className.split(/\s/);
+      const oldTypeNameIndex = iconClassNameArr.findIndex(cls => cls !== 'anticon' && cls !== 'anticon-spin' && !!cls.match(iconTypeRE));
+
+      if (oldTypeNameIndex !== -1) {
+        iconClassNameArr.splice(oldTypeNameIndex, 1, `anticon-${this.type}`);
+        this._renderer.setAttribute(this._el, 'class', iconClassNameArr.join(' '));
+      } else {
+        this._renderer.addClass(this._el, `anticon-${this.type}`);
+      }
     }
   }
 
-  private _getIconNameBack(): void {
+  private _setSVGData(svg: SVGElement): void {
     if (typeof this.type === 'string') {
-      this._renderer.addClass(this._elementRef.nativeElement, `anticon-${this.type}`);
+      this._renderer.setAttribute(svg, 'data-icon', this.type);
+      this._renderer.setAttribute(svg, 'aria-hidden', 'true');
     }
   }
 
+  private _addExtraModifications(svg: SVGElement): void {
+    this._toggleSpin(svg);
+    this._setSVGData(svg);
+  }
+
   constructor(public _iconService: NzIconService, public _elementRef: ElementRef, public _renderer: Renderer2) {
-    super(_iconService, _elementRef, _renderer); // NzIconService extends IconService so IconDirective won't complain.
+    super(_iconService, _elementRef, _renderer);
   }
 
   ngOnChanges(): void {
     if (!this.iconfont) {
-      this._getIconNameBack(); // Should get back classNames immediately.
+      // For ant design icons.
+      this._setClassName();
       this._changeIcon().then(svg => {
         this._addExtraModifications(svg);
-      }).catch(() => {
-        console.warn('[NG-ZORRO]', `You can find more about this error on http://ng.ant.design/components/icon/en`);
+      }).catch((err) => {
+        if (err) {
+          console.error(err);
+          console.warn('[NG-ZORRO]', `You can find more about this error on http://ng.ant.design/components/icon/en`);
+        }
       });
     } else {
+      // For iconfont icons.
       this._setSVGElement(this._iconService.createIconfontIcon(`#${this.iconfont}`));
     }
   }
@@ -113,8 +150,10 @@ export class NzIconDirective extends IconDirective implements OnInit, OnChanges,
    */
   ngOnInit(): void {
     this._el = this._elementRef.nativeElement;
+
+    // Make the component compatible to old class="anticon" API.
     if (this._el && !this.type) {
-      this._warnAPI();
+      this._warnAPI('old');
       this._classChangeHandler(this._el.className);
       this._classNameObserver = new MutationObserver((mutations: MutationRecord[]) => {
         mutations
@@ -127,6 +166,10 @@ export class NzIconDirective extends IconDirective implements OnInit, OnChanges,
     if (!this._el.classList.contains('anticon')) {
       this._renderer.setAttribute(this._el, 'class', `anticon ${this._el.className}`);
     }
+
+    if (this.type) {
+      this._setClassName();
+    }
   }
 
   ngOnDestroy(): void {
@@ -141,8 +184,7 @@ export class NzIconDirective extends IconDirective implements OnInit, OnChanges,
   ngAfterContentChecked(): void {
     const children = (this._elementRef.nativeElement as HTMLElement).children;
     if (children && children.length && !this.type) {
-      const child = children[ 0 ];
-      this._iconService.normalizeSvgElement(child as SVGElement);
+      this._iconService.normalizeSvgElement(children[ 0 ] as SVGElement);
     }
   }
 }
diff --git a/components/icon/nz-icon.service.ts b/components/icon/nz-icon.service.ts
@@ -69,8 +69,7 @@ export class NzIconService extends IconService {
   }
 
   createIconfontIcon(type: string): SVGElement {
-    const svgString = `<svg><use xlink:href="${type}"></svg>`;
-    return this._createSVGElementFromString(svgString);
+    return this._createSVGElementFromString(`<svg><use xlink:href="${type}"></svg>`);
   }
 
   constructor(

diff --git a/components/icon/nz-icon.spec.ts b/components/icon/nz-icon.spec.ts
@@ -28,27 +28,43 @@ describe('icon', () => {
 
     it('should get icon class name back', fakeAsync(() => {
       fixture.detectChanges();
-      expect(icons[ 0 ].nativeElement.classList.contains('anticon-question'));
-      expect(icons[ 1 ].nativeElement.classList.contains('anticon-loading'));
+      tick(1000);
+      fixture.detectChanges();
+      expect(icons[ 0 ].nativeElement.classList.contains('anticon')).toBe(true);
+      expect(icons[ 0 ].nativeElement.classList.contains('anticon-question')).toBe(true);
+      expect(icons[ 1 ].nativeElement.classList.contains('anticon-loading')).toBe(true);
+    }));
+
+    it('should change class name when type changes', fakeAsync(() => {
+      fixture.detectChanges();
+      tick(1000);
+      fixture.detectChanges();
+      testComponent.type = 'question-circle';
+      fixture.detectChanges();
+      tick(1000);
+      fixture.detectChanges();
+      expect(icons[ 0 ].nativeElement.classList.contains('anticon')).toBe(true);
+      expect(icons[ 0 ].nativeElement.classList.contains('anticon-question-circle')).toBe(true);
+      expect(icons[ 0 ].nativeElement.classList.contains('anticon-question')).not.toBe(true);
     }));
 
     it('should support spin and cancel', fakeAsync(() => {
       fixture.detectChanges();
       tick(1000);
       fixture.detectChanges();
-      expect(icons[ 0 ].nativeElement.classList.contains('anticon-spin')).toBe(true);
+      expect(icons[ 0 ].nativeElement.firstChild.classList.contains('anticon-spin')).toBe(true);
       testComponent.spin = false;
       fixture.detectChanges();
       tick(1000);
       fixture.detectChanges();
-      expect(icons[ 0 ].nativeElement.classList.contains('anticon-spin')).toBe(false);
+      expect(icons[ 0 ].nativeElement.firstChild.classList.contains('anticon-spin')).toBe(false);
     }));
 
     it('should make loading spin', fakeAsync(() => {
       fixture.detectChanges();
       tick(1000);
       fixture.detectChanges();
-      expect(icons[ 1 ].nativeElement.classList.contains('anticon-spin')).toBe(true);
+      expect(icons[ 1 ].nativeElement.firstChild.classList.contains('anticon-spin')).toBe(true);
     }));
   });
 

diff --git a/components/layout/nz-layout.module.ts b/components/layout/nz-layout.module.ts
@@ -2,6 +2,7 @@ import { CommonModule } from '@angular/common';
 import { NgModule } from '@angular/core';
 
 import { NzMatchMediaService } from '../core/services/nz-match-media.service';
+import { NzIconModule } from '../icon/nz-icon.module';
 
 import { NzContentComponent } from './nz-content.component';
 import { NzFooterComponent } from './nz-footer.component';
@@ -13,7 +14,7 @@ import { NzSiderComponent } from './nz-sider.component';
   declarations: [ NzLayoutComponent, NzHeaderComponent, NzContentComponent, NzFooterComponent, NzSiderComponent ],
   exports     : [ NzLayoutComponent, NzHeaderComponent, NzContentComponent, NzFooterComponent, NzSiderComponent ],
   providers   : [ NzMatchMediaService ],
-  imports     : [ CommonModule ]
+  imports     : [ CommonModule, NzIconModule ]
 })
 export class NzLayoutModule {
 }
diff --git a/components/layout/nz-sider.component.html b/components/layout/nz-sider.component.html
@@ -8,6 +8,6 @@
   <ng-template [ngTemplateOutlet]="nzTrigger"></ng-template>
 </div>
 <ng-template #defaultTrigger>
-  <i class="anticon" [class.anticon-left]="!nzCollapsed" [class.anticon-right]="nzCollapsed" *ngIf="!nzReverseArrow"></i>
-  <i class="anticon" [class.anticon-left]="nzCollapsed" [class.anticon-right]="!nzCollapsed" *ngIf="nzReverseArrow"></i>
+  <i nz-icon [type]="nzCollapsed ? 'right' : 'left'" *ngIf="!nzReverseArrow"></i>
+  <i nz-icon [type]="nzCollapsed ? 'left' : 'right'" *ngIf="nzReverseArrow"></i>
 </ng-template>
diff --git a/components/package.json b/components/package.json
@@ -23,7 +23,7 @@
   "dependencies": {
     "date-fns": "^1.29.0",
     "@angular/cdk": "^6.0.0",
-    "@ant-design/icons-angular": "^0.0.2"
+    "@ant-design/icons-angular": "^0.0.3"
   },
   "peerDependencies": {
     "@angular/animations": "^6.0.0",