fix(fields): formatPercent groups and follows the display locale — the last tooltip channel (#4553)

.changeset/percent-formatter-locale-4553.md

@@ -0,0 +1,64 @@
+---
+'@object-ui/fields': minor
+'@object-ui/plugin-dashboard': minor
+'@object-ui/plugin-gantt': patch
+'@object-ui/plugin-grid': patch
+---
+
+`formatPercent` groups its output and follows the display locale — the last
+tooltip/cell channel (objectui#4553).
+
+PR #4557 threaded the gantt tooltip's number and currency rows and measured that
+the percent row could not follow: `formatPercent(value, precision)` took no
+locale parameter, and its whole body was
+`${percentDisplayValue(value).toFixed(precision)}%`. It built no
+`Intl.NumberFormat` and never reached `formatDisplayNumber` — so unlike its
+siblings it did not render in the MACHINE's locale, it rendered in **no** locale:
+an ASCII decimal mark, never a grouping separator, byte-identical on every
+machine.
+
+**English output MOVES, and that is the fix.** Because the function never
+grouped, `1235%` was wrong in en-US too, not only in German. Grouping and locale
+therefore land together:
+
+| | before | after |
+|---|---|---|
+| en, 1234.5 | `1235%` | `1,235%` |
+| de, 1234.5 | `1235%` | `1.235\u00a0%` |
+| de, 80 | `80%` | `80\u00a0%` |
+
+Values below the grouping threshold are unchanged in English (`80%`, `12.5%`,
+`33.33%`), so the move is confined to four digits and up. German changes at every
+magnitude, because the no-break space before the sign is part of the locale's
+percent convention — which is what routing through `Intl` buys over appending a
+literal `%`.
+
+The scaling contract is untouched: `percentDisplayValue` still disambiguates a
+fraction-stored percent (`0.8` → 80%) from a whole one, so the list cell and the
+dashboard measure formatter still agree.
+
+Consumers are threaded in the same change, the parameter never landing
+speculatively:
+
+- **fields** — `PercentCellRenderer`, on BOTH of its paths. Its whole-percent
+  branch (`progress` / `completion` fields, which store 0-100 and must skip the
+  fraction scaling) was a second bare `toFixed` call; leaving it behind would
+  have made one grid internally inconsistent, so both branches now share one
+  locale-aware body and differ only in the scaling policy.
+- **plugin-gantt** — the tooltip percent row, completing objectui#4553's switch.
+- **plugin-grid** — the mobile card's percent cell, which sits in the same
+  density row as a date cell objectui#4272 had already localized.
+- **plugin-dashboard** — `renderFieldValue`'s percent branch. It is a plain
+  function rather than a component, so it takes the locale as an optional fourth
+  parameter beside the `tenantCurrency` already threaded that way, and both of
+  its callers pass it and declare it in their memo dependency arrays.
+
+Bumps follow each package's own `.d.ts` diff, measured in both directions.
+`@object-ui/fields` and `@object-ui/plugin-dashboard` are `minor` on the
+objectui#4272 / PR #4544 precedent — quoted from that changeset: "`@object-ui/fields`
+is `minor` because `formatDateTime`'s new optional parameter is visible in the
+package's entry `.d.ts`; the plugin packages' own `.d.ts` files are
+byte-identical, so their change is module-local." Here `formatPercent` and
+`renderFieldValue` each gain an entry-visible optional parameter, while
+plugin-gantt's and plugin-grid's `.d.ts` files are byte-identical and stay
+`patch`.

packages/fields/src/__tests__/PercentCellRenderer.locale.test.tsx

@@ -0,0 +1,152 @@
+/**
+ * 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#4553 phase 2 — the percent CELL, the fields-internal consumer of
+ * `formatPercent`.
+ *
+ * `PercentCellRenderer` had two rendering paths and BOTH were outside the
+ * display-locale channel:
+ *
+ *     const formatted = isWholePercentField
+ *       ? `${numValue.toFixed(precision)}%`   // a second bare toFixed path
+ *       : formatPercent(numValue, precision); // no locale to pass
+ *
+ * The branch exists for a real reason — a field named `progress` / `completion`
+ * stores 0-100, so it must NOT go through `percentDisplayValue`'s fraction
+ * scaling — but it had quietly become a second place where a percent was
+ * formatted, and it was the more primitive of the two. Threading only the
+ * `formatPercent` half would have made ONE grid internally inconsistent: a
+ * `progress` column ungrouped and unlocalized beside a `rate` column that was
+ * neither. So both branches now render through the same locale-aware body and
+ * differ only in the scaling policy, which is all the branch was ever about.
+ *
+ * ── Directions, predicted in writing BEFORE the run ──────────────────────
+ * Runner machine locale en-US.
+ *
+ *   de, ordinary percent 1234.5   `1235%` → `1.235 %`   RED
+ *   de, `progress` 1234.5         `1235%` → `1.235 %`   RED (the second path)
+ *   en, ordinary percent 1234.5   `1235%` → `1,235%`    RED (grouping move)
+ *   en, `progress` 1234.5         `1235%` → `1,235%`    RED (grouping move)
+ *   the fraction/whole SPLIT itself                     PIN, green both sides
+ *   small-value en output                               PIN, green both sides
+ *
+ * Provider-mounted by construction (objectui#4514) — the pure-function cases
+ * live in `percent-formatter-locale-4553.test.ts`.
+ */
+
+import React from 'react';
+import { describe, it, expect, afterEach } from 'vitest';
+import { render, screen, cleanup } from '@testing-library/react';
+import '@testing-library/jest-dom';
+import { I18nProvider, LocalizationProvider } from '@object-ui/i18n';
+import { PercentCellRenderer } from '../index';
+
+/** German writes a NO-BREAK SPACE (U+00A0) before the percent sign. */
+const NBSP = '\u00a0';
+
+/**
+ * A session: the UI language the user picked plus the tenant's regional
+ * default. `useDisplayLocale()` resolves tenant regional default → active UI
+ * language → 'en'.
+ */
+function renderCell(
+  value: unknown,
+  field: Record<string, unknown>,
+  language: string,
+  tenantLocale?: string,
+) {
+  return render(
+    <I18nProvider
+      config={{ defaultLanguage: language, detectBrowserLanguage: false }}
+      persistLanguage={false}
+    >
+      <LocalizationProvider value={{ locale: tenantLocale }}>
+        <PercentCellRenderer
+          value={value as any}
+          field={{ type: 'percent', ...field } as any}
+        />
+      </LocalizationProvider>
+    </I18nProvider>,
+  );
+}
+
+/** The cell's text, read off the value span beside the decorative bar. */
+function cellText(): string {
+  return document.body.textContent ?? '';
+}
+
+afterEach(() => cleanup());
+
+describe('PercentCellRenderer follows the display locale (objectui#4553)', () => {
+  it('de renders the German percent form', () => {
+    renderCell(1234.5, { name: 'win_rate' }, 'de');
+    expect(cellText()).toContain(`1.235${NBSP}%`);
+  });
+
+  /**
+   * The SECOND path — `progress` matches WHOLE_PERCENT_FIELD_PATTERN, so this
+   * value skips fraction scaling and used to skip `formatPercent` entirely.
+   */
+  it('de renders the German form on the whole-percent path too', () => {
+    renderCell(1234.5, { name: 'progress' }, 'de');
+    expect(cellText()).toContain(`1.235${NBSP}%`);
+  });
+
+  it('en groups at four digits on both paths — the deliberate output move', () => {
+    const { unmount } = renderCell(1234.5, { name: 'win_rate' }, 'en');
+    expect(cellText()).toContain('1,235%');
+    unmount();
+
+    renderCell(1234.5, { name: 'progress' }, 'en');
+    expect(cellText()).toContain('1,235%');
+  });
+
+  it('an explicit tenant locale outranks the active UI language', () => {
+    renderCell(1234.5, { name: 'win_rate' }, 'en', 'de');
+    expect(cellText()).toContain(`1.235${NBSP}%`);
+    expect(cellText()).not.toContain('1,235%');
+  });
+});
+
+describe('PercentCellRenderer keeps its scaling contract (objectui#4553 must-not-change)', () => {
+  /**
+   * PIN, green both sides — and the reason the whole-percent branch was kept
+   * rather than collapsed into `formatPercent`. The same stored number means
+   * different things in the two columns, and that must not have changed.
+   */
+  it('an ordinary percent scales a fraction; a progress field does not', () => {
+    const { unmount } = renderCell(0.5, { name: 'win_rate' }, 'en');
+    // Fraction-stored: 0.5 → 50%.
+    expect(cellText()).toContain('50%');
+    unmount();
+
+    renderCell(0.5, { name: 'progress' }, 'en');
+    // Whole-percent field: 0.5 really is half a percent, rounded to 1% at
+    // precision 0 — NOT 50%.
+    expect(cellText()).toContain('1%');
+    expect(cellText()).not.toContain('50%');
+  });
+
+  /** PIN: small-value English output is byte-identical across the change. */
+  it('en small-value output is unchanged (must-not-change)', () => {
+    renderCell(33.33, { name: 'win_rate', precision: 2 }, 'en');
+    expect(cellText()).toContain('33.33%');
+  });
+
+  /** PIN: the decorative bar still reports the un-formatted magnitude. */
+  it('the progressbar still carries the numeric value', () => {
+    renderCell(33.33, { name: 'win_rate' }, 'en');
+    expect(screen.getByRole('progressbar')).toHaveAttribute('aria-valuenow', '33.33');
+  });
+
+  /** PIN: the null path is untouched — and the hook runs before it. */
+  it('renders the empty value for null without crashing on the hook', () => {
+    expect(() => renderCell(null, { name: 'win_rate' }, 'de')).not.toThrow();
+  });
+});

packages/fields/src/__tests__/percent-formatter-locale-4553.test.ts

@@ -0,0 +1,147 @@
+/**
+ * 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#4553 phase 2 — `formatPercent` was the last formatter outside the
+ * display-locale channel, and it was outside it in a different way from its
+ * siblings.
+ *
+ * PR #4557 threaded the gantt tooltip's number and currency rows and measured
+ * that percent could not follow: `formatPercent(value, precision)` took no
+ * locale parameter, and its whole body was
+ * `${percentDisplayValue(value).toFixed(precision)}%`. It built no
+ * `Intl.NumberFormat` and never reached `formatDisplayNumber`, so its output
+ * was not the MACHINE's locale (the defect its siblings had) but NO locale at
+ * all: an ASCII decimal mark, never a grouping separator, byte-identical on
+ * every machine on earth.
+ *
+ * That is why grouping and locale land together here. `1235%` is not merely
+ * un-German — it is wrong in en-US too, where the number is `1,235%`. So the
+ * English output MOVES, and that move is the fix, not a regression.
+ *
+ * ── Directions, predicted in writing BEFORE the run, then measured ───────
+ * Runner: node v22.22.2 / ICU 78.2 / machine locale en-US.
+ *
+ *   en 1234.5 p0   `1235%`   → `1,235%`      RED — the grouping move
+ *   de 1234.5 p0   `1235%`   → `1.235 %`     RED — separators inverted + NBSP
+ *   de 80     p0   `80%`     → `80 %`        RED — de writes a no-break space
+ *                                             before the sign; en does not
+ *   de 12.5   p1   `12.5%`   → `12,5 %`      RED — the ruling's named form
+ *   en 80     p0   `80%`     → `80%`         PIN, green both sides
+ *   en 12.5   p1   `12.5%`   → `12.5%`       PIN, green both sides
+ *   fraction scaling (0.8 → 80%)             PIN, green both sides
+ *
+ * The `en` pins are byte-identical because `en` and the runner's `en-US` agree
+ * and because grouping only shows from four digits up — they are NOT evidence
+ * the fix works; the `de` cases and the en GROUPING case carry that.
+ *
+ * Provider-less by construction (objectui#4514): these are pure-function cases
+ * that touch no hook. The provider-mounted half of this card lives in
+ * `PercentCellRenderer.locale.test.tsx`.
+ */
+
+import { describe, it, expect } from 'vitest';
+import { formatPercent } from '../index';
+
+/**
+ * German puts a NO-BREAK SPACE (U+00A0) between the number and the percent
+ * sign. Written as an escape rather than pasted, so the expectation is
+ * readable and greppable in both spellings.
+ */
+const NBSP = '\u00a0';
+
+describe('formatPercent groups its output (objectui#4553)', () => {
+  /**
+   * THE en RED CASE, and the one that justifies calling this a defect rather
+   * than a localization nicety: four digits went out ungrouped in English too.
+   */
+  it('en groups from four digits up — the output MOVES, and that is the fix', () => {
+    expect(formatPercent(1234.5, 0, 'en')).toBe('1,235%');
+    expect(formatPercent(1000000, 0, 'en')).toBe('1,000,000%');
+  });
+
+  /** PIN, green both sides: below the grouping threshold nothing changes. */
+  it('en output below the grouping threshold is byte-identical (must-not-change)', () => {
+    expect(formatPercent(80, 0, 'en')).toBe('80%');
+    expect(formatPercent(12.5, 1, 'en')).toBe('12.5%');
+    expect(formatPercent(33.33, 2, 'en')).toBe('33.33%');
+    expect(formatPercent(0, 0, 'en')).toBe('0%');
+  });
+});
+
+describe('formatPercent follows the display locale (objectui#4553)', () => {
+  /**
+   * The separators invert AND a no-break space appears before the sign, so the
+   * German form cannot coincide with the machine's on this runner.
+   */
+  it('de inverts the separators and spaces the percent sign', () => {
+    expect(formatPercent(1234.5, 0, 'de')).toBe(`1.235${NBSP}%`);
+    expect(formatPercent(12.5, 1, 'de')).toBe(`12,5${NBSP}%`);
+  });
+
+  /**
+   * Even with no separator in play, German still differs from English: the
+   * space before the sign is part of the locale's percent CONVENTION, which is
+   * what routing through `Intl` buys over appending a literal '%'.
+   */
+  it('de spaces the sign even for a value with no separators', () => {
+    expect(formatPercent(80, 0, 'de')).toBe(`80${NBSP}%`);
+  });
+
+  /** `Intl` accepts `'zh'` verbatim — no mapping table anywhere. */
+  it('zh renders its own convention', () => {
+    expect(formatPercent(1234.5, 0, 'zh')).toBe('1,235%');
+  });
+
+  /**
+   * A malformed tag from a tenant config must never take a cell down —
+   * `formatDisplayNumber` catches it and retries without the locale.
+   */
+  it('a malformed locale tag falls back instead of throwing', () => {
+    expect(() => formatPercent(80, 0, 'not a locale')).not.toThrow();
+    expect(formatPercent(80, 0, 'not a locale')).toContain('80');
+  });
+});
+
+describe('formatPercent keeps its existing contract (objectui#4553 must-not-change)', () => {
+  /**
+   * PIN: the fraction/whole disambiguation is `percentDisplayValue`'s, shared
+   * with the dashboard measure formatter. The locale parameter must not have
+   * moved it.
+   */
+  it('still scales a fraction-stored percent and passes a whole one through', () => {
+    expect(formatPercent(0.8, 0, 'en')).toBe('80%');
+    expect(formatPercent(0.5, 0, 'en')).toBe('50%');
+    expect(formatPercent(0.075, 1, 'en')).toBe('7.5%');
+    // >= 1 is already in display magnitude and is NOT scaled again.
+    expect(formatPercent(80, 0, 'en')).toBe('80%');
+    expect(formatPercent(100, 0, 'en')).toBe('100%');
+  });
+
+  it('still honors the precision it is given', () => {
+    expect(formatPercent(33.333, 0, 'en')).toBe('33%');
+    expect(formatPercent(33.333, 1, 'en')).toBe('33.3%');
+    expect(formatPercent(33.333, 2, 'en')).toBe('33.33%');
+  });
+
+  it('still handles negatives', () => {
+    expect(formatPercent(-45.5, 1, 'en')).toBe('-45.5%');
+  });
+
+  /**
+   * The parameter is OPTIONAL and third, matching `formatNumber(value,
+   * decimals, locale)` and `formatCurrency(value, currency, locale)`. An
+   * existing caller passing nothing still gets the runtime default locale —
+   * though it now also gets grouping, which is the deliberate output move.
+   */
+  it('is callable with no locale, and with no precision either', () => {
+    expect(() => formatPercent(80)).not.toThrow();
+    expect(formatPercent(80)).toBe('80%');
+    expect(formatPercent(80, 0)).toBe('80%');
+  });
+});