Skip to content

Commit 0437edb

Browse files
maria-rcksmaria-rcks
committed
feat(web): refresh pull request details
1 parent 4e4d9b3 commit 0437edb

11 files changed

Lines changed: 555 additions & 441 deletions

‎apps/web/src/components/ChatView.tsx‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6160,7 +6160,6 @@ function ChatViewContent(props: ChatViewProps) {
61606160
? "thread"
61616161
: "page"
61626162
}
6163-
chromeVariant="collapse"
61646163
composerDraftTarget={composerDraftTarget}
61656164
onStateChange={handlePullRequestTabStatusChange}
61666165
/>

‎apps/web/src/components/HeaderFilterMenu.tsx‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,14 @@
11
import { ChevronDownIcon } from "lucide-react";
2+
import type { ReactNode } from "react";
23

34
import { Button } from "./ui/button";
45
import { Menu, MenuPopup, MenuRadioGroup, MenuRadioItem, MenuTrigger } from "./ui/menu";
56

67
export interface HeaderFilterMenuOption<Value extends string> {
78
value: Value;
89
label: string;
10+
icon?: ReactNode;
11+
disabledReason?: string;
912
}
1013

1114
export function HeaderFilterMenu<Value extends string>({
@@ -37,8 +40,16 @@ export function HeaderFilterMenu<Value extends string>({
3740
<MenuPopup align={align} side="bottom" className={popupClassName ?? "min-w-40"}>
3841
<MenuRadioGroup value={current.value} onValueChange={(next) => onChange(next as Value)}>
3942
{options.map((option) => (
40-
<MenuRadioItem key={option.value} value={option.value}>
41-
{option.label}
43+
<MenuRadioItem
44+
key={option.value}
45+
value={option.value}
46+
disabled={option.disabledReason !== undefined}
47+
title={option.disabledReason}
48+
>
49+
<span className="flex min-w-0 items-center gap-2">
50+
{option.icon}
51+
{option.label}
52+
</span>
4253
</MenuRadioItem>
4354
))}
4455
</MenuRadioGroup>

‎apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx‎

Lines changed: 266 additions & 332 deletions
Large diffs are not rendered by default.

‎apps/web/src/components/pullRequest/PullRequestGhosts.tsx‎

Lines changed: 91 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -45,46 +45,113 @@ export function PullRequestListGhost({
4545
<GhostBar className="size-4 rounded-full" />
4646
<div className="min-w-0 space-y-1.5">
4747
<GhostBar className={cn("h-3.5", TITLE_WIDTHS[index % TITLE_WIDTHS.length])} />
48-
<GhostBar
49-
className={cn("bg-muted-foreground/10", META_WIDTHS[index % META_WIDTHS.length])}
50-
/>
48+
<GhostBar className={META_WIDTHS[index % META_WIDTHS.length]} />
5149
</div>
5250
<div className="flex flex-col items-end gap-1.5">
5351
<GhostBar className="w-12" />
54-
<GhostBar className="w-16 bg-muted-foreground/10" />
52+
<GhostBar className="w-16" />
5553
</div>
5654
</div>
5755
))}
5856
</div>
5957
);
6058
}
6159

62-
/** The summary's own shape: a title, a byline, the facts rows, the description. */
60+
/**
61+
* The detail panel's current expanded shape. Keeping the chrome, summary facts, and description
62+
* boundaries in the ghost prevents the loaded pull request from replacing one layout with
63+
* another a moment later.
64+
*/
6365
export function PullRequestDetailGhost() {
6466
return (
6567
<div
6668
role="status"
6769
aria-label="Loading pull request"
68-
className="animate-ghost-pulse space-y-6 px-4 py-5"
70+
className="animate-ghost-pulse flex h-full min-h-0 flex-col overflow-hidden bg-background"
6971
>
70-
<div className="space-y-2">
71-
<GhostBar className="h-5 w-4/5" />
72-
<GhostBar className="w-2/5 bg-muted-foreground/10" />
72+
<div className="shrink-0 border-b border-border/60">
73+
<div className="flex h-7 items-center justify-between gap-3 px-4">
74+
<div className="flex min-w-0 flex-1 items-center gap-1.5">
75+
<GhostBar className="w-24" />
76+
<GhostBar className="w-9" />
77+
</div>
78+
<div className="flex shrink-0 items-center gap-1">
79+
<GhostBar className="h-5 w-16 rounded-md" />
80+
<GhostBar className="size-5 rounded-md" />
81+
</div>
82+
</div>
83+
84+
<div className="px-4 pb-4 pt-1">
85+
<GhostBar className="h-5 w-4/5 max-w-md" />
86+
<div className="mt-2 flex items-center gap-1.5">
87+
<GhostBar className="size-4 rounded-full" />
88+
<GhostBar className="w-24" />
89+
</div>
90+
<div className="mt-4 flex min-w-0 items-center gap-2">
91+
<GhostBar className="h-6 w-24 rounded-md" />
92+
<GhostBar className="size-3 rounded-full" />
93+
<GhostBar className="h-6 w-32 rounded-md" />
94+
<div className="ml-auto flex shrink-0 items-center gap-2">
95+
<GhostBar className="w-10" />
96+
<GhostBar className="w-20" />
97+
</div>
98+
</div>
99+
</div>
100+
101+
<div className="flex min-h-10 items-center justify-between gap-3 border-t border-border/60 px-4 py-2">
102+
<div className="flex items-center gap-1 p-0.5">
103+
<GhostBar className="h-6 w-16 rounded-md" />
104+
<GhostBar className="h-6 w-16 rounded-md" />
105+
<GhostBar className="h-6 w-12 rounded-md" />
106+
</div>
107+
<GhostBar className="w-20" />
108+
</div>
73109
</div>
74-
<div className="space-y-3">
75-
{Array.from({ length: 4 }, (_, index) => (
76-
<div key={index} className="flex items-center gap-3">
110+
111+
<div className="min-h-0 flex-1 overflow-hidden">
112+
<section className="px-4 py-3">
113+
<div className="grid min-h-8 grid-cols-[6rem_minmax(0,1fr)] items-center gap-2">
114+
<div className="flex items-center gap-1.5">
115+
<GhostBar className="size-3.5 rounded-full" />
116+
<GhostBar className="w-14" />
117+
</div>
118+
<div className="flex items-center gap-1">
119+
<GhostBar className="size-4 rounded-full" />
120+
<GhostBar className="size-4 rounded-full" />
121+
<GhostBar className="ml-1 size-5 rounded-md" />
122+
</div>
123+
</div>
124+
<div className="grid min-h-8 grid-cols-[6rem_minmax(0,1fr)] items-center gap-2">
125+
<div className="flex items-center gap-1.5">
126+
<GhostBar className="size-3.5 rounded-full" />
127+
<GhostBar className="w-10" />
128+
</div>
129+
<div className="flex items-center gap-1">
130+
<GhostBar className="h-5 w-24 rounded-full" />
131+
<GhostBar className="h-5 w-20 rounded-full" />
132+
</div>
133+
</div>
134+
<div className="grid min-h-8 grid-cols-[6rem_minmax(0,1fr)] items-center gap-2">
135+
<div className="flex items-center gap-1.5">
136+
<GhostBar className="size-3.5 rounded-full" />
137+
<GhostBar className="w-14" />
138+
</div>
139+
<GhostBar className="w-20" />
140+
</div>
141+
</section>
142+
143+
<section className="border-t border-border/60">
144+
<div className="flex min-h-11 items-center gap-1.5 px-4 py-3">
145+
<GhostBar className="h-4 w-24" />
77146
<GhostBar className="size-3.5 rounded-full" />
78-
<GhostBar className="w-20 bg-muted-foreground/10" />
79-
<GhostBar className={TITLE_WIDTHS[(index + 1) % TITLE_WIDTHS.length]} />
80147
</div>
81-
))}
82-
</div>
83-
<div className="space-y-2 pt-1">
84-
<GhostBar className="w-full bg-muted-foreground/10" />
85-
<GhostBar className="w-11/12 bg-muted-foreground/10" />
86-
<GhostBar className="w-4/5 bg-muted-foreground/10" />
87-
<GhostBar className="w-2/3 bg-muted-foreground/10" />
148+
<div className="space-y-2 px-4 pb-4">
149+
<GhostBar className="w-full" />
150+
<GhostBar className="w-11/12" />
151+
<GhostBar className="w-4/5" />
152+
<GhostBar className="w-2/3" />
153+
</div>
154+
</section>
88155
</div>
89156
</div>
90157
);
@@ -113,7 +180,7 @@ export function PullRequestTimelineGhost({ rows = 6 }: { rows?: number }) {
113180
<div key={index} className="relative pb-5">
114181
<GhostBar className="absolute -left-[1.55rem] top-1 size-2 rounded-full" />
115182
<GhostBar className={cn("h-3.5", TITLE_WIDTHS[index % TITLE_WIDTHS.length])} />
116-
<GhostBar className="mt-1.5 w-16 bg-muted-foreground/10" />
183+
<GhostBar className="mt-1.5 w-16" />
117184
</div>
118185
))}
119186
</div>
@@ -134,8 +201,8 @@ export function PullRequestConversationGhost({ rows = 3 }: { rows?: number }) {
134201
<GhostBar className="size-5 shrink-0 rounded-full" />
135202
<div className="flex-1 space-y-1.5">
136203
<GhostBar className={META_WIDTHS[index % META_WIDTHS.length]} />
137-
<GhostBar className="w-full bg-muted-foreground/10" />
138-
<GhostBar className="w-3/4 bg-muted-foreground/10" />
204+
<GhostBar className="w-full" />
205+
<GhostBar className="w-3/4" />
139206
</div>
140207
</div>
141208
))}

