fix(grid): notify change detection when column state changes - 22.1.x - #17603
Open
viktorkombov wants to merge 1 commit into
Open
fix(grid): notify change detection when column state changes - 22.1.x#17603viktorkombov wants to merge 1 commit into
viktorkombov wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
One or more issues must be addressed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR fixes stale Grid toolbar and column-action views under zoneless change detection.
Changes:
- Marks toolbar views dirty on column state changes.
- Adds teardown and dirty marking for advanced filtering updates.
- Adds a custom-elements regression test for column removal.
File summaries
| File | Description |
|---|---|
| projects/igniteui-angular/grids/core/src/toolbar/grid-toolbar.base.ts | Updated as part of this pull request. |
| projects/igniteui-angular/grids/core/src/toolbar/grid-toolbar-advanced-filtering.component.ts | Updated as part of this pull request. |
| projects/igniteui-angular/grids/core/src/column-actions/column-actions.component.ts | Updated as part of this pull request. |
| projects/igniteui-angular-elements/src/app/custom-strategy.spec.ts | Updated as part of this pull request. |
Review details
Suppressed comments (1)
projects/igniteui-angular-elements/src/app/custom-strategy.spec.ts:379
- This added test only covers the column-removal path. The production changes also add zoneless refresh paths for the pinning/hiding counts and the advanced-filtering count, but this suite never instantiates those elements or asserts their rendered indicators; the existing grid tests call
detectChanges()explicitly and cannot catch this stale-view regression. Please add Elements scenarios that apply/clear advanced filtering and pin/unhide a column, then assert the button content without manually triggering change detection.
it('should update the open column hiding dropdown when a column is removed', async () => {
// The column actions list renders grid._columns through pure pipes. With the dropdown
// already open, removing a column resets the QueryList with no click and no element
// insert - none of the triggers Angular's zoneless scheduler recognises - so without an
// explicit notification the list keeps rendering the collection as it was before.
- Files reviewed: 4/4 changed files
- Comments generated: 2
- Review effort level: Lite
Comment on lines
+58
to
+64
| this.grid?.advancedFilteringExpressionsTreeChange | ||
| .pipe(takeUntilDestroyed(this.destroyRef)) | ||
| .subscribe(filteringTree => { | ||
| this.numberOfColumns = this.extractUniqueFieldNamesFromFilterTree(filteringTree).length; | ||
| // The tree is changed from the advanced filtering dialog, i.e. outside of a check of | ||
| // this view, so nothing else marks it dirty in a zoneless app. | ||
| this.cdr.markForCheck(); |
Comment on lines
+203
to
+204
| this.grid?.columnPinned.pipe(takeUntilDestroyed(this.destroyRef)).subscribe(markDirty); | ||
| this.grid?.columnVisibilityChanged.pipe(takeUntilDestroyed(this.destroyRef)).subscribe(markDirty); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #17598
Description
The column actions list and the toolbar action buttons read grid state
directly but never marked their own views dirty. Under zone.js every
timer and overlay callback produced a tick, so the indicators refreshed
incidentally. Zoneless recognises only markForCheck, setInput, signal
updates, listeners and attaching a dirty view - none of which a QueryList
reset or an EventEmitter is - so the views can stay stale until an
unrelated interaction checks them.
Each component now marks itself dirty from the state it renders:
column actions on columnList.changes, the pinning and hiding buttons on
columnPinned and columnVisibilityChanged, and advanced filtering on
advancedFilteringExpressionsTreeChange. The advanced filtering
subscription also gains the teardown it was missing.
Motivation / Context
Type of Change (check all that apply):
Component(s) / Area(s) Affected:
How Has This Been Tested?
Test Configuration:
Screenshots / Recordings
Checklist:
feature/README.MDupdates for the feature docsREADME.MDCHANGELOG.MDupdates for newly added functionalityng updatemigrations for the breaking changes (migrations guidelines)