fix(host): review round 5 — draft-pending action gating, in-flow errors, IME guards
- Open and New folder disable while a path draft is uncommitted: targetPath still names the previous selection/listing, and committing against it while a different path shows in the header adopts the wrong directory. - The Miller columns keep their own row so a status/error line renders below them inside the card instead of competing as a third flex item the dialog clips off-screen. - Both text inputs (path editor, folder name) carry the IME composition guard the workspace-name inputs already had: a composing Enter confirms the candidate, never submits.
This commit is contained in:
@@ -41,6 +41,16 @@
|
|||||||
|
|
||||||
/* Deep chains scroll inside the trail (the effect pins the tail into view)
|
/* Deep chains scroll inside the trail (the effect pins the tail into view)
|
||||||
* so the edit zone to the right never leaves the bar. */
|
* so the edit zone to the right never leaves the bar. */
|
||||||
|
/* The Miller columns keep their own row so a status/error line below never
|
||||||
|
* competes with the fixed column widths for horizontal space. */
|
||||||
|
.millerRow {
|
||||||
|
display: flex;
|
||||||
|
align-items: stretch;
|
||||||
|
flex: 1 1 0;
|
||||||
|
min-height: 0;
|
||||||
|
gap: 20px;
|
||||||
|
}
|
||||||
|
|
||||||
.crumbTrail {
|
.crumbTrail {
|
||||||
display: flex;
|
display: flex;
|
||||||
align-items: center;
|
align-items: center;
|
||||||
@@ -113,10 +123,9 @@
|
|||||||
* the hairline divider centered between them; each column scrolls alone. */
|
* the hairline divider centered between them; each column scrolls alone. */
|
||||||
.content {
|
.content {
|
||||||
display: flex;
|
display: flex;
|
||||||
align-items: stretch;
|
flex-direction: column;
|
||||||
flex: 1 1 0;
|
flex: 1 1 0;
|
||||||
min-height: 0;
|
min-height: 0;
|
||||||
gap: 20px;
|
|
||||||
padding: 16px 24px 0;
|
padding: 16px 24px 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -120,6 +120,9 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen,
|
|||||||
// Deep ancestry overflows the trail; keep its tail (the current directory
|
// Deep ancestry overflows the trail; keep its tail (the current directory
|
||||||
// and the edit zone beside it) in view whenever the chain changes.
|
// and the edit zone beside it) in view whenever the chain changes.
|
||||||
const crumbTrailRef = useRef<HTMLSpanElement | null>(null)
|
const crumbTrailRef = useRef<HTMLSpanElement | null>(null)
|
||||||
|
// IME confirmation (Enter selecting a candidate) must not submit either
|
||||||
|
// text input; the same guard the workspace-name inputs carry.
|
||||||
|
const composingRef = useRef(false)
|
||||||
|
|
||||||
/** Replace the whole view with one freshly listed level (no selection). */
|
/** Replace the whole view with one freshly listed level (no selection). */
|
||||||
const navigate = useCallback((path?: string) => {
|
const navigate = useCallback((path?: string) => {
|
||||||
@@ -242,6 +245,10 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen,
|
|||||||
// focus trap, so every parent control goes inert (Shift-Tab or AT must not
|
// focus trap, so every parent control goes inert (Shift-Tab or AT must not
|
||||||
// close, adopt, or retarget underneath the child).
|
// close, adopt, or retarget underneath the child).
|
||||||
const parentInert = busy || folderDraft !== null
|
const parentInert = busy || folderDraft !== null
|
||||||
|
// An uncommitted path draft makes targetPath stale relative to the header:
|
||||||
|
// committing actions must not act on the previous selection/listing while
|
||||||
|
// a different path is displayed.
|
||||||
|
const draftPending = pathDraft !== null
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<Modal
|
<Modal
|
||||||
@@ -298,8 +305,10 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen,
|
|||||||
autoFocus
|
autoFocus
|
||||||
disabled={parentInert}
|
disabled={parentInert}
|
||||||
onChange={(event) => { setPathDraft(event.target.value) }}
|
onChange={(event) => { setPathDraft(event.target.value) }}
|
||||||
|
onCompositionStart={() => { composingRef.current = true }}
|
||||||
|
onCompositionEnd={() => { composingRef.current = false }}
|
||||||
onKeyDown={(event) => {
|
onKeyDown={(event) => {
|
||||||
if (event.key === 'Enter') {
|
if (event.key === 'Enter' && !composingRef.current) {
|
||||||
event.preventDefault()
|
event.preventDefault()
|
||||||
const target = pathDraft.trim()
|
const target = pathDraft.trim()
|
||||||
if (target !== '') navigate(target)
|
if (target !== '') navigate(target)
|
||||||
@@ -315,25 +324,27 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen,
|
|||||||
</div>
|
</div>
|
||||||
</div>
|
</div>
|
||||||
<div className={css.content}>
|
<div className={css.content}>
|
||||||
{parent !== null && (
|
<div className={css.millerRow}>
|
||||||
<LevelColumn
|
{parent !== null && (
|
||||||
entries={parent.entries}
|
<LevelColumn
|
||||||
selectedPath={selected?.path ?? null}
|
entries={parent.entries}
|
||||||
busy={parentInert}
|
selectedPath={selected?.path ?? null}
|
||||||
onPick={select}
|
busy={parentInert}
|
||||||
wide={!twoPane}
|
onPick={select}
|
||||||
/>
|
wide={!twoPane}
|
||||||
)}
|
/>
|
||||||
{twoPane && <span className={css.divider} />}
|
)}
|
||||||
{twoPane && child !== null && (
|
{twoPane && <span className={css.divider} />}
|
||||||
<LevelColumn
|
{twoPane && child !== null && (
|
||||||
entries={child.entries}
|
<LevelColumn
|
||||||
selectedPath={null}
|
entries={child.entries}
|
||||||
busy={parentInert}
|
selectedPath={null}
|
||||||
onPick={advance}
|
busy={parentInert}
|
||||||
wide={false}
|
onPick={advance}
|
||||||
/>
|
wide={false}
|
||||||
)}
|
/>
|
||||||
|
)}
|
||||||
|
</div>
|
||||||
{loading && <div className={css.status} role="status">{t('browser.loading')}</div>}
|
{loading && <div className={css.status} role="status">{t('browser.loading')}</div>}
|
||||||
{error !== null && <div className={css.error} role="alert">{error}</div>}
|
{error !== null && <div className={css.error} role="alert">{error}</div>}
|
||||||
</div>
|
</div>
|
||||||
@@ -341,7 +352,7 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen,
|
|||||||
<Button
|
<Button
|
||||||
variant="outline"
|
variant="outline"
|
||||||
icon={<IconPlusOutline16 size={14} />}
|
icon={<IconPlusOutline16 size={14} />}
|
||||||
disabled={parent === null || loading || parentInert}
|
disabled={parent === null || loading || parentInert || draftPending}
|
||||||
onClick={() => {
|
onClick={() => {
|
||||||
setFolderDraft('')
|
setFolderDraft('')
|
||||||
setCreateError(null)
|
setCreateError(null)
|
||||||
@@ -354,7 +365,7 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen,
|
|||||||
<Button
|
<Button
|
||||||
variant="primary"
|
variant="primary"
|
||||||
className={clsx(css.footerAction)}
|
className={clsx(css.footerAction)}
|
||||||
disabled={targetPath === null || loading || parentInert}
|
disabled={targetPath === null || loading || parentInert || draftPending}
|
||||||
/* v8 ignore next -- narrowing guard: Open disables while no target exists. */
|
/* v8 ignore next -- narrowing guard: Open disables while no target exists. */
|
||||||
onClick={() => { if (targetPath !== null) onOpen(targetPath) }}
|
onClick={() => { if (targetPath !== null) onOpen(targetPath) }}
|
||||||
>
|
>
|
||||||
@@ -380,8 +391,10 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen,
|
|||||||
autoFocus
|
autoFocus
|
||||||
disabled={creatingFolder}
|
disabled={creatingFolder}
|
||||||
onChange={(event) => { setFolderDraft(event.target.value) }}
|
onChange={(event) => { setFolderDraft(event.target.value) }}
|
||||||
|
onCompositionStart={() => { composingRef.current = true }}
|
||||||
|
onCompositionEnd={() => { composingRef.current = false }}
|
||||||
onKeyDown={(event) => {
|
onKeyDown={(event) => {
|
||||||
if (event.key === 'Enter') {
|
if (event.key === 'Enter' && !composingRef.current) {
|
||||||
event.preventDefault()
|
event.preventDefault()
|
||||||
confirmCreate()
|
confirmCreate()
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -284,6 +284,59 @@ describe('DirectoryBrowser', () => {
|
|||||||
await waitFor(() => { expect(screen.getByText('harness')).toBeTruthy() })
|
await waitFor(() => { expect(screen.getByText('harness')).toBeTruthy() })
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it('disables Open and New folder while a path draft is uncommitted', async () => {
|
||||||
|
mount()
|
||||||
|
await waitFor(() => { expect(screen.getByRole('listitem')).toBeTruthy() })
|
||||||
|
fireEvent.click(screen.getByRole('button', { name: 'browser.editPath' }))
|
||||||
|
// targetPath still names the previous listing; committing actions must
|
||||||
|
// not act on it while a different path is displayed in the header.
|
||||||
|
expect(screen.getByRole<HTMLButtonElement>('button', { name: 'browser.open' }).disabled).toBe(true)
|
||||||
|
expect(screen.getByRole<HTMLButtonElement>('button', { name: 'browser.newFolder' }).disabled).toBe(true)
|
||||||
|
fireEvent.keyDown(screen.getByLabelText('browser.editPath'), { key: 'Escape' })
|
||||||
|
expect(screen.getByRole<HTMLButtonElement>('button', { name: 'browser.open' }).disabled).toBe(false)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('ignores Enter while an IME composition is active in either input', async () => {
|
||||||
|
const b = mount()
|
||||||
|
await waitFor(() => { expect(screen.getByRole('listitem')).toBeTruthy() })
|
||||||
|
// Path editor: a composing Enter confirms the candidate, not the path.
|
||||||
|
fireEvent.click(screen.getByRole('button', { name: 'browser.editPath' }))
|
||||||
|
const pathInput = screen.getByLabelText('browser.editPath')
|
||||||
|
fireEvent.change(pathInput, { target: { value: DOCS } })
|
||||||
|
const listCalls = b.listDirectory.mock.calls.length
|
||||||
|
fireEvent.compositionStart(pathInput)
|
||||||
|
fireEvent.keyDown(pathInput, { key: 'Enter' })
|
||||||
|
expect(b.listDirectory.mock.calls.length).toBe(listCalls)
|
||||||
|
fireEvent.compositionEnd(pathInput)
|
||||||
|
fireEvent.keyDown(pathInput, { key: 'Enter' })
|
||||||
|
await waitFor(() => { expect(b.listDirectory).toHaveBeenLastCalledWith(DOCS) })
|
||||||
|
// Create dialog: same guard.
|
||||||
|
fireEvent.click(screen.getByRole('button', { name: 'browser.newFolder' }))
|
||||||
|
const nameInput = screen.getByLabelText('browser.folderName')
|
||||||
|
fireEvent.change(nameInput, { target: { value: '新建' } })
|
||||||
|
fireEvent.compositionStart(nameInput)
|
||||||
|
fireEvent.keyDown(nameInput, { key: 'Enter' })
|
||||||
|
expect(b.createDirectory).not.toHaveBeenCalled()
|
||||||
|
fireEvent.compositionEnd(nameInput)
|
||||||
|
fireEvent.keyDown(nameInput, { key: 'Enter' })
|
||||||
|
await waitFor(() => { expect(b.createDirectory).toHaveBeenCalledWith(DOCS, '新建') })
|
||||||
|
})
|
||||||
|
|
||||||
|
it('surfaces a two-pane navigation failure as an alert below the columns', async () => {
|
||||||
|
const b = mount()
|
||||||
|
await waitFor(() => { expect(screen.getByRole('listitem')).toBeTruthy() })
|
||||||
|
fireEvent.click(rowButton(screen.getByRole('listitem')))
|
||||||
|
await waitFor(() => { expect(columns()).toHaveLength(2) })
|
||||||
|
b.listDirectory.mockImplementation(async () => {
|
||||||
|
throw new DirectoryBrowseError({ code: 'directory-unreadable', message: 'denied', details: { path: HOME } })
|
||||||
|
})
|
||||||
|
fireEvent.click(within(screen.getByRole('navigation')).getByRole('button', { name: 'browser.home' }))
|
||||||
|
await waitFor(() => { expect(screen.getByRole('alert').textContent).toBe('denied') })
|
||||||
|
// Both panes survive the failure; the alert renders in the flow, not as a
|
||||||
|
// third column competing for the fixed widths.
|
||||||
|
expect(columns()).toHaveLength(2)
|
||||||
|
})
|
||||||
|
|
||||||
it('ignores dismissal while adoption is busy', async () => {
|
it('ignores dismissal while adoption is busy', async () => {
|
||||||
const b = mount({ busy: true })
|
const b = mount({ busy: true })
|
||||||
await waitFor(() => { expect(screen.getByRole('dialog')).toBeTruthy() })
|
await waitFor(() => { expect(screen.getByRole('dialog')).toBeTruthy() })
|
||||||
|
|||||||
Reference in New Issue
Block a user