History

Keep dialogs and sheets open on an outside click (#1814)

Rhys Sullivan committed 2 months ago

Commit 66fb1a41 parent 1cd81d614 files · +195 −35Browse files

Unified · Split

14 files
  1. .changeset/dialog-outside-click-keeps-form-state.md +11 −0
  2. apps/cloud/src/routes/app/billing_.plans.tsx +4 −2
  3. apps/cloud/src/web/components/support-slot.tsx +2 −1
  4. packages/react/src/components/add-account-modal.tsx +3 −6
  5. packages/react/src/components/command.tsx +6 −0
  6. packages/react/src/components/dialog.tsx +15 −12
  7. packages/react/src/components/remove-oauth-app-dialog.tsx +2 −1
  8. packages/react/src/components/sheet.tsx +13 −10
  9. packages/react/src/components/sidebar.tsx +3 −0
  10. packages/react/src/lib/outside-dismiss.test.ts +77 −0
  11. packages/react/src/lib/outside-dismiss.ts +53 −0
  12. packages/react/src/pages/admin-users.tsx +2 −1
  13. packages/react/src/pages/api-keys.tsx +2 −1
  14. packages/react/src/pages/org.tsx +2 −1
.changeset/dialog-outside-click-keeps-form-state.md+11−0
@@ -0,0 +1,11 @@
1+---
2+"@executor-js/react": patch
3+---
4+
5+**A click outside a dialog or sheet no longer discards the form inside it**
6+
7+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.
8+
9+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.
10+
11+`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.
apps/cloud/src/routes/app/billing_.plans.tsx+4−2
@@ -401,7 +401,8 @@ function EnterpriseContactDialog() { · 400 unmodified lines
401401 Contact us
402402 </Button>
403403 </DialogTrigger>
404− <DialogContent>
404+ {/* Static contact details only: clicking away dismisses it. */}
405+ <DialogContent dismissOnOutsideClick>
405406 <DialogHeader>
406407 <DialogTitle>Talk to us about Enterprise</DialogTitle>
407408 <DialogDescription>
@@ -449,7 +450,8 @@ function SlackContactCta() { · 41 unmodified lines
449450 <span aria-hidden>→</span>
450451 </Button>
451452 </DialogTrigger>
452− <DialogContent>
453+ {/* Static contact details only: clicking away dismisses it. */}
454+ <DialogContent dismissOnOutsideClick>
453455 <DialogHeader>
454456 <DialogTitle>Get in touch on Slack</DialogTitle>
455457 <DialogDescription>
apps/cloud/src/web/components/support-slot.tsx+2−1
@@ -44,7 +44,8 @@ export function SupportSlot() { · 43 unmodified lines
4444 <HelpIcon className="size-3.5 text-muted-foreground" />
4545 Get support
4646 </Button>
47− <DialogContent className="sm:max-w-[440px]">
47+ {/* Static links only: clicking away dismisses it. */}
48+ <DialogContent dismissOnOutsideClick className="sm:max-w-[440px]">
4849 <DialogHeader>
4950 <DialogTitle className="font-display text-xl">Get support</DialogTitle>
5051 <DialogDescription className="text-sm leading-relaxed">
packages/react/src/components/add-account-modal.tsx+3−6
@@ -2357,15 +2357,12 @@ function AddAccountModalView(props: AddAccountModalProps) { · 2356 unmodified lines
23572357 // Non-modal for the same reason as the health-check editor sheet: a modal
23582358 // dialog's react-remove-scroll locks the wheel to the dialog subtree, so
23592359 // the operation combobox's PORTALED popup (and the modal body while it is
2360− // open) cannot scroll. The overlay still dims and outside-click still
2361− // closes; the portaled-popup guard in DialogContent keeps option clicks
2362− // from dismissing.
2360+ // open) cannot scroll. The overlay still dims, so the dialog keeps its
2361+ // modal look. This form holds credentials and a half-built auth method, so
2362+ // it takes DialogContent's default: an outside click does not dismiss it.
23632363 <Dialog open={open} onOpenChange={onOpenChange} modal={false}>
23642364 <DialogContent
23652365 forceOverlay
2366− onInteractOutside={
2367− oauthClientHandoff?.action === "reconnect" ? (event) => event.preventDefault() : undefined
2368− }
23692366 className={cn(
23702367 "max-h-[85vh] overflow-x-hidden overflow-y-auto",
23712368 (addingMethod && createCustomMethod) || oauthRegistering || oauthEditing
packages/react/src/components/command.tsx+6−0
@@ -26,12 +26,17 @@ function CommandDialog({ · 25 unmodified lines
2626 children,
2727 className,
2828 showCloseButton = true,
29+ dismissOnOutsideClick = true,
2930 ...props
3031 }: React.ComponentProps<typeof Dialog> & {
3132 title?: string;
3233 description?: string;
3334 className?: string;
3435 showCloseButton?: boolean;
36+ /** A command palette holds nothing but a search string, and clicking away is
37+ * the expected way to leave it, so this stays on — unlike `DialogContent`,
38+ * whose default protects form state. */
39+ dismissOnOutsideClick?: boolean;
3540 }) {
3641 return (
3742 <Dialog {...props}>
@@ -42,6 +47,7 @@ function CommandDialog({ · 4 unmodified lines
4247 <DialogContent
4348 className={cn("overflow-hidden p-0", className)}
4449 showCloseButton={showCloseButton}
50+ dismissOnOutsideClick={dismissOnOutsideClick}
4551 >
4652 <Command className="**:data-[slot=command-input-wrapper]:h-12 [&_[cmdk-group-heading]]:px-2 [&_[cmdk-group-heading]]:font-medium [&_[cmdk-group-heading]]:text-muted-foreground [&_[cmdk-group]]:px-2 [&_[cmdk-group]:not([hidden])_~[cmdk-group]]:pt-0 [&_[cmdk-input-wrapper]_svg]:h-5 [&_[cmdk-input-wrapper]_svg]:w-5 [&_[cmdk-input]]:h-12 [&_[cmdk-item]]:px-2 [&_[cmdk-item]]:py-3 [&_[cmdk-item]_svg]:h-5 [&_[cmdk-item]_svg]:w-5">
4753 {children}
packages/react/src/components/dialog.tsx+15−12
@@ -2,6 +2,7 @@ import * as React from "react"; · 1 unmodified line
22 import { XIcon } from "lucide-react";
33 import { Dialog as DialogPrimitive } from "radix-ui";
44
5+import { applyOutsideDismissPolicy } from "../lib/outside-dismiss";
56 import { cn } from "../lib/utils";
67 import { Button } from "./button";
78
@@ -37,25 +38,26 @@ function DialogOverlay({ · 29 unmodified lines
3738 );
3839 }
3940
40−// base-ui popups (combobox/select) portal their list OUTSIDE the dialog content,
41−// so clicking an option reads as an interaction outside the dialog and would
42−// dismiss it before the selection lands. Keep the dialog open for interactions
43−// that originate inside such a popup.
44−const PORTALED_POPUP_SELECTOR = "[data-slot='combobox-content'],[data-slot='select-content']";
45−
4641 function DialogContent({
4742 className,
4843 children,
4944 showCloseButton = true,
45+ dismissOnOutsideClick = false,
5046 onInteractOutside,
47+ onPointerDownOutside,
5148 forceOverlay = false,
5249 ...props
5350 }: React.ComponentProps<typeof DialogPrimitive.Content> & {
5451 showCloseButton?: boolean;
52+ /** Let a click outside the dialog close it. Off by default: a stray click on
53+ * the page behind a form must not discard what the user typed. Turn it on
54+ * for a dialog with nothing to lose — a confirmation, a picker, a read-only
55+ * panel. Escape and the close button close either way. */
56+ dismissOnOutsideClick?: boolean;
5557 /** Pair with `<Dialog modal={false}>`: Radix renders no overlay in non-modal
5658 * mode, so this renders a plain dim layer instead. It still eats outside
57− * clicks (which dismiss via onInteractOutside), so the dialog keeps its
58− * modal look while the wheel stays free for portaled popups. */
59+ * clicks, so the dialog keeps its modal look while the wheel stays free for
60+ * portaled popups. */
5961 forceOverlay?: boolean;
6062 }) {
6163 return (
@@ -66,12 +68,13 @@ function DialogContent({ · 4 unmodified lines
6668 ) : null}
6769 <DialogPrimitive.Content
6870 data-slot="dialog-content"
71+ onPointerDownOutside={(event) => {
72+ onPointerDownOutside?.(event);
73+ applyOutsideDismissPolicy(event, dismissOnOutsideClick);
74+ }}
6975 onInteractOutside={(event) => {
70− const target = event.detail.originalEvent.target;
71− if (target instanceof Element && target.closest(PORTALED_POPUP_SELECTOR)) {
72− event.preventDefault();
73− }
7476 onInteractOutside?.(event);
77+ applyOutsideDismissPolicy(event, dismissOnOutsideClick);
7578 }}
7679 className={cn(
7780 "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",
packages/react/src/components/remove-oauth-app-dialog.tsx+2−1
@@ -29,7 +29,8 @@ export function RemoveOAuthAppDialog(props: { · 28 unmodified lines
2929 const inUse = connections.length > 0;
3030 return (
3131 <Dialog open onOpenChange={(open: boolean) => (open ? undefined : props.onClose())}>
32− <DialogContent>
32+ {/* A confirmation with nothing to lose: clicking away cancels it. */}
33+ <DialogContent dismissOnOutsideClick>
3334 <DialogHeader>
3435 <DialogTitle>Remove {String(client.slug)}?</DialogTitle>
3536 <DialogDescription>
packages/react/src/components/sheet.tsx+13−10
@@ -4,6 +4,7 @@ import * as React from "react"; · 3 unmodified lines
44 import { XIcon } from "lucide-react";
55 import { Dialog as SheetPrimitive } from "radix-ui";
66
7+import { applyOutsideDismissPolicy } from "../lib/outside-dismiss";
78 import { cn } from "../lib/utils";
89
910 function Sheet({ ...props }: React.ComponentProps<typeof SheetPrimitive.Root>) {
@@ -38,34 +39,36 @@ function SheetOverlay({ · 28 unmodified lines
3839 );
3940 }
4041
41−// base-ui popups (combobox/select) portal their list OUTSIDE the dialog content,
42−// so clicking an option reads as an interaction outside the sheet and would
43−// dismiss it before the selection lands. Keep the sheet open for interactions
44−// that originate inside such a popup.
45−const PORTALED_POPUP_SELECTOR = "[data-slot='combobox-content'],[data-slot='select-content']";
46−
4742 function SheetContent({
4843 className,
4944 children,
5045 side = "right",
5146 showCloseButton = true,
47+ dismissOnOutsideClick = false,
5248 onInteractOutside,
49+ onPointerDownOutside,
5350 ...props
5451 }: React.ComponentProps<typeof SheetPrimitive.Content> & {
5552 side?: "top" | "right" | "bottom" | "left";
5653 showCloseButton?: boolean;
54+ /** Let a click outside the sheet close it. Off by default: a stray click on
55+ * the page behind a form must not discard what the user typed. Turn it on
56+ * for a sheet with nothing to lose — navigation, a picker, a read-only
57+ * panel. Escape and the close button close either way. */
58+ dismissOnOutsideClick?: boolean;
5759 }) {
5860 return (
5961 <SheetPortal>
6062 <SheetOverlay />
6163 <SheetPrimitive.Content
6264 data-slot="sheet-content"
65+ onPointerDownOutside={(event) => {
66+ onPointerDownOutside?.(event);
67+ applyOutsideDismissPolicy(event, dismissOnOutsideClick);
68+ }}
6369 onInteractOutside={(event) => {
64− const target = event.detail.originalEvent.target;
65− if (target instanceof Element && target.closest(PORTALED_POPUP_SELECTOR)) {
66− event.preventDefault();
67− }
6870 onInteractOutside?.(event);
71+ applyOutsideDismissPolicy(event, dismissOnOutsideClick);
6972 }}
7073 className={cn(
7174 "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",
packages/react/src/components/sidebar.tsx+3−0
@@ -168,6 +168,9 @@ function Sidebar({ · 167 unmodified lines
168168 return (
169169 <Sheet open={openMobile} onOpenChange={setOpenMobile} {...props}>
170170 <SheetContent
171+ // Navigation only. Tapping the page behind it is the normal way to
172+ // put the mobile nav away, and nothing is lost.
173+ dismissOnOutsideClick
171174 data-sidebar="sidebar"
172175 data-slot="sidebar"
173176 data-mobile="true"
packages/react/src/lib/outside-dismiss.test.ts+77−0
@@ -0,0 +1,77 @@
1+import { describe, expect, it } from "@effect/vitest";
2+
3+import {
4+ applyOutsideDismissPolicy,
5+ dismissesOnOutsideInteraction,
6+ PORTALED_POPUP_SELECTOR,
7+} from "./outside-dismiss";
8+
9+/**
10+ * The property under test: a dialog or sheet keeps what the user typed.
11+ *
12+ * Radix closes an overlay surface whenever an outside interaction reaches it
13+ * un-prevented, so "does not dismiss" is the thing that has to be asserted, not
14+ * assumed. Escape is Radix's own `onEscapeKeyDown` path and never passes
15+ * through here, which is why no case below can close a surface with a key.
16+ */
17+describe("dismissesOnOutsideInteraction", () => {
18+ it("keeps the surface open by default", () => {
19+ expect(
20+ dismissesOnOutsideInteraction({
21+ dismissOnOutsideClick: false,
22+ insidePortaledPopup: false,
23+ }),
24+ ).toBe(false);
25+ });
26+
27+ it("closes the surface when it opts in", () => {
28+ expect(
29+ dismissesOnOutsideInteraction({ dismissOnOutsideClick: true, insidePortaledPopup: false }),
30+ ).toBe(true);
31+ });
32+
33+ it("never closes on a click inside a portaled popup, even when it opts in", () => {
34+ // A combobox or select renders its list outside the surface, so choosing an
35+ // option arrives as an outside interaction. Dismissing there would drop the
36+ // selection before it lands.
37+ expect(
38+ dismissesOnOutsideInteraction({ dismissOnOutsideClick: true, insidePortaledPopup: true }),
39+ ).toBe(false);
40+ });
41+});
42+
43+/** A stand-in for the event Radix dispatches, recording whether it was blocked. */
44+const outsideEvent = () => {
45+ let prevented = false;
46+ return {
47+ event: {
48+ detail: { originalEvent: { target: { nodeName: "DIV" } } },
49+ preventDefault: () => {
50+ prevented = true;
51+ },
52+ },
53+ prevented: () => prevented,
54+ };
55+};
56+
57+describe("applyOutsideDismissPolicy", () => {
58+ it("blocks a plain outside click by default", () => {
59+ const outside = outsideEvent();
60+ applyOutsideDismissPolicy(outside.event, false);
61+ expect(outside.prevented()).toBe(true);
62+ });
63+
64+ it("lets a plain outside click through when the surface opts in", () => {
65+ const outside = outsideEvent();
66+ applyOutsideDismissPolicy(outside.event, true);
67+ expect(outside.prevented()).toBe(false);
68+ });
69+});
70+
71+describe("PORTALED_POPUP_SELECTOR", () => {
72+ it("covers both popup slots that portal out of a surface", () => {
73+ expect(PORTALED_POPUP_SELECTOR).toBe(
74+ "[data-slot='combobox-content'],[data-slot='select-content']",
75+ );
76+ });
77+});
packages/react/src/lib/outside-dismiss.ts+53−0
@@ -0,0 +1,53 @@
1+/**
2+ * Outside-interaction policy shared by `DialogContent` and `SheetContent`.
3+ *
4+ * Radix dismisses an overlay surface when an outside interaction is not
5+ * default-prevented. That loses whatever the user typed, and a stray click on
6+ * the page behind a form is easy to make. So the default here is the opposite
7+ * of Radix's: an outside interaction keeps the surface open. Escape and the
8+ * close button are unaffected — they still close.
9+ *
10+ * A surface with nothing to lose (a confirmation, a picker, a read-only panel)
11+ * opts back in with `dismissOnOutsideClick`.
12+ */
13+
14+/** base-ui popups (combobox/select) portal their list OUTSIDE the surface, so a
15+ * click on an option reads as an interaction outside it. Such a click must
16+ * never dismiss, even when the surface opts in. */
17+export const PORTALED_POPUP_SELECTOR =
18+ "[data-slot='combobox-content'],[data-slot='select-content']";
19+
20+/** True when the interaction started inside a popup this surface portals out. */
21+export const isInsidePortaledPopup = (target: unknown): boolean =>
22+ typeof Element !== "undefined" &&
23+ target instanceof Element &&
24+ target.closest(PORTALED_POPUP_SELECTOR) !== null;
25+
26+/**
27+ * Whether an outside interaction should close the surface.
28+ *
29+ * Pure so the decision is testable without a DOM: the caller does the element
30+ * lookup and passes the answer in.
31+ */
32+export const dismissesOnOutsideInteraction = (input: {
33+ readonly dismissOnOutsideClick: boolean;
34+ readonly insidePortaledPopup: boolean;
35+}): boolean => input.dismissOnOutsideClick && !input.insidePortaledPopup;
36+
37+/** The shape Radix hands to `onInteractOutside` and `onPointerDownOutside`. */
38+type OutsideInteractionEvent = {
39+ readonly detail: { readonly originalEvent: { readonly target: unknown } };
40+ readonly preventDefault: () => void;
41+};
42+
43+/** Apply the policy to a Radix outside-interaction event. */
44+export const applyOutsideDismissPolicy = (
45+ event: OutsideInteractionEvent,
46+ dismissOnOutsideClick: boolean,
47+): void => {
48+ const dismisses = dismissesOnOutsideInteraction({
49+ dismissOnOutsideClick,
50+ insidePortaledPopup: isInsidePortaledPopup(event.detail.originalEvent.target),
51+ });
52+ if (!dismisses) event.preventDefault();
53+};
packages/react/src/pages/admin-users.tsx+2−1
@@ -670,7 +670,8 @@ export function AdminUsersPage() { · 669 unmodified lines
670670 })}
671671
672672 <Sheet open={selected !== null} onOpenChange={(open) => !open && setSelected(null)}>
673− <SheetContent className="w-full gap-0 p-0 sm:max-w-xl">
673+ {/* A read-only detail panel: clicking away closes it. */}
674+ <SheetContent dismissOnOutsideClick className="w-full gap-0 p-0 sm:max-w-xl">
674675 {selected && (
675676 <>
676677 <SheetHeader className="border-b border-border">
packages/react/src/pages/api-keys.tsx+2−1
@@ -540,7 +540,8 @@ function OrgApiKeysSectionBody() { · 539 unmodified lines
540540 if (!open) setConfirmRevoke(null);
541541 }}
542542 >
543− <DialogContent className="sm:max-w-[480px]">
543+ {/* A confirmation with nothing to lose: clicking away cancels it. */}
544+ <DialogContent dismissOnOutsideClick className="sm:max-w-[480px]">
544545 <DialogHeader>
545546 <DialogTitle className="font-display text-xl">Revoke organization key</DialogTitle>
546547 <DialogDescription className="text-sm leading-relaxed">
packages/react/src/pages/org.tsx+2−1
@@ -496,7 +496,8 @@ function UpgradeDialog(props: { · 495 unmodified lines
496496 }) {
497497 return (
498498 <Dialog open={props.open} onOpenChange={props.onOpenChange}>
499− <DialogContent className="sm:max-w-[400px]">
499+ {/* Informational only: nothing to lose, so clicking away dismisses it. */}
500+ <DialogContent dismissOnOutsideClick className="sm:max-w-[400px]">
500501 <DialogHeader>
501502 <DialogTitle className="font-display text-xl">You are at your member limit</DialogTitle>
502503 <DialogDescription className="text-sm leading-relaxed">