[pickers] Use the locale week day on the Luxon adapter #19230
Conversation
flaviendelangle
added
type: bug
|
Deploy preview: https://deploy-preview-19230--material-ui-x.netlify.app/ Bundle size report
|
|
|
||
| export function isWeekend(adapter: Adapter, value: SchedulerValidDate): boolean { | ||
| const dayOfWeek = adapter.getDayOfWeek(value); | ||
| const sunday = adapter.format(adapter.date('2025-08-09'), 'weekday'); |
There was a problem hiding this comment.
I'm struggling to find a better way to detect the week end 😬
I'm not even sure if some adapter have a locale-based notion of "Week end".
There was a problem hiding this comment.
But the weekend might not be on Saturday and Sunday for every locale. 🤔
Doesn't US technically have the weekend on Sunday and Monday?
There was a problem hiding this comment.
Scratch that previous question part.
Regardless, based on ChatGPT information, some countries do have weekends on different days:
There was a problem hiding this comment.
Doesn't US technically have the weekend on Sunday and Monday?
No, the week start on Sunday, but the week end is still Sartuday and Sunday AFAIK.
Those are two distinct concepts, sadly enough we don't use "week-end" to mean the end of the week 👨 🔫 . So for me the behavior of the Scheduler before this PR was already broken, probably even more than after the PR.
With that being said, I'm not surprised that not all country and culture have Saturday + Sunday as week end. Bryntum has a concept of working days (that user can define manually) which is probably what you want in a lot of scenarios (to include public holdays for example). But I'm not sure it's always what we want and I'm wondering if there is a disctinct concept of week end.
It seems that Bryntum uses there concept of "non working days" for the equivalent of the UI we built. BUT at the same time they have a class specific to week end that apply to Saturday and Sunday (they don't implement the nuance Chat GPT gave, or at least it's not documented).
TLDR: It's a mess and I think starting with the current approach and then research how to best introduce a notion of working / non-working days is fine.
There was a problem hiding this comment.
WDYT about adding a isWeekend method to the adapter?
Looks like Luxon and date-fns have the means to resolve this, whereas for others we can use getWeekInfo as a basis and fallback to your solution if weekInfo is not available (i.e. Firefox).
There was a problem hiding this comment.
It would probably be the right solution.
I think we can add it to the Base UI version and keep it hardcoded for now in the Scheduler while we use a copy paste of the MUI X adapters (I'd like to avoid having 3 versions of the adapter to maintain 😆 )
There was a problem hiding this comment.
Yeah, the notion of work days is probably the best shot at being the silver bullet for the Scheduler's case. 👌
I agree with your proposal. 👍
There was a problem hiding this comment.
To summarize:
- Hardcode Saturday and Sunday in the Scheduler for now
- Add a
isWeekendmethod on the Base UI adapters - When the Scheduler starts using the Base UI adapters, remove the hardcoded days and use the method instead
- When we introduce a notion of working / non-working days, set the default value to equal the week end days (using
isWeekEnd).
flaviendelangle
added
type: bug
Backport from the Base UI version of the adapter.