From 47ace94962a714a561d7cfbdbaa4c721ef6b0598 Mon Sep 17 00:00:00 2001 From: Bilal Bakr <62337003+Bil0000@users.noreply.github.com> Date: Mon, 14 Sep 2026 20:46:28 +0300 Subject: [PATCH] fix(web): use consistent PR section toggles (#11763) --- .../PullRequestSummaryTab.test.tsx | 141 ++++++++++++++++++ .../pullRequest/PullRequestSummaryTab.tsx | 124 +++++++-------- 2 files changed, 193 insertions(+), 72 deletions(-) create mode 100644 apps/web/src/components/pullRequest/PullRequestSummaryTab.test.tsx diff --git a/apps/web/src/components/pullRequest/PullRequestSummaryTab.test.tsx b/apps/web/src/components/pullRequest/PullRequestSummaryTab.test.tsx new file mode 100644 index 0000000000000000000000000000000000000000..6bf532e5aefdc069b5eca64e26d8b6d1cd4c3be8 --- /dev/null +++ b/apps/web/src/components/pullRequest/PullRequestSummaryTab.test.tsx @@ -0,0 +1,141 @@ +import { EnvironmentId, ProjectId, type PullRequestDetailView } from "@t3tools/contracts"; +import { act } from "react"; +import { create, type ReactTestRenderer } from "react-test-renderer"; +import { afterEach, beforeEach, expect, it, vi } from "vite-plus/test"; + +vi.mock("~/state/use-atom-command", () => ({ useAtomCommand: () => vi.fn() })); +vi.mock("~/state/pullRequests", () => ({ pullRequestEnvironment: {} })); +vi.mock("~/browser/useOpenLink", () => ({ useOpenLink: () => vi.fn() })); +vi.mock("./PullRequestMarkdown", () => ({ + PullRequestMarkdown: ({ text }: { text: string }) =>

{text}

, +})); + +import { PullRequestSummaryTab } from "./PullRequestSummaryTab"; + +const detail: PullRequestDetailView = { + provider: "github", + projectId: ProjectId.make("project"), + projectTitle: "Project", + workspaceRoot: "/workspace", + repository: "owner/repo", + number: 1, + title: "Test pull request", + body: "Original description", + url: "https://github.com/owner/repo/pull/1", + author: { login: "author", name: null, avatarUrl: null }, + viewer: "author", + state: "open", + isDraft: false, + mergeability: "mergeable", + additions: 1, + deletions: 0, + changedFiles: 1, + headBranch: "feature", + baseBranch: "main", + createdAt: "2026-09-01T00:00:00Z", + updatedAt: "2026-09-01T00:00:00Z", + mergedAt: null, + closedAt: null, + reviewers: [], + labels: [], + checks: [{ name: "Unit tests", status: "success", description: null, url: null }], + comments: [], + commentCount: 0, + commentsTruncated: false, + reviewThreads: [], + commits: [], + mergeCapabilities: { merge: false, squash: false, rebase: false }, + capabilities: { + diff: true, + comment: true, + search: true, + actions: [], + mergeMethods: [], + review: { inlineComment: false, reply: false, resolve: false, verdicts: [] }, + reviewers: { request: false, listCandidates: false }, + edit: { changeRequest: true, comment: true }, + }, + viewerPermissions: { + actions: [], + comment: true, + resolve: false, + verdicts: [], + requestReviewers: false, + }, +}; + +let renderer: ReactTestRenderer; +beforeEach(() => vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true)); +afterEach(() => { + act(() => renderer?.unmount()); + vi.unstubAllGlobals(); +}); + +function render(value = detail) { + return ( + {}} + /> + ); +} + +function heading(title: string) { + const section = renderer.root + .findAllByType("section") + .find((section) => section.props["aria-label"] === title)!; + return section + .findAllByType("button") + .find((button) => button.findAllByType("span").some((span) => span.children.includes(title)))!; +} + +function click(title: string) { + act(() => + heading(title).props.onClick({ nativeEvent: {}, preventDefault() {}, stopPropagation() {} }), + ); +} + +it("toggles checks from their heading and resets sections for another pull request", () => { + act(() => { + renderer = create(render()); + }); + expect( + renderer.root.findAllByType("span").some((span) => span.children.includes("Unit tests")), + ).toBe(false); + click("Checks"); + expect( + renderer.root.findAllByType("span").some((span) => span.children.includes("Unit tests")), + ).toBe(true); + click("Checks"); + expect(heading("Checks").props["aria-expanded"]).toBe(false); + click("Checks"); + click("Description"); + act(() => + renderer.update(render({ ...detail, url: "https://github.com/owner/repo/pull/2", number: 2 })), + ); + expect(heading("Checks").props["aria-expanded"]).toBe(false); + expect(heading("Description").props["aria-expanded"]).toBe(true); +}); + +it("keeps an unsaved description when collapsed and reopened", () => { + act(() => { + renderer = create(render()); + }); + act(() => renderer.root.findByProps({ "aria-label": "Edit description" }).props.onClick()); + act(() => + renderer.root.findByType("textarea").props.onChange({ + target: { value: "Unsaved description" }, + currentTarget: { value: "Unsaved description" }, + nativeEvent: {}, + }), + ); + click("Description"); + expect(heading("Description").props["aria-expanded"]).toBe(false); + click("Description"); + expect(renderer.root.findByType("textarea").props.value).toBe("Unsaved description"); +}); diff --git a/apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx b/apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx index 26aa05bd8b48973a2501d835456728a37fab3e08..7c7c72d8d163e9346a6b249189a9b8e4e912b56a 100644 --- a/apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx +++ b/apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx @@ -15,7 +15,7 @@ import { TagIcon, UsersIcon, } from "lucide-react"; -import { useId, useRef, useState, type ReactNode } from "react"; +import { useRef, useState, type ReactNode } from "react"; import { useAtomCommand } from "~/state/use-atom-command"; import { pullRequestEnvironment } from "~/state/pullRequests"; @@ -237,11 +237,13 @@ function MetaRow({ function Section({ title, defaultOpen = true, + keepMounted = false, actions, children, }: { title: string; defaultOpen?: boolean; + keepMounted?: boolean; /** Heading controls stay separate from the collapse trigger so they remain independently usable. */ actions?: ReactNode; children: ReactNode; @@ -271,6 +273,7 @@ function Section({ } data-pull-request-summary-section > {/* The heading rides the top of the scroll box the way a diff's file header does, so a @@ -292,7 +295,7 @@ function Section({ {actions} - +
{children}
 @@ -334,9 +337,6 @@ export function PullRequestSummaryTab({ // Keyed by the pull request, so opening another one starts at the end of its conversation // rather than wherever the last one had been read back to. const [shown, setShown] = useState({ url: detail.url, count: COMMENT_PAGE }); - const checksId = useId(); - const [expandedChecksUrl, setExpandedChecksUrl] = useState(null); - const showChecks = expandedChecksUrl === detail.url; const shownComments = shown.url === detail.url ? shown.count : COMMENT_PAGE; // Windowed by recency regardless of display order: expanding always reaches further back in // time, whether the newest comment currently reads first or last. @@ -472,7 +472,7 @@ export function PullRequestSummaryTab({ return (
-
+
} label="Reviewers"> @@ -601,7 +601,7 @@ export function PullRequestSummaryTab({
-
+
{bodyScope === detail.url ? ( )}
-
+
-
+
{detail.checks.length === 0 ? (

No checks reported.

) : ( -
-
- Checks - -
-
- {(showChecks ? detail.checks : []).map((check, index) => { - const finding = { kind: "check", check } as const; - const failing = check.status === "failure" || check.status === "cancelled"; - return ( -
- - {/* Only where there is something to fix. A passing check has no failure to + + {/* Only where there is something to fix. A passing check has no failure to reproduce, and the button would be an invitation to waste a thread. */} - {onFixFinding && failing ? ( - - ) : null} -
- ); - })} -
-
+ {onFixFinding && failing ? ( + + ) : null} +
+ ); + }) )} - +