mirror of
https://github.com/remorses/gpuix.git
synced 2026-10-29 03:04:52 +08:00
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:
1 parent
148c9e82b8
commit
6382a80c34
10 files changed
+212
-54
No files matched your search
@@ -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.
|
||||
@@ -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.
|
||||
|
||||
|
||||
@@ -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
|
||||
}
|
||||
@@ -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), _| {
|
||||
|
||||
@@ -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",
|
||||
]
|
||||
`)
|
||||
})
|
||||
})
|
||||
@@ -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 }}
|
||||
|
||||
@@ -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"])
|
||||
})
|
||||
})
|
||||
@@ -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 } },
|
||||
|
||||
Reference in new issue
Block a user