Skip to content

Commit

Permalink
perf(module:typography): do not run change detection on input and `…
Browse files Browse the repository at this point in the history 
…keydown` events (#7185)
  • Loading branch information
@arturovt
arturovt committed Feb 18, 2022
1 parent 3580fb0 commit ad547fb
Show file tree
Hide file tree
Showing 2 changed files with 169 additions and 52 deletions.
153 changes: 103 additions & 50 deletions components/typography/text-edit.component.ts
View file
Expand Up @@ -3,6 +3,7 @@
* found in the LICENSE file at https://github.com/NG-ZORRO/ng-zorro-antd/blob/master/LICENSE
*/

import { ENTER, ESCAPE } from '@angular/cdk/keycodes';
import {
ChangeDetectionStrategy,
ChangeDetectorRef,
Expand All @@ -11,15 +12,15 @@ import {
EventEmitter,
Input,
NgZone,
OnDestroy,
OnInit,
Output,
ViewChild,
ViewEncapsulation
} from '@angular/core';
import { Subject } from 'rxjs';
import { take, takeUntil } from 'rxjs/operators';
import { BehaviorSubject, fromEvent, Observable } from 'rxjs';
import { filter, switchMap, take, takeUntil, withLatestFrom } from 'rxjs/operators';

import { NzDestroyService } from 'ng-zorro-antd/core/services';
import { NzTSType } from 'ng-zorro-antd/core/types';
import { NzI18nService, NzTextI18nInterface } from 'ng-zorro-antd/i18n';
import { NzAutosizeDirective } from 'ng-zorro-antd/input';
Expand All @@ -28,70 +29,125 @@ import { NzAutosizeDirective } from 'ng-zorro-antd/input';
selector: 'nz-text-edit',
exportAs: 'nzTextEdit',
template: `
<button
*ngIf="!editing"
nz-tooltip
nz-trans-button
class="ant-typography-edit"
[nzTooltipTitle]="tooltip === null ? null : tooltip || locale?.edit"
(click)="onClick()"
>
<ng-container *nzStringTemplateOutlet="icon; let icon">
<i nz-icon [nzType]="icon"></i>
</ng-container>
</button>
<ng-container *ngIf="editing">
<textarea
#textarea
nz-input
nzAutosize
(input)="onInput($event)"
(blur)="confirm()"
(keydown.esc)="onCancel()"
(keydown.enter)="onEnter($event)"
></textarea>
<ng-template [ngIf]="editing" [ngIfElse]="notEditing">
<textarea #textarea nz-input nzAutosize (blur)="confirm()"></textarea>
<button nz-trans-button class="ant-typography-edit-content-confirm" (click)="confirm()">
<i nz-icon nzType="enter"></i>
</button>
</ng-container>
</ng-template>
<ng-template #notEditing>
<button
nz-tooltip
nz-trans-button
class="ant-typography-edit"
[nzTooltipTitle]="tooltip === null ? null : tooltip || locale?.edit"
(click)="onClick()"
>
<ng-container *nzStringTemplateOutlet="icon; let icon">
<i nz-icon [nzType]="icon"></i>
</ng-container>
</button>
</ng-template>
`,
changeDetection: ChangeDetectionStrategy.OnPush,
encapsulation: ViewEncapsulation.None,
preserveWhitespaces: false
preserveWhitespaces: false,
providers: [NzDestroyService]
})
export class NzTextEditComponent implements OnInit, OnDestroy {
export class NzTextEditComponent implements OnInit {
editing = false;
locale!: NzTextI18nInterface;
private destroy$ = new Subject();

@Input() text?: string;
@Input() icon: NzTSType = 'edit';
@Input() tooltip?: null | NzTSType;
@Output() readonly startEditing = new EventEmitter<void>();
@Output() readonly endEditing = new EventEmitter<string>(true);
@ViewChild('textarea', { static: false }) textarea!: ElementRef<HTMLTextAreaElement>;
@ViewChild('textarea', { static: false })
set textarea(textarea: ElementRef<HTMLTextAreaElement> | undefined) {
if (textarea) {
this.textarea$.next(textarea);
}
}
@ViewChild(NzAutosizeDirective, { static: false }) autosizeDirective!: NzAutosizeDirective;

beforeText?: string;
currentText?: string;
nativeElement = this.host.nativeElement;

// We could've saved the textarea within some private property (e.g. `_textarea`) and have a getter,
// but having subject makes the code more reactive and cancellable (e.g. event listeners will be
// automatically removed and re-added through the `switchMap` below).
private textarea$ = new BehaviorSubject<ElementRef<HTMLTextAreaElement> | null>(null);

constructor(
private zone: NgZone,
private host: ElementRef,
private ngZone: NgZone,
private host: ElementRef<HTMLElement>,
private cdr: ChangeDetectorRef,
private i18n: NzI18nService
private i18n: NzI18nService,
private destroy$: NzDestroyService
) {}

ngOnInit(): void {
this.i18n.localeChange.pipe(takeUntil(this.destroy$)).subscribe(() => {
this.locale = this.i18n.getLocaleData('Text');
this.cdr.markForCheck();
});
}

ngOnDestroy(): void {
this.destroy$.next();
this.destroy$.complete();
const textarea$: Observable<ElementRef<HTMLTextAreaElement>> = this.textarea$.pipe(
filter((textarea): textarea is ElementRef<HTMLTextAreaElement> => textarea !== null)
);

textarea$
.pipe(
switchMap(
textarea =>
// Caretaker note: we explicitly should call `subscribe()` within the root zone.
// `runOutsideAngular(() => fromEvent(...))` will just create an observable within the root zone,
// but `addEventListener` is called when the `fromEvent` is subscribed.
new Observable<KeyboardEvent>(subscriber =>
this.ngZone.runOutsideAngular(() =>
fromEvent<KeyboardEvent>(textarea.nativeElement, 'keydown').subscribe(subscriber)
)
)
),
takeUntil(this.destroy$)
)
.subscribe(event => {
// Caretaker note: adding modifier at the end (for instance `(keydown.esc)`) will tell Angular to add
// an event listener through the `KeyEventsPlugin`, which always runs `ngZone.runGuarded()` internally.
// We're interested only in escape and enter keyboard buttons, otherwise Angular will run change detection
// on any `keydown` event.
if (event.keyCode !== ESCAPE && event.keyCode !== ENTER) {
return;
}

this.ngZone.run(() => {
if (event.keyCode === ESCAPE) {
this.onCancel();
} else {
this.onEnter(event);
}
this.cdr.markForCheck();
});
});

textarea$
.pipe(
switchMap(
textarea =>
new Observable<KeyboardEvent>(subscriber =>
this.ngZone.runOutsideAngular(() =>
fromEvent<KeyboardEvent>(textarea.nativeElement, 'input').subscribe(subscriber)
)
)
),
takeUntil(this.destroy$)
)
.subscribe(event => {
this.currentText = (event.target as HTMLTextAreaElement).value;
});
}

onClick(): void {
Expand All @@ -107,11 +163,6 @@ export class NzTextEditComponent implements OnInit, OnDestroy {
this.endEditing.emit(this.currentText);
}

onInput(event: Event): void {
const target = event.target as HTMLTextAreaElement;
this.currentText = target.value;
}

onEnter(event: Event): void {
event.stopPropagation();
event.preventDefault();
Expand All @@ -124,13 +175,15 @@ export class NzTextEditComponent implements OnInit, OnDestroy {
}

focusAndSetValue(): void {
this.zone.onStable.pipe(take(1), takeUntil(this.destroy$)).subscribe(() => {
if (this.textarea?.nativeElement) {
this.textarea.nativeElement.focus();
this.textarea.nativeElement.value = this.currentText || '';
this.autosizeDirective.resizeToFitContent();
this.cdr.markForCheck();
}
});
this.ngZone.onStable
.pipe(take(1), withLatestFrom(this.textarea$), takeUntil(this.destroy$))
.subscribe(([, textarea]) => {
if (textarea) {
textarea.nativeElement.focus();
textarea.nativeElement.value = this.currentText || '';
this.autosizeDirective.resizeToFitContent();
this.cdr.markForCheck();
}
});
}
}
68 changes: 66 additions & 2 deletions components/typography/typography.spec.ts
View file
@@ -1,20 +1,23 @@
import { ENTER } from '@angular/cdk/keycodes';
import { CAPS_LOCK, ENTER, ESCAPE, TAB } from '@angular/cdk/keycodes';
import { OverlayContainer } from '@angular/cdk/overlay';
import { CommonModule } from '@angular/common';
import { Component, NgZone, ViewChild } from '@angular/core';
import { ApplicationRef, Component, NgZone, ViewChild } from '@angular/core';
import { ComponentFixture, fakeAsync, flush, inject, TestBed, tick } from '@angular/core/testing';
import { By } from '@angular/platform-browser';
import { NoopAnimationsModule } from '@angular/platform-browser/animations';

import {
createKeyboardEvent,
dispatchFakeEvent,
dispatchKeyboardEvent,
dispatchMouseEvent,
MockNgZone,
typeInElement
} from 'ng-zorro-antd/core/testing';
import { NzSafeAny } from 'ng-zorro-antd/core/types';
import { NzIconTestModule } from 'ng-zorro-antd/icon/testing';

import { NzTextEditComponent } from '.';
import { NzTypographyComponent } from './typography.component';
import { NzTypographyModule } from './typography.module';

Expand Down Expand Up @@ -480,6 +483,67 @@ describe('typography', () => {
});
});

// Caretaker note: this is moved to a separate `describe` block because the first `describe` block
// mocks the `NgZone` with `MockNgZone`.
describe('change detection behavior', () => {
let componentElement: HTMLElement;
let fixture: ComponentFixture<NzTestTypographyEditComponent>;

beforeEach(() => {
TestBed.configureTestingModule({
imports: [CommonModule, NzTypographyModule, NzIconTestModule, NoopAnimationsModule],
declarations: [NzTestTypographyEditComponent]
}).compileComponents();
});

beforeEach(() => {
fixture = TestBed.createComponent(NzTestTypographyEditComponent);
componentElement = fixture.debugElement.nativeElement;
fixture.detectChanges();
});

it('should not run change detection on `input` event', () => {
componentElement.querySelector<HTMLButtonElement>('.ant-typography-edit')!.click();
fixture.detectChanges();

const appRef = TestBed.inject(ApplicationRef);
spyOn(appRef, 'tick');

const nzTextEdit = fixture.debugElement.query(By.directive(NzTextEditComponent));
const textarea: HTMLTextAreaElement = nzTextEdit.nativeElement.querySelector('textarea');

textarea.value = 'some-value';
dispatchFakeEvent(textarea, 'input');

expect(appRef.tick).not.toHaveBeenCalled();
expect(nzTextEdit.componentInstance.currentText).toEqual('some-value');
});

it('should not run change detection on non-handled keydown events', done => {
componentElement.querySelector<HTMLButtonElement>('.ant-typography-edit')!.click();
fixture.detectChanges();

const ngZone = TestBed.inject(NgZone);
const appRef = TestBed.inject(ApplicationRef);
const spy = spyOn(appRef, 'tick');

const nzTextEdit = fixture.debugElement.query(By.directive(NzTextEditComponent));
const textarea: HTMLTextAreaElement = nzTextEdit.nativeElement.querySelector('textarea');

dispatchKeyboardEvent(textarea, 'keydown', TAB);
dispatchKeyboardEvent(textarea, 'keydown', CAPS_LOCK);

expect(spy).not.toHaveBeenCalled();

dispatchKeyboardEvent(textarea, 'keydown', ESCAPE);

ngZone.onMicrotaskEmpty.subscribe(() => {
expect(spy).toHaveBeenCalledTimes(1);
done();
});
});
});

@Component({
template: `
<h1 nz-typography>h1. Ant Design</h1>
Expand Down

0 comments on commit ad547fb

Please sign in to comment.