Skip to content

Commit

Permalink
fix(cdk/scrolling): fix virtual scrolling jankiness with run coalescing
Browse files Browse the repository at this point in the history
This reverts commit ee9abb8.
  • Loading branch information
mmalerba committed Apr 25, 2024
1 parent e2bfb91 commit f8c0b8d
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 20 deletions.
52 changes: 33 additions & 19 deletions src/cdk/scrolling/virtual-scroll-viewport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,17 @@

import {Directionality} from '@angular/cdk/bidi';
import {ListRange} from '@angular/cdk/collections';
import {Platform} from '@angular/cdk/platform';
import {
afterNextRender,
booleanAttribute,
ChangeDetectionStrategy,
ChangeDetectorRef,
Component,
ElementRef,
inject,
Inject,
Injector,
Input,
NgZone,
OnDestroy,
Expand All @@ -25,21 +28,20 @@ import {
ViewChild,
ViewEncapsulation,
} from '@angular/core';
import {Platform} from '@angular/cdk/platform';
import {
animationFrameScheduler,
asapScheduler,
Observable,
Subject,
Observer,
Subject,
Subscription,
} from 'rxjs';
import {auditTime, startWith, takeUntil} from 'rxjs/operators';
import {ScrollDispatcher} from './scroll-dispatcher';
import {CdkScrollable, ExtendedScrollToOptions} from './scrollable';
import {VIRTUAL_SCROLL_STRATEGY, VirtualScrollStrategy} from './virtual-scroll-strategy';
import {ViewportRuler} from './viewport-ruler';
import {CdkVirtualScrollRepeater} from './virtual-scroll-repeater';
import {VIRTUAL_SCROLL_STRATEGY, VirtualScrollStrategy} from './virtual-scroll-strategy';
import {CdkVirtualScrollable, VIRTUAL_SCROLLABLE} from './virtual-scrollable';

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

private _injector = inject(Injector);

private _isDestroyed = false;

constructor(
public override elementRef: ElementRef<HTMLElement>,
private _changeDetectorRef: ChangeDetectorRef,
Expand Down Expand Up @@ -250,6 +256,8 @@ export class CdkVirtualScrollViewport extends CdkVirtualScrollable implements On
this._detachedSubject.complete();
this._viewportChanges.unsubscribe();

this._isDestroyed = true;

super.ngOnDestroy();
}

Expand Down Expand Up @@ -498,23 +506,29 @@ export class CdkVirtualScrollViewport extends CdkVirtualScrollable implements On

/** Run change detection. */
private _doChangeDetection() {
this._isChangeDetectionPending = false;

// Apply the content transform. The transform can't be set via an Angular binding because
// bypassSecurityTrustStyle is banned in Google. However the value is safe, it's composed of
// string literals, a variable that can only be 'X' or 'Y', and user input that is run through
// the `Number` function first to coerce it to a numeric value.
this._contentWrapper.nativeElement.style.transform = this._renderedContentTransform;
// Apply changes to Angular bindings. Note: We must call `markForCheck` to run change detection
// from the root, since the repeated items are content projected in. Calling `detectChanges`
// instead does not properly check the projected content.
this.ngZone.run(() => this._changeDetectorRef.markForCheck());

const runAfterChangeDetection = this._runAfterChangeDetection;
this._runAfterChangeDetection = [];
for (const fn of runAfterChangeDetection) {
fn();
if (this._isDestroyed) {
return;
}

this.ngZone.run(() => {
this._changeDetectorRef.markForCheck();
afterNextRender(
() => {
this._isChangeDetectionPending = false;
// Apply the content transform. The transform can't be set via an Angular binding because
// bypassSecurityTrustStyle is banned in Google. However the value is safe, it's composed of
// string literals, a variable that can only be 'X' or 'Y', and user input that is run through
// the `Number` function first to coerce it to a numeric value.
this._contentWrapper.nativeElement.style.transform = this._renderedContentTransform;
const runAfterChangeDetection = this._runAfterChangeDetection;
this._runAfterChangeDetection = [];
for (const fn of runAfterChangeDetection) {
fn();
}
},
{injector: this._injector},
);
});
}

/** Calculates the `style.width` and `style.height` for the spacer element. */
Expand Down
2 changes: 1 addition & 1 deletion src/dev-app/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,6 @@ bootstrapApplication(DevApp, {
{provide: Directionality, useClass: DevAppDirectionality},
cachedAppState.zoneless
? provideExperimentalZonelessChangeDetection()
: provideZoneChangeDetection({eventCoalescing: true}),
: provideZoneChangeDetection({eventCoalescing: true, runCoalescing: true}),
],
});

0 comments on commit f8c0b8d

Please sign in to comment.