diff --git a/.changeset/dialog-outside-click-keeps-form-state.md b/.changeset/dialog-outside-click-keeps-form-state.md new file mode 100644 index 0000000000..4de1322c3b --- /dev/null +++ b/.changeset/dialog-outside-click-keeps-form-state.md @@ -0,0 +1,11 @@ +--- +"@executor-js/react": patch +--- + +**A click outside a dialog or sheet no longer discards the form inside it** + +Radix dismisses an overlay surface on any un-prevented outside interaction, and `DialogContent` and `SheetContent` only prevented that for clicks landing in a portaled combobox or select popup. Every other outside click fell through to dismissal, so a stray click on the page behind a form — after switching windows to copy an ID, for example — closed the surface and destroyed what the user had typed. These surfaces unmount their state on close by design, so nothing was recoverable. + +The default is now the opposite: an outside interaction keeps the surface open. Escape and the close button are unchanged and still close it. `DialogContent` and `SheetContent` take a new `dismissOnOutsideClick` prop for surfaces with nothing to lose — confirmations, pickers, and read-only panels — and the portaled-popup guard still applies there, so choosing a combobox option never dismisses. + +`CommandDialog` sets `dismissOnOutsideClick` on by default, because a command palette holds only a search string and clicking away is the expected way to leave it. diff --git a/apps/cloud/src/routes/app/billing_.plans.tsx b/apps/cloud/src/routes/app/billing_.plans.tsx index 9332325766..76f9944d7d 100644 --- a/apps/cloud/src/routes/app/billing_.plans.tsx +++ b/apps/cloud/src/routes/app/billing_.plans.tsx @@ -401,7 +401,8 @@ function EnterpriseContactDialog() { Contact us - + {/* Static contact details only: clicking away dismisses it. */} + Talk to us about Enterprise @@ -449,7 +450,8 @@ function SlackContactCta() { → - + {/* Static contact details only: clicking away dismisses it. */} + Get in touch on Slack diff --git a/apps/cloud/src/web/components/support-slot.tsx b/apps/cloud/src/web/components/support-slot.tsx index 834fc4b6ed..1d46f5d66b 100644 --- a/apps/cloud/src/web/components/support-slot.tsx +++ b/apps/cloud/src/web/components/support-slot.tsx @@ -44,7 +44,8 @@ export function SupportSlot() { Get support - + {/* Static links only: clicking away dismisses it. */} + Get support diff --git a/packages/react/src/components/add-account-modal.tsx b/packages/react/src/components/add-account-modal.tsx index 43f72ef5e6..45b5a8c561 100644 --- a/packages/react/src/components/add-account-modal.tsx +++ b/packages/react/src/components/add-account-modal.tsx @@ -2357,15 +2357,12 @@ function AddAccountModalView(props: AddAccountModalProps) { // Non-modal for the same reason as the health-check editor sheet: a modal // dialog's react-remove-scroll locks the wheel to the dialog subtree, so // the operation combobox's PORTALED popup (and the modal body while it is - // open) cannot scroll. The overlay still dims and outside-click still - // closes; the portaled-popup guard in DialogContent keeps option clicks - // from dismissing. + // open) cannot scroll. The overlay still dims, so the dialog keeps its + // modal look. This form holds credentials and a half-built auth method, so + // it takes DialogContent's default: an outside click does not dismiss it. event.preventDefault() : undefined - } className={cn( "max-h-[85vh] overflow-x-hidden overflow-y-auto", (addingMethod && createCustomMethod) || oauthRegistering || oauthEditing diff --git a/packages/react/src/components/command.tsx b/packages/react/src/components/command.tsx index c5f1ab047b..cdf3a946ca 100644 --- a/packages/react/src/components/command.tsx +++ b/packages/react/src/components/command.tsx @@ -26,12 +26,17 @@ function CommandDialog({ children, className, showCloseButton = true, + dismissOnOutsideClick = true, ...props }: React.ComponentProps & { title?: string; description?: string; className?: string; showCloseButton?: boolean; + /** A command palette holds nothing but a search string, and clicking away is + * the expected way to leave it, so this stays on — unlike `DialogContent`, + * whose default protects form state. */ + dismissOnOutsideClick?: boolean; }) { return ( @@ -42,6 +47,7 @@ function CommandDialog({ {children} diff --git a/packages/react/src/components/dialog.tsx b/packages/react/src/components/dialog.tsx index 51020ea2d7..91ae54ed51 100644 --- a/packages/react/src/components/dialog.tsx +++ b/packages/react/src/components/dialog.tsx @@ -2,6 +2,7 @@ import * as React from "react"; import { XIcon } from "lucide-react"; import { Dialog as DialogPrimitive } from "radix-ui"; +import { applyOutsideDismissPolicy } from "../lib/outside-dismiss"; import { cn } from "../lib/utils"; import { Button } from "./button"; @@ -37,25 +38,26 @@ function DialogOverlay({ ); } -// base-ui popups (combobox/select) portal their list OUTSIDE the dialog content, -// so clicking an option reads as an interaction outside the dialog and would -// dismiss it before the selection lands. Keep the dialog open for interactions -// that originate inside such a popup. -const PORTALED_POPUP_SELECTOR = "[data-slot='combobox-content'],[data-slot='select-content']"; - function DialogContent({ className, children, showCloseButton = true, + dismissOnOutsideClick = false, onInteractOutside, + onPointerDownOutside, forceOverlay = false, ...props }: React.ComponentProps & { showCloseButton?: boolean; + /** Let a click outside the dialog close it. Off by default: a stray click on + * the page behind a form must not discard what the user typed. Turn it on + * for a dialog with nothing to lose — a confirmation, a picker, a read-only + * panel. Escape and the close button close either way. */ + dismissOnOutsideClick?: boolean; /** Pair with ``: Radix renders no overlay in non-modal * mode, so this renders a plain dim layer instead. It still eats outside - * clicks (which dismiss via onInteractOutside), so the dialog keeps its - * modal look while the wheel stays free for portaled popups. */ + * clicks, so the dialog keeps its modal look while the wheel stays free for + * portaled popups. */ forceOverlay?: boolean; }) { return ( @@ -66,12 +68,13 @@ function DialogContent({ ) : null} { + onPointerDownOutside?.(event); + applyOutsideDismissPolicy(event, dismissOnOutsideClick); + }} onInteractOutside={(event) => { - const target = event.detail.originalEvent.target; - if (target instanceof Element && target.closest(PORTALED_POPUP_SELECTOR)) { - event.preventDefault(); - } onInteractOutside?.(event); + applyOutsideDismissPolicy(event, dismissOnOutsideClick); }} className={cn( "fixed top-[50%] left-[50%] z-50 grid w-full max-w-[calc(100%-2rem)] translate-x-[-50%] translate-y-[-50%] gap-4 rounded-lg border bg-background p-6 shadow-lg duration-200 outline-none data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=closed]:zoom-out-95 data-[state=open]:animate-in data-[state=open]:fade-in-0 data-[state=open]:zoom-in-95 sm:max-w-lg", diff --git a/packages/react/src/components/remove-oauth-app-dialog.tsx b/packages/react/src/components/remove-oauth-app-dialog.tsx index 77bfa0409e..717a555bd7 100644 --- a/packages/react/src/components/remove-oauth-app-dialog.tsx +++ b/packages/react/src/components/remove-oauth-app-dialog.tsx @@ -29,7 +29,8 @@ export function RemoveOAuthAppDialog(props: { const inUse = connections.length > 0; return ( (open ? undefined : props.onClose())}> - + {/* A confirmation with nothing to lose: clicking away cancels it. */} + Remove {String(client.slug)}? diff --git a/packages/react/src/components/sheet.tsx b/packages/react/src/components/sheet.tsx index 7ee17a9cba..2045006659 100644 --- a/packages/react/src/components/sheet.tsx +++ b/packages/react/src/components/sheet.tsx @@ -4,6 +4,7 @@ import * as React from "react"; import { XIcon } from "lucide-react"; import { Dialog as SheetPrimitive } from "radix-ui"; +import { applyOutsideDismissPolicy } from "../lib/outside-dismiss"; import { cn } from "../lib/utils"; function Sheet({ ...props }: React.ComponentProps) { @@ -38,34 +39,36 @@ function SheetOverlay({ ); } -// base-ui popups (combobox/select) portal their list OUTSIDE the dialog content, -// so clicking an option reads as an interaction outside the sheet and would -// dismiss it before the selection lands. Keep the sheet open for interactions -// that originate inside such a popup. -const PORTALED_POPUP_SELECTOR = "[data-slot='combobox-content'],[data-slot='select-content']"; - function SheetContent({ className, children, side = "right", showCloseButton = true, + dismissOnOutsideClick = false, onInteractOutside, + onPointerDownOutside, ...props }: React.ComponentProps & { side?: "top" | "right" | "bottom" | "left"; showCloseButton?: boolean; + /** Let a click outside the sheet close it. Off by default: a stray click on + * the page behind a form must not discard what the user typed. Turn it on + * for a sheet with nothing to lose — navigation, a picker, a read-only + * panel. Escape and the close button close either way. */ + dismissOnOutsideClick?: boolean; }) { return ( { + onPointerDownOutside?.(event); + applyOutsideDismissPolicy(event, dismissOnOutsideClick); + }} onInteractOutside={(event) => { - const target = event.detail.originalEvent.target; - if (target instanceof Element && target.closest(PORTALED_POPUP_SELECTOR)) { - event.preventDefault(); - } onInteractOutside?.(event); + applyOutsideDismissPolicy(event, dismissOnOutsideClick); }} className={cn( "fixed z-50 flex flex-col gap-4 bg-background shadow-lg transition ease-in-out data-[state=closed]:animate-out data-[state=closed]:duration-300 data-[state=open]:animate-in data-[state=open]:duration-500", diff --git a/packages/react/src/components/sidebar.tsx b/packages/react/src/components/sidebar.tsx index 054c1a050a..96f395e12a 100644 --- a/packages/react/src/components/sidebar.tsx +++ b/packages/react/src/components/sidebar.tsx @@ -168,6 +168,9 @@ function Sidebar({ return ( { + it("keeps the surface open by default", () => { + expect( + dismissesOnOutsideInteraction({ + dismissOnOutsideClick: false, + insidePortaledPopup: false, + }), + ).toBe(false); + }); + + it("closes the surface when it opts in", () => { + expect( + dismissesOnOutsideInteraction({ dismissOnOutsideClick: true, insidePortaledPopup: false }), + ).toBe(true); + }); + + it("never closes on a click inside a portaled popup, even when it opts in", () => { + // A combobox or select renders its list outside the surface, so choosing an + // option arrives as an outside interaction. Dismissing there would drop the + // selection before it lands. + expect( + dismissesOnOutsideInteraction({ dismissOnOutsideClick: true, insidePortaledPopup: true }), + ).toBe(false); + }); +}); + +/** A stand-in for the event Radix dispatches, recording whether it was blocked. */ +const outsideEvent = () => { + let prevented = false; + return { + event: { + detail: { originalEvent: { target: { nodeName: "DIV" } } }, + preventDefault: () => { + prevented = true; + }, + }, + prevented: () => prevented, + }; +}; + +describe("applyOutsideDismissPolicy", () => { + it("blocks a plain outside click by default", () => { + const outside = outsideEvent(); + applyOutsideDismissPolicy(outside.event, false); + expect(outside.prevented()).toBe(true); + }); + + it("lets a plain outside click through when the surface opts in", () => { + const outside = outsideEvent(); + applyOutsideDismissPolicy(outside.event, true); + expect(outside.prevented()).toBe(false); + }); +}); + +describe("PORTALED_POPUP_SELECTOR", () => { + it("covers both popup slots that portal out of a surface", () => { + expect(PORTALED_POPUP_SELECTOR).toBe( + "[data-slot='combobox-content'],[data-slot='select-content']", + ); + }); +}); diff --git a/packages/react/src/lib/outside-dismiss.ts b/packages/react/src/lib/outside-dismiss.ts new file mode 100644 index 0000000000..94e36eaa28 --- /dev/null +++ b/packages/react/src/lib/outside-dismiss.ts @@ -0,0 +1,53 @@ +/** + * Outside-interaction policy shared by `DialogContent` and `SheetContent`. + * + * Radix dismisses an overlay surface when an outside interaction is not + * default-prevented. That loses whatever the user typed, and a stray click on + * the page behind a form is easy to make. So the default here is the opposite + * of Radix's: an outside interaction keeps the surface open. Escape and the + * close button are unaffected — they still close. + * + * A surface with nothing to lose (a confirmation, a picker, a read-only panel) + * opts back in with `dismissOnOutsideClick`. + */ + +/** base-ui popups (combobox/select) portal their list OUTSIDE the surface, so a + * click on an option reads as an interaction outside it. Such a click must + * never dismiss, even when the surface opts in. */ +export const PORTALED_POPUP_SELECTOR = + "[data-slot='combobox-content'],[data-slot='select-content']"; + +/** True when the interaction started inside a popup this surface portals out. */ +export const isInsidePortaledPopup = (target: unknown): boolean => + typeof Element !== "undefined" && + target instanceof Element && + target.closest(PORTALED_POPUP_SELECTOR) !== null; + +/** + * Whether an outside interaction should close the surface. + * + * Pure so the decision is testable without a DOM: the caller does the element + * lookup and passes the answer in. + */ +export const dismissesOnOutsideInteraction = (input: { + readonly dismissOnOutsideClick: boolean; + readonly insidePortaledPopup: boolean; +}): boolean => input.dismissOnOutsideClick && !input.insidePortaledPopup; + +/** The shape Radix hands to `onInteractOutside` and `onPointerDownOutside`. */ +type OutsideInteractionEvent = { + readonly detail: { readonly originalEvent: { readonly target: unknown } }; + readonly preventDefault: () => void; +}; + +/** Apply the policy to a Radix outside-interaction event. */ +export const applyOutsideDismissPolicy = ( + event: OutsideInteractionEvent, + dismissOnOutsideClick: boolean, +): void => { + const dismisses = dismissesOnOutsideInteraction({ + dismissOnOutsideClick, + insidePortaledPopup: isInsidePortaledPopup(event.detail.originalEvent.target), + }); + if (!dismisses) event.preventDefault(); +}; diff --git a/packages/react/src/pages/admin-users.tsx b/packages/react/src/pages/admin-users.tsx index 1fb2fdc40b..49fae31d68 100644 --- a/packages/react/src/pages/admin-users.tsx +++ b/packages/react/src/pages/admin-users.tsx @@ -670,7 +670,8 @@ export function AdminUsersPage() { })} !open && setSelected(null)}> - + {/* A read-only detail panel: clicking away closes it. */} + {selected && ( <> diff --git a/packages/react/src/pages/api-keys.tsx b/packages/react/src/pages/api-keys.tsx index eb15084cb7..7d8c7eff84 100644 --- a/packages/react/src/pages/api-keys.tsx +++ b/packages/react/src/pages/api-keys.tsx @@ -500,7 +500,8 @@ function OrgApiKeysSectionBody() { if (!open) setConfirmRevoke(null); }} > - + {/* A confirmation with nothing to lose: clicking away cancels it. */} + Revoke organization key diff --git a/packages/react/src/pages/org.tsx b/packages/react/src/pages/org.tsx index 8797533af7..a762e2b1af 100644 --- a/packages/react/src/pages/org.tsx +++ b/packages/react/src/pages/org.tsx @@ -447,7 +447,8 @@ function UpgradeDialog(props: { }) { return ( - + {/* Informational only: nothing to lose, so clicking away dismisses it. */} + You are at your member limit