From e45d050dadcf065e4a9dcad559cecc45cfc47c06 Mon Sep 17 00:00:00 2001 From: MisbahAnsar Date: Sun, 24 May 2026 19:10:55 +0530 Subject: [PATCH 1/2] fixed and added modal --- packages/web/src/app/globals.css | 20 ++++ .../web/src/components/ProjectSidebar.tsx | 41 ++++++-- .../components/RemoveProjectConfirmModal.tsx | 95 +++++++++++++++++++ .../__tests__/ProjectSidebar.test.tsx | 59 +++++++++++- 4 files changed, 201 insertions(+), 14 deletions(-) create mode 100644 packages/web/src/components/RemoveProjectConfirmModal.tsx diff --git a/packages/web/src/app/globals.css b/packages/web/src/app/globals.css index 1e0d69f91e..306c03b779 100644 --- a/packages/web/src/app/globals.css +++ b/packages/web/src/app/globals.css @@ -4591,6 +4591,26 @@ html.light .xterm .xterm-viewport:hover::-webkit-scrollbar-thumb:active { box-shadow: 0 24px 64px rgb(0 0 0 / 18%); } +.project-settings-modal--confirm { + width: min(480px, calc(100vw - 48px)); + max-height: none; +} + +.project-settings-modal__confirm-body { + margin: 0; + padding: 0 20px 18px; + color: var(--color-text-secondary); + font-size: 13px; + line-height: 1.5; +} + +.project-settings-modal__confirm-actions { + display: flex; + justify-content: flex-end; + gap: 8px; + padding: 0 20px 20px; +} + .project-settings-modal__header { display: flex; align-items: flex-start; diff --git a/packages/web/src/components/ProjectSidebar.tsx b/packages/web/src/components/ProjectSidebar.tsx index 24c21691fa..6f03aa8c9f 100644 --- a/packages/web/src/components/ProjectSidebar.tsx +++ b/packages/web/src/components/ProjectSidebar.tsx @@ -13,6 +13,8 @@ import { projectDashboardPath, projectReviewPath, projectSessionPath } from "@/l import { ThemeToggle } from "./ThemeToggle"; import { AddProjectModal } from "./AddProjectModal"; import { ProjectSettingsModal } from "./ProjectSettingsModal"; +import { RemoveProjectConfirmModal } from "./RemoveProjectConfirmModal"; +import { ToastProvider, useToast } from "./Toast"; /** Minimal shape needed to render an orchestrator link in the sidebar. */ export interface ProjectSidebarOrchestrator { @@ -110,7 +112,11 @@ export function ProjectSidebar(props: ProjectSidebarProps) { if (props.projects.length === 0) { return ; } - return ; + return ( + + + + ); } interface SessionRowProps { @@ -284,6 +290,7 @@ function ProjectSidebarInner({ onMobileClose, }: ProjectSidebarProps) { const router = useRouter(); + const { showToast } = useToast(); const _isLoading = loading || sessions === null; const [expandedProjects, setExpandedProjects] = useState>( @@ -304,6 +311,7 @@ function ProjectSidebarInner({ const [projectMenuOpenId, setProjectMenuOpenId] = useState(null); const [projectSettingsProjectId, setProjectSettingsProjectId] = useState(null); const [deletingProjectId, setDeletingProjectId] = useState(null); + const [projectPendingRemoval, setProjectPendingRemoval] = useState(null); const [removedProjectIds, setRemovedProjectIds] = useState>(new Set()); const [addProjectOpen, setAddProjectOpen] = useState(false); const settingsRef = useRef(null); @@ -565,11 +573,19 @@ function ProjectSidebarInner({ }); }; - const handleRemoveProject = async (project: ProjectInfo) => { - const confirmed = window.confirm( - `Remove project ${project.name} from AO? This clears its AO sessions/history and removes it from the portfolio, but keeps the repository folder on disk.`, - ); - if (!confirmed) return; + const requestRemoveProject = (project: ProjectInfo) => { + setProjectMenuOpenId(null); + setProjectPendingRemoval(project); + }; + + const cancelRemoveProject = () => { + if (deletingProjectId) return; + setProjectPendingRemoval(null); + }; + + const confirmRemoveProject = async () => { + const project = projectPendingRemoval; + if (!project) return; setDeletingProjectId(project.id); try { @@ -591,7 +607,7 @@ function ProjectSidebarInner({ next.delete(project.id); return next; }); - setProjectMenuOpenId(null); + setProjectPendingRemoval(null); if (activeProjectId === project.id) { router.push("/"); } else if ("refresh" in router && typeof router.refresh === "function") { @@ -599,7 +615,8 @@ function ProjectSidebarInner({ } onMobileClose?.(); } catch (error) { - window.alert(error instanceof Error ? error.message : "Failed to remove project."); + const message = error instanceof Error ? error.message : "Failed to remove project."; + showToast(message, "error"); } finally { setDeletingProjectId(null); } @@ -972,7 +989,7 @@ function ProjectSidebarInner({ type="button" className="project-sidebar__proj-menu-item project-sidebar__proj-menu-item--danger" role="menuitem" - onClick={() => void handleRemoveProject(project)} + onClick={() => requestRemoveProject(project)} disabled={deletingProjectId === project.id} > {deletingProjectId === project.id ? "Removing..." : "Remove project"} @@ -1185,6 +1202,12 @@ function ProjectSidebarInner({ projectId={projectSettingsProjectId} onClose={() => setProjectSettingsProjectId(null)} /> + void confirmRemoveProject()} + /> ); } diff --git a/packages/web/src/components/RemoveProjectConfirmModal.tsx b/packages/web/src/components/RemoveProjectConfirmModal.tsx new file mode 100644 index 0000000000..36cf15acf1 --- /dev/null +++ b/packages/web/src/components/RemoveProjectConfirmModal.tsx @@ -0,0 +1,95 @@ +"use client"; + +import { useEffect, useRef } from "react"; +import type { ProjectInfo } from "@/lib/project-name"; + +export const REMOVE_PROJECT_CONFIRM_MESSAGE = + "This clears its AO sessions/history and removes it from the portfolio, but keeps the repository folder on disk."; + +interface RemoveProjectConfirmModalProps { + project: ProjectInfo | null; + busy: boolean; + onCancel: () => void; + onConfirm: () => void; +} + +export function RemoveProjectConfirmModal({ + project, + busy, + onCancel, + onConfirm, +}: RemoveProjectConfirmModalProps) { + const modalRef = useRef(null); + + useEffect(() => { + if (!project) return; + modalRef.current?.focus(); + }, [project]); + + useEffect(() => { + if (!project) return; + const handleKeyDown = (event: KeyboardEvent) => { + if (event.key === "Escape" && !busy) { + event.preventDefault(); + onCancel(); + } + }; + document.addEventListener("keydown", handleKeyDown); + return () => document.removeEventListener("keydown", handleKeyDown); + }, [project, busy, onCancel]); + + if (!project) return null; + + return ( +
+
event.stopPropagation()} + > +
+
+

