ceos/unsloth
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

studio: restore focus to opener when settings dialog closes (#5612)

Browse source 
The settings dialog opens via a global Ctrl+, keydown handler in
__root.tsx, not via a <DialogTrigger>. Radix's FocusScope tries to
capture document.activeElement at mount as the focus-restore target,
but settings-dialog.tsx schedules a requestAnimationFrame that focuses
the active tab button right after mount, racing FocusScope's previous-
focus capture. On Escape or close-button click, focus then lands on
<body> instead of the textarea (or button, or wherever the user was).

A Playwright focus-management probe confirmed: open dialog, press Tab
15 times (trap holds), press Escape, document.activeElement === BODY.
This is a WCAG 2.4.3 (Focus Order) violation: keyboard-only users
have to re-Tab from the start of the page after every settings visit.

Fix: capture document.activeElement in the Zustand store at the moment
openDialog() runs, then restore via onCloseAutoFocus on DialogContent.
Use opener.isConnected so a stale node from a re-rendered tree falls
back to Radix's default. closeDialog deliberately does NOT clear the
opener slot - onCloseAutoFocus reads it on the render after open=false,
so clearing in the same set() would null it before restoration.

Probe re-run confirms focus restored to the TEXTAREA opener after
Escape, after close-button click, on both repeats. Tab + Shift+Tab
trap still holds (unchanged Radix behaviour).

Co-authored-by: danielhanchen <michaelhan2050@gmail.com>
This commit is contained in:
Daniel Han 2026-05-19 06:56:48 -07:00 • committed by GitHub
commit cf53ff6861
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 25 additions and 0 deletions
Download patch file Download diff file Expand all files Collapse all files

10
studio/frontend/src/features/settings/settings-dialog.tsx
View file

@ -74,6 +74,7 @@ export function SettingsDialog() {
const activeTab = useSettingsDialogStore((s) => s.activeTab);
const setActiveTab = useSettingsDialogStore((s) => s.setActiveTab);
const closeDialog = useSettingsDialogStore((s) => s.closeDialog);
const opener = useSettingsDialogStore((s) => s.opener);
const reduced = useReducedMotion();
const tabButtonRefs = useRef<Record<SettingsTab, HTMLButtonElement | null>>({
general: null,
@ -98,6 +99,15 @@ export function SettingsDialog() {
<DialogContent
showCloseButton={false}
overlayClassName="bg-background/40"
onCloseAutoFocus={(e) => {
// Restore focus to the element that triggered openDialog().
// Radix's FocusScope races our rAF-scheduled tab-button focus
// and loses the previous-focus reference, so we restore by hand.
if (opener && opener.isConnected) {
e.preventDefault();
opener.focus({ preventScroll: true });
}
}}
className={cn(
"!max-w-none h-[560px] w-[820px] p-0 overflow-hidden",
"shadow-border rounded-xl border-border",

15
studio/frontend/src/features/settings/stores/settings-dialog-store.ts
View file

@ -15,6 +15,11 @@ export type SettingsTab =
interface SettingsDialogState {
open: boolean;
activeTab: SettingsTab;
// Element focused at the moment openDialog() ran. Radix's FocusScope
// would normally track this, but the rAF-scheduled focus() in
// settings-dialog.tsx races its previous-focus capture, leaving focus
// on <body> after close. We restore explicitly via onCloseAutoFocus.
opener: HTMLElement | null;
openDialog: (tab?: SettingsTab) => void;
closeDialog: () => void;
setActiveTab: (tab: SettingsTab) => void;
@ -47,11 +52,21 @@ function loadInitialTab(): SettingsTab {
export const useSettingsDialogStore = create<SettingsDialogState>((set) => ({
open: false,
activeTab: loadInitialTab(),
opener: null,
openDialog: (tab) =>
set((state) => ({
open: true,
activeTab: tab ?? state.activeTab,
opener:
typeof document !== "undefined" &&
document.activeElement instanceof HTMLElement &&
document.activeElement !== document.body
? document.activeElement
: null,
})),
// Do NOT clear `opener` here. onCloseAutoFocus runs on the next render
// pass after `open: false` lands, so the opener must still be readable
// from the store at that point. The next openDialog() overwrites it.
closeDialog: () => set({ open: false }),
setActiveTab: (tab) => {
try {