From 83c73b34daad3fb95724cd3d1798df6bc70f33cc Mon Sep 17 00:00:00 2001 From: Duncan Date: Tue, 18 Aug 2026 17:27:06 -0400 Subject: [PATCH] fix(desktop): land deep-link focus on model and env-key targets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two deep-link focus targets never received focus in the merged edit dialog. The model/provider combobox is disabled while model discovery is in flight, so the one-shot rAF focus effect fired against a disabled control (a silent no-op) and latched its guard, never retrying. Guard the latch on focusability and add modelDiscoveryLoading to the deps so the effect re-runs and focuses once discovery resolves. The env_key target lives inside the Advanced accordion, which seeds collapsed, so EnvVarsEditor never mounted to receive its focus key. Add an effect that expands Advanced when an env_key target is requested. Replace the blanket onOpenAutoFocus suppression — which stranded focus on whenever the target wasn't focusable yet — with conditional suppression: claim focus here only when the normalized target is already focusable, otherwise let Radix autofocus a baseline control while the targeted effects move focus once the target materializes. Extract the three focus effects and the autofocus handler into useAgentEditDeepLinkFocus, deduplicating the target-id resolution and keeping AgentEditMergedDialog.tsx under the desktop file-size ratchet. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- .../agents/ui/AgentEditMergedDialog.tsx | 37 ++----- .../agents/ui/useAgentEditDeepLinkFocus.ts | 102 ++++++++++++++++++ 2 files changed, 112 insertions(+), 27 deletions(-) create mode 100644 desktop/src/features/agents/ui/useAgentEditDeepLinkFocus.ts diff --git a/desktop/src/features/agents/ui/AgentEditMergedDialog.tsx b/desktop/src/features/agents/ui/AgentEditMergedDialog.tsx index 7c8d3a06e5..68557d8397 100644 --- a/desktop/src/features/agents/ui/AgentEditMergedDialog.tsx +++ b/desktop/src/features/agents/ui/AgentEditMergedDialog.tsx @@ -63,6 +63,7 @@ import { import type { EnvVarsValue } from "./EnvVarsEditor"; import { formatPersonaNamePoolText } from "./personaDialogState"; import { useAgentEditRuntimeState } from "./useAgentEditRuntimeState"; +import { useAgentEditDeepLinkFocus } from "./useAgentEditDeepLinkFocus"; // ── Types / Component ──────────────────────────────────────────────────────── @@ -608,30 +609,14 @@ export function AgentEditMergedDialog({ }); // ── Focus target (R2) ────────────────────────────────────────────────────── - const normalizedFieldFocusFiredRef = React.useRef(false); - // biome-ignore lint/correctness/useExhaustiveDependencies: intentional — reset guard on open/context-switch; pure ref mutation is correct - React.useEffect(() => { - normalizedFieldFocusFiredRef.current = false; - }, [open, inst?.pubkey, def?.id]); - - // biome-ignore lint/correctness/useExhaustiveDependencies: intentional — llmProviderFieldVisible is the availability signal; inst?.pubkey handles agent-switch - React.useEffect(() => { - if (!open || !initialFocus) return; - if (initialFocus.type !== "normalized_field") return; - if (normalizedFieldFocusFiredRef.current) return; - const targetId = - initialFocus.field === "provider" - ? "edit-agent-llm-provider" - : "edit-agent-model"; - const el = document.getElementById(targetId); - if (!(el instanceof HTMLElement)) return; - normalizedFieldFocusFiredRef.current = true; - const id = requestAnimationFrame(() => { - el.scrollIntoView({ block: "nearest" }); - el.focus(); - }); - return () => cancelAnimationFrame(id); - }, [open, initialFocus, inst?.pubkey, llmProviderFieldVisible]); + const { onOpenAutoFocus } = useAgentEditDeepLinkFocus({ + open, + initialFocus, + contextKey: `${inst?.pubkey ?? ""}:${def?.id ?? ""}`, + llmProviderFieldVisible, + modelDiscoveryLoading, + setShowAdvancedFields, + }); // ── Render ───────────────────────────────────────────────────────────────── function handleSubmit() { @@ -682,9 +667,7 @@ export function AgentEditMergedDialog({ data-testid="edit-agent-dialog" footerClassName="border-t-0 pt-0" headerClassName="pb-2" - // Deep-link focus targets are owned by the rAF effects above; suppress - // Radix's name-input open-autofocus so it can't steal the requested field. - onOpenAutoFocus={initialFocus ? (e) => e.preventDefault() : undefined} + onOpenAutoFocus={onOpenAutoFocus} title={dialogTitle} footer={
diff --git a/desktop/src/features/agents/ui/useAgentEditDeepLinkFocus.ts b/desktop/src/features/agents/ui/useAgentEditDeepLinkFocus.ts new file mode 100644 index 0000000000..699bbf6a2a --- /dev/null +++ b/desktop/src/features/agents/ui/useAgentEditDeepLinkFocus.ts @@ -0,0 +1,102 @@ +import * as React from "react"; +import type { EditAgentFocusTarget } from "@/features/agents/openEditAgentEvent"; + +/** + * Deep-link focus coordination for the merged agent edit dialog. + * + * A card can open the dialog with a target field to focus. Two of those + * targets are not focusable at open time: the provider/model combobox trigger + * is `disabled` while model discovery is in flight, and an `env_key` row lives + * inside the Advanced accordion, which seeds collapsed so its editor is + * unmounted. This hook lands focus on the requested target once it becomes + * reachable, and returns the `onOpenAutoFocus` handler that keeps Radix from + * racing (or stranding) that focus. + */ +export function useAgentEditDeepLinkFocus({ + open, + initialFocus, + contextKey, + llmProviderFieldVisible, + modelDiscoveryLoading, + setShowAdvancedFields, +}: { + open: boolean; + initialFocus?: EditAgentFocusTarget; + /** Reset the one-shot guard when the edited agent/definition changes. */ + contextKey: string | undefined; + llmProviderFieldVisible: boolean; + modelDiscoveryLoading: boolean; + setShowAdvancedFields: (next: boolean) => void; +}): { + onOpenAutoFocus: ((event: Event) => void) | undefined; +} { + const normalizedFieldFocusFiredRef = React.useRef(false); + // biome-ignore lint/correctness/useExhaustiveDependencies: intentional — reset guard on open/context-switch; pure ref mutation is correct + React.useEffect(() => { + normalizedFieldFocusFiredRef.current = false; + }, [open, contextKey]); + + // biome-ignore lint/correctness/useExhaustiveDependencies: intentional — llmProviderFieldVisible and modelDiscoveryLoading are the availability signals; contextKey handles agent-switch + React.useEffect(() => { + if (!open || initialFocus?.type !== "normalized_field") return; + if (normalizedFieldFocusFiredRef.current) return; + const el = document.getElementById(normalizedFieldId(initialFocus)); + if (!(el instanceof HTMLElement)) return; + // The model/provider combobox trigger is disabled while model discovery is + // in flight; focusing a disabled control is a silent no-op. Wait for it to + // become focusable — this effect re-runs when modelDiscoveryLoading flips — + // and only latch the one-shot guard once focus can actually land. + if (el.hasAttribute("disabled")) return; + normalizedFieldFocusFiredRef.current = true; + const id = requestAnimationFrame(() => { + el.scrollIntoView({ block: "nearest" }); + el.focus(); + }); + return () => cancelAnimationFrame(id); + }, [ + open, + initialFocus, + contextKey, + llmProviderFieldVisible, + modelDiscoveryLoading, + ]); + + // env_key deep links live inside the Advanced accordion, which seeds + // collapsed on open — so EnvVarsEditor (which consumes the focus key) is + // unmounted and can never receive focus. Expand Advanced when an env_key + // target is requested so the editor mounts and its own focus effect can run. + // biome-ignore lint/correctness/useExhaustiveDependencies: intentional — re-expand on open/target-change; contextKey handles agent-switch + React.useEffect(() => { + if (!open || initialFocus?.type !== "env_key") return; + setShowAdvancedFields(true); + }, [open, initialFocus, contextKey]); + + const onOpenAutoFocus = initialFocus + ? (event: Event) => { + // When a target is supplied we cannot let Radix's open-autofocus race + // the targeted focus effects. But blanket suppression strands focus on + // whenever the target isn't focusable yet (model combobox + // disabled during discovery; env-key row unmounted while Advanced is + // collapsed). So suppress ONLY when the normalized target is already + // focusable and claim it here; otherwise let Radix autofocus a sane + // first control as the baseline and let the targeted effects move focus + // once the target materializes. + if (initialFocus.type !== "normalized_field") return; + const target = document.getElementById(normalizedFieldId(initialFocus)); + if (target instanceof HTMLElement && !target.hasAttribute("disabled")) { + event.preventDefault(); + target.focus(); + } + } + : undefined; + + return { onOpenAutoFocus }; +} + +function normalizedFieldId( + focus: Extract, +): string { + return focus.field === "provider" + ? "edit-agent-llm-provider" + : "edit-agent-model"; +}