fix(console): a long flow screen or action parameter list keeps Submit / Confirm on screen (objectui#12080)

.changeset/12080-screen-dialog-reach.md

@@ -0,0 +1,11 @@
+---
+'@object-ui/app-shell': patch
+---
+
+fix(console): a long flow screen or action parameter list keeps its Submit / Confirm on screen (objectui#12080)
+
+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.
+
+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.
+
+No new metadata key: the dialog sizes itself by the number of fields.

packages/app-shell/src/views/ActionParamDialog.dialogReach-12080.test.tsx

@@ -0,0 +1,127 @@
+/**
+ * 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#12080 — the action parameter dialog takes the screen dialog's bound:
+ * at most 90vh tall, the param list as the one scrolling region, Confirm in a
+ * footer OUTSIDE it; a long list widens to two columns, a short one keeps the
+ * default width.
+ *
+ * ⚠️ The test DOM computes no layout, so nothing here can say Confirm is ON
+ * SCREEN — that was measured in Chromium with a real mouse wheel and a real
+ * click, and the readings are on the pull request. These tests pin the
+ * contract that measurement rests on, and that the two dialogs share it.
+ */
+import { describe, it, expect, vi } from 'vitest';
+import { render, screen } from '@testing-library/react';
+import type { ActionParamDef } from '@object-ui/core';
+import { ActionParamDialog } from './ActionParamDialog';
+import { FlowRunner } from './FlowRunner';
+
+const params = (n: number, over: Partial<ActionParamDef> = {}): ActionParamDef[] =>
+  Array.from({ length: n }, (_, i) => ({ name: `p${i}`, label: `Param ${i}`, type: 'text', ...over }));
+
+function openDialog(list: ActionParamDef[]) {
+  render(
+    <ActionParamDialog
+      state={{ open: true, params: list, title: 'Backfill Executed Contract', resolve: vi.fn() }}
+      onOpenChange={() => {}}
+    />,
+  );
+  return { content: screen.getByRole('dialog'), body: screen.getByTestId('action-param-body') };
+}
+
+/** `a` comes before `b` in document order. */
+const precedes = (a: Node, b: Node) => (a.compareDocumentPosition(b) & Node.DOCUMENT_POSITION_FOLLOWING) !== 0;
+
+/** The tokens that make up the bound, on the dialog and on its scrolling body. */
+const CONTENT_BOUND = ['flex', 'flex-col', 'max-h-[90vh]'];
+const BODY_SCROLL = ['-mx-6', 'px-6', 'min-h-0', 'flex-1', 'overflow-y-auto'];
+
+describe('ActionParamDialog — Confirm stays reachable on a long `params` list (objectui#12080)', () => {
+  it('bounds a long list, scrolls only the list, and keeps Confirm outside the scrolling region', async () => {
+    const { content, body } = openDialog(params(13));
+    for (const token of CONTENT_BOUND) expect(content).toHaveClass(token);
+    expect(content).not.toHaveClass('overflow-y-auto');
+    for (const token of BODY_SCROLL) expect(body).toHaveClass(token);
+
+    const inputs = await screen.findAllByRole('textbox');
+    expect(inputs).toHaveLength(13);
+    for (const input of inputs) expect(body).toContainElement(input);
+    const heading = screen.getByRole('heading', { name: 'Backfill Executed Contract' });
+    const confirm = screen.getByRole('button', { name: /confirm/i });
+    const cancel = screen.getByRole('button', { name: /cancel/i });
+    for (const outside of [heading, confirm, cancel]) expect(body).not.toContainElement(outside);
+    expect(precedes(heading, body)).toBe(true);
+    expect(precedes(body, confirm)).toBe(true);
+  });
+
+  it('widens a long list to two columns from `sm` up', () => {
+    const { content, body } = openDialog(params(13));
+    expect(content).toHaveClass('sm:max-w-3xl');
+    expect(body).toHaveClass('grid');
+    expect(body).toHaveClass('sm:grid-cols-2');
+  });
+
+  it('control: a short list keeps the default width and its single column', () => {
+    const { content, body } = openDialog(params(2));
+    // No width class of its own, so `DialogContent`'s default `max-w-lg` holds.
+    expect(content.className).not.toMatch(/sm:max-w-/);
+    expect(content).toHaveClass('max-w-lg');
+    expect(body).not.toHaveClass('sm:grid-cols-2');
+    expect(content).toHaveClass('max-h-[90vh]');
+    expect(body).not.toContainElement(screen.getByRole('button', { name: /confirm/i }));
+  });
+
+  it.each([
+    [8, false],
+    [9, true],
+  ])('switches width at more than eight params: %i params wide=%s', (n, wide) => {
+    const { content } = openDialog(params(n));
+    expect(content.classList.contains('sm:max-w-3xl')).toBe(wide);
+  });
+
+  it('counts the params the dialog SHOWS: a param gated off by `visible: false` does not widen it', () => {
+    const list = [...params(8), { name: 'hidden', label: 'Hidden', type: 'text', visible: 'false' }];
+    const { content } = openDialog(list);
+    expect(content).not.toHaveClass('sm:max-w-3xl');
+  });
+
+  it('is the SAME bound as the flow screen dialog, so the two cannot drift apart unseen', () => {
+    openDialog(params(13));
+    const paramContent = screen.getByRole('dialog');
+    // A second modal marks the first `aria-hidden`, so both are read with `hidden: true`.
+    const paramBody = screen.getByTestId('action-param-body');
+    render(
+      <FlowRunner
+        state={{
+          flowName: 'contract_intake',
+          runId: 'run-1',
+          screen: {
+            nodeId: 'details',
+            fields: Array.from({ length: 13 }, (_, i) => ({ name: `f${i}`, label: `Field ${i}`, type: 'text' })),
+          },
+        }}
+        authFetch={vi.fn()}
+        baseUrl=""
+        onClose={() => {}}
+        onComplete={() => {}}
+      />,
+    );
+    const screenContent = screen.getAllByRole('dialog', { hidden: true }).find((d) => d !== paramContent)!;
+    const screenBody = screen.getByTestId('flow-screen-body');
+    for (const token of [...CONTENT_BOUND, 'sm:max-w-3xl']) {
+      expect(paramContent).toHaveClass(token);
+      expect(screenContent).toHaveClass(token);
+    }
+    for (const token of BODY_SCROLL) {
+      expect(paramBody).toHaveClass(token);
+      expect(screenBody).toHaveClass(token);
+    }
+  });
+});

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

@@ -22,6 +22,17 @@
  * objectui#8672 ruling A for the reference-bearing pickers).
  *
  * Returns collected param values or null on cancel.
+ *
+ * ## A long `params` list still reaches Confirm (objectui#12080)
+ *
+ * The dialog takes the bound `FlowRunner`'s screen dialog has: at most `90vh`
+ * tall, the param list in the ONE scrolling region, the header above it and
+ * the footer below it. The upstream `DialogContent` has no height bound, so a
+ * list taller than the window used to overflow both edges of a fixed,
+ * scroll-locked overlay — its first required params above the top, Confirm
+ * below the bottom, and nothing a mouse wheel could move. A list of more than
+ * {@link NARROW_PARAM_MAX_COUNT} params also widens to two columns from `sm`
+ * up; a shorter one keeps the default width and its single column.
  */
 
 import { Suspense, useState, useEffect, useMemo } from 'react';
@@ -37,6 +48,7 @@ import {
   Collapsible,
   CollapsibleTrigger,
   CollapsibleContent,
+  cn,
 } from '@object-ui/components';
 import { ChevronDown, Lock } from 'lucide-react';
 import { useObjectTranslation, pickLocalized } from '@object-ui/i18n';
@@ -75,6 +87,14 @@ interface ActionParamDialogProps {
   onOpenChange: (open: boolean) => void;
 }
 
+/**
+ * The most params the dialog keeps its default single-column width for; a
+ * longer list gets the wide two-column dialog (objectui#12080 — see the
+ * header). The same count as `FlowRunner`'s `NARROW_SCREEN_MAX_FIELDS`, so a
+ * flow screen and an action's params lay out alike.
+ */
+const NARROW_PARAM_MAX_COUNT = 8;
+
 /**
  * Filter action params by their optional `visible` predicate, evaluated on the
  * canonical entry (`evalRowPredicate`) against the host predicate scope
@@ -478,11 +498,17 @@ export function ActionParamDialog({ state, onOpenChange }: ActionParamDialogProp
     setErrors(prev => ({ ...prev, [name]: false }));
   };
 
+  // A long list widens to two columns (objectui#12080 — see the header).
+  const isLongList = visibleParams.length > NARROW_PARAM_MAX_COUNT;
+
   return (
     <Dialog open={state.open} onOpenChange={(open) => {
       if (!open) handleCancel();
     }}>
-      <DialogContent>
+      {/* Bounded (objectui#12080): a flex column at most 90vh tall, in which
+          only the body below can give up height (`min-h-0`) and scroll, so the
+          header and the footer always stay on screen. */}
+      <DialogContent className={cn('flex max-h-[90vh] flex-col', isLongList && 'sm:max-w-3xl')}>
         <DialogHeader>
           <DialogTitle>{state.title || t('actionDialog.title')}</DialogTitle>
           {/* `whitespace-pre-line` so a description composed of more than one
@@ -494,7 +520,13 @@ export function ActionParamDialog({ state, onOpenChange }: ActionParamDialogProp
           </DialogDescription>
         </DialogHeader>
 
-        <div className="grid gap-4 py-4">
+        {/* The ONE scrolling region (objectui#12080). `-mx-6 px-6` runs it to
+            the dialog's edges, so the scrollbar sits on the border and a
+            control's focus ring is not clipped at the sides. */}
+        <div
+          className={cn('-mx-6 grid min-h-0 flex-1 gap-4 overflow-y-auto px-6 py-4', isLongList && 'sm:grid-cols-2')}
+          data-testid="action-param-body"
+        >
           {visibleParams.map((rawParam) => {
             const param = {
               ...rawParam,

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

@@ -39,10 +39,11 @@
  *    a retry that cannot work; the input and the reason stay on screen
  *    together.
  * 2. **The message had one carrier, and it was the transient one.** The toast
- *    is kept — it is the console's failure idiom and it is viewport-fixed, so
- *    it survives a tall `object-form` step scrolled past its own header — and
- *    an inline destructive `Alert` (`role="alert"`) now carries the same
- *    sentence inside the dialog, next to the values that produced it.
+ *    is kept — it is the console's failure idiom and it is viewport-fixed —
+ *    and an inline destructive `Alert` (`role="alert"`) now carries the same
+ *    sentence inside the dialog, next to the values that produced it. It sits
+ *    above the scrolling body (objectui#12080), so it stays in view however
+ *    far a tall screen is scrolled.
  * 3. **Success invalidated the wrong thing.** Both hosts answer `onComplete`
  *    with `notifyDataChanged({ objectName: <this page's object> })`, which is
  *    the record the user is LOOKING at — never the record the flow WROTE. The
@@ -143,6 +144,25 @@
  * `defaultValue` each key carries, which `check:i18n-keys` pins to its `en`
  * value. The server's own refusal sentence is passed through untranslated by
  * design — it is prose the backend composed, not a string with a key.
+ *
+ * ## A long screen still reaches its Submit (objectui#12080)
+ *
+ * The dialog is height-bounded on every screen, not only on an `object-form`
+ * step: at most `90vh` tall, with the header (and any notice under it) on top,
+ * the screen body in the ONE scrolling region, and the footer below it. The
+ * upstream `DialogContent` has no height bound, so a flat screen taller than
+ * the window used to overflow both edges of a fixed, scroll-locked overlay —
+ * the heading above the top, Submit below the bottom, and nothing a mouse
+ * wheel could move. Keeping the footer OUTSIDE the scrolling region is what
+ * keeps Submit on screen however long the screen is.
+ *
+ * Width follows the screen's length, not a layout key: no spec key asks for
+ * one, and the renderer can size by content. A flat screen declaring more
+ * than {@link NARROW_SCREEN_MAX_FIELDS} fields gets the `object-form` step's
+ * wider dialog and two columns from `sm` up; a shorter one keeps the narrow
+ * single column it always had. The count is the DECLARED one, so a field a
+ * `visibleWhen` reveals mid-edit never makes the dialog jump width.
+ * `ActionParamDialog` takes the same bound for a long `params` list.
  */
 import { Suspense, useEffect, useState } from 'react';
 import {
@@ -155,6 +175,7 @@ import {
   DialogTitle,
   DialogDescription,
   Button,
+  cn,
 } from '@object-ui/components';
 import { notifyDataChanged, useObjectTranslation } from '@object-ui/react';
 import {
@@ -171,6 +192,7 @@ import {
   isObjectFormScreen,
   initialScreenValues,
   screenFieldBoundViolations,
+  screenFields,
   visibleScreenFields,
   type ScreenFieldSpec,
   type ScreenSpec,
@@ -179,6 +201,16 @@ import { interpretFlowResponse } from '../utils/flowResponse.js';
 
 export type { ScreenSpec, ScreenFieldSpec } from './ScreenView.js';
 
+/**
+ * The most declared fields a flat screen keeps the narrow single-column dialog
+ * for; a longer one gets the wide two-column dialog (objectui#12080 — see the
+ * header). Eight is the most single-line fields the narrow column was
+ * measured to show whole, with no scrolling, inside the `90vh` bound on a
+ * 1440×900 window — a reading taken once in Chromium for objectui#12080 and
+ * recorded on its pull request; nothing here re-derives it.
+ */
+const NARROW_SCREEN_MAX_FIELDS = 8;
+
 /** The `flows` group of a spec `TranslationData` — typed by the spec, so the address below is checked against it. */
 type FlowsTranslation = NonNullable<TranslationData['flows']>;
 
@@ -402,9 +434,9 @@ export function FlowRunner({ state, authFetch, baseUrl, onClose, onComplete, dat
     // #31). See utils/flowResponse.
     const outcome = interpretFlowResponse<ScreenSpec>(res, json, 'Resume');
     if (outcome.kind === 'failed') {
-      // Two carriers on purpose (`c40f3b8ca`): the toast is fixed to the viewport and
-      // reaches a user scrolled to the bottom of a tall object-form step; the
-      // inline Alert stays with the values that caused it.
+      // Two carriers on purpose (`c40f3b8ca`): the toast is the console's
+      // viewport-fixed failure idiom; the inline Alert stays in the dialog,
+      // above the scrolling body, with the values that caused it.
       toast.error(outcome.error);
       setResumeError({ message: outcome.error, retryable: outcome.retryable });
       return;
@@ -531,10 +563,17 @@ export function FlowRunner({ state, authFetch, baseUrl, onClose, onComplete, dat
   // withdrawing the submit affordance is one behaviour with two causes, and
   // splitting it is how the `object-form` arm of it gets forgotten.
   const terminal = refused !== null || (resumeError !== null && !resumeError.retryable);
+  // A long flat screen widens to two columns (objectui#12080 — see the header).
+  const isLongScreen = !isObjectForm && screenFields(screen).length > NARROW_SCREEN_MAX_FIELDS;
 
   return (
     <Dialog open onOpenChange={(o) => { if (!o && !submitting) onClose(); }}>
-      <DialogContent className={isObjectForm ? 'sm:max-w-3xl max-h-[90vh] overflow-y-auto' : 'sm:max-w-md'}>
+      {/* Bounded on every screen (objectui#12080): a flex column at most 90vh
+          tall, in which only the body below can give up height (`min-h-0`)
+          and scroll, so the header and the footer always stay on screen. */}
+      <DialogContent
+        className={cn('flex max-h-[90vh] flex-col', isObjectForm || isLongScreen ? 'sm:max-w-3xl' : 'sm:max-w-md')}
+      >
         <DialogHeader>
           {/* The flow being run (objectui#11092), above the step's own heading,
               which stays the dialog's title. */}
@@ -565,34 +604,42 @@ export function FlowRunner({ state, authFetch, baseUrl, onClose, onComplete, dat
           </Alert>
         )}
 
-        {/* The screen body pulls in lazily-loaded chunks (an `object-form` step
-            mounts ObjectForm, whose field widgets are lazy). Without a boundary
-            HERE, that suspension unwinds to the host's nearest <Suspense> — a
-            route-level one on some surfaces — which swaps the whole page for a
-            fallback and destroys the host's state, taking this dialog (and the
-            run it is driving) with it. */}
-        <Suspense fallback={<div className="py-6 text-sm text-muted-foreground">{t('common.loading', { defaultValue: 'Loading…' })}</div>}>
-          <ScreenView
-            screen={shown}
-            values={values}
-            onValueChange={setVal}
-            dataSource={dataSource}
-            objects={objects}
-            objectForm={{
-              onSuccess: onObjectFormSaved,
-              onCancel: onClose,
-              // Withdrawn once the run is gone: the record this step created was
-              // already persisted, so a second Save would duplicate it AND still
-              // have no suspension to resume.
-              showSubmit: !terminal,
-              showCancel: true,
-              submitText: t('flowRunner.saveAndContinue', { defaultValue: 'Save & Continue' }),
-              cancelText: terminal
-                ? t('common.close', { defaultValue: 'Close' })
-                : t('common.cancel', { defaultValue: 'Cancel' }),
-            }}
-          />
-        </Suspense>
+        {/* The ONE scrolling region (objectui#12080). `-mx-6 px-6` runs it to
+            the dialog's edges, so the scrollbar sits on the border and a
+            control's focus ring is not clipped at the sides. */}
+        <div className="-mx-6 min-h-0 flex-1 overflow-y-auto px-6" data-testid="flow-screen-body">
+          {/* The screen body pulls in lazily-loaded chunks (an `object-form` step
+              mounts ObjectForm, whose field widgets are lazy). Without a boundary
+              HERE, that suspension unwinds to the host's nearest <Suspense> — a
+              route-level one on some surfaces — which swaps the whole page for a
+              fallback and destroys the host's state, taking this dialog (and the
+              run it is driving) with it. */}
+          <Suspense fallback={<div className="py-6 text-sm text-muted-foreground">{t('common.loading', { defaultValue: 'Loading…' })}</div>}>
+            <ScreenView
+              screen={shown}
+              values={values}
+              onValueChange={setVal}
+              dataSource={dataSource}
+              objects={objects}
+              // Two columns from `sm` up; `space-y-0` replaces the body's own
+              // stack spacing, which would offset every grid cell but the last.
+              className={isLongScreen ? 'grid gap-4 space-y-0 sm:grid-cols-2' : undefined}
+              objectForm={{
+                onSuccess: onObjectFormSaved,
+                onCancel: onClose,
+                // Withdrawn once the run is gone: the record this step created was
+                // already persisted, so a second Save would duplicate it AND still
+                // have no suspension to resume.
+                showSubmit: !terminal,
+                showCancel: true,
+                submitText: t('flowRunner.saveAndContinue', { defaultValue: 'Save & Continue' }),
+                cancelText: terminal
+                  ? t('common.close', { defaultValue: 'Close' })
+                  : t('common.cancel', { defaultValue: 'Cancel' }),
+              }}
+            />
+          </Suspense>
+        </div>
 
         {!isObjectForm && (
           <DialogFooter>