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.

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

perf: do not trigger change detection internally on mouseleave #417

Merged
merged 1 commit into from
May 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 35 additions & 35 deletions src/lib/picker/category.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,11 @@ import {
OnInit,
Output,
SimpleChanges,
ViewChild
ViewChild,
} from '@angular/core';
import { Observable, Subject } from 'rxjs';
import { EmojiFrequentlyService } from './emoji-frequently.service';


@Component({
selector: 'emoji-category',
template: `
Expand All @@ -34,9 +33,7 @@ import { EmojiFrequentlyService } from './emoji-frequently.service';
</span>
</div>

<div
*ngIf="virtualize; else normalRenderTemplate"
>
<div *ngIf="virtualize; else normalRenderTemplate">
<div *ngIf="filteredEmojis$ | async as filteredEmojis">
<ngx-emoji
*ngFor="let emoji of filteredEmojis; trackBy: trackById"
Expand All @@ -53,7 +50,7 @@ import { EmojiFrequentlyService } from './emoji-frequently.service';
[hideObsolete]="hideObsolete"
[useButton]="emojiUseButton"
(emojiOver)="emojiOver.emit($event)"
(emojiLeave)="emojiLeave.emit($event)"
(emojiLeaveOutsideAngular)="emojiLeaveOutsideAngular.emit($event)"
(emojiClick)="emojiClick.emit($event)"
></ngx-emoji>
</div>
Expand Down Expand Up @@ -82,31 +79,31 @@ import { EmojiFrequentlyService } from './emoji-frequently.service';
</section>

<ng-template #normalRenderTemplate>
<ngx-emoji
*ngFor="let emoji of emojisToDisplay; trackBy: trackById"
[emoji]="emoji"
[size]="emojiSize"
[skin]="emojiSkin"
[isNative]="emojiIsNative"
[set]="emojiSet"
[sheetSize]="emojiSheetSize"
[forceSize]="emojiForceSize"
[tooltip]="emojiTooltip"
[backgroundImageFn]="emojiBackgroundImageFn"
[imageUrlFn]="emojiImageUrlFn"
[hideObsolete]="hideObsolete"
[useButton]="emojiUseButton"
(emojiOver)="emojiOver.emit($event)"
(emojiLeave)="emojiLeave.emit($event)"
(emojiClick)="emojiClick.emit($event)"
></ngx-emoji>
<ngx-emoji
*ngFor="let emoji of emojisToDisplay; trackBy: trackById"
[emoji]="emoji"
[size]="emojiSize"
[skin]="emojiSkin"
[isNative]="emojiIsNative"
[set]="emojiSet"
[sheetSize]="emojiSheetSize"
[forceSize]="emojiForceSize"
[tooltip]="emojiTooltip"
[backgroundImageFn]="emojiBackgroundImageFn"
[imageUrlFn]="emojiImageUrlFn"
[hideObsolete]="hideObsolete"
[useButton]="emojiUseButton"
(emojiOver)="emojiOver.emit($event)"
(emojiLeaveOutsideAngular)="emojiLeaveOutsideAngular.emit($event)"
(emojiClick)="emojiClick.emit($event)"
></ngx-emoji>
</ng-template>
`,
changeDetection: ChangeDetectionStrategy.OnPush,
preserveWhitespaces: false,
})
export class CategoryComponent implements OnChanges, OnInit, AfterViewInit {
@Input() emojis: any[] | null = null
@Input() emojis: any[] | null = null;
@Input() hasStickyPosition = true;
@Input() name = '';
@Input() perLine = 9;
Expand All @@ -130,12 +127,15 @@ export class CategoryComponent implements OnChanges, OnInit, AfterViewInit {
@Input() emojiImageUrlFn?: Emoji['imageUrlFn'];
@Input() emojiUseButton?: boolean;
@Output() emojiOver: Emoji['emojiOver'] = new EventEmitter();
@Output() emojiLeave: Emoji['emojiLeave'] = new EventEmitter();
/**
* Note: the suffix is added explicitly so we know the event is dispatched outside of the Angular zone.
*/
@Output() emojiLeaveOutsideAngular: Emoji['emojiLeave'] = new EventEmitter();
@Output() emojiClick: Emoji['emojiClick'] = new EventEmitter();
@ViewChild('container', { static: true }) container!: ElementRef;
@ViewChild('label', { static: true }) label!: ElementRef;
containerStyles: any = {};
emojisToDisplay: any[] = []
emojisToDisplay: any[] = [];
private filteredEmojisSubject = new Subject<any[] | null | undefined>();
filteredEmojis$: Observable<any[] | null | undefined> = this.filteredEmojisSubject.asObservable();
labelStyles: any = {};
Expand Down Expand Up @@ -188,13 +188,12 @@ export class CategoryComponent implements OnChanges, OnInit, AfterViewInit {
minHeight: `${this.rows * (this.emojiSize + 12) + 28}px`,
};

this.ref?.detectChanges();
this.ref.detectChanges();

this.handleScroll(this.container.nativeElement.parentNode.parentNode.scrollTop);
}


get noEmojiToDisplay():boolean{
get noEmojiToDisplay(): boolean {
return this.emojisToDisplay.length === 0;
}

Expand Down Expand Up @@ -222,7 +221,10 @@ export class CategoryComponent implements OnChanges, OnInit, AfterViewInit {
const { top, height } = this.container.nativeElement.getBoundingClientRect();
const parentHeight = this.container.nativeElement.parentNode.parentNode.clientHeight;

if (parentHeight + (parentHeight + this.virtualizeOffset) >= top && -height - (parentHeight + this.virtualizeOffset) <= top) {
if (
parentHeight + (parentHeight + this.virtualizeOffset) >= top &&
-height - (parentHeight + this.virtualizeOffset) <= top
) {
this.filteredEmojisSubject.next(this.emojisToDisplay);
} else {
this.filteredEmojisSubject.next([]);
Expand All @@ -248,13 +250,12 @@ export class CategoryComponent implements OnChanges, OnInit, AfterViewInit {
return;
}

let frequentlyUsed =
this.recent || this.frequently.get(this.perLine, this.totalFrequentLines);
let frequentlyUsed = this.recent || this.frequently.get(this.perLine, this.totalFrequentLines);
if (!frequentlyUsed || !frequentlyUsed.length) {
frequentlyUsed = this.frequently.get(this.perLine, this.totalFrequentLines);
}
if (!frequentlyUsed.length) {
return
return;
}
this.emojis = frequentlyUsed
.map(id => {
Expand All @@ -266,7 +267,6 @@ export class CategoryComponent implements OnChanges, OnInit, AfterViewInit {
return id;
})
.filter(id => !!this.emojiService.getData(id));

}

updateDisplay(display: 'none' | 'block') {
Expand Down
33 changes: 33 additions & 0 deletions src/lib/picker/ngx-emoji/emoji.component.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { ApplicationRef, Component } from '@angular/core';
import { TestBed } from '@angular/core/testing';

import { EmojiModule } from './emoji.module';

describe('EmojiComponent', () => {
it('should trigger change detection whenever `emojiLeave` has observers', () => {
@Component({
template: '<ngx-emoji (emojiLeave)="onEmojiLeave()"></ngx-emoji>',
})
class TestComponent {
onEmojiLeave() {}
}

TestBed.configureTestingModule({
imports: [EmojiModule],
declarations: [TestComponent],
});

const fixture = TestBed.createComponent(TestComponent);
fixture.detectChanges();

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

const emoji = fixture.nativeElement.querySelector('span.emoji-mart-emoji');
emoji.dispatchEvent(new MouseEvent('mouseleave'));

expect(appRef.tick).toHaveBeenCalledTimes(1);
expect(fixture.componentInstance.onEmojiLeave).toHaveBeenCalled();
});
});
131 changes: 93 additions & 38 deletions src/lib/picker/ngx-emoji/emoji.component.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,17 @@
import {
ChangeDetectionStrategy,
Component,
ElementRef,
EventEmitter,
Input,
NgZone,
OnChanges,
OnDestroy,
Output,
ViewChild,
inject,
} from '@angular/core';
import { EMPTY, Subject, fromEvent, switchMap, takeUntil } from 'rxjs';

import { EmojiData } from './data/data.interfaces';
import { DEFAULT_BACKGROUNDFN, EmojiService } from './emoji.service';
Expand Down Expand Up @@ -37,45 +43,48 @@ export interface EmojiEvent {
@Component({
selector: 'ngx-emoji',
template: `
<button
*ngIf="useButton && isVisible"
type="button"
(click)="handleClick($event)"
(mouseenter)="handleOver($event)"
(mouseleave)="handleLeave($event)"
[attr.title]="title"
[attr.aria-label]="label"
class="emoji-mart-emoji"
[class.emoji-mart-emoji-native]="isNative"
[class.emoji-mart-emoji-custom]="custom"
>
<span [ngStyle]="style">
<ng-template [ngIf]="isNative">{{ unified }}</ng-template>
<ng-content></ng-content>
</span>
</button>

<span
*ngIf="!useButton && isVisible"
(click)="handleClick($event)"
(mouseenter)="handleOver($event)"
(mouseleave)="handleLeave($event)"
[attr.title]="title"
[attr.aria-label]="label"
class="emoji-mart-emoji"
[class.emoji-mart-emoji-native]="isNative"
[class.emoji-mart-emoji-custom]="custom"
>
<span [ngStyle]="style">
<ng-template [ngIf]="isNative">{{ unified }}</ng-template>
<ng-content></ng-content>
<ng-template [ngIf]="isVisible">
<button
*ngIf="useButton; else spanTpl"
#button
type="button"
(click)="handleClick($event)"
(mouseenter)="handleOver($event)"
[attr.title]="title"
[attr.aria-label]="label"
class="emoji-mart-emoji"
[class.emoji-mart-emoji-native]="isNative"
[class.emoji-mart-emoji-custom]="custom"
>
<span [ngStyle]="style">
<ng-template [ngIf]="isNative">{{ unified }}</ng-template>
<ng-content></ng-content>
</span>
</button>
</ng-template>

<ng-template #spanTpl>
<span
#button
(click)="handleClick($event)"
(mouseenter)="handleOver($event)"
[attr.title]="title"
[attr.aria-label]="label"
class="emoji-mart-emoji"
[class.emoji-mart-emoji-native]="isNative"
[class.emoji-mart-emoji-custom]="custom"
>
<span [ngStyle]="style">
<ng-template [ngIf]="isNative">{{ unified }}</ng-template>
<ng-content></ng-content>
</span>
</span>
</span>
</ng-template>
`,
changeDetection: ChangeDetectionStrategy.OnPush,
preserveWhitespaces: false,
})
export class EmojiComponent implements OnChanges, Emoji {
export class EmojiComponent implements OnChanges, Emoji, OnDestroy {
@Input() skin: Emoji['skin'] = 1;
@Input() set: Emoji['set'] = 'apple';
@Input() sheetSize: Emoji['sheetSize'] = 64;
Expand All @@ -91,7 +100,14 @@ export class EmojiComponent implements OnChanges, Emoji {
@Input() sheetColumns?: number;
@Input() useButton?: boolean;
@Output() emojiOver: Emoji['emojiOver'] = new EventEmitter();
/**
* Note: `emojiLeave` and `emojiLeaveOutsideAngular` are dispatched on the same event, but for different
* purposes. The `emojiLeaveOutsideAngular` would be set up in category component so we don't care
* about zone context the callback is being called in. The `emojiLeave` is for backwards compatibility
* if anyone is listening to this event explicitly in their code.
*/
@Output() emojiLeave: Emoji['emojiLeave'] = new EventEmitter();
@Output() emojiLeaveOutsideAngular: Emoji['emojiLeave'] = new EventEmitter();
@Output() emojiClick: Emoji['emojiClick'] = new EventEmitter();
style: any;
title?: string = undefined;
Expand All @@ -103,7 +119,28 @@ export class EmojiComponent implements OnChanges, Emoji {
@Input() backgroundImageFn: Emoji['backgroundImageFn'] = DEFAULT_BACKGROUNDFN;
@Input() imageUrlFn?: Emoji['imageUrlFn'];

constructor(private emojiService: EmojiService) {}
@ViewChild('button', { static: false })
set button(button: ElementRef<HTMLElement> | undefined) {
// Note: `runOutsideAngular` is used to trigger `addEventListener` outside of the Angular zone
// too. See `setupMouseEnterListener`. The `switchMap` will subscribe to `fromEvent` considering
// the context where the factory is called in.
this.ngZone.runOutsideAngular(() => this.button$.next(button?.nativeElement));
}

/**
* The subject used to emit whenever view queries are run and `button` or `span` is set/removed.
* We use subject to keep the reactive behavior so we don't have to add and remove event listeners manually.
*/
private readonly button$ = new Subject<HTMLElement | undefined>();

private readonly destroy$ = new Subject<void>();

private readonly ngZone = inject(NgZone);
private readonly emojiService = inject(EmojiService);

constructor() {
this.setupMouseLeaveListener();
}

ngOnChanges() {
if (!this.emoji) {
Expand Down Expand Up @@ -184,6 +221,10 @@ export class EmojiComponent implements OnChanges, Emoji {
return (this.isVisible = true);
}

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

getData() {
return this.emojiService.getData(this.emoji, this.skin, this.set);
}
Expand All @@ -202,8 +243,22 @@ export class EmojiComponent implements OnChanges, Emoji {
this.emojiOver.emit({ emoji, $event });
}

handleLeave($event: Event) {
const emoji = this.getSanitizedData();
this.emojiLeave.emit({ emoji, $event });
private setupMouseLeaveListener(): void {
this.button$
.pipe(
// Note: `EMPTY` is used to remove event listener once the DOM node is removed.
switchMap(button => (button ? fromEvent(button, 'mouseleave') : EMPTY)),
takeUntil(this.destroy$),
)
.subscribe($event => {
const emoji = this.getSanitizedData();
this.emojiLeaveOutsideAngular.emit({ emoji, $event });
// Note: this is done for backwards compatibility. We run change detection if developers
// are listening to `emojiLeave` in their code. For instance:
// `<ngx-emoji (emojiLeave)="..."></ngx-emoji>`.
if (this.emojiLeave.observed) {
this.ngZone.run(() => this.emojiLeave.emit({ emoji, $event }));
}
});
}
}
2 changes: 1 addition & 1 deletion src/lib/picker/picker.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
[emojiImageUrlFn]="imageUrlFn"
[emojiUseButton]="useButton"
(emojiOver)="handleEmojiOver($event)"
(emojiLeave)="handleEmojiLeave()"
(emojiLeaveOutsideAngular)="handleEmojiLeave()"
(emojiClick)="handleEmojiClick($event)"
></emoji-category>
</section>
Expand Down
Loading