fix(grid): notify change detection when column state changes - #17602
Open
viktorkombov wants to merge 2 commits into
Open
fix(grid): notify change detection when column state changes#17602viktorkombov wants to merge 2 commits into
viktorkombov wants to merge 2 commits into
Conversation
viktorkombov
added
❌ status: awaiting-test
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
Fixes stale grid toolbar indicators in zoneless Angular Elements views.
Changes:
- Adds change-detection notifications for column and filtering state changes.
- Adds teardown for advanced-filtering subscriptions.
- Adds an 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/grids/core/src/column-actions/column-actions.component.ts:206
ngDoCheck()already diffsgrid.columnListand incrementspipeTriggerwhen that list changes (seecolumn-actions.component.ts:388-394). Because this callback increments it as well, every add/remove causes two trigger updates and reruns all three pure pipes twice; the callback only needs to mark the view dirty so the existing differ runs in the scheduled check.
this.pipeTrigger++;
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Lite
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)