feat(grid): expose editorOptions for the default editors and improve built-in editing for date/time cols (input formats)

[ddaribo]

Related to #14009 and #14010

Additional information (check all that apply):

• [ ] Bug fix
• [x] New functionality
• [ ] Documentation
• [ ] Demos
• [ ] CI/CD

Checklist:

• [x] All relevant tags have been applied to this PR
• [x] This PR includes unit tests covering all the new code (test guidelines)
• [x] This PR includes API docs for newly added methods/properties (api docs guidelines)
• [x] This PR includes feature/README.MD updates for the feature docs
• [ ] This PR includes general feature table updates in the root README.MD
• [x] This PR includes CHANGELOG.MD updates for newly added functionality
• [ ] This PR contains breaking changes
• [ ] This PR includes ng update migrations for the breaking changes (migrations guidelines)
• [x] This PR includes behavioral changes and the feature specification has been updated with them - compare revisions

[ddaribo]

@ChronosSF, with the introduced changes, the two properties hardcoding default date/time fromats are left unused: defaultTimeFormat and defaultDateTimeFormat (ref in column.component.ts and ref in grid.interface.ts). Since we discussed these are rather controversial as part of the public API, should they be removed? If so, should this be done in the current PR, or to log a separate issue?

[ChronosSF]

@ChronosSF, with the introduced changes, the two properties hardcoding default date/time fromats are left unused: defaultTimeFormat and defaultDateTimeFormat (ref in column.component.ts and ref in grid.interface.ts). Since we discussed these are rather controversial as part of the public API, should they be removed? If so, should this be done in the current PR, or to log a separate issue?

So normally defaultTimeFormat and defaultDateTimeFormat being hidden/internal should be safe to remove, but since they were sneaked in a public interface it gets more difficult. Can you check where this FieldType is used? I can't remember using this interface. I hope it's not something people can use to represent a column. Check the samples browser / docs for it. If it's used we may need to add a migration and deprecate the props first.

[ddaribo]

@ChronosSF, It is used in the Query Builder, so indirectly in the grid advanced filtering dialog as well. | fields | FieldType[] | An array of fields to be filtered. Contains information about label, field, type, operands. | The ColumnType extends it. FieldType is only mentioned in the Query Builder's topic code-snippets here and is not imported in the demos since the fields are declared as any[]. The two properties are not referenced anywhere in the topics/samples and FieldType is not mentioned in any other topic.

[jackofdiamond5]

The changes look good, though I have a little question. With the introduction of the editorOptions property on the column, I can now set the dateTimeFormat of the column which properly applies the mask/date string I've set while in edit mode. However, this affects only edit mode and I still have to use the pipeArgs to make any changes to the display format (using NG's DatePipe). To me this is an unneeded separation, as if I want to have both display and input/edit formats modified, I will have to set both the property and add a template/modify the pipeArgs. I think it would be more user friendly to do this in one place only - like for example through the exposed editorOptions - set the display and input formats there, ditch the pipeArgs for dates entirely and rely on the editors to handle everything afterwards. As the editors do use the NG's DatePipe internally when they build the display format, so no need to do it on the column level imo. Regarding the input format, with the newest implementation that's been added in the DateTimeUtils we should be able to handle NG's shorthand formats in the editors, like we've mapped them to whatever formats we've deemed appropriate. So atm at least I cannot see a reason why we don't leave the editors to handle both input and display formats, providing they've been given such throgh the editorOptions.

One little thing that I saw, which may have been discussed previously is that through the editorOptions I can set a shortTime string to a column with a data type set to date. Which results in a rather funny behavior where during editing you see a time format and while the cell is outside of editing you see the display format of the date portion. Additionally, it is a date picker, so yeah - DateTimePicker confirmed I guess 😆?

Anyway, the same is true with a time column where we set the dateTimeFormat to shortDate. And while I agree that it's a niche case, it doesn't hurt to guard against it and to default to a format that will actually make sense in the given context and be useful rather than looking broken.

[ChronosSF]

The changes look good, though I have a little question. With the introduction of the editorOptions property on the column, I can now set the dateTimeFormat of the column which properly applies the mask/date string I've set while in edit mode. However, this affects only edit mode and I still have to use the pipeArgs to make any changes to the display format (using NG's DatePipe). To me this is an unneeded separation, as if I want to have both display and input/edit formats modified, I will have to set both the property and add a template/modify the pipeArgs. I think it would be more user friendly to do this in one place only - like for example through the exposed editorOptions - set the display and input formats there, ditch the pipeArgs for dates entirely and rely on the editors to handle everything afterwards. As the editors do use the NG's DatePipe internally when they build the display format, so no need to do it on the column level imo. Regarding the input format, with the newest implementation that's been added in the DateTimeUtils we should be able to handle NG's shorthand formats in the editors, like we've mapped them to whatever formats we've deemed appropriate. So atm at least I cannot see a reason why we don't leave the editors to handle both input and display formats, providing they've been given such throgh the editorOptions.

