Skip to content

feat: render 12/24 hour format according to user's preference and add hourCycle option#329

Open
silverwind wants to merge 11 commits intogithub:mainfrom
silverwind:24h
Open

feat: render 12/24 hour format according to user's preference and add hourCycle option#329
silverwind wants to merge 11 commits intogithub:mainfrom
silverwind:24h

Conversation

@silverwind
Copy link

@silverwind silverwind commented Oct 28, 2025

Fixes: #276

In my testing on MacOS, neither Chrome, Safari or Firefox did follow the OS's "24-hour time" setting, so from what I gather, browsers seem to try to determine this based on locale only currently.

Before (with 24h preference):

Screenshot 2025-10-28 at 00 52 56

After (with 24h preference):

Screenshot 2025-10-28 at 00 52 37

With user config override:
Screenshot 2025-10-28 at 01 45 04

@silverwind silverwind requested a review from a team as a code owner October 28, 2025 00:00
@silverwind silverwind marked this pull request as draft October 28, 2025 00:31
@silverwind silverwind marked this pull request as ready for review October 28, 2025 00:41
@siddharthkp
Copy link
Contributor

siddharthkp commented Nov 10, 2025

@silverwind I'm not entirely sure, but would it be possible to add some tests as well?

@silverwind
Copy link
Author

Yes, some tests should be possible and given that they run in Chromium they might actually return some useful results.

@silverwind
Copy link
Author

Maybe @keithamus also has some feedback.

@silverwind
Copy link
Author

silverwind commented Nov 10, 2025

Also, I'm certain now that Intl.Locale.prototype.getHourCycles is a flawed approach that will not work reliably because it obtains the hourCycle preference for a given locale which not necessarily matches what the user has configured. It fails in my case where I have en-US configured as OS locale but have 24h cycle set in the OS. In https://stackoverflow.com/questions/27647918, there are various workarounds being discussed and all of them basically rely on checking how dates as being formatted as string.

@siddharthkp
Copy link
Contributor

siddharthkp commented Nov 11, 2025

Yes, some tests should be possible and given that they run in Chromium they might actually return some useful results.

Sounds good! Assigning it back to you then :)

No obligation, of course! Let me know if you'd like one of us to pick up the tests instead

@Peronia
Copy link

Peronia commented Jan 30, 2026

Hi, I came here via the bug with the time display in Gitea. What is the current status? Will this feature still be merged?

@francinelucca
Copy link
Contributor

👋🏽 @Peronia we're still happy to get this in, looks like there's some conflict resolution that needs to happen and also we're waiting on some tests for final approval

@silverwind
Copy link
Author

Besides the conflict, more testing in different OSs and locales would be great. I can only test MacOS, would be interested in knowing whether this works in Windows/Linux as well.

@silverwind silverwind marked this pull request as draft February 13, 2026 16:46
@silverwind silverwind force-pushed the 24h branch 2 times, most recently from bea0768 to a2d1259 Compare February 14, 2026 10:15
@silverwind silverwind changed the title Render 12/24 hour format according to user's preference Render 12/24 hour format according to user's preference and add hourCycle option Feb 14, 2026
Add hour-cycle to observedAttributes so dynamic attribute changes
trigger re-renders. Add test suite covering all four hour cycle values,
title formatting, ancestor inheritance, attribute override precedence,
and dynamic re-rendering. Fix existing locale test to use explicit
hour-cycle attribute.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@silverwind silverwind marked this pull request as ready for review February 14, 2026 10:37
Copilot AI review requested due to automatic review settings February 14, 2026 10:37
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds an hour-cycle configuration path to <relative-time> so rendered times can use a 12/24-hour clock based on user/browser preference, with an explicit override via attribute (and inheritance from ancestors / documentElement).

Changes:

  • Introduces an hourCycle getter + hour-cycle observed attribute and uses it to influence time formatting.
  • Adds a new [hourCycle] test suite covering explicit values, inheritance, and re-render on attribute change.
  • Updates README and examples to document/show the new hour-cycle usage.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.

File Description
src/relative-time-element.ts Adds hour-cycle detection + wiring into Intl.DateTimeFormat options.
test/relative-time.js Adds coverage for hour-cycle values, inheritance, and attribute-change re-rendering.
README.md Documents hourCycle / hour-cycle in the attribute table.
examples/index.html Demonstrates hour-cycle="h12" and hour-cycle="h23".

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@silverwind
Copy link
Author

Tests are added and minor issues identified by Claude are resolved. I also let Claude do a self-review and it thinks the approach is solid.

- Use resolvedOptions().hour12 instead of whitespace heuristic for
  browser hour cycle detection
- Pass hourCycle directly to Intl.DateTimeFormat instead of converting
  to hour12, preserving h11/h12/h23/h24 distinctions
- Remove isHour12() helper, no longer needed
- Validate hourCycle getter against allowed values
- Add positive assertion to h24 test
- Fix README formatting for hourCycle default value

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@silverwind
Copy link
Author

silverwind commented Feb 14, 2026

Manually tested latest changes, works fine in Firefox on MacOS with 24h OS preference:

image

@silverwind
Copy link
Author

silverwind commented Feb 14, 2026

I also validated the minimum browser versions for these new APIs (Chrome 73, Firefox 58, Safari 13, Edge 18) and they align with the browser compatibilty in the README.

@silverwind silverwind changed the title Render 12/24 hour format according to user's preference and add hourCycle option feat: render 12/24 hour format according to user's preference and add hourCycle option Feb 14, 2026
// browser's resolved DateTimeFormat options.
function isBrowser12hCycle(): boolean {
try {
return new Intl.DateTimeFormat([], {hour: 'numeric'}).resolvedOptions().hour12 === true
Copy link
Contributor

Choose a reason for hiding this comment

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

I no longer work at GitHub, nor do I maintain this repository any more, but I was curious about this change.

I think this sould probably use the defined locale (this.#lang) so that if a page allows customisation of the rendered locale, this will take precedent over the browser.

Copy link
Author

@silverwind silverwind Feb 14, 2026

Choose a reason for hiding this comment

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

I think hour cycle is a personal user preference and the document language (which #lang derives from) should not overrule that preference. I prefer 24h and don't want to see 12h just because I happen to browse a en-US document.

The current implementation here uses DateTimeFormat([]) which makes the browser use its "default locale". How a browser determines it is not specced, but here is what Chrome on MacOS does as of today (written by Claude):

  1. [] → empty locale list — V8's CanonicalizeLocaleList returns an empty vector.
  2. Fallback to ICU default locale — V8 calls icu::Locale::getDefault().getName() and converts it to a BCP 47 tag.
  3. ICU default was set at renderer startup — In content/renderer/renderer_main.cc, Chromium calls
    base::i18n::SetICUDefaultLocale() with the value from Chrome's GetApplicationLocale(). On macOS, this reads
    NSBundle.preferredLocalizations[0] — the first OS language Chrome has a .pak translation for.
  4. Hour cycle from CLDR data — V8 creates an ICU DateTimePatternGenerator for the resolved locale and calls
    udatpg_getDefaultHourCycle(), which looks up the locale's region in CLDR's timeData (e.g., US → h12, GB/DE → h23).
  5. No OS override — Chrome ships its own bundled ICU (not Apple's system ICU), so macOS settings like
    AppleICUForce24HourTime or the "24-hour time" toggle have zero effect. The hour cycle comes entirely from CLDR data for
    the resolved locale's region.

In my case 24h is determined from my locale's region (EU), and the OS setting has no effect because Google decided to not use Apple's ICU.

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.

Render 12/24 hour format according to user's preference

5 participants