feat(web): auto-expand the first group in the changes panel (#1169)

2026-05-30 18:07:36 +01:00
parent 55ed674a15
commit 9f9b5c3f72
6 changed files with 168 additions and 5 deletions
@@ -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 (
    <div className="flex flex-col">
@@ -187,6 +196,7 @@ function ChangesPanelTimeline(props: TimelineProps) {
            isLast={!showCommitsList}
            onOpenDiff={props.onOpenDiffFile}
            repoDisplayName={props.repoDisplayName}
+            defaultCollapsed={firstSection !== "pr"}
          />
        </div>
      )}
@@ -212,6 +222,7 @@ function ChangesPanelTimeline(props: TimelineProps) {
        <CommitsSection
          commits={mergedCommits}
          isLast={!showCommits}
+          defaultCollapsed={firstSection !== "commits"}
          onOpenCommitDetail={props.onOpenCommitDetail}
          onRevertCommit={props.onRevertCommit}
          onAmendCommit={props.dialogs.handleOpenAmendDialog}
@@ -93,3 +93,46 @@ describe("buildPrByRepoMap", () => {
    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<Parameters<typeof firstVisibleSection>[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");
+  });
+});
@@ -138,6 +138,32 @@ export function filterUnpushedCommits<T extends { commit_sha: string }>(
  );
 }

+/** 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;
@@ -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(<PRFilesSection files={[prFile("a.ts")]} isLast onOpenDiff={vi.fn()} />);
+    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(
+      <PRFilesSection
+        files={[prFile("a.ts")]}
+        isLast
+        onOpenDiff={vi.fn()}
+        defaultCollapsed={false}
+      />,
+    );
+    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(
+      <CommitsSection
+        commits={[
+          {
+            commit_sha: "abc123",
+            commit_message: "first",
+            insertions: 1,
+            deletions: 0,
+            pushed: false,
+            repository_name: undefined,
+          },
+        ]}
+        isLast
+        defaultCollapsed={false}
+      />,
+    );
+    const toggle = screen.getByTestId(COMMITS_SECTION_TOGGLE_TID);
+    expect(toggle.getAttribute(ARIA_EXPANDED)).toBe("true");
+    expect(screen.getByTestId(COMMIT_ROW_TID)).toBeTruthy();
+  });
+});
@@ -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({
              <button
                type="button"
                className="flex items-center gap-1 text-[11px] font-medium uppercase tracking-wider text-foreground/70 cursor-pointer hover:text-foreground/90"
-                onClick={() => setCollapsed((c) => !c)}
+                onClick={() => {
+                  setUserToggled(true);
+                  setCollapsed((c) => !c);
+                }}
                aria-expanded={!collapsed}
                data-testid={`${testId ?? label.toLowerCase()}-collapse-toggle`}
              >
@@ -136,6 +152,8 @@ type CommitsSectionProps = {
  perRepoStatus?: Array<{ repository_name: string; ahead: number }>;
  /** Existing PR URL keyed by repository_name; "" key for single-repo. */
  prByRepo?: Record<string, string | undefined>;
+  /** Initial collapse state. Defaults to collapsed; the panel expands it when it is the first visible section. */
+  defaultCollapsed?: boolean;
 };

 // Commits grouping shares the helper above with files — see @/lib/group-by-repo.
@@ -153,6 +171,7 @@ export function CommitsSection({
  repoBaseBranch,
  perRepoStatus,
  prByRepo,
+  defaultCollapsed = true,
 }: CommitsSectionProps) {
  const groups = groupByRepositoryName(commits, (c) => c.repository_name);
  const aheadByRepo = new Map((perRepoStatus ?? []).map((s) => [s.repository_name, s.ahead]));
@@ -179,7 +198,7 @@ export function CommitsSection({
      label="Commits"
      count={commits.length}
      isLast={isLast}
-      defaultCollapsed
+      defaultCollapsed={defaultCollapsed}
      data-testid="commits-section"
      action={sectionAction}
    >
@@ -529,6 +548,8 @@ type PRFilesSectionProps = {
  onOpenDiff: (path: string, options?: OpenDiffOptions) => void;
  /** Maps a repository_name to a human-readable label (used for the per-repo header). */
  repoDisplayName?: (repositoryName: string) => string | undefined;
+  /** Initial collapse state. Defaults to collapsed; the panel expands it when it is the first visible section. */
+  defaultCollapsed?: boolean;
 };

 export function PRFilesSection({
@@ -536,6 +557,7 @@ export function PRFilesSection({
  isLast,
  onOpenDiff,
  repoDisplayName,
+  defaultCollapsed = true,
 }: PRFilesSectionProps) {
  return (
    <TimelineSection
@@ -543,7 +565,7 @@ export function PRFilesSection({
      label="PR Changes"
      count={files.length}
      isLast={isLast}
-      defaultCollapsed
+      defaultCollapsed={defaultCollapsed}
      data-testid="pr-changes-section"
    >
      {files.length > 0 && (
@@ -318,6 +318,18 @@ test.describe("Changes panel section ordering", () => {
    // PR files should appear above commits
    await expectAbove(prFiles, commits);

+    // Review mode (PR, no local changes): PR Changes is the first visible
+    // section, so it is expanded by default — the panel never opens looking
+    // empty. Commits, being second, stays collapsed.
+    await expect(testPage.getByTestId("pr-changes-section-collapse-toggle")).toHaveAttribute(
+      "aria-expanded",
+      "true",
+    );
+    await expect(testPage.getByTestId("commits-section-collapse-toggle")).toHaveAttribute(
+      "aria-expanded",
+      "false",
+    );
+
    // No unstaged or staged sections
    await expect(testPage.getByTestId("unstaged-files-section")).not.toBeVisible();
    await expect(testPage.getByTestId("staged-files-section")).not.toBeVisible();