Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

perf(module:select): do not run change detection unnecessarily on click events #7175

Merged
merged 1 commit into from Feb 18, 2022
Merged

perf(module:select): do not run change detection unnecessarily on click events #7175

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
54 changes: 32 additions & 22 deletions components/select/select.component.ts
View file
Open in desktop
Expand Up @@ -19,6 +19,7 @@ import {
forwardRef,
Host,
Input,
NgZone,
OnChanges,
OnDestroy,
OnInit,
Expand All @@ -31,7 +32,7 @@ import {
ViewEncapsulation
} from '@angular/core';
import { ControlValueAccessor, NG_VALUE_ACCESSOR } from '@angular/forms';
import { BehaviorSubject, combineLatest, merge } from 'rxjs';
import { BehaviorSubject, combineLatest, fromEvent, merge } from 'rxjs';
import { startWith, switchMap, takeUntil } from 'rxjs/operators';

import { slideMotion } from 'ng-zorro-antd/core/animation';
Expand Down Expand Up @@ -119,7 +120,6 @@ export type NzSelectSizeType = 'large' | 'default' | 'small';
[cdkConnectedOverlayTransformOriginOn]="'.ant-select-dropdown'"
[cdkConnectedOverlayPanelClass]="nzDropdownClassName!"
[cdkConnectedOverlayOpen]="nzOpen"
(overlayKeydown)="onOverlayKeyDown($event)"
(overlayOutsideClick)="onClickOutside($event)"
(detach)="setOpenState(false)"
(positionChange)="onPositionChange($event)"
Expand Down Expand Up @@ -161,8 +161,7 @@ export type NzSelectSizeType = 'large' | 'default' | 'small';
'[class.ant-select-focused]': 'nzOpen || focused',
'[class.ant-select-single]': `nzMode === 'default'`,
'[class.ant-select-multiple]': `nzMode !== 'default'`,
'[class.ant-select-rtl]': `dir === 'rtl'`,
'(click)': 'onHostClick()'
'[class.ant-select-rtl]': `dir === 'rtl'`
}
})
export class NzSelectComponent implements ControlValueAccessor, OnInit, AfterContentInit, OnChanges, OnDestroy {
Expand Down Expand Up @@ -292,14 +291,6 @@ export class NzSelectComponent implements ControlValueAccessor, OnInit, AfterCon
this.clearInput();
}

onHostClick(): void {
if ((this.nzOpen && this.nzShowSearch) || this.nzDisabled) {
return;
}

this.setOpenState(!this.nzOpen);
}

updateListOfContainerItem(): void {
let listOfContainerItem = this.listOfTagAndTemplateItem
.filter(item => !item.nzHide)
Expand Down Expand Up @@ -384,12 +375,6 @@ export class NzSelectComponent implements ControlValueAccessor, OnInit, AfterCon
this.clearInput();
}

onOverlayKeyDown(e: KeyboardEvent): void {
if (e.keyCode === ESCAPE) {
this.setOpenState(false);
}
}

onKeyDown(e: KeyboardEvent): void {
if (this.nzDisabled) {
return;
Expand Down Expand Up @@ -474,7 +459,7 @@ export class NzSelectComponent implements ControlValueAccessor, OnInit, AfterCon
}

onClickOutside(event: MouseEvent): void {
if (!this.elementRef.nativeElement.contains(event.target)) {
if (!this.host.nativeElement.contains(event.target as HTMLElement)) {
this.setOpenState(false);
}
}
Expand Down Expand Up @@ -516,10 +501,11 @@ export class NzSelectComponent implements ControlValueAccessor, OnInit, AfterCon
}

constructor(
private ngZone: NgZone,
private destroy$: NzDestroyService,
public nzConfigService: NzConfigService,
private cdr: ChangeDetectorRef,
private elementRef: ElementRef,
private host: ElementRef<HTMLElement>,
private platform: Platform,
private focusMonitor: FocusMonitor,
@Optional() private directionality: Directionality,
Expand Down Expand Up @@ -592,7 +578,7 @@ export class NzSelectComponent implements ControlValueAccessor, OnInit, AfterCon

ngOnInit(): void {
this.focusMonitor
.monitor(this.elementRef, true)
.monitor(this.host, true)
.pipe(takeUntil(this.destroy$))
.subscribe(focusOrigin => {
if (!focusOrigin) {
Expand Down Expand Up @@ -640,6 +626,29 @@ export class NzSelectComponent implements ControlValueAccessor, OnInit, AfterCon
});

this.dir = this.directionality.value;

this.ngZone.runOutsideAngular(() =>
fromEvent(this.host.nativeElement, 'click')
.pipe(takeUntil(this.destroy$))
.subscribe(() => {
if ((this.nzOpen && this.nzShowSearch) || this.nzDisabled) {
return;
}

this.ngZone.run(() => this.setOpenState(!this.nzOpen));
})
);

// Caretaker note: we could've added this listener within the template `(overlayKeydown)="..."`,
// but with this approach, it'll run change detection on each keyboard click, and also it'll run
// `markForCheck()` internally, which means the whole component tree (starting from the root and
// going down to the select component) will be re-checked and updated (if needed).
// This is safe to do that manually since `setOpenState()` calls `markForCheck()` if needed.
this.cdkConnectedOverlay.overlayKeydown.pipe(takeUntil(this.destroy$)).subscribe(event => {
if (event.keyCode === ESCAPE) {
this.setOpenState(false);
}
});
}

ngAfterContentInit(): void {
Expand Down Expand Up @@ -679,8 +688,9 @@ export class NzSelectComponent implements ControlValueAccessor, OnInit, AfterCon
});
}
}

ngOnDestroy(): void {
cancelRequestAnimationFrame(this.requestId);
this.focusMonitor.stopMonitoring(this.elementRef);
this.focusMonitor.stopMonitoring(this.host);
}
}