Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 64 additions & 40 deletions apps/web/src/components/pull-requests/PullRequestDetailPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,10 @@ export function PullRequestDetailPanel({
return (
<div
ref={panelRoot}
className="flex h-full min-h-0 min-w-0 flex-col"
// The header and tabs answer to this panel's own width rather than the
// window's: beside a thread it is a sidebar 272px wide on a wide screen,
// and the phone layout is what fits there.
className="@container/pr flex h-full min-h-0 min-w-0 flex-col"
data-testid="pull-request-detail"
>
<PullRequestDetailHeader
Expand All @@ -419,9 +422,16 @@ export function PullRequestDetailPanel({
/>

{/* The order toggle shares the row but not the tablist: it is not a tab,
and inside one it would answer to the arrow keys as though it were. */}
and inside one it would answer to the arrow keys as though it were.
The tabs are drawn whole at every width; in a narrow panel they close
ranks and the control beside them keeps its glyph and loses its words,
so nothing is pushed past the edge. */}
<div className="flex shrink-0 items-center border-b border-border px-4">
<div role="tablist" aria-label="Pull request" className="flex items-center gap-5">
<div
role="tablist"
aria-label="Pull request"
className="flex shrink-0 items-center gap-5 @max-md/pr:gap-3"
>
<PageTabButton
label={DETAIL_TAB_LABELS.summary}
active={activeTab === "summary"}
Expand Down Expand Up @@ -451,16 +461,25 @@ export function PullRequestDetailPanel({
/>
</div>
{activeTab === "timeline" ? (
<button
type="button"
className={cn(TEXT_BUTTON_CLASS, "ml-auto inline-flex items-center gap-1 pb-2 text-xs")}
aria-label={timelineOrder === "newest" ? "Show oldest first" : "Show newest first"}
data-testid="pull-request-timeline-order"
onClick={() => setTimelineOrder(timelineOrder === "newest" ? "oldest" : "newest")}
<TooltipWrapper
tooltip={timelineOrder === "newest" ? "Show oldest first" : "Show newest first"}
>
<ArrowDownUpIcon aria-hidden className="size-3" />
{timelineOrder === "newest" ? "Newest first" : "Oldest first"}
</button>
<button
type="button"
className={cn(
TEXT_BUTTON_CLASS,
"ml-auto inline-flex shrink-0 items-center gap-1 pb-2 text-xs",
)}
aria-label={timelineOrder === "newest" ? "Show oldest first" : "Show newest first"}
data-testid="pull-request-timeline-order"
onClick={() => setTimelineOrder(timelineOrder === "newest" ? "oldest" : "newest")}
>
<ArrowDownUpIcon aria-hidden className="size-3" />
<span className="@max-md/pr:sr-only">
{timelineOrder === "newest" ? "Newest first" : "Oldest first"}
</span>
</button>
</TooltipWrapper>
) : null}
{/* The rollup the header used to spell out on a line of its own. It is
a way into the Summary's Checks section rather than a label, so it
Expand Down Expand Up @@ -600,7 +619,7 @@ function PullRequestDetailHeader({
return (
<div className="shrink-0 px-4 pt-2 pb-3">
{/* Row 1: where it lives, and what can be done to it. */}
<div className="flex min-w-0 items-start gap-2 max-md:flex-wrap">
<div className="flex min-w-0 items-start gap-2 @max-md/pr:flex-wrap">
<div className="flex h-7 min-w-0 flex-1 items-center gap-1.5 font-mono text-xs text-muted-foreground">
{/* On a phone the list is not on screen, so the way back sits where
a back arrow belongs: first, at the top left. */}
Expand All @@ -624,7 +643,7 @@ function PullRequestDetailHeader({
<span className="shrink-0">Auto-merge on</span>
) : null}
</div>
<div className="flex shrink-0 flex-wrap items-center justify-end gap-1.5 max-md:w-full">
<div className="flex shrink-0 flex-wrap items-center justify-end gap-1.5 @max-md/pr:w-full">
{showCheckout ? (
<PullRequestCheckoutMenu
detail={detail}
Expand Down Expand Up @@ -664,9 +683,10 @@ function PullRequestDetailHeader({
</div>

{/* Row 3: who wrote it, when it last moved, and the one command that
takes the branch on a machine this app is not running on. */}
<div className="mt-2 flex min-w-0 items-center gap-2 text-xs text-muted-foreground">
<span className="flex min-w-0 flex-1 items-center gap-1.5">
takes the branch on a machine this app is not running on. The command
drops to a line of its own before it can squeeze the author out. */}
<div className="mt-2 flex min-w-0 flex-wrap items-center gap-x-2 gap-y-1 text-xs text-muted-foreground">
<span className="flex min-w-0 flex-auto items-center gap-1.5">
{detail.author ? (
<>
<PullRequestActorLabel actor={detail.author} className="font-medium" />
Expand All @@ -678,9 +698,9 @@ function PullRequestDetailHeader({
<PullRequestCheckoutCommand provider={detail.provider} number={detail.number} />
</div>

{/* Row 4: the branches this joins, and how much it changes. Below `md`
the counts drop to a line of their own: the branch names are what the
row is for, and sharing the line leaves them a letter each. */}
{/* Row 4: the branches this joins, and how much it changes. In a narrow
panel the counts drop to a line of their own: the branch names are
what the row is for, and sharing the line leaves them a letter each. */}
<div className="mt-3 flex min-w-0 flex-wrap items-center gap-x-2 gap-y-1 font-mono text-xs text-muted-foreground/70">
<span className="flex min-w-0 flex-1 items-center gap-1.5">
<BranchCopyButton branch={detail.baseBranch} />
Expand Down Expand Up @@ -717,7 +737,7 @@ function PullRequestDetailHeader({
</TooltipWrapper>
) : null}
</span>
<span className="flex shrink-0 items-center gap-2 max-md:w-full">
<span className="flex shrink-0 items-center gap-2 @max-md/pr:w-full">
<span className="flex items-center gap-1">
<FileDiffIcon aria-hidden className="size-3.5" />
{pluralize(detail.changedFiles, "file")}
Expand Down Expand Up @@ -815,7 +835,8 @@ function PullRequestCheckoutMenu({
* The rollup of the checks, beside the tabs: a glyph, a phrase, and a way down
* to the rows it counts. The glyph is drawn here rather than taken whole from
* the presentation module, because inside a button a second tooltip trigger
* would fight the button for the pointer.
* would fight the button for the pointer. In a narrow panel the phrase is for
* screen readers and the tooltip only, and the glyph stands for it.
*/
function PullRequestChecksRollup({
summary,
Expand All @@ -825,23 +846,26 @@ function PullRequestChecksRollup({
readonly onShowChecks: () => void;
}) {
const tone = pullRequestChecksTone(summary.state);
const headline = formatPullRequestChecksHeadline(summary);
return (
<button
type="button"
className={cn(
TEXT_BUTTON_CLASS,
"ml-auto inline-flex shrink-0 items-center gap-1.5 pb-2 text-xs",
)}
data-testid="pull-request-checks-rollup"
onClick={onShowChecks}
>
{tone ? (
<span className={cn("shrink-0", tone.className)}>
<tone.Icon aria-hidden className="size-3.5" />
</span>
) : null}
{formatPullRequestChecksHeadline(summary)}
</button>
<TooltipWrapper tooltip={headline}>
<button
type="button"
className={cn(
TEXT_BUTTON_CLASS,
"ml-auto inline-flex shrink-0 items-center gap-1.5 pb-2 text-xs",
)}
data-testid="pull-request-checks-rollup"
onClick={onShowChecks}
>
{tone ? (
<span className={cn("shrink-0", tone.className)}>
<tone.Icon aria-hidden className="size-3.5" />
</span>
) : null}
<span className="@max-md/pr:sr-only">{headline}</span>
</button>
</TooltipWrapper>
);
}

Expand Down Expand Up @@ -977,11 +1001,11 @@ function PullRequestTitle({
tabIndex={-1}
className="group/title flex min-w-0 flex-1 items-center gap-1.5 rounded-sm focus-ring"
>
{/* A phone's column, and a tablet's, are narrow enough that one truncated
{/* A phone's column, and a sidebar's, are narrow enough that one truncated
line says almost nothing, so there it wraps to two before it gives
up. */}
<span
className="min-w-0 text-base font-semibold leading-snug max-lg:line-clamp-2 lg:truncate"
className="min-w-0 text-base font-semibold leading-snug @max-lg/pr:line-clamp-2 @lg/pr:truncate"
title={detail.title}
>
{detail.title}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ export function PullRequestSummaryTab({
<PullRequestMetaRow icon={<UsersIcon aria-hidden className="size-3.5" />} label="Reviewers">
<span className="flex min-w-0 flex-1 flex-wrap items-center gap-x-3 gap-y-1">
{detail.reviewers.length === 0 ? (
<span className="text-muted-foreground/55">No reviewers</span>
<span className="whitespace-nowrap text-muted-foreground/55">No reviewers</span>
) : (
detail.reviewers.map((reviewer) => (
<PullRequestReviewerLabel key={`${reviewer.kind}:${reviewer.id}`} {...reviewer} />
Expand Down Expand Up @@ -224,7 +224,9 @@ function PullRequestMetaRow({
{icon}
{label}
</span>
<span className="flex min-w-0 items-center gap-2">{children}</span>
{/* The row wraps rather than overflows: in a narrow panel the Request
button sits under the reviewers instead of past the edge. */}
<span className="flex min-w-0 flex-wrap items-center gap-x-2 gap-y-1">{children}</span>
</div>
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -472,8 +472,9 @@ export function PullRequestDetailSkeleton({ onClose }: { readonly onClose?: () =
<Skeleton className="h-2.5 w-16 rounded-full" />
<Skeleton className="h-2.5 w-20 rounded-full" />
</div>
{/* Row 4: base ← head, with the file count at the far end. */}
<div className="mt-3 flex items-center gap-1.5">
{/* Row 4: base ← head, with the file count at the far end, which wraps
under the branches where the header's own would. */}
<div className="mt-3 flex flex-wrap items-center gap-1.5">
<Skeleton className="h-2.5 w-14 rounded-full" />
<Skeleton className="h-2.5 w-3 rounded-full" />
<Skeleton className="h-2.5 w-32 rounded-full" />
Expand Down
Loading