diff --git a/packages/app/e2e/regression/mobile-model-pickers.spec.ts b/packages/app/e2e/regression/mobile-model-pickers.spec.ts index 8eaf44173bd..7e535a5bb3b 100644 --- a/packages/app/e2e/regression/mobile-model-pickers.spec.ts +++ b/packages/app/e2e/regression/mobile-model-pickers.spec.ts @@ -117,7 +117,7 @@ for (const direction of ["ltr", "rtl"] as const) { await models.getByRole("button", { name: "Connect an integration", exact: true }).click() await expect(models).toBeHidden() const providers = page.getByRole("dialog", { name: "Connect an integration", exact: true }) - await expect(providers.getByRole("searchbox", { name: "Search providers", exact: true })).toBeVisible() + await expect(providers.getByRole("searchbox", { name: "Search providers", exact: true })).toBeFocused() await expect(page.getByRole("dialog")).toHaveCount(1) }) } diff --git a/packages/app/src/providers/models/unpaid.tsx b/packages/app/src/providers/models/unpaid.tsx index 2d6be0fb033..2d45d19c294 100644 --- a/packages/app/src/providers/models/unpaid.tsx +++ b/packages/app/src/providers/models/unpaid.tsx @@ -55,7 +55,7 @@ export const DialogSelectModelUnpaid: Component<{ model?: ModelState }> = (props dialog.close() } - // Focus starts on the dialog's close button, outside the list, so listen at the + // Focus starts on the dialog itself, outside the list, so listen at the // document level while the dialog is mounted instead of on the list container. let listEl: HTMLDivElement | undefined onMount(() => { diff --git a/packages/ui/src/overlays/dialog/dialog.tsx b/packages/ui/src/overlays/dialog/dialog.tsx index 39fe2b01575..e7afb897783 100644 --- a/packages/ui/src/overlays/dialog/dialog.tsx +++ b/packages/ui/src/overlays/dialog/dialog.tsx @@ -4,6 +4,12 @@ import { useDialogLayer } from "../../context/dialog" import { useI18n } from "../../context/i18n" import "./dialog.css" +const TEXT_FIELD = [ + 'input:not([type="hidden"], [type="checkbox"], [type="radio"], [type="button"], [type="submit"], [type="reset"], [type="range"], [type="file"], [type="color"], [type="image"]):not(:disabled, [readonly])', + "textarea:not(:disabled, [readonly])", + '[contenteditable="true"]', +].join(", ") + export interface DialogProps extends ParentProps { size?: "normal" | "large" | "x-large" variant?: "default" | "settings" @@ -120,15 +126,30 @@ export function Dialog(props: DialogProps) { [local.class ?? ""]: !!local.class, }} onOpenAutoFocus={(e) => { - const autofocusEl = - e.currentTarget instanceof HTMLElement ? e.currentTarget.querySelector("[autofocus]") : null + const content = e.currentTarget - if (autofocusEl) { - e.preventDefault() - // Kobalte attaches its focus trap after this event, so focusing now would leave the trap with no - // element to return to. Deferring, as Kobalte's own autofocus does, lets it pull stray focus back. - setTimeout(() => autofocusEl.focus({ preventScroll: true })) - } + if (!(content instanceof HTMLElement)) return + // Kobalte would focus the first tabbable element, usually the close button, which Escape already covers. + e.preventDefault() + // Kobalte attaches its focus trap after this event, so focusing now would leave the trap with no element + // to return to. Deferring, as Kobalte's own autofocus does, also lets children that focus themselves on + // mount keep that focus. + setTimeout(() => { + const autofocus = content.querySelector("[autofocus]") + const active = document.activeElement + const kept = + active instanceof HTMLElement && + active !== content && + content.contains(active) && + !active.closest('[data-slot="dialog-close-button"]') + ? active + : undefined + const field = Array.from(content.querySelectorAll(TEXT_FIELD)).find( + (element) => element.getClientRects().length > 0, + ) + const target = autofocus ?? kept ?? field ?? content + target.focus({ preventScroll: true }) + }) }} > {local.children}