Skip to content

Commit 674550b

Browse files
committed
fix: combobox works inside the health-check editor sheet
The operation/identity pickers are base-ui comboboxes whose popup portals to document.body, which collides with the Radix sheet the editor opens in: - The sheet runs modal, so react-dismissable-layer sets the body's pointer-events to none and react-remove-scroll locks the wheel to the sheet subtree. The portaled popup, living outside that subtree, could neither be clicked nor scrolled. Fix: pointer-events:auto on the popup, and open the editor sheet non-modal. - Clicking a portaled option is a pointer-down outside the sheet and dismissed it before the selection landed. Fix: the sheet's onInteractOutside ignores interactions originating in a combobox/select popup. Covered by e2e: opening the editor over a large spec, the operation popup scrolls and an option is clickable without the sheet closing.
1 parent 69d2ed8 commit 674550b

4 files changed

Lines changed: 166 additions & 2 deletions

File tree

e2e/scenarios/health-checks-ui.test.ts

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -641,3 +641,144 @@ scenario(
641641
}),
642642
),
643643
);
644+
645+
// ===========================================================================
646+
// 6. Edit sheet, click interaction: the combobox popup is portaled outside the
647+
// Radix sheet, so clicking an option must not be treated as an outside
648+
// interaction that dismisses the sheet (keyboard scenarios above miss this).
649+
// ===========================================================================
650+
651+
scenario(
652+
"Health checks (UI) · clicking a combobox option in the sheet selects it without closing the sheet",
653+
{},
654+
Effect.scoped(
655+
Effect.gen(function* () {
656+
const target = yield* Target;
657+
const browser = yield* Browser;
658+
const { client: makeClient } = yield* Api;
659+
const identity = yield* target.newIdentity();
660+
const client = yield* makeClient(api, identity);
661+
const slug = newSlug("hc-ui-click");
662+
663+
yield* Effect.ensuring(
664+
Effect.gen(function* () {
665+
yield* registerIdentityIntegration(client, slug, "https://identity.example.com");
666+
const operation = yield* getMeOperation(client, slug);
667+
668+
yield* browser.session(identity, async ({ page, step }) => {
669+
const sheet = page.getByRole("dialog");
670+
const operationInput = page.locator("#health-check-operation");
671+
const identityInput = page.locator("#health-check-identity");
672+
673+
await step("Open the health-check editor", async () => {
674+
await page.goto(`/integrations/${slug}`, { waitUntil: "networkidle" });
675+
await page.getByRole("heading", { level: 3, name: "Health check" }).waitFor();
676+
await page.getByRole("button", { name: "Set up" }).click();
677+
await operationInput.waitFor();
678+
});
679+
680+
await step(
681+
"Click the operation option: it selects and the sheet stays open",
682+
async () => {
683+
await operationInput.click();
684+
await page.getByRole("option").filter({ hasText: "getMe" }).first().click();
685+
// Clicking the portaled popup option must NOT dismiss the sheet.
686+
await sheet.waitFor();
687+
expect(await operationInput.inputValue()).toContain("getMe");
688+
},
689+
);
690+
691+
await step("Click the identity option by mouse, then save", async () => {
692+
await identityInput.click();
693+
await page.getByRole("option").filter({ hasText: "email" }).first().click();
694+
await sheet.waitFor();
695+
expect(await identityInput.inputValue()).toBe("email");
696+
await page.getByRole("button", { name: "Save", exact: true }).click();
697+
await operationInput.waitFor({ state: "hidden" });
698+
});
699+
});
700+
701+
// The mouse-driven selections persisted: only possible if the option
702+
// clicks selected without dismissing the sheet first.
703+
const stored = yield* client.integrations.healthCheckGet({ params: { slug } });
704+
expect(stored).toEqual({ operation, identityField: "email" });
705+
}),
706+
client.openapi.removeSpec({ params: { slug } }).pipe(Effect.ignore),
707+
);
708+
}),
709+
),
710+
);
711+
712+
// ===========================================================================
713+
// 7. Edit sheet, scroll: a modal dialog locks body scroll, which freezes the
714+
// portaled combobox list. The editor sheet is non-modal so the list scrolls.
715+
// ===========================================================================
716+
717+
scenario(
718+
"Health checks (UI) · the combobox list scrolls inside the edit sheet",
719+
{},
720+
Effect.scoped(
721+
Effect.gen(function* () {
722+
const target = yield* Target;
723+
const browser = yield* Browser;
724+
const { client: makeClient } = yield* Api;
725+
const identity = yield* target.newIdentity();
726+
const client = yield* makeClient(api, identity);
727+
const slug = newSlug("hc-ui-scroll");
728+
729+
yield* Effect.ensuring(
730+
Effect.gen(function* () {
731+
// Many operations so the popup list overflows and must scroll.
732+
yield* client.openapi.addSpec({
733+
payload: {
734+
spec: { kind: "blob", value: largeSpec("https://big.example.com") },
735+
slug,
736+
baseUrl: "https://big.example.com",
737+
authenticationTemplate: [
738+
{
739+
slug: "apiKey",
740+
type: "apiKey",
741+
headers: { authorization: ["Bearer ", { type: "variable", name: "token" }] },
742+
},
743+
],
744+
},
745+
});
746+
747+
yield* browser.session(identity, async ({ page, step }) => {
748+
const operationInput = page.locator("#health-check-operation");
749+
const list = page.locator("[data-slot='combobox-list']").first();
750+
751+
await step("Open the operation combobox in the sheet", async () => {
752+
await page.goto(`/integrations/${slug}`, { waitUntil: "networkidle" });
753+
await page.getByRole("heading", { level: 3, name: "Health check" }).waitFor();
754+
await page.getByRole("button", { name: "Set up" }).click();
755+
await operationInput.waitFor();
756+
await operationInput.click();
757+
await list.waitFor();
758+
});
759+
760+
await step("Wheel-scroll the list: it actually moves (not scroll-locked)", async () => {
761+
const before = await list.evaluate((el) => el.scrollTop);
762+
await list.hover();
763+
await page.mouse.wheel(0, 600);
764+
// Poll: the wheel scroll must move the list (blocked → stays 0).
765+
await page.waitForFunction(
766+
(start) => {
767+
const el = document.querySelector("[data-slot='combobox-list']");
768+
return el != null && el.scrollTop > start + 20;
769+
},
770+
before,
771+
{ timeout: 5000 },
772+
);
773+
const after = await list.evaluate((el) => el.scrollTop);
774+
expect(after, "the list scrolled past its starting offset").toBeGreaterThan(before);
775+
// The sheet stayed open through the scroll interaction.
776+
await page.getByRole("dialog").waitFor();
777+
});
778+
});
779+
}),
780+
client.openapi.removeSpec({ params: { slug } }).pipe(Effect.ignore),
781+
);
782+
}),
783+
),
784+
);

