mui / mui-x

MUI X: Build complex and data-rich applications using a growing list of advanced React components, like the Data Grid, Date and Time Pickers, Charts, and more!
https://mui.com/x/
4.54k stars 1.32k forks source link

[pickers] `DatePicker` character editing fails with timezone "Europe/London", Dayjs adapter #13698

Closed milotoor closed 3 months ago

milotoor commented 4 months ago

Steps to reproduce

Link to live example: https://react-t1mufn.stackblitz.io

Steps:

  1. Click into the month field of the input (in this case the middle field)
  2. Enter any number other than 1. For example, click "2" to switch to February
  3. The input does not change

Current behavior

The input does not switch to the selected month, and appears to not respond to the character input whatsoever

Expected behavior

The DatePicker should switch to the selected month

Context

I am trying to use the "Europe/London" timezone with the DatePicker component (really the DateRangePicker but the bug applies to DatePicker, and I felt it would be simpler to illustrate the bug with the latter).

After doing some digging through the source code, the issue appears to be Dayjs adapter code, probably its adjustOffset function used by endOfYear. The useFieldCharacterEditing hook performs range validations when applying a numeric edit. In this instance the section.type is month, and sectionsValueBoundaries.month() returns the dubious value of:

{
    "minimum": 1,
    "maximum": 1
}

The month function is defined here and returns a maximum value of utils.getMonth(endOfYear) + 1, where endOfYear is defined as utils.endOfYear(today). Thus it seems that the endOfYear or getMonth function is to blame. Of the two, endOfYear seems the more likely as it is not simply calling dayjs library functions but also calls the adjustOffset function. I have not dug into this further but suspect the issue lies in there.

Interestingly this problem does not apply to other nearby timezones. For instance I have not experienced any issues with "Europe/Paris". Perhaps the proximity to UTC is relevant?

Your environment

npx @mui/envinfo Note that my project is currently using `@mui/x-date-pickers-pro@6.19.9` but I have confirmed this bug exists on the latest stable version, `7.8.0`, as seen in the attached minimal reproduction ``` System: OS: macOS 14.0 Binaries: Node: 19.9.0 - ~/.nvm/versions/node/v19.9.0/bin/node npm: 9.6.3 - ~/.nvm/versions/node/v19.9.0/bin/npm pnpm: 9.4.0 - ~/.nvm/versions/node/v19.9.0/bin/pnpm Browsers: Chrome: 126.0.6478.127 Edge: Not Found Safari: 17.0 npmPackages: @emotion/react: ^11.11.3 => 11.11.3 @emotion/styled: ^11.11.0 => 11.11.0 @mui/base: 5.0.0-beta.40 @mui/core-downloads-tracker: 5.15.17 @mui/icons-material: ^5.15.17 => 5.15.17 @mui/lab: ^5.0.0-alpha.170 => 5.0.0-alpha.170 @mui/material: ^5.15.17 => 5.15.17 @mui/private-theming: 5.15.14 @mui/styled-engine: 5.15.14 @mui/system: ^5.15.15 => 5.15.15 @mui/types: 7.2.14 @mui/utils: 5.15.14 @mui/x-data-grid: 6.19.11 @mui/x-data-grid-premium: ^6.19.11 => 6.19.11 @mui/x-data-grid-pro: 6.19.11 @mui/x-date-pickers: 6.19.9 @mui/x-date-pickers-pro: ^6.19.9 => 6.19.9 @mui/x-license-pro: 6.10.2 @types/react: ^18.3.2 => 18.3.2 react: ^18.3.1 => 18.3.1 react-dom: ^18.3.1 => 18.3.1 typescript: ^5.2.2 => 5.2.2 ```

Search keywords: DatePicker, timezone, Europe/London Order ID: 87847

LukasTy commented 4 months ago

