Skip to content

Commit

Permalink
perf(file): do not run change detection on click and keyup events…
Browse files Browse the repository at this point in the history
… when the `td-file-input` button is clicked
  • Loading branch information
arturovt committed Jul 26, 2022
1 parent 4c652a8 commit 823e7e4
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 7 deletions.
3 changes: 1 addition & 2 deletions libs/angular/file/src/file-input/file-input.component.html
@@ -1,12 +1,11 @@
<button
#fileInputButton
mat-raised-button
class="td-file-input"
type="button"
[color]="color"
[multiple]="multiple"
[disabled]="disabled"
(keyup.enter)="fileInput.click()"
(click)="fileInput.click()"
(fileDrop)="handleSelect($event)"
tdFileDrop
>
Expand Down
26 changes: 23 additions & 3 deletions libs/angular/file/src/file-input/file-input.component.spec.ts
Expand Up @@ -4,13 +4,14 @@ import {
waitForAsync,
ComponentFixture,
} from '@angular/core/testing';
import { Component } from '@angular/core';
import { ApplicationRef, Component } from '@angular/core';
import { FormsModule } from '@angular/forms';
import { By } from '@angular/platform-browser';
import { ENTER } from '@angular/cdk/keycodes';

import { CovalentFileModule } from '../file.module';
import { TdFileInputComponent } from '../file-input/file-input.component';

import { By } from '@angular/platform-browser';

describe('Component: FileInput', () => {
beforeEach(
waitForAsync(() => {
Expand Down Expand Up @@ -132,6 +133,25 @@ describe('Component: FileInput', () => {
)
);

it('should not run change detection when the button is clicked, but should redirect the click to `<input />`', () => {
const fixture: ComponentFixture<TdFileInputBasicTestComponent> =
TestBed.createComponent(TdFileInputBasicTestComponent);
fixture.detectChanges();
const appRef = TestBed.inject(ApplicationRef);
jest.spyOn(appRef, 'tick');
const button: HTMLButtonElement = fixture.debugElement.query(
By.css('.td-file-input')
).nativeElement;
const input: HTMLInputElement = fixture.debugElement.query(
By.css('.td-file-input-hidden')
).nativeElement;
jest.spyOn(input, 'click');
button.dispatchEvent(new Event('click'));
button.dispatchEvent(new KeyboardEvent('keyup', { keyCode: ENTER }));
expect(appRef.tick).not.toHaveBeenCalled();
expect(input.click).toHaveBeenCalledTimes(2);
});

// Todo do we really want to support ng model?
xit(
'should mimic file selection event and check model',
Expand Down
35 changes: 33 additions & 2 deletions libs/angular/file/src/file-input/file-input.component.ts
Expand Up @@ -12,10 +12,16 @@ import {
ViewContainerRef,
ChangeDetectorRef,
forwardRef,
OnInit,
OnDestroy,
NgZone,
} from '@angular/core';
import { coerceBooleanProperty } from '@angular/cdk/coercion';
import { TemplatePortalDirective } from '@angular/cdk/portal';
import { ENTER } from '@angular/cdk/keycodes';
import { NG_VALUE_ACCESSOR } from '@angular/forms';
import { fromEvent, merge, Subject } from 'rxjs';
import { filter, takeUntil } from 'rxjs/operators';
import {
ICanDisable,
IControlValueAccessor,
Expand Down Expand Up @@ -60,12 +66,17 @@ export const _TdFileInputMixinBase = mixinControlValueAccessor(
})
export class TdFileInputComponent
extends _TdFileInputMixinBase
implements IControlValueAccessor, ICanDisable
implements OnInit, OnDestroy, IControlValueAccessor, ICanDisable
{
private _multiple = false;

/** The native `<button class="td-file-input"></button>` element */
@ViewChild('fileInputButton', { static: true, read: ElementRef })
_inputButton!: ElementRef<HTMLElement>;

/** The native `<input type="file"> element */
@ViewChild('fileInput', { static: true }) _inputElement!: ElementRef;
@ViewChild('fileInput', { static: true })
_inputElement!: ElementRef<HTMLInputElement>;
get inputElement(): HTMLInputElement {
return this._inputElement.nativeElement;
}
Expand Down Expand Up @@ -104,13 +115,33 @@ export class TdFileInputComponent
File | FileList
>();

private _destroy$ = new Subject<void>();

constructor(
private _ngZone: NgZone,
private _renderer: Renderer2,
_changeDetectorRef: ChangeDetectorRef
) {
super(_changeDetectorRef);
}

ngOnInit(): void {
this._ngZone.runOutsideAngular(() => {
merge(
fromEvent(this._inputButton.nativeElement, 'click'),
fromEvent<KeyboardEvent>(this._inputButton.nativeElement, 'keyup').pipe(
filter((event) => event.keyCode === ENTER)
)
)
.pipe(takeUntil(this._destroy$))
.subscribe(() => this._inputElement.nativeElement.click());
});
}

ngOnDestroy(): void {
this._destroy$.next();
}

/**
* Method executed when a file is selected.
*/
Expand Down

0 comments on commit 823e7e4

Please sign in to comment.