‎apps/web/src/components/pullRequest/PullRequestListFilters.tsx‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import { cn } from "~/lib/utils";
2525
import { getSourceControlPresentationForKind } from "~/sourceControlPresentation";
2626
import { ProjectFavicon } from "../ProjectFavicon";
2727
import { InputGroup, InputGroupAddon, InputGroupInput } from "../ui/input-group";
28+
import { Button } from "../ui/button";
2829

2930
import {
3031
Menu,
@@ -261,12 +262,14 @@ export function PullRequestFiltersMenu({
261262
return (
262263
<Menu>
263264
<MenuTrigger
264-
className={cn(
265-
// The icon-button size that pairs with a full-height input, so the two read as one strip.
266-
"relative inline-flex size-9 shrink-0 items-center justify-center rounded-lg border border-input text-muted-foreground transition-colors hover:bg-accent/50 hover:text-foreground sm:size-8",
267-
filtered && "text-foreground",
268-
)}
269-
aria-label="Filter pull requests"
265+
render={
266+
<Button
267+
className={cn("relative", filtered && "[--control-icon-color:currentColor]")}
268+
size="icon"
269+
variant="outline"
270+
aria-label="Filter pull requests"
271+
/>
272+
}
270273
>
271274
<ListFilterIcon className="size-4" />
272275
{filtered ? (

‎apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -196,12 +196,12 @@ function MetaRow({
196196
children: ReactNode;
197197
}) {
198198
return (
199-
<div className="flex items-center gap-2 py-1.5 text-xs">
200-
<span className="flex w-24 shrink-0 items-center gap-1.5 text-muted-foreground">
199+
<div className="grid min-h-8 grid-cols-[6rem_minmax(0,1fr)] items-center gap-2 py-1.5 text-xs">
200+
<span className="flex min-w-0 items-center gap-1.5 text-muted-foreground">
201201
{icon}
202202
{label}
203203
</span>
204-
<span className="min-w-0 flex-1 text-foreground">{children}</span>
204+
<span className="min-w-0 text-foreground">{children}</span>
205205
</div>
206206
);
207207
}

‎apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
groupPullRequestTimelineConversations,
1717
handoffPrompt,
1818
handoffReviewComments,
19+
isStackedPullRequestBase,
1920
isThreadOwnPullRequest,
2021
orderPullRequestComments,
2122
pullRequestActionMenuHasGroup,
@@ -101,6 +102,47 @@ describe("pull request composer target", () => {
101102
});
102103
});
103104

105+
describe("stacked pull request classification", () => {
106+
it("requires a known default branch", () => {
107+
expect(isStackedPullRequestBase("main", [{ name: "main", isDefault: false }])).toBe(false);
108+
});
109+
110+
it("recognizes local and remote forms of the default branch", () => {
111+
expect(
112+
isStackedPullRequestBase("main", [{ name: "main", isDefault: true, isRemote: false }]),
113+
).toBe(false);
114+
expect(
115+
isStackedPullRequestBase("main", [
116+
{ name: "origin/main", isDefault: true, isRemote: true, remoteName: "origin" },
117+
]),
118+
).toBe(false);
119+
});
120+
121+
it("classifies a non-default base as stacked once the default is known", () => {
122+
expect(
123+
isStackedPullRequestBase("feature-base", [
124+
{ name: "origin/main", isDefault: true, isRemote: true, remoteName: "origin" },
125+
]),
126+
).toBe(true);
127+
});
128+
129+
it("does not mistake a nested branch suffix for the default branch", () => {
130+
expect(
131+
isStackedPullRequestBase("main", [
132+
{
133+
name: "origin/feature/main",
134+
isDefault: true,
135+
isRemote: true,
136+
remoteName: "origin",
137+
},
138+
]),
139+
).toBe(true);
140+
expect(
141+
isStackedPullRequestBase("1.0", [{ name: "release/1.0", isDefault: true, isRemote: false }]),
142+
).toBe(true);
143+
});
144+
});
145+
104146
describe("ordering comments", () => {
105147
it("reverses the chronological list for newest first, and leaves oldest first alone", () => {
106148
const comments = [{ createdAt: "a" }, { createdAt: "b" }, { createdAt: "c" }];

‎apps/web/src/components/pullRequest/pullRequestDetail.logic.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import type {
1010
PullRequestReviewThread,
1111
PullRequestState,
1212
PullRequestUpdateMethod,
13+
VcsRef,
1314
} from "@t3tools/contracts";
1415

1516
import { inferReviewCommentFenceLanguage, type ReviewCommentContext } from "~/reviewCommentContext";
@@ -73,6 +74,20 @@ export function pullRequestActionMenuHasGroup(
7374
return showsDraftToggle || showsAutoMerge || showsMergeMethods;
7475
}
7576

77+
export function isStackedPullRequestBase(
78+
baseBranch: string,
79+
refs: ReadonlyArray<Pick<VcsRef, "name" | "isDefault" | "isRemote" | "remoteName">>,
80+
): boolean {
81+
const defaultRef = refs.find((refName) => refName.isDefault);
82+
if (!defaultRef) return false;
83+
if (defaultRef.isRemote !== true) return defaultRef.name !== baseBranch;
84+
const remotePrefix = `${defaultRef.remoteName ?? defaultRef.name.split("/")[0]}/`;
85+
const defaultBranch = defaultRef.name.startsWith(remotePrefix)
86+
? defaultRef.name.slice(remotePrefix.length)
87+
: defaultRef.name;
88+
return defaultBranch !== baseBranch;
89+
}
90+
7691
/** Plain-language state, shown beside the author. Conflicts are a merge signal, not a state. */
7792
export function describePullRequestState(state: PullRequestState, isDraft: boolean): string {
7893
if (state === "merged") return "Merged";

‎apps/web/src/lib/openPullRequestLink.test.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,32 @@
11
import { describe, expect, it, vi } from "vite-plus/test";
22

33
import {
4+
changeRequestRepositoryUrl,
45
findProjectForChangeRequest,
56
openPullRequestLink,
67
parseChangeRequestUrl,
78
PullRequestLinkOpenError,
89
shouldOpenPullRequestExternally,
910
} from "./openPullRequestLink";
1011

12+
describe("changeRequestRepositoryUrl", () => {
13+
it("preserves repository path casing", () => {
14+
expect(
15+
changeRequestRepositoryUrl(
16+
"https://gitlab.example.test/Team/Platform/Repo/-/merge_requests/42/diffs#note_1",
17+
),
18+
).toBe("https://gitlab.example.test/Team/Platform/Repo");
19+
});
20+
21+
it("keeps pull-like segments inside nested GitLab repository paths", () => {
22+
expect(
23+
changeRequestRepositoryUrl(
24+
"https://gitlab.example.test/group/pull/123/repo/-/merge_requests/42",
25+
),
26+
).toBe("https://gitlab.example.test/group/pull/123/repo");
27+
});
28+
});
29+
1130
describe("openPullRequestLink", () => {
1231
it("opens the requested pull request URL", async () => {
1332
const openExternal = vi.fn(async () => undefined);

‎apps/web/src/lib/openPullRequestLink.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,23 @@ export function parseChangeRequestUrl(targetUrl: string): ChangeRequestLink | nu
118118
return null;
119119
}
120120

121+
/** The repository root behind a recognised change-request URL, without PR-specific state. */
122+
export function changeRequestRepositoryUrl(targetUrl: string): string | null {
123+
const changeRequest = parseChangeRequestUrl(targetUrl);
124+
if (changeRequest === null) return null;
125+
const url = new URL(targetUrl);
126+
const repositoryPath =
127+
/^(.*?)\/-\/merge_requests\/\d+(?:\/|$)/iu.exec(url.pathname)?.[1] ??
128+
/^(.*?)(?:\/pull\/\d+|\/-\/merge_requests\/\d+|\/pull-requests\/\d+|\/pullrequest\/\d+)(?:\/|$)/iu.exec(
129+
url.pathname,
130+
)?.[1];
131+
if (!repositoryPath) return null;
132+
url.pathname = repositoryPath;
133+
url.search = "";
134+
url.hash = "";
135+
return url.toString();
136+
}
137+
121138
function claim(host: string, match: RegExpExecArray | null): ChangeRequestLink | null {
122139
const repository = match?.[1];
123140
const number = Number(match?.[2]);

0 commit comments

Comments
 (0)