From 415aecc0dd07c90825f31635d0df5497c41309bd Mon Sep 17 00:00:00 2001 From: Developer Date: Wed, 15 Jul 2026 10:20:20 +0000 Subject: [PATCH] fix: constrain dialog content to the viewport - Add dynamic viewport bounds and scrollable body regions to dialogs and modals\n- Make tool-launch popups use the shared scrollable body pattern\n- Keep mobile sheets, action sheets, and notification popups scroll-contained\n- Restore the ProjectsPage test setup required for the frontend suite\n\nOpenSpec: fix-dialog-scroll-containment\nQuality gates: npm run typecheck, npm run lint, npm test (88 passed), npm run build --- .../features/tool/instance-list.tsx | 30 +++++++----- .../features/tool/start-tool-fab.tsx | 2 + .../workspace/workspace-tools-panel.test.tsx | 47 ++++++++++++++++++ .../workspace/workspace-tools-panel.tsx | 22 +++++---- apps/web/src/pages/ProjectsPage.test.tsx | 49 +++++++++---------- apps/web/src/styles/global.css | 26 +++++----- apps/web/src/styles/utilities.css | 10 ++++ .../fix-dialog-scroll-containment/change.md | 28 +++++++++++ .../fix-dialog-scroll-containment/tasks.md | 8 +++ 9 files changed, 160 insertions(+), 62 deletions(-) create mode 100644 apps/web/src/components/features/workspace/workspace-tools-panel.test.tsx create mode 100644 openspec/changes/fix-dialog-scroll-containment/change.md create mode 100644 openspec/changes/fix-dialog-scroll-containment/tasks.md diff --git a/apps/web/src/components/features/tool/instance-list.tsx b/apps/web/src/components/features/tool/instance-list.tsx index 9ead1ba..642c873 100644 --- a/apps/web/src/components/features/tool/instance-list.tsx +++ b/apps/web/src/components/features/tool/instance-list.tsx @@ -578,19 +578,23 @@ export const InstanceList = ({ {showCreate && (
-

Launch Tool

- setShowCreate(false)} - submitLabel="Launch" - /> +
+

Launch Tool

+
+
+ setShowCreate(false)} + submitLabel="Launch" + /> +
)} diff --git a/apps/web/src/components/features/tool/start-tool-fab.tsx b/apps/web/src/components/features/tool/start-tool-fab.tsx index f28521a..045321c 100644 --- a/apps/web/src/components/features/tool/start-tool-fab.tsx +++ b/apps/web/src/components/features/tool/start-tool-fab.tsx @@ -62,6 +62,7 @@ export function StartToolFAB() { +
{workspacesLoading ? (

Loading workspaces...

) : workspaces.length === 0 ? ( @@ -110,6 +111,7 @@ export function StartToolFAB() { /> )} +
)} diff --git a/apps/web/src/components/features/workspace/workspace-tools-panel.test.tsx b/apps/web/src/components/features/workspace/workspace-tools-panel.test.tsx new file mode 100644 index 0000000..3672675 --- /dev/null +++ b/apps/web/src/components/features/workspace/workspace-tools-panel.test.tsx @@ -0,0 +1,47 @@ +import "@testing-library/jest-dom/vitest"; +import { fireEvent, render, screen } from "@testing-library/react"; +import { describe, expect, it, vi } from "vitest"; + +import { WorkspaceToolsPanel } from "./workspace-tools-panel"; +import type { Workspace } from "../../../types/workspace"; + +vi.mock("../../../hooks/use-workspace-instances", () => ({ + useWorkspaceInstances: () => ({ + instances: [], + loading: false, + refresh: vi.fn(), + }), +})); + +vi.mock("../tool/tool-starter", () => ({ + ToolStarter: () =>
, +})); + +const workspace: Workspace = { + id: "workspace-1", + name: "Main", + repo_id: "repo-1", + repo_name: "repository", + repo_ssh_key_id: null, + project_id: "project-1", + project_name: "Project", + user_id: "user-1", + branch: "main", + path: "/workspace", + status: "ready", + last_sync_at: null, + created_at: "2026-01-01T00:00:00Z", + updated_at: "2026-01-01T00:00:00Z", + instance_count: 0, +}; + +describe("WorkspaceToolsPanel", () => { + it("places the tool launcher inside the shared scrollable dialog body", () => { + const { container } = render(); + + fireEvent.click(screen.getByRole("button", { name: "Start Tool" })); + + const dialogBody = container.querySelector(".dialog-body"); + expect(dialogBody).toContainElement(screen.getByTestId("tool-starter")); + }); +}); diff --git a/apps/web/src/components/features/workspace/workspace-tools-panel.tsx b/apps/web/src/components/features/workspace/workspace-tools-panel.tsx index 9af6817..4135138 100644 --- a/apps/web/src/components/features/workspace/workspace-tools-panel.tsx +++ b/apps/web/src/components/features/workspace/workspace-tools-panel.tsx @@ -64,15 +64,19 @@ export function WorkspaceToolsPanel({ workspace }: WorkspaceToolsPanelProps) { {showModal && (
setShowModal(false)}>
e.stopPropagation()}> -

Start Tool

- { - setShowModal(false); - void refresh(); - }} - onCancel={() => setShowModal(false)} - /> +
+

