Skip to content

Commit ebba0d0

Browse files
juliusmarmingejuliusmarminge
committed
fix(prs): tuck thread linking into the more menu
1 parent 39f747e commit ebba0d0

2 files changed

Lines changed: 62 additions & 41 deletions

File tree

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

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -523,6 +523,7 @@ export function PullRequestDetailPanel({
523523
listEntry.number === reference.number
524524
? listEntry
525525
: null;
526+
const [threadPickerOpen, setThreadPickerOpen] = useState(false);
526527
const [tab, setTab] = useState<DetailTab>("summary");
527528
const [timelineOrder, setTimelineOrder] = useState<"newest" | "oldest">("newest");
528529
const [codeCommitScope, setCodeCommitScope] = useState<{
@@ -1411,6 +1412,17 @@ export function PullRequestDetailPanel({
14111412

14121413
return (
14131414
<div className="flex h-full min-h-0 w-full flex-col bg-background">
1415+
{threadPickerOpen && detail ? (
1416+
<PullRequestThreadLinks
1417+
key={`${environmentId}:${detail.url}`}
1418+
display="picker"
1419+
environmentId={environmentId}
1420+
reference={reference}
1421+
url={detail.url}
1422+
threadRef={null}
1423+
onPickerOpenChange={setThreadPickerOpen}
1424+
/>
1425+
) : null}
14141426
<div className="@container/pr-header grid min-w-0 shrink-0 grid-cols-[minmax(0,1fr)_auto] items-start gap-x-2 border-b border-border/60">
14151427
<div className="ml-4 grid h-7 min-w-0 items-center overflow-hidden">
14161428
<div
@@ -1557,15 +1569,15 @@ export function PullRequestDetailPanel({
15571569
<div className="mr-4 flex h-7 shrink-0 items-center justify-end gap-1">
15581570
{detail ? (
15591571
<>
1560-
<PullRequestThreadLinks
1561-
environmentId={environmentId}
1562-
reference={reference}
1563-
url={detail.url}
1564-
threadRef={
1565-
threadRef ??
1566-
(typeof composerDraftTarget === "object" ? composerDraftTarget : null)
1567-
}
1568-
/>
1572+
{context === "page" ? (
1573+
<PullRequestThreadLinks
1574+
display="count"
1575+
environmentId={environmentId}
1576+
reference={reference}
1577+
url={detail.url}
1578+
threadRef={null}
1579+
/>
1580+
) : null}
15691581
{/* Checking a pull request out is the reason to open one here at all, so it is a
15701582
button of its own rather than a side effect of asking an agent for something.
15711583
It asks where, because the two answers are not interchangeable: one leaves your
@@ -1792,6 +1804,17 @@ export function PullRequestDetailPanel({
17921804
<MoreHorizontalIcon className="size-4" />
17931805
</MenuTrigger>
17941806
<MenuPopup align="end" side="bottom" className="min-w-72">
1807+
<PullRequestThreadLinks
1808+
display="menu-item"
1809+
environmentId={environmentId}
1810+
reference={reference}
1811+
url={detail.url}
1812+
threadRef={
1813+
threadRef ??
1814+
(typeof composerDraftTarget === "object" ? composerDraftTarget : null)
1815+
}
1816+
onPickerOpenChange={setThreadPickerOpen}
1817+
/>
17951818
<MenuItem
17961819
disabled={isInvalidating || detailQuery.isPending}
17971820
onClick={() => void refreshFromHost()}

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

Lines changed: 30 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ interface PullRequestThreadLinksProps {
2222
reference: PullRequestRef;
2323
url: string;
2424
threadRef: ScopedThreadRef | null;
25+
display: "count" | "menu-item" | "picker";
26+
onPickerOpenChange?: (open: boolean) => void;
2527
}
2628

2729
/** Thread relations belong to the detail environment, including when another environment is active. */
@@ -41,14 +43,16 @@ function EnabledPullRequestThreadLinks({
4143
reference,
4244
url,
4345
threadRef,
46+
display,
47+
onPickerOpenChange,
4448
}: PullRequestThreadLinksProps) {
4549
const parsed = parseChangeRequestUrl(url);
4650
const currentThreadRef = threadRef?.environmentId === environmentId ? threadRef : null;
4751
const thread = useThreadShell(currentThreadRef);
4852
const linking = usePullRequestLinking(environmentId);
4953
const linkedHere = linking.isLinked(thread, url);
5054
const relations = useEnvironmentQuery(
51-
linking.mode === "multiple"
55+
linking.mode === "multiple" && display !== "menu-item"
5256
? pullRequestEnvironment.linkedThreads({
5357
environmentId,
5458
input: parsed === null ? reference : { ...reference, ...parsed },
@@ -61,7 +65,6 @@ function EnabledPullRequestThreadLinks({
6165
if (relations.data !== null && relations.data !== lastRelations) {
6266
setLastRelations(relations.data);
6367
}
64-
const [pickerOpen, setPickerOpen] = useState(false);
6568
const [pending, setPending] = useState(false);
6669

6770
const changeLink = async (threadId: ThreadId, remove: boolean) => {
@@ -80,7 +83,7 @@ function EnabledPullRequestThreadLinks({
8083
setPending(false);
8184
}
8285
relations.refresh();
83-
setPickerOpen(false);
86+
onPickerOpenChange?.(false);
8487
};
8588

8689
if (parsed === null || (!linkedHere && !linking.canLink(url))) return null;
@@ -92,7 +95,7 @@ function EnabledPullRequestThreadLinks({
9295
: "Linked threads";
9396
return (
9497
<>
95-
{linkedThreads.length > 0 || relations.error !== null ? (
98+
{display === "count" && (linkedThreads.length > 0 || relations.error !== null) ? (
9699
<Menu>
97100
<MenuTrigger
98101
render={
@@ -135,47 +138,42 @@ function EnabledPullRequestThreadLinks({
135138
</MenuPopup>
136139
</Menu>
137140
) : null}
138-
{currentThreadRef !== null ? (
139-
<Button
140-
size="xs"
141-
variant="outline"
141+
{display === "menu-item" ? (
142+
<MenuItem
142143
disabled={pending}
143-
onClick={() => void changeLink(currentThreadRef.threadId, linkedHere)}
144-
aria-label={linkedHere ? "Unlink from this thread" : "Link to this thread"}
144+
onClick={() => {
145+
if (currentThreadRef !== null) {
146+
void changeLink(currentThreadRef.threadId, linkedHere);
147+
} else {
148+
onPickerOpenChange?.(true);
149+
}
150+
}}
145151
>
146152
{linkedHere ? (
147153
<UnlinkIcon aria-hidden className="size-3.5" />
148154
) : (
149155
<LinkIcon aria-hidden className="size-3.5" />
150156
)}
151-
<span className="@max-[40rem]/pr-header:hidden">
152-
{linkedHere ? "Unlink from thread" : "Link to thread"}
153-
</span>
154-
</Button>
155-
) : (
156-
<Button
157-
size="xs"
158-
variant="outline"
159-
onClick={() => setPickerOpen(true)}
160-
aria-label="Link to thread"
161-
>
162-
<LinkIcon aria-hidden className="size-3.5" />
163-
<span className="@max-[40rem]/pr-header:hidden">Link to thread</span>
164-
</Button>
165-
)}
166-
<Dialog open={pickerOpen} onOpenChange={setPickerOpen}>
167-
<DialogPopup className="max-w-md" showCloseButton={false}>
168-
<DialogTitle className="sr-only">Link pull request to a thread</DialogTitle>
169-
{pickerOpen ? (
157+
{linkedHere
158+
? "Unlink from this thread"
159+
: currentThreadRef
160+
? "Link to this thread"
161+
: "Link to thread"}
162+
</MenuItem>
163+
) : null}
164+
{display === "picker" ? (
165+
<Dialog open onOpenChange={onPickerOpenChange}>
166+
<DialogPopup className="max-w-md" showCloseButton={false}>
167+
<DialogTitle className="sr-only">Link pull request to a thread</DialogTitle>
170168
<ThreadPicker
171169
environmentId={environmentId}
172170
url={url}
173171
pending={pending}
174172
onSelect={(threadId) => void changeLink(threadId, false)}
175173
/>
176-
) : null}
177-
</DialogPopup>
178-
</Dialog>
174+
</DialogPopup>
175+
</Dialog>
176+
) : null}
179177
</>
180178
);
181179
}

0 commit comments

Comments
 (0)