Hello @milotoor. Thank you for creating this issue! 🙏 It looks like a known problem with Dayjs not working correctly with GMT+0 time zones. https://github.com/mui/mui-x/issues/9653 Could you please try using the suggested workaround or a different adapter (only Dayjs has this issue) for the time being while https://github.com/iamkun/dayjs/pull/2481 is resolved?

milotoor commented 4 months ago

It looks like a known problem with Dayjs not working correctly with GMT+0 time zones. #9653 Could you please try using the suggested workaround or a different adapter (only Dayjs has this issue) for the time being while iamkun/dayjs#2481 is resolved?

Ah, thanks for pointing that out! I tried out a modified version of the solution and it might workable, but I have a couple of concerns. The suggestion of doing this...

export const getTimezone = (timestamp: number, timezone: string) => {
  const isGMT = dayjs.tz(timestamp, timezone).offsetName() === 'GMT';
  return isGMT ? 'UTC' : timezone;
};

...doesn't seem fully applicable to the DateRangePicker (which is what my use case calls for) since there are two timestamps in a range and they could have different offsets if they span a DST boundary. You can see the results here.

Additionally, regarding the specific bug I observed, it's not the selected timestamp (the start or end of the range) that matters--it's the end of the year. So in order for this solution to work the code would have to adjust to:

const getTimezone = (timezone: string) => {
  const isGMT = dayjs.tz(undefined, timezone).endOf('year').offsetName() === 'GMT'
  return isGMT ? 'UTC' : timezone
};

That does fix the "I can't change the month with a numeric field edit" issue, but as you can see in the demo it causes issues for timestamps within the DST period (specifically every timestamp is moved back an hour, which impacts both the DateTimeRangePicker and DateRangePicker). Perhaps this could be fixed by performing an offset shift for any timestamp occurring within DST such that the date/time appear correctly within the UTC-timezoned input field, but this feels very kludgy and error prone to me.

michelengelen commented 4 months ago

@LukasTy gentle ping 🔔

michelengelen commented 3 months ago

Hey @milotoor ... could you provide the code you entered to get that demo running? I cannot access it for some reason

milotoor commented 3 months ago

could you provide the code you entered to get that demo running? I cannot access it for some reason

Strange, I can't access it either. Here's the original code:

import dayjs, { type Dayjs } from 'dayjs';
import utc from 'dayjs/plugin/utc';
import timezone from 'dayjs/plugin/timezone';
import * as React from 'react';
import { DemoContainer } from '@mui/x-date-pickers/internals/demo';
import { AdapterDayjs } from '@mui/x-date-pickers/AdapterDayjs';
import { LocalizationProvider } from '@mui/x-date-pickers/LocalizationProvider';
import { DateTimeRangePicker } from '@mui/x-date-pickers-pro/DateTimeRangePicker';
import { DateRangePicker } from '@mui/x-date-pickers-pro/DateRangePicker';
import { Divider, Typography } from '@mui/material';

dayjs.extend(utc);
dayjs.extend(timezone);

