From 9f9b5c3f726de9cc3d677e89cbd90357d7d9c85d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Sat, 30 May 2026 18:07:36 +0100 Subject: [PATCH] feat(web): auto-expand the first group in the changes panel (#1169) --- .../components/task/changes-panel-body.tsx | 13 ++++- .../task/changes-panel-helpers.test.ts | 43 ++++++++++++++++ .../components/task/changes-panel-helpers.ts | 26 ++++++++++ .../changes-panel-timeline-grouping.test.tsx | 51 ++++++++++++++++++- .../task/changes-panel-timeline.tsx | 28 ++++++++-- .../git/changes-panel-section-order.spec.ts | 12 +++++ 6 files changed, 168 insertions(+), 5 deletions(-) diff --git a/apps/web/components/task/changes-panel-body.tsx b/apps/web/components/task/changes-panel-body.tsx index 54fafebdd6..3a65492507 100644 --- a/apps/web/components/task/changes-panel-body.tsx +++ b/apps/web/components/task/changes-panel-body.tsx @@ -8,7 +8,7 @@ import { ReviewProgressBar, PRFilesSection, } from "./changes-panel-timeline"; -import { mergeCommits } from "./changes-panel-helpers"; +import { mergeCommits, firstVisibleSection } from "./changes-panel-helpers"; import type { ChangesPanelBodyProps } from "./changes-panel-data"; function ChangesPanelDialogsSection({ @@ -177,6 +177,15 @@ function ChangesPanelTimeline(props: TimelineProps) { const showCommits = props.hasStaged || props.hasCommits; const showCommitsList = props.hasStaged || hasMergedCommits; const hasSomethingAfterStaged = (props.hasPRFiles && hasLocalChanges) || showCommitsList; + // Auto-expand the first (topmost) visible section so the panel never opens + // looking empty (e.g. review mode: PR + Commits both collapsed). Unstaged / + // Staged keep their always-expanded default; only PR and Commits are gated. + const firstSection = firstVisibleSection({ + hasPRFiles: props.hasPRFiles, + hasUnstaged: props.hasUnstaged, + hasStaged: props.hasStaged, + showCommitsList, + }); return (
@@ -187,6 +196,7 @@ function ChangesPanelTimeline(props: TimelineProps) { isLast={!showCommitsList} onOpenDiff={props.onOpenDiffFile} repoDisplayName={props.repoDisplayName} + defaultCollapsed={firstSection !== "pr"} />
)} @@ -212,6 +222,7 @@ function ChangesPanelTimeline(props: TimelineProps) { { expect(map[""]).toBe("https://dev.azure.com/o/p/_git/r/pullrequest/9"); }); }); + +import { firstVisibleSection } from "./changes-panel-helpers"; + +describe("firstVisibleSection", () => { + const flags = (over: Partial[0]>) => ({ + hasPRFiles: false, + hasUnstaged: false, + hasStaged: false, + showCommitsList: false, + ...over, + }); + + it("returns null when nothing is shown", () => { + expect(firstVisibleSection(flags({}))).toBeNull(); + }); + + it("review mode: PR is first when there are no local changes", () => { + expect(firstVisibleSection(flags({ hasPRFiles: true, showCommitsList: true }))).toBe("pr"); + }); + + it("commits is first when it is the only section", () => { + expect(firstVisibleSection(flags({ showCommitsList: true }))).toBe("commits"); + }); + + it("unstaged wins over staged and commits", () => { + expect( + firstVisibleSection(flags({ hasUnstaged: true, hasStaged: true, showCommitsList: true })), + ).toBe("unstaged"); + }); + + it("staged is first when there is no unstaged", () => { + expect(firstVisibleSection(flags({ hasStaged: true, showCommitsList: true }))).toBe("staged"); + }); + + it("hybrid: local changes win over a PR (PR is not first)", () => { + expect( + firstVisibleSection(flags({ hasPRFiles: true, hasUnstaged: true, showCommitsList: true })), + ).toBe("unstaged"); + expect( + firstVisibleSection(flags({ hasPRFiles: true, hasStaged: true, showCommitsList: true })), + ).toBe("staged"); + }); +}); diff --git a/apps/web/components/task/changes-panel-helpers.ts b/apps/web/components/task/changes-panel-helpers.ts index 05d7931f02..a8bb1a8706 100644 --- a/apps/web/components/task/changes-panel-helpers.ts +++ b/apps/web/components/task/changes-panel-helpers.ts @@ -138,6 +138,32 @@ export function filterUnpushedCommits( ); } +/** Which timeline section, if any, renders first (topmost) in the Changes panel. */ +type FirstVisibleSection = "pr" | "unstaged" | "staged" | "commits" | null; + +/** + * Computes the first (topmost) visible section so the panel can auto-expand it, + * mirroring the render precedence in `ChangesPanelTimeline`: + * PR (review mode, no local changes) → Unstaged → Staged → Commits. + * + * The "review mode" PR section only sits at the top when there are no local + * working-tree changes; with local changes the PR section moves below Staged and + * is therefore never first. Returns null when nothing is shown. + */ +export function firstVisibleSection(flags: { + hasPRFiles: boolean; + hasUnstaged: boolean; + hasStaged: boolean; + showCommitsList: boolean; +}): FirstVisibleSection { + const { hasPRFiles, hasUnstaged, hasStaged, showCommitsList } = flags; + if (hasPRFiles && !hasUnstaged && !hasStaged) return "pr"; + if (hasUnstaged) return "unstaged"; + if (hasStaged) return "staged"; + if (showCommitsList) return "commits"; + return null; +} + type MergedCommit = { commit_sha: string; commit_message: string; diff --git a/apps/web/components/task/changes-panel-timeline-grouping.test.tsx b/apps/web/components/task/changes-panel-timeline-grouping.test.tsx index 8cd01070dc..86d3e08276 100644 --- a/apps/web/components/task/changes-panel-timeline-grouping.test.tsx +++ b/apps/web/components/task/changes-panel-timeline-grouping.test.tsx @@ -31,7 +31,7 @@ vi.mock("@/components/state-provider", () => ({ ) => selector({ userSettings: { changesPanelLayout: "flat" } }), })); -import { FileListSection, CommitsSection } from "./changes-panel-timeline"; +import { FileListSection, CommitsSection, PRFilesSection } from "./changes-panel-timeline"; const REPO_HEADER_TID = "changes-repo-header"; const COMMIT_ROW_TID = "commit-row"; @@ -247,3 +247,52 @@ describe("CommitsSection", () => { expect(latestByShas.sort()).toEqual(["backend-only", "frontend-new"]); }); }); + +describe("section auto-expand (defaultCollapsed prop)", () => { + const PR_TOGGLE_TID = "pr-changes-section-collapse-toggle"; + + function prFile(path: string) { + return { path, status: "modified" as const, plus: 1, minus: 0, oldPath: undefined }; + } + + it("PR Changes is collapsed by default (no prop)", () => { + render(); + expect(screen.getByTestId(PR_TOGGLE_TID).getAttribute(ARIA_EXPANDED)).toBe("false"); + expect(screen.queryByTestId("pr-files-list")).toBeNull(); + }); + + it("PR Changes is expanded when defaultCollapsed={false}", () => { + render( + , + ); + expect(screen.getByTestId(PR_TOGGLE_TID).getAttribute(ARIA_EXPANDED)).toBe("true"); + expect(screen.getByTestId("pr-files-list")).toBeTruthy(); + }); + + it("Commits is expanded when defaultCollapsed={false}", () => { + render( + , + ); + const toggle = screen.getByTestId(COMMITS_SECTION_TOGGLE_TID); + expect(toggle.getAttribute(ARIA_EXPANDED)).toBe("true"); + expect(screen.getByTestId(COMMIT_ROW_TID)).toBeTruthy(); + }); +}); diff --git a/apps/web/components/task/changes-panel-timeline.tsx b/apps/web/components/task/changes-panel-timeline.tsx index 55520007c4..7323635119 100644 --- a/apps/web/components/task/changes-panel-timeline.tsx +++ b/apps/web/components/task/changes-panel-timeline.tsx @@ -60,6 +60,19 @@ function TimelineSection({ "data-testid"?: string; }) { const [collapsed, setCollapsed] = useState(defaultCollapsed); + // Git data arrives in separate async store updates, so `defaultCollapsed` + // (derived from which section is first-visible) can flip after mount. Re-sync + // to it whenever it changes — but stop once the user has manually toggled, so + // their choice is never clobbered by a later data update. Adjusting state + // during render (storing the previous prop in state) is the React-recommended + // pattern and avoids both the setState-in-effect and ref-during-render lint + // rules. + const [userToggled, setUserToggled] = useState(false); + const [prevDefaultCollapsed, setPrevDefaultCollapsed] = useState(defaultCollapsed); + if (prevDefaultCollapsed !== defaultCollapsed) { + setPrevDefaultCollapsed(defaultCollapsed); + if (!userToggled) setCollapsed(defaultCollapsed); + } const canCollapse = collapsible && !!label; return ( @@ -79,7 +92,10 @@ function TimelineSection({