chore(*): Sync editValue changes with the formControl value and mark … #12076
Conversation
…as touched for better custom template handling.
MayaKirova
added
grid: general
❌ status: awaiting-test
| @@ -870,7 +870,6 @@ export class IgxGridCellComponent implements OnInit, OnChanges, OnDestroy, CellT | |||
| // while in edit mode subscribe to value changes on the current form control and set to editValue | |||
| this.formControl.valueChanges.pipe(takeWhile(x => this.editMode)).subscribe(value => { | |||
| this.editValue = value; | |||
There was a problem hiding this comment.
If this being a setter also goes and sets the value right back to the form control, wouldn't it sort of double dip with the validation logic? Wondering if we could use the form control as the backing for editValue entirely (assuming getting it from the service is quick), which might allow us to even skip this sync sub?
…er cell exits edit mode.
There was a problem hiding this comment.
Minor comments, of which only the getter is of some importance.
We should still leave an explanation in the docs that using the [formControl] directive to bind edit templates in combination with blur trigger will leave the cell editValue unsynced until blur/edit exit due to the way the form control handles its value.
| @@ -97,10 +97,30 @@ export class IgxCell { | |||
| public rowIndex: number, | |||
| public column, | |||
| public value: any, | |||
| public editValue: any, | |||
| public _editValue: any, | |||
There was a problem hiding this comment.
Assuming IgxCell is internal (fairly certain it is), I guess this can remain public too. Could've been done with an extra method to assign the value too.
| public rowData: any, | ||
| public grid: GridType) { | ||
| this.grid.validation.create(id.rowID, rowData); | ||
| this.editValue = this._editValue; |
There was a problem hiding this comment.
This will likely trigger an extra validation, though I think I saw the public API creating this with different value/editValue initially, so we might be stuck with this.
| const formControl = this.grid.validation.getFormControl(this.id.rowID, this.column.field); | ||
| if (formControl) { | ||
| return formControl.value; | ||
| } |
There was a problem hiding this comment.
Kind of surprised, this doesn't break the model bind with blur if the value changes, yet the getter still returns the old one. It works, but still perhaps it'd be better if we return the pending value for blur for consistency?
skrustev
added
💥 status: in-test
|
Thee only thing I found s that an error is throw when
|
skrustev
added
✅ status: verified
* fix(excel-exporter): export correct number of rows#12086 * fix(excel-exporter): fix total rowCount with multi col headers#12086 * fix(excel-exporter): fix spacing#12086
|
|
||
| constructor( | ||
| public id, | ||
| public rowIndex: number, | ||
| public column, | ||
| public value: any, | ||
| public editValue: any, | ||
| public _editValue: any, | ||
| public rowData: any, | ||
| public grid: GridType) { | ||
| this.grid.validation.create(id.rowID, rowData); |
There was a problem hiding this comment.
@MayaKirova There's one odd case where the grid public updateCell API creates a new IgxCell with a different edit value and the form control will be out of sync here. That's actually not new it seems, however, with these changes the so will be the new value used by the API call (it just uses the args) to perform the update.
That's fortunately for blur only and could be logged separately, if we consider it still a minor use, though it's API that should work regardless.
Ideally, we'd initialize the form control with the edit value regardless instead of setting it additionally.
MayaKirova
added
❌ status: awaiting-test
| const control = new FormControl(value, { updateOn: this.grid.validationTrigger }); | ||
| control.setValue(value); |
There was a problem hiding this comment.
Um.. if the control was just initialized with the same value, I'm kind of missing the point of assigning it again. Any specific reason?
There was a problem hiding this comment.
Yes. It's due to a rather weird behavior when the initial value is undefined (which we use when creating an adding row for example, all fields have undefined as value). When the form control is created for undefined it actually sets it to null instead:
So this:
new FormControl(undefined, { updateOn: this.grid.validationTrigger });
Then returns null for control.value.
Which breaks some tests.
skrustev
added
💥 status: in-test
skrustev
added
✅ status: verified
…as touched for better custom template handling.
Closes #12066
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)