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

UI: Added DateTime of Event on the Client Side in the User's Timezone #2339

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

maha-sachin
Copy link
Contributor

@maha-sachin maha-sachin commented Apr 12, 2024

Resolves #2327

Related Issue:: This PR addresses issue #2327 by adding the requested functionality to render DateTime in the user's timezone.
Description:
This PR adds functionality to render the DateTime of events on the client side in the user's timezone. Previously, the DateTime was displayed in UTC, which could be confusing for users in different timezones. With this update, the DateTime is dynamically converted to the user's local timezone using JavaScript's Date object.

Changes Made:
Modified the template to include JavaScript logic for converting DateTime to the user's timezone.
Updated the rendering of event start and end times to display in the user's local timezone.
Implemented handling for cases where no time is provided, displaying only the date.

Screenshots:
Before Modification:
Screenshot 2024-04-12 at 4 50 01 PM
After Modification:
Screenshot 2024-04-12 at 4 49 38 PM
Expected Output: This illustrates how the event date/time should be displayed in the user's local timezone.
Screenshot 2024-04-12 at 4 56 00 PM

@sabine , Could you please take some time to review it and provide your feedback? Your insights would be invaluable in ensuring that the changes meet the project's requirements and standards.

Looking forward to hearing your thoughts!

Copy link
Collaborator

@sabine sabine left a comment

Choose a reason for hiding this comment

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

Hey @maha-sachin, thanks for doing this!

Some feedback:

  1. It will help to list the time zone to make it clear to people what time zone they're seeing this in
  2. only list hh:mm, not seconds.
  3. on an event with start and end time, the time zone should be only listed once
  4. on an event without start and end time, just the dates need to be shown

@maha-sachin
Copy link
Contributor Author

Hey @maha-sachin, thanks for doing this!

Some feedback:

  1. It will help to list the time zone to make it clear to people what time zone they're seeing this in
  2. only list hh:mm, not seconds.
  3. on an event with start and end time, the time zone should be only listed once
  4. on an event without start and end time, just the dates need to be shown

Hi @sabine , Thanks again for your valuable input! I'll make these adjustments to improve the readability and usability of the event details.

@maha-sachin
Copy link
Contributor Author

Hi @sabine , Please review the changes and let me know if any further adjustments are needed.

without start time

Screenshot 2024-04-15 at 4 23 07 PM

with start and end time zone
Screenshot 2024-04-15 at 4 43 27 PM

@sabine
Copy link
Collaborator

sabine commented Apr 17, 2024

Hey @maha-sachin, I pushed a patch to make this compile and added a check that events cannot have no start time, but an end time.

For events with only a start time, currently, no time zone is being rendered:

image

long term to fix the API right now in order to avoid multiplying the number of
supported versions of the shape API in the various OCaml developer tools .

We have thus released a second beta version of OCaml 5.2.0 to give the time to developer tools to update their 5.2.0 version ahead of the release (see below for the installation instructions).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
We have thus released a second beta version of OCaml 5.2.0 to give the time to developer tools to update their 5.2.0 version ahead of the release (see below for the installation instructions).
We have thus released a second beta version of OCaml 5.2.0, so there will be time for the developer tools to update their 5.2.0 version ahead of the release (see below for the installation instructions).

I'm a little confused by this sentence. Does the above suggestion capture what you mean?


We have thus released a second beta version of OCaml 5.2.0 to give the time to developer tools to update their 5.2.0 version ahead of the release (see below for the installation instructions).

Beyond this changes of API, the new beta contains three more bug fixes and three
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Beyond this changes of API, the new beta contains three more bug fixes and three
Beyond these API changes, the new beta contains three more bug fixes and three

Copy link
Collaborator

@christinerose christinerose left a comment

Choose a reason for hiding this comment

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

I left a suggestion to hopefully clarify a sentence. If it's not accurate, we'll work together to make it more clear.

@sabine
Copy link
Collaborator

sabine commented Apr 17, 2024

@christinerose these changes have nothing to do with this PR and are most likely included accidentally

@maha-sachin
Copy link
Contributor Author

