feat: Dialog.Popup initialFocus / finalFocus, keep pending focus requests

Dialog.Popup takes Base UI's initialFocus and finalFocus: true (default),
false (stay), a ref or element, or a function returning one of these.
The shared resolveFocusTarget lives in @gpuix/native/host.

- initialFocus defaults to the popup itself. Base UI picks the first
  tabbable element, but GPUI's tab map exists only after the popup paints,
  so the first Tab walks that order instead.
- finalFocus defaults to the trigger, else the element focused when the
  popup opened. The Popup records it on open, so a dialog opened from app
  state (no Trigger, e.g. chat.tsx overlays) returns focus on close.
- Popup no longer uses autoFocus; it focuses imperatively after reading the
  previous focus.

Native fix found by the Solid test: sync_focus_handles took
pending_focus_element even when the handle did not exist yet, so a frame
rendered before the element's batch dropped the request. That is the
normal order for Solid (microtask flush) and for Windows/Linux (UI thread).
The request now stays until the handle exists. The test renderer's
focusElement uses the same request_focus path as the live renderer.

Session: ses_f2717eddcffeXQkI5JXN8eAnFD
This commit is contained in:
Tommy D. Rossi committed 2026-09-25 17:30:48 +02:00
1 parent 148c9e82b8
commit 6382a80c34
10 files changed
+212 -54

No files matched your search

