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

always show 00 for midnight #15

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

Conversation

michael-speed-elder
Copy link
Collaborator

@michael-speed-elder michael-speed-elder commented Mar 22, 2024

There are two common formats for displaying time:

  • A 24-hour range which starts the day at 00:00 and counts up to 23:59
  • A 12-hour range that never has hours lower than 1 or higher than 12, where before noon and after noon uses a suffix to distinguish between similar numbers (1:23 AM vs 1:23 PM)

For the purpose of this PR, the significant difference between these styles is what the smallest and largest possible numbers used for hours are. In 12-hour time, those numbers are "1" and "12," while in 24-hour time, they are "0" and "23." Javascript time formatting gives developers a boolean flag called hours12, which seems like the way to switch between these two systems, but this is unintuitively incorrect.

The hours12 flag might be better thought of as "how many hours should pass before I reset the count to the minimum number." In a typical 12-hour clock system, twelve hours pass before the number is reset to "1." In a typical 24-hour system, twenty-four hours pass before the number is reset to "0." So if you come from a location where you normally see time in a 12-hour range, and some developer changes the hours12 flag hoping to show you 24-hour time instead, you will actually end up in a time system with the smallest hour of "1:00" that doesn't reset back to that number until 24 hours have passed, meaning one hour a day will read as "24:XX." This is the source of the bug.

This has been updated to require counting from "0:00" to "23:59" in all locales.

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.

1 participant