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

[flcarre]

Duplicates

• I have searched the existing issues

Latest version

• I have tested the latest version

Steps to reproduce 🕹

Just a simple case of the Date Calendar

      <LocalizationProvider dateAdapter={AdapterLuxon}>
        <DateCalendar
          dayOfWeekFormatter={(day) => {
            console.log(day);
            return day;
          }}
        />
      </LocalizationProvider>

Current behavior 😯

In my console.log, i got M, T, W, T, F, S, S

Expected behavior 🤔

I would like to get the complete day (as i think it is describe in the doc) Monday, Tuesday, Wednesday, Thursday, Friday, Saturday, Sunday

Context 🔦

I want to render the 3 first letter avec the day, like Mon, Tue, Wed ....

Your environment 🌎

npx @mui/envinfo

System:
OS: macOS 13.5
Binaries:
Node: 16.19.1 - ~/.nvm/versions/node/v16.19.1/bin/node
Yarn: Not Found
npm: 8.19.3 - ~/.nvm/versions/node/v16.19.1/bin/npm
Browsers:
Chrome: 115.0.5790.170
Edge: Not Found
Safari: 16.6
npmPackages:
@emotion/react: ^11.11.1 => 11.11.1
@emotion/styled: ^11.11.0 => 11.11.0
@mui/base: 5.0.0-beta.8
@mui/core-downloads-tracker: 5.14.2
@mui/icons-material: ^5.14.0 => 5.14.1
@mui/lab: 5.0.0-alpha.137
@mui/material: ^5.14.0 => 5.14.2
@mui/private-theming: 5.13.7
@mui/styled-engine: 5.13.2
@mui/system: 5.14.1
@mui/types: 7.2.4
@mui/utils: 5.14.1
@mui/x-date-pickers: ^6.11.0 => 6.11.0
@types/react: ^18.0.14 => 18.2.16
react: ^18.2.0 => 18.2.0
react-dom: ^18.2.0 => 18.2.0
typescript: ^5.1.6 => 5.1.6

Order ID or Support key 💳 (optional)

No response

[alexfauquette]

The docs API is outdated, we do not use the day.charAt(0).toUpperCase() since mui/material-ui#5222 because it does not allow to support localization

Now, it's the adapter getWeekdays method that is used
https://github.com/mui/mui-x/blob/master/packages/x-date-pickers/src/DateCalendar/DayCalendar.tsx/#L556-L567

For Luxon, it's

public getWeekdays = () => {
  return Info.weekdaysFormat('narrow', { locale: this.locale });
};

We could add a second parameter to provide the day index such that you could do whatever you want knowing it's the 5th day of the week

[flcarre]

So we can't no longer have the complete days ?

[alexfauquette]

So we can't no longer have the complete days ?

Except if your date library allows you to define how a short date are rendered. For example, moment has the updateLocale for others I don't know.

moment.updateLocale("en-us", {
  weekdaysShort: ["Mon", "Tue", "Wed", "Thu", "Fri", "Sat", "Sun"]
});

Providing the week-day index to dayOfWeekFormatter is the only solution I've in mind that could scale with all locales

Do you want to open a PR for that?

[michelengelen]

I did investigate this a bit more in depth and as it seems we do have 2 different functions that contradict each other a bit:

1. the defaultDayOfWeekFormatter is still using day.charAt(0).toUpperCase()
   

   mui-x/packages/x-date-pickers/src/DateCalendar/DayCalendar.tsx

   Line 126 in a071e54

   const defaultDayOfWeekFormatter = (day: string) => day.charAt(0).toUpperCase();

2. we do generate the weekdays with getWeekdays, but for Luxon we do use the 'narrow' parameter, which will generate an array that looks like this: ['M', 'T', 'W', ..., 'S']. So the items in the array in itself are already shortened.

A fix for this is to just change the parameter for the getWeekdays function from 'narrow' to 'long':

