Skip to content

Commit 32d2683

Browse files
authored
fix(cdk/scrolling): fix virtual scrolling jankiness with run coalescing (#28968)
* fix(cdk/scrolling): fix virtual scrolling jankiness with run coalescing This reverts commit ee9abb8. * fix(cdk/scrolling): Move setting transform outside `afterNextRender`
1 parent f245153 commit 32d2683

File tree

2 files changed

+39
-20
lines changed

2 files changed

+39
-20
lines changed

src/cdk/scrolling/virtual-scroll-viewport.ts

Lines changed: 38 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,17 @@
88

99
import {Directionality} from '@angular/cdk/bidi';
1010
import {ListRange} from '@angular/cdk/collections';
11+
import {Platform} from '@angular/cdk/platform';
1112
import {
13+
afterNextRender,
1214
booleanAttribute,
1315
ChangeDetectionStrategy,
1416
ChangeDetectorRef,
1517
Component,
1618
ElementRef,
1719
inject,
1820
Inject,
21+
Injector,
1922
Input,
2023
NgZone,
2124
OnDestroy,
@@ -25,21 +28,20 @@ import {
2528
ViewChild,
2629
ViewEncapsulation,
2730
} from '@angular/core';
28-
import {Platform} from '@angular/cdk/platform';
2931
import {
3032
animationFrameScheduler,
3133
asapScheduler,
3234
Observable,
33-
Subject,
3435
Observer,
36+
Subject,
3537
Subscription,
3638
} from 'rxjs';
3739
import {auditTime, startWith, takeUntil} from 'rxjs/operators';
3840
import {ScrollDispatcher} from './scroll-dispatcher';
3941
import {CdkScrollable, ExtendedScrollToOptions} from './scrollable';
40-
import {VIRTUAL_SCROLL_STRATEGY, VirtualScrollStrategy} from './virtual-scroll-strategy';
4142
import {ViewportRuler} from './viewport-ruler';
4243
import {CdkVirtualScrollRepeater} from './virtual-scroll-repeater';
44+
import {VIRTUAL_SCROLL_STRATEGY, VirtualScrollStrategy} from './virtual-scroll-strategy';
4345
import {CdkVirtualScrollable, VIRTUAL_SCROLLABLE} from './virtual-scrollable';
4446

4547
/** Checks if the given ranges are equal. */
@@ -173,6 +175,10 @@ export class CdkVirtualScrollViewport extends CdkVirtualScrollable implements On
173175
/** Subscription to changes in the viewport size. */
174176
private _viewportChanges = Subscription.EMPTY;
175177

178+
private _injector = inject(Injector);
179+
180+
private _isDestroyed = false;
181+
176182
constructor(
177183
public override elementRef: ElementRef<HTMLElement>,
178184
private _changeDetectorRef: ChangeDetectorRef,
@@ -250,6 +256,8 @@ export class CdkVirtualScrollViewport extends CdkVirtualScrollable implements On
250256
this._detachedSubject.complete();
251257
this._viewportChanges.unsubscribe();
252258

259+
this._isDestroyed = true;
260+
253261
super.ngOnDestroy();
254262
}
255263

@@ -498,23 +506,34 @@ export class CdkVirtualScrollViewport extends CdkVirtualScrollable implements On
498506

499507
/** Run change detection. */
500508
private _doChangeDetection() {
501-
this._isChangeDetectionPending = false;
502-
503-
// Apply the content transform. The transform can't be set via an Angular binding because
504-
// bypassSecurityTrustStyle is banned in Google. However the value is safe, it's composed of
505-
// string literals, a variable that can only be 'X' or 'Y', and user input that is run through
506-
// the `Number` function first to coerce it to a numeric value.
507-
this._contentWrapper.nativeElement.style.transform = this._renderedContentTransform;
508-
// Apply changes to Angular bindings. Note: We must call `markForCheck` to run change detection
509-
// from the root, since the repeated items are content projected in. Calling `detectChanges`
510-
// instead does not properly check the projected content.
511-
this.ngZone.run(() => this._changeDetectorRef.markForCheck());
512-
513-
const runAfterChangeDetection = this._runAfterChangeDetection;
514-
this._runAfterChangeDetection = [];
515-
for (const fn of runAfterChangeDetection) {
516-
fn();
509+
if (this._isDestroyed) {
510+
return;
517511
}
512+
513+
this.ngZone.run(() => {
514+
// Apply changes to Angular bindings. Note: We must call `markForCheck` to run change detection
515+
// from the root, since the repeated items are content projected in. Calling `detectChanges`
516+
// instead does not properly check the projected content.
517+
this._changeDetectorRef.markForCheck();
518+
519+
// Apply the content transform. The transform can't be set via an Angular binding because
520+
// bypassSecurityTrustStyle is banned in Google. However the value is safe, it's composed of
521+
// string literals, a variable that can only be 'X' or 'Y', and user input that is run through
522+
// the `Number` function first to coerce it to a numeric value.
523+
this._contentWrapper.nativeElement.style.transform = this._renderedContentTransform;
524+
525+
afterNextRender(
526+
() => {
527+
this._isChangeDetectionPending = false;
528+
const runAfterChangeDetection = this._runAfterChangeDetection;
529+
this._runAfterChangeDetection = [];
530+
for (const fn of runAfterChangeDetection) {
531+
fn();
532+
}
533+
},
534+
{injector: this._injector},
535+
);
536+
});
518537
}
519538

520539
/** Calculates the `style.width` and `style.height` for the spacer element. */

src/dev-app/main.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,6 @@ bootstrapApplication(DevApp, {
5353
{provide: Directionality, useClass: DevAppDirectionality},
5454
cachedAppState.zoneless
5555
? provideExperimentalZonelessChangeDetection()
56-
: provideZoneChangeDetection({eventCoalescing: true}),
56+
: provideZoneChangeDetection({eventCoalescing: true, runCoalescing: true}),
5757
],
5858
});

0 commit comments

Comments
 (0)