Skip to content

refactor(combo, drop-down, select): switch to OnPush with signal-backed state - #17622

Open
viktorkombov wants to merge 20 commits into
masterfrom
vkombov/task-17606
Open

viktorkombov wants to merge 20 commits into
masterfrom
vkombov/task-17606

Conversation

@viktorkombov

@viktorkombov viktorkombov commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor

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 without detectChanges(), 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

  • Stale rendering fixed: writes from code now reach the view, for example the select's disabled, value and placeholder or the drop-down's role. formControl.disable() now disables the rendered select.
  • Selection: IgxSelectionAPIService keeps a version signal per component, so views and computeds follow selection changes.
  • Bug fix: items in a disabled igx-select-item-group are now disabled.
  • Deprecated: ngAfterViewChecked on the combos and ngOnChanges on the drop-down no longer do anything. They are kept so super calls still compile.
  • Clean-up: @HostBinding/@HostListener are replaced by host metadata, and a few unused members are removed.

Notes for reviewers

  • The combos still re-check with their host. They are OnPush but call markForCheck() in ngDoCheck, so records changed in place, like items[1].name = 'x', keep rendering. A follow-up can make that call conditional. Commits fb57fb0df7 and f3daa24949 cancel each other out.
  • Scrolling cost: an earlier measurement showed about 20 to 30% more script time than master when 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.

Run on f3daa24949 Result
Combo, Simple Combo, Drop Down, Select 741 / 741
Grid and other consumers 1,073 / 1,075, two scroll tests with fixed waits that pass on rerun
Angular Elements 21 / 21
lint:lib, build:lib pass

Angular 22.2.0, Chrome Headless 154, Windows 11.

Checklist

  • All relevant tags have been applied to this PR
  • This PR includes unit tests covering all the new code
  • This PR includes API docs for newly added methods/properties (no new public API)
  • This PR includes CHANGELOG.MD updates for newly added functionality
  • This PR contains breaking changes (for subclasses only, see Compatibility)
  • Accessibility (ARIA, keyboard navigation, focus management) has been verified by the zoneless tests

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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.selected still reads selection.first_item(...) directly, without consuming selectionRevision. After this item becomes OnPush, setSelectedItem() and clearSelection() can update the drop-down revision while the virtual row's aria-selected and 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.selected reads selection.first_item(...) directly and does not consume selectionRevision. Thus setSelectedItem()/clearSelection() only update this signal while a recycled OnPush item can keep stale aria-selected and 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 text value in the plain _text field. 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. Make text signal-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, but IgxSelectItemComponent.selected still only reads the non-reactive selection service. With the item now OnPush, select.value = ... can update the input text while the item's aria-selected attribute 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.

Comment thread projects/igniteui-angular/combo/src/combo/combo.common.ts Outdated
Comment thread projects/igniteui-angular/select/src/select/select-item.component.ts Outdated
viktorkombov and others added 8 commits September 15, 2026 16:17
…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>
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>
@viktorkombov
viktorkombov marked this pull request as ready for review September 25, 2026 09:11
viktorkombov and others added 5 commits September 30, 2026 15:33
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>
@viktorkombov viktorkombov added 🛠️ status: in-development Issues and PRs with active development on them and removed ❌ status: awaiting-test PRs awaiting manual verification labels Oct 5, 2026
- 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>
viktorkombov and others added 3 commits October 5, 2026 14:44
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>
@viktorkombov viktorkombov added ❌ status: awaiting-test PRs awaiting manual verification and removed 🛠️ status: in-development Issues and PRs with active development on them labels Oct 5, 2026
viktorkombov and others added 2 commits October 6, 2026 10:40
…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>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Migrate the Combo and Drop Down components to signals and modern Angular APIs

2 participants