Skip to content

Address remaining PR #768 review comments for locale safety and selector correctness - #770

Merged
adubovikov merged 4 commits into
homer11from
copilot/fix-code-for-comments-in-review-thread
Jun 2, 2026
Merged

Address remaining PR #768 review comments for locale safety and selector correctness#770
adubovikov merged 4 commits into
homer11from
copilot/fix-code-for-comments-in-review-thread

Conversation

Copilot AI commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

This PR applies the unresolved Copilot review feedback from thread #pullrequestreview-4408060524 in PR #768, with minimal scoped changes. It hardens locale/time formatting paths and fixes edge-case rendering in the profile locale selector.

  • Locale provider type and context contract

    • Replaced type LocalePref = "auto" | string with a non-widening union so "auto" remains a real sentinel in type narrowing.
    • Switched LocaleProviderContext to LocaleProviderState | undefined defaulting to undefined, so useLocale() correctly throws outside LocaleProvider instead of silently returning defaults.
  • Date/time option precedence

    • Updated withTimeZone to always strip opts.timeZone first, then apply the function parameter.
    • Ensures timeZone argument is authoritative (including explicit "local" semantics) regardless of caller-supplied options.
  • Profile locale selector unknown-value handling

    • Added fallback rendering for currently selected locale values not present in LOCALE_TAGS (e.g., historical/manual values), so the Select keeps a visible selected item.
  • Focused coverage for new behavior

    • Added tests for:
      • useLocale() throwing outside provider,
      • withTimeZone stripping/overriding existing opts.timeZone,
      • Profile locale selector preserving unknown stored locale values.
// datetime.ts
export function withTimeZone(opts: DateTimeFormatOptions, timeZone?: string): DateTimeFormatOptions {
  const { timeZone: _ignored, ...rest } = opts
  if (!timeZone || timeZone === 'local') return rest
  return { ...rest, timeZone }
}

@CLAassistant

CLAassistant commented Jun 2, 2026

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ fredrik-dahlgren
❌ Copilot
You have signed the CLA already but the status is still pending? Let us recheck it.

fredrik-dahlgren and others added 3 commits June 2, 2026 10:39
Hardcoded en-GB / en-US locales rendered dates the same way for every
viewer, ignoring the browser preference. Users whose locale uses ISO 8601
(e.g. sv-SE) had no way to see `2026-06-01` instead of `01/06/2026`.

Adds a LocaleProvider (mirrors ThemeProvider) backed by localStorage and
exposed under Settings → Profile → "Date & time format". The "Auto"
default resolves to `navigator.language`, so users with a Swedish
browser get ISO output without any configuration. The picker lists ~60
BCP-47 tags labelled via Intl.DisplayNames in the active locale. The
"Auto · <tag>" label always shows the real browser default rather than
the currently selected locale, so its meaning stays consistent
regardless of selection.

11 display formatters (MessageModal, TransactionModal, OTLP*,
QosPanel, ClockPanel, ResultsPanel, TimeRangePicker display, flow-data)
now read the locale from context and pass it to Intl.DateTimeFormat
instead of hardcoding `en-GB`. The cosmetic `.replace(',', '')` and the
`hour12: false` / `2-digit` overrides are dropped so the locale's
natural conventions apply; `fractionalSecondDigits: 3` is preserved
where it was (SIP packet timing).

Left untouched: `resolveTimeRange.ts` and `TimeRangePicker`'s input
formatter path. Those use `formatToParts` to read deterministic ISO
components for `<input type="datetime-local">` and timezone math — not
for display — so they keep the existing locale literal.
Copilot AI changed the title [WIP] Fix code for all comments in review thread Address remaining PR #768 review comments for locale safety and selector correctness Jun 2, 2026
Copilot AI requested a review from adubovikov June 2, 2026 10:42
@adubovikov
adubovikov marked this pull request as ready for review June 2, 2026 15:52
@adubovikov
adubovikov merged commit 253cffb into homer11 Jun 2, 2026
9 of 10 checks passed
@adubovikov
adubovikov deleted the copilot/fix-code-for-comments-in-review-thread branch June 2, 2026 16:36
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.

4 participants