From 999bc7915e2cf19f8c1d99578dac2eb1e7226d1c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 14:22:51 +0000 Subject: [PATCH 1/2] fix(fields,plugin-grid): percent surfaces take their width from `scale` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `PercentCellRenderer` and the percent arm of `useColumnSummary`'s `formatSummaryLabel` both read `precision` and handed it to `Intl` as the minimum AND maximum fraction digits. `@objectstack/spec` declares `precision` as the "Total digits" of a decimal(p, s) column and `scale` as its "Decimal places", so a decimal(10, 2) percent field rendered `25.0000000000%` in the cell with `Sum: 25.0000000000%` in the footer beneath it — the same defect objectui#2131 removed from the currency arm and objectui#2134 from the number arm, one type over. Card: objectui#9295. Both percent surfaces move together, or the cell and the footer diverge on screen. An ABSENT `scale` stays 0 — matching the currency arm's spelling and today's behaviour — and deliberately not the number arm's `undefined` (min 0 / max 20), which would print binary residue here because this path multiplies by 100 first: measured, `0.07` scales to `7.000000000000001` and `0.29` to `28.999999999999996`. Three existing fixtures declared the width with `precision`; each is restated as `scale`. The objectui#9269 convergence case that pinned the old member keeps its claim and changes only the member it declares. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01KSd9P5u2Mf4p8g4n4SD4Fx --- .../9295-percent-surfaces-read-scale.md | 49 +++++++ .../PercentCellRenderer.locale.test.tsx | 12 +- .../PercentCellRenderer.scale-9295.test.tsx | 107 ++++++++++++++ .../__tests__/PercentCellRenderer.test.tsx | 6 +- packages/fields/src/index.tsx | 30 +++- ...mnSummary.percentConvergence-9269.test.tsx | 25 ++-- ...seColumnSummary.percentScale-9295.test.tsx | 132 ++++++++++++++++++ packages/plugin-grid/src/useColumnSummary.ts | 20 ++- 8 files changed, 359 insertions(+), 22 deletions(-) create mode 100644 .changeset/9295-percent-surfaces-read-scale.md create mode 100644 packages/fields/src/__tests__/PercentCellRenderer.scale-9295.test.tsx create mode 100644 packages/plugin-grid/src/__tests__/useColumnSummary.percentScale-9295.test.tsx diff --git a/.changeset/9295-percent-surfaces-read-scale.md b/.changeset/9295-percent-surfaces-read-scale.md new file mode 100644 index 0000000000..301ac036f8 --- /dev/null +++ b/.changeset/9295-percent-surfaces-read-scale.md @@ -0,0 +1,49 @@ +--- +'@object-ui/fields': minor +'@object-ui/plugin-grid': minor +--- + +Both percent surfaces read `scale` for their fraction width, not `precision` +(objectui#9295). + +`PercentCellRenderer` in `@object-ui/fields` and the `colType === 'percent'` arm +of `formatSummaryLabel` in `@object-ui/plugin-grid`'s `useColumnSummary` each +took `precision` and handed it to `Intl` as BOTH the minimum and the maximum +fraction digits. `@objectstack/spec` declares the pair in its own words on both +the field face and the column face: `precision` is the "Total digits" of a +`decimal(p, s)` column and `scale` is its "Decimal places" — so a percent field +was padded out to the column's TOTAL width. A `decimal(10, 2)` percent field +rendered `25.0000000000%` in the cell and `Sum: 25.0000000000%` in the footer +directly beneath it. This is the identical defect objectui#2131 removed from the +currency arm and objectui#2134 from the number arm, arriving one type later; in +`useColumnSummary` the corrected percent arm now sits four lines below a currency +arm it finally agrees with. + +**Breaking, deliberately — filed as `minor` because this repo's fixed release +group forbids `major`.** Percent rendering moves in two directions: + +- A percent field or column declaring `scale` now honours it. Declaring + `scale: 2` previously rendered `25%` and now renders `25.00%`. +- A percent field or column declaring `precision` no longer pads to it. + Declaring `precision: 10` previously rendered `25.0000000000%` and now + renders `25%`. + +**Migration.** Restate the intended fraction width as `scale`, which is the +member the contract has always declared for it. Metadata carrying an accurate +`decimal(p, s)` pair — both members, as a database column exposes them — needs no +change and simply stops being padded. + +**Unchanged: a percent field that declares neither member.** An absent `scale` +is still zero fraction digits, matching the currency arm beside it, so this is +invisible to metadata that declares nothing. That default is a decision rather +than a leftover: the number cell renderer spells the same absence as +`undefined` (minimum 0, maximum 20), and copying it here would print binary +floating-point residue, because the percent path multiplies by 100 first and +`Intl` renders from the shortest decimal representation of the resulting double +— a stored `0.07` becomes `7.000000000000001` and `0.29` becomes +`28.999999999999996`. The number arm can afford an unbounded maximum because it +performs no arithmetic on the value. + +`CurrencyConfigSchema.precision` is untouched and must not be conflated with +this: it is a different surface with the opposite convention and its own +`scale` alias, and the spec says so at the field-face declaration. diff --git a/packages/fields/src/__tests__/PercentCellRenderer.locale.test.tsx b/packages/fields/src/__tests__/PercentCellRenderer.locale.test.tsx index 270f40d5da..4c2224b46f 100644 --- a/packages/fields/src/__tests__/PercentCellRenderer.locale.test.tsx +++ b/packages/fields/src/__tests__/PercentCellRenderer.locale.test.tsx @@ -128,14 +128,20 @@ describe('PercentCellRenderer keeps its scaling contract (objectui#4553 must-not renderCell(0.5, { name: 'progress' }, 'en'); // Whole-percent field: 0.5 really is half a percent, rounded to 1% at - // precision 0 — NOT 50%. + // zero fraction digits (this field declares no `scale`) — NOT 50%. expect(cellText()).toContain('1%'); expect(cellText()).not.toContain('50%'); }); - /** PIN: small-value English output is byte-identical across the change. */ + /** + * PIN: small-value English output is byte-identical across the change. + * + * The two-decimal width is declared with `scale`, ⛔ not `precision` + * (objectui#9295) — `precision` is the column's TOTAL digit count and this + * renderer no longer reads it. + */ it('en small-value output is unchanged (must-not-change)', () => { - renderCell(33.33, { name: 'win_rate', precision: 2 }, 'en'); + renderCell(33.33, { name: 'win_rate', scale: 2 }, 'en'); expect(cellText()).toContain('33.33%'); }); diff --git a/packages/fields/src/__tests__/PercentCellRenderer.scale-9295.test.tsx b/packages/fields/src/__tests__/PercentCellRenderer.scale-9295.test.tsx new file mode 100644 index 0000000000..586241b202 --- /dev/null +++ b/packages/fields/src/__tests__/PercentCellRenderer.scale-9295.test.tsx @@ -0,0 +1,107 @@ +/** + * 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#9295 — `PercentCellRenderer` read `precision` as a FRACTION-digit + * count, in the same file whose number arm records that `precision` is the + * TOTAL digit count of a `decimal(p, s)` column. + * + * `@objectstack/spec` declares the pair on the field face in its own words — + * `precision` is "Total digits (non-negative integer)" and `scale` is "Decimal + * places (non-negative integer)" — so the member to read is `scale`. This is + * the identical repair objectui#2131 made on the currency arm and + * objectui#2134 on the number arm, arriving one type later. + * + * ── Why the assertions are on RENDERED OUTPUT ─────────────────────────── + * The defect is what the user sees in the cell, and `formatPercent` is a + * shared helper that was never wrong — it formats to the width it is handed. + * Asserting the helper's return value would pin the wrong end and stay green + * on the defect. Every row below goes through the component. + * + * ── What fails before the repair ──────────────────────────────────────── + * `decimal(10, 2)` — `{ precision: 10, scale: 2 }` — rendered + * `25.0000000000%` for a stored `0.25`, padded out to the column's TOTAL + * width. The grid footer beneath it read `Sum: 25.0000000000%` for the same + * reason; its half of this card is pinned in `@object-ui/plugin-grid`. + * + * ── The ABSENT-`scale` case is a DECISION, not a leftover ─────────────── + * An absent `scale` stays `0` here, and deliberately NOT the `undefined` + * (min 0 / max 20) that `NumberCellRenderer` uses for the same absence. This + * path multiplies by 100 first (`percentDisplayValue`) and `Intl` renders from + * the shortest decimal representation of the resulting double, so an unbounded + * maximum prints binary residue: measured, a stored `0.07` scales to + * `7.000000000000001` and `0.29` to `28.999999999999996`. The last two cases + * below pin that those values stay readable. + */ +import { describe, it, expect } from 'vitest'; +import { render, screen, cleanup } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import React from 'react'; + +import { PercentCellRenderer } from '../index'; + +const renderPercent = (value: unknown, field: Record = {}) => + render( + , + ); + +/** The cell's whole text, bar included — the bar contributes none. */ +const cellText = () => screen.getByRole('progressbar').parentElement!.textContent ?? ''; + +describe('PercentCellRenderer reads `scale`, not `precision` (objectui#9295)', () => { + it('does not pad a decimal(10, 2) percent field out to ten fraction digits', () => { + renderPercent(0.25, { name: 'rate', precision: 10, scale: 2 }); + + // The card's headline reading, and the row that fails before the repair. + expect(cellText()).toContain('25.00%'); + expect(cellText()).not.toContain('25.0000000000%'); + }); + + it('ignores `precision` entirely when no `scale` is declared', () => { + // decimal(10, 0) — ten total digits, zero decimal places. `precision` + // alone must move nothing. + renderPercent(0.25, { name: 'rate', precision: 10 }); + + expect(cellText()).toContain('25%'); + expect(cellText()).not.toMatch(/\.0{3,}/); + }); + + it('honours a declared `scale` on its own', () => { + renderPercent(0.25, { name: 'rate', scale: 3 }); + expect(cellText()).toContain('25.000%'); + }); + + it('applies the same member on the WHOLE-percent branch', () => { + // `progress` takes the other scaling arm (`formatPercentBody`), which was + // handed the same wrong member. A stored 25 is 25% here, not 2500%. + renderPercent(25, { name: 'progress', precision: 10, scale: 2 }); + + expect(cellText()).toContain('25.00%'); + expect(cellText()).not.toContain('25.0000000000%'); + }); + + it('leaves a field declaring neither member exactly where it was', () => { + // MUST-NOT-CHANGE control: absent `scale` is still zero fraction digits, + // so this repair is invisible to every field that declares nothing. + renderPercent(0.12345, { name: 'rate' }); + expect(cellText()).toContain('12%'); + }); + + it('keeps an absent `scale` free of binary floating-point residue', () => { + // The measured reason the absent case is `0` and not `undefined` + // (min 0 / max 20): `0.07 * 100` is `7.000000000000001` as a double. + renderPercent(0.07, { name: 'rate' }); + expect(cellText()).toContain('7%'); + expect(cellText()).not.toContain('7.000000000000001%'); + cleanup(); + + renderPercent(0.29, { name: 'rate' }); + expect(cellText()).toContain('29%'); + expect(cellText()).not.toContain('28.999999999999996%'); + }); +}); diff --git a/packages/fields/src/__tests__/PercentCellRenderer.test.tsx b/packages/fields/src/__tests__/PercentCellRenderer.test.tsx index 9f224d26e2..6d0970dbe3 100644 --- a/packages/fields/src/__tests__/PercentCellRenderer.test.tsx +++ b/packages/fields/src/__tests__/PercentCellRenderer.test.tsx @@ -88,8 +88,10 @@ describe('PercentCellRenderer — value beats the decorative bar (issue #5066)', expect(screen.getByText('33%')).toBeInTheDocument(); first.unmount(); - // Declared precision keeps the decimals — the widest text, worst overflow. - renderPercent(33.33, { precision: 2 }); + // A declared `scale` keeps the decimals — the widest text, worst overflow. + // ⛔ NOT `precision` (objectui#9295): that is the column's TOTAL digit + // count, and reading it here is the defect that card removed. + renderPercent(33.33, { scale: 2 }); const wide = screen.getByText('33.33%'); expect(wide).toBeInTheDocument(); expect(wide).toHaveClass('shrink-0'); diff --git a/packages/fields/src/index.tsx b/packages/fields/src/index.tsx index d7a09051a6..50245db8c1 100644 --- a/packages/fields/src/index.tsx +++ b/packages/fields/src/index.tsx @@ -794,7 +794,31 @@ export function PercentCellRenderer({ value, field }: CellRendererProps): React. if (isBlankCellText(safe)) return ; const percentField = field as any; - const precision = percentField.precision ?? 0; + // Decimal places come from `scale`, NOT `precision` — the same correction + // objectui#2131 made on the currency arm and objectui#2134 on the number + // arm, arriving one type later (objectui#9295). `@objectstack/spec` declares + // the pair on the field face in its own words: `precision` is "Total digits + // (non-negative integer)" and `scale` is "Decimal places (non-negative + // integer)", so reading `precision` here padded every value out to the + // column's TOTAL width — a decimal(10, 2) percent field rendered + // `25.0000000000%`, and the grid footer beneath it read + // `Sum: 25.0000000000%` for the same reason. + // + // ⛔ NOT `CurrencyConfigSchema.precision`, which is a different surface with + // the opposite convention and its own `scale` alias — the spec warns against + // conflating them at the field-face declaration itself. + // + // An ABSENT `scale` keeps today's `0`, deliberately, and ⛔ NOT the + // `undefined` (min 0 / max 20) that `NumberCellRenderer` above uses for the + // same absence. The two are not interchangeable HERE because this path + // multiplies by 100 first (`percentDisplayValue`), and `Intl` renders from + // the shortest decimal representation of the resulting double: measured, a + // stored `0.07` becomes `7.000000000000001` and `0.29` becomes + // `28.999999999999996`, so an unbounded maximum prints binary residue + // straight to the user. `NumberCellRenderer` can afford max 20 because it + // does no arithmetic on the value. The grid footer's currency arm spells the + // same absence the same way (`?? 0`), so the cell and the footer agree. + const scale = percentField.scale ?? 0; const numValue = Number(safe); if (isNaN(numValue)) { return {String(safe)}; @@ -813,8 +837,8 @@ export function PercentCellRenderer({ value, field }: CellRendererProps): React. // would have made ONE grid internally inconsistent, which is worse than the // uniform defect it had. const formatted = isWholePercentField - ? formatPercentBody(numValue, precision, locale) - : formatPercent(numValue, precision, locale); + ? formatPercentBody(numValue, scale, locale) + : formatPercent(numValue, scale, locale); const clampedBar = Math.max(0, Math.min(100, barValue)); // Layout contract (objectstack#5066): THE NUMBER IS THE CONTENT, THE BAR IS diff --git a/packages/plugin-grid/src/__tests__/useColumnSummary.percentConvergence-9269.test.tsx b/packages/plugin-grid/src/__tests__/useColumnSummary.percentConvergence-9269.test.tsx index 12083db1e0..65b6d40f11 100644 --- a/packages/plugin-grid/src/__tests__/useColumnSummary.percentConvergence-9269.test.tsx +++ b/packages/plugin-grid/src/__tests__/useColumnSummary.percentConvergence-9269.test.tsx @@ -17,6 +17,10 @@ * const pct = (value > -1 && value < 1) ? value * 100 : value; * formatted = `${pct.toFixed(decimals)}%`; * + * ⚠️ That block is quoted as HISTORY and left verbatim. Line 1 has since + * moved again: objectui#9295 answered the member question this card fenced, + * and the arm now reads `column?.scale ?? 0`. + * * Line 2 is `percentDisplayValue` in `@object-ui/core` character for * character, so the SCALING agreed — by duplication, not by reference. The * CONVENTION was not taken at all. `percentDisplayValue`'s own doc comment @@ -195,18 +199,23 @@ describe('the grid summary percent arm takes BOTH halves from the declared sourc ); /** - * `decimals` still comes from `column.precision`, unchanged by this card. + * `decimals` comes from `column.scale`. + * + * ⭐ This case used to read `column.precision` and fenced the member as an + * explicitly NOT MEASURED question. objectui#9295 ANSWERED it: the spec + * declares `precision` as the column's total digit count and `scale` as its + * decimal places, so the percent arm joined the currency arm on `scale` and + * the branch this case pinned is gone. The CLAIM is unchanged — the footer + * honours the width the column declares — only the member that declares it. * - * ⚠️ Whether `precision` is the RIGHT member to read here is a separate, - * explicitly NOT MEASURED question (the neighbouring currency arm reads - * `scale`, with an in-code note from objectui#2131). This case pins only - * that the repair did not move it. + * The repair itself is pinned in `useColumnSummary.percentScale-9295`; this + * case keeps objectui#9269's convergence claim true across it. */ - it('still honours the width declared by the column precision', () => { - expect(summaryLabel(0.12345, 'en', { precision: 2 })).toBe( + it('still honours the width declared by the column scale', () => { + expect(summaryLabel(0.12345, 'en', { scale: 2 })).toBe( `${PREFIX}${formatPercent(0.12345, 2, 'en')}`, ); - expect(summaryLabel(0.12345, 'de-DE', { precision: 2 })).toBe( + expect(summaryLabel(0.12345, 'de-DE', { scale: 2 })).toBe( `${PREFIX}${formatPercent(0.12345, 2, 'de-DE')}`, ); }); diff --git a/packages/plugin-grid/src/__tests__/useColumnSummary.percentScale-9295.test.tsx b/packages/plugin-grid/src/__tests__/useColumnSummary.percentScale-9295.test.tsx new file mode 100644 index 0000000000..177b6a926d --- /dev/null +++ b/packages/plugin-grid/src/__tests__/useColumnSummary.percentScale-9295.test.tsx @@ -0,0 +1,132 @@ +/** + * 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#9295 — the grid column-summary footer's percent arm read + * `precision` as a FRACTION-digit count, four lines below a currency arm that + * already read `scale` and carried the objectui#2131 note saying why. + * + * `@objectstack/spec` declares the pair on the COLUMN face in its own words: + * `precision` is "Total digits (non-negative integer; for number/currency)" — + * percent is not in that list at all — and `scale` is "Decimal places + * (non-negative integer)". So the footer padded a decimal(10, 2) percent + * column out to ten fraction digits. + * + * ── The claim that actually matters is AGREEMENT ──────────────────────── + * This footer sits directly beneath the list cell, and both read the same + * member for the same reason. A fix to one alone makes them disagree on + * screen, so the agreement rows below compare this hook's own label against + * `formatPercent` — the list cell's declared source — computed in the same + * run, rather than against a hand-written string that could only restate one + * side. + * + * ⭐ A two-surface comparison is blind to a JOINT move, which is what the + * absolute-byte rows are for: they name the pre-repair output and refuse it. + */ +import { describe, it, expect } from 'vitest'; +import * as React from 'react'; +import { renderHook } from '@testing-library/react'; +import { I18nProvider, LocalizationProvider } from '@object-ui/i18n'; +import { formatPercent } from '@object-ui/fields'; +import { useColumnSummary } from '../useColumnSummary'; + +/** + * The UI language is held at `en` while the TENANT locale moves — the real + * precedence `useDisplayLocale` implements, and the convention + * `useColumnSummary.percentConvergence-9269` established in this directory. + * Holding the language keeps the footer's PREFIX in English, so the rows below + * can name a whole label instead of fishing a substring out of it. Passing the + * locale as the LANGUAGE instead translates the prefix (`Summe:`) and the + * failure then reads as a percent defect that is not there. + */ +function wrapper(locale: string) { + return ({ children }: { children: React.ReactNode }) => ( + + {children} + + ); +} + +function summaryLabel(stored: number, locale: string, column: Record = {}): string { + const cols: any[] = [{ field: 'rate', summary: 'sum', type: 'percent', ...column }]; + const { result } = renderHook(() => useColumnSummary(cols, [{ rate: stored }]), { + wrapper: wrapper(locale), + }); + return result.current.summaries.get('rate')?.label ?? ''; +} + +/** + * MEASURED, not assumed: a bundle change to `grid.summary.sum` / + * `grid.summary.pattern` would otherwise arrive here as a percent failure. + * The first case below reads it off a column with no percent type at all, so + * if that case is the red one the prefix moved and this card is not implicated. + */ +const PREFIX = 'Sum: '; + +describe('useColumnSummary percent arm reads `scale`, not `precision` (objectui#9295)', () => { + it('the label prefix this file builds on is the one the bundle produces', () => { + const cols: any[] = [{ field: 'n', summary: 'sum' }]; + const { result } = renderHook(() => useColumnSummary(cols, [{ n: 1 }]), { + wrapper: wrapper('en'), + }); + expect(result.current.summaries.get('n')?.label).toBe(`${PREFIX}1`); + }); + + it('does not pad a decimal(10, 2) percent column out to ten fraction digits', () => { + // The row that fails before the repair: `Sum: 25.0000000000%`. + expect(summaryLabel(0.25, 'en', { precision: 10, scale: 2 })).toBe(`${PREFIX}25.00%`); + }); + + it('ignores `precision` entirely when no `scale` is declared', () => { + expect(summaryLabel(0.25, 'en', { precision: 10 })).toBe(`${PREFIX}25%`); + }); + + it('honours a declared `scale` on its own', () => { + expect(summaryLabel(0.25, 'en', { scale: 3 })).toBe(`${PREFIX}25.000%`); + }); + + it('leaves a column declaring neither member exactly where it was', () => { + // MUST-NOT-CHANGE control. + expect(summaryLabel(0.12345, 'en')).toBe(`${PREFIX}12%`); + }); +}); + +describe('the footer and the list cell above it move together (objectui#9295)', () => { + /** + * Both percent surfaces resolve their width from the same member, so the + * footer's label is the cell's rendering under the same declaration. Fixing + * only one surface fails these rows in whichever direction was left behind. + */ + it.each([ + [0.25, { precision: 10, scale: 2 }, 2], + [0.25, { precision: 10 }, 0], + [0.12345, { scale: 4 }, 4], + [0.12345, {}, 0], + ])('agrees with formatPercent for %p declaring %p', (stored, column, width) => { + expect(summaryLabel(stored as number, 'en', column as Record)).toBe( + `${PREFIX}${formatPercent(stored as number, width as number, 'en')}`, + ); + }); + + it('agrees in a locale whose percent convention differs from English', () => { + // de-DE writes a no-break space before the sign; tr-TR puts the sign in + // front. The width and the convention have to survive together. + expect(summaryLabel(0.25, 'de-DE', { precision: 10, scale: 2 })).toBe( + `${PREFIX}${formatPercent(0.25, 2, 'de-DE')}`, + ); + expect(summaryLabel(0.25, 'tr-TR', { precision: 10, scale: 2 })).toBe( + `${PREFIX}${formatPercent(0.25, 2, 'tr-TR')}`, + ); + }); + + it('refuses the pre-repair bytes outright', () => { + // The absolute-byte control a two-surface comparison cannot provide. + const retired = `${PREFIX}25.0000000000%`; + expect(summaryLabel(0.25, 'en', { precision: 10, scale: 2 })).not.toBe(retired); + }); +}); diff --git a/packages/plugin-grid/src/useColumnSummary.ts b/packages/plugin-grid/src/useColumnSummary.ts index 0bd7e87dac..4a2b5a33f9 100644 --- a/packages/plugin-grid/src/useColumnSummary.ts +++ b/packages/plugin-grid/src/useColumnSummary.ts @@ -385,12 +385,20 @@ function formatSummaryLabel( // the LOCALE's rather than a literal. Taking only the first is precisely // the drift objectui#4576 already paid for once. // - // ⚠️ `decimals` still reads `precision`, NOT `scale`. Whether that is the - // right member here is a separate and deliberately unmeasured question — - // the currency arm above reads `scale` for the reason #2131 records — and - // this card's own table had precision agreeing on both sides (`12.3` reads - // `12%` either way), so it is left exactly where it was. - const decimals = column?.precision ?? 0; + // objectui#9295 — that unmeasured question is ANSWERED, and the answer is + // `scale`. `@objectstack/spec` declares the pair on the column face in its + // own words: `precision` is "Total digits (non-negative integer; for + // number/currency)" — percent is not even in that list — and `scale` is + // "Decimal places (non-negative integer)". Reading `precision` padded this + // footer out to the column's TOTAL width, so a decimal(10, 2) percent + // column summed to `Sum: 25.0000000000%` under a cell reading + // `25.0000000000%`: the identical defect #2131 removed from the currency + // arm above, one type over. Both percent surfaces move together, or this + // footer and the cell above it disagree. + // + // An ABSENT `scale` stays `0`, matching the currency arm's spelling + // directly above and the list cell's — the three agree by construction. + const decimals = column?.scale ?? 0; formatted = formatPercent(value, decimals, displayLocale); } else if (type === 'avg') { formatted = value.toLocaleString(displayLocale, { maximumFractionDigits: 2 }); From 9ef3b967fd474de45be98a947d495112e9125dc2 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 15:11:25 +0000 Subject: [PATCH 2/2] fix(plugin-detail): the summary chip follows the cell onto `scale` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The record summary chip's percent branch read `precision ?? 0` explicitly to mirror `PercentCellRenderer`, which its own comment named. When the cell moved to `scale`, the chip stayed behind and objectui#9167's pin went red on the assertion that exists for exactly that: chip and cell must be byte-equal. objectui#9167 routed this chip onto the LIST CELL as its authority, and its ACCEPT ruling turns on the two agreeing on every measured row. So the member was always incidental there — following the cell onto `scale` is what KEEPS that ruling, and staying on `precision` is what would have broken it. Card: objectui#9295. The pin's fixtures are restated from `precision: 2` to `scale: 2`, and two rows are added for the decimal(10, 2) case the card is named after: a declared `precision` beside a declared `scale` must not widen the chip, and a bare `precision` is not a width. Both of objectui#9167's assertions still hold. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01KSd9P5u2Mf4p8g4n4SD4Fx --- .../9295-percent-surfaces-read-scale.md | 38 ++++++++----- packages/plugin-detail/src/DetailView.tsx | 26 +++++++-- ...ummaryChip.percentConvention-9167.test.tsx | 56 +++++++++++++++---- .../summaryChip.percentOneRule-8728.test.tsx | 2 +- .../summaryChip.percentSource-9071.test.tsx | 2 +- 5 files changed, 91 insertions(+), 33 deletions(-) diff --git a/.changeset/9295-percent-surfaces-read-scale.md b/.changeset/9295-percent-surfaces-read-scale.md index 301ac036f8..0681d6e6f7 100644 --- a/.changeset/9295-percent-surfaces-read-scale.md +++ b/.changeset/9295-percent-surfaces-read-scale.md @@ -1,31 +1,39 @@ --- '@object-ui/fields': minor '@object-ui/plugin-grid': minor +'@object-ui/plugin-detail': minor --- -Both percent surfaces read `scale` for their fraction width, not `precision` -(objectui#9295). +All three percent surfaces read `scale` for their fraction width, not +`precision` (objectui#9295). -`PercentCellRenderer` in `@object-ui/fields` and the `colType === 'percent'` arm -of `formatSummaryLabel` in `@object-ui/plugin-grid`'s `useColumnSummary` each -took `precision` and handed it to `Intl` as BOTH the minimum and the maximum -fraction digits. `@objectstack/spec` declares the pair in its own words on both +`PercentCellRenderer` in `@object-ui/fields`, the `colType === 'percent'` arm of +`formatSummaryLabel` in `@object-ui/plugin-grid`'s `useColumnSummary`, and the +record summary chip's percent branch in `@object-ui/plugin-detail`'s +`DetailView` each took `precision` and handed it to `Intl` as BOTH the minimum +and the maximum fraction digits. `@objectstack/spec` declares the pair in its own words on both the field face and the column face: `precision` is the "Total digits" of a `decimal(p, s)` column and `scale` is its "Decimal places" — so a percent field was padded out to the column's TOTAL width. A `decimal(10, 2)` percent field -rendered `25.0000000000%` in the cell and `Sum: 25.0000000000%` in the footer -directly beneath it. This is the identical defect objectui#2131 removed from the -currency arm and objectui#2134 from the number arm, arriving one type later; in -`useColumnSummary` the corrected percent arm now sits four lines below a currency -arm it finally agrees with. +rendered `25.0000000000%` in the cell, `Sum: 25.0000000000%` in the footer +directly beneath it, and `25.0000000000%` again on the record summary chip. This +is the identical defect objectui#2131 removed from the currency arm and +objectui#2134 from the number arm, arriving one type later; in `useColumnSummary` +the corrected percent arm now sits four lines below a currency arm it finally +agrees with. + +The summary chip moves because objectui#9167 routed it onto the LIST CELL as its +authority and its ruling turns on the two being byte-equal, so the member was +always incidental there: following the cell is what KEEPS that ruling. Its pin +asserts both halves and is what caught the chip being left behind. **Breaking, deliberately — filed as `minor` because this repo's fixed release group forbids `major`.** Percent rendering moves in two directions: -- A percent field or column declaring `scale` now honours it. Declaring - `scale: 2` previously rendered `25%` and now renders `25.00%`. -- A percent field or column declaring `precision` no longer pads to it. - Declaring `precision: 10` previously rendered `25.0000000000%` and now +- A percent field, column or summary chip declaring `scale` now honours it. + Declaring `scale: 2` previously rendered `25%` and now renders `25.00%`. +- A percent field, column or summary chip declaring `precision` no longer pads + to it. Declaring `precision: 10` previously rendered `25.0000000000%` and now renders `25%`. **Migration.** Restate the intended fraction width as `scale`, which is the diff --git a/packages/plugin-detail/src/DetailView.tsx b/packages/plugin-detail/src/DetailView.tsx index 17d8be813b..c4b02888e0 100644 --- a/packages/plugin-detail/src/DetailView.tsx +++ b/packages/plugin-detail/src/DetailView.tsx @@ -1194,12 +1194,26 @@ export const DetailView: React.FC = ({ // to the field's precision would make this chip // disagree with the cell it just started agreeing with. const percentField = { ...(objField as any), ...(sectionField as any) }; - // The field's declared precision, resolved with the - // same view-over-object precedence the currency branch - // above spells, and floored at the cell's own default: - // `PercentCellRenderer` reads `field.precision ?? 0`. - const precision = percentField.precision ?? 0; - display = formatPercent(num, precision, displayLocale); + // The field's declared width, resolved with the same + // view-over-object precedence the currency branch above + // spells, and floored at the cell's own default: + // `PercentCellRenderer` reads `field.scale ?? 0`. + // + // ⭐ The MEMBER moved and the AUTHORITY did not + // (objectui#9295). This read was `precision ?? 0` until + // `@objectstack/spec` was read at source: it declares + // `precision` as the "Total digits" of a decimal(p, s) + // column and `scale` as its "Decimal places", so the + // cell was padding a decimal(10, 2) percent field out to + // ten fraction digits and this chip mirrored it there. + // objectui#9167 routed this chip onto the LIST CELL as + // the authority — its ruling turns on the two being + // byte-equal — so when the cell's member moved, staying + // on `precision` is what would have BROKEN that ruling, + // not what would have kept it. Whatever the cell reads, + // this reads; that is the whole of the coupling. + const scale = percentField.scale ?? 0; + display = formatPercent(num, scale, displayLocale); const points = summaryChipPercentPoints(num); percentValue = Math.max(0, Math.min(100, points)); } diff --git a/packages/plugin-detail/src/__tests__/summaryChip.percentConvention-9167.test.tsx b/packages/plugin-detail/src/__tests__/summaryChip.percentConvention-9167.test.tsx index 6ea5df3ae1..26aed71abf 100644 --- a/packages/plugin-detail/src/__tests__/summaryChip.percentConvention-9167.test.tsx +++ b/packages/plugin-detail/src/__tests__/summaryChip.percentConvention-9167.test.tsx @@ -89,7 +89,7 @@ afterEach(() => { * The field both surfaces are handed. `ratio` is deliberate: it does NOT match * the cell renderer's whole-percent name pattern (`progress` / `completion`), so * both surfaces are on the same fraction-inferring path. No declared - * `precision`, so both take the cell's documented default of `0`. + * `scale`, so both take the cell's documented default of `0`. */ const FIELD: FieldMetadata = { name: 'ratio', label: 'Ratio', type: 'percent' }; @@ -196,10 +196,10 @@ interface Row { * declared convention rather than merely away from the old one. */ const MOVED_ROWS: Row[] = [ - { what: 'precision — a stored ratio rounds to the field default of 0', locale: 'en', stored: 0.123, text: '12%', was: '12.3%' }, - { what: 'precision — already in points, still rounded', locale: 'en', stored: 12.3, text: '12%', was: '12.3%' }, - { what: 'precision — rounding is half-expand, as the cell has always been', locale: 'en', stored: 1.5, text: '2%', was: '1.5%' }, - { what: 'precision — three decimals collapse to the declared 0', locale: 'en', stored: 1.005, text: '1%', was: '1.005%' }, + { what: 'width — a stored ratio rounds to the field default of 0', locale: 'en', stored: 0.123, text: '12%', was: '12.3%' }, + { what: 'width — already in points, still rounded', locale: 'en', stored: 12.3, text: '12%', was: '12.3%' }, + { what: 'width — rounding is half-expand, as the cell has always been', locale: 'en', stored: 1.5, text: '2%', was: '1.5%' }, + { what: 'width — three decimals collapse to the declared 0', locale: 'en', stored: 1.005, text: '1%', was: '1.005%' }, // ⭐ THE FOUR-DIGIT ROW. `1234.5%` is wrong in en-US as well as in German, // which is the reasoning objectui#4553 recorded when it made this same move // for the list cell. @@ -207,7 +207,7 @@ const MOVED_ROWS: Row[] = [ // ⭐ THE NON-`en` ROWS. The affix and the marks are the locale's own. { what: 'affix — de-DE separates the sign with its own space', locale: 'de-DE', stored: 0.25, text: '25 %', was: '25%' }, { what: 'affix + marks — de-DE swaps the grouping and decimal marks', locale: 'de-DE', stored: 1234.5, text: '1.235 %', was: '1234.5%' }, - // ⭐ The row no bare-append implementation can produce, at any precision. + // ⭐ The row no bare-append implementation can produce, at any width. { what: 'affix — tr-TR puts the sign in FRONT', locale: 'tr-TR', stored: 0.25, text: '%25', was: '25%' }, ]; @@ -285,20 +285,56 @@ describe('the summary chip takes the percent CONVENTION from the declared source }); /** - * The field's DECLARED precision, the authority this card routes the chip onto. + * The field's DECLARED width, the authority this card routes the chip onto. * Without it the two places could agree only by both defaulting to 0, which a * chip that ignored the field entirely would also satisfy. + * + * ⭐ The MEMBER is `scale`, and it moved without this card's ruling moving + * (objectui#9295). It was `precision` until `@objectstack/spec` was read at + * source: `precision` is the "Total digits" of a decimal(p, s) column and + * `scale` is its "Decimal places", so both surfaces were padding a + * decimal(10, 2) percent field out to ten fraction digits. This card routed + * the chip onto THE LIST CELL as the authority — its own ACCEPT turns on the + * two being byte-equal on every row — so the member was always incidental and + * following the cell is what KEEPS this ruling, not what bends it. The second + * assertion below is the one that would have caught a chip left behind, and + * it did: it is how objectui#9295 found this third surface. */ it.each([ { stored: 0.25, text: '25.00%' }, { stored: 12.3, text: '12.30%' }, { stored: 1234.5, text: '1,234.50%' }, { stored: 1.005, text: '1.01%' }, - ])('reads the field\'s declared precision: a stored $stored at precision 2 reads $text', ({ stored, text }) => { - const field = { ...FIELD, precision: 2 } as FieldMetadata; + ])('reads the field\'s declared width: a stored $stored at scale 2 reads $text', ({ stored, text }) => { + const field = { ...FIELD, scale: 2 } as FieldMetadata; const { chip, cell } = bothPlaces(stored, 'en', field); - expect(chip.text, 'the chip honours the declared precision').toBe(text); + expect(chip.text, 'the chip honours the declared width').toBe(text); + expect(chip.text, 'and so states what the cell states').toBe(cell.text); + }); + + /** + * objectui#9295's own row, kept HERE because this file is where the coupling + * lives: `precision` is the TOTAL digit count, so declaring it must not widen + * either surface. A decimal(10, 2) field declares BOTH, and the chip has to + * read the decimal-places one. + */ + it.each([ + { stored: 0.25, text: '25.00%' }, + { stored: 12.3, text: '12.30%' }, + ])('ignores `precision` beside a declared `scale`: $stored reads $text', ({ stored, text }) => { + const field = { ...FIELD, precision: 10, scale: 2 } as FieldMetadata; + const { chip, cell } = bothPlaces(stored, 'en', field); + + expect(chip.text, 'the chip pads to `scale`, never to `precision`').toBe(text); + expect(chip.text, 'and so states what the cell states').toBe(cell.text); + }); + + it('ignores a `precision` declared on its own, as the cell does', () => { + const field = { ...FIELD, precision: 10 } as FieldMetadata; + const { chip, cell } = bothPlaces(0.25, 'en', field); + + expect(chip.text, 'a bare `precision` is not a width').toBe('25%'); expect(chip.text, 'and so states what the cell states').toBe(cell.text); }); diff --git a/packages/plugin-detail/src/__tests__/summaryChip.percentOneRule-8728.test.tsx b/packages/plugin-detail/src/__tests__/summaryChip.percentOneRule-8728.test.tsx index 887db3c6e2..72ae8d1892 100644 --- a/packages/plugin-detail/src/__tests__/summaryChip.percentOneRule-8728.test.tsx +++ b/packages/plugin-detail/src/__tests__/summaryChip.percentOneRule-8728.test.tsx @@ -215,7 +215,7 @@ interface Row { const ROWS: Row[] = [ // The card's own reproduction. Before the fix: text `0.123%`, bar 12.3%. - // objectui#9167 then rounded the TEXT to the field's declared precision; the + // objectui#9167 then rounded the TEXT to the field's declared width; the // bar is the same 12.3 it has drawn since objectui#8728. { what: 'a stored ratio — the card\'s reproduction', stored: 0.123, text: '12%', bar: 12.3 }, // CONTROL for the SCALING — already percentage points, so the magnitude may diff --git a/packages/plugin-detail/src/__tests__/summaryChip.percentSource-9071.test.tsx b/packages/plugin-detail/src/__tests__/summaryChip.percentSource-9071.test.tsx index f1e9020cab..0a266e10fd 100644 --- a/packages/plugin-detail/src/__tests__/summaryChip.percentSource-9071.test.tsx +++ b/packages/plugin-detail/src/__tests__/summaryChip.percentSource-9071.test.tsx @@ -228,7 +228,7 @@ describe('the summary chip reads the declared percent source (objectui#9071)', ( expect(chip.bar, 'both draw 12.3 points').toBe(12.3); expect( chip.text, - "the convention agrees too — the chip rounds to the field's precision and renders the locale's affix (objectui#9167)", + "the convention agrees too — the chip rounds to the field's declared width and renders the locale's affix (objectui#9167)", ).toBe(cell.text); expect(chip.text, 'and it is the reading the cell was already giving').toBe('12%'); });