From 8ee7f917b3b2f561e76aa11e8a589b6e10483d51 Mon Sep 17 00:00:00 2001 From: george larson Date: Sat, 13 Jun 2026 18:18:06 -0400 Subject: [PATCH 1/4] feat(home): group workspace folders by parent in the dropdown (#129) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Repository/Workspace dropdown rendered every folder flat, so with more than one workspace it was unclear which folder belonged to which. Group folders by their parent workspace under a header. - useResolvedWorkspaces now also returns the merged `parents` (stored + implicit /projects), so the dropdown can label each group by the parent's real name rather than a path basename. - WorkspaceDropdown reorders the filtered list into contiguous groups and feeds that single array to both downshift's `items` and the menu, keeping highlightedIndex/keyboard nav in lockstep with the visible order. Headers are presentational siblings injected via a new opt-in GenericDropdownMenu `renderItemPrefix` slot — they consume no downshift index (modeled on the existing numberOfRecentItems divider). Flat fallback preserved for <=1 group. - Static (no-parent) workspaces group under an 'Other' header (new i18n key, all locales). Tests: grouping by parent name, contiguity, keyboard nav skips headers, flat fallback, static-group handling. Full suite + typecheck + lint green. --- .../features/home/workspace-dropdown.test.tsx | 264 ++++++++++++++++++ .../features/home/shared/dropdown-item.tsx | 8 + .../home/shared/generic-dropdown-menu.tsx | 9 + .../workspace-dropdown/workspace-dropdown.tsx | 113 +++++++- .../home/workspace-selection-form.tsx | 2 + src/hooks/query/use-resolved-workspaces.ts | 16 +- src/i18n/translation.json | 17 ++ 7 files changed, 425 insertions(+), 4 deletions(-) create mode 100644 __tests__/components/features/home/workspace-dropdown.test.tsx diff --git a/__tests__/components/features/home/workspace-dropdown.test.tsx b/__tests__/components/features/home/workspace-dropdown.test.tsx new file mode 100644 index 000000000..c4eac7209 --- /dev/null +++ b/__tests__/components/features/home/workspace-dropdown.test.tsx @@ -0,0 +1,264 @@ +import type { ComponentProps } from "react"; +import { render, screen, within } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { describe, expect, it, vi } from "vitest"; + +import { WorkspaceDropdown } from "../../../../src/components/features/home/workspace-dropdown/workspace-dropdown"; +import { LocalWorkspace, LocalWorkspaceParent } from "#/types/workspace"; + +vi.mock("react-i18next", () => ({ + useTranslation: () => ({ t: (key: string) => key }), +})); + +// Two parents whose NAMES deliberately differ from their path basenames, so a +// passing assertion proves the header uses the parent name (not basename-of-path). +const PARENTS: LocalWorkspaceParent[] = [ + { id: "p1", name: "Projects", path: "/projects" }, + { id: "p2", name: "My Work", path: "/work" }, +]; + +const GROUPED_WORKSPACES: LocalWorkspace[] = [ + { + id: "/projects/alpha", + name: "alpha", + path: "/projects/alpha", + parentPath: "/projects", + }, + { id: "/work/beta", name: "beta", path: "/work/beta", parentPath: "/work" }, + { + id: "/projects/gamma", + name: "gamma", + path: "/projects/gamma", + parentPath: "/projects", + }, +]; + +function renderDropdown( + props: Partial> = {}, +) { + const onChange = vi.fn(); + render( + , + ); + return { onChange }; +} + +async function openMenu(user: ReturnType) { + const input = screen.getByTestId("workspace-dropdown"); + await user.click(input); + return screen.findByTestId("workspace-dropdown-menu"); +} + +describe("WorkspaceDropdown grouping (#129)", () => { + it("renders a header with the parent's NAME (not the path basename) per group", async () => { + const user = userEvent.setup(); + renderDropdown(); + const menu = await openMenu(user); + + const headers = within(menu).getAllByTestId("workspace-group-header"); + const headerText = headers.map((h) => h.textContent); + expect(headerText).toContain("Projects"); // not "projects" + expect(headerText).toContain("My Work"); // not "work" + }); + + it("groups every folder under its parent and shows all folders", async () => { + const user = userEvent.setup(); + renderDropdown(); + const menu = await openMenu(user); + + expect(within(menu).getByText("alpha")).toBeInTheDocument(); + expect(within(menu).getByText("beta")).toBeInTheDocument(); + expect(within(menu).getByText("gamma")).toBeInTheDocument(); + // alpha and gamma (both /projects) render contiguously, before beta's group + // boundary is crossed — exactly one header per distinct parent. + expect(within(menu).getAllByTestId("workspace-group-header")).toHaveLength( + 2, + ); + }); + + it("keyboard navigation skips headers: ArrowDown+Enter selects a real folder", async () => { + const user = userEvent.setup(); + const { onChange } = renderDropdown(); + await openMenu(user); + + await user.keyboard("{ArrowDown}{Enter}"); + + // A header must never be selectable; the first highlight lands on a workspace. + expect(onChange).toHaveBeenCalledTimes(1); + const selected = onChange.mock.calls[0][0] as LocalWorkspace | null; + expect(selected).not.toBeNull(); + expect(GROUPED_WORKSPACES.map((w) => w.id)).toContain(selected?.id); + }); + + it("does NOT render headers when there is only one group (flat fallback)", async () => { + const user = userEvent.setup(); + renderDropdown({ + workspaces: [ + { + id: "/projects/alpha", + name: "alpha", + path: "/projects/alpha", + parentPath: "/projects", + }, + { + id: "/projects/gamma", + name: "gamma", + path: "/projects/gamma", + parentPath: "/projects", + }, + ], + }); + const menu = await openMenu(user); + + expect(within(menu).queryByTestId("workspace-group-header")).toBeNull(); + expect(within(menu).getByText("alpha")).toBeInTheDocument(); + expect(within(menu).getByText("gamma")).toBeInTheDocument(); + }); + + it("groups static (no-parent) workspaces under their own header", async () => { + const user = userEvent.setup(); + renderDropdown({ + workspaces: [ + { + id: "/projects/alpha", + name: "alpha", + path: "/projects/alpha", + parentPath: "/projects", + }, + { id: "/standalone", name: "standalone", path: "/standalone" }, // no parentPath + ], + }); + const menu = await openMenu(user); + + const headers = within(menu).getAllByTestId("workspace-group-header"); + // one header for the named parent, one for the static group + expect(headers).toHaveLength(2); + expect(within(menu).getByText("Projects")).toBeInTheDocument(); + expect(within(menu).getByText("standalone")).toBeInTheDocument(); + }); + + it("labels the static group with the HOME$WORKSPACE_GROUP_OTHER i18n key", async () => { + const user = userEvent.setup(); + renderDropdown({ + workspaces: [ + { + id: "/projects/alpha", + name: "alpha", + path: "/projects/alpha", + parentPath: "/projects", + }, + { id: "/standalone", name: "standalone", path: "/standalone" }, + ], + }); + const menu = await openMenu(user); + + // `t` is mocked to echo the key, so this pins the i18n wiring. + expect( + within(menu).getByText("HOME$WORKSPACE_GROUP_OTHER"), + ).toBeInTheDocument(); + }); + + it("renders the static 'Other' group last, even when a standalone appears first", async () => { + const user = userEvent.setup(); + renderDropdown({ + workspaces: [ + { id: "/loose", name: "loose", path: "/loose" }, // standalone, appears FIRST + { + id: "/work/beta", + name: "beta", + path: "/work/beta", + parentPath: "/work", + }, + ], + parents: [{ id: "p2", name: "My Work", path: "/work" }], + }); + const menu = await openMenu(user); + + const headerText = within(menu) + .getAllByTestId("workspace-group-header") + .map((h) => h.textContent); + expect(headerText).toEqual(["My Work", "HOME$WORKSPACE_GROUP_OTHER"]); + }); + + it("falls back to the path basename when a folder's parent is absent from `parents`", async () => { + const user = userEvent.setup(); + renderDropdown({ + workspaces: [ + { + id: "/work/beta", + name: "beta", + path: "/work/beta", + parentPath: "/work", + }, + { + id: "/unknown/x", + name: "x", + path: "/unknown/x", + parentPath: "/unknown", + }, + ], + parents: [{ id: "p2", name: "My Work", path: "/work" }], // /unknown intentionally absent + }); + const menu = await openMenu(user); + + const headerText = within(menu) + .getAllByTestId("workspace-group-header") + .map((h) => h.textContent); + expect(headerText).toContain("My Work"); + expect(headerText).toContain("unknown"); // basename of /unknown + }); + + it("folds the group label into each option's accessible name (a11y)", async () => { + const user = userEvent.setup(); + renderDropdown(); + const menu = await openMenu(user); + + // The visual header is presentational; the group lives in each option's name. + expect( + within(menu).getByRole("option", { name: "Projects, alpha" }), + ).toBeInTheDocument(); + expect( + within(menu).getByRole("option", { name: "My Work, beta" }), + ).toBeInTheDocument(); + }); + + it("stays flat (no headers) for an all-standalone list with no parents", async () => { + const user = userEvent.setup(); + renderDropdown({ + parents: undefined, // exercises the single STATIC_GROUP_KEY group path + workspaces: [ + { id: "/a", name: "a", path: "/a" }, + { id: "/b", name: "b", path: "/b" }, + ], + }); + const menu = await openMenu(user); + + expect(within(menu).queryByTestId("workspace-group-header")).toBeNull(); + expect(within(menu).getByText("a")).toBeInTheDocument(); + expect(within(menu).getByText("b")).toBeInTheDocument(); + }); + + it("does not set an aria-label on options in flat (ungrouped) mode", async () => { + const user = userEvent.setup(); + renderDropdown({ + parents: undefined, + workspaces: [ + { id: "/a", name: "a", path: "/a" }, + { id: "/b", name: "b", path: "/b" }, + ], + }); + const menu = await openMenu(user); + + // No grouping → the option's accessible name is just its display text. + const option = within(menu).getByRole("option", { name: "a" }); + expect(option).not.toHaveAttribute("aria-label"); + }); +}); diff --git a/src/components/features/home/shared/dropdown-item.tsx b/src/components/features/home/shared/dropdown-item.tsx index d0ccba306..28e061262 100644 --- a/src/components/features/home/shared/dropdown-item.tsx +++ b/src/components/features/home/shared/dropdown-item.tsx @@ -16,6 +16,12 @@ interface DropdownItemProps { isProviderDropdown?: boolean; renderIcon?: (item: T) => React.ReactNode; itemClassName?: string; + /** + * Overrides the option's accessible name. Used to fold a group label into the + * announced name (e.g. "Projects, alpha") when the visual group header is + * presentational and therefore invisible to assistive tech. + */ + ariaLabel?: string; } export function DropdownItem({ @@ -28,10 +34,12 @@ export function DropdownItem({ isProviderDropdown = false, renderIcon, itemClassName, + ariaLabel, }: DropdownItemProps) { const itemProps = getItemProps({ index, item, + ...(ariaLabel ? { "aria-label": ariaLabel } : {}), className: cn( isProviderDropdown ? "group px-2 py-0 cursor-pointer text-xs rounded-md mx-0 my-0 h-6 flex items-center" diff --git a/src/components/features/home/shared/generic-dropdown-menu.tsx b/src/components/features/home/shared/generic-dropdown-menu.tsx index 025b9c434..be4760f7d 100644 --- a/src/components/features/home/shared/generic-dropdown-menu.tsx +++ b/src/components/features/home/shared/generic-dropdown-menu.tsx @@ -29,6 +29,13 @@ export interface GenericDropdownMenuProps { options: UseComboboxGetItemPropsOptions & Options, ) => any, // eslint-disable-line @typescript-eslint/no-explicit-any ) => React.ReactNode; + /** + * Optional presentational node rendered immediately BEFORE an item (e.g. a + * group header). It is a sibling of the item, not part of downshift's `items` + * array, so it consumes no item index — mirroring the `numberOfRecentItems` + * divider. The consumer owns when a prefix appears (e.g. at group boundaries). + */ + renderItemPrefix?: (item: T, index: number) => React.ReactNode; renderEmptyState: (inputValue: string) => React.ReactNode; stickyTopItem?: React.ReactNode; stickyFooterItem?: React.ReactNode; @@ -48,6 +55,7 @@ export function GenericDropdownMenu({ onScroll, menuRef, renderItem, + renderItemPrefix, renderEmptyState, stickyTopItem, stickyFooterItem, @@ -106,6 +114,7 @@ export function GenericDropdownMenu({ const key = itemKey(item); return ( + {renderItemPrefix?.(item, index)} {renderItem( item, index, diff --git a/src/components/features/home/workspace-dropdown/workspace-dropdown.tsx b/src/components/features/home/workspace-dropdown/workspace-dropdown.tsx index 6f731d8eb..46a3bfec9 100644 --- a/src/components/features/home/workspace-dropdown/workspace-dropdown.tsx +++ b/src/components/features/home/workspace-dropdown/workspace-dropdown.tsx @@ -8,7 +8,7 @@ import { dropdownMenuListClassName, } from "#/utils/dropdown-classes"; import { formControlFieldClassName } from "#/utils/form-control-classes"; -import { LocalWorkspace } from "#/types/workspace"; +import { LocalWorkspace, LocalWorkspaceParent } from "#/types/workspace"; import { I18nKey } from "#/i18n/declaration"; import RepoIcon from "#/icons/repo.svg?react"; import { StyledTooltip } from "#/components/shared/buttons/styled-tooltip"; @@ -19,8 +19,21 @@ import { DropdownItem } from "../shared/dropdown-item"; import { EmptyState } from "../shared/empty-state"; import { GenericDropdownMenu } from "../shared/generic-dropdown-menu"; +// Sentinel group key for standalone workspaces (no parent). Module-scoped so +// it's a stable reference for hooks and reads clearly as a non-path sentinel. +const STATIC_GROUP_KEY = "__ungrouped__"; + export interface WorkspaceDropdownProps { workspaces: LocalWorkspace[]; + /** + * The workspace parents that produced the dynamic children in `workspaces` + * (from `useResolvedWorkspaces`, including the implicit `/projects`). Used to + * label each folder's group by its parent's `name`. When two or more distinct + * groups are present the list renders grouped under headers; with one group + * (or omitted) it stays flat. A child whose `parentPath` has no matching + * parent here falls back to the path basename. + */ + parents?: LocalWorkspaceParent[]; value: LocalWorkspace | null; placeholder?: string; className?: string; @@ -40,6 +53,7 @@ export interface WorkspaceDropdownProps { export function WorkspaceDropdown({ workspaces, + parents = [], value, placeholder, className, @@ -64,6 +78,77 @@ export function WorkspaceDropdown({ ); }, [workspaces, inputValue]); + // Group the filtered list by parent so folders from the same workspace render + // contiguously under a header. The grouped array is the SINGLE source for both + // downshift's `items` and the menu render, so `highlightedIndex` and keyboard + // navigation stay in lockstep with the visible order. Headers are presentational + // siblings (see `renderItemPrefix`) and consume no downshift index; each option + // carries its group label as an accessible name so screen-reader users get the + // grouping the presentational header can't convey. + const groupKeyOf = useCallback( + (w: LocalWorkspace) => w.parentPath ?? STATIC_GROUP_KEY, + [], + ); + const parentNameByPath = useMemo(() => { + const map = new Map(); + parents.forEach((p) => map.set(p.path, p.name)); + return map; + }, [parents]); + + const { + groupedWorkspaces, + isGrouped, + headerByFirstIndex, + groupLabelByIndex, + } = useMemo(() => { + const order: string[] = []; + const byGroup = new Map(); + filteredWorkspaces.forEach((w) => { + const key = groupKeyOf(w); + if (!byGroup.has(key)) { + byGroup.set(key, []); + order.push(key); + } + byGroup.get(key)?.push(w); + }); + + // Keep the catch-all "Other" (no-parent) group last, wherever its first + // member happened to appear. Stable sort preserves named-group order. + order.sort((a, b) => { + if (a === STATIC_GROUP_KEY) return 1; + if (b === STATIC_GROUP_KEY) return -1; + return 0; + }); + + const grouping = order.length > 1; + const grouped: LocalWorkspace[] = []; + const headers = new Map(); + const labelByIndex = new Map(); + order.forEach((key) => { + // `||` (not `??`) so an empty parent name falls through to the basename. + const label = + key === STATIC_GROUP_KEY + ? t(I18nKey.HOME$WORKSPACE_GROUP_OTHER) + : parentNameByPath.get(key) || + key.split("/").filter(Boolean).pop() || + key; + if (grouping) { + headers.set(grouped.length, label); + } + (byGroup.get(key) ?? []).forEach((w) => { + labelByIndex.set(grouped.length, label); + grouped.push(w); + }); + }); + + return { + groupedWorkspaces: grouped, + isGrouped: grouping, + headerByFirstIndex: headers, + groupLabelByIndex: labelByIndex, + }; + }, [filteredWorkspaces, groupKeyOf, parentNameByPath, t]); + const handleSelectionChange = useCallback( (selectedItem: LocalWorkspace | null) => { onChange(selectedItem); @@ -89,7 +174,7 @@ export function WorkspaceDropdown({ selectedItem, closeMenu, } = useCombobox({ - items: filteredWorkspaces, + items: groupedWorkspaces, itemToString: (item) => item?.name ?? "", selectedItem: value, onSelectedItemChange: ({ selectedItem: newSelectedItem }) => { @@ -128,6 +213,11 @@ export function WorkspaceDropdown({ getItemProps={itemGetItemProps} getDisplayText={(workspace) => workspace.name} getItemKey={(workspace) => workspace.id} + ariaLabel={ + isGrouped + ? `${groupLabelByIndex.get(index) ?? ""}, ${item.name}`.trim() + : undefined + } /> ); @@ -263,7 +353,7 @@ export function WorkspaceDropdown({ isOpen={isOpen} - filteredItems={filteredWorkspaces} + filteredItems={groupedWorkspaces} inputValue={inputValue} highlightedIndex={highlightedIndex} selectedItem={selectedItem} @@ -271,6 +361,23 @@ export function WorkspaceDropdown({ getItemProps={getItemProps} menuRef={menuRef} renderItem={renderItem} + renderItemPrefix={ + isGrouped + ? (_item, index) => { + const label = headerByFirstIndex.get(index); + if (!label) return null; + return ( +
  • + {label} +
    • + ); + } + : undefined + } renderEmptyState={renderEmptyState} stickyFooterItem={stickyFooterItem} testId="workspace-dropdown-menu" diff --git a/src/components/features/home/workspace-selection-form.tsx b/src/components/features/home/workspace-selection-form.tsx index 75c417aef..9578eb82f 100644 --- a/src/components/features/home/workspace-selection-form.tsx +++ b/src/components/features/home/workspace-selection-form.tsx @@ -79,6 +79,7 @@ export function WorkspaceSelectionForm({ const { mutate: removeWorkspaceParent } = useRemoveWorkspaceParent(); const { workspaces, + parents: resolvedParents, isLoading: isLoadingWorkspaces, isError: hasWorkspaceError, error: resolvedWorkspacesError, @@ -183,6 +184,7 @@ export function WorkspaceSelectionForm({
    Date: Tue, 16 Jun 2026 07:22:30 -0400 Subject: [PATCH 2/4] a11y(home): hide presentational group header from assistive tech The group label is already folded into each option's accessible name ("group, name"), so announcing the visual header re-reads the group. role="presentation" strips the
  • semantics but leaves its text in the accessibility tree; aria-hidden="true" removes the redundant announcement. --- .../features/home/workspace-dropdown/workspace-dropdown.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/features/home/workspace-dropdown/workspace-dropdown.tsx b/src/components/features/home/workspace-dropdown/workspace-dropdown.tsx index 46a3bfec9..5dca5c3f2 100644 --- a/src/components/features/home/workspace-dropdown/workspace-dropdown.tsx +++ b/src/components/features/home/workspace-dropdown/workspace-dropdown.tsx @@ -369,6 +369,7 @@ export function WorkspaceDropdown({ return (
  • MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The recent-items divider in GenericDropdownMenu was a
    rendered as a direct child of the listbox
      — invalid list markup that accessibility tooling flags (surfaced by Copilot on PR #1). Make it an