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.

Sign up for GitHub

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

refactor(material/table): remove use of zone onStable for style scheduling #28747

Merged
merged 1 commit into from
Mar 25, 2024
Merged

refactor(material/table): remove use of zone onStable for style scheduling #28747

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
32 changes: 16 additions & 16 deletions src/cdk-experimental/table-scroll-container/table-scroll-container.spec.ts
View file
Open in desktop
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, fakeAsync, flushMicrotasks, TestBed} from '@angular/core/testing';
import {ComponentFixture, TestBed, waitForAsync} 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', fakeAsync(() => {
it('sets scrollbar track margin for sticky headers', waitForAsync(async () => {
component.stickyHeaders = ['header-1', 'header-3'];
fixture.detectChanges();
flushMicrotasks();
await new Promise(r => setTimeout(r));

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

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

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

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

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

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

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

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

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

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

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

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

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();
flushMicrotasks();
await new Promise(r => setTimeout(r));

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

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

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

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

/**
* Schedules the specified task to run at the end of the current VM turn.
Expand All @@ -56,22 +61,15 @@ export class _CoalescedStyleScheduler implements OnDestroy {
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();

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

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

this._currentSchedule = null;
});
}

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));
},
{injector: this._injector},
);
}
}
Loading
Loading