Repository navigation
UI: Group digits of counters according to the selected locale - #73713
Merged
Merged
Conversation
boring-cyborg
Bot
added
area:translations
area:UI
shahar1
force-pushed
the
ui-group-digits
branch
from
September 25, 2026 09:43
32bf8d3 to
309051a
Compare
shahar1
marked this pull request as ready for review
September 25, 2026 12:54
shahar1
requested review from
bbovenzi,
choo121600,
guan404ming,
pierrejeambrun,
ryanahamilton,
shubhamraj-git and
vatsrahul1001
as code owners
September 25, 2026 12:54
shahar1
force-pushed
the
ui-group-digits
branch
from
October 1, 2026 16:13
309051a to
958837a
Compare
bbovenzi
reviewed
Oct 1, 2026
shahar1
force-pushed
the
ui-group-digits
branch
2 times, most recently
from
October 2, 2026 06:13
19653ee to
99bb808
Compare
Counters such as the number of Dags, runs, task instances, import errors, pool slots or pagination pages were rendered as raw integers, so large values like 12345 were hard to read and ignored the conventions of the selected UI language (12,345 in English, 12.345 in German, 12 345 in French). Formatting goes through one shared hook backed by the cached Intl fallback already used for durations, because plugins can contribute UI languages whose tags Intl rejects (e.g. "pt_BR"); calling toLocaleString with such a tag throws a RangeError and would crash every page that shows a counter. The Dag warnings count now includes the parse error as originally intended, and the Dag header no longer shows "null" when max active runs is unset.
A dedicated hook is more ceremony than digit grouping needs: components read the active language from `useTranslation` and pass it to a plain function, as the two counters on main already did through `toLocaleString`. The language is a required argument rather than a default read from the i18next singleton inside the function because the React Compiler memoizes the call on its arguments, so a language read inside would not invalidate a counter whose memoized output depends only on the number.
Number(<Error>) is NaN, so after the precedence fix in the first commit the warnings button read "Dag Warnings (NaN)" whenever a Dag, run or task page failed to load. The old precedence bug had masked it once warnings loaded. The grouping tests also pin the language through a react-i18next mock: without an i18next instance, formatNumber falls through to the host ICU locale, which is "de-DE" on a German dev box and breaks the "1,234" assertions.
…h tests Both behaviour fixes shipped without a regression test. The warnings count moves into countDagWarnings next to the modal that defines what the count lists, so it can be tested without rendering the whole details layout. The three Dag-list selection counters left out of the first commit now group digits like the run and task instance selection counters do.
…s off the utils barrel The calendar backfill count and the Dag details limits were the last counters rendered outside translations without grouping, so the header read "0 of 1,000" while the details tab read "1000". system-components importing the src/utils barrel created an import cycle through errorHandling (which imports the toaster from system-components) and pulled DataTable into the lowest UI layer; importing formatNumber from its own module breaks the cycle.
shahar1
force-pushed
the
ui-group-digits
branch
from
October 3, 2026 06:09
99bb808 to
e55d768
Compare
shahar1
added
the
ready for maintainer review
bbovenzi
approved these changes
Oct 5, 2026
1 task done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR revives the idea I had in #56067 to apply digit grouping of counters according to the selected locale (but without extras this time 😅).
It does it by utilizing the native i18n functions.
Currently it applies the changes only to the English locale, once this PR is merged I'll come up with a second PR to apply it on the rest of the locales.
Screenshot (demonstrated on German, although it's not aprta part of this PR):
Details
formatNumbertakes the locale as an explicit argument (i18n.languagefromuseTranslation). The UI is compiled with the React Compiler, which memoizes a plain function call on its arguments only: a language read from the i18next singleton inside the function would keep the old grouping after a language switch. react-i18next hands out a newi18nwrapper on every change, so the explicit argument invalidates correctly (covered by theImportErrorCounttest).Intlfallback already used for durations (moved tosrc/utils/intlCache.ts), because plugins can contribute UI languages whose tagsIntlrejects (e.g.pt_BR). CallingtoLocaleString(i18n.language)with such a tag throws aRangeErrorand crashes the page; the two existing call sites that did this (DataTable,MetricSection) are migrated too.total_entries ?? 0 + Number(error)had an operator-precedence bug, andNumber(<Error>)isNaN.nullwhenmax_active_runsis unset.Tested: unit tests for
formatNumber(including the malformed-tag fallback), for the two behaviour fixes (countDagWarnings, Dag header withmax_active_runs: null), and a representative sample of the migrated call sites (StatsCard,ImportErrorCountwith regrouping on language switch,PoolBar,TaskInstanceTooltip,DagRunStateCounts), run under both an English and a German host locale; the other call sites are one-line mechanical swaps covered bytsc. eslint and prettier pass; the full UI vitest suite shows no new failures compared tomain.Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code (Opus 5.5, Fable 5.1) following the guidelines
🤖 Generated with Claude Code