Skip to content

Commit

Permalink
Revert "refactor(material/table): remove use of zone onStable for sty…
Browse files Browse the repository at this point in the history
…le sched…" (#28777)

This reverts commit 94c630c.
  • Loading branch information
mmalerba committed Mar 26, 2024
1 parent c345df7 commit b01f682
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 81 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {CollectionViewer, DataSource} from '@angular/cdk/collections';
import {Component, Type, ViewChild} from '@angular/core';
import {ComponentFixture, TestBed, waitForAsync} from '@angular/core/testing';
import {ComponentFixture, fakeAsync, flushMicrotasks, TestBed} from '@angular/core/testing';
import {BehaviorSubject, combineLatest} from 'rxjs';
import {map} from 'rxjs/operators';
import {CdkTable, CdkTableModule} from '@angular/cdk/table';
Expand Down Expand Up @@ -48,10 +48,10 @@ describe('CdkTableScrollContainer', () => {
dataRows = getRows(tableElement);
});

it('sets scrollbar track margin for sticky headers', waitForAsync(async () => {
it('sets scrollbar track margin for sticky headers', fakeAsync(() => {
component.stickyHeaders = ['header-1', 'header-3'];
fixture.detectChanges();
await new Promise(r => setTimeout(r));
flushMicrotasks();

if (platform.FIREFOX) {
// ::-webkit-scrollbar-track is not recognized by Firefox.
Expand All @@ -66,18 +66,18 @@ describe('CdkTableScrollContainer', () => {

component.stickyHeaders = [];
fixture.detectChanges();
await new Promise(r => setTimeout(r));
flushMicrotasks();

expect(scrollerStyle.getPropertyValue('margin-top')).toBe('0px');
expect(scrollerStyle.getPropertyValue('margin-right')).toBe('0px');
expect(scrollerStyle.getPropertyValue('margin-bottom')).toBe('0px');
expect(scrollerStyle.getPropertyValue('margin-left')).toBe('0px');
}));

it('sets scrollbar track margin for sticky footers', waitForAsync(async () => {
it('sets scrollbar track margin for sticky footers', fakeAsync(() => {
component.stickyFooters = ['footer-1', 'footer-3'];
fixture.detectChanges();
await new Promise(r => setTimeout(r));
flushMicrotasks();

if (platform.FIREFOX) {
// ::-webkit-scrollbar-track is not recognized by Firefox.
Expand All @@ -92,18 +92,18 @@ describe('CdkTableScrollContainer', () => {

component.stickyFooters = [];
fixture.detectChanges();
await new Promise(r => setTimeout(r));
flushMicrotasks();

expect(scrollerStyle.getPropertyValue('margin-top')).toBe('0px');
expect(scrollerStyle.getPropertyValue('margin-right')).toBe('0px');
expect(scrollerStyle.getPropertyValue('margin-bottom')).toBe('0px');
expect(scrollerStyle.getPropertyValue('margin-left')).toBe('0px');
}));

it('sets scrollbar track margin for sticky start columns', waitForAsync(async () => {
it('sets scrollbar track margin for sticky start columns', fakeAsync(() => {
component.stickyStartColumns = ['column-1', 'column-3'];
fixture.detectChanges();
await new Promise(r => setTimeout(r));
flushMicrotasks();

if (platform.FIREFOX) {
// ::-webkit-scrollbar-track is not recognized by Firefox.
Expand All @@ -120,18 +120,18 @@ describe('CdkTableScrollContainer', () => {

component.stickyStartColumns = [];
fixture.detectChanges();
await new Promise(r => setTimeout(r));
flushMicrotasks();

expect(scrollerStyle.getPropertyValue('margin-top')).toBe('0px');
expect(scrollerStyle.getPropertyValue('margin-right')).toBe('0px');
expect(scrollerStyle.getPropertyValue('margin-bottom')).toBe('0px');
expect(scrollerStyle.getPropertyValue('margin-left')).toBe('0px');
}));

it('sets scrollbar track margin for sticky end columns', waitForAsync(async () => {
it('sets scrollbar track margin for sticky end columns', fakeAsync(() => {
component.stickyEndColumns = ['column-4', 'column-6'];
fixture.detectChanges();
await new Promise(r => setTimeout(r));
flushMicrotasks();

if (platform.FIREFOX) {
// ::-webkit-scrollbar-track is not recognized by Firefox.
Expand All @@ -148,21 +148,21 @@ describe('CdkTableScrollContainer', () => {

component.stickyEndColumns = [];
fixture.detectChanges();
await new Promise(r => setTimeout(r));
flushMicrotasks();

expect(scrollerStyle.getPropertyValue('margin-top')).toBe('0px');
expect(scrollerStyle.getPropertyValue('margin-right')).toBe('0px');
expect(scrollerStyle.getPropertyValue('margin-bottom')).toBe('0px');
expect(scrollerStyle.getPropertyValue('margin-left')).toBe('0px');
}));

it('sets scrollbar track margin for a combination of sticky rows and columns', waitForAsync(async () => {
it('sets scrollbar track margin for a combination of sticky rows and columns', fakeAsync(() => {
component.stickyHeaders = ['header-1'];
component.stickyFooters = ['footer-3'];
component.stickyStartColumns = ['column-1'];
component.stickyEndColumns = ['column-6'];
fixture.detectChanges();
await new Promise(r => setTimeout(r));
flushMicrotasks();

if (platform.FIREFOX) {
// ::-webkit-scrollbar-track is not recognized by Firefox.
Expand All @@ -184,7 +184,7 @@ describe('CdkTableScrollContainer', () => {
component.stickyStartColumns = [];
component.stickyEndColumns = [];
fixture.detectChanges();
await new Promise(r => setTimeout(r));
flushMicrotasks();

expect(scrollerStyle.getPropertyValue('margin-top')).toBe('0px');
expect(scrollerStyle.getPropertyValue('margin-right')).toBe('0px');
Expand Down
40 changes: 24 additions & 16 deletions src/cdk/table/coalesced-style-scheduler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,9 @@
* found in the LICENSE file at https://angular.io/license
*/

import {
EnvironmentInjector,
Injectable,
InjectionToken,
NgZone,
afterNextRender,
inject,
} from '@angular/core';
import {Injectable, NgZone, OnDestroy, InjectionToken} from '@angular/core';
import {from, Subject} from 'rxjs';
import {take, takeUntil} from 'rxjs/operators';

/**
* @docs-private
Expand All @@ -36,11 +31,11 @@ export const _COALESCED_STYLE_SCHEDULER = new InjectionToken<_CoalescedStyleSche
* @docs-private
*/
@Injectable()
export class _CoalescedStyleScheduler {
export class _CoalescedStyleScheduler implements OnDestroy {
private _currentSchedule: _Schedule | null = null;
private _injector = inject(EnvironmentInjector);
private readonly _destroyed = new Subject<void>();

constructor(_unusedNgZone?: NgZone) {}
constructor(private readonly _ngZone: NgZone) {}

/**
* Schedules the specified task to run at the end of the current VM turn.
Expand All @@ -61,15 +56,22 @@ export class _CoalescedStyleScheduler {
this._currentSchedule!.endTasks.push(task);
}

/** Prevent any further tasks from running. */
ngOnDestroy() {
this._destroyed.next();
this._destroyed.complete();
}

private _createScheduleIfNeeded() {
if (this._currentSchedule) {
return;
}

this._currentSchedule = new _Schedule();

afterNextRender(
() => {
this._getScheduleObservable()
.pipe(takeUntil(this._destroyed))
.subscribe(() => {
while (this._currentSchedule!.tasks.length || this._currentSchedule!.endTasks.length) {
const schedule = this._currentSchedule!;

Expand All @@ -86,8 +88,14 @@ export class _CoalescedStyleScheduler {
}

this._currentSchedule = null;
},
{injector: this._injector},
);
});
}

private _getScheduleObservable() {
// Use onStable when in the context of an ongoing change detection cycle so that we
// do not accidentally trigger additional cycles.
return this._ngZone.isStable
? from(Promise.resolve(undefined))
: this._ngZone.onStable.pipe(take(1));
}
}

0 comments on commit b01f682

Please sign in to comment.