@sabine, I will update here once I fix the PR.

@maha-sachin maha-sachin force-pushed the feature/render-event-datetime-in-user-timezone-2327 branch from 5d6a199 to 90ba962 Compare April 25, 2024 19:40
@maha-sachin
Copy link
Contributor Author

Hi @sabine

@maha-sachin maha-sachin reopened this Apr 25, 2024
@maha-sachin
Copy link
Contributor Author

Hi @sabine, I have addressed the Timezone issue and reviewed the changes.
Please let me know if everything looks good to you.

This code now displays the time zone under the following conditions:

- If there is either a start time or an end time, the time zone will be displayed
- If both start time and end time are provided, the time zone will be displayed with the start time only.
- If only a start time is present, the time zone will be displayed with the start time.
- If only an end time is present, the time zone will be displayed with the end time.
- If neither start time nor end time is present, the time zone will not be displayed.

Events page:
Screenshot 2024-04-24 at 5 09 18 PM
Overview page:
Screenshot 2024-04-25 at 3 14 39 PM

Could you please take some time to review it and provide your feedback? Your insights would be invaluable in ensuring that the changes meet the project's requirements and standards.

Looking forward to hearing your thoughts!

@sabine, Could you please help me understand what might be causing this ABOVE CI/Build error and how it can be resolved?

Thank you for your assistance!

@maha-sachin
Copy link
Contributor Author

Hi @christinerose @sabine , Could you please help me understand what might be causing this ABOVE CI/Build error and how it can be resolved?

Thank you for your assistance!

Copy link
Collaborator

@sabine sabine left a comment

Choose a reason for hiding this comment

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

Hey @maha-sachin! This looks really nice, but there's two things I'd like to ask:

  1. can you turn the event card into a component, so that the same component is used in both the Events page and the Community page?
  2. can you move the JavaScript to a single place such that we don't have to repeat it in different templates (this could be done either by moving it into a file and including just the file in the template, or by having the script tag as a component).

ETA: you can ignore the build error because it is unrelated to your changes

@maha-sachin
Copy link
Contributor Author

Hey @maha-sachin! This looks really nice, but there's two things I'd like to ask:

  1. can you turn the event card into a component, so that the same component is used in both the Events page and the Community page?
  2. can you move the JavaScript to a single place such that we don't have to repeat it in different templates (this could be done either by moving it into a file and including just the file in the template, or by having the script tag as a component).

ETA: you can ignore the build error because it is unrelated to your changes

@sabine , Sure thing! I'll work on turning the event card into a reusable component and consolidating the JavaScript into a single location to avoid repetition across templates. I'll make sure to get back to you once it's done. Thanks for pointing these out!

@maha-sachin
Copy link
Contributor Author

Hey @maha-sachin! This looks really nice, but there's two things I'd like to ask:

  1. can you turn the event card into a component, so that the same component is used in both the Events page and the Community page?
  2. can you move the JavaScript to a single place such that we don't have to repeat it in different templates (this could be done either by moving it into a file and including just the file in the template, or by having the script tag as a component).

ETA: you can ignore the build error because it is unrelated to your changes

@sabine & @christinerose , I've created a JavaScript file named event_script.js for handling event-related functionality across multiple templates. Could you please advise me on where would be the best place to store this file within our project folder structure?

Additionally, when creating components in OCaml, I'd like to follow a consistent file naming convention. Could you let me know what the preferred naming convention is for component files in our project?

Your guidance on these matters would be greatly appreciated.

Thank you!

@sabine
Copy link
Collaborator

sabine commented May 15, 2024

I've created a JavaScript file named event_script.js for handling event-related functionality across multiple templates. Could you please advise me on where would be the best place to store this file within our project folder structure?

In the table of contents component components/toc.eml, we put the script as an OCaml value, and we then render it on every page that has a table of contents. Would this be an option?

Could you let me know what the preferred naming convention is for component files in our project?

We have a file components/cards.eml where a function event_card could be added.

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

Successfully merging this pull request may close these issues.

(UI) Render DateTime of Event on the Client Side in the User's Timezone
3 participants