Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
maria-rcks merged 1 commit into from
Oct 10, 2026
+174 −132
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
251 changes: 119 additions & 132 deletions apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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<PullRequestAction, string> = {
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<PullRequestAction, string> = {
merge: "Could not merge this pull request",
Expand Down Expand Up @@ -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<string | null>(null);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
}
>
<PullRequestGlyph.merged aria-hidden className="size-3.5" />
<span className="@max-[30rem]/pr-header:hidden">
{pendingAction === "enable-auto-merge"
? "Enabling..."
: pendingAutoMergeLabel}
<span className="inline-grid @max-[30rem]/pr-header:hidden">
<span aria-hidden className="invisible col-start-1 row-start-1">
{pendingAutoMergeLabel}
</span>
<span aria-hidden className="invisible col-start-1 row-start-1">
Confirm auto-merge?
</span>
<span className="col-start-1 row-start-1">
{pendingAction === "enable-auto-merge"
? "Enabling..."
: inlineConfirm.isArmed("enable-auto-merge")
? "Confirm auto-merge?"
: pendingAutoMergeLabel}
</span>
</span>
</Button>
</span>
}
/>
<TooltipPopup side="top">
{pendingAction === "enable-auto-merge" ? "Enabling..." : pendingAutoMergeLabel}
{pendingAction === "enable-auto-merge"
? "Enabling..."
: inlineConfirm.isArmed("enable-auto-merge")
? "Confirm auto-merge?"
: pendingAutoMergeLabel}
</TooltipPopup>
</Tooltip>
) : primaryAction === "auto-merge-armed" ? (
Expand Down Expand Up @@ -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
}
>
<PullRequestGlyph.merged aria-hidden className="size-3.5" />
<span className="@max-[30rem]/pr-header:hidden">
{pendingAction === "merge" ? "Merging..." : selectedMergeMethodLabel}
<span className="inline-grid @max-[30rem]/pr-header:hidden">
<span aria-hidden className="invisible col-start-1 row-start-1">
{selectedMergeMethodLabel}
</span>
<span aria-hidden className="invisible col-start-1 row-start-1">
Confirm merge?
</span>
<span className="col-start-1 row-start-1">
{pendingAction === "merge"
? "Merging..."
: inlineConfirm.isArmed("merge")
? "Confirm merge?"
: selectedMergeMethodLabel}
</span>
</span>
</Button>
</span>
}
/>
<TooltipPopup side="top">
{pendingAction === "merge" ? "Merging..." : selectedMergeMethodLabel}
{pendingAction === "merge"
? "Merging..."
: inlineConfirm.isArmed("merge")
? "Confirm merge?"
: selectedMergeMethodLabel}
</TooltipPopup>
</Tooltip>
) : (primaryAction === "merged" || primaryAction === "closed") &&
Expand Down Expand Up @@ -2101,11 +2116,18 @@ export function PullRequestDetailPanel({
) : null}
{showsMergeNow ? (
<MenuItem
closeOnClick={inlineConfirm.isArmed("merge")}
aria-label={
inlineConfirm.isArmed("merge") ? "Confirm merge?" : "Merge now"
}
disabled={actionPending}
onClick={() => setConfirmation({ open: true, action: "merge" })}
{...inlineConfirm.props(
"merge",
() => void perform("merge", selectedMergeMethod),
)}
>
<PullRequestGlyph.merged className="size-3.5" />
Merge now
{inlineConfirm.isArmed("merge") ? "Confirm merge?" : "Merge now"}
</MenuItem>
) : null}
{/* The same merge, left with the host to carry out once its requirements
Expand All @@ -2121,13 +2143,22 @@ export function PullRequestDetailPanel({
</MenuItem>
) : showsAutoMerge ? (
<MenuItem
disabled={actionPending}
onClick={() =>
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),
)}
>
<PullRequestGlyph.merged className="size-3.5" />
Enable auto-merge
{inlineConfirm.isArmed("enable-auto-merge")
? "Confirm auto-merge?"
: "Enable auto-merge"}
</MenuItem>
) : null}
{/* A preference for the merge action rather than a second action, so it
Expand Down Expand Up @@ -2200,12 +2231,16 @@ export function PullRequestDetailPanel({
<>
<MenuSeparator />
<MenuItem
closeOnClick={inlineConfirm.isArmed("close")}
aria-label={
inlineConfirm.isArmed("close") ? "Confirm close?" : "Close pull request"
}
variant="destructive"
disabled={actionPending}
onClick={() => setConfirmation({ open: true, action: "close" })}
{...inlineConfirm.props("close", () => void perform("close"))}
>
<PullRequestGlyph.closed className="size-3.5" />
Close pull request
{inlineConfirm.isArmed("close") ? "Confirm close?" : "Close pull request"}
</MenuItem>
</>
) : detail.state === "closed" && can("reopen") ? (
Expand All @@ -2220,11 +2255,16 @@ export function PullRequestDetailPanel({
<>
<MenuSeparator />
<MenuItem
closeOnClick={inlineConfirm.isArmed("revert")}
aria-label={
inlineConfirm.isArmed("revert") ? "Confirm revert?" : "Revert changes"
}
variant={inlineConfirm.isArmed("revert") ? "destructive" : "default"}
disabled={actionPending}
onClick={() => setConfirmation({ open: true, action: "revert" })}
{...inlineConfirm.props("revert", () => void perform("revert"))}
>
<RotateCcwIcon className="size-3.5" />
Revert changes
{inlineConfirm.isArmed("revert") ? "Confirm revert?" : "Revert changes"}
</MenuItem>
</>
) : null}
Expand Down Expand Up @@ -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"
}
>
<PlayIcon aria-hidden className="size-3.5" />
<span>
{pendingAction === "approve-workflows"
? "Approving..."
: "Approve workflows to run"}
<span className="inline-grid">
<span aria-hidden className="invisible col-start-1 row-start-1">
Approve workflows to run
</span>
<span aria-hidden className="invisible col-start-1 row-start-1">
Confirm approve?
</span>
<span className="col-start-1 row-start-1">
{pendingAction === "approve-workflows"
? "Approving..."
: inlineConfirm.isArmed("approve-workflows")
? "Confirm approve?"
: "Approve workflows to run"}
</span>
</span>
</Button>
</span>
Expand All @@ -2575,7 +2628,9 @@ export function PullRequestDetailPanel({
<TooltipPopup side="top">
{pendingAction === "approve-workflows"
? "Approving..."
: "Approve workflows to run"}
: inlineConfirm.isArmed("approve-workflows")
? "Confirm approve?"
: "Approve workflows to run"}
</TooltipPopup>
</Tooltip>
) : (
Expand Down Expand Up @@ -2802,74 +2857,6 @@ export function PullRequestDetailPanel({
/>
</div>
) : null}

<AlertDialog
open={confirmation.open}
onOpenChange={(open) => setConfirmation((current) => ({ ...current, open }))}
onOpenChangeComplete={(open) => {
if (!open) setConfirmation({ open: false, action: "merge" });
}}
>
<AlertDialogPopup>
<AlertDialogHeader>
<AlertDialogTitle>
{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?"}
</AlertDialogTitle>
<AlertDialogDescription>
{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.`}
</AlertDialogDescription>
</AlertDialogHeader>
<AlertDialogFooter>
<AlertDialogClose render={<Button variant="outline" size="sm" />}>
Cancel
</AlertDialogClose>
<Button
size="sm"
variant={confirmAction === "close" ? "destructive" : "default"}
disabled={actionPending}
onClick={() => {
const action = confirmAction;
setConfirmation((current) => ({ ...current, open: false }));
if (action === "merge") void perform("merge", selectedMergeMethod);
if (action === "enable-auto-merge")
void perform("enable-auto-merge", selectedMergeMethod);
if (action === "revert") void perform("revert");
if (action === "approve-workflows") void perform("approve-workflows");
if (action === "close") void perform("close");
}}
>
{confirmAction === "merge"
? selectedMergeMethodLabel
: confirmAction === "enable-auto-merge"
? "Enable auto-merge"
: confirmAction === "revert"
? "Create revert PR"
: confirmAction === "approve-workflows"
? "Approve and run"
: "Close"}
</Button>
</AlertDialogFooter>
</AlertDialogPopup>
</AlertDialog>
</div>
);
}
Loading
Loading