fix(plugin-form): object-form's modalCloseButton: false hides the modal's close button (objectui#11061)

.changeset/11061-modal-close-button.md

@@ -0,0 +1,20 @@
+---
+'@object-ui/components': minor
+'@object-ui/plugin-form': patch
+---
+
+fix(plugin-form): `object-form`'s `modalCloseButton: false` hides the modal's close button
+
+An author can now hide the close (X) button of a modal form. `modalCloseButton`
+was declared on `ObjectFormSchema` and on the `object-form` registration, and
+`ObjectForm`'s modal route forwarded it, but `ModalForm` never read it: `false`
+still drew the X, with no effect and no error. `ModalForm` now honours it on both
+of its dialog arms (the flat form and the `subforms` master-detail form). Only an
+explicit `false` hides the button; unset and `true` keep it. With the X hidden the
+modal still closes on Escape, and on the Cancel action when that is shown.
+
+`@object-ui/components`: `MobileDialogContent` gains an optional `showCloseButton`
+prop, default `true`, named after upstream Shadcn's `DialogContent` prop of the
+same purpose. `false` leaves the close button out of the DOM. Existing callers are
+unchanged. The component's exported props type gains this one optional member,
+which is why this package takes a minor bump.

content/docs/api/schema-reference.md

@@ -848,6 +848,7 @@ A smart form that auto-generates fields from an ObjectQL object. Supports simple
 | `showSubmit` / `showCancel` / `showReset` | `boolean` | Toggle action buttons. |
 | `drawerSide` | `string` | Drawer position: `"top"`, `"bottom"`, `"left"`, `"right"`. |
 | `modalSize` | `string` | Modal size: `"sm"`, `"default"`, `"lg"`, `"xl"`, `"full"`. |
+| `modalCloseButton` | `boolean` | Show the modal's close (X) button. Default `true`; `false` hides it. The modal still closes on Escape, and on the Cancel action when that is shown. |
 
 #### Spec alignment & extension keys
 

packages/components/src/__tests__/mobile-dialog-show-close-button-11061.test.tsx

@@ -0,0 +1,70 @@
+/**
+ * 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.
+ */
+
+/**
+ * `MobileDialogContent`'s `showCloseButton` switch (objectui#11061).
+ *
+ * The wrapper used to draw its close (X) button unconditionally, so
+ * `plugin-form`'s `ModalForm` had no way to honour an authored
+ * `modalCloseButton: false`. The switch defaults to `true`, which keeps every
+ * existing caller's button; only an explicit `false` leaves it out of the DOM.
+ * Hiding the X must not trap the user: Escape still reaches the `Dialog`'s
+ * `onOpenChange(false)`.
+ *
+ * No I18nProvider is mounted, so the button's accessible name is the English
+ * "Close" (see `mobile-dialog-close-no-provider-4024.test.tsx`).
+ */
+
+import { describe, it, expect, afterEach, vi } from 'vitest';
+import { render, screen, cleanup, fireEvent } from '@testing-library/react';
+import { Dialog, DialogTitle, DialogDescription } from '../ui/dialog';
+import { MobileDialogContent } from '../custom/mobile-dialog-content';
+
+afterEach(() => cleanup());
+
+function renderDialog(showCloseButton?: boolean, onOpenChange = vi.fn()) {
+  render(
+    <Dialog open onOpenChange={onOpenChange}>
+      <MobileDialogContent showCloseButton={showCloseButton}>
+        <DialogTitle>Acme Corp</DialogTitle>
+        <DialogDescription>Record modal</DialogDescription>
+      </MobileDialogContent>
+    </Dialog>,
+  );
+  return onOpenChange;
+}
+
+describe('MobileDialogContent showCloseButton (objectui#11061)', () => {
+  it('renders the close button by default', () => {
+    renderDialog();
+    expect(screen.getByRole('button', { name: 'Close' })).toBeTruthy();
+  });
+
+  it('renders the close button when showCloseButton is true', () => {
+    renderDialog(true);
+    expect(screen.getByRole('button', { name: 'Close' })).toBeTruthy();
+  });
+
+  it('leaves the close button out of the DOM when showCloseButton is false', () => {
+    renderDialog(false);
+    const dialog = screen.getByRole('dialog');
+    // The dialog itself still renders its content...
+    expect(screen.getByText('Acme Corp')).toBeTruthy();
+    // ...but carries no button at all, and the switch is not leaked onto the
+    // Radix content element as an unknown DOM attribute.
+    expect(screen.queryByRole('button', { name: /close/i })).toBeNull();
+    expect(dialog.querySelector('button')).toBeNull();
+    expect(dialog.hasAttribute('showclosebutton')).toBe(false);
+  });
+
+  it('still closes on Escape with the close button hidden', () => {
+    const onOpenChange = renderDialog(false);
+    fireEvent.keyDown(screen.getByRole('dialog'), { key: 'Escape' });
+    expect(onOpenChange).toHaveBeenCalledWith(false);
+  });
+});

packages/components/src/custom/mobile-dialog-content.tsx

@@ -106,10 +106,22 @@ export function usePopperAwareInteractOutside(
   );
 }
 
