dreampiler/kandev
1
0
Fork 0
Code Issues 43 Pull Requests Actions 2 Packages Projects Releases Wiki Activity

feat(web): highlight active-tab file row in Changes panel (#954)

Browse Source 
* 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:<path>) 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:<path> 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.
This commit is contained in:
José Almeida
2026-05-19 15:53:48 +01:00
committed by GitHub
parent 7115ee08d5
commit 54ddb0aa1e
8 changed files with 320 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/web/components/task/changes-panel-file-row.test.tsx
+54
View File
@@ -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(
<TooltipProvider>
<ul>
<FileRow
file={{ ...baseFile, path: "active.ts" }}
isPending={false}
isActive
onSelect={noopSelect}
onOpenDiff={noop}
onStage={noop}
onUnstage={noop}
onDiscard={noop}
onEditFile={noop}
/>
</ul>
</TooltipProvider>,
);
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(
<TooltipProvider>
<ul>
<FileRow
file={{ ...baseFile, path: "both.ts" }}
isPending={false}
isActive
isSelected
onSelect={noopSelect}
onOpenDiff={noop}
onStage={noop}
onUnstage={noop}
onDiscard={noop}
onEditFile={noop}
/>
</ul>
</TooltipProvider>,
);
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");
});
});
apps/web/components/task/changes-panel-file-row.tsx
+7 -1
View File
@@ -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}
>
apps/web/components/task/changes-panel-pr-files.tsx
+22 -2
View File
@@ -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}
/>
))}
</ul>
@@ -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 && (
<ul className="space-y-0.5">
{/* Multi-repo: activeFilePath carries no repo context, so identical
paths across repos light up both rows. Matches FileListBody. */}
{files.map((file) => (
<PRFileRow key={file.path} file={file} onOpenDiff={onOpenDiff} />
<PRFileRow
key={file.path}
file={file}
onOpenDiff={onOpenDiff}
isActive={file.path === activeFilePath}
/>
))}
</ul>
)}
@@ -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 (
<li
className="group flex items-center justify-between gap-2 text-sm rounded-md px-2 py-1.5 -mx-1 hover:bg-muted/60 cursor-pointer md:px-1 md:py-0.5"
data-changes-file={file.path}
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",
isActive ? "bg-accent/60 text-accent-foreground hover:bg-accent/50" : "hover:bg-muted/60",
)}
onClick={() =>
onOpenDiff(file.path, {
source: "pr",
apps/web/components/task/changes-panel-timeline.tsx
+6
View File
@@ -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}
apps/web/e2e/pages/session-page.ts
+26
View File
@@ -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:<path>` 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<void> {
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}`);
apps/web/e2e/tests/changes-panel-active-tab-highlight.spec.ts
+109
View File
@@ -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 });
});
});
apps/web/lib/state/dockview-store.test.ts
+93
View File
@@ -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<string, unknown> };
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();
});
});
apps/web/lib/state/dockview-store.ts
+3 -2
View File
@@ -687,7 +687,8 @@ export const useDockviewStore = create<DockviewStore>((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<string, unknown> | undefined)
?.path as string | undefined;
return path ?? null;
@@ -707,7 +708,7 @@ export const useDockviewStore = create<DockviewStore>((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;