export default function BasicDatePicker() {
  const [range1, setRange1] = React.useState<[Dayjs | null, Dayjs | null]>([
    dayjs.tz('2024-03-30', 'Europe/London'),
    dayjs.tz('2024-04-01', 'Europe/London'),
  ]);

  const [range2, setRange2] = React.useState<[Dayjs | null, Dayjs | null]>([
    dayjs.tz('2024-10-26', 'Europe/London'),
    dayjs.tz('2024-10-28', 'Europe/London'),
  ]);

  const [range3, setRange3] = React.useState<[Dayjs | null, Dayjs | null]>([
    dayjs.tz('2024-07-02', 'Europe/London'),
    dayjs.tz('2024-07-05', 'Europe/London'),
  ]);

  const [range4, setRange4] = React.useState<[Dayjs | null, Dayjs | null]>([
    dayjs.tz('2024-11-26', 'Europe/London'),
    dayjs.tz('2024-11-28', 'Europe/London'),
  ]);

  return (
    <LocalizationProvider dateAdapter={AdapterDayjs}>
      <DemoContainer components={['DatePicker']}>
        <Typography>Spans London DST (March 31st)</Typography>
        <DateTimeRangePicker
          value={range1}
          onChange={(newValue) => setRange1(newValue)}
          timezone="UTC"
        />

        <DateRangePicker
          value={range1}
          onChange={(newValue) => setRange1(newValue)}
          timezone="UTC"
        />

        <Divider />

        <Typography>Spans London DST (October 27th)</Typography>
        <DateTimeRangePicker
          value={range2}
          onChange={(newValue) => setRange2(newValue)}
          timezone="UTC"
        />

        <DateRangePicker
          value={range2}
          onChange={(newValue) => setRange2(newValue)}
          timezone="UTC"
        />

        <Divider />

        <Typography>Exclusively within DST</Typography>
        <DateTimeRangePicker
          value={range3}
          onChange={(newValue) => setRange3(newValue)}
          timezone="UTC"
        />

        <DateRangePicker
          value={range3}
          onChange={(newValue) => setRange3(newValue)}
          timezone="UTC"
        />

        <Divider />

        <Typography>Exclusively outside DST</Typography>
        <DateTimeRangePicker
          value={range4}
          onChange={(newValue) => setRange4(newValue)}
          timezone="UTC"
        />

        <DateRangePicker
          value={range4}
          onChange={(newValue) => setRange4(newValue)}
          timezone="UTC"
        />

        <Divider />

        <Typography>Uses "Europe/London", exclusively within DST</Typography>
        <DateTimeRangePicker
          value={range3}
          onChange={(newValue) => setRange2(newValue)}
          timezone="Europe/London"
        />

        <DateRangePicker
          value={range3}
          onChange={(newValue) => setRange2(newValue)}
          timezone="Europe/London"
        />

        <Divider />

        <Typography>Uses "Europe/London", exclusively outside DST</Typography>
        <DateTimeRangePicker
          value={range4}
          onChange={(newValue) => setRange2(newValue)}
          timezone="Europe/London"
        />

        <DateRangePicker
          value={range4}
          onChange={(newValue) => setRange2(newValue)}
          timezone="Europe/London"
        />
      </DemoContainer>
    </LocalizationProvider>
  );
}
michelengelen commented 3 months ago

Hey @milotoor are you bound to use DayJs? Considering the bug in the package this might be impossible to fix 100%, so I guess your best shot would be to switch to one of the other options.

milotoor commented 3 months ago

are you bound to use DayJs? Considering the bug in the package this might be impossible to fix 100%, so I guess your best shot would be to switch to one of the other options.

"Bound" is a strong word, but it would be a pretty large lift to drop Dayjs entirely from our application and probably not worth the effort for this reason alone. My strong preference would be to fix the library code, and I'm happy to contribute to that in whatever way I can. It looks like you opened a PR almost 10 months ago to fix the startOf method but it's been languishing for a long while. I'll follow along on that issue. Perhaps more community engagement with it will get it some attention.

michelengelen commented 3 months ago

Thanks @milotoor ... I am not sure why the maintainer is not responding to this PR or even acknowledges that the issue is there ion the first place. 🤷🏼 I am sorry that we cannot help you at this point.

milotoor commented 3 months ago

@michelengelen I think this issue may actually have been fixed in Dayjs a couple of weeks ago! Their latest release (v1.11.12) includes a "Fix zero offset issue when use tz with locale" (see this PR), and I can confirm that updating to the latest version fixes the issue for me with the DatePicker and DateRangePicker.

Thanks for your diligence working on this issue! I'm going to close it out since it appears to be resolved by updating Dayjs.

github-actions[bot] commented 3 months ago

:warning: This issue has been closed. If you have a similar problem but not exactly the same, please open a new issue. Now, if you have additional information related to this issue or things that could help future readers, feel free to leave a comment.

@milotoor: How did we do? Your experience with our support team matters to us. If you have a moment, please share your thoughts in this short Support Satisfaction survey.