feat: Update year and month dropdown order based on locale #2814
Conversation
chuganzy
force-pushed
the
dropdown-order
branch
from
6fe6d0f to
7ef49e6
Compare
| {formatYearDropdown(calendarMonth.date, dateLib)} | ||
| </span> | ||
| )} | ||
| {dropdownTypes.map((type) => { |
There was a problem hiding this comment.
This new feature will also adapt the month and year labels, not just the dropdowns.
Could you rename dropdownTypes to a more generic name that refers to the date-time format of the locale?
| </span> | ||
| )} | ||
| {dropdownTypes.map((type) => { | ||
| const uiKey = |
There was a problem hiding this comment.
For me, it's better to be explicit, as in the previous version.
Could you refactor this to have if statements that return the component with the explicit props applied to it, like:
if (type === month) {
return // return the same code block as before for month
}
if (type === year) {
return // return the same code block as before for year
}| : formatYearDropdown; | ||
|
|
||
| return ( | ||
| <Fragment key={type}> |
There was a problem hiding this comment.
Fragment is not needed, key can be passed to the Dropdown component, and span.
| expect(comparePosition()).toBe(Node.DOCUMENT_POSITION_FOLLOWING); | ||
|
|
||
| rerender(<Dropdown locale={ja} />); | ||
| expect(comparePosition()).toBe(Node.DOCUMENT_POSITION_PRECEDING); |
There was a problem hiding this comment.
This new feature will also change the way date text is displayed, so we can assert that the text is rendered in the right order using toHaveAccessibleName, like the other tests.
But to do so, we also need to update the accessible name of the calendar, could you update the labelGrid to have the same dynamic logic based on locale?
Please extract the logic to compute the month/year order based on locale to a separate function.
|
This is an interesting take. I have some questions:
|
|
Apologies for leaving this open for a while..
Many Asian countries such as Japan, China, Korea and more, and yes I added a test to cover this.
Let me come up with the way.
Because for captions and labels we already have a way to override, and it is necessary for these locales to override them to properly localize (although probably most developers would just ignore), so I initially thought it can be decoupled from this PR. However, indeed I can come up with the way to take care of it along with the previous question. |
|
@chuganzy Thanks so much for your insights here! I see that the issue isn’t just about the drop-downs, but also affects the formatting of the caption in formatCaption and the grid label in labelGrid. I think it makes sense to take a broader approach and add a function to DateLib that tells us whether the calendar should display year-first or month-first. Since this requires some familiarity with the codebase, I’m happy to take this on. |
|
Great, then I will close mine! Thanks! |
|
@chuganzy the change is available in v9.11 - see for example https://daypicker.dev/playground?locale=zh-CN does it work for you? |
|
Thanks, and it looks good! Technically, the year format is not yet 100% accurate and can see the difference between So, looks like no flag is added and was just released as a minor update? |
|
@chuganzy yes, we released this to gather more feedback. I'm not an expert in these Asian languages, so I can’t judge how accurate it is :) Do you mean the label caption is still not quite correct? I'd like to iterate on this further. |
Pull Request Template
Thanks for your PR! Make sure you have read the CONTRIBUTING guide.
What's Changed
Currently dropdown order is hardcoded, however the ideal order changes based on locale.
This PR makes use of
Intl.DateTimeFormat.prototype.formatToParts()API to get the best matching order.Type of Change
Tips for a good PR
Thanks
Additional Notes
Add any extra comments or questions here.