fix(desktop): land deep-link focus on model and env-key targets

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 <body> 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 <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
This commit is contained in:
Duncan
2026-08-18 17:27:06 -04:00
parent b5465b04de
commit 83c73b34da
2 changed files with 112 additions and 27 deletions

View File

@@ -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={
<div className="flex w-full flex-wrap items-center justify-between gap-3">

View File

@@ -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
// <body> 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<EditAgentFocusTarget, { type: "normalized_field" }>,
): string {
return focus.field === "provider"
? "edit-agent-llm-provider"
: "edit-agent-model";
}