lemmylem commented 1 year ago

Duplicates

[X] I have searched the existing issues

Latest version

[X] I have tested the latest version

Steps to reproduce 🕹

Link to live example: https://codesandbox.io/s/recursing-fast-9n729n?file=/src/DateTimePicker.js

I have set up a date + time picker combo as a demo of the weirdness I'm seeing. I'm not entirely sure if this is a bug or not, but it's strange and it's happening using the timezone prop. At the very least, it's not the behavior I would expect to see. I'm not seeing these issues if I simply use dayjs.tz and specify the time zone that way.

Steps:

Go to the above link and see that the initial time is set at 2023-04-17T15:30, which translates to 3:30pm.
Also see that the time picker (with the timezone prop of Africa/Bamako, which is GMT) is displaying the wrong time (7:30PM)
Also notice that when you click on the time picker icon to open the time selection dialog, is holds the correct value.
Switch to a previous date, and if you keep switching, you can see that for every date change, the time changes as well, which is not supposed to happen

Current behavior 😯

When I first load the date and time picker, the time picker displays the wrong time value when I use it with timezone prop. However, then you click the icon and open the dialog, it holds the correct time value. When switching dates via the MUI date picker, the time values also change.

Expected behavior 🤔

The time picker should display the correct value when configuring for a specific time zone. When time and date picker are used together, changing the date should only change the date, not the time value.

Context 🔦

I'm trying to update to the newest version of MUI, that includes the date time picker.

I have a custom date/time range picker solution for previous MUI version and trying to integrate the new time picker with the custom built one. One of the things that MUI has done is now with the new version, only the date time object is accepted. Before, it would accept strings and unix stamps.

Using dayjs.tz or setting a default time zone works as expected and I'm not seeing these issues. However, using the timezone prop doesn't work the same and I assume it's supposed to work the same.

EDIT: I have done some additional testing and it seems like this is only observable for GMT time zones (offsets of 0).

Your environment 🌎

npx @mui/envinfo

``` System: OS: Linux 4.18 Rocky Linux 8.7 (Green Obsidian) Binaries: Node: 18.14.0 - ~/.asdf/installs/nodejs/18.14.0/bin/node Yarn: Not Found npm: 9.3.1 - ~/.asdf/plugins/nodejs/shims/npm Browsers: Chrome: Not Found npmPackages: @emotion/react: 11.11.1 => 11.11.1 @emotion/styled: 11.11.0 => 11.11.0 @mui/base: 5.0.0-beta.4 @mui/lab: 5.0.0-alpha.134 @mui/material: 5.13.6 => 5.13.6 @mui/private-theming: 5.13.1 @mui/styled-engine: 5.13.2 @mui/system: 5.13.6 @mui/types: 7.2.4 @mui/utils: 5.13.6 @mui/x-date-pickers: 6.5.0 @types/react: 18.2.14 => 18.2.14 react: 18.2.0 => 18.2.0 react-dom: 18.2.0 typescript: 5.1.3 => 5.1.3 ```

Order ID or Support key 💳 (optional)

No response

lemmylem commented 1 year ago

Seems like changing all GMT+0 offsets to UTC via timezone="UTC" fixes the GMT issue, so I guess there's an easy workaround.

Does the time picker component not recognize all IANA time zones?

alexfauquette commented 1 year ago

Thanks for reporting this issue. I can confirm it only occurs on TZ UTC+00:00

https://codesandbox.io/s/quirky-kapitsa-sspwx2?file=/demo.tsx

About the following point

I have a custom date/time range picker solution for previous MUI version and trying to integrate the new time picker with the custom built one. One of the things that MUI has done is now with the new version, only the date time object is accepted. Before, it would accept strings and unix stamps.

In v5 it ways accepting string. But internally it was simply doing something similar to const parsedValue = typeof value === 'string' ? dayjs(value): value

So I'm not sure to get the link with the issue you are describing

lemmylem commented 1 year ago

Thanks for reporting this issue. I can confirm it only occurs on TZ UTC+00:00

https://codesandbox.io/s/quirky-kapitsa-sspwx2?file=/demo.tsx

About the following point

I have a custom date/time range picker solution for previous MUI version and trying to integrate the new time picker with the custom built one. One of the things that MUI has done is now with the new version, only the date time object is accepted. Before, it would accept strings and unix stamps.

In v5 it ways accepting string. But internally it was simply doing something similar to const parsedValue = typeof value === 'string' ? dayjs(value): value

