authorgravatar for maria@kuuro.netmaria <maria@kuuro.net> 2026-09-11 01:23:45-03:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-09-11 01:23:45-03:00
log18c5a1d2dfbb02856ec51b54edac82952852a58a
tree6dae7b9c8597db2de478c8bb2f74664055e1295e
parent6c69534a5800a95e1640b0571580561ef71ec461
signature Signed by PGP key B5690EEEBB952194

fix(web): simplify pull request summary sections (#10612)

6 files changed, 152 insertions(+), 141 deletions(-)

apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx+2-2
......@@ -1654,7 +1654,7 @@ export function PullRequestDetailPanel({
16541654 render={
16551655 <Button
16561656 size="xs"
1657 variant="ghost"
1657 variant="outline"
16581658 aria-label={
16591659 handoff?.startsWith("checkout") ? "Checking out..." : "Check out"
16601660 }
......@@ -1734,7 +1734,7 @@ export function PullRequestDetailPanel({
17341734 <span className="inline-flex shrink-0">
17351735 <Button
17361736 size="xs"
1737 variant="default"
1737 variant="destructive-outline"
17381738 disabled={handoff !== null}
17391739 onClick={startResolveConflicts}
17401740 aria-label={
apps/web/src/components/pullRequest/PullRequestMarkdown.tsx+7-1
......@@ -39,7 +39,13 @@ export function PullRequestMarkdown({
3939 [repositoryUrl],
4040 );
4141 return (
42 <div className={cn("space-y-3", className)} data-image-gallery>
42 <div
43 className={cn(
44 "space-y-3 [&_[data-markdown-details]]:border-0 [&_[data-markdown-details-summary]]:text-foreground/80 [&_[data-markdown-details-summary]>svg]:text-muted-foreground/60",
45 className,
46 )}
47 data-image-gallery
48 >
4349 {segments.map((segment) => {
4450 if (segment.kind === "markdown") {
4551 return (
apps/web/src/components/pullRequest/PullRequestRow.tsx+22-30
......@@ -1,4 +1,4 @@
1import { SearchIcon, UserCheckIcon } from "lucide-react";
1import { SearchIcon } from "lucide-react";
22import { PullRequestStackPopover } from "./PullRequestStackPopover";
33import { memo, type RefCallback } from "react";
44
......@@ -14,6 +14,7 @@ import {
1414 PullRequestActorLabel,
1515 PullRequestDiffStat,
1616 PullRequestMetaLine,
17 PullRequestApprovalGlyph,
1718 PullRequestStateGlyph,
1819} from "./pullRequestPresentation";
1920
......@@ -137,6 +138,26 @@ function PullRequestRowImpl({
137138 }
138139 />
139140 ) : null}
141 {/* Only a verdict somebody has actually given: "review required" is the absence of
142 one, and saying so on every unreviewed row would say nothing. */}
143 {entry.reviewDecision === "approved" ? (
144 <PullRequestApprovalGlyph />
145 ) : entry.reviewDecision === "changes-requested" ? (
146 <span className="min-w-0 truncate text-amber-600/90 dark:text-amber-400/80">
147 Changes requested
148 </span>
149 ) : null}
150 {entry.checksState === undefined ? null : (
151 <PullRequestChecksPopover
152 checksState={entry.checksState}
153 environmentId={entry.environmentId}
154 reference={{
155 projectId: entry.projectId,
156 repository: entry.repository,
157 number: entry.number,
158 }}
159 />
160 )}
140161 <PullRequestDiffStat
141162 additions={entry.additions}
142163 deletions={entry.deletions}
......@@ -195,35 +216,6 @@ function PullRequestRowImpl({
195216 labelClassName="sr-only @xs/pr-row-meta:not-sr-only @xs/pr-row-meta:truncate"
196217 />
197218 {entry.labels.length > 0 ? <PullRequestRowLabels labels={entry.labels} /> : null}
198 {/* Only a verdict somebody has actually given: "review required" is the absence of
199 one, and saying so on every unreviewed row would say nothing. */}
200 {entry.reviewDecision === "approved" ? (
201 <Tooltip>
202 <TooltipTrigger render={<span className="inline-flex shrink-0" />}>
203 <UserCheckIcon
204 aria-hidden
205 className="size-3.5 text-emerald-600/90 dark:text-emerald-400/80"
206 />
207 <span className="sr-only">Approved</span>
208 </TooltipTrigger>
209 <TooltipPopup>Approved</TooltipPopup>
210 </Tooltip>
211 ) : entry.reviewDecision === "changes-requested" ? (
212 <span className="min-w-0 truncate text-amber-600/90 dark:text-amber-400/80">
213 Changes requested
214 </span>
215 ) : null}
216 {entry.checksState === undefined ? null : (
217 <PullRequestChecksPopover
218 checksState={entry.checksState}
219 environmentId={entry.environmentId}
220 reference={{
221 projectId: entry.projectId,
222 repository: entry.repository,
223 number: entry.number,
224 }}
225 />
226 )}
227219 </PullRequestMetaLine>
228220 <span className="col-start-2 row-start-2 flex items-center justify-self-end gap-3 whitespace-nowrap text-[11px] text-muted-foreground/70 tabular-nums">
229221 <span className="hidden @sm/pr-row:inline">
apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx+94-94
......@@ -12,14 +12,13 @@ import {
1212 ChevronRightIcon,
1313 GitPullRequestClosedIcon,
1414 HammerIcon,
15 MessageSquareIcon,
1615 PencilIcon,
1716 RotateCcwIcon,
1817 SendIcon,
1918 TagIcon,
2019 UsersIcon,
2120} from "lucide-react";
22import { useRef, useState, type ReactNode } from "react";
21import { useId, useRef, useState, type ReactNode } from "react";
2322
2423import { useAtomCommand } from "~/state/use-atom-command";
2524import { pullRequestEnvironment } from "~/state/pullRequests";
......@@ -187,7 +186,7 @@ function CollapsedComment({
187186 <ChevronDownIcon
188187 aria-hidden
189188 className={cn(
190 "size-3.5 shrink-0 text-muted-foreground transition-transform",
189 "size-3.5 shrink-0 text-muted-foreground/60 transition-transform",
191190 open && "rotate-180",
192191 )}
193192 />
......@@ -229,8 +228,8 @@ function MetaRow({
229228 children: ReactNode;
230229}) {
231230 return (
232 <div className="grid min-h-8 grid-cols-[6rem_minmax(0,1fr)] items-center gap-2 py-1.5 text-xs">
233 <span className="flex min-w-0 items-center gap-1.5 text-muted-foreground">
231 <div className="grid min-w-0 grid-cols-[6rem_minmax(0,1fr)] items-center gap-2 text-xs">
232 <span className="flex items-center gap-1.5 text-muted-foreground">
234233 {icon}
235234 {label}
236235 </span>
......@@ -241,16 +240,13 @@ function MetaRow({
241240
242241function Section({
243242 title,
244 count,
245243 defaultOpen = true,
246244 actions,
247245 children,
248246}: {
249247 title: string;
250 count?: number;
251248 defaultOpen?: boolean;
252 /** Controls riding on the heading row itself. A sibling of the trigger, not a child of it —
253 a button cannot hold a button — and only while open, since they act on what is shown. */
249 /** Heading controls stay separate from the collapse trigger so they remain independently usable. */
254250 actions?: ReactNode;
255251 children: ReactNode;
256252}) {
......@@ -286,24 +282,19 @@ function Section({
286282 where it started. Opaque, because the rows it covers scroll beneath it. */}
287283 <div
288284 ref={headingRef}
289 className="sticky top-0 z-10 flex w-full items-center border-t border-border/60 bg-background pr-4"
285 className="sticky top-0 z-10 flex w-full items-center bg-background pr-4"
290286 >
291 {/* Title first, chevron riding to its right, count last: the row reads as a heading
292 with an affordance rather than a tree node. */}
293 <CollapsibleTrigger className="flex min-w-0 flex-1 items-center gap-1.5 px-4 py-3 text-left text-sm font-medium">
287 <CollapsibleTrigger className="flex min-w-0 flex-1 items-center gap-1.5 px-4 py-3 text-left text-xs font-medium text-muted-foreground hover:text-foreground">
294288 <span>{title}</span>
295289 <ChevronRightIcon
296290 aria-hidden
297291 className={cn(
298 "size-3.5 text-muted-foreground transition-transform",
292 "size-3.5 text-muted-foreground/60 transition-transform",
299293 open && "rotate-90",
300294 )}
301295 />
302 {count === undefined ? null : (
303 <span className="text-xs tabular-nums text-muted-foreground">{count}</span>
304 )}
305296 </CollapsibleTrigger>
306 {open ? actions : null}
297 {actions}
307298 </div>
308299 <CollapsiblePanel>
309300 <div className="px-4 pb-4">{children}</div>
......@@ -461,6 +452,9 @@ export function PullRequestSummaryTab({
461452 // Keyed by the pull request, so opening another one starts at the end of its conversation
462453 // rather than wherever the last one had been read back to.
463454 const [shown, setShown] = useState({ url: detail.url, count: COMMENT_PAGE });
455 const checksId = useId();
456 const [expandedChecksUrl, setExpandedChecksUrl] = useState<string | null>(null);
457 const showChecks = expandedChecksUrl === detail.url;
464458 const shownComments = shown.url === detail.url ? shown.count : COMMENT_PAGE;
465459 // Windowed by recency regardless of display order: expanding always reaches further back in
466460 // time, whether the newest comment currently reads first or last.
......@@ -596,8 +590,8 @@ export function PullRequestSummaryTab({
596590
597591 return (
598592 <div className="h-full overflow-y-auto" data-pull-request-summary-scroll>
599 <section className="px-4 py-3">
600 <div>
593 <section className="px-4 py-2.5">
594 <div className="space-y-2">
601595 <MetaRow icon={<UsersIcon className="size-3.5" />} label="Reviewers">
602596 <span className="flex min-w-0 flex-wrap items-center gap-1.5">
603597 {reviewerEntries.length === 0 ? (
......@@ -700,7 +694,7 @@ export function PullRequestSummaryTab({
700694 return (
701695 <span
702696 key={label.name}
703 className="inline-flex max-w-48 items-center gap-1.5 rounded-full border border-border/70 bg-muted/40 py-0.5 pl-1.5 pr-2 text-xs"
697 className="inline-flex max-w-48 items-center gap-1.5 rounded-full bg-muted/40 py-0.5 pl-1.5 pr-2 text-xs"
704698 >
705699 <span
706700 aria-hidden
......@@ -722,19 +716,10 @@ export function PullRequestSummaryTab({
722716 </span>
723717 </MetaRow>
724718 ) : null}
725 <MetaRow icon={<MessageSquareIcon className="size-3.5" />} label="Comments">
726 {activityPending
727 ? "Loading conversation…"
728 : activityError
729 ? "Conversation unavailable"
730 : detail.commentCount === 1
731 ? "1 comment"
732 : `${detail.commentCount} comments`}
733 </MetaRow>
734719 </div>
735720 </section>
736721
737 <Section title="Description">
722 <section aria-label="Description" className="px-4 pt-2 pb-4">
738723 <div className="group">
739724 {bodyScope === detail.url ? (
740725 <PullRequestMarkdownEditor
......@@ -781,81 +766,96 @@ export function PullRequestSummaryTab({
781766 onRefresh={onRefresh}
782767 />
783768 </div>
784 </Section>
769 </section>
785770
786 <Section title="Checks" count={detail.checks.length}>
771 <section aria-label="Checks" className="px-4 py-3">
787772 {detail.checks.length === 0 ? (
788773 <p className="text-xs text-muted-foreground">No checks reported.</p>
789774 ) : (
790 <div className="space-y-0.5">
791 {detail.checks.map((check, index) => {
792 const finding = { kind: "check", check } as const;
793 const failing = check.status === "failure" || check.status === "cancelled";
794 return (
795 <div
796 // Position too: the host decides how many runs share a name, and a repeated
797 // key would be a rendering fault on top of whatever the list already says.
798 key={`${index}:${check.name}:${check.url ?? ""}`}
799 className="group flex items-center gap-1 rounded-md pr-1 hover:bg-accent/60"
800 >
801 <button
802 type="button"
803 disabled={!check.url}
804 onClick={() => check.url && openCheck(check.url)}
805 className={cn(
806 "flex min-w-0 flex-1 items-center gap-2 rounded-md px-2 py-1.5 text-left text-xs",
807 check.url ? undefined : "cursor-default",
808 )}
775 <div>
776 <div className="flex items-center gap-1 text-xs">
777 <span className="font-medium text-muted-foreground">Checks</span>
778 <Button
779 size="icon-xs"
780 variant="ghost-muted"
781 aria-label={showChecks ? "Hide checks" : "Show checks"}
782 aria-expanded={showChecks}
783 aria-controls={checksId}
784 onClick={() => setExpandedChecksUrl(showChecks ? null : detail.url)}
785 >
786 <ChevronRightIcon
787 aria-hidden
788 className={cn("size-3.5 text-muted-foreground/60", showChecks && "rotate-90")}
789 />
790 </Button>
791 </div>
792 <div id={checksId} className={showChecks ? "mt-2" : "hidden"}>
793 {(showChecks ? detail.checks : []).map((check, index) => {
794 const finding = { kind: "check", check } as const;
795 const failing = check.status === "failure" || check.status === "cancelled";
796 return (
797 <div
798 // Position too: the host decides how many runs share a name, and a repeated
799 // key would be a rendering fault on top of whatever the list already says.
800 key={`${index}:${check.name}:${check.url ?? ""}`}
801 className="group flex items-center gap-2 rounded-md pr-1 hover:bg-accent/60"
809802 >
810 <PullRequestCheckStatusIcon status={check.status} />
811 <span className="min-w-0 flex-1 truncate">{check.name}</span>
812 <span className="shrink-0 text-muted-foreground">
813 {pullRequestCheckStatusLabel(check)}
814 </span>
815 </button>
816 {/* Only where there is something to fix. A passing check has no failure to
817 reproduce, and the button would be an invitation to waste a thread. */}
818 {onFixFinding && failing ? (
819 <Button
820 size="xs"
821 variant="ghost"
822 className="shrink-0"
823 disabled={pendingFinding !== null && pendingFinding !== undefined}
824 onClick={() => onFixFinding(finding)}
803 <button
804 type="button"
805 disabled={!check.url}
806 onClick={() => check.url && openCheck(check.url)}
807 className={cn(
808 "flex min-w-0 flex-1 items-start gap-2 rounded-md px-2 py-2 text-left text-xs leading-5 [&>svg]:mt-0.5",
809 check.url ? undefined : "cursor-default",
810 )}
825811 >
826 <HammerIcon className="size-3" />
827 {pendingFinding === pullRequestFindingKey(finding)
828 ? "Preparing..."
829 : fixCheckLabel}
830 </Button>
831 ) : null}
832 </div>
833 );
834 })}
812 <PullRequestCheckStatusIcon status={check.status} />
813 <span className="min-w-0 flex-1 wrap-anywhere">{check.name}</span>
814 <span className="shrink-0 text-muted-foreground">
815 {pullRequestCheckStatusLabel(check)}
816 </span>
817 </button>
818 {/* Only where there is something to fix. A passing check has no failure to
819 reproduce, and the button would be an invitation to waste a thread. */}
820 {onFixFinding && failing ? (
821 <Button
822 size="xs"
823 variant="ghost"
824 className="shrink-0"
825 disabled={pendingFinding !== null && pendingFinding !== undefined}
826 onClick={() => onFixFinding(finding)}
827 >
828 <HammerIcon className="size-3" />
829 {pendingFinding === pullRequestFindingKey(finding)
830 ? "Preparing..."
831 : fixCheckLabel}
832 </Button>
833 ) : null}
834 </div>
835 );
836 })}
837 </div>
835838 </div>
836839 )}
837 </Section>
840 </section>
838841
839842 <Section
840843 title="Comments"
841 {...(activityPending || activityError ? {} : { count: detail.commentCount })}
842844 actions={
843 !activityPending && !activityError && detail.comments.length > 0 ? (
844 <Button
845 size="xs"
846 variant="ghost"
847 className="h-7 shrink-0 px-2 text-[10px] text-muted-foreground"
848 aria-label={
849 commentOrder === "newest"
850 ? "Show oldest comments first"
851 : "Show newest comments first"
852 }
853 onClick={() => setCommentOrder((value) => (value === "newest" ? "oldest" : "newest"))}
854 >
855 <ArrowDownUpIcon aria-hidden className="size-3" />
856 {commentOrder === "newest" ? "Newest first" : "Oldest first"}
857 </Button>
858 ) : null
845 <Button
846 size="xs"
847 variant="ghost"
848 className="h-7 shrink-0 px-2 text-[10px] text-muted-foreground"
849 aria-label={
850 commentOrder === "newest"
851 ? "Show oldest comments first"
852 : "Show newest comments first"
853 }
854 onClick={() => setCommentOrder((value) => (value === "newest" ? "oldest" : "newest"))}
855 >
856 <ArrowDownUpIcon aria-hidden className="size-3" />
857 {commentOrder === "newest" ? "Newest first" : "Oldest first"}
858 </Button>
859859 }
860860 >
861861 {activityPending ? (
apps/web/src/components/pullRequest/ThreadPullRequestsPanel.tsx+10-13
......@@ -29,6 +29,7 @@ import { pullRequestListLines, type PullRequestListLine } from "./pullRequestLis
2929import {
3030 PullRequestActorAvatar,
3131 PullRequestDiffStat,
32 PullRequestApprovalGlyph,
3233 PullRequestStateGlyph,
3334 pullRequestChecksStatePresentation,
3435} from "./pullRequestPresentation";
......@@ -111,27 +112,23 @@ function LinkRow({
111112 <span className="min-w-0 flex-1 truncate text-sm">
112113 {snapshot?.title ?? link.repository}
113114 </span>
114 {/* Right-aligned signals, in the order a reviewer scans them: are checks green,
115 has someone ruled, how big is it. Each is absent rather than neutral when the
115 {/* Match the full PR list: review verdict, checks, then diff counts.
116 Each is absent rather than neutral when the
116117 host said nothing, so a row without them reads as unknown, not as fine. */}
117118 <span className="ml-auto flex shrink-0 items-center gap-1.5 text-[11px]">
118 {snapshot?.checksState ? <ChecksGlyph state={snapshot.checksState} /> : null}
119119 {snapshot?.state === "open" &&
120120 (snapshot.reviewDecision === "approved" ||
121121 snapshot.reviewDecision === "changes-requested") ? (
122 <span
123 className={cn(
124 snapshot.reviewDecision === "approved"
125 ? "text-emerald-600/90 dark:text-emerald-400/80"
126 : "text-amber-600/90 dark:text-amber-400/80",
127 )}
128 >
129 {snapshot.reviewDecision === "approved" ? "Approved" : "Changes requested"}
130 </span>
122 snapshot.reviewDecision === "approved" ? (
123 <PullRequestApprovalGlyph />
124 ) : (
125 <span className="text-amber-600/90 dark:text-amber-400/80">Changes requested</span>
126 )
131127 ) : null}
132128 {snapshot?.state === "open" && snapshot.mergeability === "conflicting" ? (
133129 <span className="text-destructive">Conflicts</span>
134130 ) : null}
131 {snapshot?.checksState ? <ChecksGlyph state={snapshot.checksState} /> : null}
135132 <PullRequestDiffStat
136133 additions={snapshot?.additions ?? 0}
137134 deletions={snapshot?.deletions ?? 0}
......@@ -173,7 +170,7 @@ function LinkRow({
173170 : `${link.host}/${link.repository}`}
174171 </span>
175172 {snapshot?.updatedAt ? (
176 <span className="shrink-0">· {formatRelativeTimeLabel(snapshot.updatedAt)}</span>
173 <span className="ml-auto shrink-0">{formatRelativeTimeLabel(snapshot.updatedAt)}</span>
177174 ) : null}
178175 </span>
179176 </a>
apps/web/src/components/pullRequest/pullRequestPresentation.tsx+17-1
......@@ -17,6 +17,7 @@ import {
1717 GitPullRequestDraftIcon,
1818 GitPullRequestIcon,
1919 TriangleAlertIcon,
20 UserCheckIcon,
2021} from "lucide-react";
2122import { Children, isValidElement, type ReactNode } from "react";
2223
......@@ -32,6 +33,21 @@ interface StatePresentation {
3233 readonly Icon: typeof GitPullRequestIcon;
3334}
3435
36export function PullRequestApprovalGlyph() {
37 return (
38 <Tooltip>
39 <TooltipTrigger render={<span className="inline-flex shrink-0" />}>
40 <UserCheckIcon
41 aria-hidden
42 className={cn("size-3.5", CHECK_STATUS_PRESENTATION.success.toneClassName)}
43 />
44 <span className="sr-only">Approved</span>
45 </TooltipTrigger>
46 <TooltipPopup>Approved</TooltipPopup>
47 </Tooltip>
48 );
49}
50
3551/**
3652 * How a pull request's state reads on this page. Open, closed, merged, and draft use the same
3753 * ink as the thread badge in `ThreadStatusIndicators`, so one pull request cannot look like two
......@@ -173,7 +189,7 @@ const CHECKS_STATE_PRESENTATION = {
173189 passing: {
174190 label: "All checks have passed",
175191 Icon: CircleCheckIcon,
176 toneClassName: "text-emerald-600 dark:text-emerald-300/90",
192 toneClassName: CHECK_STATUS_PRESENTATION.success.toneClassName,
177193 },
178194 failing: {
179195 label: "Some checks were not successful",