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

Add custom timezone support to moment localizer #2406

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

Conversation

ariel22411
Copy link

Add a custom timezone to moment localizer

#2400

@ariel22411 ariel22411 changed the title Add custom timezone support Add custom timezone support to moment localizer May 31, 2023
Copy link
Collaborator

@cutterbl cutterbl left a comment

Choose a reason for hiding this comment

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

I like the track you're on, but unfortunately this will not work. It assumes that all users of Big Calendar, using moment, are also using moment-timezone. Many implementations only use moment without timezone support, so initializing a date with moment.tz() will fail for them, as they don't have the necessary dependencies to extend moment.

A better option might be to

  • If timezone is passed as an argument, then
  • test if moment.tz is a thing, and
  • if not, log a console error, but gracefully ignore, and use moment() on it's own

@ariel22411 ariel22411 requested a review from cutterbl June 12, 2023 13:33
@ariel22411
Copy link
Author

ariel22411 commented Jun 18, 2023

I changed according to your review @cutterbl

@cutterbl
Copy link
Collaborator

@ariel22411 One final request here is to add an example to the documentation, in using the momentLocalizer in this per instance timezone matter, with an included brief explanation on possible use case for doing so.

@ariel22411
Copy link
Author

ariel22411 commented Sep 12, 2023

sorry for the delay, now its okay? @cutterbl

@tinsaeopus1io
Copy link

We've been using this for months, and it has been working great. But now we are having issues on the calendar week view for dates on the week of dst, where the time gutter is off by 1 hour. Please let us know if anyone experienced this issue, and has a fix or workaround @samopus1io @canopus1io

@canopus1io
Copy link

canopus1io commented Sep 21, 2023

here are a couple code sandbox examples:
America/Los_Angeles DST example
Europe/London DST example (looks correct in all cases if you have your system timezone set to Europe/London)

there are two localizer versions to compare here (moment-tz is the localizer in this PR, moment-tz modified is with one tweak that seems to fix the gutter time labels)

here is the tweak diff for the modified version:
image

@canopus1io
Copy link

@ariel22411 thoughts on this change? (only seems to be different when DST shift is not included in displayed time range for the week)

@canopus1io canopus1io mentioned this pull request Sep 21, 2023
3 tasks
@ariel22411
Copy link
Author

ariel22411 commented Feb 4, 2024

@ariel22411
Copy link
Author

The next problem is the DST slots time: (see if you can help me with that) @cutterbl
image

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.

None yet

4 participants