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

feature/localize-date-to-instance-time-zone #214

Merged

Conversation

tohuynh
Copy link
Collaborator

@tohuynh tohuynh commented Mar 30, 2022

Link to Relevant Issue

This pull request resolves
part of #112

Description of Changes

Include a description of the proposed changes.
Remove moment lib to reduce CDP-frontend lib size and decrease the app load time.

I added an IANA timeZone to the app config context and use it throughout the app to display dates. Now all dates are displayed according to the instance's time zone. For example, anyone around the world viewing Seattle's date will always see 2 PM Seattle time (if the event started at 2 PM Seattle time). Previously, it was displayed according to the user's time zone. This is done by adding a time zone to date.toLocaleDateString or date.toLocaleString

Even selecting a date in the filter component means selecting a date in the instance's time zone. For example, selecting 1/1/2022 in the Seattle instance means selecting 1/1/2022, 12 AM Seattle time. The conversion is done with getTimeZoneDate.

Link to Forked Storybook Site

If component changes (especially visual changes) are contained in this PR, we ask that you provide a link to a publicly viewable version of the Storybook docs site, so PR reviewers can see your changes without having to install and view your code locally.

Please see CONTRIBUTING.md for directions on how this can be done.
https://tohuynh.github.io/cdp-frontend/#/events/756436a9130b

@tohuynh tohuynh self-assigned this Mar 30, 2022
Copy link
Member

@evamaxfield evamaxfield left a comment

Choose a reason for hiding this comment

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

This all looks good for me. I am not going to approve and merge until CouncilDataProject/cookiecutter-cdp-deployment#83 is resolved.

Totally agree with both changes proposed here and in and cookiecutter just a matter of order-of-operations.

@tohuynh
Copy link
Collaborator Author

tohuynh commented Apr 6, 2022

This all looks good for me. I am not going to approve and merge until CouncilDataProject/cookiecutter-cdp-deployment#83 is resolved.

Totally agree with both changes proposed here and in and cookiecutter just a matter of order-of-operations.

Sounds good

Copy link
Collaborator

@BrianL3 BrianL3 left a comment

Choose a reason for hiding this comment

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

We should be sure to add some documentation to ensure people know how to properly format the timezone info that goes into the cookie cutter.

@evamaxfield
Copy link
Member

@tohuynh I am going to ship a new release of stuff today. When possible can you fix the merge conflict and I will get it into the next release -- next week or something

@tohuynh
Copy link
Collaborator Author

tohuynh commented May 24, 2022

@tohuynh I am going to ship a new release of stuff today. When possible can you fix the merge conflict and I will get it into the next release -- next week or something

Ship new release, but won't propagate these changes to all the instances, right?

@evamaxfield
Copy link
Member

Ship new release, but won't propagate these changes to all the instances, right?

This PR and the associated cookiecutter change will not be shipped in todays releases nor will it propagate to all instances until we merge and release it so no worries.

@tohuynh
Copy link
Collaborator Author

tohuynh commented May 24, 2022

I can update this PR maybe this weekend. Will need to resolve the package-lock.json conflict and modify any other recently merged code that uses local datetime to include the timezone.

@evamaxfield
Copy link
Member

Yep sorry for this taking so long. Completely my fault.

@tohuynh
Copy link
Collaborator Author

tohuynh commented May 24, 2022

Yep sorry for this taking so long. Completely my fault.

No worries!

@tohuynh tohuynh force-pushed the feature/localize-date-to-instance branch from 2ac8c5c to f206d3a Compare May 28, 2022 02:03
@tohuynh tohuynh merged commit 98ed482 into CouncilDataProject:main May 28, 2022
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

3 participants