diff --git a/.changeset/9295-percent-surfaces-read-scale.md b/.changeset/9295-percent-surfaces-read-scale.md new file mode 100644 index 0000000000..0681d6e6f7 --- /dev/null +++ b/.changeset/9295-percent-surfaces-read-scale.md @@ -0,0 +1,57 @@ +--- +'@object-ui/fields': minor +'@object-ui/plugin-grid': minor +'@object-ui/plugin-detail': minor +--- + +All three percent surfaces read `scale` for their fraction width, not +`precision` (objectui#9295). + +`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, `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, 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 +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-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%'); }); 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 });