fix(material/chips): chips erasing values when focusing out of input #26373
Conversation
vmuresanu
force-pushed
the
fix-chips-bug
branch
from
e9824fb to
f6ece21
Compare
vmuresanu
requested review from
crisbeto
and removed request for
zarend and
mmalerba
vmuresanu
force-pushed
the
fix-chips-bug
branch
2 times, most recently
from
5b6970e to
aadeb29
Compare
…bound to reactive forms
vmuresanu
force-pushed
the
fix-chips-bug
branch
from
aadeb29 to
5a4b2e9
Compare
| (edited)="edit(person, $event)"> | ||
| <mat-chip-row | ||
| *ngFor="let person of people" | ||
| [editable]="true" |
There was a problem hiding this comment.
Why was this one hardcoded to true? We have a button above that is used to toggle the editing functionality.
| /> | ||
| </mat-form-field> | ||
|
|
||
| <h4>Input is last child of chip grid with Reactive Forms</h4> |
There was a problem hiding this comment.
Why do we need a special case for when it's the last child?
There was a problem hiding this comment.
The approach with Reactive Forms is slightly different from the one without as you are working with form control. That's why I thought that it would be better to have both versions in example. If you think that it's not worth it I can take it out.
| @@ -88,6 +88,9 @@ export class MatChipRow extends MatChip implements AfterViewInit { | |||
|
|
|||
| @Input() editable: boolean = false; | |||
|
|
|||
| /** Function that maps option's control value to its display value. Used in case of editable="true" and having an object as value */ | |||
| @Input() displayWith: ((value: any) => string) | null = null; | |||
There was a problem hiding this comment.
I'm not sure that having a function like this is appropriate here. While it does allow us to map the object value to a string to be shown in the chip input, the user is still going to be editing a string so the application author won't be able to re-assign the same value back to the chip.
I'm not the one who wrote the editing functionality, but looking through the template, I can see that we allow authors to provide a their own edit input: https://github.com/angular/components/blob/main/src/material/chips/chip-row.html#L25 Maybe it'll be more appropriate to handle the value at the matChipEditInput level?
There was a problem hiding this comment.
We can have an input getByProperty (or some other name) which accepts a property name as a string. And inside matChipEditInput we can check if getByProperty is present, access the value by it, otherwise - we have a string value and we keep the default behavior. Your thoughts?
There was a problem hiding this comment.
@crisbeto can you please provide input here? I want to know if it is a good approach before doing a commit and then refactor it
Fixes #26358