Skip to content

Commit

Permalink
Revert "fix(cdk/scrolling): fix virtual scrolling jankiness with run …
Browse files Browse the repository at this point in the history
…coalescing (angular#28846)" (angular#28890)

This reverts commit d91d0d4.

This broke an app in g3, reverting while we investigate
  • Loading branch information
mmalerba authored Apr 17, 2024
1 parent 81fe8f3 commit ee9abb8
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 34 deletions.
52 changes: 19 additions & 33 deletions src/cdk/scrolling/virtual-scroll-viewport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,14 @@

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 @@ -28,20 +25,21 @@ import {
ViewChild,
ViewEncapsulation,
} from '@angular/core';
import {Platform} from '@angular/cdk/platform';
import {
animationFrameScheduler,
asapScheduler,
Observable,
Observer,
Subject,
Observer,
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 @@ -175,10 +173,6 @@ 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 @@ -256,8 +250,6 @@ export class CdkVirtualScrollViewport extends CdkVirtualScrollable implements On
this._detachedSubject.complete();
this._viewportChanges.unsubscribe();

this._isDestroyed = true;

super.ngOnDestroy();
}

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

/** Run change detection. */
private _doChangeDetection() {
if (this._isDestroyed) {
return;
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();
}

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
? ɵprovideZonelessChangeDetection()
: provideZoneChangeDetection({eventCoalescing: true, runCoalescing: true}),
: provideZoneChangeDetection({eventCoalescing: true}),
],
});

0 comments on commit ee9abb8

Please sign in to comment.