One little thing that I saw, which may have been discussed previously is that through the editorOptions I can set a shortTime string to a column with a data type set to date. Which results in a rather funny behavior where during editing you see a time format and while the cell is outside of editing you see the display format of the date portion. Additionally, it is a date picker, so yeah - DateTimePicker confirmed I guess 😆?

Anyway, the same is true with a time column where we set the dateTimeFormat to shortDate. And while I agree that it's a niche case, it doesn't hurt to guard against it and to default to a format that will actually make sense in the given context and be useful rather than looking broken.

I don't necessarily agree about the guards and the implementation of those can be quite an endeavor . As long as we don't infer the options incorrectly we can let users make mistakes if they choose to. I think it would be quite obvious for everyone using this option why this happened.

As for the disjointed options, I think you have a point there and don't forget that we also have a formatter that also fights for precedence in how the cell is rendered. We'll have to discuss this more with @damyanpetev

[ddaribo]

With the introduction of the editorOptions property on the column, I can now set the dateTimeFormat of the column which properly applies the mask/date string I've set while in edit mode. However, this affects only edit mode and I still have to use the pipeArgs to make any changes to the display format (using NG's DatePipe). To me this is an unneeded separation, as if I want to have both display and input/edit formats modified, I will have to set both the property and add a template/modify the pipeArgs.

@jackofdiamond5, If I understand you correctly, what I can say is that the agreed implementation does this, only the other way around - if the editorOptions.dateTimeFormat is not set, the pipeArgs.format (if set) is applied as input format to the built-in editors, as well as obviously display format for the cell. The editorOptions.dateTimeFormat would be handy if we need a specific input format, different from the default locale one and from the display format set via pipeArgs.

... ditch the pipeArgs for dates entirely and rely on the editors to handle everything afterwards. As the editors do use the NG's DatePipe internally when they build the display format, so no need to do it on the column level imo. Regarding the input format, with the newest implementation that's been added in the DateTimeUtils we should be able to handle NG's shorthand formats in the editors, like we've mapped them to whatever formats we've deemed appropriate. So atm at least I cannot see a reason why we don't leave the editors to handle both input and display formats, providing they've been given such throgh the editorOptions.

I may not fully understand this suggestion yet, but isn't the pipeArgs.format rather applied as format on the cell display value and not the editor? When not in edit mode, this is simply a div with formatted text, so this is handled by the column indeed.

[jackofdiamond5]

With the introduction of the editorOptions property on the column, I can now set the dateTimeFormat of the column which properly applies the mask/date string I've set while in edit mode. However, this affects only edit mode and I still have to use the pipeArgs to make any changes to the display format (using NG's DatePipe). To me this is an unneeded separation, as if I want to have both display and input/edit formats modified, I will have to set both the property and add a template/modify the pipeArgs.

@jackofdiamond5, If I understand you correctly, what I can say is that the agreed implementation does this, only the other way around - if the editorOptions.dateTimeFormat is not set, the pipeArgs.format (if set) is applied as input format to the built-in editors, as well as obviously display format for the cell. The editorOptions.dateTimeFormat would be handy if we need a specific input format, different from the default locale one and from the display format set via pipeArgs.

... ditch the pipeArgs for dates entirely and rely on the editors to handle everything afterwards. As the editors do use the NG's DatePipe internally when they build the display format, so no need to do it on the column level imo. Regarding the input format, with the newest implementation that's been added in the DateTimeUtils we should be able to handle NG's shorthand formats in the editors, like we've mapped them to whatever formats we've deemed appropriate. So atm at least I cannot see a reason why we don't leave the editors to handle both input and display formats, providing they've been given such throgh the editorOptions.

I may not fully understand this suggestion yet, but isn't the pipeArgs.format rather applied as format on the cell display value and not the editor? When not in edit mode, this is simply a div with formatted text, so this is handled by the column indeed.

I see what you mean and yeah I've missed the part where outside of edit mode the cell is just a div with formatting. So, yeah this pretty much answered my question - the formatting will be applied as expected on the editor as long as pipeArgs.format is present. In this case it makes sense for the dateTimeFormat to be applied only while editing, to provide additional customization options. Though, we may want to change the name to something that reflects that it is to be used specifically when the cell is in edit mode like dateEditFormat or something. Also, I like that this works well with a templated cell, since you can provide whatever to the DatePipe for a display value and then set the pipeArgs.format which will reflect only onto the edit mode format. And I don't know if this has been discussed or not but since this is yet another way to apply formatting to a cell, we may want to revisit the formatter and the cell templating options because one is bound to override the other and we should add this to the docs. Like in the aforementioned example - the templated cell will override the display format set by pipeArgs.format.

[damyanpetev]

Hopefully the name of the main property being editorOptions makes it pretty clear it's targeting edit inputs (though we're discussing other interactions), so I'm thinking editorOptions.dateTimeFormat should be clear enough. If we include filtering in the mix, it might make more sense as inputOptions or something else, could still hopefully make it clear at the root, the format property is long enough already 🥴

And true the options are split, if anything pipeArgs is a bit too direct with the inner workings for my taste and should've probably been display/formatOptions or something, but that ship has sailed long ago. Similarly for the order of competing pipe options, column formatters and templates - those being a bit out of scope for this change, besides making sure those sync where meaningful.