feat(home): group workspace folders by parent in the dropdown (#129)#1368
Open
georgeglarson wants to merge 4 commits into
Open
feat(home): group workspace folders by parent in the dropdown (#129)#1368georgeglarson wants to merge 4 commits into
georgeglarson wants to merge 4 commits into
Conversation
|
@georgeglarson is attempting to deploy a commit to the openhands Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds grouped workspace rendering in the Home workspace dropdown, including an “Other” bucket for standalone workspaces, and exposes resolved parent metadata so group headers can use parent display names.
Changes:
- Added a new i18n key for the standalone “Other” workspace group label.
- Extended
useResolvedWorkspacesto return merged workspace parents and wired that into the workspace selection flow. - Implemented grouped rendering + accessibility labeling in
WorkspaceDropdown, with supporting dropdown component enhancements and new tests.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/i18n/translation.json | Adds HOME$WORKSPACE_GROUP_OTHER translations for the new standalone group header. |
| src/hooks/query/use-resolved-workspaces.ts | Returns parents so UI can label groups using parent names (incl. implicit parents). |
| src/components/features/home/workspace-selection-form.tsx | Passes resolved parents into WorkspaceDropdown. |
| src/components/features/home/workspace-dropdown/workspace-dropdown.tsx | Groups workspaces by parent, adds headers, and augments option a11y names. |
| src/components/features/home/shared/generic-dropdown-menu.tsx | Adds renderItemPrefix to support non-item UI (e.g., headers) in the list. |
| src/components/features/home/shared/dropdown-item.tsx | Allows overriding option accessible name via ariaLabel. |
| tests/components/features/home/workspace-dropdown.test.tsx | Adds coverage for grouping behavior, ordering, and a11y labeling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } from "#/utils/dropdown-classes"; | ||
| import { formControlFieldClassName } from "#/utils/form-control-classes"; | ||
| import { LocalWorkspace } from "#/types/workspace"; | ||
| import { LocalWorkspace, LocalWorkspaceParent } from "#/types/workspace"; |
| import { describe, expect, it, vi } from "vitest"; | ||
|
|
||
| import { WorkspaceDropdown } from "../../../../src/components/features/home/workspace-dropdown/workspace-dropdown"; | ||
| import { LocalWorkspace, LocalWorkspaceParent } from "#/types/workspace"; |
Comment on lines
+216
to
+220
| ariaLabel={ | ||
| isGrouped | ||
| ? `${groupLabelByIndex.get(index) ?? ""}, ${item.name}`.trim() | ||
| : undefined | ||
| } |
Comment on lines
+370
to
+376
| <li | ||
| role="presentation" | ||
| data-testid="workspace-group-header" | ||
| className="px-2 pt-2 pb-1 text-xs font-medium text-[var(--oh-muted)] select-none" | ||
| > | ||
| {label} | ||
| </li> |
…nds#129) 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.
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 <li> semantics but leaves its text in
the accessibility tree; aria-hidden="true" removes the redundant
announcement.
georgeglarson
force-pushed
the
feat-129-group-workspace-folders
branch
from
302fa22 to
335a7f6
Compare
The recent-items divider in GenericDropdownMenu was a <div> rendered as a direct child of the listbox <ul> — invalid list markup that accessibility tooling flags (surfaced by Copilot on PR #1). Make it an <li role="presentation" aria-hidden="true">, matching the existing group-header pattern, so the <ul> contains only <li> children and the divider stays out of the a11y tree. Add generic-dropdown-menu.test.tsx asserting both invariants.
Match the recent-items divider by role="presentation" rather than "not an option", and assert the role explicitly, so the accessibility contract stays pinned if another non-option child is added later (CodeRabbit, PR #1).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
HUMAN:
Tested locally with
dev:mock: grouped dropdown renders parent headers, stays flat under 2 groups, and keyboard nav skips the headers.AGENT:
End-to-end evidence below. The branch was rebased onto current
main(7 commits) before opening; verification was re-run post-rebase.npm run lint→ clean:react-router typegen+tsc+eslint src+prettier --checkall pass, exit 0.npm test(fullvitest run) → 3213 passed, 5 skipped, 9 todo (3227 total), exit 0. Includes the new__tests__/components/features/home/workspace-dropdown.test.tsxsuite covering grouping order, labeling, accessibility names, keyboard nav (ArrowDown+Enter lands on a real folder, never a header), and fallback cases.npm run dev:mock(VITE_MOCK_API): the Home Repository/Workspace dropdown renders parent group headers, collapses to a flat list at <2 groups, floats the "Other" group last, and keyboard navigation skips headers.Why
The Home Repository/Workspace dropdown lists every workspace folder flat, so when folders live under different parents there's no visual ownership grouping. Closes #129. The approach was posted on the issue and approved by @DevinVinson before this PR.
Summary
parentPathplus the mergedparentsfromuseResolvedWorkspaces; grouping activates only at 2+ groups, otherwise the list stays flat.Issue Number
#129
How to Test
npm installnpm run dev:mock(runs with mocked API data)Automated:
npm run lintandnpm testboth pass (counts above).Video/Screenshots
Type
Notes
mainbefore opening; single commit. The only touched-file overlap with upstream drift (src/i18n/translation.json) merged cleanly.src/i18n/declaration.tsis a gitignored generated artifact (regenerated bymake-i18n(which runs inprelint/test/build)); the newHOME$WORKSPACE_GROUP_OTHERkey resolves undertsc.