Skip to content

Localize time formatting with Intl.DateTimeFormat#113

Merged
ggaabe merged 7 commits intoggaabe:mainfrom
borisgraeff-pxo:feat/localize-time-format-with-intl
Apr 2, 2026
Merged

Localize time formatting with Intl.DateTimeFormat#113
ggaabe merged 7 commits intoggaabe:mainfrom
borisgraeff-pxo:feat/localize-time-format-with-intl

Conversation

@borisgraeff-pxo
Copy link
Copy Markdown
Contributor

Why

Time strings generated by toText were always formatted with an English-style 12-hour clock (AM/PM), even when the specified locale was not English.
This PR makes time rendering locale-aware so generated recurrence descriptions are consistent with the selected language.

What Changed

  • Reworked time formatting in src/totext.ts to use Intl.DateTimeFormat with the resolved locale.
  • Updated formatTime to:
    • accept the rule timezone context (Temporal.ZonedDateTime),
    • format hour / minute / second combinations through Intl,
    • preserve timezone-aware output.
  • Updated timezone label extraction (tzAbbreviation) to use locale-aware Intl.DateTimeFormat fallback behavior.
  • Adjusted toText time rendering path to pass locale + timezone context when formatting byHour / byMinute / bySecond.

Tests

Updated i18n expectations in:

  • src/tests/totext.i18n.test.ts
  • src/tests/totext.i18n.extra.test.ts

Expected outputs now reflect locale-specific time conventions:

  • 24h style where applicable (e.g. 17:30 instead of 5:30 PM)
  • locale-specific day periods and timezone naming (e.g. GMT-4, localized variants instead of fixed EDT/CST)

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5f3f87a30c

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@borisgraeff-pxo borisgraeff-pxo marked this pull request as draft March 13, 2026 15:41
@borisgraeff-pxo borisgraeff-pxo marked this pull request as ready for review March 13, 2026 17:33
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 52c69d4f10

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@borisgraeff-pxo
Copy link
Copy Markdown
Contributor Author

Hi @ggaabe, could you have a look? We're just waiting for those two PRs to migrate from rrule.js to your library 🙏

@fgc-
Copy link
Copy Markdown
Contributor

fgc- commented Apr 1, 2026

lgtm 👍

I think it might just need a rebase on main, as there have been recent changes (the build is currently failing).

@borisgraeff-pxo borisgraeff-pxo force-pushed the feat/localize-time-format-with-intl branch from 4aaddb1 to b98187e Compare April 1, 2026 18:53
@borisgraeff-pxo
Copy link
Copy Markdown
Contributor Author

rebased 👍

@borisgraeff-pxo borisgraeff-pxo force-pushed the feat/localize-time-format-with-intl branch from b98187e to 3ddfb23 Compare April 2, 2026 14:17
@ggaabe ggaabe merged commit 57aad9b into ggaabe:main Apr 2, 2026
@borisgraeff-pxo
Copy link
Copy Markdown
Contributor Author

@ggaabe it seems the issue with the formatting difference between my local env and the CI is still there: https://github.com/ggaabe/rrule-temporal/actions/runs/23905071509/job/69711571666
How can I fix this? Should I create an array containing both strings and check whether one or the other matches?

@fgc-
Copy link
Copy Markdown
Contributor

fgc- commented Apr 2, 2026

Sorry @borisgraeff-pxo, I didn't see your comment before suggesting #120.
The tests were broken on my end, I fixed them and maybe pushed a little too fast 🤷🏻

That said, I think it fits with what you're suggesting.

@ggaabe, what do you think?

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.

3 participants