diff --git a/.changeset/6716-location-refusal-diagnostic.md b/.changeset/6716-location-refusal-diagnostic.md new file mode 100644 index 000000000..b47823ef3 --- /dev/null +++ b/.changeset/6716-location-refusal-diagnostic.md @@ -0,0 +1,26 @@ +--- +'@object-ui/fields': patch +--- + +`LocationField` says WHY it refused an edit, instead of refusing in silence +(objectui#6716). + +The widget refuses to emit for input it cannot accept, and used to say nothing +when it did. Two refusals shared that silence: text that is not a +comma-separated pair (pre-existing), and a pair outside the spec's coordinate +range (objectui#6714). In both, `onChange` was never called, so the typed text +vanished with `aria-invalid` reading `"false"` throughout — a screen reader was +told the control was fine right after it had rejected the entry. + +- Both arms now render a short reason and set `aria-invalid` on the control. The + range message is built from `LocationValueSchema`'s own issues, never from a + hand-copied `-90..90`, so it cannot drift from the spec. +- The box now HOLDS the refused text, so the message has something to point at + and the entry can be corrected in place. Measured first without it: with the + value derived straight from the stored one, React restores the control in the + same tick, so typing a valid coordinate one character at a time left the box + empty, stored nothing, and lit a refusal on the final keystroke too. +- Refusal is unchanged: a coordinate the platform validator rejects is still + never emitted, and never stored. The published objectui#3222 `error` slot keeps + its single author (the form renderer); the widget's own state is separate, as + `ObjectField`'s `parseError` already is. diff --git a/packages/fields/src/__tests__/LocationField.refusalDiagnostic.test.tsx b/packages/fields/src/__tests__/LocationField.refusalDiagnostic.test.tsx new file mode 100644 index 000000000..e2bc0cada --- /dev/null +++ b/packages/fields/src/__tests__/LocationField.refusalDiagnostic.test.tsx @@ -0,0 +1,317 @@ +/** + * 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#6716 — a refusal must be ANNOUNCED, not silent. + * + * `LocationField` refuses to emit for input it cannot accept, and used to say + * nothing when it did. Two refusals shared that silence: text that is not a + * comma-separated pair (pre-existing), and a pair outside the spec's coordinate + * range (objectui#6714). In both, `onChange` is simply not called; the box is a + * controlled input, so the typed text disappeared, `aria-invalid` read `"false"` + * throughout, and nothing was rendered to explain it. + * + * ⛔ This does NOT reverse #6714. Refusal stays refusal — every pin below that + * asserts "no emission" is #6714's rule, unchanged. This card only makes the + * refusal visible. + * + * ## Why the diagnostic is the WIDGET's, not the form renderer's + * + * Triage first routed the fix to `buildValidationRules`, to keep ONE producer + * for the published objectui#3222 `error` slot. That route was measured on + * `faac0d935` and is structurally unable to express this refusal: a real + * `location` branch installed there is invoked with `undefined` in BOTH refusal + * arms, because a refusal means `onChange` never fires, so the typed text never + * becomes a form value at all — while the same branch fires correctly for a + * STORED out-of-range pair. `buildValidationRules` compiles value-shaped rules; + * a refusal has no value. It still has no `location` branch and this card does + * not give it one. + * + * So the state is the widget's own, following `ObjectField`'s live precedent in + * this same directory: a second name (`parseError` there, `refusalError` here), + * never the published slot, OR-ed into `aria-invalid` and rendered as the + * widget's own short line. The published `error` keeps exactly one author — the + * last test in this file is the pin for that. + * + * ## Why the box now HOLDS the refused text + * + * That is the one piece of controlled-input semantics this card changes, and it + * was taken on a measurement rather than on taste. The minimal shape — refusal + * state only, value still derived from `value` — was BUILT and driven first. It + * is incoherent: with no draft, React restores the control in the same tick, so + * the message points at an empty box, and every keystroke of a legitimate entry + * is judged as a finished one. Measured on the minimal shape, typing a valid + * `30.27, 120.15` one character at a time: the box read `""` after all 13 + * keystrokes, the refusal was lit after 12 of them INCLUDING the last, and the + * form stored `place: null`. The diagnostic could not tell "refused" from + * "still typing" because nothing survived the keystroke. Holding the draft is + * what makes the announcement honest — the same coupling `ObjectField` has. + */ + +import { describe, it, expect, vi } from 'vitest'; +import React from 'react'; +import { render, screen, fireEvent, cleanup } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import '@testing-library/jest-dom'; +import { valueSchemaFor } from '@objectstack/spec/data'; + +import { LocationField, type LocationValue } from '../widgets/LocationField'; + +const LOCATION_SCHEMA = valueSchemaFor({ type: 'location' } as any)!; + +const field = { name: 'site', label: 'Site', type: 'location' } as any; + +/** A stored, in-range value — the "prior value" a refusal must leave standing. */ +const STORED: LocationValue = { lat: 10, lng: 20 }; + +function box(): HTMLInputElement { + return screen.getByRole('textbox') as HTMLInputElement; +} + +/** The widget's own refusal line, or `null` when it is announcing nothing. */ +function diagnostic(container: HTMLElement): string | null { + const p = container.querySelector('p'); + return p ? p.textContent : null; +} + +function mount(value: unknown = STORED, extra: Record = {}) { + cleanup(); + const onChange = vi.fn(); + const { container } = render( + , + ); + return { container, onChange }; +} + +/** + * What the SPEC says about a pair, rendered the way the widget renders it. + * + * ⛔ The expected sentence is never typed out with `90` / `180` in it. A bound + * copied into this file is a second contract that keeps passing on the day the + * schema moves — the exact failure objectui#6714 was about, reintroduced in the + * test. The oracle is the schema's own issues. + */ +function expectedRangeMessage(pair: unknown): string { + const parsed = LOCATION_SCHEMA.safeParse(pair); + if (parsed.success) throw new Error('expectedRangeMessage called on a pair the spec ACCEPTS'); + const detail = parsed.error.issues + .map((i: any) => `${i.path.join('.') || 'value'}: ${i.message}`) + .join('; '); + return `Not saved: ${detail}`; +} + +/* -------------------------------------------------------------------------- */ +/* Arm 1 — the FORMAT refusal, silent since long before #6714. */ +/* -------------------------------------------------------------------------- */ + +describe('LocationField announces a FORMAT refusal (objectui#6716)', () => { + it('marks the control invalid and says why, instead of swallowing the edit', () => { + const { container, onChange } = mount(); + fireEvent.change(box(), { target: { value: 'not a coordinate' } }); + + // The announcement: state on the control, and a reason a person can read. + expect(box()).toHaveAttribute('aria-invalid', 'true'); + expect(diagnostic(container)).toBe( + 'Not saved: enter a latitude, longitude pair (example: 30.2741, 120.1551).', + ); + // #6714's rule, untouched: the refusal still refuses. + expect(onChange).not.toHaveBeenCalled(); + }); + + it('keeps the refused text in the box, so the message has something to point at', () => { + mount(); + fireEvent.change(box(), { target: { value: 'not a coordinate' } }); + expect(box()).toHaveValue('not a coordinate'); + }); + + it('announces the pair-shaped text that still is not a pair', () => { + // `parts.length === 2` but neither half is a number — the other half of the + // format arm, and the one a "does it contain a comma" shortcut would miss. + const { container, onChange } = mount(); + fireEvent.change(box(), { target: { value: 'here, there' } }); + expect(box()).toHaveAttribute('aria-invalid', 'true'); + expect(diagnostic(container)).toContain('Not saved:'); + expect(onChange).not.toHaveBeenCalled(); + }); +}); + +/* -------------------------------------------------------------------------- */ +/* Arm 2 — the RANGE refusal #6714 added. */ +/* -------------------------------------------------------------------------- */ + +describe('LocationField announces a RANGE refusal (objectui#6716)', () => { + it('marks the control invalid and reports the SPEC\'s own complaint', () => { + const { container, onChange } = mount(); + fireEvent.change(box(), { target: { value: '999, 999' } }); + + expect(box()).toHaveAttribute('aria-invalid', 'true'); + expect(diagnostic(container)).toBe(expectedRangeMessage({ lat: 999, lng: 999 })); + expect(onChange).not.toHaveBeenCalled(); + }); + + it.each([ + ['91, 0', { lat: 91, lng: 0 }], + ['0, 181', { lat: 0, lng: 181 }], + ['-91, 0', { lat: -91, lng: 0 }], + ['0, -181', { lat: 0, lng: -181 }], + ])('names the offending coordinate for %s', (typed, pair) => { + const { container } = mount(); + fireEvent.change(box(), { target: { value: typed } }); + expect(diagnostic(container)).toBe(expectedRangeMessage(pair)); + // The message carries the key the spec complained about, and only that one. + const offending = Object.keys(pair).filter(k => (pair as any)[k] !== 0); + for (const key of offending) expect(diagnostic(container)).toContain(`${key}:`); + }); + + it('keeps the refused pair in the box', () => { + mount(); + fireEvent.change(box(), { target: { value: '999, 999' } }); + expect(box()).toHaveValue('999, 999'); + }); +}); + +/* -------------------------------------------------------------------------- */ +/* Correcting a refusal clears the announcement. */ +/* -------------------------------------------------------------------------- */ + +describe('LocationField clears the announcement when the refusal is corrected (objectui#6716)', () => { + it.each([['format', 'not a coordinate'], ['range', '999, 999']])( + 'a corrected %s refusal leaves no diagnostic and emits the value', + (_arm, refused) => { + const { container, onChange } = mount(); + fireEvent.change(box(), { target: { value: refused } }); + expect(box()).toHaveAttribute('aria-invalid', 'true'); + + fireEvent.change(box(), { target: { value: '30.2741, 120.1551' } }); + expect(box()).toHaveAttribute('aria-invalid', 'false'); + expect(diagnostic(container)).toBeNull(); + expect(onChange).toHaveBeenCalledTimes(1); + expect(onChange.mock.calls[0][0]).toEqual({ lat: 30.2741, lng: 120.1551 }); + }, + ); + + it('clearing the box clears the announcement too, and still emits null', () => { + const { container, onChange } = mount(); + fireEvent.change(box(), { target: { value: '999, 999' } }); + fireEvent.change(box(), { target: { value: '' } }); + expect(diagnostic(container)).toBeNull(); + expect(box()).toHaveAttribute('aria-invalid', 'false'); + expect(onChange).toHaveBeenCalledWith(null); + }); +}); + +/* -------------------------------------------------------------------------- */ +/* The other direction: a valid coordinate must cost nothing. */ +/* -------------------------------------------------------------------------- */ + +describe('LocationField still accepts every coordinate the spec accepts (objectui#6716)', () => { + it.each([ + ['30.2741, 120.1551', { lat: 30.2741, lng: 120.1551 }], + ['90, 180', { lat: 90, lng: 180 }], + ['-90, -180', { lat: -90, lng: -180 }], + ['0, 0', { lat: 0, lng: 0 }], + ])('%s emits, announces nothing, and stays valid', (typed, expected) => { + expect(LOCATION_SCHEMA.safeParse(expected).success).toBe(true); + const { container, onChange } = mount(null); + fireEvent.change(box(), { target: { value: typed } }); + expect(onChange.mock.calls.map(c => c[0])).toEqual([expected]); + expect(diagnostic(container)).toBeNull(); + expect(box()).toHaveAttribute('aria-invalid', 'false'); + }); + + /** + * The realistic interaction, and the pin the whole shape rests on: a person + * types a coordinate ONE CHARACTER AT A TIME. + * + * This is what fails under the minimal (no-draft) shape — measured there: + * every keystroke erased, the refusal lit on the LAST one too, and nothing + * emitted. Here the draft accumulates, the announcement clears the moment the + * text is a pair the spec accepts, and what the user typed is what is emitted. + */ + it('lets a coordinate be TYPED, character by character, and ends clean', async () => { + const user = userEvent.setup(); + const { container, onChange } = mount(null); + await user.click(box()); + await user.type(box(), '30.27, 120.15'); + + expect(box()).toHaveValue('30.27, 120.15'); + expect(box()).toHaveAttribute('aria-invalid', 'false'); + expect(diagnostic(container)).toBeNull(); + const emissions = onChange.mock.calls.map(c => c[0]); + expect(emissions[emissions.length - 1]).toEqual({ lat: 30.27, lng: 120.15 }); + }); + + /** + * A host that does NOT echo the emission back — an `onChange` spy, a + * debounced or normalising host. The draft must survive it. + * + * The first draft-sync rule written for this card ("overwrite whenever the + * draft disagrees with `value`") fired on every keystroke here and erased the + * text as it was typed, leaving `20.15` in the box for the run above. The + * rule reacts to the VALUE CHANGING instead, which is the only event that + * means "somebody else set this field". + */ + it('keeps what was typed when the host never echoes the value back', () => { + const { container, onChange } = mount(null); + fireEvent.change(box(), { target: { value: '30.27, 120.15' } }); + // `value` is still `null` — this parent ignores emissions entirely. + expect(box()).toHaveValue('30.27, 120.15'); + expect(diagnostic(container)).toBeNull(); + expect(onChange).toHaveBeenCalledTimes(1); + }); + + it('adopts a value that arrives from OUTSIDE the box', () => { + // The record finishing its load, or a host resetting the form: nothing has + // been typed, so the box must show what is now stored. + cleanup(); + const { rerender } = render(); + expect(box()).toHaveValue(''); + rerender(); + expect(box()).toHaveValue('12, 34'); + }); + + it('still carries the optional keys across an in-range edit (objectui#6664)', () => { + const { onChange } = mount({ lat: 30.2741, lng: 120.1551, altitude: 5, accuracy: 12 }); + fireEvent.change(box(), { target: { value: '31.2304, 121.4737' } }); + expect(onChange.mock.calls.map(c => c[0])).toEqual([ + { lat: 31.2304, lng: 121.4737, altitude: 5, accuracy: 12 }, + ]); + }); +}); + +/* -------------------------------------------------------------------------- */ +/* The published slot keeps exactly one author. */ +/* -------------------------------------------------------------------------- */ + +describe('LocationField leaves the published `error` slot single-authored (objectui#3222)', () => { + it('honours a HOST-produced error without rendering its text', () => { + // `error` is the form renderer's; its TEXT is drawn by ``. A + // widget that also rendered it would double-display it (the reason the + // widget contract documents `error` as read-only-for-aria-invalid). + const { container } = mount(STORED, { error: 'Host says this field is required' }); + expect(box()).toHaveAttribute('aria-invalid', 'true'); + expect(container.textContent).not.toContain('Host says this field is required'); + expect(diagnostic(container)).toBeNull(); + }); + + it('ORs the two, so a host error survives a widget-side clear', () => { + const { container } = mount(STORED, { error: 'Host says no' }); + // A refusal, then a correction: the widget's own state clears, the host's + // does not — and `aria-invalid` still reports the host's. + fireEvent.change(box(), { target: { value: '999, 999' } }); + expect(diagnostic(container)).not.toBeNull(); + fireEvent.change(box(), { target: { value: '30.2741, 120.1551' } }); + expect(diagnostic(container)).toBeNull(); + expect(box()).toHaveAttribute('aria-invalid', 'true'); + }); +}); diff --git a/packages/fields/src/widgets/LocationField.tsx b/packages/fields/src/widgets/LocationField.tsx index 28f88fa24..d078d1535 100644 --- a/packages/fields/src/widgets/LocationField.tsx +++ b/packages/fields/src/widgets/LocationField.tsx @@ -1,5 +1,5 @@ -import React from 'react'; -import { Input, EmptyValue } from '@object-ui/components'; +import React, { useEffect, useRef, useState } from 'react'; +import { Input, EmptyValue, cn } from '@object-ui/components'; import { LocationValueSchema } from '@objectstack/spec/data'; import type { LocationValue } from '@objectstack/spec/data'; import { FieldWidgetComponentProps } from './types.js'; @@ -135,6 +135,88 @@ function carryOptionalKeys(lat: number, lng: number, previous: unknown): Locatio return emitted; } +/** + * The canonical `"lat, lng"` text for a stored value — empty for anything this + * widget cannot read (objectui#6272's unreadable shapes included). + */ +function coordinateText(value: unknown): string { + return isLocationValue(value) ? `${value.lat}, ${value.lng}` : ''; +} + +/** + * What the typed text means to this widget, as ONE reading (objectui#6716). + * + * The three outcomes are exactly the three the emission rule already had — + * cleared / not a coordinate pair / a pair — lifted out of `handleChange` so + * the DRAFT-SYNC guard below judges the text by the same rule that decides + * whether to emit it. Two copies of "is this a coordinate pair" would be two + * contracts, and the one in the effect would be the one nobody tests. + */ +type ParsedDraft = + | { kind: 'cleared' } + | { kind: 'unparsable' } + | { kind: 'pair'; lat: number; lng: number }; + +function parseDraft(text: string): ParsedDraft { + if (!text.trim()) return { kind: 'cleared' }; + const parts = text.split(',').map(p => p.trim()); + if (parts.length !== 2) return { kind: 'unparsable' }; + const lat = parseFloat(parts[0]); + const lng = parseFloat(parts[1]); + if (isNaN(lat) || isNaN(lng)) return { kind: 'unparsable' }; + return { kind: 'pair', lat, lng }; +} + +/** + * Does the text in the box already MEAN the stored value? + * + * Compared by meaning, not by string: `"30.270, 120.150"` and + * `"30.27, 120.15"` denote the same coordinate, and rewriting the first into + * the second while someone is typing moves their caret for no reason. This is + * the same property `ObjectField`'s sync effect tests with a `JSON.stringify` + * round-trip, expressed for a coordinate pair. + */ +function draftDenotes(text: string, value: unknown): boolean { + const parsed = parseDraft(text); + if (parsed.kind === 'cleared') return !isLocationValue(value); + if (parsed.kind === 'unparsable') return false; + return isLocationValue(value) && value.lat === parsed.lat && value.lng === parsed.lng; +} + +/** + * What the box says when it refused text that is not a coordinate pair + * (objectui#6716) — the arm that has been silent since long before #6714. + * + * ⛔ Deliberately NOT the published `error` slot's text. `error` + * (objectui#3222) has exactly one author — the form renderer, from + * react-hook-form — and its text is drawn by ``. This sentence + * belongs to the widget's own refusal state; see it for why no host can + * produce one. + * + * It names the format AND shows it, because the format is the whole content of + * this refusal: the pair is what the box cannot read. + */ +const REFUSED_FORMAT_MESSAGE = + 'Not saved: enter a latitude, longitude pair (example: 30.2741, 120.1551).'; + +/** + * What the box says when the pair PARSED but the platform refuses its range. + * + * ⛔ The bounds are NOT written here, for the same reason + * {@link isSpecAcceptedLocation} does not test them by hand: a hand-copied + * range is a second contract that drifts silently (AGENTS.md #0.1). The + * sentence is built from the SPEC's own issues, so the day the schema moves, + * this message moves with it. + */ +function refusedRangeMessage(candidate: LocationValue): string { + const parsed = LocationValueSchema.safeParse(candidate); + if (parsed.success) return ''; + const detail = parsed.error.issues + .map(issue => `${issue.path.join('.') || 'value'}: ${issue.message}`) + .join('; '); + return `Not saved: ${detail}`; +} + /** * LocationField - Geographic coordinate input for a `type: 'location'` value. * @@ -152,58 +234,141 @@ function carryOptionalKeys(lat: number, lng: number, previous: unknown): Locatio */ export function LocationField({ value, onChange, field, readonly, error, ...props }: FieldWidgetComponentProps) { const config = field; - // For display, convert the stored pair to a "lat, lng" string. - const displayValue = isLocationValue(value) ? `${value.lat}, ${value.lng}` : ''; + + /** + * The text in the box, held HERE rather than re-derived from `value` on every + * render (objectui#6716). + * + * ⚠️ This is the one piece of controlled-input semantics this card changes, + * and it was taken on a measurement, not on taste. With the box's value + * derived straight from `value`, a refusal means no state update follows the + * change event, so React restores the control in the SAME tick and the typed + * text is gone before anything can be said about it. Measured on + * `faac0d935`, typing a perfectly valid `30.27, 120.15` one character at a + * time: the box read `""` after all 13 keystrokes, `dataSource.create` was + * called with `place: null`, and a refusal diagnostic — which each of those + * keystrokes legitimately triggers, since `"3"` is not a pair — stayed lit + * through 12 of them. A diagnostic with no draft to point at cannot tell + * "refused" from "still typing", so announcing the refusal REQUIRES holding + * the text that was refused. `ObjectField` couples the two for the same + * reason. + */ + const [draft, setDraft] = useState(() => coordinateText(value)); + + /** + * This widget's OWN refusal state (objectui#6716). + * + * Named `refusalError`, NOT `error`: `error` is the published validation slot + * on the widget contract (objectui#3222) and is destructured above — it keeps + * exactly one author, the form renderer. The same discipline, and the same + * two-name shape, as `ObjectField`'s `parseError`. + * + * It has to live here because no host can produce it: a refusal means + * `onChange` never fires, so the typed text never becomes a form value and + * `buildValidationRules` — which compiles value-shaped rules — is handed + * `undefined`. That was measured on this card before the route was chosen: a + * real `location` branch installed in `buildValidationRules` saw `undefined` + * in both refusal arms, while the same branch fired correctly for a STORED + * out-of-range pair. `buildValidationRules` still has no `location` branch, + * and this card does not give it one. + */ + const [refusalError, setRefusalError] = useState(null); + + /** + * Adopt a value that changed OUTSIDE this box — a record finishing its load, + * a host resetting the form. + * + * ⚠️ The trigger is the VALUE changing, tracked against what this widget last + * saw — never "the draft disagrees with the value". That second rule was + * written first and measured wrong: a host that does not echo an emission + * back (an `onChange` spy, a debounced or normalising host) leaves `value` + * behind the draft permanently, so the rule fired on every keystroke and + * erased the text as it was typed — the very defect this card is fixing, + * moved into its fix. Driving the standalone widget caught it: typing + * `30.27, 120.15` left `20.15` in the box. + * + * Two guards then decide whether an external change is worth overwriting + * what the person is holding: + * + * - the draft is text this widget REFUSED ⇒ leave it standing. It is the + * text the diagnostic is about, and an unsaved edit is not a background + * refresh's to discard (AGENTS.md #8). + * - the draft already DENOTES the new value ⇒ leave the user's own spelling + * alone (see {@link draftDenotes}). + */ + const lastSeenValue = useRef(value); + useEffect(() => { + if (Object.is(lastSeenValue.current, value)) return; + lastSeenValue.current = value; + if (refusalError) return; + if (draftDenotes(draft, value)) return; + // eslint-disable-next-line react-hooks/set-state-in-effect -- Required for controlled component sync + setDraft(coordinateText(value)); + }, [value, draft, refusalError]); if (readonly) { - return {displayValue || }; + // The STORED value, never the draft: a readonly field renders what is + // saved, and nothing can have been typed into it. + return {coordinateText(value) || }; } const handleChange = (e: React.ChangeEvent) => { - const val = e.target.value; - if (!val.trim()) { + const text = e.target.value; + // The box keeps what was typed — including text about to be refused, which + // is the only thing the diagnostic below can point at (objectui#6716). + setDraft(text); + + const parsed = parseDraft(text); + if (parsed.kind === 'cleared') { + setRefusalError(null); onChange(null); return; } - // Parse as coordinates (latitude, longitude) - const parts = val.split(',').map(p => p.trim()); - if (parts.length === 2) { - const lat = parseFloat(parts[0]); - const lng = parseFloat(parts[1]); - if (!isNaN(lat) && !isNaN(lng)) { - // The typed pair replaces `lat`/`lng`; `altitude`/`accuracy` survive - // the edit (objectui#6664). Key-by-key, never a spread — see above. - const emitted = carryOptionalKeys(lat, lng, value); - // objectui#6714: the SAME refusal the line below already applies to - // text that isn't a coordinate pair, extended from format to RANGE. - // Measured before choosing this: nothing downstream rejects or repairs - // the value — a real `ObjectForm` submit hands `{ lat: 999, lng: 999 }` - // straight to `dataSource.create`, with no error raised anywhere — so - // refusing HERE is the only thing standing between a typo and storage. - if (isSpecAcceptedLocation(emitted)) { - onChange(emitted); - } - } - // If the text is not a coordinate pair, or the pair is one the spec - // refuses, don't update the value — the prior value stands. + if (parsed.kind === 'unparsable') { + // The text is not a coordinate pair. The prior value stands — and since + // objectui#6716 the box says so instead of swallowing the edit. + setRefusalError(REFUSED_FORMAT_MESSAGE); + return; + } + + // The typed pair replaces `lat`/`lng`; `altitude`/`accuracy` survive the + // edit (objectui#6664). Key-by-key, never a spread — see above. + const emitted = carryOptionalKeys(parsed.lat, parsed.lng, value); + // objectui#6714: the SAME refusal applied to text that isn't a coordinate + // pair, extended from format to RANGE. Measured before choosing this: + // nothing downstream rejects or repairs the value — a real `ObjectForm` + // submit hands `{ lat: 999, lng: 999 }` straight to `dataSource.create`, + // with no error raised anywhere — so refusing HERE is the only thing + // standing between a typo and storage. + if (isSpecAcceptedLocation(emitted)) { + setRefusalError(null); + onChange(emitted); + return; } + // objectui#6716: the refusal STANDS — this card does not reverse #6714. It + // only stops the refusal from being silent. + setRefusalError(refusedRangeMessage(emitted)); }; return ( - +
+ + {refusalError &&

{refusalError}

} +
); } diff --git a/packages/plugin-form/src/ObjectForm.locationRefusal.test.tsx b/packages/plugin-form/src/ObjectForm.locationRefusal.test.tsx new file mode 100644 index 000000000..ceb82adb8 --- /dev/null +++ b/packages/plugin-form/src/ObjectForm.locationRefusal.test.tsx @@ -0,0 +1,154 @@ +/** + * 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#6716, measured end to end: a refused coordinate must be ANNOUNCED, + * and must still not be stored. + * + * The sibling file `ObjectForm.locationRange.test.tsx` is #6714's measurement — + * an out-of-range pair never reaches `dataSource.create`. This file is the + * other half: the same refusal, now visible to the person who caused it. Both + * arms are covered, the pre-existing FORMAT one (`not a coordinate`) and the + * RANGE one #6714 added (`999, 999`). + * + * ## What this measured on the base commit (`faac0d935`), before the fix + * + * Typing either text into this same form left `aria-invalid` at `"false"`, the + * field rendering only its label, the box EMPTY (React restores a controlled + * input in the same tick when no state update follows a change), and the create + * payload carrying no `place` key at all. The refusal was total and silent. + * + * ## Why the announcement comes from the widget + * + * Triage routed the fix to `buildValidationRules`, for a single producer of the + * published objectui#3222 `error` slot. Measured: that route cannot see a + * refusal at all — a refusal means `onChange` never fires, so the typed text + * never becomes a form value, and a `location` branch installed there was + * invoked with `undefined` in both arms. The last test in this file pins the + * consequence that keeps the sibling file's docblock true: `buildValidationRules` + * STILL has no `location` branch, and this card did not give it one. + */ +import { describe, it, expect, vi } from 'vitest'; +import { render, waitFor, fireEvent } from '@testing-library/react'; +import React from 'react'; +import { valueSchemaFor } from '@objectstack/spec/data'; + +import { ObjectForm } from './ObjectForm'; +import { registerAllFields, buildValidationRules } from '@object-ui/fields'; + +registerAllFields(); + +const LOCATION_SCHEMA = valueSchemaFor({ type: 'location' } as any)!; + +const siteSchema = { + name: 'site', + fields: { + title: { type: 'text', label: 'Title' }, + place: { type: 'location', label: 'Place' }, + }, +}; + +const makeDS = () => ({ + getObjectSchema: vi.fn().mockResolvedValue(siteSchema), + create: vi.fn(async (_o: string, d: any) => ({ id: 'r1', ...d })), + update: vi.fn(), + findOne: vi.fn(), +}); + +const waitInput = (c: HTMLElement, name: string) => + waitFor(() => { + const el = c.querySelector(`input[name="${name}"]`) as HTMLInputElement | null; + if (!el) throw new Error(`${name} not ready`); + return el; + }); + +/** The whole rendered row for `place`, so any text the field draws is seen. */ +function placeRowText(container: HTMLElement): string { + let node: HTMLElement | null = container.querySelector('input[name="place"]'); + for (let i = 0; i < 4 && node?.parentElement; i++) node = node.parentElement; + return (node?.textContent || '').trim(); +} + +async function typeInto(container: HTMLElement, text: string) { + fireEvent.change(await waitInput(container, 'place'), { target: { value: text } }); + const el = container.querySelector('input[name="place"]') as HTMLInputElement; + return { ariaInvalid: el.getAttribute('aria-invalid'), domValue: el.value, rowText: placeRowText(container) }; +} + +function mountForm() { + const ds = makeDS(); + const { container } = render( + , + ); + return { ds, container }; +} + +describe('ObjectForm shows WHY a location was refused (objectui#6716)', () => { + it.each([ + ['the RANGE arm', '999, 999'], + ['the FORMAT arm', 'not a coordinate'], + ])('%s marks the control invalid and renders a reason', async (_arm, typed) => { + const { ds, container } = mountForm(); + fireEvent.change(await waitInput(container, 'title'), { target: { value: 'HQ' } }); + const after = await typeInto(container, typed); + + // 1. The a11y state a screen reader reads — `"false"` here before the fix. + expect(after.ariaInvalid).toBe('true'); + // 2. A reason a person can read, which the row did not carry at all before. + expect(after.rowText).toContain('Not saved:'); + // 3. The text that was refused is still in the box to be corrected. + expect(after.domValue).toBe(typed); + + // 4. #6714's rule is untouched: the value is still NOT stored. + fireEvent.submit(container.querySelector('form') as HTMLFormElement); + await waitFor(() => expect(ds.create).toHaveBeenCalled()); + expect(ds.create.mock.calls[0][1].place).toBeUndefined(); + }); + + it('clears the announcement when the coordinate is corrected, and stores it', async () => { + const { ds, container } = mountForm(); + fireEvent.change(await waitInput(container, 'title'), { target: { value: 'HQ' } }); + expect((await typeInto(container, '999, 999')).ariaInvalid).toBe('true'); + + const fixed = await typeInto(container, '30.2741, 120.1551'); + expect(fixed.ariaInvalid).toBe('false'); + expect(fixed.rowText).not.toContain('Not saved:'); + + fireEvent.submit(container.querySelector('form') as HTMLFormElement); + await waitFor(() => expect(ds.create).toHaveBeenCalled()); + const stored = ds.create.mock.calls[0][1].place; + expect(stored).toEqual({ lat: 30.2741, lng: 120.1551 }); + expect(LOCATION_SCHEMA.safeParse(stored).success).toBe(true); + }); + + it('says nothing about a coordinate it accepts', async () => { + const { container } = mountForm(); + const after = await typeInto(container, '30.2741, 120.1551'); + expect(after.ariaInvalid).toBe('false'); + expect(after.rowText).not.toContain('Not saved:'); + }); +}); + +describe('the fix did NOT give `buildValidationRules` a location branch (objectui#6716)', () => { + it('compiles no rule for a `location` field', () => { + // The sibling #6714 pin's docblock states this as fact; it stays true under + // the widget-local route, and this is what keeps the two files honest. The + // measured reason it is not worth changing: a refusal never becomes a form + // value, so a rule here is handed `undefined`. + // It compiles rules key by key and returns `undefined` when none applied — + // so `undefined` here is "no branch matched a location field", not "the + // helper is missing". + expect(buildValidationRules({ type: 'location', name: 'place' })).toBeUndefined(); + // A control from the same call, so the reading above cannot be an artefact + // of a helper that returns `undefined` for everything. + expect(buildValidationRules({ type: 'text', name: 'title', required: true })).toEqual({ required: true }); + }); +});