From 347f1324ce3b95dc258bbc312373ecd9156226c1 Mon Sep 17 00:00:00 2001 From: Nadav Erell Date: Sun, 4 Oct 2026 01:52:30 +0300 Subject: [PATCH] Move focus into a new settings draft in the same commit Copying settings, loading previous global settings and switching to auto-discovery remount the form, then focused its first field in the next animation frame. A keystroke typed before that frame went to the old target, and one typed as the frame fired could land in the newly focused field instead. Focus now moves in a layout effect, during the commit that mounts the form. --- web/e2e/settings-connections.spec.ts | 9 ++++++++- .../settings/LocalConnectionSettings.tsx | 16 ++++++++++++---- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/web/e2e/settings-connections.spec.ts b/web/e2e/settings-connections.spec.ts index f2650d45d3..4bf4f9e374 100644 --- a/web/e2e/settings-connections.spec.ts +++ b/web/e2e/settings-connections.spec.ts @@ -508,7 +508,14 @@ test('copy into an unconfigured cluster saves edited headers without an intermed binding: 'source', integration: 'metrics', context: 'staging', source: '/test/staging', inFileName: 'staging', availability: 'available', revision: 'source-revision' }) const dialog = await openSettings(page, 'Metrics') await dialog.getByRole('button', { name: 'Copy settings from…', exact: true }).click() - await dialog.getByRole('option', { name: /staging/ }).click() + // Focus has to reach the copied form before the next frame; a later move can + // take a keystroke meant for another field. + const focusedBeforeFrame = await dialog.getByRole('option', { name: /staging/ }).evaluate(async option => { + (option as HTMLElement).click() + await Promise.resolve() + return (document.activeElement as HTMLElement | null)?.labels?.[0]?.textContent + }) + expect(focusedBeforeFrame).toBe('Metrics backend URL') await dialog.getByRole('textbox', { name: 'X-Scope-OrgID value', exact: true }).fill('new-tenant') await dialog.getByRole('textbox', { name: 'Metrics backend URL', exact: true }).fill('https://source.example/query') expect(state.writes).toHaveLength(0) diff --git a/web/src/components/settings/LocalConnectionSettings.tsx b/web/src/components/settings/LocalConnectionSettings.tsx index d9a53a87cf..562c2eb695 100644 --- a/web/src/components/settings/LocalConnectionSettings.tsx +++ b/web/src/components/settings/LocalConnectionSettings.tsx @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useRef, useState, type ReactNode } from 'react' +import { useCallback, useEffect, useLayoutEffect, useRef, useState, type ReactNode } from 'react' import { Badge, Collapse, ConfirmDialog, SelectMenu } from '@skyhook-io/k8s-ui' import { ArrowLeft, Info } from 'lucide-react' import { Tooltip } from '../ui/Tooltip' @@ -207,6 +207,14 @@ export function LocalConnectionSettings({ const editorRegion = useRef(null) const trigger = useRef(null) const mounted = useRef(true) + // Focus moves in the commit that mounts a new draft form. Deferring it to a + // later frame lets keystrokes typed in between land in a different field. + const focusAfterDraft = useRef<(() => void) | null>(null) + useLayoutEffect(() => { + const focus = focusAfterDraft.current + focusAfterDraft.current = null + focus?.() + }) const copySources = catalog.filter(c => c.integration === kind && !c.error && c.url && c.binding !== profile.target.binding) const removalContext = profile.target.context const dirty = @@ -535,7 +543,7 @@ export function LocalConnectionSettings({ setDraftGeneration((generation) => generation + 1) setError('') // Cost's first control is its source picker, a listbox trigger. - requestAnimationFrame(() => region.current?.querySelector('input:not(:disabled), button[aria-haspopup="listbox"]:not(:disabled)')?.focus()) + focusAfterDraft.current = () => region.current?.querySelector('input:not(:disabled), button[aria-haspopup="listbox"]:not(:disabled)')?.focus() }} className="text-xs text-accent-text hover:underline disabled:opacity-50 shrink-0" > @@ -555,7 +563,7 @@ export function LocalConnectionSettings({ setDraftGeneration(generation => generation + 1) setMessage('') setError('') - requestAnimationFrame(() => editorRegion.current?.querySelector('input:not(:disabled)')?.focus()) + focusAfterDraft.current = () => editorRegion.current?.querySelector('input:not(:disabled)')?.focus() } const copySource = (source: StoredConnection) => { setSelected(source) @@ -571,7 +579,7 @@ export function LocalConnectionSettings({ setDraftGeneration(generation => generation + 1) setMessage('') setError('') - requestAnimationFrame(() => editorRegion.current?.querySelector('input:not(:disabled)')?.focus()) + focusAfterDraft.current = () => editorRegion.current?.querySelector('input:not(:disabled)')?.focus() } const reuseOptions = task === 'main' && profile.state !== 'launch' ? [ ...(previous ? [{