Skip to content

Commit

Permalink
perf(module:checkbox): reduce change detection cycles (#7127)
Browse files Browse the repository at this point in the history
  • Loading branch information
arturovt committed Jan 13, 2022
1 parent 9971faa commit 15abe33
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 13 deletions.
39 changes: 27 additions & 12 deletions components/checkbox/checkbox.component.ts
Expand Up @@ -14,6 +14,7 @@ import {
EventEmitter,
forwardRef,
Input,
NgZone,
OnDestroy,
OnInit,
Optional,
Expand All @@ -22,7 +23,7 @@ import {
ViewEncapsulation
} from '@angular/core';
import { ControlValueAccessor, NG_VALUE_ACCESSOR } from '@angular/forms';
import { Subject } from 'rxjs';
import { fromEvent, Subject } from 'rxjs';
import { takeUntil } from 'rxjs/operators';

import { BooleanInput, NzSafeAny, OnChangeType, OnTouchedType } from 'ng-zorro-antd/core/types';
Expand Down Expand Up @@ -53,7 +54,6 @@ import { NzCheckboxWrapperComponent } from './checkbox-wrapper.component';
[ngModel]="nzChecked"
[disabled]="nzDisabled"
(ngModelChange)="innerCheckedChange($event)"
(click)="$event.stopPropagation()"
/>
<span class="ant-checkbox-inner"></span>
</span>
Expand All @@ -69,8 +69,7 @@ import { NzCheckboxWrapperComponent } from './checkbox-wrapper.component';
host: {
class: 'ant-checkbox-wrapper',
'[class.ant-checkbox-wrapper-checked]': 'nzChecked',
'[class.ant-checkbox-rtl]': `dir === 'rtl'`,
'(click)': 'hostClick($event)'
'[class.ant-checkbox-rtl]': `dir === 'rtl'`
}
})
export class NzCheckboxComponent implements OnInit, ControlValueAccessor, OnDestroy, AfterViewInit {
Expand All @@ -84,7 +83,7 @@ export class NzCheckboxComponent implements OnInit, ControlValueAccessor, OnDest

onChange: OnChangeType = () => {};
onTouched: OnTouchedType = () => {};
@ViewChild('inputElement', { static: true }) private inputElement!: ElementRef;
@ViewChild('inputElement', { static: true }) inputElement!: ElementRef<HTMLInputElement>;
@Output() readonly nzCheckedChange = new EventEmitter<boolean>();
@Input() nzValue: NzSafeAny | null = null;
@Input() @InputBoolean() nzAutoFocus = false;
Expand All @@ -93,12 +92,6 @@ export class NzCheckboxComponent implements OnInit, ControlValueAccessor, OnDest
@Input() @InputBoolean() nzChecked = false;
@Input() nzId: string | null = null;

hostClick(e: MouseEvent): void {
e.preventDefault();
this.focus();
this.innerCheckedChange(!this.nzChecked);
}

innerCheckedChange(checked: boolean): void {
if (!this.nzDisabled) {
this.nzChecked = checked;
Expand Down Expand Up @@ -137,6 +130,7 @@ export class NzCheckboxComponent implements OnInit, ControlValueAccessor, OnDest
}

constructor(
private ngZone: NgZone,
private elementRef: ElementRef<HTMLElement>,
@Optional() private nzCheckboxWrapperComponent: NzCheckboxWrapperComponent,
private cdr: ChangeDetectorRef,
Expand All @@ -157,13 +151,34 @@ export class NzCheckboxComponent implements OnInit, ControlValueAccessor, OnDest
this.nzCheckboxWrapperComponent.addCheckbox(this);
}

this.directionality.change?.pipe(takeUntil(this.destroy$)).subscribe((direction: Direction) => {
this.directionality.change.pipe(takeUntil(this.destroy$)).subscribe((direction: Direction) => {
this.dir = direction;
this.cdr.detectChanges();
});

this.dir = this.directionality.value;

this.ngZone.runOutsideAngular(() => {
fromEvent(this.elementRef.nativeElement, 'click')
.pipe(takeUntil(this.destroy$))
.subscribe(event => {
event.preventDefault();
this.focus();
if (this.nzDisabled) {
return;
}
this.ngZone.run(() => {
this.innerCheckedChange(!this.nzChecked);
this.cdr.markForCheck();
});
});

fromEvent(this.inputElement.nativeElement, 'click')
.pipe(takeUntil(this.destroy$))
.subscribe(event => event.stopPropagation());
});
}

ngAfterViewInit(): void {
if (this.nzAutoFocus) {
this.focus();
Expand Down
33 changes: 32 additions & 1 deletion components/checkbox/checkbox.spec.ts
@@ -1,5 +1,5 @@
import { BidiModule, Dir } from '@angular/cdk/bidi';
import { Component, DebugElement, ViewChild } from '@angular/core';
import { ApplicationRef, Component, DebugElement, ViewChild } from '@angular/core';
import { ComponentFixture, fakeAsync, flush, TestBed, waitForAsync } from '@angular/core/testing';
import { FormBuilder, FormGroup, FormsModule, ReactiveFormsModule } from '@angular/forms';
import { By } from '@angular/platform-browser';
Expand Down Expand Up @@ -126,6 +126,37 @@ describe('checkbox', () => {
fixture.detectChanges();
expect(checkbox.nativeElement.querySelector('input') === document.activeElement).toBe(false);
});
describe('change detection behavior', () => {
it('should not run change detection when the `input` is clicked', () => {
const appRef = TestBed.inject(ApplicationRef);
const event = new MouseEvent('click');

spyOn(appRef, 'tick');
spyOn(event, 'stopPropagation').and.callThrough();

const nzCheckbox = fixture.debugElement.query(By.directive(NzCheckboxComponent));
nzCheckbox.nativeElement.querySelector('.ant-checkbox-input').dispatchEvent(event);

expect(appRef.tick).not.toHaveBeenCalled();
expect(event.stopPropagation).toHaveBeenCalled();
});
it('should not run change detection when the `nz-checkbox` is clicked and it is disabled', () => {
testComponent.disabled = true;
fixture.detectChanges();

const appRef = TestBed.inject(ApplicationRef);
const event = new MouseEvent('click');

spyOn(appRef, 'tick');
spyOn(event, 'preventDefault').and.callThrough();

const nzCheckbox = fixture.debugElement.query(By.directive(NzCheckboxComponent));
nzCheckbox.nativeElement.dispatchEvent(event);

expect(appRef.tick).not.toHaveBeenCalled();
expect(event.preventDefault).toHaveBeenCalled();
});
});
});
describe('checkbox group basic', () => {
let fixture: ComponentFixture<NzTestCheckboxGroupComponent>;
Expand Down

0 comments on commit 15abe33

Please sign in to comment.