Skip to content

Commit b12f43a

Browse files
authored
perf(module:transfer): do not trigger change detection when the checkbox is clicked (#7124)
1 parent 15abe33 commit b12f43a

2 files changed

Lines changed: 62 additions & 4 deletions

File tree

components/transfer/transfer-list.component.ts

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,22 @@
44
*/
55

66
import {
7+
AfterViewInit,
78
ChangeDetectionStrategy,
89
ChangeDetectorRef,
910
Component,
11+
ElementRef,
1012
EventEmitter,
1113
Input,
14+
NgZone,
1215
Output,
16+
QueryList,
1317
TemplateRef,
18+
ViewChildren,
1419
ViewEncapsulation
1520
} from '@angular/core';
21+
import { fromEvent, merge, Observable } from 'rxjs';
22+
import { startWith, switchMap } from 'rxjs/operators';
1623

1724
import { TransferDirection, TransferItem } from './interface';
1825

@@ -30,10 +37,10 @@ import { TransferDirection, TransferItem } from './interface';
3037
[ngClass]="{ 'ant-transfer-list-content-item-disabled': disabled || item.disabled }"
3138
>
3239
<label
40+
#checkboxes
3341
nz-checkbox
3442
[nzChecked]="item.checked"
3543
(nzCheckedChange)="onItemSelect(item)"
36-
(click)="$event.stopPropagation()"
3744
[nzDisabled]="disabled || item.disabled"
3845
>
3946
<ng-container *ngIf="!render; else renderContainer">{{ item.title }}</ng-container>
@@ -111,7 +118,7 @@ import { TransferDirection, TransferItem } from './interface';
111118
'[class.ant-transfer-list-with-footer]': '!!footer'
112119
}
113120
})
114-
export class NzTransferListComponent {
121+
export class NzTransferListComponent implements AfterViewInit {
115122
// #region fields
116123

117124
@Input() direction: TransferDirection = 'left';
@@ -138,6 +145,8 @@ export class NzTransferListComponent {
138145
@Output() readonly handleSelect: EventEmitter<TransferItem> = new EventEmitter();
139146
@Output() readonly filterChange: EventEmitter<{ direction: TransferDirection; value: string }> = new EventEmitter();
140147

148+
@ViewChildren('checkboxes', { read: ElementRef }) checkboxes!: QueryList<ElementRef<HTMLLabelElement>>;
149+
141150
stat = {
142151
checkAll: false,
143152
checkHalf: false,
@@ -203,10 +212,33 @@ export class NzTransferListComponent {
203212

204213
// #endregion
205214

206-
constructor(private cdr: ChangeDetectorRef) {}
215+
constructor(private ngZone: NgZone, private cdr: ChangeDetectorRef) {}
207216

208217
markForCheck(): void {
209218
this.updateCheckStatus();
210219
this.cdr.markForCheck();
211220
}
221+
222+
ngAfterViewInit(): void {
223+
this.checkboxes.changes
224+
.pipe(
225+
startWith(this.checkboxes),
226+
switchMap(() => {
227+
const checkboxes = this.checkboxes.toArray();
228+
// Caretaker note: we explicitly should call `subscribe()` within the root zone.
229+
// `runOutsideAngular(() => fromEvent(...))` will just create an observable within the root zone,
230+
// but `addEventListener` is called when the `fromEvent` is subscribed.
231+
return new Observable<MouseEvent>(subscriber =>
232+
this.ngZone.runOutsideAngular(() =>
233+
merge(...checkboxes.map(checkbox => fromEvent<MouseEvent>(checkbox.nativeElement, 'click'))).subscribe(
234+
subscriber
235+
)
236+
)
237+
);
238+
})
239+
)
240+
.subscribe(event => {
241+
event.stopPropagation();
242+
});
243+
}
212244
}

components/transfer/transfer.spec.ts

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,15 @@
11
/* eslint-disable @typescript-eslint/no-explicit-any */
22
import { BidiModule, Dir } from '@angular/cdk/bidi';
3-
import { Component, DebugElement, Injector, OnInit, TemplateRef, ViewChild, ViewEncapsulation } from '@angular/core';
3+
import {
4+
ApplicationRef,
5+
Component,
6+
DebugElement,
7+
Injector,
8+
OnInit,
9+
TemplateRef,
10+
ViewChild,
11+
ViewEncapsulation
12+
} from '@angular/core';
413
import { ComponentFixture, fakeAsync, TestBed, tick } from '@angular/core/testing';
514
import { By } from '@angular/platform-browser';
615
import { NoopAnimationsModule } from '@angular/platform-browser/animations';
@@ -281,6 +290,23 @@ describe('transfer', () => {
281290
).attributes.getNamedItem('placeholder')!.textContent;
282291
expect(searchPhText).toBe(en_US.Transfer.searchPlaceholder);
283292
});
293+
294+
describe('change detection behavior', () => {
295+
it('should not trigger change detection when the `ant-transfer-list-content-item label` is clicked', () => {
296+
const appRef = TestBed.inject(ApplicationRef);
297+
const event = new MouseEvent('click');
298+
299+
spyOn(appRef, 'tick');
300+
spyOn(event, 'stopPropagation').and.callThrough();
301+
302+
const [label] = fixture.nativeElement.querySelectorAll('.ant-transfer-list-content-item label');
303+
304+
label.dispatchEvent(event);
305+
306+
expect(appRef.tick).not.toHaveBeenCalled();
307+
expect(event.stopPropagation).toHaveBeenCalled();
308+
});
309+
});
284310
});
285311

286312
describe('#canMove', () => {

0 commit comments

Comments
 (0)