refactor(select): use model for value input#2311
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the selection strategy in the element-ng library by replacing Input and output with Angular model signals for better state management. It updates several components, including SiRelativeDateComponent, SiPhoneNumberInputComponent, and SiSelectMultiValueDirective, to support these changes. Review feedback identified a potential runtime crash in SiRelativeDateComponent due to improper handling of undefined values, a violation of the repository style guide regarding the use of effect() for state propagation in SiSelectSelectionStrategy, and a discrepancy in JSDoc documentation for the value model in SiSelectSingleValueDirective.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
21864d0 to
8e4941f
Compare
This is a preparation for the signal form migration. As we need to have a `value` `model` for that. As a side effect we also drop the remaining `@Input` within the `si-select`. This change is causing a breaking change, as for single value we cannot provide a default so `valueChange` will always include undefined. For multi select we provide a default, so `undefined` is no longer a valid value. BREAKING CHANGE: `value/valueChange` of the `si-select` are now a signal `model`. This slightly changes the behavior: - In case of single select, `valueChange` can now emit `undefined`. - In case of multi select, `undefined` can no longer be assigned to `value`.
8e4941f to
a5e5a20
Compare
This is a preparation for the signal form migration.
As we need to have a
valuemodelfor that.As a side effect we also drop the remaining
@Inputwithin the
si-select.This change is causing a breaking change,
as for single value we cannot provide a default so
valueChangewill always include undefined.
For multi select we provide a default, so
undefinedis no longer a valid value.
BREAKING CHANGE:
value/valueChangeof thesi-selectare now a signalmodel.This slightly changes the behavior:
valueChangecan now emitundefined.undefinedcan no longer be assigned tovalue.ngModel/formControl,valueChangewill now emitDocumentation.
Examples.
Dashboards Demo.
Playwright report.
Coverage Reports: