feat(ui): add workspace-root navigation to directory browser - #724
bluelovers wants to merge 4 commits into
Conversation
Add a "Workspace Root" button (Home icon) to the directory browser dialog that navigates the view back to the configured workspace root (FileSystemListingMetadata.rootPath). Implementation: - The button is its own grid cell placed to the right of the "Select folder or enter path" label, fully separated from the New Folder and Open actions. It uses a new transparent `selector-button-ghost` variant so it reads as a navigation tool rather than a filled action button. - Clicking it calls navigateTo(rootPath()); it is disabled when there is no root path, when the user is already at the root, or while a folder is being created. - A `canGoToWorkspaceRoot` memo drives the disabled state. i18n: - Add `directoryBrowser.goToWorkspaceRoot` in en plus de, es, fr, he, ja, ne, ru, tr and zh-Hans so the tooltip/aria-label are localized. Layout details: - `.directory-browser-current` now uses a four-column grid; the flexible middle column pushes New Folder to the far right and keeps the root button adjacent to the label. Responsive rules stack the controls at <=640px and <=380px. Edge cases: - In files mode the New Folder action is hidden; the Workspace Root button remains available so users can still jump back to the root. For the Windows drives pseudo-view rootPath is empty, so the button is disabled. Validation: - No typecheck run (npx disallowed in this environment); manual browser verification of spacing/divider and locale labels is recommended.
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/35502137445 Artifacts expire in 7 days.
|
pascalandr
left a comment
pascalandr
left a comment
There was a problem hiding this comment.
The convenience is understandable, but the destination needs an explicit product contract before this shared control is added.
FileSystemListingMetadata.rootPath is the server browsing/authorization origin, not necessarily the dialog's starting folder or the active workspace. Electron and Tauri launch the server with --unrestricted-root and no explicit --workspace-root, so this value defaults to the server process working directory. The shared dialog is used for workspace selection, clone destinations, OpenCode binaries, and file attachments, each with different initialPath semantics. A Home icon labelled “Workspace Root” therefore has no stable meaning across callers.
Please first choose the intended destination and model it explicitly: capture the canonical initial location for “back to start”, pass an active workspace target from callers, use homePath for filesystem home, or clearly expose the configured browsing root. The editable path field does not provide history. CodeNomad currently persists recent opened project folders, but has no generic directory history or favourites; those should not be conflated. A per-dialog start shortcut or back/forward history can stay local, while persistent favourites deserve a separate scoped design.
There are also two confirmed implementation regressions in the inline comments. Please add a real browser regression covering directory and file modes at desktop and narrow widths, plus the edited-path navigation case.
| type="button" | ||
| class="selector-button selector-button-ghost directory-browser-go-root" | ||
| disabled={!canGoToWorkspaceRoot()} | ||
| onClick={() => void navigateTo(rootPath())} |
There was a problem hiding this comment.
This bypasses handleNavigateTo(), which deliberately clears pathInputDirty before changing directories. Reproduction: navigate below the root, edit the path field without submitting it, then click this button. The listing moves to the root but the field keeps the stale edited path; the next Open or New Folder action operates on that stale target. Route this action through the shared navigation handler (or otherwise clear/synchronize the edit state) and cover the sequence with a regression test.
| flex-shrink: 0; | ||
| } | ||
|
|
||
| .directory-browser-open-path { |
There was a problem hiding this comment.
The grid templates still reserve an open area, but this rule no longer assigns grid-area: open. Chromium therefore auto-places Open into the desktop spacer on the label row; below 640px it occupies only half the intended row, and in files mode it lands beside the root button. Restore the explicit area assignment. Please cover both dialog modes and responsive layouts so this cannot regress silently.
|
PR builds are available as GitHub Actions artifacts: https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/35512118379 Artifacts expire in 7 days.
|
Address the directory-browser review on PR NeuralNomadsAI#724. Replace the single "Workspace Root" button with conditional, de-duplicated navigation shortcuts driven by FileSystemListingMetadata: - workspace-root shortcut only when scope is "restricted" (rootPath is the configured browsing root), - user-home shortcut only when scope is "unrestricted" (uses homePath), - initial-path shortcut when initialPath is provided. Identical absolute targets collapse into a single button. Fix Regression A: shortcut clicks clear the edited-path state (setPathInputDirty(false)) before navigating, so a stale manually-edited path field no longer leaks into the next Open / New Folder action. Fix Regression B: restore grid-area: open on .directory-browser-open-path so the Open button is explicitly placed instead of being auto-placed into the label-row spacer at desktop widths and below 640px (and beside the root button in files mode). Add a browser regression test (directory-browser.test.ts + fixture) covering restricted/unrestricted scopes, dedupe, files mode, narrow widths, and the edited-path navigation case. Add i18n keys directoryBrowser.goToHome and directoryBrowser.goToInitial across all 10 locales.
Rename the workspace-root shortcut to a "Default Start Directory" shortcut (i18n key directoryBrowser.goToRoot) so the label reflects the resolved root semantics (CLI_WORKSPACE_ROOT -> --root -> cwd) instead of a parameter name. Show the start-directory shortcut whenever rootPath is present in BOTH restricted and unrestricted modes. This matches issue NeuralNomadsAI#370, where CLI_WORKSPACE_ROOT is the intended default path even under --unrestricted-root, so the server's resolved root is a valid jump target regardless of scope. Keep the home shortcut limited to unrestricted scope (its target is outside the authorized root in restricted mode) and dedupe it against rootPath. The initial-path shortcut is unchanged. All three shortcuts are still de-duplicated by their canonical absolute path. Update i18n keys/labels across all 10 locales and adjust the browser regression test: unrestricted scenarios now yield root + home + initial (3 shortcuts), and the home shortcut is selected by index.
pascalandr
left a comment
pascalandr
left a comment
There was a problem hiding this comment.
The destination contract is now much clearer, and the two production regressions from the previous review are corrected: shortcut navigation clears the edited-path state and grid-area: open is restored.
This HEAD is still not merge-ready. The new browser suite never mounts the dialog and all five tests fail in required CI; after fixing the fixture, two assertions are themselves guaranteed to fail. There is also a user-visible mismatch in how a relative initial path is resolved in unrestricted mode, plus a broken shortcut after initial-path fallback. Please address the inline findings and rerun the browser suite. No merge performed.
| let navigations: string[] = [] | ||
| const [open, setOpen] = createSignal(true) | ||
|
|
||
| render( |
There was a problem hiding this comment.
DirectoryBrowserDialog calls useI18n() immediately, so rendering it without I18nProvider throws before the dialog mounts; I18nProvider in turn requires ConfigProvider. This is why all five new tests time out waiting for .directory-browser-current-path in required CI instead of exercising any assertions. Wrap this fixture in the same ConfigProvider + I18nProvider stack used by the other real-component fixtures (and preferably surface pageerror so fixture startup failures fail fast).
| } | ||
| const initial = props.initialPath?.trim() | ||
| if (initial) { | ||
| const target = isAbsolutePathLike(initial) ? initial : resolveAbsolutePath(meta.rootPath, initial) |
There was a problem hiding this comment.
This does not preserve the server's initialPath semantics. initialize() sends a relative initial path unchanged, and unrestricted FileSystemBrowser.resolveUnrestrictedPath() resolves it against homePath; this line instead rewrites it under rootPath. With root /srv/start, home /home/user, and initial projects, the dialog opens /home/user/projects but Initial Path jumps to /srv/start/projects. The button also remains after an invalid/missing initial path made initialize() fall back to the default root, leaving a permanently failing shortcut. Capture the canonical absolute path only from a successful initial navigation and use that for the target/deduplication; omit the shortcut after fallback.
| // initialPath equals rootPath, so the workspace + initial shortcuts collapse into one. | ||
| await openFixture(page, scenario, "initialPath=/ws&mode=directories") | ||
| assert.equal(await shortcutCount(page), 1) | ||
| await page.locator(".directory-browser-shortcut").first().click() |
There was a problem hiding this comment.
Once the fixture mounts, this click still cannot succeed: the initial path is /ws, the sole deduplicated shortcut also targets /ws, and atTarget() therefore disables the button. Playwright will wait for it to become enabled and time out. Assert that the deduplicated button is disabled here, or initialize below the root before testing navigation.
| assert.equal(await page.locator(".directory-browser-open-path").count(), 1) | ||
| // Open button should span the full row at this width, not half. | ||
| const box = await page.locator(".directory-browser-open-path").boundingBox() | ||
| assert.ok(box !== null && box.width >= 300, `open button width ${box?.width} should fill the row`) |
There was a problem hiding this comment.
At a 360 px viewport the modal is at most 90vw and the body has 48 px horizontal padding, so the Open button cannot be 300 px wide; this assertion will fail after the fixture is repaired. Compare its width/edges with the grid content box instead of a hard-coded viewport threshold. Also, 360 px exercises the separate <=380px template, not the 381–640px two-column breakpoint where the original half-row/file-mode regression occurred; please cover that breakpoint in both directory and file modes as requested.
2 participants
Summary
Adds a "Workspace Root" button to the directory browser dialog that navigates the view back to the configured workspace root (
FileSystemListingMetadata.rootPath).What changed
navigateTo(rootPath())to jump back to the workspace root. It is disabled when there is no root path, when the user is already at the root, or while a folder is being created (driven by a newcanGoToWorkspaceRootmemo).New FolderandOpenactions. It uses a new transparentselector-button-ghostvariant so it reads as a navigation tool rather than a filled action button.directoryBrowser.goToWorkspaceRootforenplusde,es,fr,he,ja,ne,ru,tr, andzh-Hans(used for tooltip/aria-label)..directory-browser-currentnow uses a four-column grid; the flexible middle column pushesNew Folderto the far right and keeps the root button next to the label. Responsive rules stack the controls at<=640pxand<=380px.Edge cases
New Folderaction is hidden, but the Workspace Root button stays available so users can still jump back to the root.rootPathis empty, so the button is disabled.Validation
npxis disallowed here); manual browser verification of spacing and the localized label is recommended.🤖 Generated with CodeNomad