From 30494d03ab33b7c87f220996e840203fa5cd4f5a Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 13 Aug 2026 12:14:23 +0000 Subject: [PATCH] fix(core): dashboard measures follow the display locale (#4566) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `formatMeasure` and `formatDimensionValue` formatted every value with a bare `undefined` locale tag at all three of their `Intl` sites. `undefined` is not "the user's locale", it is the MACHINE's — neither of the repo's two locale channels. A German session read a dashboard KPI as `1,234.5` next to a grid cell rendering the same number as `1.234,5`. Both take the display locale as a new optional LAST parameter; `DatasetWidget` threads `useDisplayLocale()` into every site it formats through (KPI, grouped table measure and dimension cells, cross-tab headers and cells). English output does not move — these sites already grouped through `Intl`, so the only change is whose locale is used. Omitting the argument reproduces the previous output byte for byte. Measured: routing through `formatDisplayNumber` is behaviourally LOSSLESS (0 diffs across 32,760 combinations) but blocked by the package boundary — `@object-ui/core` is React-free and consumed by React-free packages, while `@object-ui/i18n` peer-depends on React and exports no pure-utility subpath. The surviving duplication is recorded at both ends. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3 --- .../dataset-measure-display-locale-4566.md | 55 +++++ .../__tests__/dataset-format.locale.test.ts | 179 ++++++++++++++++ packages/core/src/utils/dataset-format.ts | 122 ++++++++++- packages/i18n/src/utils/number-display.ts | 14 ++ .../plugin-dashboard/src/DatasetWidget.tsx | 34 ++- .../DatasetWidget.measureLocale.test.tsx | 194 ++++++++++++++++++ 6 files changed, 582 insertions(+), 16 deletions(-) create mode 100644 .changeset/dataset-measure-display-locale-4566.md create mode 100644 packages/core/src/utils/__tests__/dataset-format.locale.test.ts create mode 100644 packages/plugin-dashboard/src/__tests__/DatasetWidget.measureLocale.test.tsx diff --git a/.changeset/dataset-measure-display-locale-4566.md b/.changeset/dataset-measure-display-locale-4566.md new file mode 100644 index 000000000..aa905481b --- /dev/null +++ b/.changeset/dataset-measure-display-locale-4566.md @@ -0,0 +1,55 @@ +--- +'@object-ui/core': minor +'@object-ui/plugin-dashboard': patch +--- + +Dashboard dataset measures follow the display locale (objectui#4566). + +`formatMeasure` and `formatDimensionValue` in `@object-ui/core` formatted every +value with a bare `undefined` locale tag at all three of their `Intl` sites. +`undefined` is not "the user's locale", it is the MACHINE's — neither of the +repo's two locale channels. A German session read a dashboard KPI as `1,234.5` +next to a grid cell rendering the same number as `1.234,5`, and inverted +separators read as a different number, not as an unstyled one. + +Both functions take the display locale as a new OPTIONAL LAST parameter, and +`DatasetWidget` threads `useDisplayLocale()` into every site it formats through: +the KPI, the grouped table's measure and dimension cells, and the cross-tab's +header labels and cells. + +**English output does not move**, and that is the discriminator against the +sibling fix. These sites already went through `Intl` with default grouping, so +the only thing that changes is WHOSE locale is used: + +| | before | after | +|---|---|---| +| en, 1234.5 `0.0` | `1,234.5` | `1,234.5` (unchanged) | +| de, 1234.5 `0.0` | `1,234.5` | `1.234,5` | +| de, 1234.5 EUR | `€1,234.50` | `1.234,50 €` | +| de, 0.6083 `0.0%` | `60.8%` | `60,8%` | + +Contrast objectui#4553, where `formatPercent` had never grouped at all and +moving en `1235%` → `1,235%` WAS the fix. + +Omitting the new argument reproduces the previous output byte for byte, so +callers that do not thread a locale yet are unaffected. + +Two behaviours are deliberately preserved rather than "improved" alongside the +locale fix, both measured: + +- **Integers stay verbatim.** The integer branch renders no separator and no + decimal mark, so a locale has nothing to change there — and routing it through + `Intl` WOULD change it (a locale with its own numbering system re-digits it, + and `1e21` expands to 22 digits). +- **The percent sign stays a literal suffix.** `Intl`'s `style: 'percent'` + re-scales by 100, and that round trip loses precision at the top of the range + (en `100,000,000,000,000,000,000,000%` becomes + `99,999,999,999,999,990,000,000%`). The consequence — a German list cell + writing `1.234,5 %` with a no-break space where a dashboard measure writes + `1.234,5%` — is filed separately rather than smuggled in behind a locale fix. + +`@object-ui/core` is `minor` because two of its ENTRY exports gained an optional +parameter (measured in the built `.d.ts`). `@object-ui/plugin-dashboard` is +`patch`: its published declarations are unchanged — `buildPivot`'s new optional +parameter is internal, as that function is not on the package's `exports` +surface. diff --git a/packages/core/src/utils/__tests__/dataset-format.locale.test.ts b/packages/core/src/utils/__tests__/dataset-format.locale.test.ts new file mode 100644 index 000000000..f877ae19b --- /dev/null +++ b/packages/core/src/utils/__tests__/dataset-format.locale.test.ts @@ -0,0 +1,179 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#4566 — `formatMeasure` / `formatDimensionValue` follow the DISPLAY + * locale instead of the machine's. + * + * All three formatting sites in `dataset-format.ts` passed a literal + * `undefined` locale tag to `Intl`, which is neither of the repo's two locale + * channels — it is whatever locale the machine happens to run in. Both + * functions are PURE (this package is React-free), so the tag arrives as a new + * optional LAST parameter and the callers thread `useDisplayLocale()`. + * + * ── Why every case pins TWO locales ────────────────────────────────────────── + * A single de assertion is not falsifiable on its own: on a German runner it + * would pass before the fix as well, because the machine locale would already + * be German. Each case therefore pins the same value in de AND in en. Before + * the fix the threaded tag is ignored, so BOTH render in the machine's locale + * and at least one of the two assertions must fail on ANY runner. That is the + * un-fakeable signal, and it is the same property the fix delivers: the machine + * locale stops being an input. + * + * ── Directions, predicted in writing BEFORE the run ────────────────────────── + * Runner machine locale measured as en-US. + * de plain / currency / percent / dimension RED (renders en pre-fix) + * fr plain (U+202F group separator) RED + * en counterpart of each of the above GREEN both sides — these are + * the byte-identity pins, and + * en must NOT move on this card + * (grouping already existed + * here; contrast objectui#4553, + * where moving en WAS the fix) + * zh coincidence case GREEN both sides — see its own + * note; NOT a red-first case + * malformed-tag rescue GREEN both sides — a guard, + * labelled as such, not a pin + * of the defect + * integer-verbatim cases GREEN both sides (must-not-change) + */ + +import { describe, it, expect } from 'vitest'; +import { formatMeasure, formatDimensionValue } from '../dataset-format'; + +/** German puts a NO-BREAK SPACE between amount and currency sign. */ +const NBSP = '\u00a0'; +/** French groups with a NARROW NO-BREAK SPACE (U+202F in current ICU). */ +const NNBSP = '\u202f'; + +describe('formatMeasure follows the display locale (objectui#4566)', () => { + it('formats a plain measure in the threaded locale, not the machine one', () => { + expect(formatMeasure(1234.5, '0.0', undefined, undefined, 'de-DE')).toBe('1.234,5'); + expect(formatMeasure(1234.5, '0.0', undefined, undefined, 'en-US')).toBe('1,234.5'); + expect(formatMeasure(1234.5, '0.0', undefined, undefined, 'fr-FR')).toBe(`1${NNBSP}234,5`); + }); + + it('formats a currency measure with the locale-correct symbol PLACEMENT', () => { + // Not merely the separators: German writes the sign last, English first. + expect(formatMeasure(1234.5, '0.00', 'EUR', undefined, 'de-DE')).toBe(`1.234,50${NBSP}€`); + expect(formatMeasure(1234.5, '0.00', 'EUR', undefined, 'en-US')).toBe('€1,234.50'); + }); + + it('formats a percent measure in the threaded locale', () => { + // The decimal COMMA is the whole point: `60.8%` and `60,8%` read as + // different numbers to the two audiences, not as the same one restyled. + expect(formatMeasure(0.608_333_333, '0.0%', undefined, undefined, 'de-DE')).toBe('60,8%'); + expect(formatMeasure(0.608_333_333, '0.0%', undefined, undefined, 'en-US')).toBe('60.8%'); + }); + + it('honours a declared percentScale in the threaded locale', () => { + expect(formatMeasure(1, '0.0%', undefined, 'fraction', 'de-DE')).toBe('100,0%'); + expect(formatMeasure(1, '0.0%', undefined, 'fraction', 'en-US')).toBe('100.0%'); + }); + + it('falls back to a plain LOCALIZED number when the currency code is unknown', () => { + // The fallthrough survives the locale threading: `formatNumberInLocale` + // retries without the LOCALE, and a bad currency throws out of that retry + // too, so the outer catch is still reached — and what it reaches must now + // itself be localized. + expect(formatMeasure(1234, '0,0', 'NOTACODE', undefined, 'de-DE')).toBe('1.234'); + expect(formatMeasure(1234, '0,0', 'NOTACODE', undefined, 'en-US')).toBe('1,234'); + }); + + it('localizes the legacy "$" literal form without moving the literal', () => { + expect(formatMeasure(1234.5, '$0.00', undefined, undefined, 'de-DE')).toBe('$1.234,50'); + expect(formatMeasure(1234.5, '$0.00', undefined, undefined, 'en-US')).toBe('$1,234.50'); + }); +}); + +describe('formatDimensionValue follows the display locale (objectui#4566)', () => { + it('formats a fractional dimension value in the threaded locale', () => { + expect(formatDimensionValue(1234.5, 'de-DE')).toBe('1.234,5'); + expect(formatDimensionValue(1234.5, 'en-US')).toBe('1,234.5'); + }); +}); + +describe('must-not-change: en output and the non-locale forms are byte-identical', () => { + /** + * The card's discriminator against objectui#4553. There `formatPercent` had + * never grouped, so en moved and the move WAS the fix. Here every site + * already went through `Intl` with default grouping, so en moving would mean + * the threading changed something it had no business changing. + */ + it('en output is unchanged at every site', () => { + expect(formatMeasure(1234, '0,0', undefined, undefined, 'en-US')).toBe('1,234'); + expect(formatMeasure(50, '0%', undefined, undefined, 'en-US')).toBe('50%'); + expect(formatMeasure(0.75, '0%', undefined, undefined, 'en-US')).toBe('75%'); + expect(formatMeasure(12.5, '0.0', undefined, undefined, 'en-US')).toBe('12.5'); + expect(formatMeasure(1000, '$0,0', undefined, undefined, 'en-US')).toBe('$1,000'); + expect(formatMeasure(0.6667, '0.0%', undefined, 'fraction', 'en-US')).toBe('66.7%'); + }); + + /** + * OMITTING the argument must reproduce the previous output byte for byte — + * the old code passed a literal `undefined` to `Intl`, which is exactly what + * an omitted optional parameter passes now. This is what makes the parameter + * safe to add ahead of the consumers that do not thread it yet. + */ + it('omitting the locale reproduces the machine-locale behaviour', () => { + expect(formatMeasure(1234, '0,0')).toBe(formatMeasure(1234, '0,0', undefined, undefined, undefined)); + expect(formatMeasure(null)).toBe('—'); + expect(formatMeasure('n/a')).toBe('n/a'); + expect(formatDimensionValue(null)).toBe('—'); + expect(formatDimensionValue('Backlog')).toBe('Backlog'); + }); + + /** + * The integer branch renders no separator and no decimal mark, so there is + * nothing for a locale to change — and it is deliberately NOT routed through + * `Intl`, which WOULD change it (a locale with its own numbering system would + * re-digit it). Green on both sides of the fix, in every locale. + */ + it('integers stay verbatim in every locale', () => { + for (const locale of ['en-US', 'de-DE', 'zh-CN', 'fr-FR', 'ar-EG']) { + expect(formatMeasure(1234, undefined, undefined, undefined, locale)).toBe('1234'); + expect(formatDimensionValue(42, locale)).toBe('42'); + } + }); +}); + +describe('locale-tag robustness (guards, not defect pins)', () => { + /** + * ⚠️ HONEST LABELLING — this case is GREEN on both sides of the fix, because + * before it the argument was ignored entirely and nothing could throw. It + * does not pin the #4566 defect; it pins a hazard the fix ITSELF introduces + * and must not regress. A threaded tag can be malformed (`en_US` with an + * underscore is the likeliest tenant-config typo), and a bare + * `Intl.NumberFormat('en_US', …)` throws `RangeError` — un-caught that takes + * the whole widget down, which would be a worse bug than the one being fixed. + */ + it('a malformed locale tag degrades to the runtime default instead of throwing', () => { + for (const bad of ['en_US', '!!', 'e', 'de-DE-u-nu-']) { + expect(() => formatMeasure(1234.5, '0.0', undefined, undefined, bad)).not.toThrow(); + expect(formatMeasure(1234.5, '0.0', undefined, undefined, bad)).toBe( + formatMeasure(1234.5, '0.0'), + ); + expect(() => formatDimensionValue(1234.5, bad)).not.toThrow(); + } + }); + + /** + * ⚠️ HONEST LABELLING — a COINCIDENCE case, not a red-first one. zh-CN's + * number conventions are byte-identical to en-US's (measured: group `,`, + * decimal `.`), so a Chinese session cannot produce the inverted-separator + * signal the German one does and this case is green before AND after the fix. + * It is kept because it records WHY zh is absent from the red-first set — a + * later reader must not add a "zh renders differently" expectation and then + * conclude the fix is broken when it does not. + */ + it('zh-CN coincides with en-US and is therefore not a discriminator', () => { + expect(formatMeasure(1234.5, '0.0', undefined, undefined, 'zh-CN')).toBe( + formatMeasure(1234.5, '0.0', undefined, undefined, 'en-US'), + ); + }); +}); diff --git a/packages/core/src/utils/dataset-format.ts b/packages/core/src/utils/dataset-format.ts index 3ab02d56d..c53ff167b 100644 --- a/packages/core/src/utils/dataset-format.ts +++ b/packages/core/src/utils/dataset-format.ts @@ -79,6 +79,40 @@ export function percentDisplayValue(value: number): number { return value > -1 && value < 1 ? value * 100 : value; } +/** + * Format one number in the DISPLAY locale, surviving a malformed locale tag. + * + * ⚠️ SURVIVING DUPLICATION, recorded deliberately (objectui#4566). This mirrors + * the `try`/`catch` retry inside `formatDisplayNumber` + * (`@object-ui/i18n`'s `utils/number-display.ts`) — the ONE number-display + * formatter — and the mirror exists only because of a PACKAGE BOUNDARY, not + * because the behaviour differs. See {@link formatMeasure}'s note for the + * measurement that chose this over routing through it. Keep the two in step: + * a change to the retry policy there belongs here too. + * + * The retry is load-bearing rather than defensive. Before #4566 these sites + * passed a literal `undefined`, which never throws; a THREADED tag can be + * malformed, and a bare `Intl.NumberFormat('en_US', …)` throws `RangeError` + * (measured — underscore instead of hyphen is the likeliest tenant-config + * typo). Un-caught that would take the whole widget down, so a bad tag falls + * back to the runtime default exactly as `formatDisplayNumber` does. A bad + * `currency` still throws out of both attempts, which is what lets + * {@link formatMeasure}'s own `catch` fall through to plain-number formatting. + */ +function formatNumberInLocale( + value: number, + locale: string | undefined, + options: Intl.NumberFormatOptions, +): string { + try { + return new Intl.NumberFormat(locale, options).format(value); + } catch { + // Retry WITHOUT the locale, keeping every other option: rescues a malformed + // tag while still surfacing a genuinely bad `currency` to the caller. + return new Intl.NumberFormat(undefined, options).format(value); + } +} + /** * Format a MEASURE value. Currency comes from the field's declared `currency` * (locale-correct symbol via `Intl`), NOT from a "$" baked into the format @@ -86,8 +120,53 @@ export function percentDisplayValue(value: number): number { * never a misleading "$". The numeral `format` hint (e.g. "0,0", "0.0%") * controls grouping / decimals / percent; it can't be baked into the row value * server-side (the same number feeds charts), so it is applied here. + * + * `locale` is the BCP-47 tag of the active display locale — in React, whatever + * `useDisplayLocale()` returns. It is optional and LAST so every existing call + * keeps compiling, and omitting it reproduces the previous output byte for + * byte (the old code passed a literal `undefined` to `Intl`, which is exactly + * what an omitted argument passes now). + * + * Before objectui#4566 there was no way to pass one: all three formatting sites + * here hard-coded `undefined`, which is neither of the repo's two locale + * channels — it is the MACHINE's locale. A German session read a KPI as + * `1,234.5` beside a grid cell rendering the same number as `1.234,5`, and + * inverted separators read as a different number, not an unstyled one. + * + * ── Why this still formats itself instead of calling `formatDisplayNumber` ── + * The one-resolver rule says this should route through `formatDisplayNumber` + * (`@object-ui/i18n`) and retire the parallel implementation. That was measured + * across 32,760 value × format × currency × percentScale × locale combinations + * and the option mapping is LOSSLESS — routing through it changes literally no + * byte of output, because the policy layer `formatDisplayNumber` adds over + * plain `Intl` is grouping suppression keyed on a field's declared `scale`, and + * a measure has no `scale` to feed it (decimals here come from a numeral format + * PATTERN, so grouping stays on and the two agree everywhere). + * + * What blocks the routing is the PACKAGE BOUNDARY, not the behaviour. + * `@object-ui/core` is the React-free engine (see this module's header, and the + * topology table in AGENTS.md §3: "No UI-lib deps. Logic only."), while + * `@object-ui/i18n` depends on `i18next`/`react-i18next` and peer-depends on + * React, and publishes no pure-utility subpath to import in isolation — its + * `exports` map is `.` and `./locales/*`. A `core` → `i18n` edge would put + * React into the dependency closure of every React-FREE consumer of this + * package: the `object-ui` VS Code extension and `@object-ui/data-objectstack` + * both take `@object-ui/core` as a runtime dependency and declare no React. + * + * So the duplication survives on purpose and is recorded at BOTH ends (see + * `formatNumberInLocale` above and the note in `number-display.ts`). Retiring + * it for real means moving `formatDisplayNumber` DOWN into this package and + * re-exporting it from `@object-ui/i18n` — the right direction, since `core` is + * the lower layer — but that relocates a published export across a package + * boundary and is deliberately left to its own card. */ -export function formatMeasure(v: unknown, format?: string, currency?: string, percentScale?: PercentScale): string { +export function formatMeasure( + v: unknown, + format?: string, + currency?: string, + percentScale?: PercentScale, + locale?: string, +): string { if (v == null) return '—'; if (typeof v !== 'number') return String(v); @@ -95,20 +174,31 @@ export function formatMeasure(v: unknown, format?: string, currency?: string, pe if (currency) { try { - return new Intl.NumberFormat(undefined, { + return formatNumberInLocale(v, locale, { style: 'currency', currency, minimumFractionDigits: decimals ?? 0, maximumFractionDigits: decimals ?? 2, - }).format(v); + }); } catch { // Unknown currency code → fall through to plain number formatting. + // Still reachable with a locale threaded: `formatNumberInLocale` retries + // without the LOCALE, and a bad currency throws out of that retry too. } } if (!format) { // No format hint → preserve the plain rendering (integers verbatim). - return Number.isInteger(v) ? String(v) : v.toLocaleString(undefined, { maximumFractionDigits: 2 }); + // + // The integer branch stays a bare `String(v)` and is deliberately NOT + // localized: it is the one form here that renders no separator and no + // decimal mark, so there is nothing for a locale to change — and routing it + // through `Intl` WOULD change it, in two ways this card is not about + // (measured): a locale with its own numbering system would re-digit it + // (`ar-EG` 1234 → an Arabic-Indic spelling), and `1e21` would expand from + // `1e+21` to its 22 digits. Only the fractional branch below ever produced + // locale-dependent text, and that is the site #4566 fixes. + return Number.isInteger(v) ? String(v) : formatNumberInLocale(v, locale, { maximumFractionDigits: 2 }); } const isPercent = format.includes('%'); // A legacy "$" literal in the format string is still honored (explicit author @@ -127,18 +217,36 @@ export function formatMeasure(v: unknown, format?: string, currency?: string, pe const display = isPercent ? (percentScale ? (percentScale === 'fraction' ? v * 100 : v) : percentDisplayValue(v)) : v; - const body = display.toLocaleString(undefined, { minimumFractionDigits: decimals ?? 0, maximumFractionDigits: decimals ?? 0 }); + const body = formatNumberInLocale(display, locale, { + minimumFractionDigits: decimals ?? 0, + maximumFractionDigits: decimals ?? 0, + }); + // The '%' stays a LITERAL suffix rather than `Intl`'s `style: 'percent'`, + // and that is a measured choice, not an oversight (objectui#4566). The + // percent STYLE would re-scale by 100, and the round trip loses precision at + // the top of the range: en `100,000,000,000,000,000,000,000%` becomes + // `99,999,999,999,999,990,000,000%`. English output moving is the signal that + // a mapping is wrong on this card — unlike `formatPercent`'s (objectui#4553), + // where en had never grouped and moving it WAS the fix. The consequence is + // that a German session reads `1.234,5%` here and `1.234,5 %` (no-break space, + // the German percent convention) from a list cell; that divergence is real, + // is narrower than the one this card closes, and is filed separately rather + // than smuggled in behind a locale fix. return `${legacyDollar}${body}${isPercent ? '%' : ''}`; } /** * Format a non-measure (dimension / label) value — the server already resolves * dimension display labels, so this only tidies numbers and nulls. + * + * `locale` follows {@link formatMeasure}: optional, last, and omitting it + * reproduces the previous machine-locale output byte for byte. Integers stay + * verbatim here for the same measured reason as there. */ -export function formatDimensionValue(v: unknown): string { +export function formatDimensionValue(v: unknown, locale?: string): string { if (v == null) return '—'; if (typeof v === 'number') { - return Number.isInteger(v) ? String(v) : v.toLocaleString(undefined, { maximumFractionDigits: 2 }); + return Number.isInteger(v) ? String(v) : formatNumberInLocale(v, locale, { maximumFractionDigits: 2 }); } return String(v); } diff --git a/packages/i18n/src/utils/number-display.ts b/packages/i18n/src/utils/number-display.ts index 94b94e000..8d37db156 100644 --- a/packages/i18n/src/utils/number-display.ts +++ b/packages/i18n/src/utils/number-display.ts @@ -26,6 +26,20 @@ * Both policies now live here and nowhere else. Call sites bring the value and * the display width; they do not bring a locale default and they do not decide * grouping. + * + * ⚠️ ONE KNOWN EXCEPTION, recorded at both ends (objectui#4566). + * `formatMeasure` / `formatDimensionValue` in `@object-ui/core`'s + * `utils/dataset-format.ts` still build their own `Intl.NumberFormat`, and they + * mirror the malformed-locale retry below in a local `formatNumberInLocale`. + * That is NOT drift left unnoticed: the option mapping was measured lossless + * across 32,760 combinations (routing them through this function changes no + * byte of output, because a measure carries no field `scale` and so never + * reaches the grouping policy). What blocks the routing is the package + * boundary — `@object-ui/core` is React-free and consumed by React-free + * packages, while this one depends on `i18next`/`react-i18next` and peer-depends + * on React, and exports no pure-utility subpath. Retiring the duplicate means + * moving THIS module down into `@object-ui/core` and re-exporting it from here. + * Until then, a change to the retry policy below belongs in that mirror too. */ export interface DisplayNumberFormatOptions { diff --git a/packages/plugin-dashboard/src/DatasetWidget.tsx b/packages/plugin-dashboard/src/DatasetWidget.tsx index aadeb8e31..1712bd0c1 100644 --- a/packages/plugin-dashboard/src/DatasetWidget.tsx +++ b/packages/plugin-dashboard/src/DatasetWidget.tsx @@ -64,7 +64,7 @@ import { type DatasetDrillRange, } from '@object-ui/core'; import { cn, Skeleton, ChartSkeleton, GridSkeleton } from '@object-ui/components'; -import { useSafeFieldLabel, useSafeTranslate } from '@object-ui/i18n'; +import { useSafeFieldLabel, useSafeTranslate, useDisplayLocale } from '@object-ui/i18n'; import { BarChart3, AlertTriangle, Download, ArrowUpIcon, ArrowDownIcon, MinusIcon } from 'lucide-react'; import { useFilterScope } from '@object-ui/react'; import { resolveFilterPlaceholders, computeMetricDelta } from './utils'; @@ -111,11 +111,19 @@ export { pivotRowId, pivotCellKey }; * holding that combination's measure values. No re-aggregation — the dataset * already grouped by every dimension, so each cell maps to exactly one row (the * index is also what drill-through uses to read `drillRawRows`). + * + * `locale` is the display-locale tag the header LABELS are formatted in + * (objectui#4566) — this is a plain exported function, not a component, so it + * cannot read `useDisplayLocale()` itself. It affects `rowHeaders`/`colHeaders` + * only; the `id`s are opaque keys built by `pivotBucketId` from the RAW values + * and are deliberately untouched by it, so a locale change can never re-key a + * cell (which would break the `cellIndex` lookup and drill-through with it). */ export function buildPivot( rows: Array>, rowDims: string[], colDim: string, + locale?: string, ): { rowHeaders: Array<{ id: string; labels: string[] }>; colHeaders: Array<{ id: string; label: string }>; @@ -137,8 +145,8 @@ export function buildPivot( // one-element tuple costs nothing and keeps one encoding for one kind of id. const rid = pivotBucketId(rowDims.map((d) => pivotDimensionValue(row[d]))); const cid = pivotBucketId([pivotDimensionValue(row[colDim])]); - if (!rowSeen.has(rid)) { rowSeen.add(rid); rowHeaders.push({ id: rid, labels: rowDims.map((d) => formatDimensionValue(row[d])) }); } - if (!colSeen.has(cid)) { colSeen.add(cid); colHeaders.push({ id: cid, label: formatDimensionValue(row[colDim]) }); } + if (!rowSeen.has(rid)) { rowSeen.add(rid); rowHeaders.push({ id: rid, labels: rowDims.map((d) => formatDimensionValue(row[d], locale)) }); } + if (!colSeen.has(cid)) { colSeen.add(cid); colHeaders.push({ id: cid, label: formatDimensionValue(row[colDim], locale) }); } cellIndex.set(pivotCellKey(rid, cid), index); }); return { rowHeaders, colHeaders, cellIndex }; @@ -697,6 +705,14 @@ export function DatasetWidget({ widget, dataSource }: { widget: any; dataSource: const tt = useSafeTranslate(); const { fieldLabel, fieldOptionLabel } = useSafeFieldLabel(); + // The display locale every measure and dimension value below is formatted in + // (objectui#4566). `formatMeasure` / `formatDimensionValue` are pure + // functions in `@object-ui/core`, so the tag has to arrive as an argument — + // this is the one place in this component allowed to read it. Nothing here + // formats inside a `useMemo`, so unlike objectui#4542 / #4553 there is no + // dependency array to add it to; the sites below are all in the render body + // and re-run whenever the provider changes. + const displayLocale = useDisplayLocale(); // ADR-0021 dual-form: the widget's presentation-scope `filter` must flow into // the dataset query as `runtimeFilter`, or a dataset-bound widget renders the @@ -1006,7 +1022,7 @@ export function DatasetWidget({ widget, dataSource }: { widget: any; dataSource: const accentClass = metricAccentTextClass(widget?.colorVariant); return (
- {formatMeasure(value, f?.format, f?.currency, f?.percentScale)} + {formatMeasure(value, f?.format, f?.currency, f?.percentScale, displayLocale)} {delta && (
values.map((m) => ({ col, measure: m, header: values.length === 1 ? col.label : `${col.label} · ${headerLabel(m)}` })), ); - const fmtMeasure = (v: unknown, m: string) => formatMeasure(v, measureField(m)?.format, measureField(m)?.currency, measureField(m)?.percentScale); + const fmtMeasure = (v: unknown, m: string) => formatMeasure(v, measureField(m)?.format, measureField(m)?.currency, measureField(m)?.percentScale, displayLocale); // ── The comparison, stacked inside the cell (objectui#3614) ─────────── // A cross-tab's columns are already `bucket × measure`; giving the // comparison a column of its own would make them `bucket × measure × @@ -1313,7 +1329,7 @@ export function DatasetWidget({ widget, dataSource }: { widget: any; dataSource: key={i} className={cn('border-t', canDrill && 'cursor-pointer hover:bg-accent/40')} data-testid={canDrill ? 'dataset-drill-row' : undefined} - onClick={canDrill ? () => openDrill(i, drillDims.map((d) => formatDimensionValue(row[d])).filter(Boolean).join(' / ')) : undefined} + onClick={canDrill ? () => openDrill(i, drillDims.map((d) => formatDimensionValue(row[d], displayLocale)).filter(Boolean).join(' / ')) : undefined} > {columns.map((c) => { // A comparison column formats as the measure it compares. @@ -1321,8 +1337,8 @@ export function DatasetWidget({ widget, dataSource }: { widget: any; dataSource: return ( {measure - ? formatMeasure(row[c], measureField(measure)?.format, measureField(measure)?.currency, measureField(measure)?.percentScale) - : formatDimensionValue(row[c])} + ? formatMeasure(row[c], measureField(measure)?.format, measureField(measure)?.currency, measureField(measure)?.percentScale, displayLocale) + : formatDimensionValue(row[c], displayLocale)} ); })} diff --git a/packages/plugin-dashboard/src/__tests__/DatasetWidget.measureLocale.test.tsx b/packages/plugin-dashboard/src/__tests__/DatasetWidget.measureLocale.test.tsx new file mode 100644 index 000000000..37b55fb0b --- /dev/null +++ b/packages/plugin-dashboard/src/__tests__/DatasetWidget.measureLocale.test.tsx @@ -0,0 +1,194 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#4566 — the dashboard's dataset-bound measures follow the display + * locale. + * + * `formatMeasure` / `formatDimensionValue` live in `@object-ui/core`, which is + * React-free, so they cannot read `useDisplayLocale()` themselves: the tag + * arrives as their new optional last parameter and THIS component is what + * threads it. The parameter lands with its consumers (the objectui#4272 + * playbook), so every site `DatasetWidget` formats through is exercised here — + * the KPI, the flat grouped table's measure AND dimension cells, and the + * cross-tab's header labels and cells. + * + * ── Directions, predicted in writing BEFORE the run ────────────────────────── + * Runner machine locale measured as en-US. + * every de case RED pre-fix — renders the en form + * every en counterpart GREEN both sides — the byte-identity pins. + * en must NOT move on this card: these sites + * already grouped via `Intl`, so the only thing + * the fix changes is WHOSE locale is used. + * (Contrast objectui#4553, where `formatPercent` + * had never grouped and moving en WAS the fix.) + * `buildPivot` unit case RED pre-fix on the de half + * + * ⚠️ NOT asserted here, deliberately: a `useMemo` dependency. Unlike + * objectui#4542 / #4553 none of this component's formatting happens inside a + * memo — every site is in the render body — so there is no dependency array to + * add the locale to and no dep-isolation case to write. Recorded so a later + * reader does not go looking for the case that is missing. + * + * ⚠️ The objectui#4487 flake lives in the sibling `DatasetWidget.test.tsx`. + * This file mounts the same component, so a red here is verified locally and + * re-run before being owned. + */ + +import React from 'react'; +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, screen, cleanup, waitFor } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { I18nProvider, LocalizationProvider, type LocalizationValue } from '@object-ui/i18n'; +import { DatasetWidget, buildPivot } from '../DatasetWidget'; + +afterEach(cleanup); + +/** German writes a NO-BREAK SPACE between the amount and the currency sign. */ +const NBSP = '\u00a0'; + +type Row = Record; + +const makeSource = (rows: Row[], fields?: Row[]) => ({ + queryDataset: vi.fn(async () => ({ rows, ...(fields ? { fields } : {}) })), +}); + +/** + * `locale` on the LocalizationProvider is the tenant's resolved regional + * default — channel 1 of `useDisplayLocale`, which outranks the UI language. + */ +function renderIn(locale: string | undefined, widget: Row, dataSource: unknown, language = 'en') { + const value: LocalizationValue = locale ? { locale } : {}; + return render( + + + + + , + ); +} + +const REVENUE_FIELDS = [{ name: 'revenue', type: 'number', label: 'Revenue', format: '0.0' }]; +const CURRENCY_FIELDS = [{ name: 'revenue', type: 'number', label: 'Revenue', format: '0.00', currency: 'EUR' }]; +const METRIC_WIDGET = { type: 'metric', dataset: 'sales', values: ['revenue'] }; + +/** + * Testing Library's DEFAULT normalizer collapses whitespace runs to a plain + * space, and U+00A0 is whitespace — so the German currency form `1.234,50 €` + * would be compared as if it were written with an ordinary space and the + * no-break space could never be asserted. Trimming only keeps the byte the + * locale actually produces, which is the whole point of the case. + */ +const KEEP_NBSP = { normalizer: (text: string) => text.trim() }; + +describe('DatasetWidget KPI follows the display locale (objectui#4566)', () => { + it('renders the German form in a de session', async () => { + renderIn('de-DE', METRIC_WIDGET, makeSource([{ revenue: 1234.5 }], REVENUE_FIELDS)); + expect(await screen.findByText('1.234,5')).toBeInTheDocument(); + }); + + it('renders the English form unchanged in an en session (must-not-change)', async () => { + renderIn('en-US', METRIC_WIDGET, makeSource([{ revenue: 1234.5 }], REVENUE_FIELDS)); + expect(await screen.findByText('1,234.5')).toBeInTheDocument(); + }); + + it('follows the UI LANGUAGE when the tenant states no regional preference', async () => { + // Channel 2 of `useDisplayLocale` — proves the widget reads the composed + // hook rather than one provider. + renderIn(undefined, METRIC_WIDGET, makeSource([{ revenue: 1234.5 }], REVENUE_FIELDS), 'de'); + expect(await screen.findByText('1.234,5')).toBeInTheDocument(); + }); + + it('places the currency sign the way the locale does, not the way en does', async () => { + renderIn('de-DE', METRIC_WIDGET, makeSource([{ revenue: 1234.5 }], CURRENCY_FIELDS)); + // German writes the sign LAST, separated by a no-break space. + expect(await screen.findByText(`1.234,50${NBSP}€`, KEEP_NBSP)).toBeInTheDocument(); + }); + + it('keeps the en currency form byte-identical (must-not-change)', async () => { + renderIn('en-US', METRIC_WIDGET, makeSource([{ revenue: 1234.5 }], CURRENCY_FIELDS)); + // English writes it FIRST, with no space at all. + expect(await screen.findByText('€1,234.50', KEEP_NBSP)).toBeInTheDocument(); + }); +}); + +describe('DatasetWidget grouped table follows the display locale (objectui#4566)', () => { + const TABLE_WIDGET = { type: 'table', dataset: 'sales', dimensions: ['bucket'], values: ['revenue'] }; + const FIELDS = [ + { name: 'bucket', type: 'number', label: 'Bucket' }, + { name: 'revenue', type: 'number', label: 'Revenue', format: '0.0' }, + ]; + // A fractional DIMENSION value, so `formatDimensionValue` is exercised too — + // it has its own formatting site and its own new parameter. + const ROWS = [{ bucket: 9876.5, revenue: 1234.5 }]; + + it('localizes both the measure cell and the dimension cell in a de session', async () => { + renderIn('de-DE', TABLE_WIDGET, makeSource(ROWS, FIELDS)); + await waitFor(() => expect(screen.getByText('1.234,5')).toBeInTheDocument()); + expect(screen.getByText('9.876,5')).toBeInTheDocument(); + }); + + it('leaves the en session byte-identical (must-not-change)', async () => { + renderIn('en-US', TABLE_WIDGET, makeSource(ROWS, FIELDS)); + await waitFor(() => expect(screen.getByText('1,234.5')).toBeInTheDocument()); + expect(screen.getByText('9,876.5')).toBeInTheDocument(); + }); +}); + +describe('DatasetWidget cross-tab follows the display locale (objectui#4566)', () => { + // Two dimensions ⇒ `isMatrix`, which is the path through `buildPivot`. + const PIVOT_WIDGET = { type: 'pivot', dataset: 'sales', dimensions: ['region', 'bucket'], values: ['revenue'] }; + const FIELDS = [ + { name: 'region', type: 'string', label: 'Region' }, + { name: 'bucket', type: 'number', label: 'Bucket' }, + { name: 'revenue', type: 'number', label: 'Revenue', format: '0.0' }, + ]; + const ROWS = [{ region: 'EMEA', bucket: 9876.5, revenue: 1234.5 }]; + + it('localizes the across-axis header label and the cell in a de session', async () => { + renderIn('de-DE', PIVOT_WIDGET, makeSource(ROWS, FIELDS)); + // The column header comes from `buildPivot`'s `colHeaders[].label`. + await waitFor(() => expect(screen.getByText('9.876,5')).toBeInTheDocument()); + expect(screen.getByText('1.234,5')).toBeInTheDocument(); + }); + + it('leaves the en session byte-identical (must-not-change)', async () => { + renderIn('en-US', PIVOT_WIDGET, makeSource(ROWS, FIELDS)); + await waitFor(() => expect(screen.getByText('9,876.5')).toBeInTheDocument()); + expect(screen.getByText('1,234.5')).toBeInTheDocument(); + }); +}); + +describe('buildPivot localizes header labels but never the bucket IDs (objectui#4566)', () => { + const ROWS = [ + { region: 'EMEA', bucket: 9876.5, revenue: 1 }, + { region: 'AMER', bucket: 1234.5, revenue: 2 }, + ]; + + it('formats the header labels in the supplied locale', () => { + const de = buildPivot(ROWS, ['region'], 'bucket', 'de-DE'); + expect(de.colHeaders.map((c) => c.label)).toEqual(['9.876,5', '1.234,5']); + + const en = buildPivot(ROWS, ['region'], 'bucket', 'en-US'); + expect(en.colHeaders.map((c) => c.label)).toEqual(['9,876.5', '1,234.5']); + }); + + /** + * The ids are opaque keys built from the RAW values, so a locale change must + * not re-key a cell — that would break `cellIndex` and drill-through with it. + * Green on both sides of the fix; it pins a property the fix must not break. + */ + it('keys cells identically in every locale', () => { + const de = buildPivot(ROWS, ['region'], 'bucket', 'de-DE'); + const en = buildPivot(ROWS, ['region'], 'bucket', 'en-US'); + expect(de.colHeaders.map((c) => c.id)).toEqual(en.colHeaders.map((c) => c.id)); + expect(de.rowHeaders.map((r) => r.id)).toEqual(en.rowHeaders.map((r) => r.id)); + expect([...de.cellIndex.entries()]).toEqual([...en.cellIndex.entries()]); + }); + + /** Omitting the argument reproduces the previous machine-locale behaviour. */ + it('omitting the locale is byte-identical to the previous signature', () => { + const omitted = buildPivot(ROWS, ['region'], 'bucket'); + const explicitUndefined = buildPivot(ROWS, ['region'], 'bucket', undefined); + expect(omitted.colHeaders).toEqual(explicitUndefined.colHeaders); + }); +});