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

fix(module:icon): debounce icon rendering on animation frame #8579

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
91 changes: 50 additions & 41 deletions components/icon/icon.directive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* found in the LICENSE file at https://github.com/NG-ZORRO/ng-zorro-antd/blob/master/LICENSE
*/

import { isPlatformBrowser } from '@angular/common';
import {
AfterContentChecked,
ChangeDetectorRef,
Expand All @@ -11,16 +12,19 @@ import {
Input,
NgZone,
OnChanges,
OnDestroy,
OnInit,
Renderer2,
SimpleChanges,
booleanAttribute,
numberAttribute,
ExperimentalPendingTasks,
inject,
numberAttribute
DestroyRef,
PLATFORM_ID
} from '@angular/core';
import { Subject, from } from 'rxjs';
import { takeUntil } from 'rxjs/operators';
import { takeUntilDestroyed } from '@angular/core/rxjs-interop';
import { animationFrameScheduler, asapScheduler, from } from 'rxjs';
import { debounceTime, finalize } from 'rxjs/operators';

import { IconDirective, ThemeType } from '@ant-design/icons-angular';

Expand All @@ -36,7 +40,7 @@ import { NzIconPatchService, NzIconService } from './icon.service';
},
standalone: true
})
export class NzIconDirective extends IconDirective implements OnInit, OnChanges, AfterContentChecked, OnDestroy {
export class NzIconDirective extends IconDirective implements OnInit, OnChanges, AfterContentChecked {
cacheClassName: string | null = null;
@Input({ transform: booleanAttribute })
set nzSpin(value: boolean) {
Expand Down Expand Up @@ -71,7 +75,9 @@ export class NzIconDirective extends IconDirective implements OnInit, OnChanges,
private iconfont?: string;
private spin: boolean = false;

private destroy$ = new Subject<void>();
private destroyRef = inject(DestroyRef);
private pendingTasks = inject(ExperimentalPendingTasks);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private pendingTasks = inject(ExperimentalPendingTasks);
private pendingTasks = inject(PendingTasks);

private isBrowser = isPlatformBrowser(inject(PLATFORM_ID));

constructor(
private readonly ngZone: NgZone,
Expand All @@ -94,7 +100,9 @@ export class NzIconDirective extends IconDirective implements OnInit, OnChanges,
const { nzType, nzTwotoneColor, nzSpin, nzTheme, nzRotate } = changes;

if (nzType || nzTwotoneColor || nzSpin || nzTheme) {
this.changeIcon2();
// This is used to reduce the number of change detections
// while the icon is being loaded asynchronously.
this.ngZone.runOutsideAngular(() => this.changeIcon2());
} else if (nzRotate) {
this.handleRotate(this.el.firstChild as SVGElement);
} else {
Expand Down Expand Up @@ -124,46 +132,47 @@ export class NzIconDirective extends IconDirective implements OnInit, OnChanges,
}
}

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

/**
* Replacement of `changeIcon` for more modifications.
*/
private changeIcon2(): void {
this.setClassName();

// The Angular zone is left deliberately before the SVG is set
// since `_changeIcon` spawns asynchronous tasks as promise and
// HTTP calls. This is used to reduce the number of change detections
// while the icon is being loaded dynamically.
this.ngZone.runOutsideAngular(() => {
from(this._changeIcon())
.pipe(takeUntil(this.destroy$))
.subscribe({
next: svgOrRemove => {
// Get back into the Angular zone after completing all the tasks.
// Since we manually run change detection locally, we have to re-enter
// the zone because the change detection might also be run on other local
// components, leading them to handle template functions outside of the Angular zone.
this.ngZone.run(() => {
// The _changeIcon method would call Renderer to remove the element of the old icon,
// which would call `markElementAsRemoved` eventually,
// so we should call `detectChanges` to tell Angular remove the DOM node.
// #7186
this.changeDetectorRef.detectChanges();

if (svgOrRemove) {
this.setSVGData(svgOrRemove);
this.handleSpin(svgOrRemove);
this.handleRotate(svgOrRemove);
}
});
},
error: warn
});
});
// It is used to hydrate the icon component property when
// zoneless change detection is used in conjunction with server-side rendering.
const removeTask = this.pendingTasks.add();

from(this._changeIcon())
.pipe(
// We need to individually debounce the icon rendering on each animation
// frame to prevent frame drops when many icons are being rendered on the
// page, such as in a `@for` loop.
debounceTime(0, this.isBrowser ? animationFrameScheduler : asapScheduler),
takeUntilDestroyed(this.destroyRef),
finalize(removeTask)
)
.subscribe({
next: svgOrRemove => {
// Get back into the Angular zone after completing all the tasks.
// Since we manually run change detection locally, we have to re-enter
// the zone because the change detection might also be run on other local
// components, leading them to handle template functions outside of the Angular zone.
this.ngZone.run(() => {
// The _changeIcon method would call Renderer to remove the element of the old icon,
// which would call `markElementAsRemoved` eventually,
// so we should call `detectChanges` to tell Angular remove the DOM node.
// #7186
this.changeDetectorRef.detectChanges();

if (svgOrRemove) {
this.setSVGData(svgOrRemove);
this.handleSpin(svgOrRemove);
this.handleRotate(svgOrRemove);
}
});
},
error: warn
});
}

private handleSpin(svg: SVGElement): void {
Expand Down
Loading