Skip to content

Commit a328e71

Browse files
viktorkombovChronosSF
viktorkombov
and
ChronosSF
authored
fix(grid): fix zone.onStable patterns broken in zoneless change detection (#17328)
* fix(grid): fix zone.onStable patterns broken in zoneless change detection * fix(grid): improve zoneless scheduling support * fix(for-of): recalculate virtual sizes in zoneless mode * chore(*): remove repeating code * fix(grid): use afterNextRender for zoneless render scheduling * fix(grid): improve virtualized keyboard selection handling * fix(grids): migrate virt & nav to zoneless-compatible scheduling - Deferred grid work (scroll chunkLoad emit, column autosizing, filter-row rendering, post-scroll cell activation) relied on NgZone.onStable, which never emits under provideZonelessChangeDetection. Replace it with a single runAfterRenderOnce helper built on afterNextRender, and remove the private ɵNoopNgZone zoneless detection. - core: add runAfterRenderOnce(injector, cb, phase) central scheduler - grids: route for_of, grid-base, pivot-grid deferred work through it - filtering: signal-backed isFilterRowVisible; ResizeObserver-driven chip remeasure; explicit notify on column resize - navigation: notify on any scrolling nav key; subscribe before scroll in hierarchical navigation * fix(grid): refactor navigation callback handling for improved clarity * fix(grid): remove redundant resize change detection * fix(grid): remove unnecessary read option from runAfterRenderOnce calls * fix(grid): simplify summary row index retrieval in tests * fix(grid): restore navigation callback contract * fix(grid): remove zoneless scroll detection setup from tests * fix(grid): waitForChildrenResolved function to include predicate handling * fix(grid): remove unnecessary 'read' argument from recalcUpdateSizes calls * test(grid): improve virtualized navigation synchronization * fix(grid): update wait for grid events methods for improved navigation handling * test(grids): enable auto detection for deferred render tests * test(grid): use DOM events in filtering row tests --------- Co-authored-by: Stamen Stoychev <chronos.stz@gmail.com>
1 parent 9726bdf commit a328e71

23 files changed

Lines changed: 680 additions & 109 deletions

‎projects/igniteui-angular/core/src/core/utils.ts‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { isPlatformBrowser } from '@angular/common';
2-
import { Injectable, InjectionToken, PLATFORM_ID, inject } from '@angular/core';
2+
import { Injectable, InjectionToken, PLATFORM_ID, inject, afterNextRender, type AfterRenderRef, type Injector } from '@angular/core';
33
import { mergeWith } from 'lodash-es';
44
import { NEVER, Observable } from 'rxjs';
55
import { isDevMode } from '@angular/core';
@@ -8,6 +8,31 @@ import type { IgxTheme } from '../services/theme/theme.token';
88
/** @hidden @internal */
99
export const ELEMENTS_TOKEN = /*@__PURE__*/new InjectionToken<boolean>('elements environment');
1010

11+
/** @hidden @internal */
12+
export type RenderPhase = 'earlyRead' | 'write' | 'mixedReadWrite' | 'read';
13+
14+
interface AfterNextRenderSpec {
15+
earlyRead?: () => void;
16+
write?: () => void;
17+
mixedReadWrite?: () => void;
18+
read?: () => void;
19+
}
20+
21+
/**
22+
* Schedules `callback` to run once after Angular finishes the next render pass.
23+
*
24+
* Central scheduling point for all work that previously waited on `NgZone.onStable`,
25+
* which never emits in zoneless applications. Every deferred render callback in the
26+
* library goes through here, so if the scheduling needs to change (different phase,
27+
* timing or API), change it in this single place.
28+
*
29+
* @hidden @internal
30+
*/
31+
export function runAfterRenderOnce(injector: Injector, callback: () => void, phase: RenderPhase = 'mixedReadWrite'): AfterRenderRef {
32+
const spec: AfterNextRenderSpec = {};
33+
spec[phase as keyof AfterNextRenderSpec] = callback;
34+
return afterNextRender(spec, { injector });
35+
}
1136

1237
/**
1338
* Returns true if the element's direction is left-to-right

‎projects/igniteui-angular/directives/src/directives/for-of/for_of.directive.ts‎

Lines changed: 16 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
import { NgForOfContext } from '@angular/common';
2-
import { ChangeDetectorRef, ComponentRef, Directive, EmbeddedViewRef, EventEmitter, Input, IterableChanges, IterableDiffer, IterableDiffers, NgZone, OnChanges, OnDestroy, OnInit, Output, SimpleChanges, TemplateRef, TrackByFunction, ViewContainerRef, booleanAttribute, DOCUMENT, inject, afterNextRender, runInInjectionContext, EnvironmentInjector, AfterViewInit } from '@angular/core';
2+
import { ChangeDetectorRef, ComponentRef, Directive, EmbeddedViewRef, EventEmitter, Input, IterableChanges, IterableDiffer, IterableDiffers, NgZone, OnChanges, OnDestroy, OnInit, Output, SimpleChanges, TemplateRef, TrackByFunction, ViewContainerRef, booleanAttribute, DOCUMENT, inject, EnvironmentInjector, AfterViewInit } from '@angular/core';
33

44
import { DisplayContainerComponent } from './display.container';
55
import { HVirtualHelperComponent } from './horizontal.virtual.helper.component';
66
import { VirtualHelperComponent } from './virtual.helper.component';
77

88
import { IgxForOfSyncService, IgxForOfScrollSyncService } from './for_of.sync.service';
99
import { Subject } from 'rxjs';
10-
import { takeUntil, filter, throttleTime, first } from 'rxjs/operators';
11-
import { getResizeObserver } from 'igniteui-angular/core';
10+
import { takeUntil, filter, throttleTime } from 'rxjs/operators';
11+
import { getResizeObserver, runAfterRenderOnce } from 'igniteui-angular/core';
1212
import { IBaseEventArgs, PlatformUtil } from 'igniteui-angular/core';
1313
import { VirtualHelperBaseDirective } from './base.helper.component';
1414

@@ -659,13 +659,9 @@ export class IgxForOfDirective<T, U extends T[] = T[]> extends IgxForOfToken<T,U
659659
// Actual scroll delta that was added is smaller than 1 and onScroll handler doesn't trigger when scrolling < 1px
660660
const scrollOffset = this.fixedUpdateAllElements(this._virtScrollPosition);
661661
// scrollOffset = scrollOffset !== parseInt(this.igxForItemSize, 10) ? scrollOffset : 0;
662-
runInInjectionContext(this._injector, () => {
663-
afterNextRender({
664-
write: () => {
665-
this.dc.instance._viewContainer.element.nativeElement.style.transform = `translateY(${-scrollOffset}px)`;
666-
}
667-
});
668-
});
662+
runAfterRenderOnce(this._injector, () => {
663+
this.dc.instance._viewContainer.element.nativeElement.style.transform = `translateY(${-scrollOffset}px)`;
664+
}, 'write');
669665
}
670666

671667
const maxRealScrollTop = this.scrollComponent.nativeElement.scrollHeight - containerSize;
@@ -912,7 +908,7 @@ export class IgxForOfDirective<T, U extends T[] = T[]> extends IgxForOfToken<T,U
912908
// in case scrolled to specific index where after scroll heights are changed
913909
// need to adjust the offsets so that item is last in view.
914910
const updatesToIndex = this._adjustToIndex - this.state.startIndex + 1;
915-
const sumDiffs = diffs.slice(0, updatesToIndex).reduce(reducer);
911+
const sumDiffs = diffs.slice(0, updatesToIndex).reduce(reducer, 0);
916912
if (sumDiffs !== 0) {
917913
this.addScroll(sumDiffs);
918914
}
@@ -962,15 +958,10 @@ export class IgxForOfDirective<T, U extends T[] = T[]> extends IgxForOfToken<T,U
962958
const prevStartIndex = this.state.startIndex;
963959
const scrollOffset = this.fixedUpdateAllElements(this._virtScrollPosition);
964960

965-
runInInjectionContext(this._injector, () => {
966-
afterNextRender({
967-
write: () => {
968-
this.dc.instance._viewContainer.element.nativeElement.style.transform = `translateY(${-scrollOffset}px)`;
969-
}
970-
});
971-
});
972-
973-
this._zone.onStable.pipe(first()).subscribe(this.recalcUpdateSizes.bind(this));
961+
runAfterRenderOnce(this._injector, () => {
962+
this.dc.instance._viewContainer.element.nativeElement.style.transform = `translateY(${-scrollOffset}px)`;
963+
}, 'write');
964+
runAfterRenderOnce(this._injector, () => this.recalcUpdateSizes());
974965

975966
this.dc.changeDetectorRef.detectChanges();
976967
if (prevStartIndex !== this.state.startIndex) {
@@ -1182,7 +1173,7 @@ export class IgxForOfDirective<T, U extends T[] = T[]> extends IgxForOfToken<T,U
11821173
} else {
11831174
this.dc.instance._viewContainer.element.nativeElement.style.left = -scrollOffset + 'px';
11841175
}
1185-
this._zone.onStable.pipe(first()).subscribe(this.recalcUpdateSizes.bind(this));
1176+
runAfterRenderOnce(this._injector, () => this.recalcUpdateSizes());
11861177

11871178
this.dc.changeDetectorRef.detectChanges();
11881179
if (prevStartIndex !== this.state.startIndex) {
@@ -1785,14 +1776,10 @@ export class IgxGridForOfDirective<T, U extends T[] = T[]> extends IgxForOfDirec
17851776
}
17861777
const prevState = Object.assign({}, this.state);
17871778
const scrollOffset = this.fixedUpdateAllElements(this._virtScrollPosition);
1788-
runInInjectionContext(this._injector, () => {
1789-
afterNextRender({
1790-
write: () => {
1791-
this.dc.instance._viewContainer.element.nativeElement.style.transform = `translateY(${-scrollOffset}px)`;
1792-
this._zone.onStable.pipe(first()).subscribe(this.recalcUpdateSizes.bind(this, prevState));
1793-
}
1794-
});
1795-
});
1779+
runAfterRenderOnce(this._injector, () => {
1780+
this.dc.instance._viewContainer.element.nativeElement.style.transform = `translateY(${-scrollOffset}px)`;
1781+
}, 'write');
1782+
runAfterRenderOnce(this._injector, () => this.recalcUpdateSizes(prevState));
17961783

17971784
this.cdr.markForCheck();
17981785
}

‎projects/igniteui-angular/grids/core/src/grid-navigation.service.ts‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ export interface IActiveNode {
3333
layout?: IMultiRowLayoutNode;
3434
}
3535

36+
const VERTICAL_VIRTUALIZATION_NAV_KEYS = new Set(['arrowup', 'up', 'arrowdown', 'down', 'home', 'end']);
37+
3638
/** @hidden */
3739
@Injectable()
3840
export class IgxGridNavigationService {
@@ -106,8 +108,7 @@ export class IgxGridNavigationService {
106108
}
107109
const position = this.getNextPosition(this.activeNode.row, this.activeNode.column, key, shift, ctrl, event);
108110
const shouldNotifyVirtualizedKeyboardSelection =
109-
ctrl && (key === 'arrowup' || key === 'up' || key === 'arrowdown' || key === 'down') &&
110-
this.shouldPerformVerticalScroll(position.rowIndex, position.colIndex);
111+
this.shouldNotifyVirtualizedKeyboardSelection(key, position.rowIndex, position.colIndex);
111112
if (NAVIGATION_KEYS.has(key)) {
112113
event.preventDefault();
113114
this.navigateInBody(position.rowIndex, position.colIndex, (obj) => {
@@ -231,6 +232,16 @@ export class IgxGridNavigationService {
231232
|| containerHeight && endTopOffset - containerHeight > 5;
232233
}
233234

235+
protected shouldNotifyVirtualizedKeyboardSelection(key: string, rowIndex: number, visibleColIndex: number): boolean {
236+
// Any navigation key that ends up scrolling activates the target cell from the
237+
// virtualization scroll callback, which runs outside Angular's knowledge, so the
238+
// grid must be notified explicitly regardless of the ctrl modifier.
239+
const shouldCheckVerticalScroll = VERTICAL_VIRTUALIZATION_NAV_KEYS.has(key);
240+
const shouldCheckHorizontalScroll = HORIZONTAL_NAV_KEYS.has(key);
241+
242+
return (shouldCheckVerticalScroll && this.shouldPerformVerticalScroll(rowIndex, visibleColIndex)) ||
243+
(shouldCheckHorizontalScroll && this.shouldPerformHorizontalScroll(visibleColIndex, rowIndex));
244+
}
234245
public performVerticalScrollToCell(rowIndex: number, visibleColIndex = -1, cb?: () => void) {
235246
if (!this.shouldPerformVerticalScroll(rowIndex, visibleColIndex)) {
236247
if (cb) {

‎projects/igniteui-angular/grids/grid/src/column-group.spec.ts‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -608,19 +608,22 @@ describe('IgxGrid - multi-column headers #grid', () => {
608608
grid = fixture.componentInstance.grid;
609609
}));
610610

611-
it('Width should be correct. Column group with three columns. No width.', () => {
611+
it('Width should be correct. Column group with three columns. No width.', async () => {
612+
await wait(16);
613+
fixture.detectChanges();
612614
const scrWitdh = grid.nativeElement.querySelector('.igx-grid__tbody-scrollbar').getBoundingClientRect().width;
613-
const availableWidth = (parseInt(componentInstance.gridWrapperWidthPx, 10) - scrWitdh).toString();
615+
const availableWidth = parseInt(componentInstance.gridWrapperWidthPx, 10) - scrWitdh;
614616
const locationColGroup = getColGroup(grid, 'Location');
615-
const colWidth = Math.floor(parseInt(availableWidth, 10) / 3);
616-
const colWidthPx = colWidth + 'px';
617-
expect(locationColGroup.width).toBe((Math.round(colWidth) * 3) + 'px');
617+
const colWidth = availableWidth / 3;
618+
const expectWidthWithinPixel = (actualWidth: string, expectedWidth: number) =>
619+
expect(Math.abs(parseFloat(actualWidth) - expectedWidth)).toBeLessThanOrEqual(1);
620+
expectWidthWithinPixel(locationColGroup.width, availableWidth);
618621
const countryColumn = grid.getColumnByName('Country');
619-
expect(countryColumn.width).toBe(colWidthPx);
622+
expectWidthWithinPixel(countryColumn.width, colWidth);
620623
const regionColumn = grid.getColumnByName('Region');
621-
expect(regionColumn.width).toBe(colWidthPx);
624+
expectWidthWithinPixel(regionColumn.width, colWidth);
622625
const cityColumn = grid.getColumnByName('City');
623-
expect(cityColumn.width).toBe(colWidthPx);
626+
expectWidthWithinPixel(cityColumn.width, colWidth);
624627
});
625628

626629
it('Width should be correct. Column group with three columns. Width in px.', () => {

‎projects/igniteui-angular/grids/grid/src/column.spec.ts‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Component, DebugElement, TemplateRef, ViewChild, ChangeDetectionStrategy } from '@angular/core';
1+
import { Component, DebugElement, TemplateRef, ViewChild, ChangeDetectionStrategy, provideZonelessChangeDetection } from '@angular/core';
22
import { TestBed, fakeAsync, tick, waitForAsync, ComponentFixture } from '@angular/core/testing';
33
import { By } from '@angular/platform-browser';
44
import { getLocaleCurrencySymbol, registerLocaleData } from '@angular/common';
@@ -1942,3 +1942,30 @@ export class DOMAttributesAsSettersComponent {
19421942

19431943
public data = [{ id: 1, value: 1 }];
19441944
}
1945+
1946+
describe('IgxGrid column autosizing in zoneless change detection #grid', () => {
1947+
beforeEach(() => {
1948+
TestBed.configureTestingModule({
1949+
imports: [ResizableColumnsComponent, NoopAnimationsModule],
1950+
providers: [provideZonelessChangeDetection()]
1951+
});
1952+
});
1953+
1954+
it('should recalculate fit-content widths after data changes', async () => {
1955+
const fix = TestBed.createComponent(ResizableColumnsComponent);
1956+
fix.detectChanges();
1957+
await fix.whenStable();
1958+
const grid = fix.componentInstance.instance;
1959+
1960+
grid.data = [{
1961+
ID: 'VeryVeryVeryLongID',
1962+
Address: 'Avda. de la Constituci\u00f3n 2222 Obere Str. 57'
1963+
}];
1964+
await fix.whenStable();
1965+
grid.recalculateAutoSizes();
1966+
await fix.whenStable();
1967+
1968+
expect(grid.columns[0].width).toBe('164px');
1969+
expect(grid.columns[1].width).toBe('279px');
1970+
});
1971+
});

‎projects/igniteui-angular/grids/grid/src/grid-base.directive.ts‎

Lines changed: 10 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,8 @@ import {
9191
IGridResourceStrings,
9292
IgxOverlayOutletDirective,
9393
DEFAULT_LOCALE,
94-
onResourceChangeHandle
94+
onResourceChangeHandle,
95+
runAfterRenderOnce
9596
} from 'igniteui-angular/core';
9697
import { IgcTrialWatermark } from 'igniteui-trial-watermark';
9798
import { Subject, pipe, fromEvent, animationFrameScheduler, merge, BehaviorSubject, timer } from 'rxjs';
@@ -4166,9 +4167,7 @@ export abstract class IgxGridBaseDirective implements GridType,
41664167
if (this.hasColumnsToAutosize) {
41674168
this.headerContainer?.dataChanged.pipe(takeUntil(this.destroy$)).subscribe(() => {
41684169
this.cdr.detectChanges();
4169-
this.zone.onStable.pipe(first()).subscribe(() => {
4170-
this.autoSizeColumnsInView();
4171-
});
4170+
runAfterRenderOnce(this.injector, () => this.autoSizeColumnsInView());
41724171
});
41734172
}
41744173
// Window resize observer not needed because when you resize the window element the tbody container always resize so
@@ -4681,7 +4680,7 @@ export abstract class IgxGridBaseDirective implements GridType,
46814680
// reset auto-size and calculate it again.
46824681
this._columns.forEach(x => x.autoSize = undefined);
46834682
this.resetCaches();
4684-
this.zone.onStable.pipe(first()).subscribe(() => {
4683+
runAfterRenderOnce(this.injector, () => {
46854684
this.cdr.detectChanges();
46864685
this.autoSizeColumnsInView();
46874686
});
@@ -6377,7 +6376,7 @@ export abstract class IgxGridBaseDirective implements GridType,
63776376
const tmplId = args.context.templateID.type;
63786377
const index = args.context.index;
63796378
args.view.detectChanges();
6380-
this.zone.onStable.pipe(first()).subscribe(() => {
6379+
runAfterRenderOnce(this.injector, () => {
63816380
const row = tmplId === 'dataRow' ? this.gridAPI.get_row_by_index(index) : null;
63826381
const summaryRow = tmplId === 'summaryRow' ? this.summariesRowList.find((sr) => sr.dataRowIndex === index) : null;
63836382
if (row && row instanceof IgxRowDirective) {
@@ -7076,24 +7075,16 @@ export abstract class IgxGridBaseDirective implements GridType,
70767075
this.cdr.detectChanges();
70777076
}
70787077

7079-
if (this.zone.isStable) {
7078+
runAfterRenderOnce(this.injector, () => {
70807079
this.zone.run(() => {
70817080
this._applyWidthHostBinding();
70827081
this.cdr.detectChanges();
70837082
});
7084-
} else {
7085-
this.zone.onStable.pipe(first()).subscribe(() => {
7086-
this.zone.run(() => {
7087-
this._applyWidthHostBinding();
7088-
});
7089-
});
7090-
}
7083+
});
70917084
this.resetCaches(recalcFeatureWidth);
70927085
if (this.hasColumnsToAutosize) {
70937086
this.cdr.detectChanges();
7094-
this.zone.onStable.pipe(first()).subscribe(() => {
7095-
this._autoSizeColumnsNotify.next();
7096-
});
7087+
runAfterRenderOnce(this.injector, () => this._autoSizeColumnsNotify.next());
70977088
}
70987089

70997090
// in case horizontal scrollbar has appeared recalc to size correctly.
@@ -7743,19 +7734,13 @@ export abstract class IgxGridBaseDirective implements GridType,
77437734
protected verticalScrollHandler(event) {
77447735
this.verticalScrollContainer.onScroll(event);
77457736
this.disableTransitions = true;
7746-
77477737
const callback = () => {
77487738
this.verticalScrollContainer.chunkLoad.emit(this.verticalScrollContainer.state);
77497739
if (this.rowEditable) {
77507740
this.changeRowEditingOverlayStateOnScroll(this.crudService.rowInEditMode);
77517741
}
77527742
};
7753-
if (this.isZonelessChangeDetection()) {
7754-
this.cdr.detectChanges();
7755-
callback();
7756-
} else {
7757-
this.zone.onStable.pipe(first()).subscribe(callback);
7758-
}
7743+
runAfterRenderOnce(this.injector, callback);
77597744
this.disableTransitions = false;
77607745

77617746
this.hideOverlays();
@@ -7780,10 +7765,6 @@ export abstract class IgxGridBaseDirective implements GridType,
77807765
this.gridScroll.emit(args);
77817766
}
77827767

7783-
protected isZonelessChangeDetection(): boolean {
7784-
return this.zone.constructor.name === 'NoopNgZone';
7785-
}
7786-
77877768
protected hasMenuPinningActions(): boolean {
77887769
const strip = this.actionStrip;
77897770
const actionButtons = strip?.actionButtons;
@@ -7808,7 +7789,7 @@ export abstract class IgxGridBaseDirective implements GridType,
78087789
this.cdr.markForCheck();
78097790

78107791
this.zone.run(() => {
7811-
this.zone.onStable.pipe(first()).subscribe(() => {
7792+
runAfterRenderOnce(this.injector, () => {
78127793
this.parentVirtDir.chunkLoad.emit(this.headerContainer.state);
78137794
requestAnimationFrame(() => {
78147795
this.autoSizeColumnsInView();

0 commit comments

Comments
 (0)