From 9a96aa2755c1b7d25be45ad87ebdfd6eb3367332 Mon Sep 17 00:00:00 2001 From: creatixchu Date: Thu, 30 Jul 2026 00:13:09 +0800 Subject: [PATCH] =?UTF-8?q?fix(host):=20review=20round=2014=20=E2=80=94=20?= =?UTF-8?q?shared=20hanging-lister=20fake;=20create-supersession=20and=20c?= =?UTF-8?q?losing-commit=20contracts=20documented?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/client/DirectoryBrowser.tsx | 11 +++-- .../tests/directory-browser.spec.tsx | 45 +++++++++---------- 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/packages/host/directory-picker-browse/src/client/DirectoryBrowser.tsx b/packages/host/directory-picker-browse/src/client/DirectoryBrowser.tsx index e27a72a8e5..4c67d74722 100644 --- a/packages/host/directory-picker-browse/src/client/DirectoryBrowser.tsx +++ b/packages/host/directory-picker-browse/src/client/DirectoryBrowser.tsx @@ -12,7 +12,10 @@ * panes away from the display root. Selecting in the * right column shifts the view one level deeper. "New folder" opens a nested * create dialog targeting the selected folder (or the level itself) and - * selects the created folder. Open adopts the selected folder, falling back + * selects the created folder — unless a newer pick or crumb jump supersedes + * the post-create relist, in which case neither the level nor the selection + * refreshes (see closeCreateDialog's two-stage parking for the matching + * focus story). Open adopts the selected folder, falling back * to the listed level. Pure consumer of the injected browse calls — the * owning flow decides what "Open" means and owns the workspace-creation * error surface. Hidden entries are host-flagged and hidden by default; the @@ -275,8 +278,10 @@ export function DirectoryBrowser({ open, listDirectory, createDirectory, onOpen, /** * 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). + * only: it never gates the landing itself — a torn-down ref in a close + * race merely skips the parking, and committing the landing into a + * closing dialog is safe (the component already renders null, and the + * open effect resets parent/selected/child on the next open). * @returns true when `document.activeElement` is inside the miller row. */ const focusInMillerRows = useCallback((): boolean => { diff --git a/packages/host/directory-picker-browse/tests/directory-browser.spec.tsx b/packages/host/directory-picker-browse/tests/directory-browser.spec.tsx index 03db2e0b8b..c27522a6cf 100644 --- a/packages/host/directory-picker-browse/tests/directory-browser.spec.tsx +++ b/packages/host/directory-picker-browse/tests/directory-browser.spec.tsx @@ -100,6 +100,22 @@ function mount(overrides: Partial[0]> = {}) return { view, props, listDirectory, createDirectory, onOpen, onClose } } +/** + * A listDirectory fake whose explicit HOME listings hang for manual + * settlement — the initial open lists home through the absent-path form, + * so only relists and parent legs are held. + */ +function hangingHomeLister() { + const settlers: { resolve: (value: DirectoryListing) => void; reject: (reason: unknown) => void }[] = [] + const listDirectory = vi.fn(async (path?: string) => { + if (path === HOME) { + return new Promise((resolve, reject) => { settlers.push({ resolve, reject }) }) + } + return listingFor(path) + }) + return { listDirectory, settlers } +} + /** The rendered level columns, left-to-right. */ function columns(): HTMLElement[] { return screen.getAllByRole('list') @@ -270,15 +286,7 @@ describe('DirectoryBrowser', () => { }) 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((resolve, reject) => { settlers.push({ resolve, reject }) }) - } - return listingFor(path) - }) + const { listDirectory, settlers } = hangingHomeLister() mount({ listDirectory }) await waitFor(() => { expect(screen.getByRole('listitem')).toBeTruthy() }) fireEvent.click(screen.getByRole('button', { name: 'browser.newFolder' })) @@ -291,18 +299,11 @@ describe('DirectoryBrowser', () => { 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((resolve, reject) => { settlers.push({ resolve, reject }) }) - } - return listingFor(path) - }) + const { listDirectory, settlers } = hangingHomeLister() mount({ listDirectory }) await waitFor(() => { expect(screen.getByRole('listitem')).toBeTruthy() }) fireEvent.click(screen.getByRole('button', { name: 'browser.newFolder' })) @@ -395,13 +396,7 @@ describe('DirectoryBrowser', () => { }) it('re-parks focus on the re-selected row when the upgrade displaces focused rows', async () => { - const settlers: ((value: DirectoryListing) => void)[] = [] - const listDirectory = vi.fn(async (path?: string) => { - if (path === HOME) { - return new Promise((resolve) => { settlers.push(resolve) }) - } - return listingFor(path) - }) + const { listDirectory, settlers } = hangingHomeLister() mount({ listDirectory }) await waitFor(() => { expect(screen.getByRole('listitem')).toBeTruthy() }) fireEvent.click(screen.getByRole('button', { name: 'browser.editPath' })) @@ -414,7 +409,7 @@ describe('DirectoryBrowser', () => { await waitFor(() => { expect(settlers).toHaveLength(1) }) // The upgrade replaces every committed row node; focus re-parks on the // re-selected row instead of falling to body. - await act(async () => { settlers[0]!(listingFor(HOME)) }) + await act(async () => { settlers[0]!.resolve(listingFor(HOME)) }) await waitFor(() => { expect(columns()).toHaveLength(2) }) expect(document.activeElement?.textContent).toBe('Documents') expect(document.activeElement?.getAttribute('aria-current')).toBe('true')