-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[pickers] Pass the date object to dayOfWeekFormatter
and deprecate the string parameter
#10345
Conversation
…method that it will replace
…updated the prop signature of the `dayOfWeekFormatter` (and all occurrences of the interface)
Netlify deploy previewNetlify deploy preview: https://deploy-preview-10345--material-ui-x.netlify.app/ Updated pagesThese are the results for the performance tests:
|
@flaviendelangle could re-review? I have added the requested changes |
@alexfauquette could you also take a look here, since @flaviendelangle is on PTO? Thanks! 🙇🏼♂️ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great 👌
docs/data/date-pickers/adapters-locale/CustomDayOfWeekFormat.tsx
Outdated
Show resolved
Hide resolved
packages/x-date-pickers/src/AdapterDateFns/AdapterDateFns.test.tsx
Outdated
Show resolved
Hide resolved
Co-authored-by: Flavien DELANGLE <[email protected]> Signed-off-by: Michel Engelen <[email protected]>
Co-authored-by: Flavien DELANGLE <[email protected]> Signed-off-by: Michel Engelen <[email protected]>
There was a problem hiding this 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
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]>
…' into 9984/refactor-dayOfWeekFormatter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work!
dayOfWeekFormatter
and deprecate the string parameter
@flaviendelangle argos seems flaky on this test :/ |
I approved the Argos changes |
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 newgetWeek
method in the adapters. This now returns an array of date objects for the week of the given date. The signature looks like this:The new signature for the
dayOfWeekFormatter
is now:the default behaviour is kept the same (returning the initial of the weekday):
This introduces a minor breaking change.
Fixes #9984