Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new demo sample showcasing Angular signal-based forms integrated with Ignite UI for Angular form components, and wires it into the demo app navigation/routing.
Changes:
- Introduces a new
SignalFormsSampleComponentdemo (TS/HTML/SCSS) using@angular/forms/signalswith multiple Ignite UI inputs. - Adds a new
/signal-formsroute to the demo app. - Adds a navigation entry for “Signal Forms” in the main demo navigation list.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/app/signal-forms/signal-forms-sample.component.ts | New demo component implementing a signal-form model, validation, and reset/submit handlers. |
| src/app/signal-forms/signal-forms-sample.component.html | New demo UI showcasing multiple Ignite UI form components bound via [formField]. |
| src/app/signal-forms/signal-forms-sample.component.scss | Styling for the new signal forms demo page layout and error hints. |
| src/app/app.routes.ts | Registers the new signal-forms route and imports the new component. |
| src/app/app.component.ts | Adds “Signal Forms” to the navigation links list. |
You can also share your feedback on Copilot code review. Take the survey.
| <h3>Radio Group</h3> | ||
| <igx-radio-group name="priority"> | ||
| <igx-radio value="low">Low</igx-radio> | ||
| <igx-radio value="medium">Medium</igx-radio> | ||
| <igx-radio value="high">High</igx-radio> | ||
| </igx-radio-group> |
There was a problem hiding this comment.
The "Radio Group" isn’t wired to the signal form model: there is no [formField] binding for priority, so selecting a radio option won’t update model().priority (and the field won’t participate in form state). Bind the radio group to signalForm.priority (and keep the values aligned with the model’s allowed values).
| priority: string; | ||
| volume: number; | ||
| movie: string; | ||
| genres: string[]; | ||
| country: string; | ||
| birthDate: Date; | ||
| dateRange: DateRange; | ||
| meetingTime: Date; | ||
| calendarDate: Date; |
There was a problem hiding this comment.
SignalFormModel declares birthDate, dateRange, meetingTime, and calendarDate as non-nullable, but the component initializes/resets them to null. To keep the type contract accurate (and avoid accidental null dereferences later), update these fields to be nullable (e.g., Date | null, DateRange | null). Also consider narrowing priority from string to a union of the supported values (low/medium/high) since the UI only allows those.
| priority: string; | |
| volume: number; | |
| movie: string; | |
| genres: string[]; | |
| country: string; | |
| birthDate: Date; | |
| dateRange: DateRange; | |
| meetingTime: Date; | |
| calendarDate: Date; | |
| priority: 'low' | 'medium' | 'high'; | |
| volume: number; | |
| movie: string; | |
| genres: string[]; | |
| country: string; | |
| birthDate: Date | null; | |
| dateRange: DateRange | null; | |
| meetingTime: Date | null; | |
| calendarDate: Date | null; |
| protected onReset(): void { | ||
| this.model.set({ | ||
| fullName: '', | ||
| acceptTerms: false, | ||
| enableNotifications: true, | ||
| priority: 'medium', | ||
| volume: 50, | ||
| movie: '', | ||
| genres: [], | ||
| country: '', | ||
| birthDate: null, | ||
| dateRange: null, | ||
| meetingTime: null, | ||
| calendarDate: null, | ||
| }); |
There was a problem hiding this comment.
onReset() duplicates the entire initial model object literal from the model signal initialization. Consider extracting the initial state into a single constant/factory (and reuse it for both initialization and reset) to avoid future drift when fields are added/changed.
Closes #
Additional information (check all that apply):
Checklist:
feature/README.MDupdates for the feature docsREADME.MDCHANGELOG.MDupdates for newly added functionalityng updatemigrations for the breaking changes (migrations guidelines)