+export type MobileDialogContentProps = React.ComponentPropsWithoutRef<typeof DialogPrimitive.Content> & {
+  /**
+   * Whether the dialog draws its own close (X) button. Defaults to `true`;
+   * `false` leaves it out of the DOM entirely (objectui#11061), the same
+   * switch upstream Shadcn's `DialogContent` names `showCloseButton`. The
+   * dialog stays dismissable without it: Escape and a backdrop click still
+   * reach the `Dialog`'s `onOpenChange(false)` through Radix, so a caller that
+   * hides the X only has to keep whatever explicit way out its body offers.
+   */
+  showCloseButton?: boolean;
+};
+
 export const MobileDialogContent = React.forwardRef<
   React.ElementRef<typeof DialogPrimitive.Content>,
-  React.ComponentPropsWithoutRef<typeof DialogPrimitive.Content>
->(({ className, children, onInteractOutside, ...props }, ref) => {
+  MobileDialogContentProps
+>(({ className, children, onInteractOutside, showCloseButton = true, ...props }, ref) => {
   const handleInteractOutside = usePopperAwareInteractOutside(onInteractOutside);
   return (
   <DialogPortal>
@@ -135,26 +147,28 @@ export const MobileDialogContent = React.forwardRef<
       {...props}
     >
       {children}
-      <DialogPrimitive.Close
-        className={cn(
-          'absolute right-4 top-4 rounded-sm opacity-70 ring-offset-background transition-opacity',
-          'hover:opacity-100 focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2',
-          'disabled:pointer-events-none data-[state=open]:bg-accent data-[state=open]:text-muted-foreground',
-          // Mobile touch target ≥ 44×44px (WCAG 2.5.5)
-          'min-h-[44px] min-w-[44px] sm:min-h-0 sm:min-w-0 flex items-center justify-center',
-        )}
-      >
-        <X className="h-5 w-5 sm:h-4 sm:w-4" />
-        {/* objectui#4024 — the remainder objectstack#5505 could not reach.
-            That change routed the close label through `CloseSrLabel` for the
-            two SHADCN-SYNCED primitives under `src/ui/**`, via the declared
-            patch in `scripts/shadcn-local-patches.mjs`. This file is a
-            hand-written `custom/` wrapper with its own close button, outside
-            that regeneration zone, so it kept the English literal — and it is
-            what `plugin-form`'s `ModalForm` renders, i.e. exactly the
-            create/edit dialog the card measured. */}
-        <CloseSrLabel />
-      </DialogPrimitive.Close>
+      {showCloseButton && (
+        <DialogPrimitive.Close
+          className={cn(
+            'absolute right-4 top-4 rounded-sm opacity-70 ring-offset-background transition-opacity',
+            'hover:opacity-100 focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2',
+            'disabled:pointer-events-none data-[state=open]:bg-accent data-[state=open]:text-muted-foreground',
+            // Mobile touch target ≥ 44×44px (WCAG 2.5.5)
+            'min-h-[44px] min-w-[44px] sm:min-h-0 sm:min-w-0 flex items-center justify-center',
+          )}
+        >
+          <X className="h-5 w-5 sm:h-4 sm:w-4" />
+          {/* objectui#4024 — the remainder objectstack#5505 could not reach.
+              That change routed the close label through `CloseSrLabel` for the
+              two SHADCN-SYNCED primitives under `src/ui/**`, via the declared
+              patch in `scripts/shadcn-local-patches.mjs`. This file is a
+              hand-written `custom/` wrapper with its own close button, outside
+              that regeneration zone, so it kept the English literal — and it is
+              what `plugin-form`'s `ModalForm` renders, i.e. exactly the
+              create/edit dialog the card measured. */}
+          <CloseSrLabel />
+        </DialogPrimitive.Close>
+      )}
     </DialogPrimitive.Content>
   </DialogPortal>
   );

packages/plugin-form/src/ModalForm.tsx

@@ -167,7 +167,12 @@ export interface ModalFormSchema {
   modalSize?: 'sm' | 'default' | 'lg' | 'xl' | 'full';
 
   /**
-   * Whether to show a close button in the header.
+   * Whether the dialog draws its close (X) button. Only an explicit `false`
+   * hides it (objectui#11061); both dialog arms (the flat form and the
+   * `subforms` master-detail form) pass it to `MobileDialogContent`'s
+   * `showCloseButton`. Hidden, the form is still dismissable: Escape (and a
+   * backdrop click) still reach `onOpenChange(false)`, as does the Cancel
+   * action when it is shown.
    * @default true
    */
   modalCloseButton?: boolean;
@@ -310,6 +315,9 @@ export const ModalForm: React.FC<ModalFormProps> = ({
   const confirmOnDiscard = schema.confirmOnDiscard !== false;
 
   const isOpen = schema.open !== false;
+  // `modalCloseButton` (objectui#11061): only an explicit `false` hides the
+  // dialog's X; unset and `true` keep it.
+  const showCloseButton = schema.modalCloseButton !== false;
 
   // Stable form id for linking the external submit button to the form element
   const formId = useId();
@@ -1007,7 +1015,7 @@ export const ModalForm: React.FC<ModalFormProps> = ({
   if (subforms?.length && schema.mode !== 'view') {
     return (
       <Dialog open={isOpen} onOpenChange={schema.onOpenChange}>
-        <MobileDialogContent className={cn(sizeClass, 'flex flex-col h-[100dvh] sm:h-auto sm:max-h-[90vh] overflow-hidden p-0', className, schema.className)}>
+        <MobileDialogContent showCloseButton={showCloseButton} className={cn(sizeClass, 'flex flex-col h-[100dvh] sm:h-auto sm:max-h-[90vh] overflow-hidden p-0', className, schema.className)}>
           {(schema.title || schema.description) && (
             <DialogHeader className="shrink-0 px-4 pt-4 sm:px-6 sm:pt-6 pb-2 border-b">
               {schema.title && <DialogTitle>{schema.title}</DialogTitle>}
@@ -1059,7 +1067,7 @@ export const ModalForm: React.FC<ModalFormProps> = ({
         attemptClose(false);
       }}
     >
-      <MobileDialogContent className={cn(sizeClass, 'flex flex-col h-[100dvh] sm:h-auto sm:max-h-[90vh] overflow-hidden p-0', className, schema.className)}>
+      <MobileDialogContent showCloseButton={showCloseButton} className={cn(sizeClass, 'flex flex-col h-[100dvh] sm:h-auto sm:max-h-[90vh] overflow-hidden p-0', className, schema.className)}>
         {(schema.title || schema.description) && (
           <DialogHeader className="shrink-0 px-4 pt-4 sm:px-6 sm:pt-6 pb-2 border-b">
             {schema.title && <DialogTitle>{schema.title}</DialogTitle>}

packages/plugin-form/src/__tests__/modalCloseButton-11061.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.
+ */
+
+/**
+ * `object-form`'s `modalCloseButton` takes effect (objectui#11061).
+ *
+ * The key was declared on `ObjectFormSchema` and the `object-form`
+ * registration and forwarded by `ObjectForm`'s modal route, but `ModalForm`
+ * never read it: `modalCloseButton: false` still drew the dialog's X. Now
+ * `ModalForm` passes `modalCloseButton !== false` to `MobileDialogContent`'s
+ * `showCloseButton` on both of its dialog arms, the flat form and the
+ * `subforms` master-detail form.
+ *
+ * Driven through `ObjectForm` (the `object-form` renderer), so the forwarding
+ * hop is inside the pin. The default rows are the control: the same query
+ * finds the button when the key is unset or `true`.
+ */
+
+import React from 'react';
+import { describe, it, expect, vi, afterEach } from 'vitest';
+import { render, screen, fireEvent, waitFor, cleanup } from '@testing-library/react';
+import { registerAllFields } from '@object-ui/fields';
+import type { DataSource } from '@object-ui/types';
+import { ObjectForm } from '../ObjectForm';
+
+registerAllFields();
+afterEach(() => cleanup());
+
+const DEAL_SCHEMA = { name: 'deal', fields: { name: { type: 'text', label: 'Name' } } };
+
+function dataSource() {
+  return {
+    getObjectSchema: vi.fn().mockResolvedValue(DEAL_SCHEMA),
+    find: vi.fn().mockResolvedValue({ data: [] }),
+    findOne: vi.fn(),
+    create: vi.fn(),
+    update: vi.fn(),
+  } as unknown as DataSource;
+}
+
+type ObjectFormNode = React.ComponentProps<typeof ObjectForm>['schema'];
+
+async function renderModal(extra: Record<string, unknown>) {
+  const onOpenChange = vi.fn();
+  const onCancel = vi.fn();
+  render(
+    <ObjectForm
+      schema={{
+        type: 'object-form',
+        objectName: 'deal',
+        mode: 'create',
+        formType: 'modal',
+        title: 'New deal',
+        onOpenChange,
+        onCancel,
+        ...extra,
+      } as unknown as ObjectFormNode}
+      dataSource={dataSource()}
+    />,
+  );
+  await waitFor(() => expect(screen.getByRole('dialog')).toBeTruthy());
+  return { onOpenChange, onCancel };
+}
+
+/** The flat arm has finished loading once its field and footer are drawn. */
+async function flatFormLoaded() {
+  await waitFor(() => expect(screen.getByText('Name')).toBeTruthy());
+  await waitFor(() => expect(screen.getByRole('button', { name: 'Cancel' })).toBeTruthy());
+}
+
+const closeButton = () => screen.queryByRole('button', { name: /close/i });
+
+describe('ModalForm honours modalCloseButton (objectui#11061)', () => {
+  it('keeps the close button by default', async () => {
+    await renderModal({});
+    await flatFormLoaded();
+    expect(closeButton()).not.toBeNull();
+  });
+
+  it('keeps the close button when modalCloseButton is true', async () => {
+    await renderModal({ modalCloseButton: true });
+    await flatFormLoaded();
+    expect(closeButton()).not.toBeNull();
+  });
+
+  it('has no close button when modalCloseButton is false', async () => {
+    await renderModal({ modalCloseButton: false });
+    await flatFormLoaded();
+    expect(closeButton()).toBeNull();
+  });
+
+  it('still closes on Escape with the close button hidden', async () => {
+    const { onOpenChange } = await renderModal({ modalCloseButton: false });
+    await flatFormLoaded();
+    fireEvent.keyDown(screen.getByRole('dialog'), { key: 'Escape' });
+    expect(onOpenChange).toHaveBeenCalledWith(false);
+  });
+
+  it('still closes on Cancel with the close button hidden', async () => {
+    const { onOpenChange, onCancel } = await renderModal({ modalCloseButton: false });
+    await flatFormLoaded();
+    fireEvent.click(screen.getByRole('button', { name: 'Cancel' }));
+    expect(onOpenChange).toHaveBeenCalledWith(false);
+    expect(onCancel).toHaveBeenCalledTimes(1);
+  });
+
+  describe('the subforms (master-detail) arm', () => {
+    const subforms = [{ childObject: 'deal_line', relationshipField: 'deal' }];
+
+    it('keeps the close button by default', async () => {
+      await renderModal({ subforms });
+      expect(closeButton()).not.toBeNull();
+    });
+
+    it('has no close button when modalCloseButton is false, and still closes on Escape', async () => {
+      const { onOpenChange } = await renderModal({ subforms, modalCloseButton: false });
+      expect(closeButton()).toBeNull();
+      fireEvent.keyDown(screen.getByRole('dialog'), { key: 'Escape' });
+      expect(onOpenChange).toHaveBeenCalledWith(false);
+    });
+  });
+});

packages/plugin-form/src/index.tsx

@@ -271,7 +271,7 @@ ComponentRegistry.register('object-form', ObjectFormRenderer, {
     { name: 'drawerWidth', type: 'string' },
     // Modal
     { name: 'modalSize', type: 'enum', enum: ['sm', 'default', 'lg', 'xl', 'full'] },
-    { name: 'modalCloseButton', type: 'boolean', description: 'Meant to show or hide the modal presentation’s close button. `ObjectForm`’s modal route forwards it (`modalCloseButton: schema.modalCloseButton`), but `ModalForm` only declares the key and never reads it: the close button renders either way, so `false` does not hide it.' },
+    { name: 'modalCloseButton', type: 'boolean', description: 'Show the modal presentation’s close (X) button. Default `true`; `false` hides it. Forwarded by `ObjectForm`’s modal route and honoured by `ModalForm` on both of its dialog arms. With the button hidden the modal still closes on Escape, and on the Cancel action when that is shown.' },
     { name: 'contentLayout', type: 'enum', enum: ['simple', 'tabbed'], description: 'How the modal presentation lays out sections. `tabbed` differs from `simple` only when more than one section has a field to show (`ModalForm` tests `schema.contentLayout === "tabbed" && groups.length > 1`).' },
     { name: 'confirmOnDiscard', type: 'boolean', description: 'Ask before discarding unsaved edits when a drawer/modal form is dismissed. Set `false` to close immediately.' },
     // Record binding