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

[pickers] Pass the date object to dayOfWeekFormatter and deprecate the string parameter #10345

Merged

Conversation

michelengelen
Copy link
Member

Description

The dayOfWeekFormatter received only a string as parameter and to handle this the user had to manually detect which day of the week he received before handling its formatting (if he intended to change something on that).
Now the formatting function receives the date object (according to the used adapter), so the flexibility is increased by a fair margin here.

As a way of fixing this the getWeekdays method will be deprecated and replaced by the new getWeek method in the adapters. This now returns an array of date objects for the week of the given date. The signature looks like this:

getWeek(value: TDate): TDate[];

The new signature for the dayOfWeekFormatter is now:

dayOfWeekFormatter?: (weekday: TDate) => string;

the default behaviour is kept the same (returning the initial of the weekday):

(weekday: TDate) => utils.format(weekday, 'weekdayShort').charAt(0).toUpperCase()

This introduces a minor breaking change.

Fixes #9984


@michelengelen michelengelen added breaking change component: pickers This is the name of the generic UI component, not the React module! enhancement This is not a bug, nor a new feature deprecation New deprecation message v7.x labels Sep 15, 2023
@michelengelen michelengelen added this to the v7.0.0.alpha.0 milestone Sep 15, 2023
@michelengelen michelengelen self-assigned this Sep 15, 2023
@michelengelen michelengelen changed the title 9984/refactor day of week formatter [pickers] refactor dayOfWeekFormatter Sep 15, 2023
@mui-bot
Copy link

mui-bot commented Sep 15, 2023

Netlify deploy preview

Netlify deploy preview: https://deploy-preview-10345--material-ui-x.netlify.app/

Updated pages

These are the results for the performance tests:

Test case Unit Min Max Median Mean σ
Filter 100k rows ms -114.5 368.3 104 99.74 166.327
Sort 100k rows ms 1,111.5 1,984.8 1,771.8 1,631.1 351.883
Select 100k rows ms 872.8 1,213.1 872.8 993.48 119.076
Deselect 100k rows ms 203.1 496 257.3 316.1 109.718

Generated by 🚫 dangerJS against 3f0eccf

@michelengelen
Copy link
Member Author

@flaviendelangle could re-review? I have added the requested changes

@michelengelen
Copy link
Member Author

@alexfauquette could you also take a look here, since @flaviendelangle is on PTO? Thanks! 🙇🏼‍♂️

Copy link
Member

@flaviendelangle flaviendelangle left a comment

Choose a reason for hiding this comment

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

Looks great 👌

Copy link
Member

@flaviendelangle flaviendelangle left a comment

Choose a reason for hiding this comment

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

Just living a small suggestion for the doc

docs/data/date-pickers/adapters-locale/adapters-locale.md Outdated Show resolved Hide resolved
@flaviendelangle
Copy link
Member

The build error is weird, I'll have a look if you do not find its root cause

Co-authored-by: Flavien DELANGLE <[email protected]>
Signed-off-by: Michel Engelen <[email protected]>
Copy link
Member

@flaviendelangle flaviendelangle left a comment

Choose a reason for hiding this comment

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

Great work!

@flaviendelangle flaviendelangle changed the title [pickers] refactor dayOfWeekFormatter [pickers] Pass the date object to dayOfWeekFormatter and deprecate the string parameter Sep 26, 2023
@michelengelen
Copy link
Member Author

@flaviendelangle argos seems flaky on this test :/

@michelengelen michelengelen merged commit b4df709 into mui:master Sep 26, 2023
19 checks passed
@flaviendelangle
Copy link
Member

flaviendelangle commented Sep 26, 2023

I approved the Argos changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: pickers This is the name of the generic UI component, not the React module! deprecation New deprecation message enhancement This is not a bug, nor a new feature v7.x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Just got the first letter of the days in dayOfWeekFormatter using Luxon Adapter on Date Calendar
3 participants