TimePicker: Add dateOrder prop to sort day, month, and year
#62481
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
t-hamano
added
[Type] Enhancement
t-hamano
commented
Jun 11, 2024
|
Size Change: +236 B (+0.01%) Total Size: 1.76 MB
ℹ️ View Unchanged
|
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
tyxla
reviewed
Jul 4, 2024
There was a problem hiding this comment.
This is working well for me and I believe it is a good compromise that allows some flexibility while still remaining stable enough.
For a moment, I wondered whether "date order" is a good way to name it, but I think it is, because "date format" would be more confusing, as date format fields are usually more flexible and folks could start adding unexpected tokens there.
I'd prefer to defer the full review to @mirka who has recently been working on some of the time and date components.
mirka
requested changes
Jul 4, 2024
Co-authored-by: Lena Morita <lena@jaguchi.com>
|
@mirka Thanks for the review! I have tried to maintain the default behavior of the component based on feedback. 3ece19e02dd3e268fef79ca19d60e355.mp4I also added another unit test and clarified the documentation. |
t-hamano
commented
Jul 5, 2024
Comment on lines
127
to
129
| is12Hour={ is12HourFormat( | ||
| siteTimeFormat | ||
| ) } |
There was a problem hiding this comment.
I believe this prop is no longer needed as the proper time format for the locale is now applied via dataOrder prop.
There was a problem hiding this comment.
I think we still want the prop because it can show the AM/PM switcher?
There was a problem hiding this comment.
Ah, I missed that 😅 Restored the prop.
t-hamano
commented
Jul 5, 2024
mirka
approved these changes
Jul 5, 2024
Comment on lines
127
to
129
| is12Hour={ is12HourFormat( | ||
| siteTimeFormat | ||
| ) } |
There was a problem hiding this comment.
I think we still want the prop because it can show the AM/PM switcher?
Co-authored-by: Lena Morita <lena@jaguchi.com>
|
While testing this PR again, I discovered that the default value for the Beforebefore.mp4Afterafter.mp4 |
Looks good 👍 |
|
P.S. When I resolved the conflict, I noticed that the |
huubl
pushed a commit
to huubl/gutenberg
that referenced
this pull request
Jul 10, 2024
…ess#62481) * TimePicker: Add `dateOrder` prop to sort day, month, and year * Add changelog * Update unit tests * Fix grammar * Fix changelog * Refactoring * Update packages/block-library/src/post-date/edit.js Co-authored-by: Lena Morita <lena@jaguchi.com> * Revert unwanted changes * `dateOrder` prop overwrites `is12Hour` prop * Add unit test * Clarify the documentation * Restore unit test * Improve test comment * Remove unnecessary is12Hour prop * Improve doc Co-authored-by: Lena Morita <lena@jaguchi.com> * Simplify logic * Restore `is12Hour` prop * Adjust prop position * Remove `dateOrder` prop default value from `DateTimePicker` --------- Co-authored-by: t-hamano <wildworks@git.wordpress.org> Co-authored-by: tyxla <tyxla@git.wordpress.org> Co-authored-by: mirka <0mirka00@git.wordpress.org> Co-authored-by: carstingaxion <carstenbach@git.wordpress.org> Co-authored-by: ramonjd <ramonopoly@git.wordpress.org>
carstingaxion
pushed a commit
to carstingaxion/gutenberg
that referenced
this pull request
Jul 18, 2024
…ess#62481) * TimePicker: Add `dateOrder` prop to sort day, month, and year * Add changelog * Update unit tests * Fix grammar * Fix changelog * Refactoring * Update packages/block-library/src/post-date/edit.js Co-authored-by: Lena Morita <lena@jaguchi.com> * Revert unwanted changes * `dateOrder` prop overwrites `is12Hour` prop * Add unit test * Clarify the documentation * Restore unit test * Improve test comment * Remove unnecessary is12Hour prop * Improve doc Co-authored-by: Lena Morita <lena@jaguchi.com> * Simplify logic * Restore `is12Hour` prop * Adjust prop position * Remove `dateOrder` prop default value from `DateTimePicker` --------- Co-authored-by: t-hamano <wildworks@git.wordpress.org> Co-authored-by: tyxla <tyxla@git.wordpress.org> Co-authored-by: mirka <0mirka00@git.wordpress.org> Co-authored-by: carstingaxion <carstenbach@git.wordpress.org> Co-authored-by: ramonjd <ramonopoly@git.wordpress.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #57007, #38221
What?
This PR proposes to add a new
dateOrderprop to theDateTimePickerandTimePickercomponents, allowing the order of days, months, and years to be changed per locale.Why?
The order of day, month, and year may not be appropriate in all locales. For example, in China and Japan, it is natural to always have year, month, and day in that order.
How?
dataOrderprop aredmy (DD-MM-YY),mdy (MM-DD-YY), orymd (YY-MM-DD). These values are sourced from "List of date formats by country".dataOrderprop. This allows us to control the order of year, month, and day per locale.dmy (DD-MM-YY), and if a locale specifies an incorrect format, this default format will be used.This idea is based on the following approaches that have already been implemented:
Testing Instructions
On the block editor
Default Locale
en).MM-DD-YY, otherwise it should beDD-MM-YY.Other locales
YY-MM-DD.85305dbb2dec1a00c1961137ab73ae25.mp4
On the storybook
Confirm that changing the dateOrder prop swaps the order of the items.
be53454f71cb4b45405280a299c244c2.mp4