From e76199c1c18b8d276bb04843f89b7d7547643baa Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 29 Aug 2026 15:05:20 +0000 Subject: [PATCH 1/2] fix(fields): announce bad input across the type="number" widget class MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `type="number"` widget could DISPLAY one value and store another with nothing said. Typing `1e` leaves Chromium visibly showing `1e` while `.value` reads the empty string, so the widget emitted `null`, `aria-invalid` stayed `"false"`, and no diagnostic was drawn. Announce when `e.target.validity.badInput` is true — the platform's own predicate, not a renderer-side dialect — across CurrencyField, PercentField, NumberField and GeolocationField as one class, reusing objectui#6716's refusal shape. Adds the blur arm the paste route needs: pasting `1e` into an empty box never moves `.value` off `''`, so React suppresses the change event entirely. The guard ANNOUNCES; it deliberately does not refuse. Refusing would leave `props.value` unchanged and React's `updateInput` would write it back over the raw text the diagnostic points at (measured in Chromium and read out of React 19.2.8's own source). Part of #6780 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49 --- ...mberInputWidgets.badInputAnnounce.test.tsx | 392 ++++++++++++++++++ ...nputWidgets.environmentDivergence.test.tsx | 67 ++- .../__tests__/numberInputBrowserReadings.ts | 145 +++++++ packages/fields/src/widgets/CurrencyField.tsx | 109 +++-- .../fields/src/widgets/GeolocationField.tsx | 56 ++- packages/fields/src/widgets/NumberField.tsx | 70 +++- packages/fields/src/widgets/PercentField.tsx | 53 ++- .../fields/src/widgets/numberBadInput.tsx | 135 ++++++ 8 files changed, 936 insertions(+), 91 deletions(-) create mode 100644 packages/fields/src/__tests__/NumberInputWidgets.badInputAnnounce.test.tsx create mode 100644 packages/fields/src/__tests__/numberInputBrowserReadings.ts create mode 100644 packages/fields/src/widgets/numberBadInput.tsx diff --git a/packages/fields/src/__tests__/NumberInputWidgets.badInputAnnounce.test.tsx b/packages/fields/src/__tests__/NumberInputWidgets.badInputAnnounce.test.tsx new file mode 100644 index 0000000000..d675d99c35 --- /dev/null +++ b/packages/fields/src/__tests__/NumberInputWidgets.badInputAnnounce.test.tsx @@ -0,0 +1,392 @@ +/** + * 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#6780 — a `type="number"` widget may no longer DISPLAY one value and + * store another with nothing said. + * + * Ruled 2026-08-29 (option A): announce when `e.target.validity.badInput` is + * true, across the whole `type="number"` widget CLASS as one change — + * `CurrencyField`, `PercentField`, `NumberField`, `GeolocationField` — reusing + * objectui#6716's refusal shape. + * + * ## What makes this a real fix and not a second no-op + * + * objectui#6715's anchored guard was measured to be a provable no-op on this + * surface: it rejected only strings the test environment fabricates. This guard + * was measured the other way round, in Chromium 141.0.7390.37 via Playwright, + * typing each string key by key into a real number input: + * + * - it FIRES on nine keyboard-reachable states (`1e`, `1e-`, `1e+`, `5e`, `-`, + * `.`, `+`, `-.`, `e`), each of which leaves `.value` at `''` while the box + * keeps DISPLAYING the text — confirmed by screenshot-comparing the box + * against an untouched one; + * - it NEVER fires on anything a real browser puts in `.value` + * ({@link BROWSER_READINGS}, all six `badInput === false`). + * + * ## ⚠️ Why this file drives only four of those nine + * + * happy-dom and Chromium do NOT agree about `badInput` in general — see the + * matrix in `numberInputBrowserReadings.ts`. A unit test here may drive only + * the strings where happy-dom's programmatic verdict matches Chromium's typed + * verdict ({@link BAD_INPUT_AGREED}). Driving `0x10` or `12abc` would go green + * over a branch the product never executes, which is the failure objectui#6765 + * exists to prevent; driving `-` or `.` would go red over behaviour that is + * correct in the product. The other five are product behaviour this environment + * cannot reproduce, and they are covered by the browser measurement only. + * + * ## ⛔ What is deliberately NOT asserted here + * + * That the widget refuses the edit. It does not — objectui#6716's shape refuses + * and this one only announces, because refusing would make React restore the + * control and wipe the very text the diagnostic points at. The emission + * assertions below pin that on purpose. + */ + +import { describe, it, expect, vi, afterEach } from 'vitest'; +import React from 'react'; +import { render, fireEvent, cleanup, within } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { CurrencyField } from '../widgets/CurrencyField'; +import { PercentField } from '../widgets/PercentField'; +import { NumberField } from '../widgets/NumberField'; +import { GeolocationField } from '../widgets/GeolocationField'; +import { badInputMessage } from '../widgets/numberBadInput'; +import { + BROWSER_READINGS, + BAD_INPUT_AGREED, + BAD_INPUT_AGREED_CLEAN, + CHROMIUM_KEYBOARD_REACHABLE_BAD_INPUT, +} from './numberInputBrowserReadings.js'; + +afterEach(() => cleanup()); + +/** + * The four widgets of the class, each with the example its own message quotes. + * + * `mount` returns the box the guard watches plus the `onChange` spy. The host + * ECHOES the emission back into `value` the way a real form does — with a bare + * spy the control never moves off its initial value and React suppresses the + * second change (the same coupling the objectui#6765 suite needed). + */ +const WIDGETS = [ + { + name: 'CurrencyField', + example: '1234.56', + mount: () => { + const onChange = vi.fn(); + const Host = () => { + const [value, setValue] = React.useState(null); + return ( + { onChange(v); setValue(v); }) as any} + field={{ name: 'amount', type: 'currency', currency: 'USD', precision: 2 } as any} + /> + ); + }; + const { container } = render(); + return { container, onChange, box: container.querySelector('input[type=number]') as HTMLInputElement }; + }, + }, + { + name: 'PercentField', + example: '12.5', + mount: () => { + const onChange = vi.fn(); + const Host = () => { + const [value, setValue] = React.useState(null); + return ( + { onChange(v); setValue(v); }) as any} + field={{ name: 'rate', type: 'percent', precision: 2 } as any} + /> + ); + }; + const { container } = render(); + return { container, onChange, box: container.querySelector('input[type=number]') as HTMLInputElement }; + }, + }, + { + name: 'NumberField', + example: '1234', + mount: () => { + const onChange = vi.fn(); + const Host = () => { + const [value, setValue] = React.useState(null); + return ( + { onChange(v); setValue(v); }) as any} + field={{ name: 'qty', type: 'number' } as any} + /> + ); + }; + const { container } = render(); + return { container, onChange, box: container.querySelector('input[type=number]') as HTMLInputElement }; + }, + }, + { + name: 'GeolocationField (latitude)', + example: '30.2741', + mount: () => { + const onChange = vi.fn(); + const Host = () => { + const [value, setValue] = React.useState({}); + return ( + { onChange(v); setValue(v); }) as any} + field={{ name: 'where', type: 'geolocation' } as any} + /> + ); + }; + const { container } = render(); + return { container, onChange, box: container.querySelectorAll('input[type=number]')[0] as HTMLInputElement }; + }, + }, +] as const; + +/** The drawn diagnostic, read the way a person reads it. */ +const diagnostic = (container: HTMLElement): string | null => { + const p = container.querySelector('p.text-red-500'); + return p ? (p.textContent || '').trim() : null; +}; + +/* -------------------------------------------------------------------------- */ +/* 1. The announcement itself — objectui#6716's shape, on all four widgets. */ +/* -------------------------------------------------------------------------- */ + +describe.each(WIDGETS)('$name announces bad input (objectui#6780)', ({ example, mount }) => { + it('is silent before anything is typed', () => { + const { container, box } = mount(); + expect(diagnostic(container)).toBeNull(); + expect(box.getAttribute('aria-invalid')).not.toBe('true'); + }); + + it.each(BAD_INPUT_AGREED)('marks the control invalid and says why, for %s', text => { + const { container, box } = mount(); + fireEvent.change(box, { target: { value: text } }); + + // The a11y state a screen reader reads... + expect(box).toHaveAttribute('aria-invalid', 'true'); + // ...and a reason a person can read, in objectui#6716's `Not saved:` shape. + expect(diagnostic(container)).toBe(badInputMessage(example)); + expect(diagnostic(container)).toContain('Not saved:'); + }); + + it('marks the refused control with the shared refusal border', () => { + const { box } = mount(); + fireEvent.change(box, { target: { value: '1e' } }); + expect(box.className).toContain('border-red-500'); + }); + + it.each(BAD_INPUT_AGREED_CLEAN)('stays silent for %s, which the browser can read', text => { + const { container, box } = mount(); + fireEvent.change(box, { target: { value: text } }); + expect(diagnostic(container)).toBeNull(); + expect(box.getAttribute('aria-invalid')).not.toBe('true'); + }); + + it('clears the announcement once the entry is corrected', () => { + const { container, box } = mount(); + fireEvent.change(box, { target: { value: '1e' } }); + expect(diagnostic(container)).not.toBeNull(); + + fireEvent.change(box, { target: { value: '12' } }); + expect(diagnostic(container)).toBeNull(); + expect(box.getAttribute('aria-invalid')).not.toBe('true'); + }); + + /* ---- the blur arm ---- */ + + it('announces on BLUR, the route that fires no change event at all', () => { + // The measured paste case: pasting `1e` into an EMPTY box moves `.value` + // from `''` to `''`, so React's input-value tracking suppresses the + // synthetic onChange entirely — one DOM `input` event fires and no + // `onChange` reaches the widget. Modelled here by putting the box in that + // state WITHOUT a change event, then blurring. `badInput` is still true at + // blur time in Chromium, which is what makes this arm work. + const { container, box, onChange } = mount(); + box.value = '1e'; + expect(onChange).not.toHaveBeenCalled(); + expect(diagnostic(container)).toBeNull(); + + fireEvent.blur(box); + + expect(diagnostic(container)).toBe(badInputMessage(example)); + expect(box).toHaveAttribute('aria-invalid', 'true'); + }); + + it('a blur on a readable box says nothing', () => { + const { container, box } = mount(); + box.value = '12'; + fireEvent.blur(box); + expect(diagnostic(container)).toBeNull(); + }); +}); + +/* -------------------------------------------------------------------------- */ +/* 2. The guard ANNOUNCES; it does not refuse. Pinned deliberately. */ +/* -------------------------------------------------------------------------- */ + +describe('objectui#6780 announces without changing what is emitted', () => { + it('CurrencyField still emits, exactly as it did before the guard', () => { + const { box, onChange } = WIDGETS[0].mount(); + fireEvent.change(box, { target: { value: '1e' } }); + // happy-dom leaves `.value` as `'1e'`, so `parseFloat` yields 1 here; a real + // Chromium hands the same handler `''` and it emits `null`. BOTH are the + // pre-guard emission — the point is that the guard did not suppress it. + expect(onChange).toHaveBeenCalledTimes(1); + }); + + it('a cleared box is still a cleared box, and still silent', () => { + // `''` is `badInput === false` in both engines. Announcing on it would fire + // on every deletion, which is why it is in the CLEAN list. + const { container, box, onChange } = WIDGETS[0].mount(); + fireEvent.change(box, { target: { value: '5' } }); + fireEvent.change(box, { target: { value: '' } }); + expect(onChange).toHaveBeenLastCalledWith(null); + expect(diagnostic(container)).toBeNull(); + }); +}); + +/* -------------------------------------------------------------------------- */ +/* 3. Not a no-op — the property that separates this from objectui#6715. */ +/* -------------------------------------------------------------------------- */ + +describe('objectui#6780 fires on input a real browser can produce', () => { + it('never fires on any value a real Chromium was measured to emit', () => { + // The anti-no-op assertion, read off the SAME measured list the + // objectui#6765 suite pins, so there is no second dialect and the claim + // moves if the measurement moves. + const input = document.createElement('input'); + input.type = 'number'; + for (const reading of BROWSER_READINGS) { + input.value = reading; + expect( + input.validity.badInput, + `${JSON.stringify(reading)} came out of a real Chromium number input, ` + + 'so the guard must not refuse it', + ).toBe(false); + } + }); + + it('the keyboard-reachable bad-input list is non-empty and contains the reproduced defect', () => { + // objectui#6715's guard was a no-op because its "would reject" set was + // empty for anything reachable. This one's is not: nine states, measured by + // typing them into a real Chromium. + expect(CHROMIUM_KEYBOARD_REACHABLE_BAD_INPUT.length).toBe(9); + expect(CHROMIUM_KEYBOARD_REACHABLE_BAD_INPUT).toContain('1e'); + // And every string this suite drives is one of them, so nothing here is + // asserted about a state a user cannot reach. + for (const t of BAD_INPUT_AGREED) { + expect(CHROMIUM_KEYBOARD_REACHABLE_BAD_INPUT).toContain(t); + } + }); +}); + +/* -------------------------------------------------------------------------- */ +/* 4. The composite: two boxes, two independent readings. */ +/* -------------------------------------------------------------------------- */ + +describe('GeolocationField reads its two boxes independently (objectui#6780)', () => { + const mountGeo = () => { + const onChange = vi.fn(); + const Host = () => { + const [value, setValue] = React.useState({}); + return ( + { onChange(v); setValue(v); }) as any} + field={{ name: 'where', type: 'geolocation' } as any} + /> + ); + }; + const { container } = render(); + const boxes = container.querySelectorAll('input[type=number]'); + return { container, lat: boxes[0] as HTMLInputElement, lng: boxes[1] as HTMLInputElement }; + }; + + it('bad latitude does not mark the longitude box invalid', () => { + const { container, lat, lng } = mountGeo(); + fireEvent.change(lat, { target: { value: '1e' } }); + + expect(lat).toHaveAttribute('aria-invalid', 'true'); + expect(lng).toHaveAttribute('aria-invalid', 'false'); + expect(container.querySelectorAll('p.text-red-500')).toHaveLength(1); + }); + + it('names each coordinate with its own example', () => { + const { container, lat, lng } = mountGeo(); + fireEvent.change(lat, { target: { value: '1e' } }); + fireEvent.change(lng, { target: { value: '1e' } }); + + const messages = Array.from(container.querySelectorAll('p.text-red-500')).map( + p => (p.textContent || '').trim(), + ); + expect(messages).toEqual([badInputMessage('30.2741'), badInputMessage('120.1551')]); + }); + + it('the longitude box announces on blur too', () => { + const { container, lng } = mountGeo(); + lng.value = '1e'; + fireEvent.blur(lng); + expect(diagnostic(container)).toBe(badInputMessage('120.1551')); + }); +}); + +/* -------------------------------------------------------------------------- */ +/* 5. The new blur arms must not eat the host's own onBlur. */ +/* -------------------------------------------------------------------------- */ + +describe('the added onBlur composes the host handler instead of replacing it', () => { + /** + * `onBlur` is a DECLARED DOM pass-through key (`FieldWidgetDomProps`), so + * `toDomProps` delivers a host's handler onto these controls. The three + * widgets that gained an `onBlur` here write it AFTER that spread, so without + * composition they would silently drop it — this package's + * DECLARED-BUT-NOT-DELIVERED class (objectui#3290 / objectui#3222). + * + * ⛔ `CurrencyField` is deliberately absent. It has overridden the host's + * `onBlur` since long before this card, no host in this repo passes one + * today, and changing that is an unmeasured behaviour change outside this + * card's ruling. Filed separately rather than folded in here. + */ + it.each([ + ['PercentField', PercentField, { name: 'rate', type: 'percent' }], + ['NumberField', NumberField, { name: 'qty', type: 'number' }], + ['GeolocationField', GeolocationField, { name: 'where', type: 'geolocation' }], + ])('%s still calls a host onBlur', (_name, Widget: any, field: any) => { + const hostBlur = vi.fn(); + const { container } = render( + {}} field={field} onBlur={hostBlur} />, + ); + const box = container.querySelector('input[type=number]') as HTMLInputElement; + fireEvent.blur(box); + expect(hostBlur).toHaveBeenCalledTimes(1); + }); + + it('and still announces on that same blur', () => { + const hostBlur = vi.fn(); + const { container } = render( + {}} + field={{ name: 'qty', type: 'number' } as any} + onBlur={hostBlur} + />, + ); + const box = container.querySelector('input[type=number]') as HTMLInputElement; + box.value = '1e'; + fireEvent.blur(box); + expect(hostBlur).toHaveBeenCalledTimes(1); + expect(within(container).getByText(badInputMessage('1234'))).toBeInTheDocument(); + }); +}); diff --git a/packages/fields/src/__tests__/NumberInputWidgets.environmentDivergence.test.tsx b/packages/fields/src/__tests__/NumberInputWidgets.environmentDivergence.test.tsx index 68ec69fbfa..d3c97b9243 100644 --- a/packages/fields/src/__tests__/NumberInputWidgets.environmentDivergence.test.tsx +++ b/packages/fields/src/__tests__/NumberInputWidgets.environmentDivergence.test.tsx @@ -74,18 +74,15 @@ import { CurrencyField } from '../widgets/CurrencyField'; import { PercentField } from '../widgets/PercentField'; /** - * Every non-empty string a real Chromium was observed to place in - * `e.target.value` for a `type="number"` input, across all three delivery - * routes and all the inputs this card drove. MEASURED, not enumerated from the - * spec. + * The measured record moved to `numberInputBrowserReadings.ts` (objectui#6780) + * so this suite and the announcement suite read ONE set of numbers. Nothing + * about the readings changed; they are the same values this file pinned. */ -const BROWSER_READINGS = ['12', '1.23', '010', '15', '1', '12.345'] as const; - -/** - * Strings only the TEST environment can produce, because happy-dom skips the - * sanitization. These are exactly the inputs the filing card measured. - */ -const HAPPY_DOM_FABRICATIONS = ['12abc', '1.2.3', '0x10', '1e'] as const; +import { + BROWSER_READINGS, + HAPPY_DOM_FABRICATIONS, + BAD_INPUT_DISAGREEMENTS, +} from './numberInputBrowserReadings.js'; /** * A host that ECHOES the emission back into `value`, the way a real form does. @@ -135,9 +132,16 @@ describe('objectui#6765 — happy-dom does not sanitize a type="number" input', }); it('still reports validity.badInput, so it half-implements the algorithm', () => { - // Load-bearing, not trivia: `validity.badInput` is the ONE signal about - // unreadable text that agrees between this environment and the browser, so - // it is the only guard on this surface a unit test could honestly oracle. + // Load-bearing, not trivia: `validity.badInput` is the signal objectui#6780 + // built its guard on, because it is the PLATFORM's own predicate rather + // than a renderer-side dialect. + // + // ⚠️ This file used to add "the ONE signal that agrees between this + // environment and the browser". objectui#6780 measured that and it is too + // strong — see `BAD_INPUT_DISAGREEMENTS` and the matrix above it. The + // agreement is between happy-dom's PROGRAMMATIC verdict and Chromium's + // TYPED verdict, it holds for a subset, and in Chromium `badInput` is never + // true for a script write at all. The corrected statement is pinned below. const input = document.createElement('input'); input.type = 'number'; for (const text of HAPPY_DOM_FABRICATIONS) { @@ -147,6 +151,23 @@ describe('objectui#6765 — happy-dom does not sanitize a type="number" input', input.value = ''; expect(input.validity.badInput).toBe(false); }); + + it('does NOT agree with Chromium on badInput for eight measured inputs', () => { + // objectui#6780's correction, pinned so the overstated one-liner cannot + // come back. Each row is this environment's own reading; `chromiumTyped` is + // the browser's, measured by typing the string key by key. + const input = document.createElement('input'); + input.type = 'number'; + for (const row of BAD_INPUT_DISAGREEMENTS) { + input.value = row.input; + expect( + input.validity.badInput, + `happy-dom's reading of ${JSON.stringify(row.input)} moved; the ` + + 'agreement matrix in numberInputBrowserReadings.ts is now stale', + ).toBe(row.happyDom); + expect(row.happyDom).not.toBe(row.chromiumTyped); + } + }); }); describe('objectui#6765 — a whole-string guard has nothing left to reject here', () => { @@ -157,8 +178,10 @@ describe('objectui#6765 — a whole-string guard has nothing left to reject here * file to export it; copying its source here would create exactly the second * dialect of "what a number is" that AGENTS.md #0.1 forbids. `badInput` is * the better oracle anyway — it is the browser stating, in its own words, - * whether it can read the text — and it is the ONE signal on this surface - * that happy-dom and Chromium were both measured to agree on. + * whether it can read the text. ⚠️ objectui#6780 narrowed the claim that used + * to end this paragraph ("the ONE signal happy-dom and Chromium agree on"): + * they agree on a measured SUBSET, and in Chromium `badInput` is never true + * for a programmatic write at all. See `BAD_INPUT_DISAGREEMENTS`. */ it('the platform already reads every measured browser value as a whole number', () => { const input = document.createElement('input'); @@ -245,10 +268,14 @@ describe('objectui#6765 — the oracle and the product disagree, per widget', () // the empty string is the browser's only channel for "text I am showing but // cannot read", and these widgets answer it silently. // - // ⚠️ This pins that the drop IS silent so that a fix goes red here and its - // author reads this file. It is NOT an endorsement — objectui#6765 escalated - // whether to announce it, because the same drop belongs to every - // `type="number"` widget in this package, not to these two. + // ⚠️ objectui#6780 landed the announcement and this pin deliberately did + // NOT go red, which is itself the finding: it drives `''`, and an empty box + // is `badInput === false` in BOTH engines. Clearing a field is a CLEARED + // field, not unreadable text, and it must stay silent — announcing here + // would fire on every deletion. The route that is now announced is the one + // this environment cannot reach: in Chromium the user types `1e`, the box + // keeps DISPLAYING it, `.value` reads `''` AND `badInput` is true. See + // `NumberInputWidgets.badInputAnnounce.test.tsx`. for (const widget of ['currency', 'percent'] as const) { const { input: el, onChange } = renderHost(widget); fireEvent.change(el, { target: { value: '5' } }); diff --git a/packages/fields/src/__tests__/numberInputBrowserReadings.ts b/packages/fields/src/__tests__/numberInputBrowserReadings.ts new file mode 100644 index 0000000000..73a6a6d24d --- /dev/null +++ b/packages/fields/src/__tests__/numberInputBrowserReadings.ts @@ -0,0 +1,145 @@ +/** + * 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. + */ + +/** + * The MEASURED record for `type="number"` widgets, in ONE place + * (objectui#6765 measured the emissions, objectui#6780 measured `badInput`). + * + * Not a `.test.ts` file: it holds no assertions, only the numbers, so the two + * suites that read it cannot drift into two dialects of "what a browser does". + * The vitest project globs are `packages/**\/*.test.ts(x)`, so nothing here is + * collected as a test. + * + * Chromium 141.0.7390.37 via Playwright 1.62.1, `executablePath: + * '/opt/pw-browsers/chromium'`, a real page served over `http://127.0.0.1` + * (a secure context, so the real clipboard works), driven key by key. + * happy-dom 20.11.2, this package's test environment. + */ + +/** + * Every non-empty string a real Chromium was observed to place in + * `e.target.value` for a `type="number"` input, across keystrokes, real + * clipboard paste and programmatic set. MEASURED, not enumerated from the spec. + */ +export const BROWSER_READINGS = ['12', '1.23', '010', '15', '1', '12.345'] as const; + +/** + * Strings only the TEST environment can produce, because happy-dom does not + * implement the HTML value-sanitization algorithm (objectui#6765). + */ +export const HAPPY_DOM_FABRICATIONS = ['12abc', '1.2.3', '0x10', '1e'] as const; + +/* -------------------------------------------------------------------------- */ +/* objectui#6780 — the `badInput` agreement, measured rather than assumed. */ +/* -------------------------------------------------------------------------- */ + +/** + * ⚠️ CORRECTION TO THE RECORD (objectui#6780, measured on `d06059f24`). + * + * objectui#6765 / PR #6777 recorded that `validity.badInput` is "the ONE signal + * about unreadable text that agrees between this environment and the browser". + * That is TRUE for the case the card reproduced, and it is the reason option A + * is testable at all — but as a general statement it is too strong, and the + * difference decides which strings a unit test may honestly drive. + * + * The full matrix, measured on both engines: + * + * ``` + * Chromium TYPED Chromium .value=x happy-dom .value=x + * input .value badInput .value badInput .value badInput + * "1e" "" TRUE "" false "1e" TRUE + * "1e-" "" TRUE "" false "1e-" TRUE + * "1e+" "" TRUE "" false "1e+" TRUE + * "5e" "" TRUE "" false "5e" TRUE + * "-" "" TRUE "" false "" false <- + * "." "" TRUE "" false "" false <- + * "+" "" TRUE "" false "" false <- + * "-." "" TRUE "" false "" false <- + * "e" "" TRUE "" false "" false <- + * "1." "1" false "" false "1." TRUE <- + * "1e5" "1e5" false "1e5" false "1e5" TRUE <- + * "1.2.3" "1.23" false "" false "1.2.3" TRUE <- + * "0x10" "010" false "" false "0x10" TRUE <- + * "12abc" "12" false "" false "12abc" TRUE <- + * "" "" false "" false "" false + * "12" "12" false "12" false "12" false + * ``` + * + * Two facts the old one-liner hides: + * + * 1. ⭐ **In Chromium, `badInput` is NEVER true for a programmatic `.value` + * write.** Per the HTML definition it reports that the UA cannot convert + * *the user's input*; a script write has no user input to fail on. So on + * the route a unit test actually has — `fireEvent.change`, which sets + * `.value` and dispatches — Chromium answers `false` for every string, + * and happy-dom answers `true` for nine of them. On the SAME route the two + * engines agree on nothing. + * 2. The agreement that does exist, and that this guard rests on, is between + * happy-dom's programmatic verdict and Chromium's TYPED verdict. It holds + * for a real subset, and fails for eight inputs (marked `<-`). + * + * ⇒ A unit test for the announcement may drive ONLY strings where those two + * verdicts match. That is what the two lists below are for. Driving `0x10` or + * `12abc` would make a happy-dom test go green over a branch the product never + * executes — the exact failure objectui#6765 exists to prevent. + */ + +/** + * Strings where happy-dom's `.value = x` and Chromium's TYPED verdict BOTH say + * `badInput`. The only inputs a unit test may use to drive the announcement. + */ +export const BAD_INPUT_AGREED = ['1e', '1e-', '1e+', '5e'] as const; + +/** + * Strings where both engines agree there is NO bad input — so the guard must + * stay quiet. `''` is included deliberately: an empty box is a CLEARED field, + * not unreadable text, and announcing there would fire on every deletion. + */ +export const BAD_INPUT_AGREED_CLEAN = ['12', '1.23', '010', '15', '1', '12.345', ''] as const; + +/** + * The eight inputs where the two engines DISAGREE about `badInput`, pinned so + * that nobody later "extends coverage" by adding one to the lists above. + * + * `chromiumTyped` is what a real Chromium reports when a user types the string + * key by key; `happyDom` is what this test environment reports for + * `input.value = string`. + */ +export const BAD_INPUT_DISAGREEMENTS: ReadonlyArray<{ + input: string; + chromiumTyped: boolean; + happyDom: boolean; +}> = [ + // Chromium: a lone sign / dot / exponent letter is unreadable USER input. + // happy-dom sanitizes them away to `''` and then sees nothing wrong. + { input: '-', chromiumTyped: true, happyDom: false }, + { input: '.', chromiumTyped: true, happyDom: false }, + { input: '+', chromiumTyped: true, happyDom: false }, + { input: '-.', chromiumTyped: true, happyDom: false }, + { input: 'e', chromiumTyped: true, happyDom: false }, + // The other direction: happy-dom keeps residue and calls it bad, while a + // real browser filtered or accepted the same keystrokes long before. + { input: '1.', chromiumTyped: false, happyDom: true }, + { input: '1e5', chromiumTyped: false, happyDom: true }, + { input: '12abc', chromiumTyped: false, happyDom: true }, +]; + +/** + * Every state a real keyboard can reach that leaves Chromium unable to read the + * box — MEASURED by typing each one key by key into an empty `type="number"` + * input. This is the list that makes the guard a real fix rather than + * objectui#6715's provable no-op: all nine are reachable by a user, and the box + * VISIBLY displays them (screenshot-compared against an untouched box) while + * `.value` reads `''`. + * + * ⚠️ Only the first four are unit-testable (see {@link BAD_INPUT_AGREED}); the + * rest are product behaviour this environment cannot reproduce. + */ +export const CHROMIUM_KEYBOARD_REACHABLE_BAD_INPUT = [ + '1e', '1e-', '1e+', '5e', '-', '.', '+', '-.', 'e', +] as const; diff --git a/packages/fields/src/widgets/CurrencyField.tsx b/packages/fields/src/widgets/CurrencyField.tsx index c09584b565..8efa4e3680 100644 --- a/packages/fields/src/widgets/CurrencyField.tsx +++ b/packages/fields/src/widgets/CurrencyField.tsx @@ -1,9 +1,10 @@ import React from 'react'; -import { Input, EmptyValue } from '@object-ui/components'; +import { Input, EmptyValue, cn } from '@object-ui/components'; import { FieldWidgetComponentProps } from './types.js'; import { toDomProps } from './toDomProps.js'; import { useLocalization, useDisplayLocale, formatDisplayNumber } from '@object-ui/i18n'; import { resolveFieldCurrency, currencyFractionDigits, currencySymbol } from '../currency.js'; +import { useBadInputRefusal, BadInputMessage, BAD_INPUT_BORDER } from './numberBadInput.js'; /** * Format currency value for display. When `currency` is undefined the value @@ -75,6 +76,9 @@ export function CurrencyField({ value, onChange, field, readonly, error, classNa const precision = currencyField?.precision ?? (currency ? currencyFractionDigits(currency) : 2); + // Before the readonly return: hooks are unconditional (objectui#6780). + const { refusal, readBadInput } = useBadInputRefusal('1234.56'); + if (readonly) { if (value == null) return ; return ( @@ -116,17 +120,36 @@ export function CurrencyField({ value, onChange, field, readonly, error, classNa * truncation no user reaches. The oracle-vs-product table is pinned in * `__tests__/NumberInputWidgets.environmentDivergence.test.tsx`. * - * ⚠️ OPEN (objectui#6765): the last row is a SILENT drop. The box still - * DISPLAYS `1e` while `.value` reads `''`, so this widget emits `null`, - * `aria-invalid` stays `false` and no diagnostic is drawn — objectui#6716's - * class, measured here rather than assumed. It is NOT fixed here: the same - * drop belongs to every `type="number"` widget in this package - * (`NumberField`, `GeolocationField`), and the truncating rows above cannot - * be refused by any widget-side guard at all, so the route out was escalated - * rather than applied to two widgets of the four. + * ⭐ CLOSED for the last row (objectui#6780, ruled 2026-08-29): the silent + * drop is now ANNOUNCED, through `validity.badInput` — the platform's own + * predicate — across all four `type="number"` widgets of this package as one + * change. See `numberBadInput.tsx` for the measurement and for why the + * emission itself is deliberately unchanged. + * + * ⛔ STILL SILENT, and deliberately so: the TRUNCATING rows above. `1.2.3` + * stores `1.23` and `0x10` stores `10`, and no widget-side guard can refuse + * them — the browser filtered the keystrokes before `handleChange` ran, so + * the information is gone before any code here can see it. Only abandoning + * `type="number"` would recover it, which the same ruling declined (it would + * reverse objectui#2572's min/max/step and mobile-keyboard affordances). + * ⚠️ This asymmetry is documented for USERS, not just here — a control that + * warns about `1e` but silently truncates `1.2.3` teaches people that no + * warning means the value is right. See `content/docs/guide/fields.md`. */ // Parse and format on blur to ensure valid currency format const handleBlur = (e: React.FocusEvent) => { + // objectui#6780: the blur arm. React delivers no `onChange` when `.value` + // never leaves `''` — the measured shape of PASTING `1e` into an empty box + // — and `badInput` is still true at blur time, so this is the only arm that + // sees that route. + // + // ⚠️ This widget's `onBlur` has ALWAYS overridden the host's (it is written + // after the `toDomProps` spread), and that is left exactly as it was: no + // host in this repo passes `onBlur` to a field widget today (the data-table + // inline editor uses a document-level pointerdown listener instead), so + // composing it here would be an unmeasured behaviour change outside this + // card's ruling. Filed separately. + readBadInput(e.target); const val = parseFloat(e.target.value); if (!isNaN(val)) { onChange(parseFloat(val.toFixed(precision))); @@ -145,36 +168,44 @@ export function CurrencyField({ value, onChange, field, readonly, error, classNa const symbol = currency ? currencySymbol(currency, locale) : ''; return ( -
- {symbol && ( - - {symbol} - - )} - { - // Bare `parseFloat`, deliberately — see the measured note on - // `handleBlur` above (objectui#6765). The empty string is the - // browser's ONLY refusal channel on a number input, and it is also - // how it reports text it is still displaying but cannot read. - const val = e.target.value === '' ? null : parseFloat(e.target.value); - onChange(val as any); - }} - onBlur={handleBlur} - placeholder={currencyField?.placeholder || '0.00'} - disabled={readonly || props.disabled} - className={`${symbol ? 'pl-8' : ''} ${className || ''}`} - // Surface the field's declared range (e.g. `min: 0` on a budget) so the - // browser's spinner/keyboard affordances respect it (objectui#2572); - // server-side validation still owns enforcement. - min={typeof currencyField?.min === 'number' ? currencyField.min : undefined} - max={typeof currencyField?.max === 'number' ? currencyField.max : undefined} - step={Math.pow(10, -precision).toFixed(precision)} - aria-invalid={!!error} - /> +
+
+ {symbol && ( + + {symbol} + + )} + { + // Bare `parseFloat`, deliberately — see the measured note on + // `handleBlur` above (objectui#6765). The empty string is the + // browser's ONLY refusal channel on a number input, and it is also + // how it reports text it is still displaying but cannot read — which + // is why objectui#6780 asks the browser directly instead, and why the + // emission below is unchanged by that reading. + readBadInput(e.target); + const val = e.target.value === '' ? null : parseFloat(e.target.value); + onChange(val as any); + }} + onBlur={handleBlur} + placeholder={currencyField?.placeholder || '0.00'} + disabled={readonly || props.disabled} + className={cn(symbol ? 'pl-8' : '', refusal ? BAD_INPUT_BORDER : '', className)} + // Surface the field's declared range (e.g. `min: 0` on a budget) so the + // browser's spinner/keyboard affordances respect it (objectui#2572); + // server-side validation still owns enforcement. + min={typeof currencyField?.min === 'number' ? currencyField.min : undefined} + max={typeof currencyField?.max === 'number' ? currencyField.max : undefined} + step={Math.pow(10, -precision).toFixed(precision)} + // `refusal` is this widget's OWN reading and no host can produce it; + // `error` keeps its single author (objectui#3222 / objectui#6716). + aria-invalid={!!error || !!refusal} + /> +
+
); } diff --git a/packages/fields/src/widgets/GeolocationField.tsx b/packages/fields/src/widgets/GeolocationField.tsx index 728e073b26..db2bdc271c 100644 --- a/packages/fields/src/widgets/GeolocationField.tsx +++ b/packages/fields/src/widgets/GeolocationField.tsx @@ -1,9 +1,10 @@ import React, { useId } from 'react'; -import { Input, Button, Label, EmptyValue } from '@object-ui/components'; +import { Input, Button, Label, EmptyValue, cn } from '@object-ui/components'; import { MapPin, Crosshair } from 'lucide-react'; import { FieldWidgetComponentProps } from './types.js'; import { toDomProps } from './toDomProps.js'; import { toHostGroupProps } from './toHostGroupProps.js'; +import { useBadInputRefusal, BadInputMessage, BAD_INPUT_BORDER } from './numberBadInput.js'; /** * Geolocation data structure @@ -20,6 +21,14 @@ export interface GeolocationValue { */ export function GeolocationField({ value, onChange, field, readonly, error, ...props }: FieldWidgetComponentProps) { const [isLoading, setIsLoading] = React.useState(false); + /** + * TWO independent readings, one per sub-input (objectui#6780). A composite + * cannot share one refusal: `1e` in the latitude box says nothing about the + * longitude box, and a shared message could not name which half it is about. + * The example is each coordinate's own, matching `LocationField`'s. + */ + const lat = useBadInputRefusal('30.2741'); + const lng = useBadInputRefusal('120.1551'); const location = value || {}; // DOM pass-through (objectui#3318): the whitelist spread goes onto the FIRST // sub-input (latitude); the composite's validation state goes onto BOTH @@ -60,6 +69,13 @@ export function GeolocationField({ value, onChange, field, readonly, error, ...p const groupId = useId(); const subId = (name: keyof GeolocationValue) => `${groupId}-${name}`; + /** + * objectui#6780: ask the browser whether it can READ the box before trusting + * `.value`. Both sub-inputs are `type="number"`, so both can DISPLAY text + * (`1e`, `-`, `.`) while `.value` reads `''` — measured in Chromium + * 141.0.7390.37. The emission is deliberately unchanged; see + * `numberBadInput.tsx`. + */ const handleFieldChange = (fieldName: keyof GeolocationValue, fieldValue: string) => { onChange({ ...location, @@ -67,6 +83,21 @@ export function GeolocationField({ value, onChange, field, readonly, error, ...p }); }; + /** + * The blur arm — this widget had no `onBlur` at all. React delivers no + * `onChange` when `.value` never leaves `''` (the measured shape of PASTING + * `1e` into an empty box), and `badInput` is still true at blur time. + * + * ⚠️ The latitude box COMPOSES the host's `onBlur`, which `toDomProps` + * already delivers into `domProps`; a bare handler after that spread would + * silently drop a declared pass-through key. The longitude box takes no + * spread (objectui#3318), so it has none to compose. + */ + const handleLatBlur = (e: React.FocusEvent) => { + lat.readBadInput(e.target); + domProps.onBlur?.(e); + }; + const getCurrentLocation = () => { if (!navigator.geolocation) { console.error('Geolocation is not supported by this browser'); @@ -174,13 +205,20 @@ export function GeolocationField({ value, onChange, field, readonly, error, ...p id={subId('latitude')} type="number" value={location.latitude ?? ''} - onChange={(e) => handleFieldChange('latitude', e.target.value)} + onChange={(e) => { + lat.readBadInput(e.target); + handleFieldChange('latitude', e.target.value); + }} + onBlur={handleLatBlur} placeholder="37.7749" disabled={readonly || props.disabled} step="any" - className={props.className} - aria-invalid={!!error} + className={cn(lat.refusal ? BAD_INPUT_BORDER : '', props.className)} + // `refusal` is this box's OWN reading; `error` is the composite's + // published slot and keeps its single author (objectui#3222). + aria-invalid={!!error || !!lat.refusal} /> +
@@ -189,12 +227,18 @@ export function GeolocationField({ value, onChange, field, readonly, error, ...p id={subId('longitude')} type="number" value={location.longitude ?? ''} - onChange={(e) => handleFieldChange('longitude', e.target.value)} + onChange={(e) => { + lng.readBadInput(e.target); + handleFieldChange('longitude', e.target.value); + }} + onBlur={(e) => lng.readBadInput(e.target)} placeholder="-122.4194" disabled={readonly || props.disabled} step="any" - aria-invalid={!!error} + className={lng.refusal ? BAD_INPUT_BORDER : undefined} + aria-invalid={!!error || !!lng.refusal} /> +
diff --git a/packages/fields/src/widgets/NumberField.tsx b/packages/fields/src/widgets/NumberField.tsx index 65128f12d9..aac4f9b49d 100644 --- a/packages/fields/src/widgets/NumberField.tsx +++ b/packages/fields/src/widgets/NumberField.tsx @@ -1,14 +1,18 @@ import React from 'react'; -import { Input, EmptyValue } from '@object-ui/components'; +import { Input, EmptyValue, cn } from '@object-ui/components'; import { NumberFieldMetadata } from '@object-ui/types'; import { FieldWidgetComponentProps } from './types.js'; import { toDomProps } from './toDomProps.js'; +import { useBadInputRefusal, BadInputMessage, BAD_INPUT_BORDER } from './numberBadInput.js'; /** * NumberField - Numeric input with optional decimal precision * Supports min/max/step constraints and configurable decimal precision */ export function NumberField({ value, onChange, field, readonly, ...props }: FieldWidgetComponentProps) { + // Before the readonly return: hooks are unconditional (objectui#6780). + const { refusal, readBadInput } = useBadInputRefusal('1234'); + if (readonly) { return value == null ? : {value}; } @@ -28,22 +32,54 @@ export function NumberField({ value, onChange, field, readonly, ...props }: Fiel const domProps = toDomProps(props); + /** + * The blur arm objectui#6780 adds — this widget had no `onBlur` at all. + * + * React delivers no `onChange` when `.value` never leaves `''`, which is the + * measured shape of PASTING `1e` into an empty box; `badInput` is still true + * at blur time, so this is the only arm that sees that route. + * + * ⚠️ COMPOSES the host's `onBlur` rather than replacing it: `onBlur` is a + * declared DOM pass-through key that `toDomProps` already delivers here, and + * a bare handler written after the spread would silently drop it. + */ + const handleBlur = (e: React.FocusEvent) => { + readBadInput(e.target); + domProps.onBlur?.(e); + }; + return ( - { - const val = e.target.value; - onChange(val === '' ? (null as any) : Number(val)); - }} - placeholder={numberField?.placeholder} - disabled={readonly || domProps.disabled} - // Surface the field's declared range so the browser's spinner/keyboard - // affordances respect it (server-side validation still owns enforcement). - min={typeof numberField?.min === 'number' ? numberField.min : undefined} - max={typeof numberField?.max === 'number' ? numberField.max : undefined} - step={step} - /> +
+ { + // objectui#6780: ask the browser whether it can READ the box before + // trusting `.value`. The emission below is deliberately unchanged — + // see `numberBadInput.tsx` for why refusing would wipe the very text + // the diagnostic points at. + readBadInput(e.target); + const val = e.target.value; + onChange(val === '' ? (null as any) : Number(val)); + }} + onBlur={handleBlur} + placeholder={numberField?.placeholder} + disabled={readonly || domProps.disabled} + className={cn(refusal ? BAD_INPUT_BORDER : '', domProps.className)} + // Surface the field's declared range so the browser's spinner/keyboard + // affordances respect it (server-side validation still owns enforcement). + min={typeof numberField?.min === 'number' ? numberField.min : undefined} + max={typeof numberField?.max === 'number' ? numberField.max : undefined} + step={step} + // ⚠️ Written ONLY when refused. This widget does not read the published + // `error` slot (objectui#3222 never gave it one), so an unconditional + // `aria-invalid={!!refusal}` would stamp `"false"` over the correct + // value ``'s Radix Slot hands down — the exact overwrite + // objectui#3222's e2e pins call out. + {...(refusal ? { 'aria-invalid': true } : {})} + /> + +
); } diff --git a/packages/fields/src/widgets/PercentField.tsx b/packages/fields/src/widgets/PercentField.tsx index a9762b55a3..8f36368574 100644 --- a/packages/fields/src/widgets/PercentField.tsx +++ b/packages/fields/src/widgets/PercentField.tsx @@ -1,7 +1,8 @@ import React from 'react'; -import { Input, Slider, EmptyValue } from '@object-ui/components'; +import { Input, Slider, EmptyValue, cn } from '@object-ui/components'; import { FieldWidgetComponentProps } from './types.js'; import { toDomProps } from './toDomProps.js'; +import { useBadInputRefusal, BadInputMessage, BAD_INPUT_BORDER } from './numberBadInput.js'; /** * PercentField - Percentage input with configurable decimal precision @@ -12,6 +13,9 @@ export function PercentField({ value, onChange, field, readonly, error, classNam const percentField = field as any; const precision = percentField?.precision ?? 2; + // Before the readonly return below: hooks are unconditional (objectui#6780). + const { refusal, readBadInput } = useBadInputRefusal('12.5'); + // Convention detection. A field declaring `max > 1` (e.g. `max: 100`) stores // WHOLE-NUMBER percents (0–100); otherwise values are FRACTIONS (0–1) shown // as 0–100%. This matches the read-side formatter so the edit widget agrees @@ -64,13 +68,20 @@ export function PercentField({ value, onChange, field, readonly, error, classNam * and the pinned oracle-vs-product table in * `__tests__/NumberInputWidgets.environmentDivergence.test.tsx`. * - * ⚠️ OPEN (objectui#6765): the last row is a SILENT drop — the box keeps - * DISPLAYING `1e` while `.value` reads `''`, so this emits `null` with - * `aria-invalid` still `false` and no diagnostic drawn (objectui#6716's - * class). Escalated, not fixed here — it is shared by every `type="number"` - * widget in this package. + * ⭐ CLOSED for the last row (objectui#6780, ruled 2026-08-29): the silent + * drop is ANNOUNCED, via the platform's own `validity.badInput`, across all + * four `type="number"` widgets of this package as one change. The measurement + * and the reason the EMISSION is unchanged live in `numberBadInput.tsx`. + * + * ⛔ STILL SILENT, deliberately: the TRUNCATING rows above (`1.2.3` stores + * `0.0123`, `0x10` stores `0.1`). The browser filtered those keystrokes + * before this handler ran, so no widget-side guard can refuse them. Written + * down for users in `content/docs/guide/fields.md`, because a control that + * warns about `1e` while silently truncating `1.2.3` teaches people that no + * warning means the value is right. */ const handleChange = (e: React.ChangeEvent) => { + readBadInput(e.target); if (e.target.value === '') { onChange(null as any); return; @@ -80,6 +91,26 @@ export function PercentField({ value, onChange, field, readonly, error, classNam onChange(val as any); }; + /** + * The blur arm objectui#6780 adds — this widget had no `onBlur` at all. + * + * Needed because React delivers no `onChange` when `.value` never leaves + * `''`, which is the measured shape of PASTING `1e` into an empty box: one + * DOM `input` event fires, React's input-value tracking suppresses the + * synthetic change, and `badInput` is still true at blur time. + * + * ⚠️ It COMPOSES the host's `onBlur` instead of replacing it. `onBlur` is a + * declared DOM pass-through key (`FieldWidgetDomProps`), so `toDomProps` + * already delivers it here; a bare `onBlur={...}` written after that spread + * would silently drop a key the contract promises — this package's + * DECLARED-BUT-NOT-DELIVERED class (objectui#3290 / objectui#3222). + */ + const domProps = toDomProps(props); + const handleBlur = (e: React.FocusEvent) => { + readBadInput(e.target); + domProps.onBlur?.(e); + }; + const handleSliderChange = (values: number[]) => { if (readonly || props.disabled) return; if (!Array.isArray(values) || values.length === 0) { @@ -98,20 +129,24 @@ export function PercentField({ value, onChange, field, readonly, error, classNam
%
+ badInput TRUE + * typed "1." "1e5" "1.2.3" "0x10" "12abc" -> badInput false + * ``` + * + * and never on anything the browser can actually emit: `12`, `1.23`, `010`, + * `15`, `1`, `12.345` all read `badInput === false`. + * + * ## ⛔ What this deliberately does NOT do: change what is emitted + * + * objectui#6716's shape REFUSES (its `onChange` never fires and the prior value + * stands). Here that would destroy the very text the diagnostic points at, so + * these widgets keep emitting exactly what they emitted before and only stop + * being silent about it. MEASURED, in both halves: + * + * - Chromium: after typing `1e`, a script write of `.value` clears the raw + * display and flips `badInput` back to `false` — even writing `""`. + * - React 19.2.8's own `updateInput` (`react-dom/cjs/react-dom-client. + * development.js`) restores a number input with + * `if ((0 === value && "" === element.value) || element.value != value) + * element.value = "" + getToStringValue(value);` + * + * So for a box that already held `5`, refusing leaves `props.value` at `5`, + * `element.value` at `""`, and React writes `"5"` back — wiping the `5e` the + * user is looking at and the message is about. Emitting keeps `props.value` at + * `""`, the write is skipped, and the raw text survives, which is what + * objectui#6716 requires of a refusal ("keeps the refused text in the box, so + * the message has something to point at"). + * + * ⚠️ The text itself is NOT quotable. The browser displays it but never exposes + * it — `.value` is `""` and there is no other channel — so unlike + * objectui#6715's residue message this one cannot name what was typed. It + * points at the box instead. + */ + +/** The one sentence, in objectui#6716's `Not saved: …` shape. */ +export function badInputMessage(example: string): string { + return `Not saved: the text in this box is not a number. Enter a plain decimal (example: ${example}).`; +} + +/** + * The widget's OWN refusal state, and the reader that fills it. + * + * Named `refusal`, never `error`: `error` is the published validation slot on + * the widget contract (objectui#3222) with exactly one author, the form + * renderer. This is the widget's own reading, which no host can produce — the + * same two-name split `LocationField` uses (objectui#6716). + * + * `readBadInput` is called from BOTH arms, because neither alone sees every + * route (MEASURED in Chromium): + * + * - TYPED `1e` into an empty box fires 2 `input` events — the `e` keystroke + * moves `.value` from `"1"` to `""`, so React does deliver an `onChange`. + * - PASTED `1e` into an empty box fires 1 `input` event, but `.value` never + * leaves `""`; React's own input-value tracking suppresses the synthetic + * `onChange`, so the CHANGE arm never runs. Blur fires, and `badInput` is + * still `true` at blur time — which is the whole reason for the blur arm. + */ +export function useBadInputRefusal(example: string) { + const [refusal, setRefusal] = useState(null); + const readBadInput = useCallback( + (target: HTMLInputElement | null | undefined): boolean => { + const bad = target?.validity?.badInput === true; + // Setting the same value is a React bail-out, so the good path costs no + // extra render. + setRefusal(bad ? badInputMessage(example) : null); + return bad; + }, + [example], + ); + return { refusal, readBadInput }; +} + +/** + * The drawn diagnostic — objectui#6716's exact markup, in one spelling so the + * four widgets of this class cannot drift apart. + */ +export function BadInputMessage({ refusal }: { refusal: string | null }) { + if (!refusal) return null; + return

{refusal}

; +} + +/** The class that marks the refused control, in one spelling. */ +export const BAD_INPUT_BORDER = 'border-red-500 focus-visible:ring-red-500'; From adb3c3ace1806356ed4872cb9b7d500cde57a0a2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 29 Aug 2026 15:22:51 +0000 Subject: [PATCH 2/2] docs(fields): write down that filtering truncation stays silent The binding second half of #6780's ruling. A control that warns about `1e` but silently stores `1.23` for a pasted `1.2.3` teaches users that no warning means the value is right, so the asymmetry is documented rather than left implicit. The canonical explanation is the fields guide; the currency, percent and number pages carry a short pointer with their own numbers, and the package README states it for consumers rendering these widgets directly. Also collapses the new suite's `any` casts into one structural widget type (AGENTS.md #6), and adds the changeset. Part of #6780 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49 --- .../6780-number-input-badinput-announce.md | 40 +++++ content/docs/fields/currency.mdx | 18 ++ content/docs/fields/number.mdx | 18 ++ content/docs/fields/percent.mdx | 18 ++ content/docs/guide/fields.md | 43 +++++ packages/fields/README.md | 24 +++ ...mberInputWidgets.badInputAnnounce.test.tsx | 157 ++++++++---------- 7 files changed, 226 insertions(+), 92 deletions(-) create mode 100644 .changeset/6780-number-input-badinput-announce.md diff --git a/.changeset/6780-number-input-badinput-announce.md b/.changeset/6780-number-input-badinput-announce.md new file mode 100644 index 0000000000..49b32b858d --- /dev/null +++ b/.changeset/6780-number-input-badinput-announce.md @@ -0,0 +1,40 @@ +--- +"@object-ui/fields": minor +--- + +A `type="number"` field no longer displays one value and stores another in +silence (objectui#6780). + +`NumberField`, `CurrencyField`, `PercentField` and `GeolocationField` now +announce when the browser reports `validity.badInput` — it is holding text it +cannot read. The control is marked `aria-invalid="true"` and draws +`Not saved: the text in this box is not a number. Enter a plain decimal +(example: …).`, reusing the refusal shape objectui#6716 introduced for +`LocationField`. + +Measured in Chromium 141.0.7390.37 (Playwright 1.62.1), typing `1e` into an +empty number box leaves it **visibly displaying `1e`** while `.value` reads the +empty string. Before this change the widget emitted `null`, `aria-invalid` +stayed `"false"`, and nothing was said — on a money field. Nine keyboard +reachable states behave that way (`1e`, `1e-`, `1e+`, `5e`, `-`, `.`, `+`, `-.`, +`e`), and none of the six values a real browser actually emits trips the guard. + +Both a change arm and a **blur** arm are wired. Pasting `1e` into an empty box +never moves `.value` off `''`, so React's input-value tracking suppresses the +change event entirely and blur is the only arm that sees it. `PercentField`, +`NumberField` and `GeolocationField` had no `onBlur` before; the new one +composes any handler a host supplied rather than replacing it. + +The guard ANNOUNCES; it deliberately does not refuse. Refusing would leave the +React `value` prop unchanged, and React's `updateInput` writes it back over the +raw text — wiping the very entry the message points at. + +⚠️ **Filtering truncation stays silent, and cannot be made otherwise.** Pasting +`1.2.3` into a currency field stores `1.23`; `0x10` stores `10`. The browser +discards those characters as they arrive, before any widget code runs, so no +widget-side guard can refuse them — only abandoning `type="number"` could, which +would reverse objectui#2572's deliberate `min`/`max`/`step` and mobile numeric +keyboard affordances. This asymmetry is documented for users in +`content/docs/guide/fields.md` and on the currency, percent and number field +pages, because a control that warns about `1e` while silently truncating `1.2.3` +teaches people that no warning means the value is right. diff --git a/content/docs/fields/currency.mdx b/content/docs/fields/currency.mdx index d004d7ecbb..a6ce1b00b0 100644 --- a/content/docs/fields/currency.mdx +++ b/content/docs/fields/currency.mdx @@ -38,6 +38,24 @@ const amount: CurrencyFieldMetadata = { The value being edited, and the `className` / `disabled` a host supplies, are **not** metadata keys — they are runtime widget props. See [Field Widget Props](/docs/fields/widget-props). +## What the browser rewrites before this field sees it + +This widget renders a native `type="number"` input, so the browser decides what +the box accepts. Two things can happen and **only one of them is announced**: + +- **Announced.** Text the browser cannot read at all — `1e`, a lone `-`, a lone + `.` — leaves the box visibly showing what was typed while its value reads + empty. The field is marked `aria-invalid` and draws + *"Not saved: the text in this box is not a number."* +- ⚠️ **Not announced.** Entries the browser silently **truncates**: pasting + ``1.2.3`` stores ``1.23``, and `0x10` stores ``10``. No warning is + possible here — the browser discards the extra characters as they arrive, so + nothing reaches ObjectUI to check. + +⛔ **"No warning" therefore does not mean "the value is right."** Full +explanation and the reasoning: [What a number field silently +rewrites](/docs/guide/fields#what-a-number-field-silently-rewrites). + ## Supported Currencies - **USD**: US Dollar ($) diff --git a/content/docs/fields/number.mdx b/content/docs/fields/number.mdx index 3f3208ddc6..75f6dd0dd8 100644 --- a/content/docs/fields/number.mdx +++ b/content/docs/fields/number.mdx @@ -43,6 +43,24 @@ const quantity: NumberFieldMetadata = { The value being edited, and the `className` / `disabled` a host supplies, are **not** metadata keys — they are runtime widget props. See [Field Widget Props](/docs/fields/widget-props). +## What the browser rewrites before this field sees it + +This widget renders a native `type="number"` input, so the browser decides what +the box accepts. Two things can happen and **only one of them is announced**: + +- **Announced.** Text the browser cannot read at all — `1e`, a lone `-`, a lone + `.` — leaves the box visibly showing what was typed while its value reads + empty. The field is marked `aria-invalid` and draws + *"Not saved: the text in this box is not a number."* +- ⚠️ **Not announced.** Entries the browser silently **truncates**: pasting + ``1.2.3`` stores ``1.23``, and `0x10` stores ``10``. No warning is + possible here — the browser discards the extra characters as they arrive, so + nothing reaches ObjectUI to check. + +⛔ **"No warning" therefore does not mean "the value is right."** Full +explanation and the reasoning: [What a number field silently +rewrites](/docs/guide/fields#what-a-number-field-silently-rewrites). + ## Use Cases - **Quantities**: Order quantities, stock levels diff --git a/content/docs/fields/percent.mdx b/content/docs/fields/percent.mdx index 229a2be82a..7c7ca520c3 100644 --- a/content/docs/fields/percent.mdx +++ b/content/docs/fields/percent.mdx @@ -44,6 +44,24 @@ const discountRate: PercentFieldMetadata = { The value being edited, and the `className` / `disabled` a host supplies, are **not** metadata keys — they are runtime widget props. See [Field Widget Props](/docs/fields/widget-props). +## What the browser rewrites before this field sees it + +This widget renders a native `type="number"` input, so the browser decides what +the box accepts. Two things can happen and **only one of them is announced**: + +- **Announced.** Text the browser cannot read at all — `1e`, a lone `-`, a lone + `.` — leaves the box visibly showing what was typed while its value reads + empty. The field is marked `aria-invalid` and draws + *"Not saved: the text in this box is not a number."* +- ⚠️ **Not announced.** Entries the browser silently **truncates**: pasting + ``1.2.3`` stores ``0.0123``, and `0x10` stores ``0.1``. No warning is + possible here — the browser discards the extra characters as they arrive, so + nothing reaches ObjectUI to check. + +⛔ **"No warning" therefore does not mean "the value is right."** Full +explanation and the reasoning: [What a number field silently +rewrites](/docs/guide/fields#what-a-number-field-silently-rewrites). + ## Value Conversion The percent field handles automatic conversion: diff --git a/content/docs/guide/fields.md b/content/docs/guide/fields.md index 21a9560f5d..497c4e7939 100644 --- a/content/docs/guide/fields.md +++ b/content/docs/guide/fields.md @@ -84,6 +84,49 @@ Object UI comes with built-in support for the standard [ObjectStack Protocol](ht | `user` | Person picker — searches the `sys_user` object (a lookup specialized to users) | | `owner` | Record owner — a `user` field, typically read-only and stamped with the current user | +## What a number field silently rewrites + +`number`, `currency`, `percent` and `geolocation` render a native +`type="number"` input. The browser — not ObjectUI — decides what that box will +accept, and it rewrites some entries **before any widget code runs**. Two +different things can happen, and only one of them is announced. + +### Announced: text the browser cannot read + +If the box is left holding something that is not a complete number, the browser +reports `validity.badInput` and these widgets now say so: the control is marked +`aria-invalid="true"` and a message is drawn under it — + +> Not saved: the text in this box is not a number. Enter a plain decimal (example: 1234.56). + +Measured in Chromium 141, typing any of `1e`, `1e-`, `1e+`, `5e`, `-`, `.`, +`+`, `-.` or `e` leaves the box **visibly displaying** what was typed while its +value reads empty. Before this was announced, the field simply stored nothing +and said nothing. + +### ⚠️ NOT announced: entries the browser silently truncates + +This is the important limitation, and it is deliberate rather than an oversight. + +| you paste / type | the field stores | +|---|---| +| `1.2.3` | `1.23` | +| `0x10` | `10` | +| `12abc` | `12` | + +**No warning is shown for these, and no widget-side check can add one.** The +browser filters the keystrokes or the pasted text as it arrives, so by the time +ObjectUI sees the field the discarded characters are already gone — there is +nothing left to detect. This is native `type="number"` behaviour; recovering it +would mean giving up the numeric keyboard on mobile and the `min`/`max`/`step` +spinner on every numeric field in the product. + +⛔ **So do not read "no warning" as "the value is correct."** A warning means the +browser could not read the box at all. Silence means the browser read +*something* — which may be less than you typed. When exact input matters +(reference codes, serial numbers, anything where `1.2.3` is meaningful), declare +a `text` field, not a numeric one. + ## Using Renderers in Custom Components If you are building your own custom component (like a Kanban board card), you can leverage the registry to render fields without reinventing the wheel. diff --git a/packages/fields/README.md b/packages/fields/README.md index 0f39220ada..c0748e9aa1 100644 --- a/packages/fields/README.md +++ b/packages/fields/README.md @@ -55,6 +55,30 @@ Supported types out of the box: - **Media**: `file`, `image` - **System**: `formula`, `summary`, `auto_number` +### `type="number"` widgets: what is announced and what is not + +`NumberField`, `CurrencyField`, `PercentField` and `GeolocationField` all render +a native `type="number"` input, so the **browser** decides what the box accepts. +They share one reading of that, in `widgets/numberBadInput.tsx`: + +- **Announced.** When the browser reports `validity.badInput` — the box is + holding text it cannot convert, e.g. a typed `1e`, which Chromium keeps + DISPLAYING while `.value` reads `''` — the control is marked `aria-invalid` + and draws a `Not saved: …` message, reusing objectui#6716's refusal shape. + Both a change arm and a blur arm are wired, because pasting into an empty box + never moves `.value` and so fires no React change event at all. +- ⚠️ **Not announced, and not announceable.** Entries the browser silently + **truncates**: `1.2.3` stores `1.23`, `0x10` stores `10`. The characters are + discarded as they arrive, before any handler here runs, so no widget-side + guard can refuse them. Recovering them would mean abandoning `type="number"` + and with it the mobile numeric keyboard and the `min`/`max`/`step` spinner + (objectui#2572). + +⛔ Silence therefore means "the browser read *something*", never "the value is +correct". User-facing wording lives in +[the fields guide](../../content/docs/guide/fields.md). The measured browser vs +happy-dom matrix is in `src/__tests__/numberInputBrowserReadings.ts`. + ### Rendering form field widgets outside the form The full widget surface is exported for consumers that render field widgets diff --git a/packages/fields/src/__tests__/NumberInputWidgets.badInputAnnounce.test.tsx b/packages/fields/src/__tests__/NumberInputWidgets.badInputAnnounce.test.tsx index d675d99c35..d0aaf39b3e 100644 --- a/packages/fields/src/__tests__/NumberInputWidgets.badInputAnnounce.test.tsx +++ b/packages/fields/src/__tests__/NumberInputWidgets.badInputAnnounce.test.tsx @@ -67,89 +67,76 @@ import { afterEach(() => cleanup()); /** - * The four widgets of the class, each with the example its own message quotes. - * - * `mount` returns the box the guard watches plus the `onChange` spy. The host - * ECHOES the emission back into `value` the way a real form does — with a bare - * spy the control never moves off its initial value and React suppresses the - * second change (the same coupling the objectui#6765 suite needed). + * Every widget of this class takes the same three runtime props, but their + * `value` and `field` types differ (a number here, a `GeolocationValue` there). + * ONE structural type expresses that, so the suite needs a single `unknown` + * cast per widget instead of an `any` at every call site (AGENTS.md #6). + */ +type NumberishWidget = React.ComponentType<{ + value: unknown; + onChange: (v: unknown) => void; + field: Record; + onBlur?: React.FocusEventHandler; +}>; + +const asWidget = (w: unknown) => w as NumberishWidget; + +/** + * Mount a widget with a host that ECHOES the emission back into `value`, the + * way a real form does. Not decoration: these are CONTROLLED inputs, so with a + * bare spy host `value` never moves and React suppresses the second change — + * the same coupling the objectui#6765 suite needed. */ +function mountWidget( + Widget: NumberishWidget, + field: Record, + initial: unknown, + extra: { onBlur?: React.FocusEventHandler } = {}, +) { + const onChange = vi.fn(); + const Host = () => { + const [value, setValue] = React.useState(initial); + return ( + { onChange(v); setValue(v); }} + field={field} + {...extra} + /> + ); + }; + const { container } = render(); + const boxes = container.querySelectorAll('input[type=number]'); + return { container, onChange, boxes, box: boxes[0] as HTMLInputElement }; +} + +/** The four widgets of the class, each with the example its own message quotes. */ const WIDGETS = [ { name: 'CurrencyField', example: '1234.56', - mount: () => { - const onChange = vi.fn(); - const Host = () => { - const [value, setValue] = React.useState(null); - return ( - { onChange(v); setValue(v); }) as any} - field={{ name: 'amount', type: 'currency', currency: 'USD', precision: 2 } as any} - /> - ); - }; - const { container } = render(); - return { container, onChange, box: container.querySelector('input[type=number]') as HTMLInputElement }; - }, + mount: () => + mountWidget( + asWidget(CurrencyField), + { name: 'amount', type: 'currency', currency: 'USD', precision: 2 }, + null, + ), }, { name: 'PercentField', example: '12.5', - mount: () => { - const onChange = vi.fn(); - const Host = () => { - const [value, setValue] = React.useState(null); - return ( - { onChange(v); setValue(v); }) as any} - field={{ name: 'rate', type: 'percent', precision: 2 } as any} - /> - ); - }; - const { container } = render(); - return { container, onChange, box: container.querySelector('input[type=number]') as HTMLInputElement }; - }, + mount: () => + mountWidget(asWidget(PercentField), { name: 'rate', type: 'percent', precision: 2 }, null), }, { name: 'NumberField', example: '1234', - mount: () => { - const onChange = vi.fn(); - const Host = () => { - const [value, setValue] = React.useState(null); - return ( - { onChange(v); setValue(v); }) as any} - field={{ name: 'qty', type: 'number' } as any} - /> - ); - }; - const { container } = render(); - return { container, onChange, box: container.querySelector('input[type=number]') as HTMLInputElement }; - }, + mount: () => mountWidget(asWidget(NumberField), { name: 'qty', type: 'number' }, null), }, { name: 'GeolocationField (latitude)', example: '30.2741', - mount: () => { - const onChange = vi.fn(); - const Host = () => { - const [value, setValue] = React.useState({}); - return ( - { onChange(v); setValue(v); }) as any} - field={{ name: 'where', type: 'geolocation' } as any} - /> - ); - }; - const { container } = render(); - return { container, onChange, box: container.querySelectorAll('input[type=number]')[0] as HTMLInputElement }; - }, + mount: () => mountWidget(asWidget(GeolocationField), { name: 'where', type: 'geolocation' }, {}), }, ] as const; @@ -298,19 +285,11 @@ describe('objectui#6780 fires on input a real browser can produce', () => { describe('GeolocationField reads its two boxes independently (objectui#6780)', () => { const mountGeo = () => { - const onChange = vi.fn(); - const Host = () => { - const [value, setValue] = React.useState({}); - return ( - { onChange(v); setValue(v); }) as any} - field={{ name: 'where', type: 'geolocation' } as any} - /> - ); - }; - const { container } = render(); - const boxes = container.querySelectorAll('input[type=number]'); + const { container, boxes } = mountWidget( + asWidget(GeolocationField), + { name: 'where', type: 'geolocation' }, + {}, + ); return { container, lat: boxes[0] as HTMLInputElement, lng: boxes[1] as HTMLInputElement }; }; @@ -363,27 +342,21 @@ describe('the added onBlur composes the host handler instead of replacing it', ( ['PercentField', PercentField, { name: 'rate', type: 'percent' }], ['NumberField', NumberField, { name: 'qty', type: 'number' }], ['GeolocationField', GeolocationField, { name: 'where', type: 'geolocation' }], - ])('%s still calls a host onBlur', (_name, Widget: any, field: any) => { + ])('%s still calls a host onBlur', (_name, Widget, field) => { const hostBlur = vi.fn(); - const { container } = render( - {}} field={field} onBlur={hostBlur} />, - ); - const box = container.querySelector('input[type=number]') as HTMLInputElement; + const { box } = mountWidget(asWidget(Widget), field, undefined, { onBlur: hostBlur }); fireEvent.blur(box); expect(hostBlur).toHaveBeenCalledTimes(1); }); it('and still announces on that same blur', () => { const hostBlur = vi.fn(); - const { container } = render( - {}} - field={{ name: 'qty', type: 'number' } as any} - onBlur={hostBlur} - />, + const { container, box } = mountWidget( + asWidget(NumberField), + { name: 'qty', type: 'number' }, + undefined, + { onBlur: hostBlur }, ); - const box = container.querySelector('input[type=number]') as HTMLInputElement; box.value = '1e'; fireEvent.blur(box); expect(hostBlur).toHaveBeenCalledTimes(1);