Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fields] Fix year editing on valid date #7834

Merged
merged 2 commits into from
Feb 8, 2023

Conversation

flaviendelangle
Copy link
Member

No description provided.

@mui-bot
Copy link

mui-bot commented Feb 6, 2023

Messages
📖 Netlify deploy preview: https://deploy-preview-7834--material-ui-x.netlify.app/

These are the results for the performance tests:

Test case Unit Min Max Median Mean σ
Filter 100k rows ms 615.2 998.1 641.5 732.28 147.06
Sort 100k rows ms 607.8 1,107.8 607.8 900.82 167.099
Select 100k rows ms 212.1 311 252.5 258.14 39.034
Deselect 100k rows ms 132.6 317.2 172.6 194.36 64.245

Generated by 🚫 dangerJS against bf213c5

@flaviendelangle flaviendelangle added bug 🐛 Something doesn't work component: pickers This is the name of the generic UI component, not the React module! labels Feb 6, 2023
@flaviendelangle flaviendelangle requested review from LukasTy and removed request for LukasTy February 6, 2023 09:04
@flaviendelangle flaviendelangle marked this pull request as draft February 6, 2023 09:04
@@ -434,7 +434,24 @@ export const doesSectionHaveTrailingZeros = <TDate>(

// We can't use `changeSectionValueFormat`, because `utils.parse('1', 'YYYY')` returns `1971` instead of `1`.
if (dateSectionName === 'year') {
return utils.formatByString(utils.setYear(utils.date()!, 1), format).length > 1;
const formatted2001 = utils.formatByString(utils.setYear(utils.date()!, 2001), format);
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure if that's the best approach
The challenge here is to know if we have trailing zeroes or not on the year.
But a year format can be 2-digit or 4-digit.

Copy link
Member

Choose a reason for hiding this comment

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

Seems like this is a "necessary evil" to cover the different year formats adapters can have. 🙈

@flaviendelangle flaviendelangle marked this pull request as ready for review February 6, 2023 13:20
Copy link
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.

Is this issue already known? It's a problem with next as well.

Screen.Recording.2023-02-06.at.15.09.28.mov

This behaviour seems somewhat strange. Is it supposed to work like that or is there some sort of a bug?
The first time we are entering date by inputting 02—only after 2 is pressed the value is presented.
While after that, even when deleting the whole date, any change in year section produces some sort of a value, and seemly wrong value? Because entering 0 in empty YY section produces year 20 (2020) 🙈
P.S. With date-fns adapter it seems to work correctly, while with dayjs it is acting a bit strangely...

Screen.Recording.2023-02-06.at.15.12.55.mov

@@ -434,7 +434,24 @@ export const doesSectionHaveTrailingZeros = <TDate>(

// We can't use `changeSectionValueFormat`, because `utils.parse('1', 'YYYY')` returns `1971` instead of `1`.
if (dateSectionName === 'year') {
return utils.formatByString(utils.setYear(utils.date()!, 1), format).length > 1;
const formatted2001 = utils.formatByString(utils.setYear(utils.date()!, 2001), format);
Copy link
Member

Choose a reason for hiding this comment

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

Seems like this is a "necessary evil" to cover the different year formats adapters can have. 🙈

@flaviendelangle
Copy link
Member Author

flaviendelangle commented Feb 8, 2023

This behaviour seems somewhat strange. Is it supposed to work like that or is there some sort of a bug?

If I understand the problem correctly:
It seems to be because we don't clean the autocompletion query when pressing Backspace.
I'll create a standalone PR 👍

EDIT: Fixed by #7855

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: pickers This is the name of the generic UI component, not the React module!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants