feat(frontend): remove a project from the sidebar#219
Conversation
The daemon and CLI already support DELETE /api/v1/projects/{id}, but the
renderer had no UI for it. Add a per-row kebab (SidebarMenuAction, revealed on
hover) with Project settings + a destructive Remove project, wired to the
existing endpoint with an optimistic sidebar update; if the removed project was
the active route, fall back Home. ao project rm only archives the registration,
so this is reversible.
Closes #218
Greptile SummaryThis PR surfaces the existing
Confidence Score: 4/5Safe to merge; the only unhandled case is the kebab appearing in the collapsed icon rail, which is a visual oddity rather than a data-loss risk. The API call, cache update, and navigation are all wired correctly and mirror existing patterns. The kebab action is missing the group-data-[collapsible=icon]:hidden guard that the session-count span uses, so in icon/collapsed mode it can appear on hover and open a dropdown that overlaps the main panel. The test covers the happy path but skips the active-project-removal branch that calls goHome(). frontend/src/renderer/components/Sidebar.tsx — verify SidebarMenuAction behavior in the collapsed/icon rail before shipping. Important Files Changed
Sequence DiagramsequenceDiagram
actor User
participant Sidebar
participant Shell as _shell removeProject
participant API
participant Cache as React Query Cache
participant Router
User->>Sidebar: hover project row, kebab appears
User->>Sidebar: click kebab, click Remove project
Sidebar->>Shell: await onRemoveProject(projectId)
Shell->>API: "DELETE /api/v1/projects/{id}"
API-->>Shell: 200 OK
Shell->>Cache: updateWorkspaces, filter out projectId
Shell-->>Sidebar: resolves
Sidebar->>Sidebar: "check activeProjectId === projectId"
Sidebar->>Router: selection.goHome() if active
Router-->>User: renders home route
Reviews (1): Last reviewed commit: "feat(frontend): remove a project from th..." | Re-trigger Greptile |
| const handleRemove = () => { | ||
| void (async () => { | ||
| try { | ||
| await onRemoveProject(workspace.id); | ||
| // The route for a removed project no longer resolves; fall back home. | ||
| if (selection.activeProjectId === workspace.id) selection.goHome(); | ||
| } catch (err) { | ||
| // The app has no toast surface yet; the 15s workspace refetch resyncs | ||
| // the sidebar, so log and let the row reappear if the delete failed. | ||
| console.error("remove project failed", err); | ||
| } | ||
| })(); | ||
| }; |
There was a problem hiding this comment.
Navigation fires after the cache update, not atomically with it.
onRemoveProject (in _shell.tsx) removes the project from the React Query cache before it returns, which schedules a re-render. selection.goHome() is then called in the next microtask. In React 18's automatic-batching model both state changes are normally merged into one paint, but if the router renders the active project route before the navigation lands, the outlet may briefly see a missing project and throw/404. Moving goHome() into removeProject (in _shell.tsx, where it already owns the cache mutation) and accepting a navigate param would make the two operations atomic from the shell's perspective — but even as-is this is unlikely to surface visually due to batching.
| describe("Sidebar", () => { | ||
| it("removes a project from the row kebab menu", async () => { | ||
| const user = userEvent.setup(); | ||
| const onRemoveProject = renderSidebar(); | ||
|
|
||
| await user.click(screen.getByRole("button", { name: "Actions for my-app" })); | ||
| await user.click(await screen.findByText("Remove project")); | ||
|
|
||
| expect(onRemoveProject).toHaveBeenCalledWith("proj-1"); |
There was a problem hiding this comment.
goHome navigation branch isn't exercised by any test. The test stubs useParams to return {}, so activeProjectId is always undefined and the if (selection.activeProjectId === workspace.id) selection.goHome() branch in handleRemove is never reached. A second test case that sets useParams to return { projectId: "proj-1" } would cover the navigation on active-project removal and catch any regression in that branch.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| <DropdownMenu> | ||
| <DropdownMenuTrigger asChild> | ||
| <SidebarMenuAction showOnHover aria-label={`Actions for ${workspace.name}`}> | ||
| <MoreHorizontal aria-hidden="true" /> | ||
| </SidebarMenuAction> | ||
| </DropdownMenuTrigger> |
There was a problem hiding this comment.
The
SidebarMenuAction doesn't carry the group-data-[collapsible=icon]:hidden class that the session-count <span> uses to hide itself in the icon/collapsed rail. When the sidebar is collapsed to the 48 px letter rail, hovering the icon tile will still reveal the kebab button — and since DropdownMenuContent opens side="right", the menu will pop out from an already-narrow icon rail, likely overlapping the main content area.
| <DropdownMenu> | |
| <DropdownMenuTrigger asChild> | |
| <SidebarMenuAction showOnHover aria-label={`Actions for ${workspace.name}`}> | |
| <MoreHorizontal aria-hidden="true" /> | |
| </SidebarMenuAction> | |
| </DropdownMenuTrigger> | |
| <DropdownMenu> | |
| <DropdownMenuTrigger asChild> | |
| <SidebarMenuAction showOnHover aria-label={`Actions for ${workspace.name}`} className="group-data-[collapsible=icon]:hidden"> | |
| <MoreHorizontal aria-hidden="true" /> | |
| </SidebarMenuAction> | |
| </DropdownMenuTrigger> |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Closes #218.
Problem
The daemon and CLI both support removing a project (
DELETE /api/v1/projects/{id}/ao project rm), but the renderer had no UI for it — once added, a project could only be removed from the CLI. Surfaced while click-testing the app.Change
SidebarMenuActionwithshowOnHover, an absolute-positioned sibling of the row button — no nested-button issue). It reveals on hover where the session count fades, and opens a menu with Project settings and a destructive Remove project._shell.tsx: newremoveProjectcallback callsapiClient.DELETE("/api/v1/projects/{id}"), then optimistically drops the row from the workspace cache. If the removed project is the active route, the sidebar navigates Home.No backend changes — the endpoint already exists.
ao project rmonly archives the registration (repo on disk untouched, re-addable), so this is reversible.Test
Sidebar.test.tsx(new): opens the row kebab and clicks Remove project, assertingonRemoveProjectis called with the project id. (Router hooks stubbed, since the Sidebar reads selection from the router.)npm run typecheckclean; prettier clean; the renderer vitest suite passes.Notes