Skip to content

Commit b7237bb

Browse files
authored
fix(components): don't close dialog when interacting with a portalled dropdown (#2072)
Radix Select / Popover / DropdownMenu render their flyout into a portal at document.body, outside the dialog DOM. Clicking an empty part of an open dropdown in a create/edit form registered as an 'interact outside' and closed the whole dialog. MobileDialogContent now guards onInteractOutside: when the interaction's real target is inside a Radix popper layer it is ignored (the popper dismisses itself); a genuine backdrop click still closes the dialog. Logic is extracted as isInsidePopperLayer with unit coverage. Closes #2071
1 parent 6046927 commit b7237bb

3 files changed

Lines changed: 105 additions & 2 deletions

File tree

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
'@object-ui/components': patch
3+
---
4+
5+
fix(components): keep MobileDialogContent open when interacting with a portalled dropdown
6+
7+
Radix Select / Popover / DropdownMenu render their flyout into a portal at
8+
`document.body`, outside the dialog's DOM. Clicking an empty part of an open
9+
dropdown registered as an "interact outside" and closed the entire dialog
10+
(create/edit forms). `MobileDialogContent` now guards `onInteractOutside`:
11+
interactions whose real target is inside a Radix popper layer are ignored
12+
(the popper dismisses itself), while a genuine backdrop click still closes the
13+
dialog as before.
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
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+
* MobileDialogContent guards against a portalled dropdown closing the whole
9+
* dialog. Radix Select / Popover render their flyout into a portal at
10+
* `document.body` (outside the dialog DOM), so clicking empty space inside an
11+
* open dropdown reads as an "interact outside" and used to dismiss the dialog.
12+
* `isInsidePopperLayer` is the predicate that suppresses that case while
13+
* leaving a genuine backdrop click (which still closes the dialog) untouched.
14+
*/
15+
16+
import { describe, it, expect, afterEach } from 'vitest';
17+
import { isInsidePopperLayer } from '../custom/mobile-dialog-content';
18+
19+
function mount(html: string): HTMLElement {
20+
const host = document.createElement('div');
21+
host.innerHTML = html;
22+
document.body.appendChild(host);
23+
return host;
24+
}
25+
26+
afterEach(() => {
27+
document.body.innerHTML = '';
28+
});
29+
30+
describe('isInsidePopperLayer', () => {
31+
it("treats a click inside a Radix popper wrapper as the dialog's own dropdown", () => {
32+
const host = mount(
33+
'<div data-radix-popper-content-wrapper><div role="listbox"><div id="opt">option</div></div></div>',
34+
);
35+
expect(isInsidePopperLayer(host.querySelector('#opt'))).toBe(true);
36+
});
37+
38+
it('matches the select content / viewport variants too', () => {
39+
const host = mount(
40+
'<div data-radix-select-content><div data-radix-select-viewport><span id="item">x</span></div></div>',
41+
);
42+
expect(isInsidePopperLayer(host.querySelector('#item'))).toBe(true);
43+
expect(isInsidePopperLayer(host.querySelector('[data-radix-select-content]'))).toBe(true);
44+
});
45+
46+
it('does NOT match a genuine backdrop / outside-the-dropdown target', () => {
47+
const host = mount('<div id="overlay" data-radix-dialog-overlay></div>');
48+
expect(isInsidePopperLayer(host.querySelector('#overlay'))).toBe(false);
49+
});
50+
51+
it('is null/undefined safe', () => {
52+
expect(isInsidePopperLayer(null)).toBe(false);
53+
expect(isInsidePopperLayer(undefined)).toBe(false);
54+
});
55+
});

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

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,48 @@ import { X } from 'lucide-react';
2323
import { cn } from '../lib/utils';
2424
import { DialogOverlay, DialogPortal } from '../ui/dialog';
2525

26+
/**
27+
* Radix Select / Popover / DropdownMenu render their flyout into a portal at
28+
* `document.body` — physically OUTSIDE this DialogContent's DOM. So clicking an
29+
* empty part of an open dropdown reads as an "interact outside" and would close
30+
* the whole dialog. Suppress that: if the interaction's real target sits inside
31+
* a Radix popper layer, keep the dialog open (the popper closes itself). A real
32+
* backdrop click (target = overlay) is untouched and still closes the dialog.
33+
*/
34+
const POPPER_LAYER_SELECTOR =
35+
'[data-radix-popper-content-wrapper],[data-radix-select-content],[data-radix-select-viewport]';
36+
37+
/**
38+
* True when `target` sits inside a Radix popper flyout (Select / Popover /
39+
* DropdownMenu). Such elements are portalled to `document.body`, so an
40+
* "interact outside" the dialog whose target is one of them is really an
41+
* interaction with the dialog's own dropdown — it must not close the dialog.
42+
*/
43+
export function isInsidePopperLayer(target: Element | null | undefined): boolean {
44+
return !!target?.closest?.(POPPER_LAYER_SELECTOR);
45+
}
46+
2647
export const MobileDialogContent = React.forwardRef<
2748
React.ElementRef<typeof DialogPrimitive.Content>,
2849
React.ComponentPropsWithoutRef<typeof DialogPrimitive.Content>
29-
>(({ className, children, ...props }, ref) => (
50+
>(({ className, children, onInteractOutside, ...props }, ref) => {
51+
const handleInteractOutside = React.useCallback(
52+
(event: Parameters<NonNullable<typeof onInteractOutside>>[0]) => {
53+
const target = (event.detail?.originalEvent?.target ?? null) as Element | null;
54+
if (isInsidePopperLayer(target)) {
55+
event.preventDefault();
56+
return;
57+
}
58+
onInteractOutside?.(event);
59+
},
60+
[onInteractOutside],
61+
);
62+
return (
3063
<DialogPortal>
3164
<DialogOverlay />
3265
<DialogPrimitive.Content
3366
ref={ref}
67+
onInteractOutside={handleInteractOutside}
3468
className={cn(
3569
// Mobile-first: full-screen
3670
'fixed inset-0 z-50 w-full bg-background p-4 shadow-lg duration-200',
@@ -63,5 +97,6 @@ export const MobileDialogContent = React.forwardRef<
6397
</DialogPrimitive.Close>
6498
</DialogPrimitive.Content>
6599
</DialogPortal>
66-
));
100+
);
101+
});
67102
MobileDialogContent.displayName = 'MobileDialogContent';

0 commit comments

Comments
 (0)