From b0188d68ea2e93ea83e4999767aa17cd8fcd359d Mon Sep 17 00:00:00 2001 From: Priyanshu Choudhary <57816400+Priyanchew@users.noreply.github.com> Date: Fri, 22 May 2026 18:39:32 +0530 Subject: [PATCH] feat(web): refine add-project folder picker UX - Keyboard nav (arrows/typeahead/Enter) now works when focus is on the modal dialog, not only after clicking into the browser pane - Announce git-repo status to screen readers via folder-row aria-label - Square the drive picker to match the toolbar buttons - Redraw the broken refresh icon as a clean circular arrow - Drop the redundant current-path line below the breadcrumb - Remove the Recent sidebar; reclaim its width for the folder list - Add a selectable "this folder" row so the current directory - and git repos with no subfolders - can be added - Reveal a descend chevron on row hover/selection - Place Project ID and Project Name on one two-column row Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/web/src/app/globals.css | 177 +++++++----------- .../src/components/AddProjectModal.parts.tsx | 41 +--- .../web/src/components/AddProjectModal.tsx | 46 ++--- .../web/src/components/DirectoryBrowser.tsx | 157 +++++++++------- .../__tests__/DirectoryBrowser.test.tsx | 98 +++++----- 5 files changed, 224 insertions(+), 295 deletions(-) diff --git a/packages/web/src/app/globals.css b/packages/web/src/app/globals.css index 890793fd1..24280961b 100644 --- a/packages/web/src/app/globals.css +++ b/packages/web/src/app/globals.css @@ -4736,7 +4736,7 @@ html.light .xterm .xterm-viewport:hover::-webkit-scrollbar-thumb:active { height: 28px; min-width: 76px; border: 1px solid var(--color-border-default); - border-radius: var(--radius-base); + border-radius: 0; background: var(--color-bg-elevated); color: var(--color-text-secondary); font-family: var(--font-mono); @@ -4751,8 +4751,8 @@ html.light .xterm .xterm-viewport:hover::-webkit-scrollbar-thumb:active { } .add-project-modal__drive-select:focus { - outline: 2px solid var(--color-accent); - outline-offset: 2px; + outline: 1px solid var(--color-accent); + outline-offset: -1px; color: var(--color-text-primary); } @@ -4772,88 +4772,9 @@ html.light .xterm .xterm-viewport:hover::-webkit-scrollbar-thumb:active { min-height: 0; height: 100%; display: grid; - grid-template-columns: 176px minmax(0, 1fr); - background: var(--color-bg-surface); - overflow: hidden; -} - -.add-project-browser__sidebar { - min-width: 0; - min-height: 0; - overflow-y: auto; - border-right: 1px solid var(--color-border-subtle); - background: var(--color-bg-sidebar); -} - -.add-project-browser__sidebar-section { - border-bottom: 1px solid var(--color-border-subtle); -} - -.add-project-browser__sidebar-toggle { - width: 100%; - min-height: 34px; - display: flex; - align-items: center; - justify-content: space-between; - gap: 8px; - padding: 0 10px; - background: transparent; - border: 0; - font-family: var(--font-sans); - font-size: 11px; - font-weight: 600; - letter-spacing: 0.06em; - text-transform: uppercase; - color: var(--color-text-tertiary); -} - -.add-project-browser__sidebar-toggle:hover, -.add-project-browser__sidebar-toggle:focus-visible { - color: var(--color-text-primary); - background: var(--color-bg-surface); -} - -.add-project-browser__sidebar-chevron { - color: var(--color-text-tertiary); - transition: transform 160ms ease; -} - -.add-project-browser__sidebar-chevron.is-open { - transform: rotate(180deg); -} - -.add-project-browser__sidebar-body { - display: flex; - flex-direction: column; - padding: 0 6px 8px; -} - -.add-project-browser__sidebar-item { - min-width: 0; - min-height: 30px; - overflow: hidden; - text-overflow: ellipsis; - white-space: nowrap; - border: 0; - background: transparent; - padding: 0 6px; - text-align: left; - font-family: var(--font-mono); - font-size: 11px; - color: var(--color-text-secondary); -} - -.add-project-browser__sidebar-item:hover, -.add-project-browser__sidebar-item:focus-visible { - background: var(--color-bg-surface); - color: var(--color-text-primary); -} - -.add-project-browser__main { - min-width: 0; - min-height: 0; - display: grid; grid-template-rows: auto minmax(0, 1fr); + background: var(--color-bg-surface); + overflow: hidden; } .add-project-browser__current { @@ -4895,14 +4816,6 @@ html.light .xterm .xterm-viewport:hover::-webkit-scrollbar-thumb:active { color: var(--color-text-secondary); } -.add-project-browser__current-path { - margin-top: 4px; - font-family: var(--font-mono); - font-size: 12px; - color: var(--color-text-primary); - word-break: break-all; -} - .add-project-browser__rows { min-height: 0; overflow-y: auto; @@ -4944,15 +4857,54 @@ html.light .xterm .xterm-viewport:hover::-webkit-scrollbar-thumb:active { } .add-project-browser__row-name { + flex: 1; min-width: 0; overflow: hidden; text-overflow: ellipsis; white-space: nowrap; } -.add-project-browser__badge { - margin-left: auto; +.add-project-browser__row-tag { flex: 0 0 auto; + margin-left: 8px; + font-family: var(--font-sans); + font-size: 10px; + font-weight: 600; + letter-spacing: 0.04em; + text-transform: uppercase; + color: var(--color-text-tertiary); +} + +.add-project-browser__row-chevron { + flex: 0 0 auto; + margin-left: 8px; + display: inline-flex; + align-items: center; + color: var(--color-text-tertiary); + opacity: 0; + transition: opacity 120ms ease-out; +} + +.add-project-browser__row:hover .add-project-browser__row-chevron, +.add-project-browser__row.is-selected .add-project-browser__row-chevron { + opacity: 1; +} + +.add-project-browser__row--current { + background: var(--color-bg-elevated); +} + +.add-project-browser__empty-row { + margin: 0; + padding: 12px; + font-family: var(--font-sans); + font-size: 12px; + color: var(--color-text-tertiary); +} + +.add-project-browser__badge { + flex: 0 0 auto; + margin-left: 8px; border: 1px solid var(--color-border-default); border-radius: 999px; padding: 1px 6px; @@ -5006,6 +4958,25 @@ html.light .xterm .xterm-viewport:hover::-webkit-scrollbar-thumb:active { gap: 8px; } +.add-project-modal__formrow { + min-width: 0; + display: flex; + align-items: center; + gap: 16px; +} + +.add-project-modal__field { + flex: 1; + min-width: 0; + display: flex; + align-items: center; + gap: 8px; +} + +.add-project-modal__field .add-project-modal__selection-path { + flex: 1; +} + .add-project-modal__selection-label { flex: none; font-family: var(--font-sans); @@ -5173,20 +5144,10 @@ html.light .xterm .xterm-viewport:hover::-webkit-scrollbar-thumb:active { display: block; } - .add-project-browser { - grid-template-columns: 1fr; - grid-template-rows: auto minmax(0, 1fr); - } - - .add-project-browser__sidebar { - max-height: 112px; - border-right: 0; - border-bottom: 1px solid var(--color-border-subtle); - } - - .add-project-browser__sidebar-body { - max-height: 76px; - overflow-y: auto; + .add-project-modal__formrow { + flex-direction: column; + align-items: stretch; + gap: 8px; } .add-project-modal__footer { diff --git a/packages/web/src/components/AddProjectModal.parts.tsx b/packages/web/src/components/AddProjectModal.parts.tsx index 5d8eac1c5..9d196ef78 100644 --- a/packages/web/src/components/AddProjectModal.parts.tsx +++ b/packages/web/src/components/AddProjectModal.parts.tsx @@ -1,6 +1,5 @@ import type { ReactNode } from "react"; -const RECENT_PATHS_KEY = "ao:add-project:recent"; const WINDOWS_DRIVE_ROOT_PATTERN = /^[A-Za-z]:[\\/]?$/; const WINDOWS_ABSOLUTE_PATTERN = /^[A-Za-z]:[\\/]/; const UNC_ROOT_PATTERN = /^\\\\[^\\]+\\[^\\]+\\?$/; @@ -78,22 +77,6 @@ export function getBreadcrumbs(currentPath: string): Array<{ label: string; path return crumbs; } -export function loadRecentPaths(): string[] { - if (typeof window === "undefined") return []; - try { - const parsed = JSON.parse(window.localStorage.getItem(RECENT_PATHS_KEY) ?? "[]"); - return Array.isArray(parsed) ? parsed.filter((value): value is string => typeof value === "string") : []; - } catch { - return []; - } -} - -export function saveRecentPath(input: string) { - if (typeof window === "undefined") return; - const next = [input, ...loadRecentPaths().filter((value) => value !== input)].slice(0, 5); - window.localStorage.setItem(RECENT_PATHS_KEY, JSON.stringify(next)); -} - function Glyph({ children, className, @@ -106,28 +89,6 @@ function Glyph({ return ; } -export function SidebarSection({ - title, - open, - onToggle, - children, -}: { - title: string; - open: boolean; - onToggle: () => void; - children: ReactNode; -}) { - return ( -
- - {open ?
{children}
: null} -
- ); -} - const iconPath = { fill: "none", stroke: "currentColor", strokeWidth: 1.5, strokeLinejoin: "miter" as const }; const iconStroke = { fill: "none", stroke: "currentColor", strokeWidth: 1.75, strokeLinecap: "square" as const }; const compoundStroke = { fill: "none", stroke: "currentColor", strokeWidth: 1.5, strokeLinecap: "square" as const, strokeLinejoin: "miter" as const }; @@ -153,7 +114,7 @@ export function ArrowUpIcon() { } export function RefreshIcon() { - return ; + return ; } export function SortChevronIcon() { diff --git a/packages/web/src/components/AddProjectModal.tsx b/packages/web/src/components/AddProjectModal.tsx index bfd76520d..57d6ff75b 100644 --- a/packages/web/src/components/AddProjectModal.tsx +++ b/packages/web/src/components/AddProjectModal.tsx @@ -6,7 +6,6 @@ import { deriveProjectIdFromPath, deriveProjectNameFromPath, joinBrowsePath, - saveRecentPath, } from "@/components/AddProjectModal.parts"; import { DirectoryBrowser } from "@/components/DirectoryBrowser"; import { useDirectoryBrowser } from "@/hooks/useDirectoryBrowser"; @@ -125,7 +124,6 @@ export function AddProjectModal({ open, onClose }: AddProjectModalProps) { else setNetworkError(message); return; } - saveRecentPath(resolvedPath); const nextProjectId = body?.projectId ?? projectId.trim(); onClose(); router.push(`/projects/${encodeURIComponent(nextProjectId)}`); @@ -224,27 +222,29 @@ export function AddProjectModal({ open, onClose }: AddProjectModalProps) { Selected {selectedBrowsePath || "No directory selected"} -
- - setProjectIdInput(event.target.value)} - className="add-project-modal__selection-path" - /> -
-
- - setProjectNameInput(event.target.value)} - className="add-project-modal__selection-path" - /> +
+
+ + setProjectIdInput(event.target.value)} + className="add-project-modal__selection-path" + /> +
+
+ + setProjectNameInput(event.target.value)} + className="add-project-modal__selection-path" + /> +
{selectedNotice} diff --git a/packages/web/src/components/DirectoryBrowser.tsx b/packages/web/src/components/DirectoryBrowser.tsx index 2e1020c93..a62dcfbbb 100644 --- a/packages/web/src/components/DirectoryBrowser.tsx +++ b/packages/web/src/components/DirectoryBrowser.tsx @@ -1,6 +1,6 @@ "use client"; -import { useEffect, useMemo, useRef, useState } from "react"; +import { useEffect, useMemo, useRef } from "react"; import { ArrowUpIcon, ChevronLeftIcon, @@ -8,9 +8,7 @@ import { FolderIcon, getBreadcrumbs, joinBrowsePath, - loadRecentPaths, RefreshIcon, - SidebarSection, } from "@/components/AddProjectModal.parts"; import type { UseDirectoryBrowser } from "@/hooks/useDirectoryBrowser"; @@ -28,13 +26,17 @@ function isFolderRowTarget(target: EventTarget | null): boolean { return target instanceof HTMLElement && target.classList.contains("add-project-browser__row"); } +// A button/link handles Enter natively (breadcrumbs, toolbar buttons). Enter-to-descend +// must skip those so it does not double-fire alongside the element's own activation. +function isActivatableElement(target: EventTarget | null): boolean { + return target instanceof HTMLButtonElement || target instanceof HTMLAnchorElement; +} + export function DirectoryBrowser({ browser }: DirectoryBrowserProps) { const toolbarRef = useRef(null); const contentRef = useRef(null); const typeaheadBufferRef = useRef(""); const typeaheadLastTsRef = useRef(0); - const [recentOpen, setRecentOpen] = useState(true); - const [recentPaths] = useState(() => loadRecentPaths()); const selectedIndex = useMemo( () => browser.directoryEntries.findIndex( @@ -46,11 +48,18 @@ export function DirectoryBrowser({ browser }: DirectoryBrowserProps) { useEffect(() => { const handleKeyDown = (event: KeyboardEvent) => { const target = event.target; - const isInsideBrowser = - target instanceof Node && - (toolbarRef.current?.contains(target) || contentRef.current?.contains(target)); - if (!isInsideBrowser) return; - if (isTextEditingTarget(event.target)) return; + if (!(target instanceof Node)) return; + if (isTextEditingTarget(target)) return; + // In scope when the event comes from inside the browser panes OR from an + // ancestor that contains them (the modal dialog focused on open, document.body). + // The plain ancestor `.contains` checks fail for that case because the dialog + // is an ancestor of the panes, not a descendant. + const content = contentRef.current; + const inScope = + (content?.contains(target) ?? false) || + (toolbarRef.current?.contains(target) ?? false) || + (content !== null && target.contains(content)); + if (!inScope) return; if (event.key.length === 1 && !event.ctrlKey && !event.metaKey && !event.altKey) { const now = Date.now(); @@ -86,7 +95,7 @@ export function DirectoryBrowser({ browser }: DirectoryBrowserProps) { if ( event.key === "Enter" && selectedIndex >= 0 && - isFolderRowTarget(event.target) && + (isFolderRowTarget(target) || !isActivatableElement(target)) && !event.metaKey && !event.ctrlKey && !event.altKey && @@ -101,6 +110,11 @@ export function DirectoryBrowser({ browser }: DirectoryBrowserProps) { return () => document.removeEventListener("keydown", handleKeyDown); }, [browser, selectedIndex]); + const breadcrumbs = getBreadcrumbs(browser.browsePath); + const currentName = breadcrumbs[breadcrumbs.length - 1]?.label ?? browser.browsePath; + const showCurrentFolder = browser.browsePath !== "~" && browser.currentDirectory !== null; + const currentSelected = browser.selectedBrowsePath === browser.browsePath; + return ( <>
@@ -169,70 +183,74 @@ export function DirectoryBrowser({ browser }: DirectoryBrowserProps) {
-
- -
-
-
Current folder
-
- {getBreadcrumbs(browser.browsePath).map((crumb) => ( - - ))} -
-
{browser.browsePath}
- {browser.error ? ( -
-

Directory browser unavailable

-

{browser.error}

-
- ) : browser.loading ? ( -
-

Loading folders

-

Fetching directories for this location.

-
- ) : browser.directoryEntries.length === 0 ? ( -
-

No visible folders here

-

Try navigating up or picking a different location.

-
- ) : ( -
- {browser.parentPath ? ( - - ) : null} - {browser.directoryEntries.map((entry) => { +
+ {browser.error ? ( +
+

Directory browser unavailable

+

{browser.error}

+
+ ) : browser.loading ? ( +
+

Loading folders

+

Fetching directories for this location.

+
+ ) : ( +
+ {showCurrentFolder ? ( + + ) : null} + {browser.parentPath ? ( + + ) : null} + {browser.directoryEntries.length === 0 ? ( +

No subfolders in this location.

+ ) : ( + browser.directoryEntries.map((entry) => { const nextPath = joinBrowsePath(browser.browsePath, entry.name); return ( ); - })} -
- )} -
+ }) + )} +
+ )}
diff --git a/packages/web/src/components/__tests__/DirectoryBrowser.test.tsx b/packages/web/src/components/__tests__/DirectoryBrowser.test.tsx index d2ff2868b..03726ee33 100644 --- a/packages/web/src/components/__tests__/DirectoryBrowser.test.tsx +++ b/packages/web/src/components/__tests__/DirectoryBrowser.test.tsx @@ -4,8 +4,6 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { DirectoryBrowser } from "@/components/DirectoryBrowser"; import { useDirectoryBrowser, type UseDirectoryBrowser } from "@/hooks/useDirectoryBrowser"; -const localStorageDescriptor = Object.getOwnPropertyDescriptor(window, "localStorage"); - function Harness() { const browser = useDirectoryBrowser(); useEffect(() => { @@ -17,7 +15,6 @@ function Harness() { describe("DirectoryBrowser", () => { afterEach(() => { vi.unstubAllGlobals(); - if (localStorageDescriptor) Object.defineProperty(window, "localStorage", localStorageDescriptor); }); it("renders folders from the browse API and selects on click", async () => { @@ -57,7 +54,7 @@ describe("DirectoryBrowser", () => { render(); - const repoRow = await screen.findByRole("button", { name: "repo" }); + const repoRow = await screen.findByRole("button", { name: "repo, Git repository" }); const plainRow = await screen.findByRole("button", { name: "plain" }); expect(repoRow.querySelector(".add-project-browser__row-icon")).not.toBeNull(); @@ -172,63 +169,20 @@ describe("DirectoryBrowser", () => { await waitFor(() => expect(row?.className).toContain("is-selected")); }); - it("lists recent folders and navigates on click", async () => { - const store = new Map([["ao:add-project:recent", JSON.stringify(["~/projects/old"])]]); - Object.defineProperty(window, "localStorage", { - value: { - getItem: (k: string) => store.get(k) ?? null, - setItem: (k: string, v: string) => store.set(k, v), - removeItem: (k: string) => store.delete(k), - clear: () => store.clear(), - get length() { - return store.size; - }, - key: (i: number) => [...store.keys()][i] ?? null, - }, - writable: true, - configurable: true, - }); - const fetchMock = vi.fn().mockResolvedValue({ ok: true, json: async () => ({ entries: [], roots: [] }) }); - vi.stubGlobal("fetch", fetchMock); - - render(); - - fireEvent.click(await screen.findByText("~/projects/old")); - await waitFor(() => - expect(fetchMock).toHaveBeenCalledWith("/api/filesystem/browse?path=~%2Fprojects%2Fold"), - ); - await waitFor(() => expect(screen.getAllByText("~/projects/old")).toHaveLength(2)); - }); - - it("selects the recent folder path when navigating from the sidebar", async () => { - const store = new Map([["ao:add-project:recent", JSON.stringify(["~/projects/old"])]]); - Object.defineProperty(window, "localStorage", { - value: { - getItem: (k: string) => store.get(k) ?? null, - setItem: (k: string, v: string) => store.set(k, v), - removeItem: (k: string) => store.delete(k), - clear: () => store.clear(), - get length() { - return store.size; - }, - key: (i: number) => [...store.keys()][i] ?? null, - }, - writable: true, - configurable: true, - }); + it("renders a selectable, git-aware current-folder row", () => { const browser = { - browsePath: "~", - selectedBrowsePath: "~", + browsePath: "~/workspace/app", + selectedBrowsePath: "~/workspace/app/sub", setSelectedBrowsePath: vi.fn(), - directoryEntries: [], - currentDirectory: null, + directoryEntries: [{ name: "sub", isDirectory: true, isGitRepo: false, hasLocalConfig: false }], + currentDirectory: { isGitRepo: true, hasLocalConfig: false }, roots: [], selectedRootPath: "", - locationInput: "~", + locationInput: "~/workspace/app", setLocationInput: vi.fn(), loading: false, error: null, - parentPath: null, + parentPath: "~/workspace", canGoBack: false, canGoForward: false, browse: vi.fn(), @@ -241,9 +195,10 @@ describe("DirectoryBrowser", () => { render(); - fireEvent.click(screen.getByRole("button", { name: "~/projects/old" })); + const currentRow = screen.getByRole("button", { name: "app, current folder, Git repository" }); + fireEvent.click(currentRow); - expect(browser.browse).toHaveBeenCalledWith("~/projects/old", { selectedPath: "~/projects/old" }); + expect(browser.setSelectedBrowsePath).toHaveBeenCalledWith("~/workspace/app"); }); it("does not descend on modified Enter when a folder is selected", async () => { @@ -325,4 +280,35 @@ describe("DirectoryBrowser", () => { expect(browser.reset).not.toHaveBeenCalled(); }); + + it("handles keyboard navigation when focus is on an ancestor container", async () => { + vi.stubGlobal( + "fetch", + vi.fn().mockResolvedValue({ + ok: true, + json: async () => ({ + entries: [ + { name: "alpha", isDirectory: true, isGitRepo: false, hasLocalConfig: false }, + { name: "beta", isDirectory: true, isGitRepo: false, hasLocalConfig: false }, + ], + roots: [], + }), + }), + ); + + render( +
+ +
, + ); + + const alpha = (await screen.findByText("alpha")).closest("button"); + expect(alpha).not.toBeNull(); + + // The dialog wrapper is an ancestor of the browser panes — the state focus + // lands on when the modal opens. Keyboard nav must still respond. + fireEvent.keyDown(screen.getByTestId("dialog"), { key: "ArrowDown" }); + + await waitFor(() => expect(alpha?.className).toContain("is-selected")); + }); });