+15
View File
@@ -0,0 +1,15 @@
---
'@gpuix/native': patch
'@gpuix/react': minor
'@gpuix/solid': minor
---
`Dialog.Popup` takes `initialFocus` and `finalFocus`, like Base UI.
```tsx
<Dialog.Popup initialFocus={searchRef} finalFocus={composerRef}>
```
Each accepts `true` (default), `false` (focus stays), a ref or element, or a function that returns one of these. `initialFocus` defaults to the popup itself. `finalFocus` defaults to the trigger, else to the element focused when the popup opened, so a dialog opened from app state with no `Trigger` now returns focus on close. The Popup no longer takes `autoFocus` for this.
`focusElement(id)` now keeps the request until that element's focus handle exists. Before, a frame that rendered before the element's batch arrived dropped it. This happened with Solid, which applies mutations in a microtask, and on Windows and Linux, which render on their own thread. The test renderer now uses the same path.
+21 -3
View File
@@ -2227,11 +2227,29 @@ import * as Dialog from '@gpuix/react/dialog'
| `Trigger` | Tab stop. Click, Enter, or Space opens |
| `Portal` | Full-window deferred layer. Paints over `<virtual-list>`. Centers its children by default. Modal: blocks clicks and the wheel behind it |
| `Backdrop` | Press closes the dialog |
| `Popup` | Takes focus on open. Modal: Tab and Shift+Tab stay inside. Close gives focus back to the trigger |
| `Popup` | Moves focus in on open and out on close. Modal: Tab and Shift+Tab stay inside |
| `Close` | Tab stop. Click, Enter, or Space closes |
The Popup focuses **itself**, so the first Tab enters it. To focus a field
instead, pass `autoFocus={false}` to the Popup and `autoFocus` to the field.
`initialFocus` and `finalFocus` on the Popup choose where focus goes, like
Base UI:
```tsx
<Dialog.Popup initialFocus={searchRef} finalFocus={composerRef}>
```
| Value | `initialFocus` (open) | `finalFocus` (close) |
|---|---|---|
| unset / `true` | The popup itself, so the first Tab enters it | The trigger, else the element focused when the popup opened |
| ref or element | That element | That element |
| `false` | Focus stays | Focus stays |
| function | Returns one of the above. `null` means the default | Same |
The default for `initialFocus` differs from Base UI, which picks the first
tabbable element. GPUI's tab order exists only after the popup has painted,
so GPUIX focuses the popup and lets the first Tab walk that order.
A dialog opened from app state, with no `Trigger`, still returns focus: the
Popup records the focused element when it opens.
A Select or Tooltip inside the Popup opens above it, and Escape closes it first.
+33
View File
@@ -251,3 +251,36 @@ export function unregisterEventHandlers(
): void {
eventHandlers.delete(elementId)
}
/** Anything a framework hands out for a mounted host element. */
export interface FocusableNode {
readonly id: number
}
/**
* Where an overlay moves focus when it opens or closes, like Base UI's
* `initialFocus` / `finalFocus`:
*
* - `true` or unset: the overlay's default target
* - `false`: focus does not move
* - an element or a ref to one: that element
* - a function: returns one of the above; `null` means the default
*/
export type FocusTarget<Node extends FocusableNode = FocusableNode> =
| boolean
| Node
| { readonly current: Node | null }
| (() => Node | boolean | null | undefined)
/** Resolve a `FocusTarget` to an element id, or null when focus must stay. */
export function resolveFocusTarget<Node extends FocusableNode>(
target: FocusTarget<Node> | undefined,
fallback: () => number | null
): number | null {
const value = typeof target === "function" ? target() : target
if (value === undefined && typeof target === "function") return null
if (value === undefined || value === null || value === true) return fallback()
if (value === false) return null
if ("current" in value) return value.current?.id ?? fallback()
return value.id
}
+10 -5
View File
@@ -4431,7 +4431,7 @@ impl VirtualListEntry {
}
impl GpuixView {
fn request_focus(&mut self, id: u64, window: &mut gpui::Window, cx: &mut gpui::Context<Self>) {
pub(crate) fn request_focus(&mut self, id: u64, window: &mut gpui::Window, cx: &mut gpui::Context<Self>) {
self.reveal_virtual_list_ancestor(id);
if let Some(handle) = self.focus_handles.get(&id) {
self.pending_focus_element = None;
@@ -4653,10 +4653,15 @@ impl GpuixView {
}
}
if let Some(id) = self.pending_focus_element.take() {
if let Some(handle) = self.focus_handles.get(&id) {
handle.focus(window, cx);
}
// Keep the request until the handle exists. A frame can render before
// the batch that creates the element arrives (Solid flushes in a
// microtask, Windows and Linux render on their own thread).
if let Some(handle) = self
.pending_focus_element
.and_then(|id| self.focus_handles.get(&id).cloned())
{
handle.focus(window, cx);
self.pending_focus_element = None;
}
self.focus_subscriptions.retain(|(id, event), _| {
+4 -7
View File
@@ -467,13 +467,10 @@ impl TestGpuixRenderer {
let view = view.clone();
cx.update_window(window, |_, window, app| {
view.update(app, |view, cx| {
view.reveal_virtual_list_ancestor(id);
if let Some(handle) = view.focus_handles.get(&id) {
handle.focus(window, cx);
}
cx.notify();
});
// Same path as the live renderer: an element whose focus
// handle does not exist yet (Solid applies its mutations in
// a microtask) takes focus when its handle is created.
view.update(app, |view, cx| view.request_focus(id, window, cx));
})
.map_err(|e| Error::from_reason(e.to_string()))?;
@@ -1,7 +1,7 @@
/** End-to-end tests for headless floating controls over the native GPUI pipeline. */
// @ts-nocheck
import React, { useState } from "react"
import React, { useRef, useState } from "react"
import { beforeEach, describe, expect, it } from "vitest"
import * as ComboboxPrimitive from "../components/combobox"
import { FloatingLayer } from "../components/floating"
@@ -1240,4 +1240,45 @@ describeNative("floating controls", () => {
}
`)
})
it("focuses initialFocus on open and returns focus without a Trigger", () => {
function Demo() {
const [open, setOpen] = useState(false)
const fieldRef = useRef(null)
return (
<div style={{ display: "flex", flexDirection: "column", width: 600, height: 400, padding: 12, gap: 8 }}>
<div
autoFocus
tabIndex={0}
testId="launcher"
onKeyDown={(event) => {
if (event.key === "enter") setOpen(true)
}}
style={{ width: 80, height: 24 }}
/>
<DialogPrimitive.Root open={open} onOpenChange={setOpen}>
<DialogPrimitive.Portal>
<DialogPrimitive.Popup initialFocus={fieldRef} style={{ width: 300, padding: 16, backgroundColor: "#1e293b" }}>
<div tabIndex={0} testId="first" style={{ width: 60, height: 24 }} />
<input ref={fieldRef} testId="field" style={{ width: 160, height: 28 }} />
</DialogPrimitive.Popup>
</DialogPrimitive.Portal>
</DialogPrimitive.Root>
</div>
)
}
testRoot.render(<Demo />)
const focused = () => testRoot.renderer.getElement(testRoot.renderer.getFocusedElementId())?.testId
const steps = []
testRoot.renderer.simulateKeystrokes("enter")
steps.push(`open: ${focused()}`)
testRoot.renderer.simulateKeystrokes("escape")
steps.push(`close: ${focused()}`)
expect(steps).toMatchInlineSnapshot(`
[
"open: field",
"close: launcher",
]
`)
})
})
+28 -23
View File
@@ -10,7 +10,8 @@ import React, {
} from "react"
import type { ReactElement, ReactNode } from "react"
import type { EventPayload } from "@gpuix/native"
import type { KeyEvent, Props, PublicInstance } from "../types/host.js"
import { resolveFocusTarget } from "@gpuix/native/host"
import type { FocusTarget, KeyEvent, Props, PublicInstance } from "../types/host.js"
import { useGpuix } from "../hooks/use-gpuix.js"
import { useWindowSize } from "../hooks/use-window-size.js"
import {
@@ -26,8 +27,6 @@ interface DialogContextValue {
disablePointerDismissal: boolean
setOpen: (open: boolean) => void
triggerRef: React.MutableRefObject<PublicInstance | null>
/** Focused element when the dialog opened, for a dialog without a trigger. */
returnFocusRef: React.MutableRefObject<number | null>
}
const DialogContext = createContext<DialogContextValue | null>(null)
@@ -62,25 +61,18 @@ export function Dialog({
modal = true,
disablePointerDismissal = false,
}: DialogProps): ReactElement {
const { renderer } = useGpuix()
const [open, setOpenState] = useControllableState({
const [open, setOpen] = useControllableState({
value: openProp,
defaultValue: defaultOpen,
onChange: onOpenChange,
})
const triggerRef = useRef<PublicInstance | null>(null)
const returnFocusRef = useRef<number | null>(null)
const setOpen = (next: boolean) => {
if (next && !open) returnFocusRef.current = renderer?.getFocusedElementId?.() ?? null
setOpenState(next)
}
const context: DialogContextValue = {
open,
modal,
disablePointerDismissal,
setOpen,
triggerRef,
returnFocusRef,
}
return <DialogContext.Provider value={context}>{children}</DialogContext.Provider>
}
@@ -187,29 +179,43 @@ export const DialogBackdrop = forwardRef<PublicInstance, DialogBackdropProps>(
}
)
export interface DialogPopupProps extends Props {}
export interface DialogPopupProps extends Props {
/** Focus on open. Default: the popup itself, so the first Tab enters it. */
initialFocus?: FocusTarget<PublicInstance>
/** Focus on close. Default: the trigger, else the element focused before open. */
finalFocus?: FocusTarget<PublicInstance>
}
/**
* The dialog surface. Focuses itself on open, so the first Tab enters it, and
* gives focus back to the trigger (or the element focused before) on close.
* Pass `autoFocus={false}` and put `autoFocus` on a child to focus that instead.
*/
/** The dialog surface. Moves focus in on open and back out on close. */
export const DialogPopup = forwardRef<PublicInstance, DialogPopupProps>(
function DialogPopup(
{ style, onKeyDown, autoFocus = true, tabIndex = -1, role = "dialog", ...props },
{ style, onKeyDown, initialFocus, finalFocus, tabIndex = -1, role = "dialog", ...props },
forwardedRef
) {
const context = useDialogContext("DialogPopup")
const { renderer } = useGpuix()
const popupRef = useRef<PublicInstance | null>(null)
const { triggerRef, returnFocusRef } = context
const focusTargets = useRef({ initialFocus, finalFocus })
focusTargets.current = { initialFocus, finalFocus }
const { triggerRef } = context
useLayoutEffect(() => {
if (!context.open || !renderer) return
// Read before moving focus, so a dialog opened from app state (no
// trigger) still knows where to return.
const previous = renderer.getFocusedElementId?.() ?? null
const initial = resolveFocusTarget(
focusTargets.current.initialFocus,
() => popupRef.current?.id ?? null
)
if (initial !== null) renderer.focusElement?.(initial)
return () => {
const target = triggerRef.current?.id ?? returnFocusRef.current
if (target != null) renderer.focusElement?.(target)
const final = resolveFocusTarget(
focusTargets.current.finalFocus,
() => triggerRef.current?.id ?? previous
)
if (final !== null) renderer.focusElement?.(final)
}
}, [context.open, renderer, triggerRef, returnFocusRef])
}, [context.open, renderer, triggerRef])
if (!context.open) return null
return (
<DismissableLayer onEscapeKeyDown={() => context.setOpen(false)}>
@@ -220,7 +226,6 @@ export const DialogPopup = forwardRef<PublicInstance, DialogPopupProps>(
setRefs(value, forwardedRef)
}}
role={role}
autoFocus={autoFocus}
tabIndex={tabIndex}
// A press inside the popup must never reach the backdrop behind it.
style={{ pointerEvents: "auto", ...style }}
+1
View File
@@ -129,6 +129,7 @@ export type {
HighlightSpec,
ImgInstance,
ImgProps,
FocusTarget,
InputProps,
KeyEvent,
LinearGradientBackground,
@@ -21,6 +21,7 @@ import {
TooltipTrigger,
} from "../index.js"
import { createTestRoot, hasNativeTestRenderer } from "../testing.js"
import type { HostElement } from "../host.js"
describe.skipIf(!hasNativeTestRenderer)("Solid controls", () => {
it("selects with keyboard and click", () => {
@@ -206,4 +207,38 @@ describe.skipIf(!hasNativeTestRenderer)("Solid controls", () => {
"press backdrop: focus=open text=Open",
])
})
it("focuses initialFocus on open and returns focus without a Trigger", () => {
const [open, setOpen] = createSignal(false)
let field!: HostElement
const app = createTestRoot()
app.render(() => (
<div style={{ display: "flex", flexDirection: "column", width: 600, height: 400, gap: 8 }}>
<div
autoFocus
tabIndex={0}
testId="launcher"
onKeyDown={(event) => {
if (event.key === "enter") setOpen(true)
}}
style={{ width: 80, height: 24 }}
/>
<Dialog open={open()} onOpenChange={setOpen}>
<DialogPortal>
<DialogPopup initialFocus={() => field} style={{ width: 300, padding: 16, backgroundColor: "#1e293b" }}>
<div tabIndex={0} testId="first" style={{ width: 60, height: 24 }} />
<input ref={field} testId="field" style={{ width: 160, height: 28 }} />
</DialogPopup>
</DialogPortal>
</Dialog>
</div>
))
const focused = () => app.renderer.getElement(app.renderer.getFocusedElementId()!)?.testId
const steps: string[] = []
app.renderer.simulateKeystrokes("enter")
steps.push(`open: ${focused()}`)
app.renderer.simulateKeystrokes("escape")
steps.push(`close: ${focused()}`)
expect(steps).toEqual(["open: field", "close: launcher"])
})
})
+23 -15
View File
@@ -5,12 +5,15 @@ import {
createContext,
createSignal,
onCleanup,
onMount,
Show,
splitProps,
useContext,
type JSX,
} from "solid-js"
import type { EventPayload } from "@gpuix/native"
import type { HostProps, KeyEvent } from "@gpuix/native/host"
import { resolveFocusTarget } from "@gpuix/native/host"
import type { FocusTarget, HostProps, KeyEvent } from "@gpuix/native/host"
import type { HostElement } from "../host.js"
import { jsx } from "../jsx-runtime.js"
import { createWindowSize } from "../primitives.js"
@@ -23,8 +26,6 @@ interface DialogState {
setOpen(open: boolean): void
pointerDismissal(): boolean
trigger: { current: HostElement | null }
/** Focused element when the dialog opened, for a dialog without a trigger. */
returnFocus: { current: number | null }
}
const DialogContext = createContext<DialogState>()
@@ -51,7 +52,6 @@ export interface DialogProps {
}
export function Dialog(props: DialogProps): JSX.Element {
const renderer = useGpuixRequired()
const [internalOpen, setInternalOpen] = createSignal(props.defaultOpen ?? false)
const open = () => props.open === undefined ? internalOpen() : props.open
const state: DialogState = {
@@ -59,11 +59,9 @@ export function Dialog(props: DialogProps): JSX.Element {
modal: () => props.modal ?? true,
pointerDismissal: () => !props.disablePointerDismissal,
trigger: { current: null },
returnFocus: { current: null },
setOpen(next) {
const previous = open()
if (previous === next) return
if (next) state.returnFocus.current = renderer.getFocusedElementId?.() ?? null
if (props.open === undefined) setInternalOpen(next)
props.onOpenChange?.(next)
},
@@ -171,14 +169,15 @@ export function DialogBackdrop(props: DialogBackdropProps): JSX.Element {
export interface DialogPopupProps extends HostProps {
children?: JSX.Element
ref?: (element: HostElement) => void
/** Focus on open. Default: the popup itself, so the first Tab enters it. */
initialFocus?: FocusTarget<HostElement>
/** Focus on close. Default: the trigger, else the element focused before open. */
finalFocus?: FocusTarget<HostElement>
}
/**
* The dialog surface. Focuses itself on open, so the first Tab enters it, and
* gives focus back to the trigger (or the element focused before) on close.
* Pass `autoFocus={false}` and put `autoFocus` on a child to focus that instead.
*/
export function DialogPopup(props: DialogPopupProps): JSX.Element {
/** The dialog surface. Moves focus in on open and back out on close. */
export function DialogPopup(allProps: DialogPopupProps): JSX.Element {
const [focus, props] = splitProps(allProps, ["initialFocus", "finalFocus"])
const state = context("DialogPopup")
const renderer = useGpuixRequired()
let popup: HostElement | null = null
@@ -186,9 +185,19 @@ export function DialogPopup(props: DialogPopupProps): JSX.Element {
get when() { return state.open() },
keyed: true,
get children() {
// Read before moving focus, so a dialog opened from app state (no
// trigger) still knows where to return. The popup is not built yet.
const previous = renderer.getFocusedElementId?.() ?? null
onMount(() => {
const initial = resolveFocusTarget(focus.initialFocus, () => popup?.id ?? null)
if (initial !== null) renderer.focusElement?.(initial)
})
onCleanup(() => {
const target = state.trigger.current?.id ?? state.returnFocus.current
if (target != null) renderer.focusElement?.(target)
const final = resolveFocusTarget(
focus.finalFocus,
() => state.trigger.current?.id ?? previous
)
if (final !== null) renderer.focusElement?.(final)
})
return createComponent(DismissableLayer, {
onEscapeKeyDown: () => state.setOpen(false),
@@ -200,7 +209,6 @@ export function DialogPopup(props: DialogPopupProps): JSX.Element {
props.ref?.(element)
},
get role() { return props.role ?? "dialog" },
get autoFocus() { return props.autoFocus ?? true },
get tabIndex() { return props.tabIndex ?? -1 },
// A press inside the popup must never reach the backdrop behind it.
get style() { return { pointerEvents: "auto", ...props.style } },