Repository navigation
refactor(combo, drop-down, select): switch to OnPush with signal-backed state - #17622
viktorkombov wants to merge 20 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved reactive dependencies can leave remote handling and OnPush item ARIA, selection, or text state stale.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR migrates Combo, Simple Combo, Drop Down, and Select to OnPush change detection with signal-backed state while preserving their public APIs.
Changes:
- Updates components, items, groups, queries, and host metadata for OnPush.
- Adds reactive selection tracking and modernizes overlay and grouping behavior.
- Adds zoneless regression tests and removes unnecessary change detection.
File summaries
| File | Summary |
|---|---|
projects/igniteui-angular/simple-combo/src/simple-combo/simple-combo.component.ts |
Adds OnPush state and signal queries. |
projects/igniteui-angular/simple-combo/src/simple-combo/simple-combo.component.spec.ts |
Adds zoneless state tests. |
projects/igniteui-angular/select/src/select/select.component.ts |
Adds OnPush and signal-backed properties. |
projects/igniteui-angular/select/src/select/select.component.spec.ts |
Adds zoneless Select tests. |
projects/igniteui-angular/select/src/select/select-item.component.ts |
Enables OnPush for select items. |
projects/igniteui-angular/select/src/select/select-group.component.ts |
Enables OnPush for select groups. |
projects/igniteui-angular/drop-down/src/drop-down/drop-down.component.ts |
Adds reactive state and selection revision tracking. |
projects/igniteui-angular/drop-down/src/drop-down/drop-down.component.spec.ts |
Adds zoneless Drop Down tests. |
projects/igniteui-angular/drop-down/src/drop-down/drop-down.base.ts |
Migrates host bindings and state to signals. |
projects/igniteui-angular/drop-down/src/drop-down/drop-down-navigation.directive.ts |
Migrates host bindings and listeners. |
projects/igniteui-angular/drop-down/src/drop-down/drop-down-item.component.ts |
Enables OnPush and updates focus handling. |
projects/igniteui-angular/drop-down/src/drop-down/drop-down-item.base.ts |
Migrates item host bindings and state. |
projects/igniteui-angular/drop-down/src/drop-down/drop-down-group.component.ts |
Adds OnPush and signal-backed group state. |
projects/igniteui-angular/drop-down/src/drop-down/autocomplete/autocomplete.directive.ts |
Removes unnecessary change detection. |
projects/igniteui-angular/combo/src/combo/combo.pipes.ts |
Adjusts filtered-data updates during rendering. |
projects/igniteui-angular/combo/src/combo/combo.component.ts |
Adds OnPush, signals, queries, and selection tracking. |
projects/igniteui-angular/combo/src/combo/combo.component.spec.ts |
Adds zoneless Combo tests. |
projects/igniteui-angular/combo/src/combo/combo.component.html |
Updates reactive template bindings. |
projects/igniteui-angular/combo/src/combo/combo.common.ts |
Adds signal-backed state, queries, and computed values. |
projects/igniteui-angular/combo/src/combo/combo.api.ts |
Makes transition state reactive. |
projects/igniteui-angular/combo/src/combo/combo-item.component.ts |
Enables OnPush and reactive selection. |
projects/igniteui-angular/combo/src/combo/combo-dropdown.component.ts |
Enables OnPush and reactive single-mode state. |
projects/igniteui-angular/combo/src/combo/combo-add-item.component.ts |
Enables OnPush and host metadata. |
Review details
Suppressed comments (4)
projects/igniteui-angular/drop-down/src/drop-down/drop-down-item.component.ts:14
- The indexed branch of
IgxDropDownItemComponent.selectedstill readsselection.first_item(...)directly, without consumingselectionRevision. After this item becomes OnPush,setSelectedItem()andclearSelection()can update the drop-down revision while the virtual row'saria-selectedand selected class remain unchanged. Make this getter read the reactive drop-down selection/revision, or do not switch indexed items to OnPush yet.
changeDetection: ChangeDetectionStrategy.OnPush,
projects/igniteui-angular/drop-down/src/drop-down/drop-down.component.ts:603
- For virtualized items,
IgxDropDownItemComponent.selectedreadsselection.first_item(...)directly and does not consumeselectionRevision. ThussetSelectedItem()/clearSelection()only update this signal while a recycled OnPush item can keep stalearia-selectedand selected-class state in zoneless or programmatic flows. Make the virtual item getter depend on the revision (or another reactive selection signal) and cover virtual selection updates.
this.selectionRevision.update(revision => revision + 1);
projects/igniteui-angular/select/src/select/select-item.component.ts:7
- The OnPush item also leaves its public
textvalue in the plain_textfield. When the selected item's text is changed programmatically, the select component's[value]="selectionValue"binding has no signal dependency on that setter, so the input can keep displaying the old text until some unrelated check occurs. Maketextsignal-backed (or otherwise notify the select) as part of this OnPush migration.
changeDetection: ChangeDetectionStrategy.OnPush,
projects/igniteui-angular/select/src/select/select.component.ts:698
- The new revision is consumed by
IgxSelectComponent.selectedItem, butIgxSelectItemComponent.selectedstill only reads the non-reactive selection service. With the item now OnPush,select.value = ...can update the input text while the item'saria-selectedattribute and selected class remain stale until an unrelated check. Make select items consume this revision (or update an item signal) and add a programmatic selection assertion.
this.selectionRevision.update(revision => revision + 1);
- Files reviewed: 23/23 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
…into vkombov/task-17606
…into vkombov/task-17606 Resolve conflicts between the signal/OnPush migration and the virtual-scroll migration of the combos (#17281): keep signal-backed state, adopt the virtual-scroll adapter in the drop-down, keep both new test sets, and port the recycling test to the virtual-scroll API. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…into vkombov/task-17606
Commit 5c0917b swept in my-changes.patch, the performance app's combo-grid sample and its route, and the virtualization-compare demo sample. Remove them from the branch; the local copies stay untracked. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…into vkombov/task-17606
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
- Fix NG0103 in the simple combo when a binding reads its value earlier in the same view: share the combo's value guard as setValueIfChanged. - Restore master's decorator queries. Template content queries are now decorated accessors over signals, which brings back static: true timing and the protected QueryList members. - Track selection changes in IgxSelectionAPIService with a version per component id and drop the selectionRevision counters. Selection mutators run untracked, so effects that call them do not loop. - Keep master's API: deprecated no-op ngAfterViewChecked, protected QueryLists, IgxAutocompleteDirective.cdr and _focusedItem typed any. - Fix items in a disabled select group staying enabled: the group now provides itself as IgxDropDownGroupComponent. - Move the select host class into the host object, remove standalone flags, align backing-signal names and fix misplaced doc comments. - Document the accessor breaking change, the NG0600 note and the deprecation in CHANGELOG, and add specs for every fix. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…into vkombov/task-17606
- Bind the toggle id in the drop-down and select templates instead of copying it in ngOnChanges, so an id set from code reaches the toggle as well. ngOnChanges stays as a deprecated no-op for subclasses that call super.ngOnChanges(). - Back the combo totalItemCount with a signal instead of calling markForCheck from its setter. - Remove the unused protected _width and _height drop-down base fields. - Remove the redundant standalone: true flags. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The combo hosts called markForCheck from every ngDoCheck, so that records mutated in place keep rendering. That checks them whenever their host is checked, which is what ChangeDetectionStrategy.Eager declares, so declare it instead. Their items and drop-down list stay OnPush. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This reverts commit fb57fb0. The combos are declared OnPush again and keep calling markForCheck from ngDoCheck, so records mutated in place still render on the next host check. Moving them to Eager, or to OnPush without that call, is left for a separate, agreed change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ault Angular 22.2 uses ChangeDetectionStrategy.OnPush by default, so the explicit `changeDetection: ChangeDetectionStrategy.OnPush` is redundant. Remove it, and the unused import, from the combo, simple-combo, drop-down, select and virtual-scroll components and from the spec hosts added on this branch. Hosts that declare `Eager` keep it, as that is a real opt-out. Move this branch's CHANGELOG entries out of the released 22.2.0 section into a new Unreleased section, and reword the OnPush note: the components no longer opt out of the default strategy with `Eager`. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Closes #17606
Description
Moves Combo, Simple Combo, Drop Down and Select, with their items and groups, to
OnPush. That is 11 components. The state their templates read is now signal-backed behind the existing properties, so setting it from code repaints the view withoutdetectChanges(), with or without zone.js.No public API is renamed or removed. Converting inputs to
input()is left for a separate PR with migrations.Main changes
disabled,valueandplaceholderor the drop-down'srole.formControl.disable()now disables the rendered select.IgxSelectionAPIServicekeeps a version signal per component, so views andcomputeds follow selection changes.igx-select-item-groupare now disabled.ngAfterViewCheckedon the combos andngOnChangeson the drop-down no longer do anything. They are kept sosupercalls still compile.@HostBinding/@HostListenerare replaced byhostmetadata, and a few unused members are removed.Notes for reviewers
markForCheck()inngDoCheck, so records changed in place, likeitems[1].name = 'x', keep rendering. A follow-up can make that call conditional. Commitsfb57fb0df7andf3daa24949cancel each other out.masterwhen scrolling a combo. The cause is not found yet.Compatibility
Only classes that extend these components can break, for example a subclass that redeclares one of these properties as a field (
TS2610). The CHANGELOG lists every affected member and the fix. No migration is needed.How Has This Been Tested?
69 new tests cover updates from code without
detectChanges(), effects that run once, content projected after initialization and records changed in place.f3daa24949lint:lib,build:libAngular 22.2.0, Chrome Headless 154, Windows 11.
Checklist
CHANGELOG.MDupdates for newly added functionality