From 0a318f5ab65f6a9fd9f54bed015f0eb1efbe62ca Mon Sep 17 00:00:00 2001
From: Alex Blank <alexblank@fastmail.com>
Date: Thu, 27 Aug 2026 23:38:14 +0200
Subject: [PATCH] feat(model-selector): support popup menu lifecycle

---
 ui/src/components/ModelSelector.test.tsx | 37 +++++++++++++++++++-
 ui/src/components/ModelSelector.tsx      | 44 ++++++++++++++++++++----
 2 files changed, 73 insertions(+), 8 deletions(-)

diff --git a/ui/src/components/ModelSelector.test.tsx b/ui/src/components/ModelSelector.test.tsx
index ed0367a..a0f721a 100644
--- a/ui/src/components/ModelSelector.test.tsx
+++ b/ui/src/components/ModelSelector.test.tsx
@@ -1,4 +1,4 @@
-import { render, screen, waitFor } from "@testing-library/react";
+import { fireEvent, render, screen, waitFor } from "@testing-library/react";
 import userEvent from "@testing-library/user-event";
 import { describe, expect, it, vi } from "vitest";
 import type { Model } from "../workspace/types";
@@ -76,6 +76,41 @@ describe("ModelSelector", () => {
 		await waitFor(() => expect(trigger).toHaveFocus());
 	});
 
+	it("notifies menu open and every close path", async () => {
+		const user = userEvent.setup();
+		const onOpen = vi.fn();
+		const onClose = vi.fn();
+		const { unmount } = render(
+			<ModelSelector
+				models={models}
+				onSelect={vi.fn().mockResolvedValue(true)}
+				onOpen={onOpen}
+				onClose={onClose}
+			/>,
+		);
+		const trigger = screen.getByRole("button", { name: /Current model/i });
+
+		await user.click(trigger);
+		expect(onOpen).toHaveBeenCalledTimes(1);
+		fireEvent.pointerDown(document.body);
+		expect(onClose).toHaveBeenCalledTimes(1);
+
+		await user.click(trigger);
+		await user.keyboard("{Escape}");
+		expect(onOpen).toHaveBeenCalledTimes(2);
+		expect(onClose).toHaveBeenCalledTimes(2);
+
+		await user.click(trigger);
+		await user.click(screen.getByRole("option", { name: /Claude Sonnet/ }));
+		expect(onOpen).toHaveBeenCalledTimes(3);
+		expect(onClose).toHaveBeenCalledTimes(3);
+
+		await user.click(trigger);
+		unmount();
+		expect(onOpen).toHaveBeenCalledTimes(4);
+		expect(onClose).toHaveBeenCalledTimes(4);
+	});
+
 	it("supports labeled downward placement for toolbar and Settings reuse", () => {
 		const { container } = render(
 			<ModelSelector
diff --git a/ui/src/components/ModelSelector.tsx b/ui/src/components/ModelSelector.tsx
index ce83bd3..46113eb 100644
--- a/ui/src/components/ModelSelector.tsx
+++ b/ui/src/components/ModelSelector.tsx
@@ -92,6 +92,8 @@ export function ModelSelector({
 	label = "Model",
 	placement = "up",
 	className = "",
+	onOpen,
+	onClose,
 }: {
 	models: Model[];
 	current?: AgentState["model"];
@@ -101,13 +103,20 @@ export function ModelSelector({
 	label?: string;
 	placement?: "up" | "down";
 	className?: string;
+	onOpen?: () => void;
+	onClose?: () => void;
 }) {
 	const [open, setOpen] = useState(false);
 	const [query, setQuery] = useState("");
 	const [selecting, setSelecting] = useState(false);
+	const openRef = useRef(false);
+	const onOpenRef = useRef(onOpen);
+	const onCloseRef = useRef(onClose);
 	const rootRef = useRef<HTMLDivElement>(null);
 	const triggerRef = useRef<HTMLButtonElement>(null);
 	const searchRef = useRef<HTMLInputElement>(null);
+	onOpenRef.current = onOpen;
+	onCloseRef.current = onClose;
 	const labelId = useId();
 	const valueId = useId();
 	const menuId = useId();
@@ -133,6 +142,29 @@ export function ModelSelector({
 		? `${current.provider} · ${selectedName ?? "Current model"}`
 		: "Current model";
 	const busy = pending || selecting;
+	const openMenu = () => {
+		if (openRef.current) return;
+		openRef.current = true;
+		setQuery("");
+		setOpen(true);
+		onOpenRef.current?.();
+	};
+	const closeMenu = (restoreFocus = false) => {
+		if (!openRef.current) return;
+		openRef.current = false;
+		setOpen(false);
+		onCloseRef.current?.();
+		if (restoreFocus)
+			window.requestAnimationFrame(() => triggerRef.current?.focus());
+	};
+
+	useEffect(() => {
+		return () => {
+			if (!openRef.current) return;
+			openRef.current = false;
+			onCloseRef.current?.();
+		};
+	}, []);
 
 	useEffect(() => {
 		if (!open) return;
@@ -142,11 +174,10 @@ export function ModelSelector({
 		const closeOnEscape = (event: KeyboardEvent) => {
 			if (event.key !== "Escape") return;
 			event.preventDefault();
-			setOpen(false);
-			window.requestAnimationFrame(() => triggerRef.current?.focus());
+			closeMenu(true);
 		};
 		const closeOutside = (event: PointerEvent) => {
-			if (!rootRef.current?.contains(event.target as Node)) setOpen(false);
+			if (!rootRef.current?.contains(event.target as Node)) closeMenu();
 		};
 		window.addEventListener("keydown", closeOnEscape);
 		window.addEventListener("pointerdown", closeOutside);
@@ -162,8 +193,7 @@ export function ModelSelector({
 		setSelecting(true);
 		try {
 			if (!(await onSelect(model))) return;
-			setOpen(false);
-			window.requestAnimationFrame(() => triggerRef.current?.focus());
+			closeMenu(true);
 		} finally {
 			setSelecting(false);
 		}
@@ -186,8 +216,8 @@ export function ModelSelector({
 					aria-labelledby={`${labelId} ${valueId}`}
 					disabled={availableModels.length === 0 || busy || disabled}
 					onClick={() => {
-						setQuery("");
-						setOpen((value) => !value);
+						if (openRef.current) closeMenu();
+						else openMenu();
 					}}
 				>
 					<span id={valueId}>{selectedLabel}</span>