--- a/packages/x-date-pickers/src/AdapterLuxon/AdapterLuxon.ts
+++ b/packages/x-date-pickers/src/AdapterLuxon/AdapterLuxon.ts
@@ -524,7 +524,7 @@ export class AdapterLuxon implements MuiPickersAdapter<DateTime, string> {
   };

   public getWeekdays = () => {
-    return Info.weekdaysFormat('narrow', { locale: this.locale });
+    return Info.weekdaysFormat('long', { locale: this.locale });
   };

   public getWeekArray = (value: DateTime) => {

applying the fix locally and using a custom dayOfWeekFormatter function:

<DateCalendar
  views={['day']}
  dayOfWeekFormatter={(day) => day.slice(0, 2).toUpperCase()}
/>

@flaviendelangle (tagging you, since @alexfauquette is still on PTO): do you see any problem with that fix?

[LukasTy]

A fix for this is to just change the parameter for the getWeekdays function from 'narrow' to 'long':

That is a really nice observation and suggestion! 👍
One little problem is that it would technically be a tiny breaking change. 🙈 🤷

[michelengelen]

@flaviendelangle I just checked and the only place getWeekdays is called is here

mui-x/packages/x-date-pickers/src/DateCalendar/DayCalendar.tsx

Lines 556 to 566 in a071e54

	{utils.getWeekdays().map((day, i) => (
	<PickersCalendarWeekDayLabel
	key={day + i.toString()}
	variant="caption"
	role="columnheader"
	aria-label={utils.format(utils.addDays(startOfCurrentWeek, i), 'weekday')}
	className={classes.weekDayLabel}
	>
	{dayOfWeekFormatter?.(day) ?? day}
	</PickersCalendarWeekDayLabel>
	))}

---

I just noticed the current return value of getWeekdays is inconsistent:

DateFns

mui-x/packages/x-date-pickers/src/AdapterDateFns/AdapterDateFns.ts

Lines 545 to 551 in a071e54

	public getWeekdays = () => {
	const now = new Date();
	return eachDayOfInterval({
	start: startOfWeek(now, { locale: this.locale }),
	end: endOfWeek(now, { locale: this.locale }),
	}).map((day) => this.formatByString(day, 'EEEEEE'));
	};

returnValue: ['Mo', 'Tu', 'We', 'Th', 'Fr', 'Sa', 'Su']

Luxon

mui-x/packages/x-date-pickers/src/AdapterLuxon/AdapterLuxon.ts

Lines 526 to 528 in a071e54

	public getWeekdays = () => {
	return Info.weekdaysFormat('narrow', { locale: this.locale });
	};

returnValue: ['M', 'T', 'W', 'T', 'F', 'S', 'S']

DayJS

mui-x/packages/x-date-pickers/src/AdapterDayjs/AdapterDayjs.ts

Lines 682 to 687 in a071e54

	public getWeekdays = () => {
	const start = this.dayjs().startOf('week');
	return [0, 1, 2, 3, 4, 5, 6].map((diff) =>
	this.formatByString(this.addDays(start, diff), 'dd'),
	);
	};

returnValue: ['Mo', 'Tu', 'We', 'Th', 'Fr', 'Sa', 'Su']

MomentJS

mui-x/packages/x-date-pickers/src/AdapterMoment/AdapterMoment.ts

Line 587 in a071e54

public getWeekdays = () => this.syncMomentLocale(() => defaultMoment.weekdaysShort(true));

returnValue: ['Mon', 'Tue', 'Wed', 'Thu', 'Fri', 'Sat', 'Sun']

---

this in itself is a problem, right?

[flaviendelangle]

If we have some formats in the date libraries that would help us have a more consistent behaviors across adapters, we should clearly go for it.

But I would not be surprised if not all date libraries support exactly the same formats for this topic.

If you want to have a look 👌

[michelengelen]

@flaviendelangle should we maybe add a parameter (with it defaulting to the current return value) to the getWeekdays function to not break current implementations of it

- getWeekdays(): string[];
+ getWeekdays(format: 'full'|'narrow'|'short'): string[];

Where ...

• 'full' returns ['Monday', 'Tuesday', ... ]
• 'narrow' returns ['Mon', 'Tue', ... ]
• 'short' returns ['M', 'T', ... ]

defaulting to 'short'

WDYT?

[LukasTy]

should we maybe add a parameter (with it defaulting to the current return value) to the getWeekdays function to not break current implementations of it

What would that accomplish?
We are only using this function internally and doing so wouldn't change anything for the end-users. 🤔

[flaviendelangle]

We are only using this function internally and doing so wouldn't change anything for the end-users

Strictly speaking the adapters are public API, but for this kind of BC, I prefer to create a new method and replace on the next major.
We would only use 1 value of this new param so it would not be very usefull

---

I'm not a fan of the current API to format the day of the week.
We have a getWeekdays method that returns a list of string.
And then we call dayOfWeekFormatter for each day if defined, otherwise we take the 1st letter.

I would prefer if getWeekdays returned a list of dates (we can create a new method to avoid a breaking change and then replace in v7).
dayOfWeekFormatter would receive a date.
And the default dayOfWeekFormatter would format using weekday and would take the 1st letter capitalized.

For me that's more flexible that passing a string and more coherent with the rest of our API.

[LukasTy]

I would prefer if getWeekdays returned a list of dates (we can create a new method to avoid a breaking change and then replace in v7).
<...>
For me that's more flexible that passing a string and more coherent with the rest of our API.

Agreed, I've also always felt that exposing date objects instead of strings to the formatter would end up being more flexible. 💯

[michelengelen]

We are only using this function internally and doing so wouldn't change anything for the end-users

Strictly speaking the adapters are public API, but for this kind of BC, I prefer to create a new method and replace on the next major. We would only use 1 value of this new param so it would not be very usefull

That's what I was thinking. If anyone uses it we would potentially break that.

I would prefer if getWeekdays returned a list of dates (we can create a new method to avoid a breaking change and then replace in v7). dayOfWeekFormatter would receive a date. And the default dayOfWeekFormatter would format using weekday and would take the 1st letter capitalized.

Yes... that would be ideal... I thought the same: having the restriction of passing a pure string to the formatter did seem a bit unflexible. I will come up with something. I do have enough info now to create a solution for that.

Thanks y'all 🙇🏼

[flaviendelangle]

That's what I was thinking. If anyone uses it we would potentially break that.

To give a little context
The adapters are public API but we never recommend people to modify them and they make little sense outside of the picker scope.
So I am not worried to make breaking change on them during a minor if it improves our product.
But of course during minors, we must deprecate the old methods, create new methods (even if they are called getWeekdaysV7 😆 ).

[flaviendelangle]

By the way, we don't even need an adapter method, we have everything we need in the adapter without it, we just need to do what we are doing on the dayjs method:

export const getWeekDays = <TDate>(utils: MuiPickersAdapter<TDate>, date: TDate) => {
  const start = utils.startOfWeek(date);
  return [0, 1, 2, 3, 4, 5, 6].map((diff) => utils.addDays(start, diff));
}

And in DateCalendar we format and take the 1st letter.

[michelengelen]

By the way, we don't even need an adapter method, we have everything we need in the adapter without it, we just need to do what we are doing on the dayjs method:

export const getWeekDays = <TDate>(utils: MuiPickersAdapter<TDate>, date: TDate) => {
  const start = utils.startOfWeek(date);
  return [0, 1, 2, 3, 4, 5, 6].map((diff) => utils.addDays(start, diff));
}

And in DateCalendar we format and take the 1st letter.

But we would potentially break (even if just a few) applications that rely on this method, right? As mentioned this is pretty unlikely, but still possible, since it is technically an open API

[flaviendelangle]

But we would potentially break (even if just a few) applications that rely on this method, right?

We can stop using the method and then remove the method during the next alpha.
I did the same with mergeDateAndTime and getMonthsInYear.

[alexplumb]

As far as I can tell, this breaks a solution people (including me) have been using for a few years to force Luxon to use Sunday as the first day of the week.

#12821
mui/material-ui-pickers#1270

Pretty niche use case but there are people out there that use custom adapters that depend on extending this method. I'm not really sure what the best way to solve this would be - perhaps adding a way to the LocalizationProvider for passing in a custom weekday array?

[BrianWhiting]

@alexplumb Since 2021, I've been using the workaround noted in mui/material-ui-pickers#1270 to force Luxon to use Sunday as the first day of the week and just noticed that it no longer works. I was able to fix it by adding an override for the startOfWeek method as follows:

  startOfWeek = (value: DateTime) => {
    return value.startOf('week').minus({ days: 1 });
  }

Do you see any issue with this fix?

[flaviendelangle]

Hi,

Sorry for the broken behavior, we are discussing a potential cleaner solution in mui/material-ui#10805