Skip to content

Commit a80fef7

Browse files
fix(fields): a summary reads one stored value the same in the form and in the cell (objectui#11752) (#11753)
Fixes #11752 Clause-②: no A summary (roll-up) field now reads one stored value the same in the read-only record form and in a table cell. Implemented by session `session_01CGZy1BGCjdN5cXqL9cnvB8` (seat `domain:ui#3`, round 1, claim comment 6028618112). ## What changed - **`packages/fields/src/widgets/SummaryField.tsx`**, the read-only form face. It picked a format by `summaryOperations.function`: a `count` with `String(value)`, the other four at two fixed decimals. It now reads a summary's value the way the summary cell reads it (`FormulaCellRenderer`, which `summary` is registered to), whatever the function: - the shared emptiness floor, `isEmptyValue` on `coerceToSafeValue(value)`, as the cell takes it (it was `value == null`); - a JS number through `formatNumberFieldValue(value, field, locale)` with `useDisplayLocale()`: the call `NumberCellRenderer`, `NumberField` and `FormulaField` make; - anything else as the coerced text the cell prints. The widget no longer reads `summaryOperations`. Its span classes are unchanged. The shape and comments follow `FormulaField` (objectui#11748). - **No cell changed.** `packages/fields/src/index.tsx` is untouched: the summary cell already drew a JS number through the number cell. - **Prose that described the old face** moves with it (AGENTS.md #2): `content/docs/fields/summary.mdx` ("Aggregation Functions"); the `summaryOperations` doc comment on `SummaryFieldMetadata` (`packages/types/src/field-types.ts`) and on the form field (`packages/types/src/form.ts`); and the key-reads comment in `form.ts` that said the summary widget reads `summaryOperations`. - **Changeset** `.changeset/11752-summary-number-face.md`: `@object-ui/fields` patch, `@object-ui/types` patch. No export, prop or language-pack key is added, so `Clause-②` stays `no`. ## Measured on the base tree, before the fix The new pin file `packages/fields/src/__tests__/summaryFaces.numberFace-11752.test.tsx` was run on base `4c0de52` with the old widget: `Tests 50 failed | 70 passed (120)`. Every failure is a FORM leg. No CELL leg failed, so the cell already read the number face. Received against expected, copied from that run: | value | form face (base) | cell face | |---|---|---| | `sum` 15750.5 | `15750.50` | `15,750.5` | | `count` 12000 | `12000` | `12,000` | | `sum` / `count` 0 | `0.00` | `0` | | `avg` 10/3 | `3.33` | `3.3333333333333335` | | `sum` 15750.5, de-DE | `15750.50` | `15.750,5` | | `count` holding an empty string | a blank span | the empty-value affordance | The dispatch's mechanism hypotheses 1 and 4 hold as stated. Hypothesis 4, measured: with no `scale`, `formatNumberFieldValue` keeps a count whole and groups `12000` as `12,000`. On `@objectstack/spec` 17.6.0 and 17.7.0 alike, `resolveFieldScale` answers `undefined` for `{ type: 'summary' }` and `2` for `{ type: 'summary', scale: 2 }`. ## The pins Pins run per aggregation function (`count` / `sum` / `avg` / `min` / `max`) x face (form / cell) x locale (en-US, zh-CN). Each case renders under a real `I18nProvider` and `LocalizationProvider`: - the four non-count functions on `15750.5` read `15,750.5` and not `15750.50`. A declared `scale: 2` reads `15,750.50`; - `count` on `12000` reads `12,000` with no fraction, and on `42` reads `42`; - `0` reads `0` for every function, not `0.00` and not the affordance; - the number is not drawn in monospace. de-DE reaches the formatter (`15.750,5`); - equality with the `number` field's own faces: the form face equals `NumberField`'s read-only text and the cell equals `NumberCellRenderer`, across five values; - `null`, `undefined`, `''` and `[]` draw the shared affordance in both faces. A string of digits stays text; - **the equality leg:** twelve (function, value, declaration) rows, `scale` and `useGrouping` included. Each row is drawn in both faces in both sessions, and the form text is asserted equal to the cell text. `packages/fields/src/widgets/__tests__/formula-summary-spec-reads-11070.test.tsx` pinned the retired per-function face (`15750.50`). That is fixture triage, handled by replacement: its summary block pinned the very branch this PR removes. It now pins that the function moves nothing, and its `summary_type` retirement leg compares against a field with no roll-up. ## Ablation (from the committed fix) At `a31973f` (the fix commit), the base `SummaryField.tsx` was written into the working tree. It carries `toFixed(2)` once and `formatNumberFieldValue` zero times. The pin file then read `Tests 50 failed | 70 passed (120)`, exit 1, which is the same 50 form legs as on the base tree. The file was restored with `git checkout HEAD --`. The restored blob equals the HEAD blob (`598a6f1`), and `git diff HEAD` is empty. No build is involved: the pin imports the widget and the barrel by relative source path. ## The enumeration pin `git grep -n "toFixed(" -- packages/fields/src/widgets ':!*.test.*' ':!**/__tests__/**'` gave 17 hits on `4c0de52` and gives 16 on this branch. The `SummaryField` hit is gone. Its new comment says "two fixed decimals" rather than spell the call, so the pin counts no new prose. Each hit is classified below, cited by its text: | file | hit | record-value read face? | why | |---|---|---|---| | `SummaryField.tsx` | `value.toFixed(2)` | **yes, fixed here** | the card's subject | | `VectorField.tsx` | `num.toFixed(4)` | no, not under this rule | It previews the first three components of an embedding beside `(nD)`. Its cell, `VectorCellRenderer`, draws the literal `[Vector]` and no number, so there is no second number face to agree with. `formatNumberFieldValue` reads a field's `scale` and `useGrouping`, which say nothing about an embedding's components. Whether a vector deserves a richer face is the separate question the cell's docblock already names. | | `GridField.tsx` | `` `${currency} ${n.toFixed(digits)}` `` | no | the `catch` in `currencyText` when `Intl` refuses a currency code | | `number-format.ts` | `` `${currency} ${value.toFixed(fractionDigits)}` `` | no | `formatCurrency`'s `catch` fallback | | `number-format.ts` | `` `${displayValue.toFixed(precision)}%` `` | no | `formatPercentPoints`'s `catch` fallback | | `GridField.tsx` | `Number(v.toFixed(scale))` | no | `computeRow` rounds a computed stored value; not display | | `CurrencyField.tsx` | `parseFloat(val.toFixed(fractionDigits))` | no | `onChange` normalisation | | `CurrencyField.tsx` | `Math.pow(10, -fractionDigits).toFixed(fractionDigits)` | no | input `step` | | `PercentField.tsx` | `Math.pow(10, -scale).toFixed(scale)` | no | input `step` | | `PercentField.tsx` | `toDisplay(value).toFixed(scale)` | no | a comment | | `number-format.ts` | `toDisplay(value).toFixed(scale)` | no | a comment | | `FormulaField.tsx` (two) | `toFixed(2)` | no | comments, from objectui#11748 | | `FileField.tsx` | `(file.size / 1024).toFixed(1)` | no | a file size | | `file-size-guard.ts` | `(maxSize / BYTES_PER_MB).toFixed(1)` | no | the file size limit text | | `GeolocationField.tsx` | `loc.latitude.toFixed(6)` with `loc.longitude.toFixed(6)` (one line) | no | coordinates | | `GeolocationField.tsx` | `location.accuracy.toFixed(0)` | no | location accuracy | ## Local verification, at `77c9417` `77c9417` is the head this PR opens on: the fix and changeset commits, then a merge commit of `origin/main` (`c0862c1`, `@objectstack/*` 17.7.0). It was a merge, not a rebase, and it was clean. Every run below was made on that head, after a fresh `pnpm install` and a rebuild of the `@object-ui/fields` dependency closure, unless its row names `21266b3` (the commit before the merge). | command | exit | the gate's own line | |---|---|---| | `pnpm exec vitest run packages/fields/ packages/types/ examples/schema-catalog/test/fields-form-hosted.test.tsx` (the run counted 601 files: fields plus types) | 0 | `Test Files 600 passed, 1 skipped (601)`, `Tests 13780 passed, 7 skipped (13787)` | | `pnpm exec vitest run examples/schema-catalog/test/fields-form-hosted.test.tsx packages/fields/src/__tests__/summaryFaces.numberFace-11752.test.tsx` | 0 | `Test Files 2 passed (2)`, `Tests 355 passed (355)` | | `pnpm --filter @object-ui/fields type-check` (echoes `tsc --noEmit && tsc -p tsconfig.test.json`; `--listFiles` shows the new and the edited test files are included) | 0 | — | | `pnpm --filter @object-ui/types type-check` (echoes `tsc --noEmit && tsc -p tsconfig.examples.json && tsc -p tsconfig.test.json`) | 0 | — | | `pnpm exec eslint` on the five touched source and test files | 0 | `0 errors`; the warnings are `no-explicit-any` and predate this branch (the `SummaryField` one is the same `FieldWidgetComponentProps` generic on base) | | `pnpm check:control-bytes` | 0 | `check-control-bytes: OK` | | `pnpm check:test-path-roots` | 0 | `check-test-path-roots: OK` | | `pnpm check:changeset-claims` | 0 | `No pending changeset names a file this change touches.` | | `pnpm check:pending-changeset-literals` | 0 | `No test source names a pending changeset.` | | `pnpm check:new-line-citations` | 0 | `VERDICT new-cross-file-line-citations: 0 new citation(s)` | | `node scripts/check-changeset-presence.mjs` | 0 | `5 source file(s) of 2 released package(s) changed, and this change declares 1 changeset(s)` | | `pnpm check:doc-fences` / `check:doc-types` / `check:doc-example-ids` (on `21266b3`; the merge touched no `content/docs` file) | 0 | each prints its green line | | `pnpm check:comment-mask-corpus` / `check:unreferenced-sources` / `check:esm-specifiers` (on `21266b3`) | 0 | each prints its green line | NOT MEASURED: `check:doc-snippets` and `check:doc-examples`. Both exit 2 with "the packages these examples import are not built", which means the prerequisite was not met; it is not a red gate. They type-check TypeScript code blocks in the docs. The `summary.mdx` change adds no code block, only prose and a bullet list. CI builds the tree and runs them. No locale pack and no package export changed, so the i18n and readme-exports gates are not owed. ## Acceptance notes - **An average now reads at its own precision in the form.** Two fixed decimals are gone from the form face. With no `scale`, a fraction reads at its natural precision, as the cell already did: `10/3` reads `3.3333333333333335` in both faces. The author's lever is `scale` on the field, and both faces honour it. The spec's `FieldSchema` accepts `scale` and `useGrouping` on a `summary` field (measured: `safeParse` succeeds). `SummaryFieldMetadata` in `@object-ui/types` does not declare `scale`, so a typed objectui literal would refuse it, just as `FormulaFieldMetadata` does today. It is not added here, because that would be a new declared member. No first-party producer declares `scale` on a summary. - **A count under a declared `scale`** reads at that width in both faces (`scale: 2` reads `12,000.00`). The declaration wins, as it already did in the cell. "A count stays whole" holds for every summary that declares no `scale`, which covers every named producer. - **A summary that declares `returnType`.** The spec's flat `FieldSchema` accepts one on a `summary` (measured). The cell honours it, since it is `FormulaCellRenderer`, and this widget does not. No producer declares one: no `Field.summary` in objectstack's `examples/` declares `returnType` or `scale`. This is dormant, recorded here only. Carrier: none. - **Styling.** The form's number text keeps `font-medium tabular-nums`. The cell's number is `tabular-nums` without the weight. The card pins text, and the weight is unchanged. - **Outside the `toFixed(` pin.** `GridField`'s list-mode number column and its footer totals format with `toLocaleString(locale)`, not `formatNumberFieldValue`. Those are inner-column faces of a `grid` field, whose cell draws `[Grid]`, so the card's two-faces rule does not reach them. Recorded, not changed. Carrier: none. - **The board reading moved.** After the dispatch read the board at 01:05Z, `main` gained `c0862c1` (PR objectui#11742). The dispatch said that PR does not touch this card's files. It does touch `packages/types/src/field-types.ts`, in six lines outside the `summaryOperations` comment. The merge was clean. The tests, type-checks, eslint and the diff-derived gates above were re-run after it. - Currency display for a summary over currency fields stays with objectui#10221 and is not addressed here. --- _Generated by [Claude Code](https://claude.ai/code/session_01CGZy1BGCjdN5cXqL9cnvB8)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent c0862c1 commit a80fef7

7 files changed

Lines changed: 362 additions & 38 deletions

File tree

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
---
2+
'@object-ui/fields': patch
3+
'@object-ui/types': patch
4+
---
5+
6+
A summary (roll-up) field reads the same in a read-only form and in a table cell (objectui#11752).
7+
8+
The read-only form face (`SummaryField`) formatted the value by the roll-up's aggregation function: a `count` as it arrived, a `sum` / `avg` / `min` / `max` at two fixed decimals. The table cell, which `summary` shares with `formula`, draws a number as the number cell does. So one stored value read two ways: a `sum` over `15750.5` read `15750.50` in the form and `15,750.5` in the cell, and a `count` of `12000` read `12000` and `12,000`.
9+
10+
The form face now reads the value the way the cell does, whatever the function:
11+
12+
- **number**: formatted as a number field formats it, in the viewer's display locale. With no `scale` on the field the value keeps its own precision, so `15750.5` reads `15,750.5` and a `count` of `12000` reads `12,000` in en-US and zh-CN, in both faces. A count stays whole. A `scale` on the field fixes the width in both faces (`scale: 2` reads `15,750.50`). Two fixed decimals are no longer added, so an average reads at its own precision unless the field declares a `scale`.
13+
- **empty**: an empty string or empty list draws the empty-value mark, as in the cell, where the form drew a blank. A stored `0` (a `count` or `sum` over no child rows) reads `0`, where the form read `0.00`.
14+
- **anything else**: the value as text, as the cell prints it.
15+
16+
No export, prop or language-pack key is added. The `summaryOperations` doc comments in `@object-ui/types`, on `SummaryFieldMetadata` and on the form field, now describe this rule in place of the retired per-function face.

‎content/docs/fields/summary.mdx‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,18 @@ they are runtime widget props. See [Field Widget Props](/docs/fields/widget-prop
4949

5050
## Aggregation Functions
5151

52-
The summary field formats values by `summaryOperations.function`: `count` as it
53-
arrives, the other four to two decimal places.
52+
The function decides what the backend computes, not how the value is drawn. The
53+
form and the table cell read the same rule for every function, so one stored
54+
value reads the same in both:
55+
56+
- **a number**: Formatted as a number field formats it, in the viewer's display
57+
locale (`15750.5` reads `15,750.5` in en-US). With no `scale` on the field it
58+
keeps its own precision, so a `count` reads as a whole number (`12000` reads
59+
`12,000`); a `scale` on the field sets a fixed width in both places (`scale: 2`
60+
reads `15,750.50`)
61+
- **anything else**: The value as text; an empty value reads as empty
62+
63+
The five functions:
5464

5565
- **count**: Count of related records
5666
- **sum**: Sum of numeric field values
Lines changed: 243 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,243 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
/**
10+
* objectui#11752 — a summary (roll-up) field draws one stored value one way,
11+
* in the read-only form face (`SummaryField`) and in the table cell
12+
* (`getCellRenderer('summary')`, which is `FormulaCellRenderer`).
13+
*
14+
* Before this card the form face formatted the value by itself, by the
15+
* roll-up's aggregation `function`: `count` with `String(value)`, the other
16+
* four with `toFixed(2)`. So a `sum` over `15750.5` read `15750.50`, ungrouped
17+
* and in every locale, while the cell drew the same JS number through the
18+
* number cell (objectui#11683) and read `15,750.5`.
19+
*
20+
* The rule both faces now read, whatever the function:
21+
*
22+
* - the shared emptiness floor (`isEmptyValue`) on the coerced value;
23+
* - a JS number through `formatNumberFieldValue`, the call every number face
24+
* makes: the display locale's grouping, the width a declared `scale`
25+
* gives, the value's natural precision when none is declared. A `count`
26+
* is a whole number, so it reads whole;
27+
* - anything else as the text the cell prints.
28+
*
29+
* The pins run per aggregation function x face x locale, then compare each
30+
* face with the `number` field's own face for the same value, and end with an
31+
* equality leg that compares the two summary faces with each other rather than
32+
* with a literal, so a change that moves one face alone goes red.
33+
*
34+
* Every case renders through a real `I18nProvider` and `LocalizationProvider`,
35+
* so `zh-CN` is a Chinese session on both channels. `textContent` is the
36+
* measured property: it is what a copy takes and a screen reader reads.
37+
*/
38+
import React from 'react';
39+
import { describe, it, expect, afterEach } from 'vitest';
40+
import { render, cleanup } from '@testing-library/react';
41+
import { I18nProvider, LocalizationProvider } from '@object-ui/i18n';
42+
import { NumberCellRenderer, NumberField, getCellRenderer } from '../index';
43+
import { SummaryField } from '../widgets/SummaryField';
44+
45+
afterEach(() => cleanup());
46+
47+
const noop = () => {};
48+
49+
/** A session per locale: the UI language and the display locale agree. */
50+
const SESSIONS = {
51+
'en-US': 'en',
52+
'zh-CN': 'zh',
53+
} as const;
54+
type Locale = keyof typeof SESSIONS;
55+
const LOCALES = Object.keys(SESSIONS) as Locale[];
56+
57+
function renderSession(locale: Locale, node: React.ReactElement) {
58+
return render(
59+
<I18nProvider config={{ defaultLanguage: SESSIONS[locale], detectBrowserLanguage: false }} persistLanguage={false}>
60+
<LocalizationProvider value={{ locale }}>{node}</LocalizationProvider>
61+
</I18nProvider>,
62+
);
63+
}
64+
65+
function textOf(locale: Locale, node: React.ReactElement): string {
66+
const { container } = renderSession(locale, node);
67+
const text = container.textContent ?? '';
68+
cleanup();
69+
return text;
70+
}
71+
72+
const FUNCTIONS = ['count', 'sum', 'avg', 'min', 'max'] as const;
73+
type Fn = (typeof FUNCTIONS)[number];
74+
type Extra = Record<string, unknown>;
75+
76+
/** A roll-up shaped like the showcase's `expense_report.total_amount`. */
77+
const summaryField = (fn: Fn, extra: Extra = {}) =>
78+
({
79+
type: 'summary',
80+
name: 'total_amount',
81+
summaryOperations: { object: 'showcase_expense_line', field: 'amount', function: fn },
82+
...extra,
83+
}) as never;
84+
85+
const Cell = getCellRenderer('summary');
86+
87+
/** The read-only form face, as a record form renders a summary widget. */
88+
const formFace = (fn: Fn, value: unknown, extra: Extra = {}) => (
89+
<SummaryField value={value} onChange={noop} field={summaryField(fn, extra)} readonly />
90+
);
91+
92+
/** The table cell face, resolved the way every list surface resolves it. */
93+
const cellFace = (fn: Fn, value: unknown, extra: Extra = {}) => (
94+
<Cell value={value} field={summaryField(fn, extra)} />
95+
);
96+
97+
const FACES = [
98+
['form', formFace],
99+
['cell', cellFace],
100+
] as const;
101+
102+
/** The shared "No value" affordance. */
103+
const affordance = (root: HTMLElement) => root.querySelector('[data-slot="empty-value"]');
104+
105+
/** What a `number` field draws for the same value, per face. */
106+
const numberCell = (locale: Locale, value: number, extra: Extra = {}) =>
107+
textOf(locale, <NumberCellRenderer value={value} field={{ type: 'number', name: 'n', ...extra } as never} />);
108+
const numberForm = (locale: Locale, value: number, extra: Extra = {}) =>
109+
textOf(locale, <NumberField value={value} onChange={noop} field={{ type: 'number', name: 'n', ...extra } as never} readonly />);
110+
111+
// ── per function x face x locale ──────────────────────────────────────────
112+
113+
/** The grouped reading of `15750.5`; both sessions group with a comma. */
114+
const SUM_TEXT: Record<Locale, string> = { 'en-US': '15,750.5', 'zh-CN': '15,750.5' };
115+
/** The grouped reading of a count of `12000`. */
116+
const COUNT_TEXT: Record<Locale, string> = { 'en-US': '12,000', 'zh-CN': '12,000' };
117+
118+
describe('every aggregation function reads the number face, in both faces (objectui#11752)', () => {
119+
describe.each(LOCALES)('%s', (locale) => {
120+
describe.each(['sum', 'avg', 'min', 'max'] as const)('%s', (fn) => {
121+
it.each(FACES)("%s — 15750.5 reads grouped at its own precision, not toFixed's 15750.50", (_face, draw) => {
122+
const text = textOf(locale, draw(fn, 15750.5));
123+
expect(text).toBe(SUM_TEXT[locale]);
124+
expect(text).not.toBe('15750.50');
125+
});
126+
127+
it.each(FACES)('%s — a declared scale is the width', (_face, draw) => {
128+
expect(textOf(locale, draw(fn, 15750.5, { scale: 2 }))).toBe(`${SUM_TEXT[locale]}0`);
129+
});
130+
});
131+
132+
describe('count', () => {
133+
it.each(FACES)('%s — a count stays whole, and groups like any number', (_face, draw) => {
134+
const text = textOf(locale, draw('count', 12000));
135+
expect(text).toBe(COUNT_TEXT[locale]);
136+
expect(text).not.toMatch(/[.]\d/);
137+
});
138+
139+
it.each(FACES)('%s — a small count reads as it is', (_face, draw) => {
140+
expect(textOf(locale, draw('count', 42))).toBe('42');
141+
});
142+
});
143+
144+
describe.each(FUNCTIONS)('%s', (fn) => {
145+
// `count` and `sum` take `0` over an empty child set (objectstack's
146+
// `summaryEmptySetValue`): a stored zero is a value, not an empty one.
147+
it.each(FACES)('%s — 0 reads 0, not 0.00 and not the empty affordance', (_face, draw) => {
148+
const { container } = renderSession(locale, draw(fn, 0));
149+
expect(container.textContent).toBe('0');
150+
expect(affordance(container)).toBeNull();
151+
});
152+
153+
it.each(FACES)('%s — the number is not drawn in monospace', (_face, draw) => {
154+
const { container } = renderSession(locale, draw(fn, 15750.5));
155+
expect(container.querySelector('.font-mono')).toBeNull();
156+
});
157+
});
158+
});
159+
160+
it.each(FACES)('%s — the display locale reaches the formatter: de-DE groups with a dot', (_face, draw) => {
161+
const { container } = render(
162+
<LocalizationProvider value={{ locale: 'de-DE' }}>{draw('sum', 15750.5)}</LocalizationProvider>,
163+
);
164+
expect(container.textContent).toBe('15.750,5');
165+
});
166+
});
167+
168+
// ── the number field's own faces ──────────────────────────────────────────
169+
170+
describe("each summary face is the `number` field's face for the same value (objectui#11752)", () => {
171+
const VALUES: Array<[string, number, Extra]> = [
172+
['a fraction', 15750.5, {}],
173+
['a whole count', 12000, {}],
174+
['an average with no declared width', 10 / 3, {}],
175+
['a declared scale', 15750.5, { scale: 2 }],
176+
['a negative minimum', -42.25, {}],
177+
];
178+
179+
describe.each(LOCALES)('%s', (locale) => {
180+
it.each(VALUES)('%s', (_label, value, extra) => {
181+
for (const fn of FUNCTIONS) {
182+
expect(textOf(locale, formFace(fn, value, extra)), `form, ${fn}`).toBe(numberForm(locale, value, extra));
183+
expect(textOf(locale, cellFace(fn, value, extra)), `cell, ${fn}`).toBe(numberCell(locale, value, extra));
184+
}
185+
});
186+
});
187+
});
188+
189+
// ── empty and non-number values ───────────────────────────────────────────
190+
191+
describe('empty and non-number values read the cell text (objectui#11752)', () => {
192+
// `min` / `max` / `avg` stay `null` over an empty child set. The form face
193+
// read `value == null`, so `''` and `[]` drew a blank span with no
194+
// accessible name here and the shared affordance in the table.
195+
it.each(FACES)("%s — null, undefined, '' and [] draw the shared empty affordance", (_face, draw) => {
196+
for (const fn of FUNCTIONS) {
197+
for (const value of [null, undefined, '', []]) {
198+
const { container } = renderSession('en-US', draw(fn, value));
199+
expect(affordance(container), `${fn} holding ${JSON.stringify(value)}`).not.toBeNull();
200+
cleanup();
201+
}
202+
}
203+
});
204+
205+
// A summary declares no `returnType`, so only a JS number is a number: a
206+
// string of digits stays the text the cell prints.
207+
it.each(FACES)('%s — a string of digits stays text', (_face, draw) => {
208+
expect(textOf('en-US', draw('sum', '15750.5'))).toBe('15750.5');
209+
});
210+
});
211+
212+
// ── the equality leg ──────────────────────────────────────────────────────
213+
214+
/**
215+
* One stored value reads the same in the form and in the cell: every row is
216+
* an (aggregation function, value, declaration) triple, drawn in both faces
217+
* in both sessions, and the two texts are compared with each other.
218+
*/
219+
describe('one stored value, one reading: the form face equals the cell (objectui#11752)', () => {
220+
const ROWS: Array<[string, Fn, unknown, Extra]> = [
221+
['count, small', 'count', 42, {}],
222+
['count, large', 'count', 12000, {}],
223+
['count, empty child set', 'count', 0, {}],
224+
['sum, the card value', 'sum', 15750.5, {}],
225+
['sum, empty child set', 'sum', 0, {}],
226+
['sum, declared scale', 'sum', 15750.5, { scale: 2 }],
227+
['sum, declared ungrouped', 'sum', 15750.5, { useGrouping: false }],
228+
['avg, a repeating fraction', 'avg', 10 / 3, {}],
229+
['avg, declared scale', 'avg', 10 / 3, { scale: 2 }],
230+
['min, negative', 'min', -42.25, {}],
231+
['max, large', 'max', 1234567.891, {}],
232+
['max, numeric string', 'max', '15750.5', {}],
233+
];
234+
235+
describe.each(LOCALES)('%s', (locale) => {
236+
it.each(ROWS)('%s', (_label, fn, value, extra) => {
237+
const form = textOf(locale, formFace(fn, value, extra));
238+
const cell = textOf(locale, cellFace(fn, value, extra));
239+
expect(form).not.toBe('');
240+
expect(form).toBe(cell);
241+
});
242+
});
243+
});
Lines changed: 50 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,69 @@
11
import React from 'react';
22
import { EmptyValue } from '@object-ui/components';
3-
import type { SummaryFieldMetadata } from '@object-ui/types';
3+
import { useDisplayLocale } from '@object-ui/i18n';
4+
import { isEmptyValue } from '@object-ui/core';
45
import { FieldWidgetComponentProps } from './types.js';
6+
import { formatNumberFieldValue } from './number-format.js';
7+
import { coerceToSafeValue } from '../coerceToSafeValue.js';
58

69
/**
710
* SummaryField - Read-only aggregation field
811
* Values are aggregated from related records and cannot be edited
12+
*
13+
* ## One value, one reading (objectui#11752)
14+
*
15+
* This is the read-only FORM face of a roll-up; its TABLE face is the formula
16+
* cell, `FormulaCellRenderer` in this package's barrel, which `summary` is
17+
* registered to. The two used to format the value each by itself, so one
18+
* stored value read two ways: this face picked a format by the roll-up's
19+
* aggregation `function` (`count` as it arrived, the other four at two fixed
20+
* decimals), so a `sum` over `15750.5` read `15750.50`, ungrouped and in every
21+
* locale, beside the cell's `15,750.5`.
22+
*
23+
* This face now reads the value the way the cell reads a summary's value,
24+
* whatever the function:
25+
*
26+
* - the emptiness FLOOR (`isEmptyValue`) on the coerced value, as the cell
27+
* takes it, so `''` and `[]` read the shared affordance in both faces;
28+
* - a JS number: `formatNumberFieldValue`, the one call every number face
29+
* makes (the display locale's grouping, the width `resolveFieldScale`
30+
* answers for a declared `scale`, the value's natural precision when none
31+
* is declared). A `count` is a whole number, so it reads whole; an author
32+
* who wants a fixed width on a `sum` or an `avg` declares `scale`, and both
33+
* faces honour it.
34+
* - anything else: the coerced value as text, as the cell prints it. A
35+
* summary declares no `returnType`, so a string of digits is not a number.
36+
*
37+
* The rule is spelled here and in the cell, because the barrel and this lazily
38+
* loaded widget may not import each other (the reason `number-format.ts`
39+
* gives). `__tests__/summaryFaces.numberFace-11752.test.tsx` draws every
40+
* aggregation function in both faces and compares the two texts, so neither
41+
* can move alone.
942
*/
1043
export function SummaryField({ value, field, ...props }: FieldWidgetComponentProps<any>) {
11-
// The aggregation `function` of the spec's `summaryOperations` roll-up
12-
// (`{ object, field, function }`), the shape object metadata carries. It is
13-
// the ONLY spelling read: the snake_case `summary_type` this widget used to
14-
// read is retired (objectui#11070).
15-
const summaryType = (field as SummaryFieldMetadata | undefined)?.summaryOperations?.function ?? 'count';
44+
// Before the empty-value early return: the hook count must not change when a
45+
// value flips between null and set.
46+
const locale = useDisplayLocale();
1647

17-
if (value == null) {
48+
// The floor, on the coerced value, exactly as the formula cell takes it.
49+
// This read `value == null`, so `''` and `[]` drew a blank span with no
50+
// accessible name here and the shared affordance in the table.
51+
const safe = coerceToSafeValue(value);
52+
if (isEmptyValue(safe)) {
1853
return <EmptyValue className={props.className} />;
1954
}
2055

21-
let displayValue: string;
22-
if (summaryType === 'count') {
23-
displayValue = String(value);
24-
} else if (['sum', 'avg', 'min', 'max'].includes(summaryType)) {
25-
displayValue = typeof value === 'number' ? value.toFixed(2) : String(value);
26-
} else {
27-
displayValue = String(value);
28-
}
56+
// Only a JS number is a number, the cell's rule for a field with no
57+
// `returnType` (objectui#11683); the aggregation `function` decides nothing
58+
// here, so `summaryOperations` is not read.
59+
const text =
60+
typeof value === 'number' && !isNaN(value)
61+
? formatNumberFieldValue(value, field, locale)
62+
: String(safe);
2963

3064
return (
3165
<span className={`text-sm font-medium tabular-nums text-gray-700 ${props.className || ''}`}>
32-
{displayValue}
66+
{text}
3367
</span>
3468
);
3569
}

0 commit comments

Comments
 (0)