So I'm not sure to get the link with the issue you are describing

Right, I misspoke. We received unix in milliseconds from the server, converted it to string, then gave it to the date picker and it did all the rest of the magic and it worked that way.

We used a custom "time picker," which was basically just a dropdown of times that a user can select.

Now, everything is much easier and simpler just accepting a dayjs object as the currency of date and time.

alexfauquette commented 1 year ago

Unfortunately, it does not look like something we could easily solve. I did not manage to extract the bug in our codebase, and from dayjs repository issues, it seems they have a few issues about UTC+00:00 timezone

Should have a double look latter

lemmylem commented 1 year ago

Unfortunately, it does not look like something we could easily solve. I did not manage to extract the bug in our codebase, and from dayjs repository issues, it seems they have a few issues about UTC+00:00 timezone

UTC time convert error (and temporary solving method) iamkun/dayjs#2303

startOf not working when the timezone is set to UTC+0:00 iamkun/dayjs#2373

Should have a double look latter

Thanks for looking into this.

It does seem like a tricky one. I was seeing weird behavior with GMT+0 time zones, but there is a workaround for this particular problem (for the time being), and I'd like to run this by you to see if you see potential pitfalls.

So for my app, the user specifies their own time zone via our app internals and we receive the time zone from the server in IANA format (i.e America/Chicago, Africa/Bamako, etc). But as described in this thread, there are issues with GMT+0 offsets.

