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

Time setting seems to default to 24h #850

Open
MelissaAutumn opened this issue Feb 4, 2025 · 8 comments
Open

Time setting seems to default to 24h #850

MelissaAutumn opened this issue Feb 4, 2025 · 8 comments
Assignees
Labels
bug Something isn't working

Comments

@MelissaAutumn
Copy link
Member

I'm guessing this is related to the store changes, but it seems like it at least breaks sanity tests.

cc @rwood-moz @devmount

@MelissaAutumn MelissaAutumn added the bug Something isn't working label Feb 4, 2025
@rwood-moz
Copy link
Contributor

rwood-moz commented Feb 4, 2025

Example test failure here.

Expected string: "Wednesday, February 5, 2025 11:30 AM"
Received string: "Wednesday, February 5, 2025 11:30"

The test started failing after production release 0406. Before the release 12HR time was used (as expected by the test). I'm guessing maybe this is the change that caused the date/time to now appear as 24hr time (by default)?

@devmount Was the default always 24 hour format but your PR above fixed it so that the defaults were applied? In which case I will update the production sanity E2E test to expect 24 hour format now instead. :)

@devmount
Copy link
Collaborator

devmount commented Feb 4, 2025

I guess I'm to blame here 😅 I implemented the general settings store in #828 which set 24h as default iirc and nobody complained 😂

@devmount
Copy link
Collaborator

devmount commented Feb 4, 2025

@rwood-moz Yes, in my understanding it was like this. If you rather want h12 as default, we can implement that too.

@rwood-moz
Copy link
Contributor

rwood-moz commented Feb 4, 2025

@devmount No blame, just trying to figure out what happened :) For the default time format (12 or 24hr) whatever you and @MelissaAutumn prefer works for me! If it's 24 hr I can update the tests easily. :)

@devmount
Copy link
Collaborator

devmount commented Feb 4, 2025

@MelissaAutumn I think you approved the h24 default? Any opinion on this?

@MelissaAutumn
Copy link
Member Author

We should probably be consistent with default settings between services. Most of the team is 12 hour time based, but dunno about user stats.

cc @malini @ryanjjung @radishmouse @aaspinwall for any input / suggestions.

@malini
Copy link
Contributor

malini commented Feb 4, 2025

I don't know either, honestly.

I'd default to 12hr for users and they can swap to 24hrs. My guess is that people used to 12hrs would be immediately confused by seeing 18:00 and might think it an error, but someone used to 24 hours would see 6PM and think "ugh, gotta switch to 24 hour mode".

@devmount devmount self-assigned this Feb 5, 2025
@devmount
Copy link
Collaborator

devmount commented Feb 5, 2025

Alright, I'll switch this to 12 hr default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants