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 calendar select correct date in negative UTC offset timezones #5731

Closed

Conversation

darrelhong
Copy link
Contributor

@darrelhong darrelhong commented Feb 5, 2023

This PR fixes an issue brought up in #5510. It seems to be caused modifications to date in handleDateChange.
demo:
https://user-images.githubusercontent.com/39296145/216810183-b8ca0036-416b-45c1-be25-530eb6c90729.mov

  1. Date picker selection returns date at 00:00:00 in local timezone
  2. Default 'UTC' string is passed to zonedTimeToUtc
  3. zonedTimeToUtc returns equivalent datetime in UTC after comparing local timezone to 'UTC' time
  4. Any local timezone that is negative offset from UTC will result in the date being change to the day before.

For better visualisation:
BEFORE
Screen Shot 2023-02-05 at 2 08 35 AM

AFTER
Screen Shot 2023-02-05 at 2 07 19 AM

Solution

Instead of specifying 'UTC' to zoneTimeToUtc, provide the local timezone instead. This fixes the issue but I'm not actually sure whether its correct as it feels like its just reversing the use of zoneTimeToUtc. More info at marnusw/date-fns-tz.js#174

I believe this can be discussed further @karrui

Breaking Changes
No - this PR is backwards compatible

@darrelhong
Copy link
Contributor Author

@karrui @justynoh @mantariksh any update on this, maybe could use this a temp fix before a more comprehensive fix

@LinHuiqing
Copy link
Contributor

Hi @darrelhong, thanks for your contributions! Sorry we took so long to get around to this. We've decided to solve the datepicker issue by removing the concept of timezone altogether in this fix (#6025). As you mentioned, your fix will be essentially undo-ing the effects of zoneTimeToUtc, which is removing the concept of timezones, so in effect, what your PR does is the same as what #6025 does.
As such, we'll be closing this PR. Do let us know if you have any comments on the chosen fix though. Thank you!

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