Start Tool

+
+
+ { + setShowModal(false); + void refresh(); + }} + onCancel={() => setShowModal(false)} + /> +
)} diff --git a/apps/web/src/pages/ProjectsPage.test.tsx b/apps/web/src/pages/ProjectsPage.test.tsx index dc51c46..6f94793 100644 --- a/apps/web/src/pages/ProjectsPage.test.tsx +++ b/apps/web/src/pages/ProjectsPage.test.tsx @@ -1,17 +1,26 @@ +import "@testing-library/jest-dom/vitest"; import { cleanup, fireEvent, render, screen, waitFor, within } from "@testing-library/react"; import { MemoryRouter } from "react-router-dom"; import { afterEach, describe, expect, it, vi } from "vitest"; import { ProjectsPage } from "./ProjectsPage"; import * as projectsApi from "../api/projects"; +import { SessionsProvider } from "../state/sessions"; +import type { ProjectWithRepos } from "../types"; -const mockProjects = [ +vi.mock("../api/sessions", () => ({ + getUserSessions: vi.fn().mockResolvedValue([]), +})); + +const mockProjects: ProjectWithRepos[] = [ { id: "proj-1", name: "Alpha Project", description: "First project", owner_id: "user-1", default_ssh_key_id: null, + repositories: [], + created_at: "2026-07-01T00:00:00Z", }, { id: "proj-2", @@ -19,6 +28,8 @@ const mockProjects = [ description: null, owner_id: "user-1", default_ssh_key_id: null, + repositories: [], + created_at: "2026-07-01T00:00:00Z", }, ]; @@ -31,9 +42,7 @@ describe("ProjectsPage", () => { it("renders loading state initially", () => { vi.spyOn(projectsApi, "listProjects").mockImplementation(() => new Promise(() => {})); render( - - - + ); expect(screen.getByText(/loading projects/i)).toBeInTheDocument(); }); @@ -41,9 +50,7 @@ describe("ProjectsPage", () => { it("renders project list after loading", async () => { vi.spyOn(projectsApi, "listProjects").mockResolvedValue(mockProjects); render( - - - + ); await waitFor(() => { @@ -56,9 +63,7 @@ describe("ProjectsPage", () => { it("renders empty state when no projects", async () => { vi.spyOn(projectsApi, "listProjects").mockResolvedValue([]); render( - - - + ); await waitFor(() => { @@ -69,9 +74,7 @@ describe("ProjectsPage", () => { it("renders error state with retry button", async () => { vi.spyOn(projectsApi, "listProjects").mockRejectedValue(new Error("fail")); render( - - - + ); await waitFor(() => { @@ -85,9 +88,7 @@ describe("ProjectsPage", () => { const createMock = vi.spyOn(projectsApi, "createProject").mockResolvedValue(mockProjects[0]); render( - - - + ); await waitFor(() => { @@ -118,9 +119,7 @@ describe("ProjectsPage", () => { vi.spyOn(projectsApi, "listProjects").mockResolvedValue([]); render( - - - + ); await waitFor(() => { @@ -138,16 +137,14 @@ describe("ProjectsPage", () => { const updateMock = vi.spyOn(projectsApi, "updateProject").mockResolvedValue(mockProjects[0]); render( - - - + ); await waitFor(() => { expect(screen.getByText("Alpha Project")).toBeInTheDocument(); }); - const alphaCard = screen.getByText("Alpha Project").closest(".project-card") as HTMLElement | null; + const alphaCard = screen.getByText("Alpha Project").closest(".project-list-item") as HTMLElement | null; if (!alphaCard) throw new Error("Card not found"); fireEvent.click(within(alphaCard).getByRole("button", { name: /edit/i })); @@ -171,16 +168,14 @@ describe("ProjectsPage", () => { const deleteMock = vi.spyOn(projectsApi, "deleteProject").mockResolvedValue(undefined); render( - - - + ); await waitFor(() => { expect(screen.getByText("Alpha Project")).toBeInTheDocument(); }); - const alphaCard = screen.getByText("Alpha Project").closest(".project-card") as HTMLElement | null; + const alphaCard = screen.getByText("Alpha Project").closest(".project-list-item") as HTMLElement | null; if (!alphaCard) throw new Error("Card not found"); fireEvent.click(within(alphaCard).getByRole("button", { name: /delete/i })); diff --git a/apps/web/src/styles/global.css b/apps/web/src/styles/global.css index 905a0d2..ca6ecd1 100644 --- a/apps/web/src/styles/global.css +++ b/apps/web/src/styles/global.css @@ -453,12 +453,16 @@ backdrop-filter: blur(2px); } -.dialog { +.dialog, +.modal-content, +.commit-dialog { width: 100%; max-width: 32rem; max-height: calc(100vh - var(--space-8)); + max-height: calc(100dvh - var(--space-8)); display: flex; flex-direction: column; + min-height: 0; overflow: hidden; background: var(--panel); border: 1px solid var(--border); @@ -468,16 +472,6 @@ .modal-content { /* deprecated alias */ - width: 100%; - max-width: 32rem; - max-height: calc(100vh - var(--space-8)); - display: flex; - flex-direction: column; - overflow: hidden; - background: var(--panel); - border: 1px solid var(--border); - border-radius: var(--radius-lg); - box-shadow: var(--shadow-xl); } .dialog-lg { @@ -500,9 +494,13 @@ line-height: var(--line-height-tight); } -.dialog-body { +.dialog-body, +.modal-body { flex: 1; + min-height: 0; overflow: auto; + overscroll-behavior: contain; + -webkit-overflow-scrolling: touch; padding: var(--space-4); } @@ -543,9 +541,11 @@ } .dialog, - .modal-content { + .modal-content, + .commit-dialog { max-width: 100%; max-height: calc(100vh - var(--space-6)); + max-height: calc(100dvh - var(--space-6)); border-radius: var(--radius-lg) var(--radius-lg) 0 0; } } diff --git a/apps/web/src/styles/utilities.css b/apps/web/src/styles/utilities.css index 87f7faa..32f2972 100644 --- a/apps/web/src/styles/utilities.css +++ b/apps/web/src/styles/utilities.css @@ -2328,8 +2328,10 @@ a.nav-item, width: 100%; max-width: 600px; max-height: 70vh; + max-height: 70dvh; display: flex; flex-direction: column; + min-height: 0; animation: slide-up 0.2s ease-out; } @@ -2365,8 +2367,12 @@ a.nav-item, } .mobile-bottom-sheet-content { + flex: 1; + min-height: 0; padding: 8px 0; overflow-y: auto; + overscroll-behavior: contain; + -webkit-overflow-scrolling: touch; } .mobile-bottom-sheet-item { @@ -3240,7 +3246,10 @@ a:active, width: 100%; max-width: 500px; max-height: 80vh; + max-height: 80dvh; overflow-y: auto; + overscroll-behavior: contain; + -webkit-overflow-scrolling: touch; animation: slideUp 0.3s ease; padding-bottom: env(safe-area-inset-bottom, 0); } @@ -3602,6 +3611,7 @@ a:active, max-width: none; border-radius: 12px; max-height: 70vh; + max-height: 70dvh; } } diff --git a/openspec/changes/fix-dialog-scroll-containment/change.md b/openspec/changes/fix-dialog-scroll-containment/change.md new file mode 100644 index 0000000..92266c2 --- /dev/null +++ b/openspec/changes/fix-dialog-scroll-containment/change.md @@ -0,0 +1,28 @@ +# Constrain and Scroll Edit Dialogs and Popups + +## Summary + +Ensure every edit dialog, form popup, and modal popup remains usable on a viewport that is shorter than its content. Dialog chrome must stay within the visible viewport while the content area scrolls independently. + +## Problem + +The shared dialog system constrains only components that follow its `dialog-header` / `dialog-body` structure. Several tool-launch and commit dialogs place form content directly inside the container or use a bespoke container, so long forms can be clipped. Existing viewport sizing also relies on `vh`, which is unreliable when mobile browser chrome changes height. + +## Scope + +- Shared dialog and modal CSS in `apps/web/src/styles/global.css`. +- Mobile notification dropdown sizing in `apps/web/src/styles/utilities.css`. +- Tool-launch and commit popup markup that does not currently provide a scrollable content region. + +## Acceptance Criteria + +- [ ] Dialogs and modal popups are bounded by the current visible viewport, including mobile dynamic viewport changes. +- [ ] Headers and footer/action bars remain visible while long form content scrolls independently. +- [ ] Tool-launch and commit popups use the shared scrollable content pattern. +- [ ] Mobile sheets, action sheets, and notification dropdowns remain scrollable without propagating scroll gestures to the page. +- [ ] Relevant frontend tests, typecheck, lint, and production build pass. + +## Non-Goals + +- Redesigning dialog visuals or interaction flows. +- Changing page-level scrolling outside overlays. diff --git a/openspec/changes/fix-dialog-scroll-containment/tasks.md b/openspec/changes/fix-dialog-scroll-containment/tasks.md new file mode 100644 index 0000000..a6e9a6f --- /dev/null +++ b/openspec/changes/fix-dialog-scroll-containment/tasks.md @@ -0,0 +1,8 @@ +# Constrain and Scroll Edit Dialogs and Popups — Tasks + +- [x] Audit all dialog, modal, sheet, action-sheet, and popup implementations. +- [x] Strengthen shared dialog/modal viewport and body scrolling rules. +- [x] Update bespoke tool-launch and commit popups to use scrollable content regions. +- [x] Add focused coverage for the shared scrollable dialog-body markup. +- [x] Run frontend typecheck, lint, tests (88 passed), and production build. +- [x] Update project maps for changed source files.