mirror of
https://github.com/deepseek-ai/deepseek-harness
synced 2026-08-15 21:04:50 +00:00
fix(host): review round 2 — dialog-scoped blur cancel, editing-only focus hold, selection exempt from filters
This commit is contained in:
@@ -55,8 +55,9 @@
|
||||
align-items: stretch;
|
||||
flex: 1 1 0;
|
||||
min-height: 0;
|
||||
/* Columns already end in an 8px scrollbar clearance, so the divider only
|
||||
* needs a slim gap of its own on each side. */
|
||||
/* 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. */
|
||||
gap: 12px;
|
||||
overflow-x: auto;
|
||||
scrollbar-width: none;
|
||||
|
||||
@@ -65,9 +65,12 @@ function displayCrumbs(listing: DirectoryListing, homeLabel: string): DirectoryE
|
||||
}
|
||||
|
||||
/**
|
||||
* The listing's platform separator, read from the host-stamped home path —
|
||||
* never sniffed from typed text or entry paths, where a backslash is a legal
|
||||
* POSIX name character rather than a platform fact.
|
||||
* The listing's platform separator, inferred from the home path the host
|
||||
* stamped — never from typed text or entry paths, where a backslash is a
|
||||
* legal POSIX name character. Still a heuristic at the last step: a POSIX
|
||||
* home directory whose own name contains a backslash would misread.
|
||||
* TODO: replace with a host-stamped `separator` field on the wire
|
||||
* DirectoryListing so the platform fact travels verbatim.
|
||||
*/
|
||||
function separatorOf(listing: DirectoryListing): '\\' | '/' {
|
||||
return listing.home.includes('\\') ? '\\' : '/'
|
||||
@@ -91,15 +94,20 @@ function draftPrefixFor(listing: DirectoryListing, draft: string | null): string
|
||||
}
|
||||
|
||||
/** One column of folder rows (the Miller view renders one or two of these). */
|
||||
function LevelColumn({ entries, selectedPath, busy, onPick, showHidden, filterPrefix }: {
|
||||
function LevelColumn({ entries, selectedPath, busy, onPick, showHidden, filterPrefix, pathEditing }: {
|
||||
entries: readonly DirectoryEntry[]
|
||||
selectedPath: string | null
|
||||
busy: boolean
|
||||
onPick: (entry: DirectoryEntry) => void
|
||||
showHidden: boolean
|
||||
filterPrefix: string | null
|
||||
pathEditing: boolean
|
||||
}) {
|
||||
const visible = entries.filter((entry) => {
|
||||
// The selection is exempt from both filters: it anchors the two-pane
|
||||
// view (crumbs and the child pane point at it), so neither the hidden
|
||||
// filter after a dot-reveal pick nor a prefix miss may orphan it.
|
||||
if (entry.path === selectedPath) return true
|
||||
if (filterPrefix !== null && !entry.name.toLowerCase().startsWith(filterPrefix.toLowerCase())) return false
|
||||
// A dot-led prefix names hidden entries explicitly, so matching ones
|
||||
// surface even while the toggle keeps the rest hidden.
|
||||
@@ -118,10 +126,11 @@ function LevelColumn({ entries, selectedPath, busy, onPick, showHidden, filterPr
|
||||
aria-current={selected || undefined}
|
||||
className={clsx(css.row, selected && css.rowSelected)}
|
||||
disabled={busy}
|
||||
// Keep focus where it is (the path editor, notably): a focus
|
||||
// steal on mousedown would blur-cancel the editor, unmount the
|
||||
// filtered rows mid-gesture, and drop this very click.
|
||||
onMouseDown={(event) => { event.preventDefault() }}
|
||||
// While the path editor is open, keep focus in it: a focus
|
||||
// steal on mousedown would blur the editor and (in engines
|
||||
// where the blur lands before our guards) drop this click.
|
||||
// Outside editing, rows keep native focus behavior.
|
||||
onMouseDown={pathEditing ? (event) => { event.preventDefault() } : undefined}
|
||||
onClick={() => { onPick(entry) }}
|
||||
>
|
||||
{selected
|
||||
@@ -457,16 +466,19 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen,
|
||||
cancelPathEdit()
|
||||
}
|
||||
}}
|
||||
// Clicking anywhere outside the editor reads as leaving it:
|
||||
// focus loss cancels the edit like Escape. Enter keeps focus
|
||||
// Focus leaving the DIALOG reads as leaving the editor and
|
||||
// cancels like Escape. Three guarded non-cancel paths: window
|
||||
// or tab focus loss (document no longer focused); a focus
|
||||
// move that stays inside the dialog card (keyboard Tab onto
|
||||
// the filtered rows or the footer toggle); and pointer paths,
|
||||
// where rows and the toggle suppress focus steal on mousedown
|
||||
// while editing so their click lands first. Enter keeps focus
|
||||
// in the input while its navigation is in flight, so a
|
||||
// submitted path is never withdrawn by this handler; rows and
|
||||
// the show-hidden toggle suppress focus steal on mousedown so
|
||||
// a click on them lands before any cancel. Window/tab focus
|
||||
// loss also fires blur in some engines — only a focus move
|
||||
// within a focused document reads as leaving the editor.
|
||||
onBlur={() => {
|
||||
// submitted path is never withdrawn here.
|
||||
onBlur={(event) => {
|
||||
if (!document.hasFocus()) return
|
||||
if (event.relatedTarget instanceof HTMLElement
|
||||
&& event.relatedTarget.closest('[role="dialog"]') !== null) return
|
||||
cancelPathEdit()
|
||||
}}
|
||||
/>
|
||||
@@ -483,6 +495,7 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen,
|
||||
onPick={select}
|
||||
showHidden={showHidden}
|
||||
filterPrefix={draftPrefixFor(parent, pathDraft)}
|
||||
pathEditing={draftPending}
|
||||
/>
|
||||
)}
|
||||
{twoPane && <span className={css.divider} />}
|
||||
@@ -494,6 +507,7 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen,
|
||||
onPick={advance}
|
||||
showHidden={showHidden}
|
||||
filterPrefix={draftPrefixFor(child, pathDraft)}
|
||||
pathEditing={draftPending}
|
||||
/>
|
||||
)}
|
||||
</div>
|
||||
@@ -523,9 +537,10 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen,
|
||||
aria-pressed={showHidden}
|
||||
disabled={parentInert}
|
||||
// The toggle composes with the path editor (dot-led prefixes and
|
||||
// this filter interleave): don't steal focus, so toggling never
|
||||
// blur-cancels a draft mid-thought.
|
||||
onMouseDown={(event) => { event.preventDefault() }}
|
||||
// this filter interleave): while editing, don't steal focus, so
|
||||
// toggling never blur-cancels a draft mid-thought. Outside editing
|
||||
// it keeps native focus behavior.
|
||||
onMouseDown={draftPending ? (event) => { event.preventDefault() } : undefined}
|
||||
onClick={() => { setShowHidden(prev => !prev) }}
|
||||
>
|
||||
{showHidden && <IconCheckOutline16 size={14} />}
|
||||
|
||||
@@ -29,6 +29,18 @@ function listingFor(path?: string): DirectoryListing {
|
||||
],
|
||||
truncated: false,
|
||||
},
|
||||
[`${HOME}/.config`]: {
|
||||
path: `${HOME}/.config`,
|
||||
home: HOME,
|
||||
crumbs: [
|
||||
{ name: '/', path: '/', hidden: false },
|
||||
{ name: 'home', path: '/home', hidden: false },
|
||||
{ name: 'u', path: HOME, hidden: false },
|
||||
{ name: '.config', path: `${HOME}/.config`, hidden: true },
|
||||
],
|
||||
entries: [],
|
||||
truncated: false,
|
||||
},
|
||||
[DOCS]: {
|
||||
path: DOCS,
|
||||
home: HOME,
|
||||
@@ -261,23 +273,58 @@ describe('DirectoryBrowser', () => {
|
||||
expect(within(columns()[1]!).queryAllByRole('listitem')).toHaveLength(0)
|
||||
expect(within(columns()[0]!).getByText('Documents')).toBeTruthy()
|
||||
// Erasing back into the parent's own path moves the filter to the LEFT
|
||||
// pane and releases the right one.
|
||||
// pane and releases the right one. The selected row is exempt (it
|
||||
// anchors the two-pane view), so it alone survives the miss.
|
||||
fireEvent.change(input, { target: { value: `${HOME}/zz` } })
|
||||
expect(within(columns()[0]!).queryAllByRole('listitem')).toHaveLength(0)
|
||||
expect(within(columns()[0]!).getAllByRole('listitem').map(item => item.textContent)).toEqual(['Documents'])
|
||||
expect(within(columns()[1]!).getByText('harness')).toBeTruthy()
|
||||
})
|
||||
|
||||
it('keeps the path editor open when blur comes from window focus loss', async () => {
|
||||
it('keeps the draft and filter through window focus loss and in-dialog focus moves', async () => {
|
||||
mount()
|
||||
await waitFor(() => { expect(screen.getByRole('listitem')).toBeTruthy() })
|
||||
fireEvent.click(screen.getByRole('button', { name: 'browser.editPath' }))
|
||||
const input = screen.getByLabelText<HTMLInputElement>('browser.editPath')
|
||||
fireEvent.change(input, { target: { value: `${HOME}/do` } })
|
||||
// A blur while the document itself lost focus (window switch, dev-tools
|
||||
// focus) must not discard the draft.
|
||||
// focus) must not discard the draft: value and filter both survive.
|
||||
const hasFocus = vi.spyOn(document, 'hasFocus').mockReturnValue(false)
|
||||
fireEvent.blur(input)
|
||||
expect(screen.getByLabelText('browser.editPath', { selector: 'input' })).toBeTruthy()
|
||||
hasFocus.mockRestore()
|
||||
expect(screen.getByLabelText<HTMLInputElement>('browser.editPath', { selector: 'input' }).value).toBe(`${HOME}/do`)
|
||||
expect(screen.getByRole('listitem').textContent).toBe('Documents')
|
||||
// A keyboard focus move that stays inside the dialog (Tab onto the
|
||||
// filtered row) keeps the draft too — the results stay reachable.
|
||||
fireEvent.blur(input, { relatedTarget: rowButton(screen.getByRole('listitem')) })
|
||||
expect(screen.getByLabelText<HTMLInputElement>('browser.editPath', { selector: 'input' }).value).toBe(`${HOME}/do`)
|
||||
// Toggling show-hidden mid-edit suppresses focus steal: the draft and
|
||||
// its filter survive the toggle in both directions.
|
||||
const toggle = screen.getByRole('button', { name: 'browser.showHidden' })
|
||||
fireEvent.mouseDown(toggle)
|
||||
fireEvent.click(toggle)
|
||||
expect(toggle.getAttribute('aria-pressed')).toBe('true')
|
||||
expect(screen.getByLabelText<HTMLInputElement>('browser.editPath', { selector: 'input' }).value).toBe(`${HOME}/do`)
|
||||
expect(screen.getByRole('listitem').textContent).toBe('Documents')
|
||||
// Focus landing outside the dialog cancels like Escape.
|
||||
fireEvent.blur(input, { relatedTarget: document.body })
|
||||
expect(screen.queryByLabelText('browser.editPath', { selector: 'input' })).toBeNull()
|
||||
})
|
||||
|
||||
it('a picked dot-revealed hidden row stays visible as the selection', async () => {
|
||||
mount()
|
||||
await waitFor(() => { expect(screen.getByRole('listitem')).toBeTruthy() })
|
||||
fireEvent.click(screen.getByRole('button', { name: 'browser.editPath' }))
|
||||
const input = screen.getByLabelText<HTMLInputElement>('browser.editPath')
|
||||
fireEvent.change(input, { target: { value: `${HOME}/.co` } })
|
||||
const row = rowButton(screen.getByRole('listitem'))
|
||||
expect(row.textContent).toBe('.config')
|
||||
fireEvent.mouseDown(row)
|
||||
fireEvent.click(row)
|
||||
// The pick cleared the draft (and with it the dot-reveal), but the
|
||||
// selection is exempt from the hidden filter: the anchor row survives.
|
||||
expect(screen.queryByLabelText('browser.editPath', { selector: 'input' })).toBeNull()
|
||||
await waitFor(() => { expect(columns()).toHaveLength(2) })
|
||||
expect(within(columns()[0]!).getByText('.config')).toBeTruthy()
|
||||
})
|
||||
|
||||
it('picking a filtered row adopts it and closes the path editor', async () => {
|
||||
|
||||
Reference in New Issue
Block a user