So what I do to "fix" this issue is I simply check whether or not the given time zone is GMT (which means GMT+0). If it is GMT, then just return "UTC" (because the timezone prop does work for GMT+0 offsets if identified by the string UTC.

It's as simple as this:

export const getTimeZone = (timestamp: number, timeZone: string) => {
  const isGMT = dayjs.tz(timestamp, timeZone).offsetName() === 'GMT';

  return isGMT ? 'UTC' : timeZone;
};

This works for me in all cases I tested. So I can simply do this:

const myTimeZone = getTimeZone(12345678910, "Server/Provided/Time/Zone");

return (
<TimePicker timezone={myTimeZone} />
)

Like I mentioned in my findings, if for example, instead of doing timezone="Africa/Bamako", you do timezone="UTC", it works and it worked for every GMT+0 time zone offset.

From the looks of everything, it works like it's supposed to. However, I'm not really a big fan of hackery and unsure if this could cause any unforeseen side effects I'm not aware of.

Mohan3298 commented 1 year ago

How to set India timezone format

alexfauquette commented 1 year ago

@Mohan3298 Please refer to the timezone documentation. If you face an issue with it, open a distinct issue since it seems unrelated to this one

Don't forget to upgrade to the latest version since its a recent feature

SiarheiLazakovich commented 1 year ago

It seems like the issue gets even weirder. If the DatePicker value is null and the initial timezone is GMT+, when you change the timezone to UTC or GMT-, it becomes broken too.

playground: https://codesandbox.io/s/twilight-dust-pw5lcl?file=/src/DateTimePicker.js

https://github.com/mui/mui-x/assets/10497370/54b0d7ab-bd8f-425d-aac4-5ed5b14beb9d

Update: If the new timezone is less than the previous one, the calendar becomes broken. For example, changing from America/Belize (GMT-6) to America/Chicago (GMT-5) is okay, but changing from America/Belize (GMT-6) to America/Creston (GMT-7) is broken.

michelengelen commented 1 year ago

@alexfauquette I do have a fix for that! Will open a PR shortly! 💯

michelengelen commented 1 year ago

FYI: its not related to UTC. Any timezone that has +00:00/+00:00 (like UTC) does not work

lemmylem commented 1 year ago

FYI: its not related to UTC. Any timezone that has +00:00/+00:00 (like UTC) does not work

I your fix regarding to my OP? Is it a mui issue or a dayjs issue? I have since switched to Moment and it's doing fairly well for the time being.

michelengelen commented 1 year ago

@lemmylem the fix i am working on seems to be an issue with our adapter. the dates that get passed to the PickersDay are incorrect/not respecting the timezone. Not sure yet if the adapter has an issue or if it is DayJS yet, so stay tuned.

There is one fix i do have for the related issue from @SiarheiLazakovich (https://github.com/mui/mui-x/issues/9653#issuecomment-1706667620)

lemmylem commented 1 year ago

@lemmylem the fix i am working on seems to be an issue with our adapter. the dates that get passed to the PickersDay are incorrect/not respecting the timezone. Not sure yet if the adapter has an issue or if it is DayJS yet, so stay tuned.

There is one fix i do have for the related issue from @SiarheiLazakovich (#9653 (comment))

Excellent work. I figured it was something to do with MUI but I didn't have the bandwidth to look through the source code and find the problem in much detail. Keep me posted as I'll be watching this thread in the meantime.

Thanks again for your work!

michelengelen commented 1 year ago

Hey @lemmylem

I did investigate this a bit more in-depth and I discovered that the underlying issue is indeed caused by dayjs.

A short explanation: when using UTC (or UTC-like timzones, that have +00:00/+00:00 offsets) the internal date calculation is broken. To me it looks like it subtracts the system timezone offset so that any value we receive is incorrect.

In my case using 'Africa/Abidjan' timezone results in this (i am in the 'Europ/Berlin' timzone having an offset of+01:00/+02:00`)

dayjs().tz('Africa/Abidjan').startOf('month').format() // => '2023-09-30T22:00:00Z'

which is clearly incorrect.

This does affect the display of any picker component at the moment, so it is a much bigger issue than we anticipated at first.

I am currently discussing steps we can take here with the team, so stay tuned. Please be sure that we will follow up on this. Sorry that we cannot provide a holistic solution to your problem just yet.

michelengelen commented 1 year ago

Hey @lemmylem I did take the time yesterday evening to investigate so I could push a fix to the dayjs repo. https://github.com/iamkun/dayjs/pull/2481

Lets see when the maintainer gets to it.

I hope this resolves all timezone formatting issues we currently have.

cc @flaviendelangle

Cheewbacca commented 1 year ago

Does this also cause a problem with duplicated dates? https://codesandbox.io/s/datepicker-with-dayjs-forked-f5y5j5?file=/src/App.js

I see that removing .tz is a fix, but not for me

michelengelen commented 1 year ago

Hey @Cheewbacca We do support timezones directly in the components. There is a dedicated section about it in our docs. Also consult our docs about the usage of UTC with dayjs here.

flaviendelangle commented 1 year ago

The codesandbox is a valid usage of our timezone API

I would suggest to avoid converting to a dayjs object during render and instead convert it in state (fixed example)

But we still have the same bug. It may have something with the DST that finishes at the end of October.

michelengelen commented 1 year ago

The codesandbox is a valid usage of our timezone API

I would suggest to avoid converting to a dayjs object during render and instead convert it in state (fixed example)

But we still have the same bug. It may have something with the DST that finishes at the end of October.

OH, yes ... DST is another issue with dayjs, but there is an active PR that seems like it'll soon be merged here. My latests tests with this PR did seem to fix a lot of the issues regarding the timezone/UTC problems

alexfauquette commented 1 year ago

About daylight saving: #10584

Cheewbacca commented 1 year ago

The codesandbox is a valid usage of our timezone API

I would suggest to avoid converting to a dayjs object during render and instead convert it in state (fixed example)

But we still have the same bug. It may have something with the DST that finishes at the end of October.

Hello there, actually I have duplicated dates in the fixed example provided by you as @alexfauquette has. Maybe it is actually caused by DST, my timezone is 'Europe/Kiev'. Tag me if I can provide some more info or help with something else

flaviendelangle commented 1 year ago

Hello there, actually I have duplicated dates in the fixed example provided by you as @alexfauquette has

Yes, that's one I'm saying in the last sentence of my message. It's not a fix to the bug described, just a general advise to avoid unwanted computation and behaviors.

TaRaNTuLaH commented 1 year ago

Since I've also stumbled upon issue with DST I will share a solution that is relatively easy:

dayjs.tz.setDefault('IANA_TZ_CODE');

const dayjsWithTimeZone = (...args: DayjsArgsType[]) => {
  return dayjs(...args).tz();
};

const myNewFunction = (date: Dayjs) => {
    const dateWithDST = date.startOf('day').toDate(); //this is to get UTC date
    return dayjsWithTimeZone(dateWithDST);
  }

myNewFunction(date) now returns Dayjs Object with applied DST, and we can manipulate it however we like.

Works like a charm 😄

arthurbalduini commented 1 month ago

The update of dayJS lib to a version after v1.11.12 seems to fix the issue described here.

13919 bumps the version into our repo and potentially fixes this issue on a future relase.

IAluI commented 1 month ago

@arthurbalduini This fix doesn't seem to solve the problem (live example). DateTimePicker works well only when props timezone="UTC" and date created via dayjs.utc().

If use relativeTime plugin, props timezone="UTC" and date create via dayjs.tz() (default dayjs timezone set to UTC), then initial value in input incorrect. But with further interaction it works fine.

When props timezone="default" and and date created via dayjs.tz() (default dayjs timezone set to UTC) or dayjs.utc() it still happens

Switch to a previous date, and if you keep switching, you can see that for every date change, the time changes as well, which is not supposed to happen

flaviendelangle commented 1 week ago

I tried to summarize the remaining UTC problems with dayjs with the latest version of dayjs and the latest version of our package. Feel free to correct me if I'm saying a use-case is corrected when it's not :pray: I can't guarantee what can be fixed on our side but at least we will have a clearer vision of the remaining issues...

Initial message by @lemmylem (link)

Reproduction case

:heavy_check_mark: All the days are correctly rendered in the DatePicker
:heavy_check_mark: The time in the field of the TimePicker is now 03:30 PM which is consistent with the that date string below and consistent with the value in the view.
:warning: The time rendered in the TimePicker still changes when picking a value in the DatePicker

Comment by @SiarheiLazakovich (link)

Reproduction case

:heavy_check_mark: All the days are correctly rendered when switching to UTC or GMT-

Comment by @Cheewbacca (link)

Reproduction case

:warning: The 29th of October 2023 is still duplicated (DST switch)

flaviendelangle commented 1 week ago

Here is a reproduction case striped of all the MUI code for the time changing in the initial message:

import dayjs, { Dayjs } from 'dayjs';
import timezone from 'dayjs/plugin/timezone';
import utc from 'dayjs/plugin/utc';

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

// 1. Date passed in props.value
const date = dayjs.utc('2023-04-17T15:30');

// 2. Date passed to `DayCalendar` with the rendered timezone
const dateWithTimezone = dayjs.tz(date, 'Africa/Bamako');

// 3. Date returned by `DayCalendar` to `DateCalendar` when clicking on the 1st day of the 2nd week
const firstDayOfSecondWeek = dateWithTimezone.startOf('month').startOf('week').add(7, 'day');

// 4. Date returned by `DateCalendar` to `usePickerValue`
let newDate = firstDayOfSecondWeek;
newDate = newDate.set('hour', dateWithTimezone.hour());
newDate = newDate.set('minute', dateWithTimezone.minute());
newDate = newDate.set('second', dateWithTimezone.second());
newDate = newDate.set('millisecond', dateWithTimezone.millisecond());

// 5. Date passed to `onChange` callback with the input timezone
const newDateWithInputTimezone = newDate.utc();
console.log(newDateWithInputTimezone.format('MM/DD/YYYY h:mm A'));

We can replace the .utc() in step 5 with the what the method does inside dayjs:

// 5. Date passed to `onChange` callback with the input timezone
// const newDateWithInputTimezone = newDate.utc();
const newDateWithInputTimezone = dayjs(new Date(newDate.valueOf()), {
  locale: newDate.$L,
  utc: true,
});

For me the problem is that the utc plugins override the valueOf() method to add the following logic:

  proto.valueOf = function () {
    const addedOffset = !this.$utils().u(this.$offset)
      ? this.$offset + (this.$x.$localOffset || this.$d.getTimezoneOffset()) : 0
    return this.$d.valueOf() - (addedOffset * MILLISECONDS_A_MINUTE)
  }

I'm in Europe/Paris timezone (UTC-2 for the date we are working on), so this.$d.getTimezoneOffset() returns -120, which causes the 2 hour shift you see in the example. This shift is only applied if .$offset is defined. So I think either we are doing something wrong (but I can't find why), or dayjs ends up in a weird situation where it thinks the value has a JS Date stored in the user locale timezone and tries to convert it back in UTC, but the JS Date is actually stored in the expected timezone (Africa/Bamako).

Running delete newDate.$offset; fixes the issue, but it's probably not a viable solution and could introduce new bugs :grimacing: .

mui / mui-x

[pickers] Timezone management fails with `dayjs` id the timezone is UTC+00:00 #9653

Duplicates

Latest version

Steps to reproduce 🕹

Current behavior 😯

Expected behavior 🤔

Context 🔦

Your environment 🌎

Order ID or Support key 💳 (optional)

13919 bumps the version into our repo and potentially fixes this issue on a future relase.

Initial message by @lemmylem (link)

Comment by @SiarheiLazakovich (link)

Comment by @Cheewbacca (link)