+
{
+ if (element === null) return;
+ return observeResize(element, ([entry]) => {
+ if (entry) setOverflowing(entry.contentRect.height > 240);
+ });
+ }}
+ >
{overflowing && !expanded ? (
@@ -47,7 +47,7 @@ export function PullRequestCommentBody({
aria-expanded={expanded}
aria-controls={id}
onClick={() => {
- if (expanded) content.current?.parentElement?.scrollIntoView({ block: "nearest" });
+ if (expanded) clip.current?.scrollIntoView({ block: "nearest" });
setExpanded(!expanded);
}}
>
diff --git a/apps/web/src/components/pullRequest/useInlineActionConfirm.browser.test.tsx b/apps/web/src/components/pullRequest/useInlineActionConfirm.browser.test.tsx
new file mode 100644
index 0000000000000..90a0cffd859b6
--- /dev/null
+++ b/apps/web/src/components/pullRequest/useInlineActionConfirm.browser.test.tsx
@@ -0,0 +1,78 @@
+import { render } from "../../../test/render";
+import { expect, it, vi } from "vite-plus/test";
+import { userEvent } from "vite-plus/test/browser";
+
+import { useInlineActionConfirm } from "./useInlineActionConfirm";
+
+function MergeButton({ scopeKey, onConfirm }: { scopeKey: string; onConfirm: () => void }) {
+ const confirm = useInlineActionConfirm(scopeKey);
+ return (
+
+ );
+}
+
+function pressEscape(): KeyboardEvent {
+ const event = new KeyboardEvent("keydown", { key: "Escape", bubbles: true, cancelable: true });
+ window.dispatchEvent(event);
+ return event;
+}
+
+it("confirms on the second click and disarms on Escape", async () => {
+ const onConfirm = vi.fn();
+ await using screen = await render(
);
+ const button = screen.getByRole("button");
+
+ await userEvent.click(button);
+ await expect.element(button).toHaveTextContent("Confirm merge?");
+ expect(pressEscape().defaultPrevented).toBe(true);
+ await expect.element(button).toHaveTextContent("Merge");
+
+ await userEvent.click(button);
+ await userEvent.click(button);
+ expect(onConfirm).toHaveBeenCalledTimes(1);
+ await expect.element(button).toHaveTextContent("Merge");
+});
+
+it("stops swallowing Escape once the scope changes", async () => {
+ const onConfirm = vi.fn();
+ await using screen = await render(
);
+ const button = screen.getByRole("button");
+
+ await userEvent.click(button);
+ await expect.element(button).toHaveTextContent("Confirm merge?");
+ await screen.rerender(
);
+ await expect.element(button).toHaveTextContent("Merge");
+ expect(pressEscape().defaultPrevented).toBe(false);
+
+ await screen.rerender(
);
+ await expect.element(button).toHaveTextContent("Merge");
+});
+
+// The hook's owner stays mounted while the armed button goes away, as when the primary
+// action changes under an armed merge button.
+function ToolbarWithMergeButton({ shown }: { shown: boolean }) {
+ const confirm = useInlineActionConfirm("a");
+ return shown ? (
+
+ ) : (
+
gone
+ );
+}
+
+it("stops swallowing Escape once the armed button unmounts", async () => {
+ await using screen = await render(
);
+ const button = screen.getByRole("button");
+
+ await userEvent.click(button);
+ await expect.element(button).toHaveTextContent("Confirm merge?");
+ await screen.rerender(
);
+ await expect.element(screen.getByText("gone")).toBeVisible();
+ expect(pressEscape().defaultPrevented).toBe(false);
+
+ await screen.rerender(
);
+ await expect.element(screen.getByRole("button")).toHaveTextContent("Merge");
+});
diff --git a/apps/web/src/components/pullRequest/useInlineActionConfirm.ts b/apps/web/src/components/pullRequest/useInlineActionConfirm.ts
index 69224f7090110..3729e025544f0 100644
--- a/apps/web/src/components/pullRequest/useInlineActionConfirm.ts
+++ b/apps/web/src/components/pullRequest/useInlineActionConfirm.ts
@@ -1,51 +1,68 @@
-import { useEffect, useState, type MouseEvent } from "react";
+import { useState, type MouseEvent } from "react";
+
+interface ArmedAction {
+ readonly scopeKey: string;
+ readonly action: string;
+ readonly element: HTMLElement;
+ // Stable per arming, so React attaches it once. Detaching it (disarm, scope change, or the
+ // armed element unmounting) removes the listeners and disarms.
+ readonly ref: (node: HTMLElement | null) => (() => void) | undefined;
+}
export function useInlineActionConfirm(scopeKey: string) {
- const [armed, setArmed] = useState<{
- scopeKey: string;
- action: string;
- element: HTMLElement;
- } | null>(null);
+ const [armed, setArmed] = useState
(null);
+ if (armed !== null && armed.scopeKey !== scopeKey) setArmed(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);
+ const arm = (action: string, element: HTMLElement) => {
+ const entry: ArmedAction = {
+ scopeKey,
+ action,
+ element,
+ ref: (node) => {
+ if (node === null) return undefined;
+ const controller = new AbortController();
+ const { signal } = controller;
+ const release = () => setArmed((current) => (current === entry ? null : current));
+ document.addEventListener(
+ "pointerdown",
+ (event) => {
+ if (event.target instanceof Node && !node.contains(event.target)) release();
+ },
+ { capture: true, signal },
+ );
+ window.addEventListener(
+ "keydown",
+ (event) => {
+ if (event.key !== "Escape") return;
+ event.preventDefault();
+ event.stopPropagation();
+ release();
+ },
+ { capture: true, signal },
+ );
+ window.addEventListener("blur", release, { signal });
+ const timeout = window.setTimeout(release, 3000);
+ return () => {
+ controller.abort();
+ window.clearTimeout(timeout);
+ release();
+ };
+ },
};
- }, [armed, scopeKey]);
+ setArmed(entry);
+ };
return {
isArmed: (action: string) => active?.action === action,
props: (action: string, onConfirm: () => void) => ({
+ ref: active?.action === action ? active.ref : undefined,
onClick: (event: MouseEvent) => {
if (active?.action === action && active.element === event.currentTarget) {
setArmed(null);
onConfirm();
} else {
- setArmed({ scopeKey, action, element: event.currentTarget });
+ arm(action, event.currentTarget);
}
},
onBlur: () => setArmed(null),