From 824a9ec225e396b4d483950aa2e0d7713cce9545 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 06:30:26 +0000 Subject: [PATCH 1/2] fix(fields): compose the host onBlur in CurrencyField and TagsField MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both widgets wrote their own `onBlur` AFTER the `{...toDomProps(props)}` spread, so a host-supplied handler was overridden and never reached the control. `onBlur` is a DECLARED DOM pass-through key — named in `FieldWidgetDomProps` and in `SDUI_DOM_PASS_THROUGH_KEYS`, and forwarded by `toDomProps` — so this is the declared-but-not-delivered class. Resolve `toDomProps(props)` into `domProps` and call `domProps.onBlur?.(e)` at the end of the local handler, the idiom the other four widgets of this package already use. Not the no-op it was filed as: the form renderer hosts every field through react-hook-form's `Controller` and spreads the controller field — which always carries an `onBlur` — into the widget's props, so these two widgets were opting currency and tags fields out of blur-mode validation on every form in the repo. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_013hfmP9hoMd3dJwTh85J4yB --- ...mberInputWidgets.badInputAnnounce.test.tsx | 23 ++- .../__tests__/TagsField.hostOnBlur.test.tsx | 116 +++++++++++++ .../__tests__/hostOnBlurDelivery-e2e.test.tsx | 159 ++++++++++++++++++ packages/fields/src/widgets/CurrencyField.tsx | 32 +++- packages/fields/src/widgets/TagsField.tsx | 32 +++- 5 files changed, 345 insertions(+), 17 deletions(-) create mode 100644 packages/fields/src/__tests__/TagsField.hostOnBlur.test.tsx create mode 100644 packages/fields/src/__tests__/hostOnBlurDelivery-e2e.test.tsx diff --git a/packages/fields/src/__tests__/NumberInputWidgets.badInputAnnounce.test.tsx b/packages/fields/src/__tests__/NumberInputWidgets.badInputAnnounce.test.tsx index d0aaf39b3e..3ff8904422 100644 --- a/packages/fields/src/__tests__/NumberInputWidgets.badInputAnnounce.test.tsx +++ b/packages/fields/src/__tests__/NumberInputWidgets.badInputAnnounce.test.tsx @@ -328,20 +328,27 @@ describe('GeolocationField reads its two boxes independently (objectui#6780)', ( 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). + * `toDomProps` delivers a host's handler onto these controls. A widget that + * writes its own `onBlur` AFTER that spread silently drops 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. + * ⭐ `CurrencyField` joined this list in objectui#6802. It was excluded here + * on the reading that it "has overridden the host's `onBlur` since long + * before this card, no host in this repo passes one today". ⛔ The second + * half of that is FALSE, and the note is corrected rather than moved: the + * form renderer hosts every field through react-hook-form's `Controller` and + * spreads the controller field — which always carries an `onBlur` — into the + * widget's props, so this widget was opting currency fields out of blur-mode + * validation on every form in the repo. The user-visible reproduction lives + * in `hostOnBlurDelivery-e2e.test.tsx`; `TagsField`, which carried the same + * shape and is not a `type="number"` widget, is pinned in + * `TagsField.hostOnBlur.test.tsx`. */ it.each([ ['PercentField', PercentField, { name: 'rate', type: 'percent' }], ['NumberField', NumberField, { name: 'qty', type: 'number' }], ['GeolocationField', GeolocationField, { name: 'where', type: 'geolocation' }], + ['CurrencyField', CurrencyField, { name: 'amount', type: 'currency', currency: 'USD' }], ])('%s still calls a host onBlur', (_name, Widget, field) => { const hostBlur = vi.fn(); const { box } = mountWidget(asWidget(Widget), field, undefined, { onBlur: hostBlur }); diff --git a/packages/fields/src/__tests__/TagsField.hostOnBlur.test.tsx b/packages/fields/src/__tests__/TagsField.hostOnBlur.test.tsx new file mode 100644 index 0000000000..56b0595007 --- /dev/null +++ b/packages/fields/src/__tests__/TagsField.hostOnBlur.test.tsx @@ -0,0 +1,116 @@ +/** + * 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. + */ + +/** + * `TagsField` composes the host's `onBlur` instead of replacing it + * (objectui#6802) — and still commits the typed draft. + * + * `onBlur` is a DECLARED DOM pass-through key: named in `FieldWidgetDomProps` + * (`../widgets/types.ts`) and in `SDUI_DOM_PASS_THROUGH_KEYS` + * (`@object-ui/core`), and forwarded by `toDomProps`. This widget wrote + * `onBlur={() => addTag(draft)}` AFTER its `{...toDomProps(props)}` spread, so + * a host's handler was overwritten and never reached the input — this package's + * DECLARED-BUT-NOT-DELIVERED class (objectui#3290 / objectui#3222). + * + * ⚠️ Why a pin is required rather than nice-to-have: nothing that existed + * before could go red on this. The regression is invisible to every other test + * in the package, because none of them supplies a host `onBlur` — so the assert + * has to DRIVE a host handler through the real widget, not restate the + * predicate. The user-visible half (blur-mode validation through the real form + * renderer) is pinned in `hostOnBlurDelivery-e2e.test.tsx`; the four + * `type="number"` widgets carry the same pin in + * `NumberInputWidgets.badInputAnnounce.test.tsx`. + * + * ⛔ Both halves are asserted together on purpose. Composition that dropped + * `addTag` would satisfy a host-only assertion while losing the tag the user + * just typed, and the old `addTag`-only handler satisfies a tag-only assertion + * while losing the host — one assertion each way cannot tell those apart. + */ + +import React from 'react'; +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { render, fireEvent, cleanup } from '@testing-library/react'; +import '@testing-library/jest-dom'; + +import { TagsField } from '../widgets/TagsField'; + +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +/** + * Mount with a host that ECHOES the emission back into `value`, the way a real + * form does — `TagsField` is a controlled input, so a bare spy host would leave + * `value` frozen and hide any second emission. + */ +function mountTags(extra: { onBlur?: React.FocusEventHandler } = {}) { + const onChange = vi.fn(); + const Host = () => { + const [value, setValue] = React.useState([]); + return ( + { + onChange(v); + setValue(v); + }} + field={{ name: 'labels', type: 'tags' } as any} + {...extra} + /> + ); + }; + const { container } = render(); + const box = container.querySelector('input') as HTMLInputElement; + return { container, onChange, box }; +} + +describe('TagsField and a host-supplied onBlur (objectui#6802)', () => { + it('calls the host onBlur', () => { + const hostBlur = vi.fn(); + const { box } = mountTags({ onBlur: hostBlur }); + + fireEvent.blur(box); + + expect(hostBlur).toHaveBeenCalledTimes(1); + }); + + it('hands the host the real focus event, not a fabricated one', () => { + const hostBlur = vi.fn(); + const { box } = mountTags({ onBlur: hostBlur }); + + fireEvent.blur(box); + + // react-hook-form's controller `onBlur` reads the event it is handed; a + // composition that called `domProps.onBlur?.()` with no argument would pass + // the count assertion above and still break a real host. + const event = hostBlur.mock.calls[0]?.[0]; + expect(event).toBeDefined(); + expect(event.target).toBe(box); + }); + + it('still commits the typed draft as a tag on that same blur', () => { + const hostBlur = vi.fn(); + const { box, onChange } = mountTags({ onBlur: hostBlur }); + + fireEvent.change(box, { target: { value: 'urgent' } }); + fireEvent.blur(box); + + expect(onChange).toHaveBeenCalledWith(['urgent']); + expect(hostBlur).toHaveBeenCalledTimes(1); + }); + + it('works with no host handler at all — the key is optional', () => { + const { box, onChange } = mountTags(); + + fireEvent.change(box, { target: { value: 'urgent' } }); + expect(() => fireEvent.blur(box)).not.toThrow(); + + expect(onChange).toHaveBeenCalledWith(['urgent']); + }); +}); diff --git a/packages/fields/src/__tests__/hostOnBlurDelivery-e2e.test.tsx b/packages/fields/src/__tests__/hostOnBlurDelivery-e2e.test.tsx new file mode 100644 index 0000000000..ed7678d305 --- /dev/null +++ b/packages/fields/src/__tests__/hostOnBlurDelivery-e2e.test.tsx @@ -0,0 +1,159 @@ +/** + * 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. + */ + +/** + * A host `onBlur` must survive the widget it is handed to — measured through + * the REAL form renderer, not a hand-built host (objectui#6802). + * + * ## Why this file exists at all + * + * `onBlur` is a DECLARED DOM pass-through key: it is named in + * `FieldWidgetDomProps` (`../widgets/types.ts`), named in + * `SDUI_DOM_PASS_THROUGH_KEYS` (`@object-ui/core`), and forwarded by + * `toDomProps`. `CurrencyField` and `TagsField` each wrote their own + * `onBlur={…}` AFTER the `{...toDomProps(props)}` spread, so the host's handler + * was overwritten and never reached the control — this package's + * DECLARED-BUT-NOT-DELIVERED class (objectui#3290 / objectui#3222). + * + * ## The measurement that makes this a REAL defect, not a latent one + * + * objectui#6802 was filed and triaged as LATENT, on the reading that "no host + * in this repo passes `onBlur` to a field widget". ⛔ That is FALSE on `main`, + * and this file is the reproduction. + * + * The form renderer hosts every field through react-hook-form's `Controller` + * (`` in `@object-ui/components`) and spreads the controller's field + * object into the widget's props: + * + * ```tsx + * render={({ field: formField, fieldState }) => ( + * … renderFieldComponent(resolvedType, { ...fieldProps, …, ...formField, … }) + * ``` + * (`packages/components/src/renderers/form/form.tsx`) + * + * A react-hook-form controller field is `{ name, value, onChange, onBlur, ref, + * disabled }` — so **`onBlur` is on the props of every registered field widget + * in every form this repo renders**, and it is the handler that marks the field + * touched and, under `validationMode: 'onBlur'` / `'onTouched'`, runs its + * validation. A widget that overrides it does not drop a hypothetical key: it + * silently opts that field type out of blur-mode validation while every other + * field type on the same form keeps it. + * + * `validationMode` is authorable (`ObjectFormSchema`, wired to react-hook-form's + * `mode`), so an author reaches this with metadata alone. + * + * ## What is asserted + * + * The widget-level composition pins live next to their widgets + * (`NumberInputWidgets.badInputAnnounce.test.tsx` for the four `type="number"` + * widgets, `TagsField.hostOnBlur.test.tsx` for tags). THIS file asserts the + * consequence a user can see: a blur-mode required field announces on blur. + * `NumberField` — repaired in objectui#6780 — is the CONTROL: it proves the + * harness really does produce a blur-mode failure, so a red currency row is + * about the widget and not about the fixture. + * + * The widgets are registered raw rather than through `registerAllFields()`, + * which wraps every loader in `React.lazy`: an unbounded module load inside a + * bounded `findBy`/`waitFor` window is the repo's known flake generator + * (AGENTS.md 测试纪律, objectui#3010). Same component, no Suspense boundary. + */ + +import { describe, it, expect, beforeAll, beforeEach, afterEach, vi } from 'vitest'; +import { render, screen, fireEvent, waitFor, cleanup } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { ComponentRegistry } from '@object-ui/core'; +// Module scope: pulls in the form renderer's registration side effect. +import '@object-ui/components'; + +import { CurrencyField } from '../widgets/CurrencyField'; +import { TagsField } from '../widgets/TagsField'; +import { NumberField } from '../widgets/NumberField'; + +beforeAll(() => { + ComponentRegistry.register('currency', CurrencyField as any, { + namespace: 'field', + skipFallback: true, + }); + ComponentRegistry.register('tags', TagsField as any, { + namespace: 'field', + skipFallback: true, + }); + ComponentRegistry.register('number', NumberField as any, { + namespace: 'field', + skipFallback: true, + }); +}, 30000); + +beforeEach(() => { + if (!(Element.prototype as any).scrollIntoView) { + (Element.prototype as any).scrollIntoView = () => {}; + } +}); + +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +/** The real form renderer, in the blur-driven validation mode authors declare. */ +function renderBlurModeForm(field: any) { + const Form = ComponentRegistry.get('form')!; + return render( +
{}, + }} + />, + ); +} + +const controlOf = (name: string, selector: string): HTMLElement => { + const el = document.querySelector(`[data-field="${name}"] ${selector}`); + if (!el) throw new Error(`no ${selector} rendered for field "${name}"`); + return el as HTMLElement; +}; + +describe('the form renderer really does hand a field widget an onBlur (objectui#6802)', () => { + it('CONTROL: NumberField — repaired in objectui#6780 — announces on blur', async () => { + renderBlurModeForm({ name: 'qty', label: 'Qty', type: 'number', required: true }); + + fireEvent.blur(controlOf('qty', 'input[type=number]')); + + await waitFor(() => { + expect(screen.getByText('Qty is required')).toBeInTheDocument(); + }); + }); + + it('CurrencyField announces on blur instead of swallowing the host handler', async () => { + renderBlurModeForm({ name: 'amount', label: 'Amount', type: 'currency', required: true }); + + fireEvent.blur(controlOf('amount', 'input[type=number]')); + + await waitFor(() => { + expect(screen.getByText('Amount is required')).toBeInTheDocument(); + }); + }); + + it('TagsField announces on blur instead of swallowing the host handler', async () => { + renderBlurModeForm({ name: 'labels', label: 'Labels', type: 'tags', required: true }); + + fireEvent.blur(controlOf('labels', 'input')); + + await waitFor(() => { + expect(screen.getByText('Labels is required')).toBeInTheDocument(); + }); + }); +}); diff --git a/packages/fields/src/widgets/CurrencyField.tsx b/packages/fields/src/widgets/CurrencyField.tsx index 8efa4e3680..07a8ac4461 100644 --- a/packages/fields/src/widgets/CurrencyField.tsx +++ b/packages/fields/src/widgets/CurrencyField.tsx @@ -136,6 +136,8 @@ export function CurrencyField({ value, onChange, field, readonly, error, classNa * 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`. */ + const domProps = toDomProps(props); + // 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` @@ -143,17 +145,33 @@ export function CurrencyField({ value, onChange, field, readonly, error, classNa // — 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. + // ⚠️ COMPOSES the host's `onBlur` rather than replacing it (objectui#6802). + // `onBlur` is a declared DOM pass-through key (`FieldWidgetDomProps`, + // `SDUI_DOM_PASS_THROUGH_KEYS`) that `toDomProps` already delivers here, so + // the bare handler this used to be — written AFTER the spread — silently + // dropped it: this package's DECLARED-BUT-NOT-DELIVERED class + // (objectui#3290 / objectui#3222). + // + // ⛔ NOT the no-op the card assumed. objectui#6802 was filed on the reading + // that "no host in this repo passes `onBlur` to a field widget"; that is + // FALSE. The form renderer hosts every field through react-hook-form's + // `Controller` and spreads the controller field — `{ name, value, onChange, + // onBlur, ref, disabled }` — into the widget's props + // (`components/src/renderers/form/form.tsx`). That `onBlur` is what marks + // the field touched and, under an authored `validationMode: 'onBlur'` / + // `'onTouched'`, runs its validation. Overriding it opted currency fields + // out of blur-mode validation while every sibling field kept it. The + // reproduction through the real renderer is + // `__tests__/hostOnBlurDelivery-e2e.test.tsx`. readBadInput(e.target); const val = parseFloat(e.target.value); if (!isNaN(val)) { onChange(parseFloat(val.toFixed(precision))); } + // Last: the widget's own rounding emission lands before the host is told + // the field was touched, so a blur-mode validator reads the parsed value + // rather than the raw one. + domProps.onBlur?.(e); }; // ONE channel for the symbol (objectui#4414). This used to be a hand-written @@ -176,7 +194,7 @@ export function CurrencyField({ value, onChange, field, readonly, error, classNa )} { diff --git a/packages/fields/src/widgets/TagsField.tsx b/packages/fields/src/widgets/TagsField.tsx index 9288be2ffd..b4067b71cc 100644 --- a/packages/fields/src/widgets/TagsField.tsx +++ b/packages/fields/src/widgets/TagsField.tsx @@ -33,6 +33,34 @@ export function TagsField({ value, onChange, field, readonly, className, error, const removeTag = (t: string) => onChange(tags.filter((x) => x !== t)); + const domProps = toDomProps(props); + + /** + * Commit the typed draft as a tag on blur — and then hand the event on to the + * host (objectui#6802). + * + * ⚠️ `onBlur` is a DECLARED DOM pass-through key (`FieldWidgetDomProps`, + * `SDUI_DOM_PASS_THROUGH_KEYS`) that `toDomProps` already delivers onto this + * control, so the bare `onBlur={() => addTag(draft)}` this used to be — + * written AFTER the spread — silently overrode it: this package's + * DECLARED-BUT-NOT-DELIVERED class (objectui#3290 / objectui#3222). + * + * ⛔ NOT a no-op. The form renderer hosts every field through + * react-hook-form's `Controller` and spreads the controller field — + * `{ name, value, onChange, onBlur, ref, disabled }` — into the widget's + * props (`components/src/renderers/form/form.tsx`), so the dropped handler is + * the one that marks the field touched and runs blur-mode validation. The + * reproduction through the real renderer is + * `__tests__/hostOnBlurDelivery-e2e.test.tsx`. + * + * Order matters: `addTag` runs FIRST so a host validating on blur reads the + * committed list rather than the one the user had before typing the tag. + */ + const handleBlur = (e: React.FocusEvent) => { + addTag(draft); + domProps.onBlur?.(e); + }; + const onKeyDown = (e: React.KeyboardEvent) => { if (e.key === 'Enter' || e.key === ',') { e.preventDefault(); @@ -61,11 +89,11 @@ export function TagsField({ value, onChange, field, readonly, className, error, // DOM pass-through onto the real focusable control (objectui#3318): // the whitelist spread carries the form renderer's id / // aria-describedby to the input the user actually types in. - {...toDomProps(props)} + {...domProps} value={draft} onChange={(e) => setDraft(e.target.value)} onKeyDown={onKeyDown} - onBlur={() => addTag(draft)} + onBlur={handleBlur} disabled={props.disabled} // Placeholder chain (objectui#3342): the author-declared // `field.placeholder` wins; otherwise the widget's own copy arrives From d9317103a82ae1257a13ac9ef2a56d9075d9c73d Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 30 Aug 2026 06:32:54 +0000 Subject: [PATCH 2/2] chore(changeset): declare the CurrencyField/TagsField host onBlur composition Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_013hfmP9hoMd3dJwTh85J4yB --- .changeset/6802-currency-tags-host-onblur.md | 27 ++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 .changeset/6802-currency-tags-host-onblur.md diff --git a/.changeset/6802-currency-tags-host-onblur.md b/.changeset/6802-currency-tags-host-onblur.md new file mode 100644 index 0000000000..f6615d6e73 --- /dev/null +++ b/.changeset/6802-currency-tags-host-onblur.md @@ -0,0 +1,27 @@ +--- +'@object-ui/fields': patch +--- + +`CurrencyField` and `TagsField` now compose a host-supplied `onBlur` instead of +overriding it (objectui#6802). + +`onBlur` is a DECLARED DOM pass-through key — named in `FieldWidgetDomProps` +and in `SDUI_DOM_PASS_THROUGH_KEYS`, and forwarded by `toDomProps` — but both +widgets wrote their own `onBlur={…}` AFTER the `{...toDomProps(props)}` spread, +so the host's handler was overwritten and never reached the control. Each now +resolves `toDomProps(props)` into `domProps` and calls `domProps.onBlur?.(e)` +at the end of its own handler, the idiom the other four widgets of this package +already use. + +⚠️ This is a REAL behaviour change, not the no-op the finding was filed as. The +form renderer hosts every field through react-hook-form's `Controller` and +spreads the controller field — `{ name, value, onChange, onBlur, ref, disabled }` +— into the widget's props, so the overridden handler was the one that marks a +field touched and runs its validation. Concretely: on a form declaring +`validationMode: 'onBlur'` or `'onTouched'`, currency and tags fields were +silently opted out of blur-mode validation while every sibling field type kept +it. They now behave like the rest. + +Currency keeps emitting its rounded value before handing the event on, so a +blur-mode validator reads the parsed amount rather than the raw text; tags +still commits the typed draft first, so the validator reads the committed list.