feat(datetime): formatOptions for time button and header - #29009
Conversation
revert example
Co-authored by: Sean Perkins <sean@ionic.io>
github-actions
Bot
added
package: core
| /** | ||
| * Formatting options, separated by date and time. | ||
| */ | ||
| @Prop() formatOptions?: DatetimeFormatOptions; |
There was a problem hiding this comment.
Optional: It would be cool to have a test HTML page to mess around with this ready to go. Maybe something with a text box that lets you enter a value for formatOptions dynamically, and a button to update the datetime with what you've entered? Definitely only do this if you have spare time though.
There was a problem hiding this comment.
It would be great to also improve the JSDoc comment here. This is the information that will show in the docs and intellisense.
Would be good to include what format it requires and maybe a link out to MDN for those rules.
There was a problem hiding this comment.
i improved the comment
| /** | ||
| * Formatting options, separated by date and time. | ||
| */ | ||
| @Prop() formatOptions?: DatetimeFormatOptions; |
There was a problem hiding this comment.
It would be great to also improve the JSDoc comment here. This is the information that will show in the docs and intellisense.
Would be good to include what format it requires and maybe a link out to MDN for those rules.
liamdebeasi
left a comment
liamdebeasi
left a comment
There was a problem hiding this comment.
I'm still looking into the type error we discussed on Slack
averyrousseau
left a comment
averyrousseau
left a comment
There was a problem hiding this comment.
Nice work on this!
|
Can we point this towards a feature branch instead of main? Might need to create a Also, it might be worth merging this into a branch and have that branch get merged into |
liamdebeasi
left a comment
liamdebeasi
left a comment
There was a problem hiding this comment.
Functionality is mostly good. I had one questions on some of the validation, and then a few suggestions for code cleanup.
liamdebeasi
left a comment
liamdebeasi
left a comment
There was a problem hiding this comment.
Implementation works well, just one question for my understanding.
| printIonWarning(`Datetime: The '${presentation}' presentation requires a date object in formatOptions.`); | ||
| } | ||
| break; | ||
| case 'time': |
There was a problem hiding this comment.
mainly for my understanding: This is still needed for the upcoming ion-datetime-button integration right? For presentation="time" there's nothing in the datetime UI that would need formatOptions:
However, this would be needed for the time display in datetime button. Am I understanding correctly?
There was a problem hiding this comment.
That's correct!
5 participants
Issue number: Internal
What is the current behavior?
The Datetime header and time button have default date formatting that cannot be set by the developer.
What is the new behavior?
Does this introduce a breaking change?