Remove project

+

+ Remove {project.name}? +

+
+ +
+ +

{REMOVE_PROJECT_CONFIRM_MESSAGE}

+ +
+ + +
+
+
+ ); +} diff --git a/packages/web/src/components/__tests__/ProjectSidebar.test.tsx b/packages/web/src/components/__tests__/ProjectSidebar.test.tsx index 6f7150c374..79a5e7f9d9 100644 --- a/packages/web/src/components/__tests__/ProjectSidebar.test.tsx +++ b/packages/web/src/components/__tests__/ProjectSidebar.test.tsx @@ -1,5 +1,5 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; -import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { fireEvent, render, screen, waitFor, within } from "@testing-library/react"; import { ProjectSidebar } from "@/components/ProjectSidebar"; import { makePR, makeSession } from "@/__tests__/helpers"; @@ -263,10 +263,6 @@ describe("ProjectSidebar", () => { json: async () => ({ ok: true }), }); vi.stubGlobal("fetch", fetchMock); - vi.stubGlobal( - "confirm", - vi.fn(() => true), - ); render( { fireEvent.click(screen.getByRole("button", { name: /Project actions for Project Two/i })); fireEvent.click(await screen.findByRole("menuitem", { name: "Remove project" })); + const dialog = await screen.findByRole("dialog", { name: /Remove project/i }); + fireEvent.click(within(dialog).getByRole("button", { name: "Remove from AO" })); + await waitFor(() => { expect(fetchMock).toHaveBeenCalledWith("/api/projects/project-2", { method: "DELETE" }); expect(mockRefresh).toHaveBeenCalled(); @@ -287,6 +286,56 @@ describe("ProjectSidebar", () => { }); }); + it("cancels remove project without calling the API", async () => { + const fetchMock = vi.fn(); + vi.stubGlobal("fetch", fetchMock); + + render( + , + ); + + fireEvent.click(screen.getByRole("button", { name: /Project actions for Project Two/i })); + fireEvent.click(await screen.findByRole("menuitem", { name: "Remove project" })); + + const dialog = await screen.findByRole("dialog", { name: /Remove project/i }); + fireEvent.click(within(dialog).getByRole("button", { name: "Cancel" })); + + expect(screen.queryByRole("dialog", { name: /Remove project/i })).not.toBeInTheDocument(); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it("shows a toast when remove project fails", async () => { + const fetchMock = vi.fn().mockResolvedValue({ + ok: false, + json: async () => ({ error: "Cannot remove active project." }), + }); + vi.stubGlobal("fetch", fetchMock); + + render( + , + ); + + fireEvent.click(screen.getByRole("button", { name: /Project actions for Project Two/i })); + fireEvent.click(await screen.findByRole("menuitem", { name: "Remove project" })); + + const dialog = await screen.findByRole("dialog", { name: /Remove project/i }); + fireEvent.click(within(dialog).getByRole("button", { name: "Remove from AO" })); + + expect(await screen.findByText("Cannot remove active project.")).toBeInTheDocument(); + expect(fetchMock).toHaveBeenCalledWith("/api/projects/project-2", { method: "DELETE" }); + expect(screen.getByRole("button", { name: /^Project Two 0$/ })).toBeInTheDocument(); + }); + it("shows non-done worker sessions for the expanded active project", () => { render( Date: Sun, 24 May 2026 19:48:12 +0530 Subject: [PATCH 2/2] fixed all the issues --- .../web/src/components/ProjectSidebar.tsx | 18 +++-- .../components/RemoveProjectConfirmModal.tsx | 36 +++++++--- .../RemoveProjectConfirmModal.test.tsx | 66 +++++++++++++++++++ 3 files changed, 105 insertions(+), 15 deletions(-) create mode 100644 packages/web/src/components/__tests__/RemoveProjectConfirmModal.test.tsx diff --git a/packages/web/src/components/ProjectSidebar.tsx b/packages/web/src/components/ProjectSidebar.tsx index 6f03aa8c9f..1cf2439a16 100644 --- a/packages/web/src/components/ProjectSidebar.tsx +++ b/packages/web/src/components/ProjectSidebar.tsx @@ -573,17 +573,17 @@ function ProjectSidebarInner({ }); }; - const requestRemoveProject = (project: ProjectInfo) => { + const requestRemoveProject = useCallback((project: ProjectInfo) => { setProjectMenuOpenId(null); setProjectPendingRemoval(project); - }; + }, []); - const cancelRemoveProject = () => { + const cancelRemoveProject = useCallback(() => { if (deletingProjectId) return; setProjectPendingRemoval(null); - }; + }, [deletingProjectId]); - const confirmRemoveProject = async () => { + const confirmRemoveProject = useCallback(async () => { const project = projectPendingRemoval; if (!project) return; @@ -620,7 +620,11 @@ function ProjectSidebarInner({ } finally { setDeletingProjectId(null); } - }; + }, [activeProjectId, onMobileClose, projectPendingRemoval, router, showToast]); + + const handleConfirmRemoveProject = useCallback(() => { + void confirmRemoveProject(); + }, [confirmRemoveProject]); if (collapsed) { return ( @@ -1206,7 +1210,7 @@ function ProjectSidebarInner({ project={projectPendingRemoval} busy={projectPendingRemoval !== null && deletingProjectId === projectPendingRemoval.id} onCancel={cancelRemoveProject} - onConfirm={() => void confirmRemoveProject()} + onConfirm={handleConfirmRemoveProject} /> ); diff --git a/packages/web/src/components/RemoveProjectConfirmModal.tsx b/packages/web/src/components/RemoveProjectConfirmModal.tsx index 36cf15acf1..9ecea5588b 100644 --- a/packages/web/src/components/RemoveProjectConfirmModal.tsx +++ b/packages/web/src/components/RemoveProjectConfirmModal.tsx @@ -6,6 +6,9 @@ import type { ProjectInfo } from "@/lib/project-name"; export const REMOVE_PROJECT_CONFIRM_MESSAGE = "This clears its AO sessions/history and removes it from the portfolio, but keeps the repository folder on disk."; +const FOCUSABLE_SELECTOR = + 'button, [href], input, select, textarea, [tabindex]:not([tabindex="-1"])'; + interface RemoveProjectConfirmModalProps { project: ProjectInfo | null; busy: boolean; @@ -23,19 +26,36 @@ export function RemoveProjectConfirmModal({ useEffect(() => { if (!project) return; - modalRef.current?.focus(); - }, [project]); + const modal = modalRef.current; + if (!modal) return; - useEffect(() => { - if (!project) return; - const handleKeyDown = (event: KeyboardEvent) => { + const focusable = modal.querySelectorAll(FOCUSABLE_SELECTOR); + if (focusable.length === 0) return; + + const first = focusable[0]; + const last = focusable[focusable.length - 1]; + first.focus(); + + function handleKeyDown(event: KeyboardEvent) { if (event.key === "Escape" && !busy) { event.preventDefault(); onCancel(); + return; + } + + if (event.key === "Tab") { + if (event.shiftKey && document.activeElement === first) { + event.preventDefault(); + last.focus(); + } else if (!event.shiftKey && document.activeElement === last) { + event.preventDefault(); + first.focus(); + } } - }; - document.addEventListener("keydown", handleKeyDown); - return () => document.removeEventListener("keydown", handleKeyDown); + } + + modal.addEventListener("keydown", handleKeyDown); + return () => modal.removeEventListener("keydown", handleKeyDown); }, [project, busy, onCancel]); if (!project) return null; diff --git a/packages/web/src/components/__tests__/RemoveProjectConfirmModal.test.tsx b/packages/web/src/components/__tests__/RemoveProjectConfirmModal.test.tsx new file mode 100644 index 0000000000..595af947b2 --- /dev/null +++ b/packages/web/src/components/__tests__/RemoveProjectConfirmModal.test.tsx @@ -0,0 +1,66 @@ +import { describe, expect, it, vi } from "vitest"; +import { fireEvent, render, screen } from "@testing-library/react"; +import { RemoveProjectConfirmModal } from "../RemoveProjectConfirmModal"; + +const project = { id: "project-1", name: "Project One", sessionPrefix: "project-1" }; + +describe("RemoveProjectConfirmModal", () => { + it("traps Tab focus within the dialog", () => { + render( + , + ); + + const dialog = screen.getByRole("dialog", { name: /Remove Project One/i }); + const close = screen.getByRole("button", { name: "Close" }); + const remove = screen.getByRole("button", { name: "Remove from AO" }); + + remove.focus(); + fireEvent.keyDown(dialog, { key: "Tab" }); + expect(document.activeElement).toBe(close); + + close.focus(); + fireEvent.keyDown(dialog, { key: "Tab", shiftKey: true }); + expect(document.activeElement).toBe(remove); + }); + + it("calls onCancel when Escape is pressed", () => { + const onCancel = vi.fn(); + render( + , + ); + + fireEvent.keyDown(screen.getByRole("dialog", { name: /Remove Project One/i }), { + key: "Escape", + }); + + expect(onCancel).toHaveBeenCalledTimes(1); + }); + + it("does not call onCancel on Escape while busy", () => { + const onCancel = vi.fn(); + render( + , + ); + + fireEvent.keyDown(screen.getByRole("dialog", { name: /Remove Project One/i }), { + key: "Escape", + }); + + expect(onCancel).not.toHaveBeenCalled(); + }); +});