fix(forms): Support Signal Forms in form controls - #17566
Conversation
The `[formField]` interop `NgControl` exposes signal-backed getters only. It has no `statusChanges`, `valueChanges`, `validator`, `markAsTouched` or `setValue`, so igxInput, checkbox, switch, radio group, select, combo, simple combo and the date, time and date range pickers threw on init. Add `NgControlAdapter` in core as the single access path to the bound `NgControl`. It detects the backend and derives the missing observables from a root effect over the signal getters, keeping change detection order identical to the observable case. Controls no longer read `NgControl` internals directly. Closes #17556
rkaraivanov
added
❌ status: awaiting-test
There was a problem hiding this comment.
🔵 Needs a closer look
It introduces a new core abstraction (NgControlAdapter) and updates multiple form controls’ initialization/validation wiring, which warrants final human review despite the added tests.
Pull request overview
This PR fixes initialization/runtime failures when Ignite UI form controls are bound via Angular Signal Forms ([formField]) by routing all NgControl access through a new adapter that normalizes missing observable APIs and control methods in the signal-backed interop control.
Changes:
- Added
NgControlAdapterincoreto provide a single, backend-aware access path forNgControl(observable vs signal). - Updated multiple form controls (input, checkbox/switch base, radio group, select, combo/simple-combo, date/time pickers) to use the adapter instead of reading
NgControlinternals directly. - Added Signal Forms-focused unit tests for the affected components and updated docs/README/CHANGELOG to document the new compatibility.
File summaries
| File | Description |
|---|---|
| skills/igniteui-angular-components/references/form-controls.md | Documents how to use Ignite UI controls with Signal Forms via [formField]. |
| projects/igniteui-angular/time-picker/src/time-picker/time-picker.component.ts | Uses NgControlAdapter for required/validity/status handling under Signal Forms. |
| projects/igniteui-angular/time-picker/src/time-picker/time-picker.component.spec.ts | Adds Signal Forms coverage for required/invalid/disabled behaviors. |
| projects/igniteui-angular/switch/src/switch/switch.component.spec.ts | Adds Signal Forms coverage for required/invalid/disabled behaviors. |
| projects/igniteui-angular/simple-combo/src/simple-combo/simple-combo.component.spec.ts | Adds Signal Forms coverage for required/invalid/disabled behaviors. |
| projects/igniteui-angular/select/src/select/select.component.ts | Switches status/required/validity logic to the adapter for Signal Forms support. |
| projects/igniteui-angular/select/src/select/select.component.spec.ts | Adds Signal Forms coverage for required/invalid/disabled behaviors. |
| projects/igniteui-angular/radio/src/radio/radio-group/radio-group.directive.ts | Uses the adapter to safely consume status/required/validators under Signal Forms. |
| projects/igniteui-angular/radio/src/radio/radio-group/radio-group.directive.spec.ts | Adds Signal Forms coverage for required/invalid behaviors. |
| projects/igniteui-angular/input-group/src/input-group/directives-input/input.directive.ts | Uses the adapter for status/value/touched tracking and write/touch interop. |
| projects/igniteui-angular/input-group/src/input-group/directives-input/input.directive.spec.ts | Adds Signal Forms coverage for required/invalid/disabled/reset behaviors. |
| projects/igniteui-angular/input-group/README.md | Notes igxInput compatibility with Signal Forms ([formField]). |
| projects/igniteui-angular/directives/src/directives/checkbox/checkbox-base.directive.ts | Switches checkbox/switch validity & required resolution to the adapter. |
| projects/igniteui-angular/date-picker/src/date-range-picker/date-range-picker.component.ts | Uses the adapter for status/required/validity + signal-backend revalidation hook. |
| projects/igniteui-angular/date-picker/src/date-range-picker/date-range-picker.component.spec.ts | Adds Signal Forms coverage for required/invalid/disabled behaviors. |
| projects/igniteui-angular/date-picker/src/date-range-picker/date-range-picker-inputs.common.ts | Uses adapter-based setValue handling to support signal-backend “ignored write” semantics. |
| projects/igniteui-angular/date-picker/src/date-picker/date-picker.component.ts | Switches status/required/validity logic to the adapter for Signal Forms support. |
| projects/igniteui-angular/date-picker/src/date-picker/date-picker.component.spec.ts | Adds Signal Forms coverage for required/invalid/disabled behaviors. |
| projects/igniteui-angular/core/src/public_api.ts | Exports the new NgControlAdapter from the core public API. |
| projects/igniteui-angular/core/src/core/ng-control-adapter.ts | Introduces NgControlAdapter and signal-backed observable derivations via root effects. |
| projects/igniteui-angular/combo/src/combo/combo.component.spec.ts | Adds Signal Forms coverage for required/invalid/disabled behaviors. |
| projects/igniteui-angular/combo/src/combo/combo.common.ts | Switches combo validity/required/status wiring to use the adapter. |
| projects/igniteui-angular/checkbox/src/checkbox/checkbox.component.spec.ts | Adds Signal Forms coverage for required/invalid/disabled behaviors. |
| CHANGELOG.md | Adds an Unreleased entry documenting Signal Forms compatibility across form controls. |
Review details
- Files reviewed: 24/24 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| if (this.control) { | ||
| this._statusChanges$ = this.control.statusChanges.subscribe(this.onStatusChanged.bind(this)); | ||
| this._valueChanges$ = this.control.valueChanges.subscribe(this.onValueChanged.bind(this)); | ||
| this._touchedChanges$ = this.control.touchedChanges.subscribe(this.updateValidityState.bind(this)); | ||
| } |
viktorkombov
added
💥 status: in-test
viktorkombov
left a comment
viktorkombov
left a comment
There was a problem hiding this comment.
Tested on a zoneless demo page (provideZonelessChangeDetection()) with [formField] on each of the ten controls. All six also reproduce under zone.js, so none of them are zoneless-specific.
Two blocking:
- Checkbox / switch / radio throw on any validator that reads
control.value, becauserequireddetection now probes with{}. Reactive Forms path, no Signal Forms involved. Confirmed as a regression by reverting the two files to master individually. - Radio group gets stuck disabled under Signal Forms — it disables but never re-enables. Looks pre-existing rather than introduced here, but this is what makes it reachable.
Four more, not blocking:
- The conditional
requiredmarker goes stale on select, combo and both pickers. It corrects on the user's first interaction with the field, then sticks for good. - A satisfied custom rule never reaches
VALID, where Reactive Forms do. - The date range picker ends up drawing the asterisk for a rule that is switched off. Half of that one is pre-existing and hits Reactive Forms too.
Validators.requiredTruenow counts as required on checkbox/switch/radio where master said no.
Also checked, and clean, so nobody re-treads them:
- the adapter's effects tear down on
@ifdestruction and repeated mount/unmount, with no leaked emissions - switching
[formField]between fields at runtime submit()marking untouched fields touchedclear()writing back to the model- the date range picker's projected two-input write fallback, on both backends
- rapid model updates coalescing into one transition
| * Signal Forms `submit()` only marks fields touched, so touched changes count too | ||
| * or the errors would never surface. | ||
| */ | ||
| public get statusChanges(): Observable<unknown> { |
There was a problem hiding this comment.
required isn't in the watched tuple, so a rule that flips while the field stays valid produces no emission and onStatusChanged doesn't run.
required on igx-input-group--required = false (stale)
first interaction igx-input-group--required = true (catches up)
required off igx-input-group--required = true (stale again)
any further interaction igx-input-group--required = true (stuck)
It corrects on the next transition of valid, invalid, pending, disabled, dirty or touched. In practice that's the user's first interaction, which flips touched and dirty.
Both of those are one-way, so nothing refreshes it after that. A programmatic value change doesn't help either.
Same on combo, date picker and time picker. igxInput only goes stale on aria-required, since its asterisk comes through the directive's own @Input.
Specific to the signal backend added here — the Reactive Forms path still emits through updateValueAndValidity() and is untouched.
| } | ||
|
|
||
| /** Signal Forms expose no validator list, only `required` and the current errors. */ | ||
| public get hasValidators(): boolean { |
There was a problem hiding this comment.
required || invalid — a valid, non-required field with a custom rule is neither. So it reports no validators, and updateValidityState drops to INITIAL where Reactive Forms give VALID.
To see it: use validate(p.role, ...) instead of required(), then satisfy it while the control has focus. The success state never appears. Same for a resolved validateAsync.
The observable branch below keeps the old validator || asyncValidator check, so this is specific to the signal backend added here.
Note the check sits inside the touchedOrDirty branch, which makes the result order-dependent.
| Validators.required | ||
| ); | ||
| if (this.control.hasValidators) { | ||
| this._required = this.control.required; |
There was a problem hiding this comment.
NgControlAdapter.required probes the validator with a bare {}, so every custom validator gets a control with value === undefined. Anything that dereferences the value throws:
const v = (c: AbstractControl): ValidationErrors | null =>
(c.value as string[]).length === 0 ? { empty: true } : null;
form = new FormGroup({ accepted: new FormControl<unknown>([], v) });<igx-checkbox formControlName="accepted">Accept</igx-checkbox>TypeError: Cannot read properties of undefined (reading 'length') out of ngAfterViewInit, and change detection stops at that control. Master returns false for the same setup. radio-group.directive.ts:520 has the same change and the same failure.
| if (this._ngControl) { | ||
| this._statusChanges$ = this._ngControl.statusChanges!.subscribe(this.onStatusChanged.bind(this)); | ||
| if (this._control) { | ||
| this._statusChanges$ = this._control.statusChanges.subscribe(this.onStatusChanged.bind(this)); |
There was a problem hiding this comment.
The asterisk is CSS on .igx-input-group--required, and that class is a host binding over inputGroup.isRequired, so the value has to be written and then painted. Two things go wrong.
Cause one: this subscription doesn't emit for a required-only change. onStatusChanged never runs, setRequiredToInputs() (line 938) isn't called, and nothing is written.
Cause two: when something else does make it run, it writes inside a Promise.resolve().then() (line 1078) with no markForCheck(). The write sits unpainted until a later pass — by which point the rule may have flipped back:
isRequired written --required painted
required on false, false, false false, false, false
fields touched true, true, true false, false, false
required off true, true, true true, true, true
On where these come from. The first is new, since it's the signal backend of the adapter this PR adds. The second is not — it reproduces on the plain Reactive Forms path with no Signal Forms involved, which is worth knowing on its own:
ctrl.addValidators(Validators.required);
ctrl.updateValueAndValidity();
// isRequired written = true, --required painted = false, and it never paintsSo fixing the adapter won't clear this one. With the adapter watching required, the marker still didn't render until a markForCheck() went in.
Two reasons the new spec stays green:
- its setup is
detectChanges(); tick(); detectChanges();, which manually does the job of the missingmarkForCheck() - the test component uses
required(path.range)with nowhen, so the rule is on from the start and the initial emission covers it
| Validators.required | ||
| ); | ||
| if (this.control.hasValidators) { | ||
| this._required = this.control.required; |
There was a problem hiding this comment.
Side effect of the same change: checkbox, switch and radio now report required = true and aria-required="true" for Validators.requiredTrue, where master reported false. igxInput already reported true on master, so this aligns them. Flagging in case it should be in the CHANGELOG.
|
if (this.ngControl!.disabled) {
button.disabled = this.ngControl!.disabled;
}The write is guarded by the condition it's writing, so it only ever sets isDisabled = signal(false);
f = form(this.model, p => disabled(p.season, { when: () => this.isDisabled() }));Set Not introduced here — Also, going into disabled the native |
|
Description
The
[formField]interopNgControlexposes signal-backed getters only. It has nostatusChanges,valueChanges,validator,markAsTouchedorsetValue, so igxInput, checkbox, switch, radio group, select, combo, simple combo and the date, time and date range pickers threw on init.Add
NgControlAdapterin core as the single access path to the boundNgControl. It detects the backend and derives the missing observables from a root effect over the signal getters, keeping change detection order identical to the observable case. Controls no longer readNgControlinternals directly.Closes #17556
Type of Change (check all that apply):
How Has This Been Tested?
Checklist:
CHANGELOG.MDupdates for newly added functionality