diff --git a/CLAUDE.md b/CLAUDE.md index 096ca0ff7..7e6c9d601 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -117,7 +117,10 @@ make vet # go vet ### End-to-End Tests -**E2E tests are non-negotiable.** Every major feature, bug fix, and refactor must include e2e tests that exercise the full stack (HTTP API with real SQLite). Even small changes merit e2e coverage when they touch API behavior, data flow between layers, or anything a user would notice if it broke. When in doubt, write the e2e test — the cost of a missing one is always higher than the cost of writing it. +Coverage of real behavior is non-negotiable; the lane is chosen to avoid the one expensive cost — the **browser**, not the Go backend. Two independent axes: + +- **Browser only when you must.** Reserve Playwright for real rendering/layout: screenshots/video, `getBoundingClientRect`, scroll/sticky/overflow geometry, container queries, pointer drag, viewport emulation, canvas/xterm, computed CSS pixels. Everything else runs in **Vitest + jsdom** (`vp test`) — mount the real `App.svelte` via `frontend/src/test/appHarness.ts`. Mounting the whole app or using routing is not a reason to stay in Playwright. +- **Real Go backend by default; mocking is the exception.** Backend-dependent behavior (sync, persistence, capabilities, normalization, wire shape) must hit real Go (`frontend/tests/e2e-full/` or `internal/server/` Go tests) — don't assert backend-computed values through a hand-written fixture. Mock the API (`frontend/src/test/mockApiFetch.ts`, never fork the Playwright copy) only for the rare scenario the seeded server can't produce. ### Test Guidelines @@ -168,7 +171,7 @@ make vet # go vet - Use conventional commit messages whose subject explains the reason or user-visible outcome, not just the mechanical change. Good subjects answer "why does this commit exist?" (for example, `fix: restore workspace activity for launched agents`), while vague mechanics such as `fix: run agents under tmux` are not acceptable on their own - Commit bodies must add any important context about the bug, regression, constraint, or tradeoff that motivated the change; do not rely on the diff to explain intent - Run tests before committing when applicable -- Before pushing any frontend change, you must have run the full affected Playwright e2e suite locally after the final frontend/test edit; focused component tests, type checks, and CI-only verification are not enough. +- Before pushing any frontend change, you must have run the full affected suite locally after the final frontend/test edit — the full `vp test` Vitest run, plus the full affected Playwright e2e suite whenever the change touches Playwright specs or the shared mock fixtures they rely on; type checks and CI-only verification are not enough. - Never push new workstreams unless explicitly asked. When addressing review feedback or CI failures on an existing PR, an agent may push after the fix is implemented and relevant local validation has run. ## Pull Requests diff --git a/frontend/src/App.activity-collapse.test.ts b/frontend/src/App.activity-collapse.test.ts new file mode 100644 index 000000000..3762a3c5d --- /dev/null +++ b/frontend/src/App.activity-collapse.test.ts @@ -0,0 +1,215 @@ +// Threaded activity collapse behavior exercised through the real app shell +// with the API mocked at the fetch boundary. + +import { cleanup, screen, waitFor } from "@testing-library/svelte"; +import { fireEvent } from "@testing-library/svelte"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vite-plus/test"; + +import { installAppDomGlobals, mountApp, resetKeyboardModuleState, type MountedApp } from "./test/appHarness.js"; +import { jsonResponse, type MockRouteOverride } from "./test/mockApiFetch.js"; + +function event(id: string, number: number, type: string, created: string): unknown { + return { + id, + cursor: id, + activity_type: type, + author: "marius", + body_preview: "", + created_at: created, + item_number: number, + item_state: "open", + item_title: number === 42 ? "Add browser regression coverage" : "Refactor theme system", + item_type: "pr", + item_url: `https://github.com/acme/widgets/pull/${number}`, + platform_host: "github.com", + repo_owner: "acme", + repo_name: "widgets", + repo: { + provider: "github", + platform_host: "github.com", + owner: "acme", + name: "widgets", + repo_path: "acme/widgets", + capabilities: {}, + }, + }; +} + +function activitySettings(viewMode: "flat" | "threaded"): MockRouteOverride { + return (req) => { + if (req.method !== "GET" || req.url.pathname !== "/api/v1/settings") return null; + return jsonResponse({ + repos: [ + { + provider: "github", + platform_host: "github.com", + owner: "acme", + name: "widgets", + repo_path: "acme/widgets", + is_glob: false, + matched_repo_count: 1, + }, + ], + activity: { + view_mode: viewMode, + time_range: "7d", + hide_closed: false, + hide_bots: false, + collapse_threads: false, + }, + terminal: { + font_family: "", + font_size: 14, + scrollback: 1000, + line_height: 1, + letter_spacing: 0, + cursor_blink: true, + font_ligatures: false, + renderer: "xterm", + }, + agents: [], + }); + }; +} + +function activityItems(items: unknown[]): MockRouteOverride { + return (req) => { + if (req.method !== "GET" || req.url.pathname !== "/api/v1/activity") return null; + return jsonResponse({ capped: false, items }); + }; +} + +const defaultEvents = [ + event("a1", 42, "comment", "2026-03-30T14:00:00Z"), + event("a2", 42, "review", "2026-03-30T13:00:00Z"), + event("b1", 55, "comment", "2026-03-30T12:00:00Z"), +]; + +async function mountThreadedActivity(): Promise { + const app = await mountApp("/?view=threaded", { + overrides: [activitySettings("threaded"), activityItems(defaultEvents)], + }); + await waitFor(() => expect(itemRows()).toHaveLength(2)); + return app; +} + +function itemRows(): Element[] { + return Array.from(document.querySelectorAll(".item-row")); +} + +function eventRows(): Element[] { + return Array.from(document.querySelectorAll(".event-row")); +} + +describe("threaded activity collapse", () => { + vi.setConfig({ testTimeout: 20_000 }); + + beforeEach(() => { + installAppDomGlobals(); + }); + + afterEach(async () => { + cleanup(); + vi.unstubAllGlobals(); + localStorage.clear(); + await resetKeyboardModuleState(); + }); + + it("collapses, drills into one item, and persists across reload", async () => { + const app = await mountThreadedActivity(); + expect(eventRows().length).toBeGreaterThan(0); + + await fireEvent.click(screen.getByRole("button", { name: "Collapse all" })); + await waitFor(() => expect(eventRows()).toHaveLength(0)); + expect(itemRows()).toHaveLength(2); + + // Drill into a single item via its caret. + await fireEvent.click(itemRows()[0]!.querySelector(".thread-caret")!); + await waitFor(() => expect(eventRows().length).toBeGreaterThan(0)); + + // Collapse-all wrote ?collapsed=1; a reload (remount at the current + // URL) restores the collapsed state and clears the session-only + // single-item override. + expect(window.location.search).toContain("collapsed=1"); + app.unmount(); + + await mountApp(window.location.pathname + window.location.search, { + overrides: [activitySettings("threaded"), activityItems(defaultEvents)], + }); + await waitFor(() => expect(itemRows()).toHaveLength(2)); + expect(eventRows()).toHaveLength(0); + }); + + it("collapse control works while the side detail pane is open", async () => { + await mountThreadedActivity(); + + // Open a detail by clicking the item row body (not the caret). + await fireEvent.click(itemRows()[0]!.querySelector(".item-title")!); + await waitFor(() => expect(document.querySelector(".activity-detail")).not.toBeNull()); + expect(document.querySelector(".activity-pane")).not.toBeNull(); + + // Opening the side pane switches the feed into compact mode, where the + // control keeps its accessible name (CSS hides only the text label — + // that visual hiding is asserted in the browser lane, see + // tests/e2e/activity-collapse-compact-label.spec.ts). Here we verify the + // behavior jsdom can see: compact mode is active and the control still + // collapses the threads. + expect(document.querySelector(".activity-feed--compact")).not.toBeNull(); + const collapseBtn = screen.getByRole("button", { name: "Collapse all" }); + + await fireEvent.click(collapseBtn); + await waitFor(() => expect(eventRows()).toHaveLength(0)); + }); + + it("expand all restores every item's events", async () => { + await mountThreadedActivity(); + const initialCount = eventRows().length; + expect(initialCount).toBeGreaterThan(0); + + await fireEvent.click(screen.getByRole("button", { name: "Collapse all" })); + await waitFor(() => expect(eventRows()).toHaveLength(0)); + + // The control flips to Expand all; clicking it brings every event back. + await fireEvent.click(screen.getByRole("button", { name: "Expand all" })); + await waitFor(() => expect(eventRows()).toHaveLength(initialCount)); + }); + + // Starting from flat view mode, the test switches to Threaded through the + // View dropdown before exercising the collapse controls. + it("switches to threaded via the View dropdown, then collapse/expand all", async () => { + await mountApp("/", { + overrides: [activitySettings("flat"), activityItems(defaultEvents)], + }); + await waitFor(() => expect(document.querySelector(".activity-table .activity-row")).not.toBeNull()); + + await fireEvent.click(screen.getByRole("button", { name: "View" })); + await fireEvent.click(screen.getByRole("button", { name: "Threaded" })); + await waitFor(() => expect(document.querySelector(".threaded-view .item-row")).not.toBeNull()); + + const initialCount = eventRows().length; + expect(initialCount).toBeGreaterThan(0); + + await fireEvent.click(screen.getByRole("button", { name: "Collapse all" })); + await waitFor(() => expect(eventRows()).toHaveLength(0)); + expect(document.querySelector(".threaded-view .item-row")).not.toBeNull(); + + await fireEvent.click(screen.getByRole("button", { name: "Expand all" })); + await waitFor(() => expect(eventRows().length).toBeGreaterThan(0)); + }); + + it("a single caret expands only its own item after collapse all", async () => { + await mountThreadedActivity(); + const fullCount = eventRows().length; + expect(fullCount).toBeGreaterThan(1); + + await fireEvent.click(screen.getByRole("button", { name: "Collapse all" })); + await waitFor(() => expect(eventRows()).toHaveLength(0)); + + await fireEvent.click(itemRows()[0]!.querySelector(".thread-caret")!); + + // Only the clicked item's events reappear; the rest stay collapsed. + await waitFor(() => expect(eventRows().length).toBeGreaterThan(0)); + const partial = eventRows().length; + expect(partial).toBeLessThan(fullCount); + }); +}); diff --git a/frontend/src/App.activity-thread-runs.test.ts b/frontend/src/App.activity-thread-runs.test.ts new file mode 100644 index 000000000..8d446f45e --- /dev/null +++ b/frontend/src/App.activity-thread-runs.test.ts @@ -0,0 +1,154 @@ +// Same-author event runs collapse into a single summary row in threaded +// activity, rendered through the real app shell with the API mocked at the +// fetch boundary. + +import { cleanup, waitFor } from "@testing-library/svelte"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vite-plus/test"; + +import { installAppDomGlobals, mountApp, resetKeyboardModuleState } from "./test/appHarness.js"; +import { jsonResponse, type MockRouteOverride } from "./test/mockApiFetch.js"; + +const REPO = { + provider: "github", + platform_host: "github.com", + owner: "acme", + name: "widgets", + repo_path: "acme/widgets", + capabilities: {}, +}; + +function prEvent(args: { + id: string; + type: "comment" | "review" | "commit" | "force_push" | "new_pr"; + author: string; + createdAt: string; +}): unknown { + return { + id: args.id, + cursor: args.id, + activity_type: args.type, + author: args.author, + body_preview: "", + created_at: args.createdAt, + item_number: 42, + item_state: "open", + item_title: "Add browser regression coverage", + item_type: "pr", + item_url: "https://github.com/acme/widgets/pull/42", + platform_host: "github.com", + repo_owner: "acme", + repo_name: "widgets", + repo: REPO, + }; +} + +const settingsOverride: MockRouteOverride = (req) => { + if (req.method !== "GET" || req.url.pathname !== "/api/v1/settings") return null; + return jsonResponse({ + repos: [ + { + provider: "github", + platform_host: "github.com", + owner: "acme", + name: "widgets", + repo_path: "acme/widgets", + is_glob: false, + matched_repo_count: 1, + }, + ], + activity: { + view_mode: "threaded", + time_range: "7d", + hide_closed: false, + hide_bots: false, + collapse_threads: false, + }, + terminal: { + font_family: "", + font_size: 14, + scrollback: 1000, + line_height: 1, + letter_spacing: 0, + cursor_blink: true, + font_ligatures: false, + renderer: "xterm", + }, + agents: [], + }); +}; + +function activityItems(items: unknown[]): MockRouteOverride { + return (req) => { + if (req.method !== "GET" || req.url.pathname !== "/api/v1/activity") return null; + return jsonResponse({ capped: false, items }); + }; +} + +async function mountThreadedActivity(items: unknown[]): Promise { + await mountApp("/?view=threaded", { + overrides: [settingsOverride, activityItems(items)], + }); + await waitFor(() => expect(document.querySelector(".item-row")).not.toBeNull()); +} + +function collapsedEventRows(): Element[] { + return Array.from(document.querySelectorAll(".event-row.collapsed-event")); +} + +function plainEventRows(): Element[] { + return Array.from(document.querySelectorAll(".event-row:not(.collapsed-event)")); +} + +describe("threaded activity run collapse", () => { + vi.setConfig({ testTimeout: 20_000 }); + + beforeEach(() => { + installAppDomGlobals(); + }); + + afterEach(async () => { + cleanup(); + vi.unstubAllGlobals(); + localStorage.clear(); + await resetKeyboardModuleState(); + }); + + it("collapses a run of three or more reviews from the same author", async () => { + await mountThreadedActivity([ + prEvent({ id: "r5", type: "review", author: "alice", createdAt: "2026-04-27T15:00:00Z" }), + prEvent({ id: "r4", type: "review", author: "alice", createdAt: "2026-04-27T14:00:00Z" }), + prEvent({ id: "r3", type: "review", author: "alice", createdAt: "2026-04-27T13:00:00Z" }), + prEvent({ id: "r2", type: "review", author: "alice", createdAt: "2026-04-27T12:00:00Z" }), + prEvent({ id: "r1", type: "review", author: "alice", createdAt: "2026-04-27T11:00:00Z" }), + ]); + + const collapsed = collapsedEventRows().filter((row) => row.textContent?.includes("5 reviews")); + expect(collapsed).toHaveLength(1); + expect(collapsed[0]!.querySelector(".evt-review")).not.toBeNull(); + expect(plainEventRows()).toHaveLength(0); + }); + + it("collapses comments and reviews into separate runs by event type", async () => { + await mountThreadedActivity([ + prEvent({ id: "c3", type: "comment", author: "alice", createdAt: "2026-04-27T16:00:00Z" }), + prEvent({ id: "c2", type: "comment", author: "alice", createdAt: "2026-04-27T15:00:00Z" }), + prEvent({ id: "c1", type: "comment", author: "alice", createdAt: "2026-04-27T14:00:00Z" }), + prEvent({ id: "r3", type: "review", author: "alice", createdAt: "2026-04-27T13:00:00Z" }), + prEvent({ id: "r2", type: "review", author: "alice", createdAt: "2026-04-27T12:00:00Z" }), + prEvent({ id: "r1", type: "review", author: "alice", createdAt: "2026-04-27T11:00:00Z" }), + ]); + + expect(collapsedEventRows().filter((row) => row.textContent?.includes("3 comments"))).toHaveLength(1); + expect(collapsedEventRows().filter((row) => row.textContent?.includes("3 reviews"))).toHaveLength(1); + }); + + it("leaves short runs of comments unrolled", async () => { + await mountThreadedActivity([ + prEvent({ id: "c2", type: "comment", author: "alice", createdAt: "2026-04-27T13:00:00Z" }), + prEvent({ id: "c1", type: "comment", author: "alice", createdAt: "2026-04-27T12:00:00Z" }), + ]); + + expect(collapsedEventRows()).toHaveLength(0); + expect(plainEventRows()).toHaveLength(2); + }); +}); diff --git a/frontend/src/App.cheatsheet.test.ts b/frontend/src/App.cheatsheet.test.ts new file mode 100644 index 000000000..7e4e7f066 --- /dev/null +++ b/frontend/src/App.cheatsheet.test.ts @@ -0,0 +1,57 @@ +// The ? shortcut opens the cheatsheet through the app shell's global +// keydown handler, view-scoped shortcuts appear under "On this view", and +// Escape closes it. + +import { cleanup, screen, waitFor } from "@testing-library/svelte"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vite-plus/test"; + +import { installAppDomGlobals, mountApp, pressKey, resetKeyboardModuleState } from "./test/appHarness.js"; + +function cheatsheetDialog(): HTMLElement | null { + return screen.queryByRole("dialog", { name: "Keyboard shortcuts" }); +} + +describe("cheatsheet", () => { + vi.setConfig({ testTimeout: 20_000 }); + + beforeEach(() => { + installAppDomGlobals(); + }); + + afterEach(async () => { + cleanup(); + vi.unstubAllGlobals(); + localStorage.clear(); + await resetKeyboardModuleState(); + }); + + it("? opens the cheatsheet and shows j/k under On this view", async () => { + await mountApp("/pulls"); + await waitFor(() => expect(document.querySelector("[data-test='pr-list']")).not.toBeNull()); + + pressKey("?", { shift: true }); + const sheet = await waitFor(() => { + const dialog = cheatsheetDialog(); + expect(dialog).not.toBeNull(); + return dialog!; + }); + + // j and k navigate PRs on /pulls — they should appear under "On this view". + const onThisView = Array.from(sheet.querySelectorAll(".cheatsheet-section")).find((section) => + section.textContent?.includes("On this view"), + ); + expect(onThisView).toBeTruthy(); + expect(onThisView!.textContent).toMatch(/Next pull request|Previous pull request/i); + }); + + it("Escape closes the cheatsheet", async () => { + await mountApp("/pulls"); + await waitFor(() => expect(document.querySelector("[data-test='pr-list']")).not.toBeNull()); + + pressKey("?", { shift: true }); + await waitFor(() => expect(cheatsheetDialog()).not.toBeNull()); + + pressKey("Escape"); + await waitFor(() => expect(cheatsheetDialog()).toBeNull()); + }); +}); diff --git a/frontend/src/App.issue-routing.test.ts b/frontend/src/App.issue-routing.test.ts new file mode 100644 index 000000000..92db4516b --- /dev/null +++ b/frontend/src/App.issue-routing.test.ts @@ -0,0 +1,152 @@ +// Issue detail routes must preserve the platform host in detail requests +// (direct load and popstate), and the detail meta row renders assignees. +// The app is mounted for real with fetch mocked at the network boundary so +// the asserted host is the one the app actually sent. + +import { cleanup, waitFor } from "@testing-library/svelte"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vite-plus/test"; + +import { firePopstate, installAppDomGlobals, mountApp, resetKeyboardModuleState } from "./test/appHarness.js"; +import { jsonResponse, type MockRequest, type MockRouteOverride } from "./test/mockApiFetch.js"; + +const mirrorIssueDetail = { + issue: { + ID: 2, + RepoID: 2, + GitHubID: 202, + Number: 7, + URL: "https://ghe.example.com/acme/widgets/issues/7", + Title: "Mirror host issue", + Author: "marius", + State: "open", + Body: "", + CommentCount: 1, + LabelsJSON: "[]", + CreatedAt: "2026-03-28T14:00:00Z", + UpdatedAt: "2026-03-30T14:00:00Z", + LastActivityAt: "2026-03-30T14:00:00Z", + ClosedAt: null, + Starred: false, + }, + events: [], + platform_host: "ghe.example.com", + repo_owner: "acme", + repo_name: "widgets", + detail_loaded: true, + detail_fetched_at: "2026-03-30T14:00:00Z", +}; + +const assignedIssueDetail = { + issue: { + ...mirrorIssueDetail.issue, + ID: 3, + GitHubID: 303, + Number: 12, + URL: "https://ghe.example.com/acme/widgets/issues/12", + Title: "Issue with assignees", + CommentCount: 0, + assignees: ["alice", "bob"], + }, + events: [], + platform_host: "ghe.example.com", + repo_owner: "acme", + repo_name: "widgets", + detail_loaded: true, + detail_fetched_at: "2026-03-30T14:00:00Z", +}; + +function mirrorIssueRoutes(detail: unknown, number: number): MockRouteOverride { + return (req) => { + if (req.method !== "GET") return null; + const { pathname } = req.url; + const legacy = new RegExp(`^/api/v1/repos/acme/widgets/issues/${number}$`); + const hosted = `/api/v1/host/ghe.example.com/issues/github/acme/widgets/${number}`; + if (legacy.test(pathname) || pathname === hosted) { + return jsonResponse(detail); + } + return null; + }; +} + +/** + * Hosts a request carried for the mirror issue: the provider-aware + * /host/{platform_host}/ path segment or an explicit platform_host query + * param on the legacy repo route. + */ +function seenHosts(requests: MockRequest[], number: number): string[] { + const hosts: string[] = []; + for (const req of requests) { + const { pathname } = req.url; + if (pathname === `/api/v1/host/ghe.example.com/issues/github/acme/widgets/${number}`) { + hosts.push("ghe.example.com"); + } else if (pathname === `/api/v1/repos/acme/widgets/issues/${number}`) { + hosts.push(req.url.searchParams.get("platform_host") ?? ""); + } + } + return hosts; +} + +function detailTitle(): string { + return document.querySelector(".issue-detail .detail-title")?.textContent ?? ""; +} + +describe("issue route platform host", () => { + vi.setConfig({ testTimeout: 20_000 }); + + beforeEach(() => { + installAppDomGlobals(); + }); + + afterEach(async () => { + cleanup(); + vi.unstubAllGlobals(); + localStorage.clear(); + await resetKeyboardModuleState(); + }); + + it("direct issue load preserves platform_host in detail requests", async () => { + const app = await mountApp("/host/ghe.example.com/issues/github/acme/widgets/7", { + overrides: [mirrorIssueRoutes(mirrorIssueDetail, 7)], + }); + + await waitFor(() => expect(detailTitle()).toContain("Mirror host issue")); + expect(seenHosts(app.api.requests, 7)).toContain("ghe.example.com"); + }); + + it("popstate preserves platform_host in detail requests", async () => { + const app = await mountApp("/issues", { + overrides: [mirrorIssueRoutes(mirrorIssueDetail, 7)], + }); + await waitFor(() => expect(document.body.textContent).toContain("Theme toggle does not stick")); + + await firePopstate("/host/ghe.example.com/issues/github/acme/widgets/7"); + + await waitFor(() => expect(detailTitle()).toContain("Mirror host issue")); + expect(seenHosts(app.api.requests, 7)).toContain("ghe.example.com"); + }); +}); + +describe("issue detail assignees", () => { + vi.setConfig({ testTimeout: 20_000 }); + + beforeEach(() => { + installAppDomGlobals(); + }); + + afterEach(async () => { + cleanup(); + vi.unstubAllGlobals(); + localStorage.clear(); + await resetKeyboardModuleState(); + }); + + it("renders assignees in the meta row when present", async () => { + await mountApp("/host/ghe.example.com/issues/github/acme/widgets/12", { + overrides: [mirrorIssueRoutes(assignedIssueDetail, 12)], + }); + + await waitFor(() => expect(detailTitle()).toContain("Issue with assignees")); + const assigneeList = document.querySelector(".issue-detail .meta-row [data-user-list-editor='assignees']"); + expect(assigneeList?.textContent).toContain("alice, bob"); + }); +}); diff --git a/frontend/src/App.keyboard-shortcuts.test.ts b/frontend/src/App.keyboard-shortcuts.test.ts new file mode 100644 index 000000000..adce55f22 --- /dev/null +++ b/frontend/src/App.keyboard-shortcuts.test.ts @@ -0,0 +1,95 @@ +// Global shortcuts served by the keyboard registry — j/k list navigation, +// the Cmd+[ sidebar toggle, and the routes where Cmd+[ is reserved +// (consumed without toggling) because no sidebar target exists. + +import { cleanup, screen, waitFor } from "@testing-library/svelte"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vite-plus/test"; + +import { installAppDomGlobals, mountApp, pressKey, resetKeyboardModuleState } from "./test/appHarness.js"; + +describe("migrated global shortcuts", () => { + vi.setConfig({ testTimeout: 20_000 }); + + beforeEach(() => { + installAppDomGlobals(); + }); + + afterEach(async () => { + cleanup(); + vi.unstubAllGlobals(); + localStorage.clear(); + await resetKeyboardModuleState(); + }); + + it("j and k navigate the PR list", async () => { + await mountApp("/pulls"); + await waitFor(() => expect(document.querySelector(".pr-list-row")).not.toBeNull()); + + pressKey("j"); + await waitFor(() => expect(document.querySelector(".pr-list-row.selected")).not.toBeNull()); + + pressKey("k"); + await waitFor(() => expect(document.querySelector(".pr-list-row.selected")).not.toBeNull()); + }); + + it("Cmd+[ toggles the sidebar", async () => { + await mountApp("/pulls"); + const sidebar = await waitFor(() => { + const el = document.querySelector("[data-test='sidebar']"); + expect(el).not.toBeNull(); + return el!; + }); + const wasCollapsed = sidebar.getAttribute("data-collapsed") === "true"; + + pressKey("[", { meta: true }); + + await waitFor(() => + expect(document.querySelector("[data-test='sidebar']")?.getAttribute("data-collapsed")).toBe( + (!wasCollapsed).toString(), + ), + ); + }); + + it("Cmd+[ is reserved on Activity without toggling the sidebar", async () => { + localStorage.setItem("middleman-sidebar", "collapsed"); + await mountApp("/"); + + const expandButton = await waitFor(() => screen.getByRole("button", { name: "Expand sidebar" })); + expect(document.querySelectorAll("header kbd[aria-label$='-[']")).toHaveLength(0); + + // Toggle sidebar must not be offered in the palette on this page. + pressKey("k", { meta: true }); + const palette = await waitFor(() => screen.getByRole("dialog", { name: "Command palette" })); + expect( + Array.from(palette.querySelectorAll(".palette-row")).filter((row) => row.textContent?.includes("Toggle sidebar")), + ).toHaveLength(0); + pressKey("Escape"); + await waitFor(() => expect(screen.queryByRole("dialog", { name: "Command palette" })).toBeNull()); + + // Nor in the cheatsheet. + pressKey("?", { shift: true }); + const cheatsheet = await waitFor(() => screen.getByRole("dialog", { name: "Keyboard shortcuts" })); + expect(cheatsheet.textContent).not.toContain("Toggle sidebar"); + pressKey("Escape"); + await waitFor(() => expect(screen.queryByRole("dialog", { name: "Keyboard shortcuts" })).toBeNull()); + + // The chord is still consumed (reserved) so the browser default never + // fires, but the collapsed sidebar stays collapsed. + const event = pressKey("[", { meta: true }); + expect(event.defaultPrevented).toBe(true); + expect(screen.getByRole("button", { name: "Expand sidebar" })).toBeTruthy(); + }); + + it("Cmd+[ is reserved on compact PR list without toggling persisted sidebar state", async () => { + await mountApp("/pulls", { viewportWidth: 480 }); + await waitFor(() => expect(document.body.textContent).toContain("Add browser regression coverage")); + + expect(document.querySelectorAll(".sidebar")).toHaveLength(0); + localStorage.removeItem("middleman-sidebar"); + + const event = pressKey("[", { meta: true }); + + expect(event.defaultPrevented).toBe(true); + expect(localStorage.getItem("middleman-sidebar")).toBeNull(); + }); +}); diff --git a/frontend/src/App.palette-commands.test.ts b/frontend/src/App.palette-commands.test.ts new file mode 100644 index 000000000..a11b46ca5 --- /dev/null +++ b/frontend/src/App.palette-commands.test.ts @@ -0,0 +1,98 @@ +// Palette open/close wiring and command dispatch through the real app +// shell's global keydown handler, with the API mocked at the fetch +// boundary. + +import { cleanup, screen, waitFor } from "@testing-library/svelte"; +import { fireEvent } from "@testing-library/svelte"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vite-plus/test"; + +import { installAppDomGlobals, mountApp, pressKey, resetKeyboardModuleState } from "./test/appHarness.js"; + +function paletteDialog(): HTMLElement | null { + return screen.queryByRole("dialog", { name: "Command palette" }); +} + +function paletteInput(): HTMLInputElement { + const input = document.querySelector(".palette-input"); + expect(input).not.toBeNull(); + return input!; +} + +async function waitForPaletteOpenAndFocused(): Promise { + await waitFor(() => { + expect(paletteDialog()).not.toBeNull(); + expect(document.activeElement).toBe(paletteInput()); + }); +} + +describe("palette command dispatch", () => { + vi.setConfig({ testTimeout: 20_000 }); + + beforeEach(() => { + installAppDomGlobals(); + }); + + afterEach(async () => { + cleanup(); + vi.unstubAllGlobals(); + localStorage.clear(); + await resetKeyboardModuleState(); + }); + + it("header trigger and Cmd+Shift+P open the palette", async () => { + await mountApp("/pulls"); + const trigger = await waitFor(() => screen.getByRole("button", { name: "Open command palette" })); + await fireEvent.click(trigger); + await waitForPaletteOpenAndFocused(); + + pressKey("Escape"); + await waitFor(() => expect(paletteDialog()).toBeNull()); + + pressKey("P", { meta: true, shift: true }); + await waitForPaletteOpenAndFocused(); + + pressKey("P", { meta: true, shift: true }); + await waitFor(() => expect(paletteDialog()).toBeNull()); + }); + + it("'>' filters to commands; running Open settings navigates", async () => { + await mountApp("/pulls"); + await waitFor(() => expect(document.querySelector("[data-test='pr-list']")).not.toBeNull()); + + pressKey("k", { meta: true }); + await waitForPaletteOpenAndFocused(); + + await fireEvent.input(paletteInput(), { target: { value: ">settings" } }); + pressKey("Enter", {}, paletteInput()); + + await waitFor(() => expect(window.location.pathname).toBe("/settings")); + }); + + it("typing a single character in the search input does not fire global j", async () => { + await mountApp("/pulls"); + // Wait for PR rows to render so .pr-list-row.selected has a chance + // to appear if the j shortcut leaks through. + await waitFor(() => expect(document.querySelector(".pr-list-row")).not.toBeNull()); + + pressKey("k", { meta: true }); + await waitForPaletteOpenAndFocused(); + + const before = document.querySelectorAll(".pr-list-row.selected").length; + pressKey("j", {}, paletteInput()); + await fireEvent.input(paletteInput(), { target: { value: "j" } }); + const after = document.querySelectorAll(".pr-list-row.selected").length; + expect(after).toBe(before); + }); + + it("Cmd+P inside the palette closes it instead of opening browser print", async () => { + await mountApp("/pulls"); + + pressKey("k", { meta: true }); + await waitForPaletteOpenAndFocused(); + + const event = pressKey("p", { meta: true }, paletteInput()); + await waitFor(() => expect(paletteDialog()).toBeNull()); + // The shortcut must be consumed so the browser print dialog never opens. + expect(event.defaultPrevented).toBe(true); + }); +}); diff --git a/frontend/src/App.palette-focus-trap.test.ts b/frontend/src/App.palette-focus-trap.test.ts new file mode 100644 index 000000000..cb030009b --- /dev/null +++ b/frontend/src/App.palette-focus-trap.test.ts @@ -0,0 +1,54 @@ +// Tab and Shift+Tab must keep focus inside the open palette. The trap is +// programmatic (the dialog's keydown handler moves focus and prevents +// default), so jsdom exercises the same code path a browser does. + +import { cleanup, screen, waitFor } from "@testing-library/svelte"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vite-plus/test"; + +import { installAppDomGlobals, mountApp, pressKey, resetKeyboardModuleState } from "./test/appHarness.js"; + +describe("palette focus trap", () => { + vi.setConfig({ testTimeout: 20_000 }); + + beforeEach(() => { + installAppDomGlobals(); + }); + + afterEach(async () => { + cleanup(); + vi.unstubAllGlobals(); + localStorage.clear(); + await resetKeyboardModuleState(); + }); + + it("Tab and Shift+Tab cycle within the open palette", async () => { + await mountApp("/pulls"); + + // The palette is opened from anywhere via Meta+K (Ctrl+K on non-mac); + // both bindings are wired to palette.open in defaultActions. + pressKey("k", { meta: true }); + + const input = await waitFor(() => { + const el = document.querySelector(".palette-input"); + expect(el).not.toBeNull(); + expect(document.activeElement).toBe(el); + return el!; + }); + + const palette = document.querySelector(".palette"); + expect(palette).not.toBeNull(); + + // Forward Tab: focus must stay inside the .palette dialog. + const tab = pressKey("Tab", {}, input); + expect(palette!.contains(document.activeElement)).toBe(true); + expect(tab.defaultPrevented).toBe(true); + + // Reverse Tab: same containment guarantee. + pressKey("Tab", { shift: true }, document.activeElement!); + expect(palette!.contains(document.activeElement)).toBe(true); + + // Escape closes the palette and tears down the modal frame. + pressKey("Escape", {}, document.activeElement!); + await waitFor(() => expect(screen.queryByRole("dialog", { name: "Command palette" })).toBeNull()); + }); +}); diff --git a/frontend/src/App.palette-pr-detail-commands.test.ts b/frontend/src/App.palette-pr-detail-commands.test.ts new file mode 100644 index 000000000..bbbc53d54 --- /dev/null +++ b/frontend/src/App.palette-pr-detail-commands.test.ts @@ -0,0 +1,176 @@ +// PR-detail palette commands (`pr.approve`, `pr.ready`, +// `pr.approveWorkflows`). The merge palette command is intentionally not +// registered (the trigger lives in PullDetail.svelte's local component +// state). The app is mounted for real so the asserted approve POST flows +// through the same closure the detail-pane button uses. + +import { cleanup, screen, waitFor } from "@testing-library/svelte"; +import { fireEvent } from "@testing-library/svelte"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vite-plus/test"; + +import { installAppDomGlobals, mountApp, pressKey, resetKeyboardModuleState } from "./test/appHarness.js"; +import { jsonResponse, type MockRouteOverride } from "./test/mockApiFetch.js"; + +const capabilities = { + read_repositories: true, + read_merge_requests: true, + read_issues: true, + read_comments: true, + read_releases: true, + read_ci: true, + read_labels: true, + comment_mutation: true, + state_mutation: true, + merge_mutation: true, + label_mutation: true, + review_mutation: true, + workflow_approval: true, + ready_for_review: true, + issue_mutation: true, + review_draft_mutation: false, + review_thread_resolution: false, + read_review_threads: false, + native_multiline_ranges: false, + mutation_head_binding: false, + thread_reply: false, + thread_resolve: false, + supported_review_actions: [], +}; + +const repo = { + provider: "github", + platform_host: "github.com", + repo_path: "acme/widgets", + owner: "acme", + name: "widgets", + capabilities, +}; + +const closedPR55: MockRouteOverride = (req) => { + if (req.method !== "GET" || req.url.pathname !== "/api/v1/pulls/github/acme/widgets/55") return null; + return jsonResponse({ + merge_request: { + ID: 3, + RepoID: 1, + GitHubID: 301, + Number: 55, + URL: "https://github.com/acme/widgets/pull/55", + Title: "Refactor theme system", + Author: "luisa", + State: "closed", + IsDraft: false, + Body: "Consolidates theme tokens.", + HeadBranch: "refactor/theme", + BaseBranch: "main", + Additions: 80, + Deletions: 40, + CommentCount: 0, + ReviewDecision: "", + CIStatus: "pending", + CIChecksJSON: "[]", + CreatedAt: "2026-03-29T14:00:00Z", + UpdatedAt: "2026-03-30T14:00:00Z", + LastActivityAt: "2026-03-30T14:00:00Z", + MergedAt: null, + ClosedAt: "2026-03-30T14:00:00Z", + KanbanStatus: "new", + Starred: false, + repo_owner: "acme", + repo_name: "widgets", + platform_host: "github.com", + repo, + worktree_links: [], + }, + events: [], + repo, + repo_owner: "acme", + repo_name: "widgets", + platform_host: "github.com", + detail_loaded: true, + detail_fetched_at: "2026-03-30T14:00:00Z", + worktree_links: [], + }); +}; + +function paletteInput(): HTMLInputElement { + const input = document.querySelector(".palette-input"); + expect(input).not.toBeNull(); + return input!; +} + +async function openPaletteWith(query: string): Promise { + pressKey("k", { meta: true }); + await waitFor(() => { + expect(screen.queryByRole("dialog", { name: "Command palette" })).not.toBeNull(); + expect(document.activeElement).toBe(paletteInput()); + }); + await fireEvent.input(paletteInput(), { target: { value: query } }); +} + +function paletteRowsNamed(pattern: RegExp): HTMLElement[] { + // Palette rows render as