Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

#23838: 24-hours time format fixed #10011

Conversation

nawarajshah
Copy link
Contributor

@nawarajshah nawarajshah commented Jan 27, 2023

Fixing the issue from element-hq/element-web#23838

While exporting the chat, it forces AM / PM output instead of 24-hour format, and it is easily fixed by making hour12: false for the toLocaleString function

Checklist

  • Tests written for new code (and old code if feasible)
  • Linter and other CI checks pass
  • Sign-off given on the changes (see CONTRIBUTING.md)

Signed-off-by: Nawaraj Shah shah.nawaraj.ns@gmail.com


This PR currently has none of the required changelog labels.

A reviewer can add one of: T-Deprecation, T-Enhancement, T-Defect, T-Task to indicate what type of change this is, or add Type: [enhancement/defect/task] to the description and I'll add them for you.

While exporting the chat, it forces AM / PM output instead of 24 hour format, and it is easily fixed by making hour12: false for toLocaleString function
@nawarajshah nawarajshah requested a review from a team as a code owner January 27, 2023 14:36
@github-actions github-actions bot added the Z-Community-PR Issue is solved by a community member's PR label Jan 27, 2023
@dbkr
Copy link
Member

dbkr commented Jan 27, 2023

This looks like a visual change so I'd expect to see a linked issue where we the change is agreed upon, as well as the before and after screenshots mentioned in CONTRIBUTING.md. You've also checked to say you've done DCO signoff but there's no signoff here.

@nawarajshah
Copy link
Contributor Author

i am new on contributing, so if I check something that I shouldn't check, please let me know.
before
Screenshot (25)

after
Screenshot (24)

Copy link
Contributor

@germain-gg germain-gg left a comment

Choose a reason for hiding this comment

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

This code is used throughout the whole product and not only in the context of chat exports.

You will also need to write tests for this PR to be merged as per then contributing guidelines https://github.com/vector-im/element-web/blob/develop/CONTRIBUTING.mdA

Vaibhavg4651 added a commit to Vaibhavg4651/matrix-react-sdk that referenced this pull request Feb 2, 2023
@T-bond
Copy link

T-bond commented Feb 22, 2023

Wouldn't using the user's preference for the 12/24 hour format better than hard coding it? Also using user's locale.

@nawarajshah
Copy link
Contributor Author

Wouldn't using the user's preference for the 12/24 hour format better than hard coding it? Also using user's locale.

I use the available option, it is not hard-coding I think.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Z-Community-PR Issue is solved by a community member's PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants