diff --git a/.changeset/9248-screen-field-bound-help-reference.md b/.changeset/9248-screen-field-bound-help-reference.md new file mode 100644 index 0000000000..6eeff0bf79 --- /dev/null +++ b/.changeset/9248-screen-field-bound-help-reference.md @@ -0,0 +1,30 @@ +--- +'@object-ui/app-shell': patch +--- + +feat(app-shell): the screen-flow dialog renders a screen field's declared bound, help text and lookup target + +`@objectstack/spec` gave a flow screen field three keys, each spelled as the +object field spells it: the numeric bound `min` / `max`, the help text +`inlineHelpText`, and the lookup target `reference`. The ruling that added them +made shipping them WITH their rendering its condition. The dialog read none of +them, so a declared bound, a declared help sentence and a declared lookup +target were all silently inert on screen. + +The console's flow screen dialog (and the Studio screen preview, which shares +its renderer) now draws them: + +- `min` / `max` go on a numeric input as its native bounds, and Submit refuses + a value outside them with the field named, beside the `required` check. The + comparison is the one the engine applies again when the run resumes: + inclusive bounds, a present number only, so an empty optional field is never + refused. +- `inlineHelpText` is drawn under the control and tied to it with + `aria-describedby`. The placeholder is not used for it, because the browser + clears the placeholder on the first keystroke. +- A `type: 'lookup'` field with a `reference` opens the same record picker the + object form uses, listing that object's records through the data source the + dialog already holds. A lookup with no `reference` (a run suspended before + the key existed) keeps the plain input it always had. + +A screen field declaring none of the keys renders exactly as before. diff --git a/packages/app-shell/src/views/FlowRunner.tsx b/packages/app-shell/src/views/FlowRunner.tsx index e03f5d9fe0..3215083b55 100644 --- a/packages/app-shell/src/views/FlowRunner.tsx +++ b/packages/app-shell/src/views/FlowRunner.tsx @@ -106,6 +106,10 @@ * - the screen `description` — outside the spec's flows face (in neither key * list), so an off-spec `description` a bundle carries anyway is ignored and * the authored text is drawn; + * - a field's `inlineHelpText` (objectstack#17306) — drawn as authored for as + * long as the spec's per-field face leaves it out of + * `FLOW_SCREEN_FIELD_COPY_KEYS`. The overlay walks that constant, so the day + * the spec lists it the help text is translated with no edit here; * - the runner chrome (Cancel / Submit / Submitting… / the terminal toast) — * the console's own words, ruled into its message catalog (objectstack#7646); * - `FlowSchema.successMessage` — off the translation surface by design. @@ -145,6 +149,7 @@ import { ScreenView, isObjectFormScreen, initialScreenValues, + screenFieldBoundViolations, visibleScreenFields, type ScreenFieldSpec, type ScreenSpec, @@ -413,6 +418,25 @@ export function FlowRunner({ state, authFetch, baseUrl, onClose, onComplete, dat ); return; } + // The declared `min` / `max` (objectstack#17306), refused here naming the + // field, beside `required` and over the same on-screen list. A courtesy, + // not the enforcement: the engine re-checks the bounds on resume, and + // `screenFieldBoundViolations` applies its comparison — inclusive, a + // present number only, so an empty optional field is never refused. + const outOfBounds = screenFieldBoundViolations(shown, values); + if (outOfBounds.length) { + toast.error( + outOfBounds + .map(({ field, bound, limit }) => { + const label = field.label || field.name; + return bound === 'min' + ? t('validation.min', { field: label, min: limit, defaultValue: '{{field}} must be at least {{min}}' }) + : t('validation.max', { field: label, max: limit, defaultValue: '{{field}} must be at most {{max}}' }); + }) + .join(t('validation.formInvalidJoiner', { defaultValue: ', ' })), + ); + return; + } setSubmitting(true); try { await resumeWith(values); diff --git a/packages/app-shell/src/views/ScreenView.boundHelpReference-9248.test.tsx b/packages/app-shell/src/views/ScreenView.boundHelpReference-9248.test.tsx new file mode 100644 index 0000000000..e849d3116e --- /dev/null +++ b/packages/app-shell/src/views/ScreenView.boundHelpReference-9248.test.tsx @@ -0,0 +1,213 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#9248 — a flow screen field renders the three keys + * objectstack#17306 gave it, spelled as the spec spells them: the numeric + * bound `min` / `max`, the help text `inlineHelpText`, and the lookup target + * `reference`. The ruling's condition is that the keys ship WITH their + * rendering, so each gets a pin on the rendered body, and a field carrying + * none of them is the control that renders as it did before. + * + * The submit-time refusal of an out-of-bound value is the runner's, pinned on + * the dialog in `__tests__/FlowRunner.bounds-9248.test.tsx`; this file pins + * the comparison it calls (`screenFieldBoundViolations`) against the engine's + * resume rule. + */ + +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { render, screen, cleanup, fireEvent, waitFor } from '@testing-library/react'; +// Module scope, per AGENTS.md 测试纪律: the lookup arm reaches `LookupField` +// through a `React.lazy` factory inside `@object-ui/fields`; importing the +// barrel here resolves that factory before any test's `findBy` budget starts. +import '@object-ui/fields'; +import { ScreenView, screenFieldBoundViolations, type ScreenSpec } from './ScreenView'; + +afterEach(cleanup); + +function specOf(fields: ScreenSpec['fields']): ScreenSpec { + return { nodeId: 'n1', title: 'Step', fields } as ScreenSpec; +} + +function renderScreen(fields: ScreenSpec['fields'], extra: { values?: Record; dataSource?: unknown } = {}) { + return render( + {}} + dataSource={extra.dataSource} + />, + ); +} + +describe('objectui#9248 — `min` / `max` land on the numeric input', () => { + it('a number field declaring `min: 1, max: 10` carries both native bounds', () => { + renderScreen([{ name: 'qty', label: 'Quantity', type: 'number', min: 1, max: 10 }]); + const input = screen.getByRole('spinbutton', { name: 'Quantity' }); + expect(input).toHaveAttribute('min', '1'); + expect(input).toHaveAttribute('max', '10'); + }); + + it('a currency field is numeric too, and a lone bound is carried alone', () => { + renderScreen([{ name: 'amount', label: 'Amount', type: 'currency', min: 0 }]); + const input = screen.getByRole('spinbutton', { name: 'Amount' }); + expect(input).toHaveAttribute('min', '0'); + expect(input).not.toHaveAttribute('max'); + }); + + it('a bound declared on a non-numeric input is not written onto it (a date input would read it as a date)', () => { + const { container } = renderScreen([{ name: 'due', label: 'Due', type: 'date', min: 1, max: 10 }]); + const input = container.querySelector('#ff-due'); + expect(input).toHaveAttribute('type', 'date'); + expect(input).not.toHaveAttribute('min'); + expect(input).not.toHaveAttribute('max'); + }); +}); + +describe('objectui#9248 — `screenFieldBoundViolations` applies the engine resume rule', () => { + const qty = { name: 'qty', label: 'Quantity', type: 'number', min: 1, max: 10 }; + + it('refuses a value above `max` and below `min`, naming the field and the bound', () => { + expect(screenFieldBoundViolations(specOf([qty]), { qty: 11 })).toEqual([ + { field: qty, bound: 'max', limit: 10 }, + ]); + expect(screenFieldBoundViolations(specOf([qty]), { qty: 0 })).toEqual([ + { field: qty, bound: 'min', limit: 1 }, + ]); + }); + + it('the bounds are inclusive: the limits themselves pass', () => { + expect(screenFieldBoundViolations(specOf([qty]), { qty: 10 })).toEqual([]); + expect(screenFieldBoundViolations(specOf([qty]), { qty: 1 })).toEqual([]); + }); + + it('an empty optional field is not refused — absence is `required`\'s question', () => { + expect(screenFieldBoundViolations(specOf([qty]), {})).toEqual([]); + expect(screenFieldBoundViolations(specOf([qty]), { qty: undefined })).toEqual([]); + }); + + it('a field its `visibleWhen` hides is not compared; shown, it is', () => { + const gated = { ...qty, visibleWhen: 'bulk == true' }; + const fields = [{ name: 'bulk', label: 'Bulk', type: 'boolean' }, gated]; + expect(screenFieldBoundViolations(specOf(fields), { bulk: false, qty: 99 })).toEqual([]); + expect(screenFieldBoundViolations(specOf(fields), { bulk: true, qty: 99 })).toEqual([ + { field: gated, bound: 'max', limit: 10 }, + ]); + }); + + it('CONTROL — a field declaring no bound is never refused, whatever it holds', () => { + expect(screenFieldBoundViolations(specOf([{ name: 'qty', type: 'number' }]), { qty: 1e9 })).toEqual([]); + }); +}); + +describe('objectui#9248 — `inlineHelpText` is drawn under the control and describes it', () => { + it('renders below the input and is linked by `aria-describedby`', () => { + renderScreen([{ name: 'note', label: 'Note', type: 'text', inlineHelpText: 'Shown below' }]); + const input = screen.getByRole('textbox', { name: 'Note' }); + const help = screen.getByText('Shown below'); + + expect(input).toHaveAttribute('aria-describedby', help.id); + expect(help.id).not.toBe(''); + expect(input).toHaveAccessibleDescription('Shown below'); + // BELOW: the help element follows the control in document order. + expect(input.compareDocumentPosition(help) & Node.DOCUMENT_POSITION_FOLLOWING).toBeTruthy(); + // Not the placeholder — that carrier is gone the moment the user types. + expect(input).not.toHaveAttribute('placeholder', 'Shown below'); + }); + + it('stays on screen while the input holds a value', () => { + renderScreen( + [{ name: 'note', label: 'Note', type: 'text', placeholder: 'e.g. ACME', inlineHelpText: 'Shown below' }], + { values: { note: 'typed' } }, + ); + expect(screen.getByRole('textbox', { name: 'Note' })).toHaveValue('typed'); + expect(screen.getByText('Shown below')).toBeVisible(); + }); + + it('describes every control arm, not only the text input', () => { + renderScreen([ + { name: 'body', label: 'Body', type: 'textarea', inlineHelpText: 'Body help' }, + { name: 'agree', label: 'Agree', type: 'boolean', inlineHelpText: 'Agree help' }, + { name: 'kind', label: 'Kind', options: [{ label: 'A', value: 'a' }], inlineHelpText: 'Kind help' }, + { name: 'qty', label: 'Qty', type: 'number', inlineHelpText: 'Qty help' }, + ]); + expect(screen.getByRole('textbox', { name: 'Body' })).toHaveAccessibleDescription('Body help'); + expect(screen.getByRole('checkbox', { name: 'Agree' })).toHaveAccessibleDescription('Agree help'); + expect(screen.getByRole('combobox', { name: 'Kind' })).toHaveAccessibleDescription('Kind help'); + expect(screen.getByRole('spinbutton', { name: 'Qty' })).toHaveAccessibleDescription('Qty help'); + }); +}); + +const ACCOUNTS = [ + { id: 'acc-1', name: 'Northwind Traders' }, + { id: 'acc-2', name: 'Contoso Ltd' }, +]; + +function makeDataSource() { + const queries: string[] = []; + return { + queries, + find: vi.fn(async (objectName: string) => { + queries.push(objectName); + return { data: ACCOUNTS, total: ACCOUNTS.length, hasMore: false, pageSize: 50 }; + }), + findOne: vi.fn(async (_o: string, id: string) => ACCOUNTS.find((a) => a.id === id) ?? null), + getObjectSchema: async (name: string) => ({ name, fields: { id: { type: 'text' }, name: { type: 'text' } } }), + }; +} + +describe('objectui#9248 — a `type: \'lookup\'` field resolves its picker from `reference`', () => { + it('the picker lists `account` records from the screen\'s data source', async () => { + const ds = makeDataSource(); + // No `SchemaRendererProvider` here on purpose: the only way the picker can + // reach this data source is the `dataSource` prop `ScreenView` threads. + renderScreen([{ name: 'acct', label: 'Account', type: 'lookup', reference: 'account', required: true }], { + dataSource: ds, + }); + + const trigger = await screen.findByTestId('lookup-trigger-acct'); + expect(trigger).toHaveAttribute('id', 'ff-acct'); + expect(trigger).toHaveAttribute('aria-required', 'true'); + fireEvent.click(trigger); + + await waitFor(() => expect(screen.getByText('Northwind Traders')).toBeInTheDocument()); + expect(screen.getByText('Contoso Ltd')).toBeInTheDocument(); + // Asserted on the object NAME, not a call count: a picker that queried the + // wrong object would satisfy a bare count. + expect(ds.queries).toContain('account'); + expect(ds.queries.every((q) => q === 'account')).toBe(true); + }); + + it('a lookup with no `reference` keeps the plain input it always had (a run suspended before the key)', () => { + renderScreen([{ name: 'acct', label: 'Account', type: 'lookup' }]); + expect(screen.getByRole('textbox', { name: 'Account' })).toBeInTheDocument(); + expect(screen.queryByTestId('lookup-trigger-acct')).toBeNull(); + }); +}); + +describe('objectui#9248 — CONTROL: a field declaring none of the keys renders as before', () => { + it('no bound, no description, no help element, no picker', () => { + const { container } = renderScreen([ + { name: 'qty', label: 'Qty', type: 'number' }, + { name: 'title', label: 'Title', type: 'text', placeholder: 'Type here' }, + { name: 'agree', label: 'Agree', type: 'boolean' }, + { name: 'acct', label: 'Account', type: 'text' }, + ]); + const qty = screen.getByRole('spinbutton', { name: 'Qty' }); + expect(qty).not.toHaveAttribute('min'); + expect(qty).not.toHaveAttribute('max'); + for (const control of [ + qty, + screen.getByRole('textbox', { name: 'Title' }), + screen.getByRole('checkbox', { name: 'Agree' }), + screen.getByRole('textbox', { name: 'Account' }), + ]) { + expect(control).not.toHaveAttribute('aria-describedby'); + } + expect(container.querySelector('[id$="-help"]')).toBeNull(); + expect(container.querySelector('[data-testid^="lookup-trigger"]')).toBeNull(); + // Each field is still exactly a label and its control. + for (const block of container.querySelectorAll('.space-y-1\\.5')) { + expect(block.children).toHaveLength(2); + } + }); +}); diff --git a/packages/app-shell/src/views/ScreenView.tsx b/packages/app-shell/src/views/ScreenView.tsx index 9a98f91ede..dd4ad24afe 100644 --- a/packages/app-shell/src/views/ScreenView.tsx +++ b/packages/app-shell/src/views/ScreenView.tsx @@ -14,7 +14,31 @@ * It owns no submit/resume behaviour and no Dialog chrome — the caller frames * it (the runtime wraps it in a Dialog + footer and resumes the run; the * preview wraps it in a card and hides the persist bar). + * + * ## The three screen-field keys objectstack#17306 added + * + * The spec gave a screen field `min` / `max`, `inlineHelpText` and `reference`, + * each spelled as the object field (`FieldSchema`) spells it, on the ruling + * that the keys ship WITH their rendering. They are read straight off the + * re-exported `ScreenFieldSpec`; there is no local spelling and no alias + * (the spec refuses `help` / `helpText` / `hint` / `tooltip` and the lookup + * target's neighbouring spellings by name, so none of them reaches a client): + * + * - `min` / `max` land on the numeric input as the native bounds, and + * {@link screenFieldBoundViolations} is the submit-time comparison — the + * engine's resume check re-applies the same rule server-side. + * - `inlineHelpText` is drawn UNDER the control and tied to it with + * `aria-describedby`. It is not the placeholder: that one is gone the moment + * the user types, and help text has to stay readable. + * - `reference` resolves a `type: 'lookup'` field's picker — the SAME + * `LookupField` widget the object form and `ActionParamDialog` use, reached + * through `getLazyFieldWidget('lookup')` and fed the screen's data source. + * A lookup with no `reference` (a run suspended before the key existed) has + * no target to resolve and keeps the plain input it always had. + * + * A field declaring none of them renders exactly as it did before. */ +import { Suspense } from 'react'; import { Input, Label, @@ -29,6 +53,15 @@ import { } from '@object-ui/components'; import { ObjectForm } from '@object-ui/plugin-form'; import { evalFieldPredicate } from '@object-ui/core'; +import { getLazyFieldWidget } from '@object-ui/fields'; + +/** + * The object form's lookup picker (`LookupField`), through the shared lazy + * widget registry `ActionParamDialog` also reads. Resolved once at module + * scope, not in render (`react-hooks/static-components`); the registry caches + * one lazy component per type, so this is the same component the dialog gets. + */ +const LookupFieldWidget = getLazyFieldWidget('lookup'); /** * The screen-pause contract is OWNED by `@objectstack/spec/contracts` @@ -143,6 +176,47 @@ export function initialScreenValues(screen: ScreenSpec): Record return v; } +/** One declared bound a collected value breaks — see {@link screenFieldBoundViolations}. */ +export interface ScreenFieldBoundViolation { + field: ScreenFieldSpec; + bound: 'min' | 'max'; + limit: number; +} + +/** + * The declared `min` / `max` bounds the collected values break, over the + * fields actually on screen (the list {@link visibleScreenFields} answers). + * + * The client refusal is a courtesy — the engine re-checks the same bounds when + * the run resumes (`validateScreenInputs` in `@objectstack/service-automation`) + * — so it applies the SAME comparison, and nothing beyond it: + * + * - only a PRESENT finite number is compared, whatever the field's `type`. An + * empty optional field holds no value, and absence is `required`'s question, + * never a bound's; + * - the bounds are inclusive: `min` refuses a value below it, `max` one above; + * - a field its `visibleWhen` hides is not compared — the user was never asked + * for it (the objectstack-ai/objectstack#3528 dead-end, applied to bounds). + */ +export function screenFieldBoundViolations( + screen: ScreenSpec, + values: Record, +): ScreenFieldBoundViolation[] { + const out: ScreenFieldBoundViolation[] = []; + for (const field of visibleScreenFields(screen, values)) { + const value = values[field.name]; + if (typeof value !== 'number' || !Number.isFinite(value)) continue; + if (typeof field.min === 'number' && value < field.min) out.push({ field, bound: 'min', limit: field.min }); + if (typeof field.max === 'number' && value > field.max) out.push({ field, bound: 'max', limit: field.max }); + } + return out; +} + +/** The id of a screen field's help-text element, which its control names in `aria-describedby`. */ +function screenFieldHelpId(name: string): string { + return `ff-${name}-help`; +} + /** Submit/cancel wiring for the object-form body — runtime persists & resumes; * the design preview hides the bar (`showSubmit`/`showCancel` false). */ export interface ScreenObjectFormActions { @@ -217,20 +291,35 @@ export function ScreenView({ screen, values, onValueChange, dataSource, objects, return (
- {visibleScreenFields(screen, values).map((f) => ( -
-
+ ); + })}
); } @@ -245,7 +334,21 @@ export function ScreenView({ screen, values, onValueChange, dataSource, objects, * defect; `__tests__/spec-symbol-parity.test.ts` pins that the spec does not own * this name. */ -export function ScreenFieldInput({ field, value, onChange }: { field: ScreenFieldSpec; value: unknown; onChange: (v: unknown) => void }) { +export function ScreenFieldInput({ + field, + value, + onChange, + describedBy, + dataSource, +}: { + field: ScreenFieldSpec; + value: unknown; + onChange: (v: unknown) => void; + /** Id of the element describing this control (its help text), for `aria-describedby`. */ + describedBy?: string; + /** Data source a `type: 'lookup'` picker queries its `reference` object through. */ + dataSource?: unknown; +}) { const id = `ff-${field.name}`; const t = (field.type || 'text').toLowerCase(); // The required STATE, announced on the control itself — the label's `*` is @@ -256,10 +359,33 @@ export function ScreenFieldInput({ field, value, onChange }: { field: ScreenFiel // `undefined` when optional, so the attribute is omitted rather than "false". const ariaRequired = field.required ? true : undefined; + // A lookup with a declared target resolves its picker from it (objectstack#17306). + // The widget is the object form's own `LookupField`, through the shared lazy + // registry — so it sits behind a Suspense of its own: the Studio preview + // renders this body with no boundary around it, and an unbounded suspension + // would swap whatever ancestor boundary it reached for a fallback. + // `dataSource` is passed explicitly; absent, the widget falls back to the + // `SchemaRendererContext` one, exactly as it does in `ActionParamDialog`. + if (t === 'lookup' && typeof field.reference === 'string' && field.reference.trim() !== '') { + return ( + + ); + } + if (Array.isArray(field.options) && field.options.length > 0) { return (