packages/react/src/components/combobox.tsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,11 @@ function ComboboxContent({
103103
align={align}
104104
alignOffset={alignOffset}
105105
anchor={anchor}
106-
className="isolate z-50"
106+
// `pointer-events-auto` re-enables interaction when the popup is portaled
107+
// out of a modal dialog (Radix sets `pointer-events: none` on the body
108+
// for modal dialogs/sheets; without this the portaled list can't be
109+
// scrolled or clicked). Harmless outside a dialog (auto is the default).
110+
className="isolate z-50 pointer-events-auto"
107111
>
108112
<ComboboxPrimitive.Popup
109113
data-slot="combobox-content"

packages/react/src/components/health-check-editor.tsx

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -483,7 +483,12 @@ function HealthCheckEditorSheet(props: {
483483
const canPreview = livePreview !== undefined && livePreview.templates.length > 0;
484484

485485
return (
486-
<Sheet open={open} onOpenChange={onOpenChange}>
486+
// Non-modal: a modal Radix dialog locks body scroll (react-remove-scroll)
487+
// and disables outside pointer events, which kills the combobox popup
488+
// (portaled to <body>, outside the dialog subtree) - it can't be scrolled or
489+
// clicked. The overlay still renders and the dismissal guard still keeps the
490+
// sheet open while interacting with the popup.
491+
<Sheet open={open} onOpenChange={onOpenChange} modal={false}>
487492
<SheetContent side="right">
488493
<SheetHeader>
489494
<SheetTitle>Health check</SheetTitle>

packages/react/src/components/sheet.tsx

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,18 @@ function SheetOverlay({
3838
);
3939
}
4040

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+
4147
function SheetContent({
4248
className,
4349
children,
4450
side = "right",
4551
showCloseButton = true,
52+
onInteractOutside,
4653
...props
4754
}: React.ComponentProps<typeof SheetPrimitive.Content> & {
4855
side?: "top" | "right" | "bottom" | "left";
@@ -53,6 +60,13 @@ function SheetContent({
5360
<SheetOverlay />
5461
<SheetPrimitive.Content
5562
data-slot="sheet-content"
63+
onInteractOutside={(event) => {
64+
const target = event.detail.originalEvent.target;
65+
if (target instanceof Element && target.closest(PORTALED_POPUP_SELECTOR)) {
66+
event.preventDefault();
67+
}
68+
onInteractOutside?.(event);
69+
}}
5670
className={cn(
5771
"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",
5872
side === "right" &&

0 commit comments

Comments
 (0)