diff --git a/__tests__/components/features/home/generic-dropdown-menu.test.tsx b/__tests__/components/features/home/generic-dropdown-menu.test.tsx new file mode 100644 index 000000000..48df3c488 --- /dev/null +++ b/__tests__/components/features/home/generic-dropdown-menu.test.tsx @@ -0,0 +1,70 @@ +import { render, screen } from "@testing-library/react"; +import { describe, expect, it } from "vitest"; + +import { GenericDropdownMenu } from "../../../../src/components/features/home/shared/generic-dropdown-menu"; + +interface Item { + id: string; + name: string; +} + +const ITEMS: Item[] = [ + { id: "a", name: "a" }, + { id: "b", name: "b" }, +]; + +function renderMenu(props: Record = {}) { + return render( + + isOpen + filteredItems={ITEMS} + inputValue="" + highlightedIndex={-1} + selectedItem={null} + // eslint-disable-next-line @typescript-eslint/no-explicit-any + getMenuProps={(opts: any) => ({ ...opts })} + // eslint-disable-next-line @typescript-eslint/no-explicit-any + getItemProps={(opts: any) => ({ ...opts })} + renderItem={(item) => ( +
  • + {item.name} +
    • + )} + renderEmptyState={() =>
  • elements as direct children of the listbox
      (valid list markup)", () => { + // numberOfRecentItems > 0 triggers the recent-items divider after the + // first item. A
        may only contain
      • children; a
        divider is + // invalid markup that a11y tooling flags (Copilot, agent-canvas#1). + renderMenu({ numberOfRecentItems: 1 }); + + const list = screen.getByTestId("generic-menu"); + const nonLiChildren = Array.from(list.children).filter( + (el) => el.tagName !== "LI", + ); + + expect(nonLiChildren.map((el) => el.tagName)).toEqual([]); + }); + + it("keeps the recent-items divider out of the accessibility tree", () => { + renderMenu({ numberOfRecentItems: 1 }); + + // The divider must be presentational so screen readers don't announce a + // phantom list item. Match it by its role directly (not "not an option") + // so the assertion stays pinned if another non-option child is added. + const list = screen.getByTestId("generic-menu"); + const divider = Array.from(list.children).find( + (el) => el.getAttribute("role") === "presentation", + ); + expect(divider).toBeDefined(); + expect(divider).toHaveAttribute("role", "presentation"); + expect(divider).toHaveAttribute("aria-hidden", "true"); + }); +}); 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..a5d22882b 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, @@ -115,7 +124,11 @@ export function GenericDropdownMenu({ )} {numberOfRecentItems > 0 && index === numberOfRecentItems - 1 && ( -
        +
        • + ); + } + : 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({