fix(host): review round 5 nits — refocus covers Enter/Escape exits; trailing pressed check; narrowed v8 ignores
This commit is contained in:
@@ -233,10 +233,15 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen,
|
|||||||
})
|
})
|
||||||
}, [launchListing])
|
}, [launchListing])
|
||||||
|
|
||||||
// An editing-time pick parks focus on the selection after commit; the
|
// Editor-close focus parking (consumed by the refocus effect below the
|
||||||
// flag is set by select() and consumed by the refocus effect below the
|
// miller-row ref): a pick parks on the selection's row, Enter and an
|
||||||
// miller-row ref.
|
// input-focused Escape park on the crumb edit zone that replaces the
|
||||||
|
// input. Pointer-out cancels never set (or clear) these — yanking focus
|
||||||
|
// back from wherever the user clicked would be worse than the fall.
|
||||||
const refocusPick = useRef(false)
|
const refocusPick = useRef(false)
|
||||||
|
const refocusEditZone = useRef(false)
|
||||||
|
const pathInputRef = useRef<HTMLInputElement | null>(null)
|
||||||
|
const editZoneRef = useRef<HTMLButtonElement | null>(null)
|
||||||
|
|
||||||
/** Select a row of the listed level and preview its children on the right. */
|
/** Select a row of the listed level and preview its children on the right. */
|
||||||
const select = useCallback((entry: DirectoryEntry) => {
|
const select = useCallback((entry: DirectoryEntry) => {
|
||||||
@@ -373,17 +378,33 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen,
|
|||||||
const row = millerRowRef.current
|
const row = millerRowRef.current
|
||||||
if (row !== null && childPath !== undefined) row.scrollLeft = row.scrollWidth
|
if (row !== null && childPath !== undefined) row.scrollLeft = row.scrollWidth
|
||||||
}, [childPath])
|
}, [childPath])
|
||||||
// An editing-time pick unmounts the focused input, and a right-pane pick
|
// Every editor exit that would drop focus to body re-parks it after
|
||||||
// additionally replaces the picked button's whole column (advance swaps
|
// commit, so keyboard traversal stays inside the dialog (the Modal has no
|
||||||
// both panes): park focus on the selection's row — aria-current in the
|
// focus trap): a pick lands on the selection's row — aria-current in the
|
||||||
// freshly rendered left pane — after commit, so keyboard traversal stays
|
// freshly rendered left pane, which survives even a right-pane advance
|
||||||
// inside the dialog (the Modal has no focus trap).
|
// replacing the picked button's column — while Enter and an input-focused
|
||||||
|
// Escape land on the crumb edit zone that replaces the input.
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (!refocusPick.current) return
|
if (pathDraft !== null) return
|
||||||
refocusPick.current = false
|
if (refocusPick.current) {
|
||||||
/* v8 ignore next 2 -- narrowing guard: the pick that set the flag just rendered its aria-current row inside the miller row. */
|
refocusPick.current = false
|
||||||
const row = millerRowRef.current?.querySelector<HTMLButtonElement>('button[aria-current="true"]')
|
refocusEditZone.current = false
|
||||||
row?.focus()
|
const rowHost = millerRowRef.current
|
||||||
|
/* v8 ignore next -- narrowing guard: the miller row is mounted whenever a pick just committed. */
|
||||||
|
if (rowHost === null) return
|
||||||
|
const row = rowHost.querySelector<HTMLButtonElement>('button[aria-current="true"]')
|
||||||
|
/* v8 ignore next -- narrowing guard: the pick that set the flag just rendered its aria-current row. */
|
||||||
|
if (row === null) return
|
||||||
|
row.focus()
|
||||||
|
return
|
||||||
|
}
|
||||||
|
if (refocusEditZone.current) {
|
||||||
|
refocusEditZone.current = false
|
||||||
|
const zone = editZoneRef.current
|
||||||
|
/* v8 ignore next -- narrowing guard: crumb mode renders the edit zone whenever the editor just closed. */
|
||||||
|
if (zone === null) return
|
||||||
|
zone.focus()
|
||||||
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
if (!open) return null
|
if (!open) return null
|
||||||
@@ -424,6 +445,10 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen,
|
|||||||
// document listener — the same containment the input previously
|
// document listener — the same containment the input previously
|
||||||
// provided for itself.
|
// provided for itself.
|
||||||
event.stopPropagation()
|
event.stopPropagation()
|
||||||
|
// Escape while the input holds focus is about to unmount it; with
|
||||||
|
// focus already parked on a row, that row survives the cancel and
|
||||||
|
// keeps focus naturally.
|
||||||
|
if (document.activeElement === pathInputRef.current) refocusEditZone.current = true
|
||||||
cancelPathEdit()
|
cancelPathEdit()
|
||||||
}}
|
}}
|
||||||
// Focus leaving THIS dialog card while editing cancels like Escape.
|
// Focus leaving THIS dialog card while editing cancels like Escape.
|
||||||
@@ -442,6 +467,10 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen,
|
|||||||
/* v8 ignore next -- narrowing guard: this scope always renders inside the Modal card. */
|
/* v8 ignore next -- narrowing guard: this scope always renders inside the Modal card. */
|
||||||
if (card === null) return
|
if (card === null) return
|
||||||
if (event.relatedTarget instanceof Node && card.contains(event.relatedTarget)) return
|
if (event.relatedTarget instanceof Node && card.contains(event.relatedTarget)) return
|
||||||
|
// The user moved focus out of the card themselves: cancel without
|
||||||
|
// re-parking (a lingering Enter-failure flag must not yank focus
|
||||||
|
// back either).
|
||||||
|
refocusEditZone.current = false
|
||||||
cancelPathEdit()
|
cancelPathEdit()
|
||||||
}}
|
}}
|
||||||
>
|
>
|
||||||
@@ -475,6 +504,7 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen,
|
|||||||
// listing itself fails, typing an absolute path is the one
|
// listing itself fails, typing an absolute path is the one
|
||||||
// remaining way forward.
|
// remaining way forward.
|
||||||
disabled={parentInert}
|
disabled={parentInert}
|
||||||
|
ref={editZoneRef}
|
||||||
onClick={() => {
|
onClick={() => {
|
||||||
// Opening the editor supersedes any pending listing: a
|
// Opening the editor supersedes any pending listing: a
|
||||||
// settlement landing before the first keystroke would
|
// settlement landing before the first keystroke would
|
||||||
@@ -502,6 +532,7 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen,
|
|||||||
value={pathDraft}
|
value={pathDraft}
|
||||||
aria-label={t('browser.editPath')}
|
aria-label={t('browser.editPath')}
|
||||||
autoFocus
|
autoFocus
|
||||||
|
ref={pathInputRef}
|
||||||
disabled={parentInert}
|
disabled={parentInert}
|
||||||
onChange={(event) => {
|
onChange={(event) => {
|
||||||
// Editing the draft supersedes any in-flight navigation:
|
// Editing the draft supersedes any in-flight navigation:
|
||||||
@@ -521,7 +552,13 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen,
|
|||||||
// Trim only detects a blank draft; the Host gets the
|
// Trim only detects a blank draft; the Host gets the
|
||||||
// original text — a real directory name may end in
|
// original text — a real directory name may end in
|
||||||
// whitespace, and trimming would list its sibling.
|
// whitespace, and trimming would list its sibling.
|
||||||
if (pathDraft.trim() !== '') navigate(pathDraft)
|
if (pathDraft.trim() !== '') {
|
||||||
|
// Success will unmount the still-focused input; park
|
||||||
|
// focus on the returning crumb edit zone (a failure
|
||||||
|
// keeps the editor, so the flag waits until close).
|
||||||
|
refocusEditZone.current = true
|
||||||
|
navigate(pathDraft)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}}
|
}}
|
||||||
/>
|
/>
|
||||||
@@ -586,8 +623,10 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen,
|
|||||||
onMouseDown={draftPending ? (event) => { event.preventDefault() } : undefined}
|
onMouseDown={draftPending ? (event) => { event.preventDefault() } : undefined}
|
||||||
onClick={() => { setShowHidden(prev => !prev) }}
|
onClick={() => { setShowHidden(prev => !prev) }}
|
||||||
>
|
>
|
||||||
{showHidden && <IconCheckOutline16 size={14} />}
|
|
||||||
{t('browser.showHidden')}
|
{t('browser.showHidden')}
|
||||||
|
{/* Trailing check (Menu's selected vocabulary): the label never
|
||||||
|
* shifts when the pressed state toggles. */}
|
||||||
|
{showHidden && <IconCheckOutline16 size={14} />}
|
||||||
</button>
|
</button>
|
||||||
<span className={css.footerGap} />
|
<span className={css.footerGap} />
|
||||||
<Button variant="outline" className={clsx(css.footerAction)} disabled={parentInert} onClick={onClose}>{t('browser.cancel')}</Button>
|
<Button variant="outline" className={clsx(css.footerAction)} disabled={parentInert} onClick={onClose}>{t('browser.cancel')}</Button>
|
||||||
|
|||||||
@@ -227,6 +227,9 @@ describe('DirectoryBrowser', () => {
|
|||||||
fireEvent.keyDown(input, { key: 'Enter' })
|
fireEvent.keyDown(input, { key: 'Enter' })
|
||||||
await waitFor(() => { expect(screen.getByRole('listitem').textContent).toBe('harness') })
|
await waitFor(() => { expect(screen.getByRole('listitem').textContent).toBe('harness') })
|
||||||
expect(columns()).toHaveLength(1)
|
expect(columns()).toHaveLength(1)
|
||||||
|
// The submitted navigation unmounted the focused input; focus parks on
|
||||||
|
// the crumb edit zone that replaced it.
|
||||||
|
expect(document.activeElement).toBe(screen.getByRole('button', { name: 'browser.editPath' }))
|
||||||
fireEvent.click(screen.getByRole('button', { name: 'browser.editPath' }))
|
fireEvent.click(screen.getByRole('button', { name: 'browser.editPath' }))
|
||||||
const again = screen.getByLabelText<HTMLInputElement>('browser.editPath')
|
const again = screen.getByLabelText<HTMLInputElement>('browser.editPath')
|
||||||
fireEvent.change(again, { target: { value: ' ' } })
|
fireEvent.change(again, { target: { value: ' ' } })
|
||||||
@@ -234,6 +237,8 @@ describe('DirectoryBrowser', () => {
|
|||||||
expect(b.listDirectory).toHaveBeenCalledTimes(2)
|
expect(b.listDirectory).toHaveBeenCalledTimes(2)
|
||||||
fireEvent.keyDown(again, { key: 'Escape' })
|
fireEvent.keyDown(again, { key: 'Escape' })
|
||||||
expect(screen.queryByLabelText('browser.editPath', { selector: 'input' })).toBeNull()
|
expect(screen.queryByLabelText('browser.editPath', { selector: 'input' })).toBeNull()
|
||||||
|
// Escape with focus in the input parks focus on the returning edit zone.
|
||||||
|
expect(document.activeElement).toBe(screen.getByRole('button', { name: 'browser.editPath' }))
|
||||||
})
|
})
|
||||||
|
|
||||||
it('prefix-filters the listed level from the draft tail, dot revealing hidden matches', async () => {
|
it('prefix-filters the listed level from the draft tail, dot revealing hidden matches', async () => {
|
||||||
@@ -324,9 +329,12 @@ describe('DirectoryBrowser', () => {
|
|||||||
// Tab parked focus on the result row; Escape must still mean "leave
|
// Tab parked focus on the result row; Escape must still mean "leave
|
||||||
// path editing", not "close the whole dialog".
|
// path editing", not "close the whole dialog".
|
||||||
const row = rowButton(screen.getByRole('listitem'))
|
const row = rowButton(screen.getByRole('listitem'))
|
||||||
|
row.focus()
|
||||||
fireEvent.keyDown(row, { key: 'Escape' })
|
fireEvent.keyDown(row, { key: 'Escape' })
|
||||||
expect(screen.queryByLabelText('browser.editPath', { selector: 'input' })).toBeNull()
|
expect(screen.queryByLabelText('browser.editPath', { selector: 'input' })).toBeNull()
|
||||||
expect(b.onClose).not.toHaveBeenCalled()
|
expect(b.onClose).not.toHaveBeenCalled()
|
||||||
|
// Focus was already on a surviving row, so nothing re-parks it.
|
||||||
|
expect(document.activeElement).toBe(row)
|
||||||
// With no draft left, Escape falls through to the Modal and closes.
|
// With no draft left, Escape falls through to the Modal and closes.
|
||||||
fireEvent.keyDown(row, { key: 'Escape' })
|
fireEvent.keyDown(row, { key: 'Escape' })
|
||||||
expect(b.onClose).toHaveBeenCalledTimes(1)
|
expect(b.onClose).toHaveBeenCalledTimes(1)
|
||||||
|
|||||||
Reference in New Issue
Block a user