fix(host): review round 13 — probe never gates a landing; relist supersession covered, false ignores removed

This commit is contained in:
creatixchu
2026-07-29 23:56:21 +08:00
parent 1b89267d23
commit 413321e261
3 changed files with 74 additions and 19 deletions

View File

@@ -128,8 +128,8 @@
flex-direction: column;
flex: 1 1 0;
min-height: 0;
/* Right inset is slimmer than the left: the trailing column's own 8px
* scrollbar clearance makes up the optical difference. */
/* Right inset is slimmer than the left: the trailing column's own
* scrollbar clearance (see .column) makes up the optical difference. */
padding: 16px 16px 16px 24px;
}
@@ -144,8 +144,8 @@
flex: 1 1 0;
min-height: 0;
/* 12px of row gap on each side of the divider; the left side reads wider
* by the column's trailing 8px scrollbar clearance, which is deliberate —
* the thumb needs that room, the right pane's rows do not. */
* by the column's trailing scrollbar clearance (see .column) — the thumb
* needs that room, the right pane's rows do not. */
gap: 12px;
overflow-x: auto;
scrollbar-width: none;

View File

@@ -272,6 +272,20 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen,
const refocusEditZone = useRef(false)
const editZoneRef = useRef<HTMLButtonElement | null>(null)
/**
* Whether the focused element sits among the miller rows — probed before
* a landing replaces the row nodes, to decide focus parking. A probe
* only: it never gates the landing itself (a torn-down ref in a close
* race merely skips the parking).
* @returns true when `document.activeElement` is inside the miller row.
*/
const focusInMillerRows = useCallback((): boolean => {
const rowHost = millerRowRef.current
/* v8 ignore next -- close-race guard: the commit-to-effect window is not deterministically reproducible. */
if (rowHost === null) return false
return rowHost.contains(document.activeElement)
}, [])
/**
* Launch a follow-up listing under the CURRENT supersession seq: a newer
* intent aborts it like the leg it continues, and it supersedes nothing.
@@ -312,11 +326,10 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen,
// The landing replaces every row key; a slow jump leaves the OLD
// rows tabbable meanwhile (parentInert excludes loading), so focus
// may live among them. With no selection yet the edit zone is the
// park target (body-guarded, like every other exit).
const rowHost = millerRowRef.current
/* v8 ignore next -- close-race guard: the commit-to-effect window is not deterministically reproducible. */
if (rowHost === null) return
if (rowHost.contains(document.activeElement)) refocusEditZone.current = true
// park target (body-guarded, like every other exit). The probe never
// gates the commit below — stranding the dialog in loading over a
// focus check would be far worse than a skipped parking.
if (focusInMillerRows()) refocusEditZone.current = true
setParent(target)
setSelected(null)
setChild(null)
@@ -338,13 +351,7 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen,
// The upgrade replaces every committed row node; if focus lives
// among them (Tab reached the rows during the parent leg), arm the
// refocus effect so it re-parks on the re-selected row.
const rowHost = millerRowRef.current
// A close race can clear the ref before the close effect's
// supersede runs (commit precedes passive effects): drop the
// upgrade, the dialog is going away.
/* v8 ignore next -- close-race guard: the commit-to-effect window is not deterministically reproducible. */
if (rowHost === null) return
if (rowHost.contains(document.activeElement)) refocusPick.current = true
if (focusInMillerRows()) refocusPick.current = true
setParent(parentLevel)
setSelected(match)
setChild(target)
@@ -358,7 +365,7 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen,
setLoading(false)
setError(failureText(reason))
})
}, [launchListing, continueScan])
}, [launchListing, continueScan, focusInMillerRows])
/**
* Close the nested create dialog. Its unmount drops focus to body (the
@@ -486,13 +493,14 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen,
const { seq, scan } = launchListing(targetPath)
setLoading(true)
scan.then((level) => {
/* v8 ignore next -- same fence as navigate/select; the modal blocks superseding input */
// The nested dialog closed before this relist launched, so the card
// is interactive meanwhile: a pick or crumb jump supersedes it.
if (seq !== requestSeq.current) return
setParent(level)
setLoading(false)
select({ name, path: createdPath, hidden: false })
}, (reason: unknown) => {
/* v8 ignore next -- same fence as navigate/select; the modal blocks superseding input */
// Same interactive-window fence as the success branch above.
if (seq !== requestSeq.current) return
setLoading(false)
setError(failureText(reason))

View File

@@ -269,6 +269,53 @@ describe('DirectoryBrowser', () => {
expect(document.activeElement).toBe(screen.getByRole('button', { name: 'browser.editPath' }))
})
it('a pick during the post-create relist supersedes it: late settlements drop', async () => {
const settlers: { resolve: (value: DirectoryListing) => void; reject: (reason: unknown) => void }[] = []
const listDirectory = vi.fn(async (path?: string) => {
// Explicit HOME requests are the relists; the initial open lists home
// through the absent-path form.
if (path === HOME) {
return new Promise<DirectoryListing>((resolve, reject) => { settlers.push({ resolve, reject }) })
}
return listingFor(path)
})
mount({ listDirectory })
await waitFor(() => { expect(screen.getByRole('listitem')).toBeTruthy() })
fireEvent.click(screen.getByRole('button', { name: 'browser.newFolder' }))
fireEvent.change(screen.getByLabelText('browser.folderName'), { target: { value: 'ghost' } })
fireEvent.click(screen.getByRole('button', { name: 'browser.create' }))
// The nested dialog is gone while the relist hangs; the card is
// interactive, and picking a row supersedes the relist.
await waitFor(() => { expect(settlers).toHaveLength(1) })
fireEvent.click(rowButton(screen.getByRole('listitem')))
await waitFor(() => { expect(columns()).toHaveLength(2) })
// The stale relist settles late and must not land or select ghost.
await act(async () => { settlers[0]!.resolve({ ...listingFor(HOME), entries: [] }) })
expect(within(columns()[0]!).getByText('Documents')).toBeTruthy()
expect(rowButton(within(columns()[0]!).getByRole('listitem')).getAttribute('aria-current')).toBe('true')
})
it('a rejection of a superseded post-create relist is equally silent', async () => {
const settlers: { resolve: (value: DirectoryListing) => void; reject: (reason: unknown) => void }[] = []
const listDirectory = vi.fn(async (path?: string) => {
if (path === HOME) {
return new Promise<DirectoryListing>((resolve, reject) => { settlers.push({ resolve, reject }) })
}
return listingFor(path)
})
mount({ listDirectory })
await waitFor(() => { expect(screen.getByRole('listitem')).toBeTruthy() })
fireEvent.click(screen.getByRole('button', { name: 'browser.newFolder' }))
fireEvent.change(screen.getByLabelText('browser.folderName'), { target: { value: 'ghost' } })
fireEvent.click(screen.getByRole('button', { name: 'browser.create' }))
await waitFor(() => { expect(settlers).toHaveLength(1) })
fireEvent.click(rowButton(screen.getByRole('listitem')))
await waitFor(() => { expect(columns()).toHaveLength(2) })
await act(async () => { settlers[0]!.reject(new Error('late')) })
expect(screen.queryByRole('alert')).toBeNull()
expect(columns()).toHaveLength(2)
})
it('a failed create relist parks focus on the edit zone with the error shown', async () => {
let relists = 0
const listDirectory = vi.fn(async (path?: string) => {