From 54ddb0aa1e9d6833deecb13f9188e8a124e2813a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Almeida?= <57680069+zeval@users.noreply.github.com> Date: Tue, 19 May 2026 15:53:48 +0100 Subject: [PATCH] feat(web): highlight active-tab file row in Changes panel (#954) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(web): track active file path for diff panels Extend resolveFilePath so activeFilePath updates when a file-diff panel (preview:file-diff or diff:file:) becomes the active dockview panel. Also re-subscribe onDidParametersChange for the preview file-diff slot so swapping the preview to a different file refreshes the path. * feat(web): highlight active-tab file row in Changes panel When a file's diff tab is the active dockview panel, mark the matching row in unstaged, staged, and PR Changes lists with data-active="true" and reuse the existing bg-accent/60 highlight (same as multi-select). PRFileRow also gains data-changes-file so it shares the existing row selector with FileRow. * test(web): cover active-tab highlight unit + E2E Unit test in FileRow guards the data-active attribute and bg-accent/60 class when isActive (with and without isSelected). New E2E spec drives the full flow: open file-a diff, switch to file-b, close the preview, and verify highlight follows when a file moves from unstaged to staged. * fix(web): use distinct e2e task titles for active-tab highlight spec clickTab("Changes") fails strict-mode when the task title also contains the word "Changes" — Playwright matches both the session tab and the dockview Changes tab. Renames to "Active Highlight (Staged) Test". * test(web): close pinned diff:file panels in closeFileDiffPreview The helper previously closed only the preview slot, leaving pinned diff:file: panels (created when a preview is promoted) active. Sweeps every diff:file: panel so the row settles to data-active=false regardless of how the diff was opened. * test(web): unit-cover resolveFilePath panel-id branches Exercises every branch of the dockview-store resolveFilePath closure: file:, diff:file:, preview:file-editor, and preview:file-diff (the last two via params.path), plus the null-out paths. Catches regressions in the panel-id mapping without paying for an E2E round-trip. * fix(web): strip "changes" from active-tab spec profile names The agent profile name becomes the dockview session tab title, so a profile named "changes-active-highlight" makes clickTab("Changes") match two .dv-default-tab elements (session tab + Changes tab) and trip strict-mode. Rename to "active-highlight" / "active-highlight- staged" to keep the locator unambiguous. * docs(web): note multi-repo path collision in PRFilesRepoGroup Mirrors the FileListBody comment so future authors see the same caveat in both file-list components: activeFilePath carries the path but not the repo, so identical paths across repos light up both rows. * fix(web): snapshot panels before closing in closeFileDiffPreview dockview's api.panels is a live reference; calling panel.api.close() removes the panel in place, so iterating the live array skipped every other diff:file:* panel when two or more were open. Filter into a local array before closing. --- .../task/changes-panel-file-row.test.tsx | 54 +++++++++ .../task/changes-panel-file-row.tsx | 8 +- .../task/changes-panel-pr-files.tsx | 24 +++- .../task/changes-panel-timeline.tsx | 6 + apps/web/e2e/pages/session-page.ts | 26 +++++ ...changes-panel-active-tab-highlight.spec.ts | 109 ++++++++++++++++++ apps/web/lib/state/dockview-store.test.ts | 93 +++++++++++++++ apps/web/lib/state/dockview-store.ts | 5 +- 8 files changed, 320 insertions(+), 5 deletions(-) create mode 100644 apps/web/e2e/tests/changes-panel-active-tab-highlight.spec.ts create mode 100644 apps/web/lib/state/dockview-store.test.ts diff --git a/apps/web/components/task/changes-panel-file-row.test.tsx b/apps/web/components/task/changes-panel-file-row.test.tsx index 7f81617e90..35fa3e4fcc 100644 --- a/apps/web/components/task/changes-panel-file-row.test.tsx +++ b/apps/web/components/task/changes-panel-file-row.test.tsx @@ -108,3 +108,57 @@ describe("FileRow truncation (regression: path overlaps diff stats in narrow pan }); }); }); + +describe("FileRow active-tab highlight", () => { + it("renders data-active='true' and bg-accent/60 when isActive", () => { + const { container } = render( + +
    + +
+ , + ); + + const row = container.querySelector("[data-changes-file='active.ts']") as HTMLElement | null; + expect(row).not.toBeNull(); + expect(row!.getAttribute("data-active")).toBe("true"); + expect(row!.className).toContain("bg-accent/60"); + }); + + it("keeps highlight when isActive and isSelected are both true", () => { + const { container } = render( + +
    + +
+ , + ); + + const row = container.querySelector("[data-changes-file='both.ts']") as HTMLElement | null; + expect(row).not.toBeNull(); + expect(row!.getAttribute("data-active")).toBe("true"); + expect(row!.getAttribute("data-selected")).toBe("true"); + expect(row!.className).toContain("bg-accent/60"); + }); +}); diff --git a/apps/web/components/task/changes-panel-file-row.tsx b/apps/web/components/task/changes-panel-file-row.tsx index 61df3cb7a2..b4cd90610a 100644 --- a/apps/web/components/task/changes-panel-file-row.tsx +++ b/apps/web/components/task/changes-panel-file-row.tsx @@ -41,6 +41,8 @@ type FileRowProps = { file: ChangedFile; isPending: boolean; isSelected?: boolean; + /** True when this file's diff/editor tab is the currently active dockview panel. */ + isActive?: boolean; onSelect?: (path: string, e: React.MouseEvent) => boolean; onOpenDiff: (path: string, options?: OpenDiffOptions) => void; // Multi-repo: handlers receive the file's repository_name so the per-file @@ -56,6 +58,7 @@ export function FileRow({ file, isPending, isSelected, + isActive, onSelect, onOpenDiff, onStage, @@ -81,10 +84,13 @@ export function FileRow({ data-testid={`file-row-${file.path.replace(/[/\\]/g, "-")}`} data-changes-file={file.path} data-selected={isSelected ? "true" : "false"} + data-active={isActive ? "true" : "false"} className={cn( "group flex items-center justify-between gap-2 text-sm rounded-md px-2 py-1.5 -mx-1 cursor-pointer", "md:px-1 md:py-0.5", - isSelected ? "bg-accent/60 text-accent-foreground hover:bg-accent/50" : "hover:bg-muted/60", + isSelected || isActive + ? "bg-accent/60 text-accent-foreground hover:bg-accent/50" + : "hover:bg-muted/60", )} onClick={handleClick} > diff --git a/apps/web/components/task/changes-panel-pr-files.tsx b/apps/web/components/task/changes-panel-pr-files.tsx index cee7d5be94..b90f081992 100644 --- a/apps/web/components/task/changes-panel-pr-files.tsx +++ b/apps/web/components/task/changes-panel-pr-files.tsx @@ -3,6 +3,8 @@ import { useState } from "react"; import { IconChevronDown, IconChevronRight, IconGitPullRequest } from "@tabler/icons-react"; import { LineStat } from "@/components/diff-stat"; +import { cn } from "@/lib/utils"; +import { useDockviewStore } from "@/lib/state/dockview-store"; import { FileStatusIcon } from "./file-status-icon"; import { groupByRepositoryName } from "@/lib/group-by-repo"; import type { PRChangedFile } from "./changes-panel-timeline"; @@ -26,6 +28,7 @@ export function PRFilesGroupedList({ onOpenDiff, repoDisplayName, }: PRFilesSectionContentProps) { + const activeFilePath = useDockviewStore((s) => s.activeFilePath); const groups = groupByRepositoryName(files, (f) => f.repository_name); const showRepoHeaders = groups.length > 1 || (groups[0]?.repositoryName ?? "") !== ""; return ( @@ -38,6 +41,7 @@ export function PRFilesGroupedList({ files={group.items} showHeader={showRepoHeaders} onOpenDiff={onOpenDiff} + activeFilePath={activeFilePath} /> ))} @@ -50,12 +54,14 @@ function PRFilesRepoGroup({ files, showHeader, onOpenDiff, + activeFilePath, }: { repositoryName: string; displayName?: string; files: PRChangedFile[]; showHeader: boolean; onOpenDiff: (path: string, options?: OpenDiffOptions) => void; + activeFilePath: string | null; }) { const [collapsed, setCollapsed] = useState(false); const label = displayName ?? repositoryName ?? ""; @@ -82,8 +88,15 @@ function PRFilesRepoGroup({ )} {!collapsed && ( )} @@ -94,9 +107,11 @@ function PRFilesRepoGroup({ function PRFileRow({ file, onOpenDiff, + isActive, }: { file: PRChangedFile; onOpenDiff: (path: string, options?: OpenDiffOptions) => void; + isActive?: boolean; }) { const lastSlash = file.path.lastIndexOf("/"); const folder = lastSlash === -1 ? "" : file.path.slice(0, lastSlash); @@ -104,7 +119,12 @@ function PRFileRow({ return (
  • onOpenDiff(file.path, { source: "pr", diff --git a/apps/web/components/task/changes-panel-timeline.tsx b/apps/web/components/task/changes-panel-timeline.tsx index 278054dd32..722c4c672e 100644 --- a/apps/web/components/task/changes-panel-timeline.tsx +++ b/apps/web/components/task/changes-panel-timeline.tsx @@ -6,6 +6,7 @@ import { useMultiSelect } from "@/hooks/use-multi-select"; import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip"; import { cn } from "@/lib/utils"; import type { FileInfo } from "@/lib/state/store"; +import { useDockviewStore } from "@/lib/state/dockview-store"; import { FileRow, BulkActionBar } from "./changes-panel-file-row"; import { type CommitItem } from "./commit-row"; import { groupByRepositoryName, isSingleRepoGroup } from "@/lib/group-by-repo"; @@ -274,6 +275,10 @@ type FileListBodyProps = { function FileListBody(props: FileListBodyProps) { const { variant, files, pendingStageFiles, multiSelect } = props; + // Multi-repo nuance: activeFilePath carries the path but not repo; same path + // in two repos will light up both rows. Matches existing routing limit noted + // in FileRowProps comments. + const activeFilePath = useDockviewStore((s) => s.activeFilePath); const groups = useMemo(() => groupByRepositoryName(files, (f) => f.repositoryName), [files]); // Per-repo collapsed state: keyed by repositoryName. Default expanded; // setting an entry to true collapses that group. Persists across re-renders @@ -294,6 +299,7 @@ function FileListBody(props: FileListBodyProps) { file={file} isPending={pendingStageFiles.has(`${file.repositoryName ?? ""}::${file.path}`)} isSelected={multiSelect.isSelected(file.path)} + isActive={file.path === activeFilePath} onSelect={multiSelect.handleClick} onOpenDiff={props.onOpenDiff} onStage={props.onStage} diff --git a/apps/web/e2e/pages/session-page.ts b/apps/web/e2e/pages/session-page.ts index 9027cdc5ea..88050e460e 100644 --- a/apps/web/e2e/pages/session-page.ts +++ b/apps/web/e2e/pages/session-page.ts @@ -1028,6 +1028,32 @@ export class SessionPage { return this.changes.locator("[data-selected='true']"); } + /** All file rows in the changes panel currently marked as the active tab. */ + changesActiveRows(): Locator { + return this.changes.locator("[data-active='true']"); + } + + /** + * Close every file-diff panel in dockview: the `preview:file-diff` slot AND + * any pinned `diff:file:` panels created by promoting the preview. + * After this resolves, no diff tab is active so the changes-panel rows + * settle to `data-active="false"`. + */ + async closeFileDiffPreview(): Promise { + await this.page.evaluate(() => { + type PanelApi = { close: () => void }; + type Panel = { id: string; api: PanelApi }; + type Api = { panels: Panel[]; getPanel: (i: string) => Panel | undefined }; + const api = (window as unknown as { __dockviewApi__?: Api }).__dockviewApi__; + if (!api) return; + api.getPanel("preview:file-diff")?.api.close(); + // Snapshot before iterating: panel.api.close() mutates api.panels in + // place, so iterating the live array would skip every other panel. + const pinned = [...api.panels].filter((p) => p.id.startsWith("diff:file:")); + for (const panel of pinned) panel.api.close(); + }); + } + /** Bulk action bar for a variant (unstaged/staged). */ changesBulkActionBar(variant: "unstaged" | "staged"): Locator { return this.changes.getByTestId(`bulk-actions-${variant}`); diff --git a/apps/web/e2e/tests/changes-panel-active-tab-highlight.spec.ts b/apps/web/e2e/tests/changes-panel-active-tab-highlight.spec.ts new file mode 100644 index 0000000000..0e64176b85 --- /dev/null +++ b/apps/web/e2e/tests/changes-panel-active-tab-highlight.spec.ts @@ -0,0 +1,109 @@ +import { test, expect } from "../fixtures/test-base"; +import path from "node:path"; +import { + GitHelper, + makeGitEnv, + openTaskSession, + createStandardProfile, +} from "../helpers/git-helper"; + +test.describe("Changes Panel — Active-Tab Highlight", () => { + test("clicking a file row highlights the row while its diff tab is active", async ({ + testPage, + apiClient, + seedData, + backend, + }) => { + const repoDir = path.join(backend.tmpDir, "repos", "e2e-repo"); + const git = new GitHelper(repoDir, makeGitEnv(backend.tmpDir)); + + const profile = await createStandardProfile(apiClient, "active-highlight"); + await apiClient.createTaskWithAgent(seedData.workspaceId, "Active Highlight Test", profile.id, { + description: "/e2e:simple-message", + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }); + + const session = await openTaskSession(testPage, "Active Highlight Test"); + await session.clickTab("Changes"); + + git.createFile("active-a.ts", "a"); + git.createFile("active-b.ts", "b"); + + await expect(testPage.getByTestId("unstaged-files-section")).toBeVisible({ timeout: 15_000 }); + const fileA = session.changesFileRow("active-a.ts"); + const fileB = session.changesFileRow("active-b.ts"); + await expect(fileA).toBeVisible({ timeout: 15_000 }); + await expect(fileB).toBeVisible({ timeout: 15_000 }); + + // Both start inactive. + await expect(fileA).toHaveAttribute("data-active", "false"); + await expect(fileB).toHaveAttribute("data-active", "false"); + + // Open file-a → row A becomes active. + await fileA.click(); + await expect(fileA).toHaveAttribute("data-active", "true", { timeout: 5_000 }); + await expect(fileB).toHaveAttribute("data-active", "false"); + await expect(session.changesActiveRows()).toHaveCount(1); + + // Switch to file-b → highlight follows. + await fileB.click(); + await expect(fileB).toHaveAttribute("data-active", "true", { timeout: 5_000 }); + await expect(fileA).toHaveAttribute("data-active", "false"); + await expect(session.changesActiveRows()).toHaveCount(1); + + // Close the diff preview → nothing highlighted. + await session.closeFileDiffPreview(); + await expect(fileA).toHaveAttribute("data-active", "false", { timeout: 5_000 }); + await expect(fileB).toHaveAttribute("data-active", "false"); + await expect(session.changesActiveRows()).toHaveCount(0); + }); + + test("highlight follows a file when it moves from unstaged to staged", async ({ + testPage, + apiClient, + seedData, + backend, + }) => { + const repoDir = path.join(backend.tmpDir, "repos", "e2e-repo"); + const git = new GitHelper(repoDir, makeGitEnv(backend.tmpDir)); + + const profile = await createStandardProfile(apiClient, "active-highlight-staged"); + await apiClient.createTaskWithAgent( + seedData.workspaceId, + "Active Highlight Staged Test", + profile.id, + { + description: "/e2e:simple-message", + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }, + ); + + const session = await openTaskSession(testPage, "Active Highlight Staged Test"); + await session.clickTab("Changes"); + + git.createFile("staged-active.ts", "a"); + + await expect(testPage.getByTestId("unstaged-files-section")).toBeVisible({ timeout: 15_000 }); + const row = session.changesFileRow("staged-active.ts"); + await expect(row).toBeVisible({ timeout: 15_000 }); + + await row.click(); + await expect(row).toHaveAttribute("data-active", "true", { timeout: 5_000 }); + + // Stage the file via the existing per-file stage button; row re-renders in + // the staged section but must keep data-active="true" since the diff tab + // for the same path is still the active panel. + git.stageFile("staged-active.ts"); + + const stagedList = session.changes.getByTestId("staged-file-list"); + await expect(stagedList.getByText("staged-active.ts")).toBeVisible({ timeout: 15_000 }); + + // The active row should now live inside the staged list. + const stagedRow = stagedList.locator(`[data-changes-file="staged-active.ts"]`); + await expect(stagedRow).toHaveAttribute("data-active", "true", { timeout: 5_000 }); + }); +}); diff --git a/apps/web/lib/state/dockview-store.test.ts b/apps/web/lib/state/dockview-store.test.ts new file mode 100644 index 0000000000..c01977ac08 --- /dev/null +++ b/apps/web/lib/state/dockview-store.test.ts @@ -0,0 +1,93 @@ +import { describe, it, expect, beforeEach, vi } from "vitest"; +import type { DockviewApi } from "dockview-react"; +import { useDockviewStore } from "./dockview-store"; + +type ActivePanelEvent = { id: string }; +type CapturedHandlers = { + active: ((e?: ActivePanelEvent) => void) | null; +}; + +type ParamsPanel = { id: string; params: Record }; + +function makeApi(panels: ParamsPanel[] = []): { api: DockviewApi; captured: CapturedHandlers } { + const captured: CapturedHandlers = { active: null }; + const api = { + onDidActivePanelChange: (cb: (e?: ActivePanelEvent) => void) => { + captured.active = cb; + return { dispose: vi.fn() }; + }, + onDidAddPanel: () => ({ dispose: vi.fn() }), + onDidRemovePanel: () => ({ dispose: vi.fn() }), + getPanel: (id: string) => panels.find((p) => p.id === id), + hasMaximizedGroup: () => false, + } as unknown as DockviewApi; + return { api, captured }; +} + +describe("dockview-store resolveFilePath (via onDidActivePanelChange)", () => { + beforeEach(() => { + useDockviewStore.getState().setApi(null); + }); + + it("resolves pinned file: panel id to its path", () => { + const { api, captured } = makeApi(); + useDockviewStore.getState().setApi(api); + + captured.active?.({ id: "file:src/foo.ts" }); + + expect(useDockviewStore.getState().activeFilePath).toBe("src/foo.ts"); + }); + + it("resolves pinned diff:file: panel id to its path", () => { + const { api, captured } = makeApi(); + useDockviewStore.getState().setApi(api); + + captured.active?.({ id: "diff:file:src/bar.ts" }); + + expect(useDockviewStore.getState().activeFilePath).toBe("src/bar.ts"); + }); + + it("resolves preview:file-editor panel via params.path", () => { + const { api, captured } = makeApi([ + { id: "preview:file-editor", params: { path: "src/baz.ts" } }, + ]); + useDockviewStore.getState().setApi(api); + + captured.active?.({ id: "preview:file-editor" }); + + expect(useDockviewStore.getState().activeFilePath).toBe("src/baz.ts"); + }); + + it("resolves preview:file-diff panel via params.path", () => { + const { api, captured } = makeApi([ + { id: "preview:file-diff", params: { path: "src/diff.ts" } }, + ]); + useDockviewStore.getState().setApi(api); + + captured.active?.({ id: "preview:file-diff" }); + + expect(useDockviewStore.getState().activeFilePath).toBe("src/diff.ts"); + }); + + it("clears activeFilePath when a non-file panel becomes active", () => { + const { api, captured } = makeApi(); + useDockviewStore.getState().setApi(api); + + captured.active?.({ id: "file:src/foo.ts" }); + expect(useDockviewStore.getState().activeFilePath).toBe("src/foo.ts"); + + captured.active?.({ id: "chat" }); + expect(useDockviewStore.getState().activeFilePath).toBeNull(); + }); + + it("clears activeFilePath when active-panel-change fires with no panel", () => { + const { api, captured } = makeApi(); + useDockviewStore.getState().setApi(api); + + captured.active?.({ id: "diff:file:src/bar.ts" }); + expect(useDockviewStore.getState().activeFilePath).toBe("src/bar.ts"); + + captured.active?.(undefined); + expect(useDockviewStore.getState().activeFilePath).toBeNull(); + }); +}); diff --git a/apps/web/lib/state/dockview-store.ts b/apps/web/lib/state/dockview-store.ts index 3ca8ae4db0..a33b58cd36 100644 --- a/apps/web/lib/state/dockview-store.ts +++ b/apps/web/lib/state/dockview-store.ts @@ -687,7 +687,8 @@ export const useDockviewStore = create((set, get) => ({ const resolveFilePath = (panelId: string | undefined): string | null => { if (!panelId) return null; if (panelId.startsWith("file:")) return panelId.slice(5); - if (panelId === "preview:file-editor") { + if (panelId.startsWith("diff:file:")) return panelId.slice("diff:file:".length); + if (panelId === "preview:file-editor" || panelId === "preview:file-diff") { const path = (api.getPanel(panelId)?.params as Record | undefined) ?.path as string | undefined; return path ?? null; @@ -707,7 +708,7 @@ export const useDockviewStore = create((set, get) => ({ // different file. Dockview does not refire `onDidActivePanelChange` for // params-only updates on an already-active panel, so subscribe to the // panel's own parameter-change event and refresh `activeFilePath`. - if (panel.id !== "preview:file-editor") return; + if (panel.id !== "preview:file-editor" && panel.id !== "preview:file-diff") return; paramSubs.get(panel.id)?.dispose(); const sub = panel.api.onDidParametersChange(() => { if (!panel.api.isActive) return;