diff --git a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx index 540e14f8a05f..137be41c9606 100644 --- a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx +++ b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx @@ -88,15 +88,6 @@ import { vcsEnvironment } from "~/state/vcs"; import { formatRelativeTimeLabel } from "~/timestampFormat"; import { useUiStateStore } from "~/uiStateStore"; -import { - AlertDialog, - AlertDialogClose, - AlertDialogDescription, - AlertDialogFooter, - AlertDialogHeader, - AlertDialogPopup, - AlertDialogTitle, -} from "../ui/alert-dialog"; import { EnvironmentMachineIcon } from "../EnvironmentMachineIcon"; import { Badge } from "../ui/badge"; import { Button } from "../ui/button"; @@ -183,25 +174,10 @@ import { summarizePullRequestChecks, } from "./pullRequestPresentation"; import { PullRequestGlyph } from "./pullRequestIcons"; +import { useInlineActionConfirm } from "./useInlineActionConfirm"; type DetailTab = "summary" | "timeline" | "code"; -const ACTION_SUCCESS_LABELS: Record = { - merge: "Pull request merged", - ready: "Marked ready for review", - draft: "Converted to draft", - close: "Pull request closed", - reopen: "Pull request reopened", - "update-branch": "Branch updated with the base branch", - // True whichever it did: a pull request that was already mergeable merges the moment this is - // armed, and the client has no way to tell that apart from one still waiting on something. - "enable-auto-merge": - "Auto-merge turned on — merges as soon as this is ready, sooner if it already is", - "disable-auto-merge": "Auto-merge turned off", - revert: "Revert pull request opened", - "approve-workflows": "Workflows approved", -}; - /** Said as the thing that did not happen, rather than as the operation that returned an error. */ const ACTION_FAILURE_LABELS: Record = { merge: "Could not merge this pull request", @@ -578,11 +554,7 @@ export function PullRequestDetailPanel({ const setMergeMethod = (method: PullRequestMergeMethod) => { setMergeMethodSelection({ pullRequestKey, method }); }; - const [confirmation, setConfirmation] = useState<{ - readonly open: boolean; - readonly action: "merge" | "close" | "enable-auto-merge" | "revert" | "approve-workflows"; - }>({ open: false, action: "merge" }); - const confirmAction = confirmation.action; + const inlineConfirm = useInlineActionConfirm(pullRequestKey); // Which handoff is preparing, keyed so a per-finding button can say "Preparing..." on itself // alone. One at a time whatever the key: they all check the same pull request out. const [handoff, setHandoff] = useState(null); @@ -986,7 +958,10 @@ export function PullRequestDetailPanel({ onActed?.(action, "failed"); return false; } - toastManager.add({ type: "success", title: ACTION_SUCCESS_LABELS[action] }); + // Reverting opens another PR; the current panel has no new state to show. + if (action === "revert") { + toastManager.add({ type: "success", title: "Revert pull request opened" }); + } // A branch update moves the head commit, which leaves the diff atom pointed at a comparison // that no longer exists — the same staleness the manual refresh button fixes, so it goes // through that path rather than a second one. Every other action here only changes metadata; @@ -1918,27 +1893,44 @@ export function PullRequestDetailPanel({ size="xs" variant="default" disabled={actionPending} - onClick={() => - setConfirmation({ open: true, action: "enable-auto-merge" }) - } + {...inlineConfirm.props( + "enable-auto-merge", + () => void perform("enable-auto-merge", selectedMergeMethod), + )} aria-label={ pendingAction === "enable-auto-merge" ? "Enabling..." - : pendingAutoMergeLabel + : inlineConfirm.isArmed("enable-auto-merge") + ? "Confirm auto-merge?" + : pendingAutoMergeLabel } > - - {pendingAction === "enable-auto-merge" - ? "Enabling..." - : pendingAutoMergeLabel} + + + {pendingAutoMergeLabel} + + + Confirm auto-merge? + + + {pendingAction === "enable-auto-merge" + ? "Enabling..." + : inlineConfirm.isArmed("enable-auto-merge") + ? "Confirm auto-merge?" + : pendingAutoMergeLabel} + } /> - {pendingAction === "enable-auto-merge" ? "Enabling..." : pendingAutoMergeLabel} + {pendingAction === "enable-auto-merge" + ? "Enabling..." + : inlineConfirm.isArmed("enable-auto-merge") + ? "Confirm auto-merge?" + : pendingAutoMergeLabel} ) : primaryAction === "auto-merge-armed" ? ( @@ -1970,21 +1962,44 @@ export function PullRequestDetailPanel({ size="xs" variant="default" disabled={actionPending} - onClick={() => setConfirmation({ open: true, action: "merge" })} + {...inlineConfirm.props( + "merge", + () => void perform("merge", selectedMergeMethod), + )} aria-label={ - pendingAction === "merge" ? "Merging..." : selectedMergeMethodLabel + pendingAction === "merge" + ? "Merging..." + : inlineConfirm.isArmed("merge") + ? "Confirm merge?" + : selectedMergeMethodLabel } > - - {pendingAction === "merge" ? "Merging..." : selectedMergeMethodLabel} + + + {selectedMergeMethodLabel} + + + Confirm merge? + + + {pendingAction === "merge" + ? "Merging..." + : inlineConfirm.isArmed("merge") + ? "Confirm merge?" + : selectedMergeMethodLabel} + } /> - {pendingAction === "merge" ? "Merging..." : selectedMergeMethodLabel} + {pendingAction === "merge" + ? "Merging..." + : inlineConfirm.isArmed("merge") + ? "Confirm merge?" + : selectedMergeMethodLabel} ) : (primaryAction === "merged" || primaryAction === "closed") && @@ -2101,11 +2116,18 @@ export function PullRequestDetailPanel({ ) : null} {showsMergeNow ? ( setConfirmation({ open: true, action: "merge" })} + {...inlineConfirm.props( + "merge", + () => void perform("merge", selectedMergeMethod), + )} > - Merge now + {inlineConfirm.isArmed("merge") ? "Confirm merge?" : "Merge now"} ) : null} {/* The same merge, left with the host to carry out once its requirements @@ -2121,13 +2143,22 @@ export function PullRequestDetailPanel({ ) : showsAutoMerge ? ( - setConfirmation({ open: true, action: "enable-auto-merge" }) + closeOnClick={inlineConfirm.isArmed("enable-auto-merge")} + aria-label={ + inlineConfirm.isArmed("enable-auto-merge") + ? "Confirm auto-merge?" + : "Enable auto-merge" } + disabled={actionPending} + {...inlineConfirm.props( + "enable-auto-merge", + () => void perform("enable-auto-merge", selectedMergeMethod), + )} > - Enable auto-merge + {inlineConfirm.isArmed("enable-auto-merge") + ? "Confirm auto-merge?" + : "Enable auto-merge"} ) : null} {/* A preference for the merge action rather than a second action, so it @@ -2200,12 +2231,16 @@ export function PullRequestDetailPanel({ <> setConfirmation({ open: true, action: "close" })} + {...inlineConfirm.props("close", () => void perform("close"))} > - Close pull request + {inlineConfirm.isArmed("close") ? "Confirm close?" : "Close pull request"} ) : detail.state === "closed" && can("reopen") ? ( @@ -2220,11 +2255,16 @@ export function PullRequestDetailPanel({ <> setConfirmation({ open: true, action: "revert" })} + {...inlineConfirm.props("revert", () => void perform("revert"))} > - Revert changes + {inlineConfirm.isArmed("revert") ? "Confirm revert?" : "Revert changes"} ) : null} @@ -2553,20 +2593,33 @@ export function PullRequestDetailPanel({ size="xs" variant="warning-outline" disabled={actionPending} - onClick={() => - setConfirmation({ open: true, action: "approve-workflows" }) - } + {...inlineConfirm.props( + "approve-workflows", + () => void perform("approve-workflows"), + )} aria-label={ pendingAction === "approve-workflows" ? "Approving..." - : "Approve workflows to run" + : inlineConfirm.isArmed("approve-workflows") + ? "Confirm approve?" + : "Approve workflows to run" } > - - {pendingAction === "approve-workflows" - ? "Approving..." - : "Approve workflows to run"} + + + Approve workflows to run + + + Confirm approve? + + + {pendingAction === "approve-workflows" + ? "Approving..." + : inlineConfirm.isArmed("approve-workflows") + ? "Confirm approve?" + : "Approve workflows to run"} + @@ -2575,7 +2628,9 @@ export function PullRequestDetailPanel({ {pendingAction === "approve-workflows" ? "Approving..." - : "Approve workflows to run"} + : inlineConfirm.isArmed("approve-workflows") + ? "Confirm approve?" + : "Approve workflows to run"} ) : ( @@ -2802,74 +2857,6 @@ export function PullRequestDetailPanel({ /> ) : null} - - setConfirmation((current) => ({ ...current, open }))} - onOpenChangeComplete={(open) => { - if (!open) setConfirmation({ open: false, action: "merge" }); - }} - > - - - - {confirmAction === "merge" - ? "Merge pull request?" - : confirmAction === "enable-auto-merge" - ? "Enable auto-merge?" - : confirmAction === "revert" - ? "Revert these changes?" - : confirmAction === "approve-workflows" - ? "Approve workflows to run?" - : "Close pull request?"} - - - {confirmAction === "merge" - ? `This merges #${reference.number} using ${selectedMergeMethod}.` - : confirmAction === "enable-auto-merge" - ? // The host merges this as soon as it considers the pull request ready, which - // may be immediately — there is no telling from here whether anything is - // still outstanding. - `This merges #${reference.number} using ${selectedMergeMethod} as soon as the host considers it ready, which may be immediately.` - : confirmAction === "revert" - ? `This opens a new pull request that reverses the changes merged by #${reference.number}.` - : confirmAction === "approve-workflows" - ? `This allows ${workflowApprovalsRequired} ${workflowApprovalsRequired === 1 ? "workflow" : "workflows"} from #${reference.number} to run. Review the code and workflow changes first.` - : `This closes #${reference.number} without merging it.`} - - - - }> - Cancel - - - - - ); } diff --git a/apps/web/src/components/pullRequest/useInlineActionConfirm.ts b/apps/web/src/components/pullRequest/useInlineActionConfirm.ts new file mode 100644 index 000000000000..5b4f7fc57212 --- /dev/null +++ b/apps/web/src/components/pullRequest/useInlineActionConfirm.ts @@ -0,0 +1,55 @@ +import { useEffect, useState, type MouseEvent } from "react"; + +/** Arms one control at a time, including controls rendered in a menu portal. */ +export function useInlineActionConfirm(scopeKey: string) { + const [armed, setArmed] = useState<{ + scopeKey: string; + action: string; + element: HTMLElement; + } | null>(null); + const active = armed?.scopeKey === scopeKey ? armed : null; + + useEffect(() => { + if (!armed) return; + const disarm = () => setArmed(null); + if (armed.scopeKey !== scopeKey) { + disarm(); + return; + } + const onPointerDown = (event: PointerEvent) => { + if (event.target instanceof Node && !armed.element.contains(event.target)) disarm(); + }; + const onKeyDown = (event: KeyboardEvent) => { + if (event.key === "Escape") { + event.preventDefault(); + event.stopPropagation(); + disarm(); + } + }; + const timeout = window.setTimeout(disarm, 3000); + document.addEventListener("pointerdown", onPointerDown, true); + window.addEventListener("keydown", onKeyDown, true); + window.addEventListener("blur", disarm); + return () => { + window.clearTimeout(timeout); + document.removeEventListener("pointerdown", onPointerDown, true); + window.removeEventListener("keydown", onKeyDown, true); + window.removeEventListener("blur", disarm); + }; + }, [armed, scopeKey]); + + return { + isArmed: (action: string) => active?.action === action, + props: (action: string, onConfirm: () => void) => ({ + onClick: (event: MouseEvent) => { + if (active?.action === action && active.element === event.currentTarget) { + setArmed(null); + onConfirm(); + } else { + setArmed({ scopeKey, action, element: event.currentTarget }); + } + }, + onBlur: () => setArmed(null), + }), + }; +}