Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
2 tasks done
lemmylem opened this issue Jul 12, 2023 · 23 comments
Open
2 tasks done
Labels
bug 馃悰 Something doesn't work component: pickers This is the name of the generic UI component, not the React module! time-zone Issues about time zone management

Comments

@lemmylem
Copy link

lemmylem commented Jul 12, 2023

Duplicates

  • I have searched the existing issues

Latest version

  • 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:

  1. Go to the above link and see that the initial time is set at 2023-04-17T15:30, which translates to 3:30pm.
  2. Also see that the time picker (with the timezone prop of Africa/Bamako, which is GMT) is displaying the wrong time (7:30PM)
  3. Also notice that when you click on the time picker icon to open the time selection dialog, is holds the correct value.
  4. 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 lemmylem added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jul 12, 2023
@lemmylem lemmylem changed the title Wrong time being displayed in Time Picker using timezone prop (and other weirdness) Wrong time being displayed in Time Picker using timezone prop (and other weirdness in GMT 0 offset time zones) Jul 12, 2023
@lemmylem
Copy link
Author

lemmylem commented Jul 12, 2023

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?

@zannager zannager added the component: pickers This is the name of the generic UI component, not the React module! label Jul 13, 2023
@alexfauquette alexfauquette changed the title Wrong time being displayed in Time Picker using timezone prop (and other weirdness in GMT 0 offset time zones) [pickers] Timezone management fails with dayjs id the timezone is UTC+00:00 Jul 13, 2023
@alexfauquette
Copy link
Member

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

@alexfauquette alexfauquette added bug 馃悰 Something doesn't work and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jul 13, 2023
@lemmylem
Copy link
Author

lemmylem commented Jul 13, 2023

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
Copy link
Member

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
Copy link
Author

lemmylem commented Jul 13, 2023

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

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
Copy link

How to set India timezone format

@alexfauquette
Copy link
Member

@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

@alexfauquette alexfauquette added the time-zone Issues about time zone management label Aug 7, 2023
@SiarheiLazakovich
Copy link

SiarheiLazakovich commented Sep 5, 2023

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

Screen.Recording.2023-09-05.at.16.27.16.mov

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
Copy link
Member

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

@michelengelen
Copy link
Member

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

@lemmylem
Copy link
Author

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
Copy link
Member

@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))

@lemmylem
Copy link
Author

@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 michelengelen self-assigned this Oct 16, 2023
@michelengelen
Copy link
Member

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
Copy link
Member

Hey @lemmylem I did take the time yesterday evening to investigate so I could push a fix to the dayjs repo. iamkun/dayjs#2481

Lets see when the maintainer gets to it.

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

cc @flaviendelangle

@Cheewbacca
Copy link

Cheewbacca commented Oct 18, 2023

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

image

@michelengelen
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

About daylight saving: #10584

@Cheewbacca
Copy link

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
image

@flaviendelangle
Copy link
Member

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
Copy link

TaRaNTuLaH commented Oct 26, 2023

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 馃槃

@michelengelen michelengelen removed their assignment Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 馃悰 Something doesn't work component: pickers This is the name of the generic UI component, not the React module! time-zone Issues about time zone management
Projects
None yet
Development

No branches or pull requests

9 participants