fix(module:transfer): removed change detection (#1260)

close #996
NG-ZORRO · Apr 4, 2018 · d507848 · d507848
1 parent c9686ca
commit d507848
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 19 deletions.
diff --git a/components/transfer/nz-transfer-list.component.ts b/components/transfer/nz-transfer-list.component.ts
@@ -24,7 +24,7 @@ import { TransferItem } from './interface';
   providers          : [ NzUpdateHostClassService ],
   template           : `
     <div class="ant-transfer-list-header">
-      <label nz-checkbox [(ngModel)]="stat.checkAll" (ngModelChange)="onHandleSelectAll($event)"
+      <label nz-checkbox [ngModel]="stat.checkAll" (ngModelChange)="onHandleSelectAll($event)"
         [nzIndeterminate]="stat.checkHalf">
         <span class="ant-transfer-list-header-selected">
           <span>{{ (stat.checkCount > 0 ? stat.checkCount + '/' : '') + stat.shownCount }} {{ dataSource.length > 1 ? itemsUnit : itemUnit }}</span>
@@ -45,10 +45,8 @@ import { TransferItem } from './interface';
         <ng-container *ngFor="let item of dataSource">
           <li *ngIf="!item._hiden" (click)="_handleSelect(item)" class="ant-transfer-list-content-item">
             <label nz-checkbox [ngModel]="item.checked" [nzDisabled]="item.disabled">
-              <span>
-                <ng-container *ngIf="!render; else renderContainer">{{ item.title }}</ng-container>
-                <ng-template #renderContainer [ngTemplateOutlet]="render" [ngTemplateOutletContext]="{ $implicit: item }"></ng-template>
-              </span>
+              <ng-container *ngIf="!render; else renderContainer">{{ item.title }}</ng-container>
+              <ng-template #renderContainer [ngTemplateOutlet]="render" [ngTemplateOutletContext]="{ $implicit: item }"></ng-template>
             </label>
           </li>
         </ng-container>

diff --git a/components/transfer/nz-transfer.component.ts b/components/transfer/nz-transfer.component.ts
@@ -63,8 +63,7 @@ import { TransferCanMove, TransferChange, TransferItem, TransferSearchChange, Tr
   `,
   host               : {
     '[class.ant-transfer]': 'true'
-  },
-  changeDetection    : ChangeDetectionStrategy.OnPush
+  }
 })
 export class NzTransferComponent implements OnChanges {
   private _showSearch = false;
@@ -143,7 +142,6 @@ export class NzTransferComponent implements OnChanges {
 
   handleFilterChange(ret: { direction: string, value: string }): void {
     this.nzSearchChange.emit(ret);
-    this.cd.detectChanges();
   }
 
   // endregion
@@ -155,7 +153,6 @@ export class NzTransferComponent implements OnChanges {
 
   private updateOperationStatus(direction: string, count?: number): void {
     this[ direction === 'right' ? 'leftActive' : 'rightActive' ] = (typeof count === 'undefined' ? this.getCheckedData(direction).filter(w => !w.disabled).length : count) > 0;
-    this.cd.detectChanges();
   }
 
   moveToLeft = () => this.moveTo('left');
@@ -192,7 +189,7 @@ export class NzTransferComponent implements OnChanges {
 
   // endregion
 
-  constructor(private i18n: NzI18nService, private el: ElementRef, private cd: ChangeDetectorRef) {
+  constructor(private i18n: NzI18nService, private el: ElementRef) {
   }
 
   ngOnChanges(changes: SimpleChanges): void {
@@ -201,6 +198,5 @@ export class NzTransferComponent implements OnChanges {
       this.updateOperationStatus('left');
       this.updateOperationStatus('right');
     }
-    this.cd.detectChanges();
   }
 }
diff --git a/components/transfer/transfer.spec.ts b/components/transfer/transfer.spec.ts
@@ -1,6 +1,6 @@
 // tslint:disable:no-any no-parameter-reassignment
 import { Component, DebugElement, Injector, OnInit, ViewChild, ViewEncapsulation } from '@angular/core';
-import { ComponentFixture, TestBed } from '@angular/core/testing';
+import { fakeAsync, tick, ComponentFixture, TestBed } from '@angular/core/testing';
 import { By } from '@angular/platform-browser';
 import { NoopAnimationsModule } from '@angular/platform-browser/animations';
 import { Observable } from 'rxjs/Observable';
@@ -15,14 +15,14 @@ const LEFTCOUNT = 2;
 
 describe('transfer', () => {
   let injector: Injector;
-  let fixture: ComponentFixture<TestTransferComponent | TestTransferCustomRenderComponent>;
+  let fixture: ComponentFixture<TestTransferComponent | TestTransferCustomRenderComponent | Test996Component>;
   let dl: DebugElement;
   let instance: TestTransferComponent;
   let pageObject: TransferPageObject;
   beforeEach(() => {
     injector = TestBed.configureTestingModule({
       imports: [NoopAnimationsModule, NzTransferModule],
-      declarations: [TestTransferComponent, TestTransferCustomRenderComponent]
+      declarations: [TestTransferComponent, TestTransferCustomRenderComponent, Test996Component]
     });
     fixture = TestBed.createComponent(TestTransferComponent);
     dl = fixture.debugElement;
@@ -72,13 +72,15 @@ describe('transfer', () => {
       pageObject.expectLeft(LEFTCOUNT).search('left', 'description of content1');
       expect(pageObject.leftList.querySelectorAll('.ant-transfer-list-content-item').length).toBe(1);
       (pageObject.leftList.querySelector('.ant-transfer-list-search-action') as HTMLElement).click();
+      fixture.detectChanges();
       expect(pageObject.leftList.querySelectorAll('.ant-transfer-list-content-item').length).toBe(LEFTCOUNT);
     });
 
     it('should be clear search keywords', () => {
       pageObject.expectLeft(LEFTCOUNT).search('left', '1');
       expect(pageObject.leftList.querySelectorAll('.ant-transfer-list-content-item').length).toBe(1);
       (pageObject.leftList.querySelector('.ant-transfer-list-search-action') as HTMLElement).click();
+      fixture.detectChanges();
       expect(pageObject.leftList.querySelectorAll('.ant-transfer-list-content-item').length).toBe(LEFTCOUNT);
     });
 
@@ -170,15 +172,26 @@ describe('transfer', () => {
     });
   });
 
-  xdescribe('#issues', () => {
-    xit('#996', () => {
-      const tempFixture = TestBed.createComponent(TestTransferCustomRenderComponent);
-      tempFixture.detectChanges();
-    });
+  describe('#issues', () => {
+    it('#996', fakeAsync(() => {
+      fixture = TestBed.createComponent(Test996Component);
+      dl = fixture.debugElement;
+      instance = dl.componentInstance;
+      pageObject = new TransferPageObject();
+      fixture.detectChanges();
+      expect(pageObject.getEl('[data-direction="right"] .ant-transfer-list-header .ant-checkbox').classList).not.toContain('ant-checkbox-checked');
+      pageObject.checkItem('right', 1);
+      tick();
+      fixture.detectChanges();
+      expect(pageObject.getEl('[data-direction="right"] .ant-transfer-list-header .ant-checkbox').classList).toContain('ant-checkbox-checked');
+    }));
   });
 
   class TransferPageObject {
     [key: string]: any;
+    getEl(cls: string): HTMLElement {
+      return dl.query(By.css(cls)).nativeElement as HTMLElement;
+    }
     get leftBtn(): HTMLButtonElement {
       return dl.query(By.css('.ant-transfer-operation .anticon-left')).nativeElement as HTMLButtonElement;
     }
@@ -327,3 +340,23 @@ class TestTransferCustomRenderComponent implements OnInit {
     this.nzDataSource = ret;
   }
 }
+
+@Component({
+  template: `<nz-transfer [nzDataSource]="list"></nz-transfer>`
+})
+class Test996Component implements OnInit {
+  // tslint:disable-next-line:no-any
+  list: any[] = [];
+
+  ngOnInit(): void {
+    for (let i = 0; i < 2; i++) {
+      this.list.push({
+        key     : i.toString(),
+        title   : `content${i + 1}`,
+        disabled: i % 3 < 1
+      });
+    }
+
+    [ 0, 1 ].forEach(idx => this.list[ idx ].direction = 'right');
+  }
+}