authorgravatar for maria@kuuro.netmaria <maria@kuuro.net> 2026-10-10 02:22:04-03:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-10-10 02:22:04-03:00
log1f093d119b91263deb83f0d18e1504b791fdbbd4
treec73d06a6be3b19a5f60c48f0607697b5bf08f306
parentc2a301b12af6e18148eb1d46123c4187f4bdb78e
signature Signed by PGP key B5690EEEBB952194

feat(web): pr panel actions confirm in place (#17710)

2 files changed, 174 insertions(+), 132 deletions(-)

apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx+119-132
...@@ -88,15 +88,6 @@ import { vcsEnvironment } from "~/state/vcs";...@@ -88,15 +88,6 @@ import { vcsEnvironment } from "~/state/vcs";
88import { formatRelativeTimeLabel } from "~/timestampFormat";88import { formatRelativeTimeLabel } from "~/timestampFormat";
89import { useUiStateStore } from "~/uiStateStore";89import { useUiStateStore } from "~/uiStateStore";
9090
91import {
92 AlertDialog,
93 AlertDialogClose,
94 AlertDialogDescription,
95 AlertDialogFooter,
96 AlertDialogHeader,
97 AlertDialogPopup,
98 AlertDialogTitle,
99} from "../ui/alert-dialog";
100import { EnvironmentMachineIcon } from "../EnvironmentMachineIcon";91import { EnvironmentMachineIcon } from "../EnvironmentMachineIcon";
101import { Badge } from "../ui/badge";92import { Badge } from "../ui/badge";
102import { Button } from "../ui/button";93import { Button } from "../ui/button";
...@@ -183,25 +174,10 @@ import {...@@ -183,25 +174,10 @@ import {
183 summarizePullRequestChecks,174 summarizePullRequestChecks,
184} from "./pullRequestPresentation";175} from "./pullRequestPresentation";
185import { PullRequestGlyph } from "./pullRequestIcons";176import { PullRequestGlyph } from "./pullRequestIcons";
177import { useInlineActionConfirm } from "./useInlineActionConfirm";
186178
187type DetailTab = "summary" | "timeline" | "code";179type DetailTab = "summary" | "timeline" | "code";
188180
189const ACTION_SUCCESS_LABELS: Record<PullRequestAction, string> = {
190 merge: "Pull request merged",
191 ready: "Marked ready for review",
192 draft: "Converted to draft",
193 close: "Pull request closed",
194 reopen: "Pull request reopened",
195 "update-branch": "Branch updated with the base branch",
196 // True whichever it did: a pull request that was already mergeable merges the moment this is
197 // armed, and the client has no way to tell that apart from one still waiting on something.
198 "enable-auto-merge":
199 "Auto-merge turned on — merges as soon as this is ready, sooner if it already is",
200 "disable-auto-merge": "Auto-merge turned off",
201 revert: "Revert pull request opened",
202 "approve-workflows": "Workflows approved",
203};
204
205/** Said as the thing that did not happen, rather than as the operation that returned an error. */181/** Said as the thing that did not happen, rather than as the operation that returned an error. */
206const ACTION_FAILURE_LABELS: Record<PullRequestAction, string> = {182const ACTION_FAILURE_LABELS: Record<PullRequestAction, string> = {
207 merge: "Could not merge this pull request",183 merge: "Could not merge this pull request",
...@@ -578,11 +554,7 @@ export function PullRequestDetailPanel({...@@ -578,11 +554,7 @@ export function PullRequestDetailPanel({
578 const setMergeMethod = (method: PullRequestMergeMethod) => {554 const setMergeMethod = (method: PullRequestMergeMethod) => {
579 setMergeMethodSelection({ pullRequestKey, method });555 setMergeMethodSelection({ pullRequestKey, method });
580 };556 };
581 const [confirmation, setConfirmation] = useState<{557 const inlineConfirm = useInlineActionConfirm(pullRequestKey);
582 readonly open: boolean;
583 readonly action: "merge" | "close" | "enable-auto-merge" | "revert" | "approve-workflows";
584 }>({ open: false, action: "merge" });
585 const confirmAction = confirmation.action;
586 // Which handoff is preparing, keyed so a per-finding button can say "Preparing..." on itself558 // Which handoff is preparing, keyed so a per-finding button can say "Preparing..." on itself
587 // alone. One at a time whatever the key: they all check the same pull request out.559 // alone. One at a time whatever the key: they all check the same pull request out.
588 const [handoff, setHandoff] = useState<string | null>(null);560 const [handoff, setHandoff] = useState<string | null>(null);
...@@ -986,7 +958,10 @@ export function PullRequestDetailPanel({...@@ -986,7 +958,10 @@ export function PullRequestDetailPanel({
986 onActed?.(action, "failed");958 onActed?.(action, "failed");
987 return false;959 return false;
988 }960 }
989 toastManager.add({ type: "success", title: ACTION_SUCCESS_LABELS[action] });961 // Reverting opens another PR; the current panel has no new state to show.
962 if (action === "revert") {
963 toastManager.add({ type: "success", title: "Revert pull request opened" });
964 }
990 // A branch update moves the head commit, which leaves the diff atom pointed at a comparison965 // A branch update moves the head commit, which leaves the diff atom pointed at a comparison
991 // that no longer exists — the same staleness the manual refresh button fixes, so it goes966 // that no longer exists — the same staleness the manual refresh button fixes, so it goes
992 // through that path rather than a second one. Every other action here only changes metadata;967 // through that path rather than a second one. Every other action here only changes metadata;
...@@ -1918,27 +1893,44 @@ export function PullRequestDetailPanel({...@@ -1918,27 +1893,44 @@ export function PullRequestDetailPanel({
1918 size="xs"1893 size="xs"
1919 variant="default"1894 variant="default"
1920 disabled={actionPending}1895 disabled={actionPending}
1921 onClick={() =>1896 {...inlineConfirm.props(
1922 setConfirmation({ open: true, action: "enable-auto-merge" })1897 "enable-auto-merge",
1923 }1898 () => void perform("enable-auto-merge", selectedMergeMethod),
1899 )}
1924 aria-label={1900 aria-label={
1925 pendingAction === "enable-auto-merge"1901 pendingAction === "enable-auto-merge"
1926 ? "Enabling..."1902 ? "Enabling..."
1927 : pendingAutoMergeLabel1903 : inlineConfirm.isArmed("enable-auto-merge")
1904 ? "Confirm auto-merge?"
1905 : pendingAutoMergeLabel
1928 }1906 }
1929 >1907 >
1930 <PullRequestGlyph.merged aria-hidden className="size-3.5" />1908 <PullRequestGlyph.merged aria-hidden className="size-3.5" />
1931 <span className="@max-[30rem]/pr-header:hidden">1909 <span className="inline-grid @max-[30rem]/pr-header:hidden">
1932 {pendingAction === "enable-auto-merge"1910 <span aria-hidden className="invisible col-start-1 row-start-1">
1933 ? "Enabling..."1911 {pendingAutoMergeLabel}
1934 : pendingAutoMergeLabel}1912 </span>
1913 <span aria-hidden className="invisible col-start-1 row-start-1">
1914 Confirm auto-merge?
1915 </span>
1916 <span className="col-start-1 row-start-1">
1917 {pendingAction === "enable-auto-merge"
1918 ? "Enabling..."
1919 : inlineConfirm.isArmed("enable-auto-merge")
1920 ? "Confirm auto-merge?"
1921 : pendingAutoMergeLabel}
1922 </span>
1935 </span>1923 </span>
1936 </Button>1924 </Button>
1937 </span>1925 </span>
1938 }1926 }
1939 />1927 />
1940 <TooltipPopup side="top">1928 <TooltipPopup side="top">
1941 {pendingAction === "enable-auto-merge" ? "Enabling..." : pendingAutoMergeLabel}1929 {pendingAction === "enable-auto-merge"
1930 ? "Enabling..."
1931 : inlineConfirm.isArmed("enable-auto-merge")
1932 ? "Confirm auto-merge?"
1933 : pendingAutoMergeLabel}
1942 </TooltipPopup>1934 </TooltipPopup>
1943 </Tooltip>1935 </Tooltip>
1944 ) : primaryAction === "auto-merge-armed" ? (1936 ) : primaryAction === "auto-merge-armed" ? (
...@@ -1970,21 +1962,44 @@ export function PullRequestDetailPanel({...@@ -1970,21 +1962,44 @@ export function PullRequestDetailPanel({
1970 size="xs"1962 size="xs"
1971 variant="default"1963 variant="default"
1972 disabled={actionPending}1964 disabled={actionPending}
1973 onClick={() => setConfirmation({ open: true, action: "merge" })}1965 {...inlineConfirm.props(
1966 "merge",
1967 () => void perform("merge", selectedMergeMethod),
1968 )}
1974 aria-label={1969 aria-label={
1975 pendingAction === "merge" ? "Merging..." : selectedMergeMethodLabel1970 pendingAction === "merge"
1971 ? "Merging..."
1972 : inlineConfirm.isArmed("merge")
1973 ? "Confirm merge?"
1974 : selectedMergeMethodLabel
1976 }1975 }
1977 >1976 >
1978 <PullRequestGlyph.merged aria-hidden className="size-3.5" />1977 <PullRequestGlyph.merged aria-hidden className="size-3.5" />
1979 <span className="@max-[30rem]/pr-header:hidden">1978 <span className="inline-grid @max-[30rem]/pr-header:hidden">
1980 {pendingAction === "merge" ? "Merging..." : selectedMergeMethodLabel}1979 <span aria-hidden className="invisible col-start-1 row-start-1">
1980 {selectedMergeMethodLabel}
1981 </span>
1982 <span aria-hidden className="invisible col-start-1 row-start-1">
1983 Confirm merge?
1984 </span>
1985 <span className="col-start-1 row-start-1">
1986 {pendingAction === "merge"
1987 ? "Merging..."
1988 : inlineConfirm.isArmed("merge")
1989 ? "Confirm merge?"
1990 : selectedMergeMethodLabel}
1991 </span>
1981 </span>1992 </span>
1982 </Button>1993 </Button>
1983 </span>1994 </span>
1984 }1995 }
1985 />1996 />
1986 <TooltipPopup side="top">1997 <TooltipPopup side="top">
1987 {pendingAction === "merge" ? "Merging..." : selectedMergeMethodLabel}1998 {pendingAction === "merge"
1999 ? "Merging..."
2000 : inlineConfirm.isArmed("merge")
2001 ? "Confirm merge?"
2002 : selectedMergeMethodLabel}
1988 </TooltipPopup>2003 </TooltipPopup>
1989 </Tooltip>2004 </Tooltip>
1990 ) : (primaryAction === "merged" || primaryAction === "closed") &&2005 ) : (primaryAction === "merged" || primaryAction === "closed") &&
...@@ -2101,11 +2116,18 @@ export function PullRequestDetailPanel({...@@ -2101,11 +2116,18 @@ export function PullRequestDetailPanel({
2101 ) : null}2116 ) : null}
2102 {showsMergeNow ? (2117 {showsMergeNow ? (
2103 <MenuItem2118 <MenuItem
2119 closeOnClick={inlineConfirm.isArmed("merge")}
2120 aria-label={
2121 inlineConfirm.isArmed("merge") ? "Confirm merge?" : "Merge now"
2122 }
2104 disabled={actionPending}2123 disabled={actionPending}
2105 onClick={() => setConfirmation({ open: true, action: "merge" })}2124 {...inlineConfirm.props(
2125 "merge",
2126 () => void perform("merge", selectedMergeMethod),
2127 )}
2106 >2128 >
2107 <PullRequestGlyph.merged className="size-3.5" />2129 <PullRequestGlyph.merged className="size-3.5" />
2108 Merge now2130 {inlineConfirm.isArmed("merge") ? "Confirm merge?" : "Merge now"}
2109 </MenuItem>2131 </MenuItem>
2110 ) : null}2132 ) : null}
2111 {/* The same merge, left with the host to carry out once its requirements2133 {/* The same merge, left with the host to carry out once its requirements
...@@ -2121,13 +2143,22 @@ export function PullRequestDetailPanel({...@@ -2121,13 +2143,22 @@ export function PullRequestDetailPanel({
2121 </MenuItem>2143 </MenuItem>
2122 ) : showsAutoMerge ? (2144 ) : showsAutoMerge ? (
2123 <MenuItem2145 <MenuItem
2124 disabled={actionPending}2146 closeOnClick={inlineConfirm.isArmed("enable-auto-merge")}
2125 onClick={() =>2147 aria-label={
2126 setConfirmation({ open: true, action: "enable-auto-merge" })2148 inlineConfirm.isArmed("enable-auto-merge")
2149 ? "Confirm auto-merge?"
2150 : "Enable auto-merge"
2127 }2151 }
2152 disabled={actionPending}
2153 {...inlineConfirm.props(
2154 "enable-auto-merge",
2155 () => void perform("enable-auto-merge", selectedMergeMethod),
2156 )}
2128 >2157 >
2129 <PullRequestGlyph.merged className="size-3.5" />2158 <PullRequestGlyph.merged className="size-3.5" />
2130 Enable auto-merge2159 {inlineConfirm.isArmed("enable-auto-merge")
2160 ? "Confirm auto-merge?"
2161 : "Enable auto-merge"}
2131 </MenuItem>2162 </MenuItem>
2132 ) : null}2163 ) : null}
2133 {/* A preference for the merge action rather than a second action, so it2164 {/* A preference for the merge action rather than a second action, so it
...@@ -2200,12 +2231,16 @@ export function PullRequestDetailPanel({...@@ -2200,12 +2231,16 @@ export function PullRequestDetailPanel({
2200 <>2231 <>
2201 <MenuSeparator />2232 <MenuSeparator />
2202 <MenuItem2233 <MenuItem
2234 closeOnClick={inlineConfirm.isArmed("close")}
2235 aria-label={
2236 inlineConfirm.isArmed("close") ? "Confirm close?" : "Close pull request"
2237 }
2203 variant="destructive"2238 variant="destructive"
2204 disabled={actionPending}2239 disabled={actionPending}
2205 onClick={() => setConfirmation({ open: true, action: "close" })}2240 {...inlineConfirm.props("close", () => void perform("close"))}
2206 >2241 >
2207 <PullRequestGlyph.closed className="size-3.5" />2242 <PullRequestGlyph.closed className="size-3.5" />
2208 Close pull request2243 {inlineConfirm.isArmed("close") ? "Confirm close?" : "Close pull request"}
2209 </MenuItem>2244 </MenuItem>
2210 </>2245 </>
2211 ) : detail.state === "closed" && can("reopen") ? (2246 ) : detail.state === "closed" && can("reopen") ? (
...@@ -2220,11 +2255,16 @@ export function PullRequestDetailPanel({...@@ -2220,11 +2255,16 @@ export function PullRequestDetailPanel({
2220 <>2255 <>
2221 <MenuSeparator />2256 <MenuSeparator />
2222 <MenuItem2257 <MenuItem
2258 closeOnClick={inlineConfirm.isArmed("revert")}
2259 aria-label={
2260 inlineConfirm.isArmed("revert") ? "Confirm revert?" : "Revert changes"
2261 }
2262 variant={inlineConfirm.isArmed("revert") ? "destructive" : "default"}
2223 disabled={actionPending}2263 disabled={actionPending}
2224 onClick={() => setConfirmation({ open: true, action: "revert" })}2264 {...inlineConfirm.props("revert", () => void perform("revert"))}
2225 >2265 >
2226 <RotateCcwIcon className="size-3.5" />2266 <RotateCcwIcon className="size-3.5" />
2227 Revert changes2267 {inlineConfirm.isArmed("revert") ? "Confirm revert?" : "Revert changes"}
2228 </MenuItem>2268 </MenuItem>
2229 </>2269 </>
2230 ) : null}2270 ) : null}
...@@ -2553,20 +2593,33 @@ export function PullRequestDetailPanel({...@@ -2553,20 +2593,33 @@ export function PullRequestDetailPanel({
2553 size="xs"2593 size="xs"
2554 variant="warning-outline"2594 variant="warning-outline"
2555 disabled={actionPending}2595 disabled={actionPending}
2556 onClick={() =>2596 {...inlineConfirm.props(
2557 setConfirmation({ open: true, action: "approve-workflows" })2597 "approve-workflows",
2558 }2598 () => void perform("approve-workflows"),
2599 )}
2559 aria-label={2600 aria-label={
2560 pendingAction === "approve-workflows"2601 pendingAction === "approve-workflows"
2561 ? "Approving..."2602 ? "Approving..."
2562 : "Approve workflows to run"2603 : inlineConfirm.isArmed("approve-workflows")
2604 ? "Confirm approve?"
2605 : "Approve workflows to run"
2563 }2606 }
2564 >2607 >
2565 <PlayIcon aria-hidden className="size-3.5" />2608 <PlayIcon aria-hidden className="size-3.5" />
2566 <span>2609 <span className="inline-grid">
2567 {pendingAction === "approve-workflows"2610 <span aria-hidden className="invisible col-start-1 row-start-1">
2568 ? "Approving..."2611 Approve workflows to run
2569 : "Approve workflows to run"}2612 </span>
2613 <span aria-hidden className="invisible col-start-1 row-start-1">
2614 Confirm approve?
2615 </span>
2616 <span className="col-start-1 row-start-1">
2617 {pendingAction === "approve-workflows"
2618 ? "Approving..."
2619 : inlineConfirm.isArmed("approve-workflows")
2620 ? "Confirm approve?"
2621 : "Approve workflows to run"}
2622 </span>
2570 </span>2623 </span>
2571 </Button>2624 </Button>
2572 </span>2625 </span>
...@@ -2575,7 +2628,9 @@ export function PullRequestDetailPanel({...@@ -2575,7 +2628,9 @@ export function PullRequestDetailPanel({
2575 <TooltipPopup side="top">2628 <TooltipPopup side="top">
2576 {pendingAction === "approve-workflows"2629 {pendingAction === "approve-workflows"
2577 ? "Approving..."2630 ? "Approving..."
2578 : "Approve workflows to run"}2631 : inlineConfirm.isArmed("approve-workflows")
2632 ? "Confirm approve?"
2633 : "Approve workflows to run"}
2579 </TooltipPopup>2634 </TooltipPopup>
2580 </Tooltip>2635 </Tooltip>
2581 ) : (2636 ) : (
...@@ -2802,74 +2857,6 @@ export function PullRequestDetailPanel({...@@ -2802,74 +2857,6 @@ export function PullRequestDetailPanel({
2802 />2857 />
2803 </div>2858 </div>
2804 ) : null}2859 ) : null}
2805
2806 <AlertDialog
2807 open={confirmation.open}
2808 onOpenChange={(open) => setConfirmation((current) => ({ ...current, open }))}
2809 onOpenChangeComplete={(open) => {
2810 if (!open) setConfirmation({ open: false, action: "merge" });
2811 }}
2812 >
2813 <AlertDialogPopup>
2814 <AlertDialogHeader>
2815 <AlertDialogTitle>
2816 {confirmAction === "merge"
2817 ? "Merge pull request?"
2818 : confirmAction === "enable-auto-merge"
2819 ? "Enable auto-merge?"
2820 : confirmAction === "revert"
2821 ? "Revert these changes?"
2822 : confirmAction === "approve-workflows"
2823 ? "Approve workflows to run?"
2824 : "Close pull request?"}
2825 </AlertDialogTitle>
2826 <AlertDialogDescription>
2827 {confirmAction === "merge"
2828 ? `This merges #${reference.number} using ${selectedMergeMethod}.`
2829 : confirmAction === "enable-auto-merge"
2830 ? // The host merges this as soon as it considers the pull request ready, which
2831 // may be immediately — there is no telling from here whether anything is
2832 // still outstanding.
2833 `This merges #${reference.number} using ${selectedMergeMethod} as soon as the host considers it ready, which may be immediately.`
2834 : confirmAction === "revert"
2835 ? `This opens a new pull request that reverses the changes merged by #${reference.number}.`
2836 : confirmAction === "approve-workflows"
2837 ? `This allows ${workflowApprovalsRequired} ${workflowApprovalsRequired === 1 ? "workflow" : "workflows"} from #${reference.number} to run. Review the code and workflow changes first.`
2838 : `This closes #${reference.number} without merging it.`}
2839 </AlertDialogDescription>
2840 </AlertDialogHeader>
2841 <AlertDialogFooter>
2842 <AlertDialogClose render={<Button variant="outline" size="sm" />}>
2843 Cancel
2844 </AlertDialogClose>
2845 <Button
2846 size="sm"
2847 variant={confirmAction === "close" ? "destructive" : "default"}
2848 disabled={actionPending}
2849 onClick={() => {
2850 const action = confirmAction;
2851 setConfirmation((current) => ({ ...current, open: false }));
2852 if (action === "merge") void perform("merge", selectedMergeMethod);
2853 if (action === "enable-auto-merge")
2854 void perform("enable-auto-merge", selectedMergeMethod);
2855 if (action === "revert") void perform("revert");
2856 if (action === "approve-workflows") void perform("approve-workflows");
2857 if (action === "close") void perform("close");
2858 }}
2859 >
2860 {confirmAction === "merge"
2861 ? selectedMergeMethodLabel
2862 : confirmAction === "enable-auto-merge"
2863 ? "Enable auto-merge"
2864 : confirmAction === "revert"
2865 ? "Create revert PR"
2866 : confirmAction === "approve-workflows"
2867 ? "Approve and run"
2868 : "Close"}
2869 </Button>
2870 </AlertDialogFooter>
2871 </AlertDialogPopup>
2872 </AlertDialog>
2873 </div>2860 </div>
2874 );2861 );
2875}2862}
apps/web/src/components/pullRequest/useInlineActionConfirm.ts created+55
...@@ -0,0 +1,55 @@
1import { useEffect, useState, type MouseEvent } from "react";
2
3/** Arms one control at a time, including controls rendered in a menu portal. */
4export function useInlineActionConfirm(scopeKey: string) {
5 const [armed, setArmed] = useState<{
6 scopeKey: string;
7 action: string;
8 element: HTMLElement;
9 } | null>(null);
10 const active = armed?.scopeKey === scopeKey ? armed : null;
11
12 useEffect(() => {
13 if (!armed) return;
14 const disarm = () => setArmed(null);
15 if (armed.scopeKey !== scopeKey) {
16 disarm();
17 return;
18 }
19 const onPointerDown = (event: PointerEvent) => {
20 if (event.target instanceof Node && !armed.element.contains(event.target)) disarm();
21 };
22 const onKeyDown = (event: KeyboardEvent) => {
23 if (event.key === "Escape") {
24 event.preventDefault();
25 event.stopPropagation();
26 disarm();
27 }
28 };
29 const timeout = window.setTimeout(disarm, 3000);
30 document.addEventListener("pointerdown", onPointerDown, true);
31 window.addEventListener("keydown", onKeyDown, true);
32 window.addEventListener("blur", disarm);
33 return () => {
34 window.clearTimeout(timeout);
35 document.removeEventListener("pointerdown", onPointerDown, true);
36 window.removeEventListener("keydown", onKeyDown, true);
37 window.removeEventListener("blur", disarm);
38 };
39 }, [armed, scopeKey]);
40
41 return {
42 isArmed: (action: string) => active?.action === action,
43 props: (action: string, onConfirm: () => void) => ({
44 onClick: (event: MouseEvent<HTMLElement>) => {
45 if (active?.action === action && active.element === event.currentTarget) {
46 setArmed(null);
47 onConfirm();
48 } else {
49 setArmed({ scopeKey, action, element: event.currentTarget });
50 }
51 },
52 onBlur: () => setArmed(null),
53 }),
54 };
55}