fix(fields): the percent edit widget reads scale for its fraction width (objectui#9568)

.changeset/9568-percent-widget-reads-scale.md

@@ -0,0 +1,56 @@
+---
+'@object-ui/fields': minor
+---
+
+The percent EDIT WIDGET reads `scale` for its fraction width, not `precision`
+(objectui#9568).
+
+`PercentField` took `precision` as the decimal-place count for three things at
+once: the readonly face's `toFixed` width, the number input's `step` attribute
+and the slider's step. `@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 a percent field carrying
+an accurate `decimal(p, s)` pair was padded out to the column's TOTAL width. A
+`decimal(10, 2)` percent field rendered `25.0000000000%` readonly and offered
+`step="0.0000000001"` while editing, with a slider that moved in `1e-10`
+increments. The step half is not cosmetic: it makes the control unusable for the
+value it is declared to edit.
+
+This is the correction `NumberField` beside it already carried for its own step,
+and the one objectui#9295 made on the read-only cell, the grid summary footer
+and the record summary chip. The edit widget is the face that did not move.
+
+**Behaviour moves in two directions**, for percent fields that declare these
+members:
+
+- A percent field declaring `scale` now honours it, in all three places.
+  Declaring `scale: 0` previously rendered `25.00%` and stepped by `0.01`; it
+  now renders `25%` and steps by `1`.
+- A percent field declaring `precision` no longer pads to it. Declaring
+  `precision: 10` previously rendered `25.0000000000%` and stepped by `1e-10`;
+  it now falls to the widget's own two-decimal default.
+
+**Migration.** Restate the intended fraction width as `scale`, 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.** The absent-`scale`
+width stays this widget's own two decimals (`12.35%`, `step="0.01"`), so the
+repair is invisible to metadata that declares nothing, and the whole-percent
+convention this widget detects from a declared `max` above 1 is untouched.
+
+⚠️ That default is deliberate and it is NOT the cell's: `PercentCellRenderer`
+spells the same absence as zero fraction digits. The two faces therefore still
+disagree for a field that declares nothing — `12%` in a grid cell against
+`12.35%` in the editor — as they did before this change. objectui#9568 declines
+to make widget-versus-cell agreement a premise, and closing that gap needs its
+own ruling rather than arriving as a side effect of moving the member that is
+read.
+
+`CurrencyField`'s own read of `precision` is untouched and is not the same
+shape: there the competing source is the currency's ISO 4217 minor-unit count,
+and objectui#4361 ruled an authored `precision` wins over that. It ruled nothing
+about `scale`. `CurrencyConfigSchema.precision` is a third surface again, with
+the opposite convention and its own `scale` alias, and the spec warns against
+conflating it with the field face.

packages/fields/src/__tests__/PercentField.scale-9568.test.tsx

@@ -0,0 +1,156 @@
+/**
+ * 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#9568 — the percent EDIT WIDGET read `precision` as a fraction
+ * width, so a `decimal(10, 2)` percent field showed `25.0000000000%` and
+ * stepped by `1e-10`.
+ *
+ * `@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 a decimal-place width comes
+ * from is `scale`. `NumberField` in the same directory already carries that
+ * correction for its step, and objectui#9295 made it on the read-only cell,
+ * the grid summary footer and the detail summary chip. This widget is the face
+ * that did not move, and the one the user TYPES into.
+ *
+ * ── Why the assertions are on the rendered control ──────────────────────
+ * The defect is what the user sees and what the spinner offers, so every row
+ * below goes through the component and reads the DOM it produced: the readonly
+ * span's text, and the `step` attribute the browser's spinner and keyboard
+ * affordances obey. The slider's step is not a DOM attribute, so it is driven
+ * instead — one keyboard increment, whose emitted value IS the step.
+ *
+ * ── What fails before the repair ────────────────────────────────────────
+ * Every `scale`-declaring row. `{ precision: 10, scale: 2 }` rendered
+ * `25.0000000000%` readonly, `step="0.0000000001"` editable, and a slider
+ * increment of `1e-10`; `{ scale: 0 }` was ignored outright because
+ * `precision` was absent, so the width fell to the widget's own 2.
+ *
+ * ── The ABSENT-`scale` case is a DECISION, not a leftover ───────────────
+ * It keeps this widget's own 2 (`12.35%`, `step="0.01"`), so the repair is
+ * invisible to a percent field that declares neither member — pinned below as
+ * a MUST-NOT-CHANGE control. ⚠️ `PercentCellRenderer` spells the same absence
+ * `0` and pins that, so the two faces still disagree when nothing is declared.
+ * objectui#9568 declines to make widget-versus-cell agreement a premise; this
+ * change neither widens nor closes that disagreement.
+ */
+import { describe, it, expect, vi, afterEach } from 'vitest';
+import React from 'react';
+import { render, screen, fireEvent, cleanup } from '@testing-library/react';
+import '@testing-library/jest-dom';
+
+import { PercentField } from '../widgets/PercentField';
+
+afterEach(() => cleanup());
+
+const percentField = (field: Record<string, unknown>) =>
+  ({ type: 'percent', name: 'rate', ...field }) as any;
+
+const renderReadonly = (value: number, field: Record<string, unknown> = {}) =>
+  render(
+    <PercentField value={value} onChange={() => {}} field={percentField(field)} readonly />,
+  );
+
+const renderEditable = (
+  value: number,
+  field: Record<string, unknown> = {},
+  onChange: (v: unknown) => void = () => {},
+) =>
+  render(
+    <PercentField value={value} onChange={onChange as any} field={percentField(field)} />,
+  );
+
+/** The width the readonly face renders, percent sign included. */
+const readonlyText = () => document.body.textContent ?? '';
+
+/** The granularity the browser's spinner and arrow keys obey. */
+const inputStep = () => screen.getByRole('spinbutton').getAttribute('step');
+
+describe('PercentField reads `scale`, not `precision` (objectui#9568)', () => {
+  it('does not pad a decimal(10, 2) percent field out to ten fraction digits', () => {
+    // The card's headline reading, on the readonly face.
+    renderReadonly(0.25, { precision: 10, scale: 2 });
+
+    expect(readonlyText()).toContain('25.00%');
+    expect(readonlyText()).not.toContain('25.0000000000%');
+  });
+
+  it('offers a usable spinner step for a decimal(10, 2) percent field', () => {
+    // The half that is not cosmetic: `1e-10` is not a granularity anybody can
+    // operate, and it is what the browser's arrow keys and spinner obeyed.
+    renderEditable(0.25, { precision: 10, scale: 2 });
+
+    expect(inputStep()).toBe('0.01');
+    expect(inputStep()).not.toBe('0.0000000001');
+  });
+
+  it('steps the slider by the same declared width as the input', () => {
+    // The slider's step is a prop, not an attribute, so it is DRIVEN: one
+    // ArrowRight from a stored 0.25 emits the next value one step away, in
+    // display magnitude (25 -> 25.01), stored back as a fraction. Before the
+    // repair the same keystroke emitted 0.25 + 1e-12.
+    const onChange = vi.fn();
+    renderEditable(0.25, { precision: 10, scale: 2 }, onChange);
+
+    fireEvent.keyDown(screen.getByRole('slider'), { key: 'ArrowRight' });
+
+    expect(onChange).toHaveBeenCalledTimes(1);
+    expect(onChange.mock.calls[0][0]).toBeCloseTo(0.2501, 6);
+  });
+
+  it('honours `scale: 0`, which truthiness would have dropped', () => {
+    // `scale: 0` is a valid declaration — a percent field that edits whole
+    // percents. Before the repair this field had NO declared width at all
+    // (`precision` absent), so it rendered two decimals and stepped by 0.01.
+    renderReadonly(0.25, { scale: 0 });
+    expect(readonlyText()).toContain('25%');
+    expect(readonlyText()).not.toContain('25.00%');
+    cleanup();
+
+    renderEditable(0.25, { scale: 0 });
+    expect(inputStep()).toBe('1');
+  });
+
+  it('ignores `precision` entirely when no `scale` is declared', () => {
+    // decimal(10, 0) — ten total digits. `precision` alone must move nothing,
+    // in either face.
+    renderReadonly(0.25, { precision: 10 });
+    expect(readonlyText()).toContain('25.00%');
+    expect(readonlyText()).not.toContain('25.0000000000%');
+    cleanup();
+
+    renderEditable(0.25, { precision: 10 });
+    expect(inputStep()).toBe('0.01');
+    expect(inputStep()).not.toBe('0.0000000001');
+  });
+
+  it('leaves a field declaring neither member exactly where it was', () => {
+    // MUST-NOT-CHANGE control: the absent-`scale` width stays this widget's
+    // own 2, so this repair is invisible to every field that declares nothing.
+    renderReadonly(0.12345);
+    expect(readonlyText()).toContain('12.35%');
+    cleanup();
+
+    renderEditable(0.12345);
+    expect(inputStep()).toBe('0.01');
+  });
+
+  it('applies the declared width to the whole-percent convention too', () => {
+    // A field declaring `max > 1` stores percentage POINTS, and that detection
+    // is untouched by this repair: a stored 25 is 25%, rendered at the width
+    // `scale` declares.
+    renderReadonly(25, { max: 100, precision: 10, scale: 1 });
+    expect(readonlyText()).toContain('25.0%');
+    expect(readonlyText()).not.toContain('25.0000000000%');
+    cleanup();
+
+    renderEditable(25, { max: 100, precision: 10, scale: 1 });
+    expect(inputStep()).toBe('0.1');
+  });
+});

packages/fields/src/widgets/PercentField.tsx

@@ -5,13 +5,54 @@ import { toDomProps } from './toDomProps.js';
 import { useBadInputRefusal, BadInputMessage, BAD_INPUT_BORDER } from './numberBadInput.js';
 
 /**
- * PercentField - Percentage input with configurable decimal precision
+ * PercentField - Percentage input whose decimal places follow the field's
+ * declared `scale` (see the read below)
  * Stores values as decimals (0-1) and displays as percentages (0-100%)
  * Includes a slider for interactive control.
  */
 export function PercentField({ value, onChange, field, readonly, error, className, ...props }: FieldWidgetComponentProps<number>) {
   const percentField = field as any;
-  const precision = percentField?.precision ?? 2;
+  /**
+   * Decimal places come from `scale`, NOT `precision` (objectui#9568) — the
+   * correction `NumberField` in this directory already carries, and the one
+   * objectui#9295 made on the read-only cell, the grid summary footer and the
+   * detail summary chip. This widget is the face that did not move.
+   *
+   * `@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)". Reading `precision` padded every
+   * value out to the column's TOTAL width, so a decimal(10, 2) percent field
+   * rendered `25.0000000000%` and offered a `1e-10` step — a step that is not
+   * a cosmetic defect but an unusable control for the value it is declared to
+   * edit.
+   *
+   * ⛔ NOT `CurrencyConfigSchema.precision`, which is a different surface with
+   * the opposite convention and its own `scale` alias; the spec warns against
+   * conflating the two at the field-face declaration itself. And ⛔ not
+   * `CurrencyField`'s read of `precision` either: there the competing source is
+   * the currency's own ISO 4217 minor-unit count, and objectui#4361 ruled an
+   * authored `precision` wins over THAT. It ruled nothing about `scale`, which
+   * a currency field's face does not carry a meaning for, and it pushed the
+   * contract question upstream rather than settling it here.
+   *
+   * `typeof`, not truthiness: `scale: 0` is a valid declaration (a percent
+   * field that edits whole percents) and `||` would silently drop it — the
+   * reason `NumberField` guards its own read the same way, and the guard the
+   * `max` read below already uses. `??` would keep a `0` too, but it also
+   * keeps a `scale: "2"` arriving from JSON metadata, and inventing a width
+   * from a string is the consumer-side guessing AGENTS.md #0.1 refuses.
+   *
+   * An ABSENT `scale` keeps this widget's own 2. The repair moves the MEMBER
+   * that is read and nothing else, so it is invisible to every percent field
+   * that declares neither member. ⚠️ Stated rather than papered over: that
+   * leaves this widget and `PercentCellRenderer` still disagreeing when
+   * nothing is declared — the cell spells the same absence `0` and pins it —
+   * which is a disagreement objectui#9568 explicitly declines to make a
+   * premise and this change neither widens nor closes. Whether the two faces
+   * should agree at all needs its own ruling, not a side effect of this one.
+   */
+  const declaredScale = percentField?.scale;
+  const scale = typeof declaredScale === 'number' ? declaredScale : 2;
 
   // Before the readonly return below: hooks are unconditional (objectui#6780).
   const { refusal, readBadInput } = useBadInputRefusal('12.5');
@@ -32,7 +73,7 @@ export function PercentField({ value, onChange, field, readonly, error, classNam
     if (value == null) return <EmptyValue />;
     return (
       <span className="text-sm font-medium tabular-nums">
-        {toDisplay(value).toFixed(precision)}%
+        {toDisplay(value).toFixed(scale)}%
       </span>
     );
   }
@@ -52,7 +93,7 @@ export function PercentField({ value, onChange, field, readonly, error, classNam
    * change event fires.
    *
    * Measured on Chromium 141.0.7390.37 (Playwright 1.62.1), driving THIS
-   * widget (fraction convention, `precision: 2`):
+   * widget (fraction convention, a two-decimal width):
    *
    * ```
    * typed  "12abc" -> box.value "12"    onChange(0.12)
@@ -122,8 +163,11 @@ export function PercentField({ value, onChange, field, readonly, error, classNam
     onChange(nextValue as any);
   };
 
-  // Derive slider step from precision so slider granularity matches the input
-  const sliderStep = Math.pow(10, -precision);
+  // Derive the slider step from the SAME `scale` the input and the readonly
+  // face take, so slider granularity matches the input and neither offers a
+  // value the other would round away (objectui#9568 kept the two tied on
+  // purpose — the decision is one member, read once).
+  const sliderStep = Math.pow(10, -scale);
 
   return (
     <div className="space-y-2">
@@ -137,7 +181,7 @@ export function PercentField({ value, onChange, field, readonly, error, classNam
           placeholder={percentField?.placeholder || '0'}
           disabled={readonly || props.disabled}
           className={cn('pr-8', refusal ? BAD_INPUT_BORDER : '', className)}
-          step={Math.pow(10, -precision).toFixed(precision)}
+          step={Math.pow(10, -scale).toFixed(scale)}
           // `refusal` is this widget's OWN reading, which no host can produce;
           // `error` keeps its single author (objectui#3222 / objectui#6716).
           aria-invalid={!!error || !!refusal}