feat(material/button-toggle): improve signal forms support #31964
Conversation
Adds support for using the `name` from signal formsm and adds tests to verify state synchronization via the `[control]` directive
mmalerba
requested review from
adolgachev and
ok7sai
and removed request for
a team
angular-robot
bot
added
detected: feature
| get name(): string { | ||
| return this._name; | ||
| // When using signal forms, prefer the signal forms name. | ||
| return this._signalFormsControl?.state().name() ?? this._name; |
There was a problem hiding this comment.
Users could just manually do this right?
<mat-button-toggle-group [control]="field" [name]="field().name()">Is this just about the convenience? Are we not going to have to repeat this for every custom control using ControlValueAccessor?
There was a problem hiding this comment.
Yeah, in order for Angular Material to be able to support the full control directive magic for inputs beyond the ones supported in CVA, they would either need to migrate to signal based inputs so they can support the FormUiControl interface (this is difficult for libraries that have a lot of users), or they need to manually integrate with the additional properties they care about using this pattern.
This PR is intended as an example for the kind of changes we would probably make across the library, and we can discuss how worth it or not that is before going ahead with it
| get name(): string { | ||
| return this._name; | ||
| // When using signal forms, prefer the signal forms name. | ||
| return this._signalFormsControl?.state().name() ?? this._name; |
There was a problem hiding this comment.
I think [name] should take precedence, and only use signalFormsName if it's not set?
There was a problem hiding this comment.
Once the Control directive is properly type checked it shouldn't even be possible to set [control] and [name] inputs at the same time
| private _selectionModel: SelectionModel<MatButtonToggle>; | ||
|
|
||
| private _cachedSignalFormsControl: Control<unknown> | null; | ||
| private get _signalFormsControl() { |
There was a problem hiding this comment.
should we create an injectFormControl helper for a more fluent API?
There was a problem hiding this comment.
There's no existing precedent for that, that I know of... so I would say probably no
Adds support for using the
namefrom signal forms and adds tests to verify state synchronization via the[control]directive