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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(date-picker): ignore timezone in date-picker #2870

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Birkbjo
Copy link
Member

@Birkbjo Birkbjo commented Jun 3, 2024

No description provided.

@Birkbjo Birkbjo requested a review from tomzemp June 3, 2024 16:03
Copy link
Member

@tomzemp tomzemp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me 🎉.

I tested with server in Oslo (CEST) and with browser time set to Bogotá and Vientiane, respectively. Worked as expected.

I still think it makes more sense to have substring(0,23) after toISOString() in getISOFormatLocalTimestamp, in order to get rid of the Z at the end of the string.

With this PR we:

  • select a date with Material UI date picker. Material UI thinks we want the date at 00:00 local time. If we've selected 31 May and we're in Vientiane, that is then 30 May 17:00 UTC.
  • we apply the conversion function ((date.getTime() - date.getTimezoneOffset() * 60000).toISOString(), we get a string back 2024-05-31T00:00:00.000Z
  • we send the date off and the server ignores the Z and just assumes that we want 2024-05-31T00:00:00.000 server time

I guess my argument for removing the Z is that the Z stands for zero offset (from UTC), so with the Z we are saying we are sending the time in UTC, but what we want (and what the backend does because it ignores time zones) is to have the date string representation of the time in the server time zone (without any time zone information).

@Birkbjo
Copy link
Member Author

Birkbjo commented Jun 5, 2024

Thanks for testing it @tomzemp !

still think it makes more sense to have substring(0,23) after toISOString() in getISOFormatLocalTimestamp, in order to get rid of the Z at the end of the string.

Alright, I see what you mean! My worry was that then it's not strictly an isoString anymore, but I guess the name of the function doesn't actually say that either.

Will remove the z and update the comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants