fix(desktop): let you type spaces and arrows in the session rename dialog
Renaming a session from its row menu opened the rename dialog, but the menu's close restored focus to its trigger — the session row's own <button> — instead of leaving it in the dialog input. Focus sat on the row, so Space toggled the row (selecting/deselecting the session) and the arrow keys moved the list rather than the text caret; you couldn't type a space or move within the name. Thread onCloseAutoFocus through the shared ActionsMenu / ActionsContextMenu primitives and suppress that one focus-restore when the rename item is the action that closed the menu, so the dialog input keeps focus. Every other action leaves the restore untouched. Mirrors the project menu's existing appearance-popover guard.
This commit is contained in:
committed by
brooklyn!
parent
f79440e0f4
commit
cd104ed49e
@@ -1,4 +1,4 @@
|
||||
import { cleanup, fireEvent, render, screen } from '@testing-library/react'
|
||||
import { cleanup, fireEvent, render, screen, waitFor, within } from '@testing-library/react'
|
||||
import { atom } from 'nanostores'
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
@@ -113,4 +113,28 @@ describe('SessionActionsMenu', () => {
|
||||
expect(screen.getByRole('menuitem', { name: /rename/i })).toBeTruthy()
|
||||
expect(screen.getByRole('menuitem', { name: /archive/i })).toBeTruthy()
|
||||
})
|
||||
|
||||
it('opens the rename dialog focused on its input, not the row trigger', async () => {
|
||||
renderMenu()
|
||||
|
||||
const trigger = screen.getByRole('button', { name: 'Session actions' })
|
||||
|
||||
fireEvent.pointerDown(trigger, { button: 0, pointerType: 'mouse' })
|
||||
fireEvent.pointerUp(trigger, { button: 0, pointerType: 'mouse' })
|
||||
fireEvent.click(trigger)
|
||||
|
||||
const rename = await screen.findByRole('menuitem', { name: /rename/i })
|
||||
fireEvent.click(rename)
|
||||
|
||||
// The dialog opens and its textbox takes focus. If the menu's close restored
|
||||
// focus to the row trigger instead, Space would activate the row and the
|
||||
// arrow keys would move the list rather than the caret (the reported bug).
|
||||
const dialog = await screen.findByRole('dialog')
|
||||
const input = within(dialog).getByRole('textbox')
|
||||
|
||||
// eslint-disable-next-line no-restricted-globals -- asserting real focus requires the live document
|
||||
await waitFor(() => expect(document.activeElement).toBe(input))
|
||||
// eslint-disable-next-line no-restricted-globals -- asserting real focus requires the live document
|
||||
expect(document.activeElement).not.toBe(trigger)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -189,6 +189,14 @@ function useSessionActions({
|
||||
const { t } = useI18n()
|
||||
const r = t.sidebar.row
|
||||
const [renameOpen, setRenameOpen] = useState(false)
|
||||
// The rename item opens a Dialog. When a menu closes, Radix restores focus to
|
||||
// its trigger — for a sidebar row that trigger is the row's own <button>, so
|
||||
// focus lands there instead of the dialog's input: Space then activates the
|
||||
// row (selecting the session) and the arrow keys move the list rather than
|
||||
// the caret. Suppress that one restore so the dialog keeps focus; every other
|
||||
// action leaves the restore alone (it's the correct behavior for them). Mirrors
|
||||
// the project menu's appearance-popover guard.
|
||||
const suppressCloseFocusRef = useRef(false)
|
||||
const tiles = useStore($sessionTiles)
|
||||
const selectedStoredSessionId = useStore($selectedStoredSessionId)
|
||||
|
||||
@@ -240,6 +248,8 @@ function useSessionActions({
|
||||
label: r.rename,
|
||||
onSelect: () => {
|
||||
triggerHaptic('selection')
|
||||
// Keep focus off the row trigger so it lands in the dialog input.
|
||||
suppressCloseFocusRef.current = true
|
||||
setRenameOpen(true)
|
||||
}
|
||||
}),
|
||||
@@ -438,7 +448,16 @@ function useSessionActions({
|
||||
/>
|
||||
)
|
||||
|
||||
return { renameDialog, renderItems }
|
||||
// Consumed once per close: when rename was the action that closed the menu,
|
||||
// block Radix's focus-restore to the trigger so the dialog input keeps focus.
|
||||
const onCloseAutoFocus = (event: Event) => {
|
||||
if (suppressCloseFocusRef.current) {
|
||||
suppressCloseFocusRef.current = false
|
||||
event.preventDefault()
|
||||
}
|
||||
}
|
||||
|
||||
return { onCloseAutoFocus, renameDialog, renderItems }
|
||||
}
|
||||
|
||||
interface SessionActionsMenuProps
|
||||
@@ -448,7 +467,7 @@ interface SessionActionsMenuProps
|
||||
|
||||
export function SessionActionsMenu({ children, align = 'end', sideOffset = 6, ...actions }: SessionActionsMenuProps) {
|
||||
const { t } = useI18n()
|
||||
const { renameDialog, renderItems } = useSessionActions(actions)
|
||||
const { onCloseAutoFocus, renameDialog, renderItems } = useSessionActions(actions)
|
||||
|
||||
return (
|
||||
<>
|
||||
@@ -457,6 +476,7 @@ export function SessionActionsMenu({ children, align = 'end', sideOffset = 6, ..
|
||||
ariaLabel={t.sidebar.row.sessionActions}
|
||||
contentClassName="w-40"
|
||||
items={renderItems}
|
||||
onCloseAutoFocus={onCloseAutoFocus}
|
||||
sideOffset={sideOffset}
|
||||
>
|
||||
{children}
|
||||
@@ -472,11 +492,16 @@ interface SessionContextMenuProps extends SessionActions {
|
||||
|
||||
export function SessionContextMenu({ children, ...actions }: SessionContextMenuProps) {
|
||||
const { t } = useI18n()
|
||||
const { renameDialog, renderItems } = useSessionActions(actions)
|
||||
const { onCloseAutoFocus, renameDialog, renderItems } = useSessionActions(actions)
|
||||
|
||||
return (
|
||||
<>
|
||||
<ActionsContextMenu ariaLabel={t.sidebar.row.sessionActions} contentClassName="w-40" items={renderItems}>
|
||||
<ActionsContextMenu
|
||||
ariaLabel={t.sidebar.row.sessionActions}
|
||||
contentClassName="w-40"
|
||||
items={renderItems}
|
||||
onCloseAutoFocus={onCloseAutoFocus}
|
||||
>
|
||||
{children}
|
||||
</ActionsContextMenu>
|
||||
{renameDialog}
|
||||
|
||||
@@ -99,7 +99,7 @@ export function renderActionItem(
|
||||
|
||||
interface ActionsMenuProps extends Pick<
|
||||
React.ComponentProps<typeof DropdownMenuContent>,
|
||||
'align' | 'side' | 'sideOffset'
|
||||
'align' | 'side' | 'sideOffset' | 'onCloseAutoFocus'
|
||||
> {
|
||||
/** The trigger (a kebab button). Wrapped in `DropdownMenuTrigger asChild`. */
|
||||
children: React.ReactNode
|
||||
@@ -122,6 +122,7 @@ export function ActionsMenu({
|
||||
children,
|
||||
contentClassName,
|
||||
items,
|
||||
onCloseAutoFocus,
|
||||
onOpenChange,
|
||||
open,
|
||||
side,
|
||||
@@ -134,6 +135,7 @@ export function ActionsMenu({
|
||||
align={align}
|
||||
aria-label={ariaLabel}
|
||||
className={contentClassName}
|
||||
onCloseAutoFocus={onCloseAutoFocus}
|
||||
side={side}
|
||||
sideOffset={sideOffset}
|
||||
>
|
||||
@@ -152,6 +154,7 @@ interface ActionsContextMenuProps {
|
||||
contentClassName?: string
|
||||
/** Skip the wrapper (render children bare) — e.g. nothing is actionable yet. */
|
||||
disabled?: boolean
|
||||
onCloseAutoFocus?: (event: Event) => void
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -163,7 +166,8 @@ export function ActionsContextMenu({
|
||||
children,
|
||||
contentClassName,
|
||||
disabled,
|
||||
items
|
||||
items,
|
||||
onCloseAutoFocus
|
||||
}: ActionsContextMenuProps) {
|
||||
if (disabled) {
|
||||
return <>{children}</>
|
||||
@@ -172,7 +176,7 @@ export function ActionsContextMenu({
|
||||
return (
|
||||
<ContextMenu>
|
||||
<ContextMenuTrigger asChild>{children}</ContextMenuTrigger>
|
||||
<ContextMenuContent aria-label={ariaLabel} className={contentClassName}>
|
||||
<ContextMenuContent aria-label={ariaLabel} className={contentClassName} onCloseAutoFocus={onCloseAutoFocus}>
|
||||
{items(CONTEXT_KIT)}
|
||||
</ContextMenuContent>
|
||||
</ContextMenu>
|
||||
|
||||
Reference in New Issue
Block a user