Skip to content

Commit ca44673

Browse files
committed
fix(console): a long flow screen or param list keeps Submit / Confirm on screen (objectui#12080)
A flat flow screen and the action parameter dialog had no height bound, so a screen or params list taller than the window overflowed both edges of the fixed, scroll-locked overlay: heading above the top, primary action below the bottom, nothing a mouse wheel could move. Both dialogs are now a flex column at most 90vh tall: the header on top, the fields in the one scrolling region, the footer outside it. More than eight declared fields (or shown params) also widens the dialog to the object-form step's sm:max-w-3xl with two columns from sm up; shorter screens and lists keep their width and single column. Claude-Session: https://claude.ai/code/session_01B1gHb9baeX7oioD5sHVm7z Co-authored-by: Claude <noreply@anthropic.com>
1 parent 023f00d commit ca44673

5 files changed

Lines changed: 396 additions & 38 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
'@object-ui/app-shell': patch
3+
---
4+
5+
fix(console): a long flow screen or action parameter list keeps its Submit / Confirm on screen (objectui#12080)
6+
7+
A flow `screen` dialog without an object form, and the action parameter dialog, had no height bound. A screen or a `params` list taller than the window overflowed both edges of the fixed overlay: the heading above the top, the primary action below the bottom. The overlay locks page scrolling, so a mouse wheel moved nothing, and only keyboard focus could reach Submit.
8+
9+
Both dialogs are now at most `90vh` tall. The fields scroll in their own region between the header and a footer that stays on screen. A screen with more than eight declared fields, or a list of more than eight shown params, also gets the wider dialog an object-form step already had, with two columns from the `sm` breakpoint up. Shorter screens and lists keep their previous width and single column.
10+
11+
No new metadata key: the dialog sizes itself by the number of fields.
Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
/**
10+
* objectui#12080 — the action parameter dialog takes the screen dialog's bound:
11+
* at most 90vh tall, the param list as the one scrolling region, Confirm in a
12+
* footer OUTSIDE it; a long list widens to two columns, a short one keeps the
13+
* default width.
14+
*
15+
* ⚠️ The test DOM computes no layout, so nothing here can say Confirm is ON
16+
* SCREEN — that was measured in Chromium with a real mouse wheel and a real
17+
* click, and the readings are on the pull request. These tests pin the
18+
* contract that measurement rests on, and that the two dialogs share it.
19+
*/
20+
import { describe, it, expect, vi } from 'vitest';
21+
import { render, screen } from '@testing-library/react';
22+
import type { ActionParamDef } from '@object-ui/core';
23+
import { ActionParamDialog } from './ActionParamDialog';
24+
import { FlowRunner } from './FlowRunner';
25+
26+
const params = (n: number, over: Partial<ActionParamDef> = {}): ActionParamDef[] =>
27+
Array.from({ length: n }, (_, i) => ({ name: `p${i}`, label: `Param ${i}`, type: 'text', ...over }));
28+
29+
function openDialog(list: ActionParamDef[]) {
30+
render(
31+
<ActionParamDialog
32+
state={{ open: true, params: list, title: 'Backfill Executed Contract', resolve: vi.fn() }}
33+
onOpenChange={() => {}}
34+
/>,
35+
);
36+
return { content: screen.getByRole('dialog'), body: screen.getByTestId('action-param-body') };
37+
}
38+
39+
/** `a` comes before `b` in document order. */
40+
const precedes = (a: Node, b: Node) => (a.compareDocumentPosition(b) & Node.DOCUMENT_POSITION_FOLLOWING) !== 0;
41+
42+
/** The tokens that make up the bound, on the dialog and on its scrolling body. */
43+
const CONTENT_BOUND = ['flex', 'flex-col', 'max-h-[90vh]'];
44+
const BODY_SCROLL = ['-mx-6', 'px-6', 'min-h-0', 'flex-1', 'overflow-y-auto'];
45+
46+
describe('ActionParamDialog — Confirm stays reachable on a long `params` list (objectui#12080)', () => {
47+
it('bounds a long list, scrolls only the list, and keeps Confirm outside the scrolling region', async () => {
48+
const { content, body } = openDialog(params(13));
49+
for (const token of CONTENT_BOUND) expect(content).toHaveClass(token);
50+
expect(content).not.toHaveClass('overflow-y-auto');
51+
for (const token of BODY_SCROLL) expect(body).toHaveClass(token);
52+
53+
const inputs = await screen.findAllByRole('textbox');
54+
expect(inputs).toHaveLength(13);
55+
for (const input of inputs) expect(body).toContainElement(input);
56+
const heading = screen.getByRole('heading', { name: 'Backfill Executed Contract' });
57+
const confirm = screen.getByRole('button', { name: /confirm/i });
58+
const cancel = screen.getByRole('button', { name: /cancel/i });
59+
for (const outside of [heading, confirm, cancel]) expect(body).not.toContainElement(outside);
60+
expect(precedes(heading, body)).toBe(true);
61+
expect(precedes(body, confirm)).toBe(true);
62+
});
63+
64+
it('widens a long list to two columns from `sm` up', () => {
65+
const { content, body } = openDialog(params(13));
66+
expect(content).toHaveClass('sm:max-w-3xl');
67+
expect(body).toHaveClass('grid');
68+
expect(body).toHaveClass('sm:grid-cols-2');
69+
});
70+
71+
it('control: a short list keeps the default width and its single column', () => {
72+
const { content, body } = openDialog(params(2));
73+
// No width class of its own, so `DialogContent`'s default `max-w-lg` holds.
74+
expect(content.className).not.toMatch(/sm:max-w-/);
75+
expect(content).toHaveClass('max-w-lg');
76+
expect(body).not.toHaveClass('sm:grid-cols-2');
77+
expect(content).toHaveClass('max-h-[90vh]');
78+
expect(body).not.toContainElement(screen.getByRole('button', { name: /confirm/i }));
79+
});
80+
81+
it.each([
82+
[8, false],
83+
[9, true],
84+
])('switches width at more than eight params: %i params wide=%s', (n, wide) => {
85+
const { content } = openDialog(params(n));
86+
expect(content.classList.contains('sm:max-w-3xl')).toBe(wide);
87+
});
88+
89+
it('counts the params the dialog SHOWS: a param gated off by `visible: false` does not widen it', () => {
90+
const list = [...params(8), { name: 'hidden', label: 'Hidden', type: 'text', visible: 'false' }];
91+
const { content } = openDialog(list);
92+
expect(content).not.toHaveClass('sm:max-w-3xl');
93+
});
94+
95+
it('is the SAME bound as the flow screen dialog, so the two cannot drift apart unseen', () => {
96+
openDialog(params(13));
97+
const paramContent = screen.getByRole('dialog');
98+
// A second modal marks the first `aria-hidden`, so both are read with `hidden: true`.
99+
const paramBody = screen.getByTestId('action-param-body');
100+
render(
101+
<FlowRunner
102+
state={{
103+
flowName: 'contract_intake',
104+
runId: 'run-1',
105+
screen: {
106+
nodeId: 'details',
107+
fields: Array.from({ length: 13 }, (_, i) => ({ name: `f${i}`, label: `Field ${i}`, type: 'text' })),
108+
},
109+
}}
110+
authFetch={vi.fn()}
111+
baseUrl=""
112+
onClose={() => {}}
113+
onComplete={() => {}}
114+
/>,
115+
);
116+
const screenContent = screen.getAllByRole('dialog', { hidden: true }).find((d) => d !== paramContent)!;
117+
const screenBody = screen.getByTestId('flow-screen-body');
118+
for (const token of [...CONTENT_BOUND, 'sm:max-w-3xl']) {
119+
expect(paramContent).toHaveClass(token);
120+
expect(screenContent).toHaveClass(token);
121+
}
122+
for (const token of BODY_SCROLL) {
123+
expect(paramBody).toHaveClass(token);
124+
expect(screenBody).toHaveClass(token);
125+
}
126+
});
127+
});

‎packages/app-shell/src/views/ActionParamDialog.tsx‎

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,17 @@
2222
* objectui#8672 ruling A for the reference-bearing pickers).
2323
*
2424
* Returns collected param values or null on cancel.
25+
*
26+
* ## A long `params` list still reaches Confirm (objectui#12080)
27+
*
28+
* The dialog takes the bound `FlowRunner`'s screen dialog has: at most `90vh`
29+
* tall, the param list in the ONE scrolling region, the header above it and
30+
* the footer below it. The upstream `DialogContent` has no height bound, so a
31+
* list taller than the window used to overflow both edges of a fixed,
32+
* scroll-locked overlay — its first required params above the top, Confirm
33+
* below the bottom, and nothing a mouse wheel could move. A list of more than
34+
* {@link NARROW_PARAM_MAX_COUNT} params also widens to two columns from `sm`
35+
* up; a shorter one keeps the default width and its single column.
2536
*/
2637

2738
import { Suspense, useState, useEffect, useMemo } from 'react';
@@ -37,6 +48,7 @@ import {
3748
Collapsible,
3849
CollapsibleTrigger,
3950
CollapsibleContent,
51+
cn,
4052
} from '@object-ui/components';
4153
import { ChevronDown, Lock } from 'lucide-react';
4254
import { useObjectTranslation, pickLocalized } from '@object-ui/i18n';
@@ -75,6 +87,14 @@ interface ActionParamDialogProps {
7587
onOpenChange: (open: boolean) => void;
7688
}
7789

90+
/**
91+
* The most params the dialog keeps its default single-column width for; a
92+
* longer list gets the wide two-column dialog (objectui#12080 — see the
93+
* header). The same count as `FlowRunner`'s `NARROW_SCREEN_MAX_FIELDS`, so a
94+
* flow screen and an action's params lay out alike.
95+
*/
96+
const NARROW_PARAM_MAX_COUNT = 8;
97+
7898
/**
7999
* Filter action params by their optional `visible` predicate, evaluated on the
80100
* canonical entry (`evalRowPredicate`) against the host predicate scope
@@ -478,11 +498,17 @@ export function ActionParamDialog({ state, onOpenChange }: ActionParamDialogProp
478498
setErrors(prev => ({ ...prev, [name]: false }));
479499
};
480500

501+
// A long list widens to two columns (objectui#12080 — see the header).
502+
const isLongList = visibleParams.length > NARROW_PARAM_MAX_COUNT;
503+
481504
return (
482505
<Dialog open={state.open} onOpenChange={(open) => {
483506
if (!open) handleCancel();
484507
}}>
485-
<DialogContent>
508+
{/* Bounded (objectui#12080): a flex column at most 90vh tall, in which
509+
only the body below can give up height (`min-h-0`) and scroll, so the
510+
header and the footer always stay on screen. */}
511+
<DialogContent className={cn('flex max-h-[90vh] flex-col', isLongList && 'sm:max-w-3xl')}>
486512
<DialogHeader>
487513
<DialogTitle>{state.title || t('actionDialog.title')}</DialogTitle>
488514
{/* `whitespace-pre-line` so a description composed of more than one
@@ -494,7 +520,13 @@ export function ActionParamDialog({ state, onOpenChange }: ActionParamDialogProp
494520
</DialogDescription>
495521
</DialogHeader>
496522

497-
<div className="grid gap-4 py-4">
523+
{/* The ONE scrolling region (objectui#12080). `-mx-6 px-6` runs it to
524+
the dialog's edges, so the scrollbar sits on the border and a
525+
control's focus ring is not clipped at the sides. */}
526+
<div
527+
className={cn('-mx-6 grid min-h-0 flex-1 gap-4 overflow-y-auto px-6 py-4', isLongList && 'sm:grid-cols-2')}
528+
data-testid="action-param-body"
529+
>
498530
{visibleParams.map((rawParam) => {
499531
const param = {
500532
...rawParam,

‎packages/app-shell/src/views/FlowRunner.tsx‎

Lines changed: 83 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,11 @@
3939
* a retry that cannot work; the input and the reason stay on screen
4040
* together.
4141
* 2. **The message had one carrier, and it was the transient one.** The toast
42-
* is kept — it is the console's failure idiom and it is viewport-fixed, so
43-
* it survives a tall `object-form` step scrolled past its own header — and
44-
* an inline destructive `Alert` (`role="alert"`) now carries the same
45-
* sentence inside the dialog, next to the values that produced it.
42+
* is kept — it is the console's failure idiom and it is viewport-fixed —
43+
* and an inline destructive `Alert` (`role="alert"`) now carries the same
44+
* sentence inside the dialog, next to the values that produced it. It sits
45+
* above the scrolling body (objectui#12080), so it stays in view however
46+
* far a tall screen is scrolled.
4647
* 3. **Success invalidated the wrong thing.** Both hosts answer `onComplete`
4748
* with `notifyDataChanged({ objectName: <this page's object> })`, which is
4849
* the record the user is LOOKING at — never the record the flow WROTE. The
@@ -143,6 +144,25 @@
143144
* `defaultValue` each key carries, which `check:i18n-keys` pins to its `en`
144145
* value. The server's own refusal sentence is passed through untranslated by
145146
* design — it is prose the backend composed, not a string with a key.
147+
*
148+
* ## A long screen still reaches its Submit (objectui#12080)
149+
*
150+
* The dialog is height-bounded on every screen, not only on an `object-form`
151+
* step: at most `90vh` tall, with the header (and any notice under it) on top,
152+
* the screen body in the ONE scrolling region, and the footer below it. The
153+
* upstream `DialogContent` has no height bound, so a flat screen taller than
154+
* the window used to overflow both edges of a fixed, scroll-locked overlay —
155+
* the heading above the top, Submit below the bottom, and nothing a mouse
156+
* wheel could move. Keeping the footer OUTSIDE the scrolling region is what
157+
* keeps Submit on screen however long the screen is.
158+
*
159+
* Width follows the screen's length, not a layout key: no spec key asks for
160+
* one, and the renderer can size by content. A flat screen declaring more
161+
* than {@link NARROW_SCREEN_MAX_FIELDS} fields gets the `object-form` step's
162+
* wider dialog and two columns from `sm` up; a shorter one keeps the narrow
163+
* single column it always had. The count is the DECLARED one, so a field a
164+
* `visibleWhen` reveals mid-edit never makes the dialog jump width.
165+
* `ActionParamDialog` takes the same bound for a long `params` list.
146166
*/
147167
import { Suspense, useEffect, useState } from 'react';
148168
import {
@@ -155,6 +175,7 @@ import {
155175
DialogTitle,
156176
DialogDescription,
157177
Button,
178+
cn,
158179
} from '@object-ui/components';
159180
import { notifyDataChanged, useObjectTranslation } from '@object-ui/react';
160181
import {
@@ -171,6 +192,7 @@ import {
171192
isObjectFormScreen,
172193
initialScreenValues,
173194
screenFieldBoundViolations,
195+
screenFields,
174196
visibleScreenFields,
175197
type ScreenFieldSpec,
176198
type ScreenSpec,
@@ -179,6 +201,16 @@ import { interpretFlowResponse } from '../utils/flowResponse.js';
179201

180202
export type { ScreenSpec, ScreenFieldSpec } from './ScreenView.js';
181203

204+
/**
205+
* The most declared fields a flat screen keeps the narrow single-column dialog
206+
* for; a longer one gets the wide two-column dialog (objectui#12080 — see the
207+
* header). Eight is the most single-line fields the narrow column was
208+
* measured to show whole, with no scrolling, inside the `90vh` bound on a
209+
* 1440×900 window — a reading taken once in Chromium for objectui#12080 and
210+
* recorded on its pull request; nothing here re-derives it.
211+
*/
212+
const NARROW_SCREEN_MAX_FIELDS = 8;
213+
182214
/** The `flows` group of a spec `TranslationData` — typed by the spec, so the address below is checked against it. */
183215
type FlowsTranslation = NonNullable<TranslationData['flows']>;
184216

@@ -402,9 +434,9 @@ export function FlowRunner({ state, authFetch, baseUrl, onClose, onComplete, dat
402434
// #31). See utils/flowResponse.
403435
const outcome = interpretFlowResponse<ScreenSpec>(res, json, 'Resume');
404436
if (outcome.kind === 'failed') {
405-
// Two carriers on purpose (`c40f3b8ca`): the toast is fixed to the viewport and
406-
// reaches a user scrolled to the bottom of a tall object-form step; the
407-
// inline Alert stays with the values that caused it.
437+
// Two carriers on purpose (`c40f3b8ca`): the toast is the console's
438+
// viewport-fixed failure idiom; the inline Alert stays in the dialog,
439+
// above the scrolling body, with the values that caused it.
408440
toast.error(outcome.error);
409441
setResumeError({ message: outcome.error, retryable: outcome.retryable });
410442
return;
@@ -531,10 +563,17 @@ export function FlowRunner({ state, authFetch, baseUrl, onClose, onComplete, dat
531563
// withdrawing the submit affordance is one behaviour with two causes, and
532564
// splitting it is how the `object-form` arm of it gets forgotten.
533565
const terminal = refused !== null || (resumeError !== null && !resumeError.retryable);
566+
// A long flat screen widens to two columns (objectui#12080 — see the header).
567+
const isLongScreen = !isObjectForm && screenFields(screen).length > NARROW_SCREEN_MAX_FIELDS;
534568

535569
return (
536570
<Dialog open onOpenChange={(o) => { if (!o && !submitting) onClose(); }}>
537-
<DialogContent className={isObjectForm ? 'sm:max-w-3xl max-h-[90vh] overflow-y-auto' : 'sm:max-w-md'}>
571+
{/* Bounded on every screen (objectui#12080): a flex column at most 90vh
572+
tall, in which only the body below can give up height (`min-h-0`)
573+
and scroll, so the header and the footer always stay on screen. */}
574+
<DialogContent
575+
className={cn('flex max-h-[90vh] flex-col', isObjectForm || isLongScreen ? 'sm:max-w-3xl' : 'sm:max-w-md')}
576+
>
538577
<DialogHeader>
539578
{/* The flow being run (objectui#11092), above the step's own heading,
540579
which stays the dialog's title. */}
@@ -565,34 +604,42 @@ export function FlowRunner({ state, authFetch, baseUrl, onClose, onComplete, dat
565604
</Alert>
566605
)}
567606

568-
{/* The screen body pulls in lazily-loaded chunks (an `object-form` step
569-
mounts ObjectForm, whose field widgets are lazy). Without a boundary
570-
HERE, that suspension unwinds to the host's nearest <Suspense> — a
571-
route-level one on some surfaces — which swaps the whole page for a
572-
fallback and destroys the host's state, taking this dialog (and the
573-
run it is driving) with it. */}
574-
<Suspense fallback={<div className="py-6 text-sm text-muted-foreground">{t('common.loading', { defaultValue: 'Loading…' })}</div>}>
575-
<ScreenView
576-
screen={shown}
577-
values={values}
578-
onValueChange={setVal}
579-
dataSource={dataSource}
580-
objects={objects}
581-
objectForm={{
582-
onSuccess: onObjectFormSaved,
583-
onCancel: onClose,
584-
// Withdrawn once the run is gone: the record this step created was
585-
// already persisted, so a second Save would duplicate it AND still
586-
// have no suspension to resume.
587-
showSubmit: !terminal,
588-
showCancel: true,
589-
submitText: t('flowRunner.saveAndContinue', { defaultValue: 'Save & Continue' }),
590-
cancelText: terminal
591-
? t('common.close', { defaultValue: 'Close' })
592-
: t('common.cancel', { defaultValue: 'Cancel' }),
593-
}}
594-
/>
595-
</Suspense>
607+
{/* The ONE scrolling region (objectui#12080). `-mx-6 px-6` runs it to
608+
the dialog's edges, so the scrollbar sits on the border and a
609+
control's focus ring is not clipped at the sides. */}
610+
<div className="-mx-6 min-h-0 flex-1 overflow-y-auto px-6" data-testid="flow-screen-body">
611+
{/* The screen body pulls in lazily-loaded chunks (an `object-form` step
612+
mounts ObjectForm, whose field widgets are lazy). Without a boundary
613+
HERE, that suspension unwinds to the host's nearest <Suspense> — a
614+
route-level one on some surfaces — which swaps the whole page for a
615+
fallback and destroys the host's state, taking this dialog (and the
616+
run it is driving) with it. */}
617+
<Suspense fallback={<div className="py-6 text-sm text-muted-foreground">{t('common.loading', { defaultValue: 'Loading…' })}</div>}>
618+
<ScreenView
619+
screen={shown}
620+
values={values}
621+
onValueChange={setVal}
622+
dataSource={dataSource}
623+
objects={objects}
624+
// Two columns from `sm` up; `space-y-0` replaces the body's own
625+
// stack spacing, which would offset every grid cell but the last.
626+
className={isLongScreen ? 'grid gap-4 space-y-0 sm:grid-cols-2' : undefined}
627+
objectForm={{
628+
onSuccess: onObjectFormSaved,
629+
onCancel: onClose,
630+
// Withdrawn once the run is gone: the record this step created was
631+
// already persisted, so a second Save would duplicate it AND still
632+
// have no suspension to resume.
633+
showSubmit: !terminal,
634+
showCancel: true,
635+
submitText: t('flowRunner.saveAndContinue', { defaultValue: 'Save & Continue' }),
636+
cancelText: terminal
637+
? t('common.close', { defaultValue: 'Close' })
638+
: t('common.cancel', { defaultValue: 'Cancel' }),
639+
}}
640+
/>
641+
</Suspense>
642+
</div>
596643

597644
{!isObjectForm && (
598645
<DialogFooter>

0 commit comments

Comments
 (0)