Skip to content

[DateTimeRangePicker] Fix date format resolving from views on 24hr locales#14341

Merged
arthurbalduini merged 5 commits intomui:masterfrom
arthurbalduini:date-time/fix-date-format-resolver
Aug 29, 2024
Merged

[DateTimeRangePicker] Fix date format resolving from views on 24hr locales#14341
arthurbalduini merged 5 commits intomui:masterfrom
arthurbalduini:date-time/fix-date-format-resolver

Conversation

@arthurbalduini
Copy link
Copy Markdown
Contributor

@arthurbalduini arthurbalduini commented Aug 26, 2024

Fixes #14334.
For DateTimeRangePickers the ["day"] case returns an incorrect format.

@arthurbalduini arthurbalduini added the scope: pickers Changes related to the date/time pickers. label Aug 26, 2024
@arthurbalduini arthurbalduini requested a review from LukasTy August 26, 2024 09:54
@arthurbalduini arthurbalduini self-assigned this Aug 26, 2024
Comment thread packages/x-date-pickers/src/internals/utils/date-utils.ts Outdated
@mui-bot
Copy link
Copy Markdown

mui-bot commented Aug 26, 2024

Deploy preview: https://deploy-preview-14341--material-ui-x.netlify.app/

Generated by 🚫 dangerJS against 6f06e4e

@arthurbalduini arthurbalduini force-pushed the date-time/fix-date-format-resolver branch from 0dbc59a to dbddcbf Compare August 26, 2024 11:27
@arthurbalduini arthurbalduini changed the title [pickers] fix case for day view on DateTimePickers [pickers] Fix format for day view on DateTimeRangePickers Aug 26, 2024
@arthurbalduini arthurbalduini changed the title [pickers] Fix format for day view on DateTimeRangePickers [pickers] Fix date format on DateTimeRangePickers Aug 26, 2024
@arthurbalduini arthurbalduini changed the title [pickers] Fix date format on DateTimeRangePickers [pickers] Fix date format resolver on DateTimeRangePickers Aug 27, 2024
@dosubot dosubot Bot added the size:S label Aug 27, 2024
if (isTimeView(view)) {
timeViews.push(view as TimeView);
} else {
} else if (isDatePickerView(view)) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem was an oversight that we did not exclude meridiem view, which caused resolveDateFormat to always return keyboardDate in case of a locale with 12h clock.
On 24h clock the behavior was "correct".
I've adjusted the code to avoid this inconsistency and also added an exception to not resolve dateFormat on DateTimeRange pickers as they do not have different date views other than day for now.

@LukasTy LukasTy added component: DateTimeRangePicker type: bug It doesn't behave as expected. plan: Pro Impact at least one Pro user. type: regression A bug, but worse, it used to behave as expected. labels Aug 29, 2024
@LukasTy LukasTy changed the title [pickers] Fix date format resolver on DateTimeRangePickers [DateTimeRangePicker] Fix date format resolving from views on 24hr locales Aug 29, 2024
Copy link
Copy Markdown
Member

@LukasTy LukasTy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've adjusted the approach to one that looks more correct to me.
I'm not sure why we would have differentiated the behavior based on the presence of value. 🤔

Let me know what you think about the solution. 😉

@arthurbalduini
Copy link
Copy Markdown
Contributor Author

I've adjusted the approach to one that looks more correct to me. I'm not sure why we would have differentiated the behavior based on the presence of value. 🤔

Let me know what you think about the solution. 😉

I assumed that veryfing if the value is an array (date range) would be more clear than adding a new parameter to the function to be used only in DateTimePickers, since the view verification was done in resolveDateFormat().
Let's go with your solution

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

plan: Pro Impact at least one Pro user. scope: pickers Changes related to the date/time pickers. type: regression A bug, but worse, it used to behave as expected.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[pickers][DateTimeRangePicker] Polish locale from dayjs breaks datetime format in DateTimeRangePicker since v.7.10.0

4 participants