Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
56 changes: 56 additions & 0 deletions docs/notes/frontend.md
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,21 @@ explicit cancel like the old Modal backing.

Fonte: `src/chrome/ConfirmDialog.tsx:ConfirmDialog`

<a id="one-action-per-mount"></a>

### One action per mount: the outro is not a second chance

`requestClose` guards its own timer, but that only makes the *exit*
idempotent. The dialog stays mounted for the 150ms of the outro, and the
`keydown` listener on `window` is still attached, so a second request in
that window — Escape landing after confirm, or a click that the
`modal-panel-closing` layer no longer blocks — would overwrite the
`pendingAction` that is already waiting to run. Confirming a destructive
thing would silently become cancelling it. Guard the pending action
itself: the first request wins, and it is the one that runs on exit.

Fonte: `src/chrome/ConfirmDialog.tsx:ConfirmDialog`

<a id="claim-focus-next-frame"></a>

### The close button owns initial focus; claim the next frame
Expand Down Expand Up @@ -1843,6 +1858,47 @@ Hidden tabs and reduced-motion styles may never fire animationend.

Fonte: `src/surfaces/AgentTranscript.tsx:TurnRow`

<a id="turn-enters-once"></a>

### A turn enters once, on append

Only the newly appended turn plays `turn-enter`, and only while the
experimental flag is on. History mounts dry: the first sight of the last
turn seeds the ref without animating. The id clears on `animationend`, and
a timeout at `--motion-in` covers engines where it never fires, so a
remount on scroll finds nothing to replay.

Fonte: `src/surfaces/AgentTranscript.tsx:AgentTranscriptContent`

<a id="empty-conversation-sees-nothing"></a>

### An empty conversation has seen nothing, which is not "seen the first turn"

Seeding the ref from `lastTurnId` alone cannot tell a conversation that
opened empty from one that opened with history: in both cases the id is
`null` at first, so recording it and returning leaves the first real turn
of an empty conversation mounting dry. Letting it fall through instead
animates history on load. The initialisation therefore gets its own flag
and is allowed to *not* record, once per mount — which also has to
survive the effect firing a second time for the same commit, or the first
arrival would be consumed by that repeat pass.

Fonte: `src/surfaces/AgentTranscript.tsx:AgentTranscriptContent`

<a id="enter-clears-without-animations"></a>

### Turning the flag off mid-entry clears the enter itself

The flag can go off while a turn is entering. The effect cleanup has
already cancelled the timeout that would have cleared the id, and with the
CSS off there is no `animationend` to end it either, so `turn-enter` would
sit on the turn until the next mount and replay on remount. Clearing at the
top of the effect, not in the `!animated` branch further down, because
that branch is never reached when the turn itself has not changed — which
is exactly the case where the flag flipped.

Fonte: `src/surfaces/AgentTranscript.tsx:AgentTranscriptContent`

<a id="inert-out-of-tab-order"></a>

### Folded work stays out of tab order via `inert`
Expand Down
87 changes: 84 additions & 3 deletions src/chrome/ConfirmDialog.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,9 @@ describe("ConfirmDialog", () => {
}
});

it("cancels on Escape and confirms on click", async () => {
it("cancels on Escape", async () => {
const onCancel = vi.fn();
const onConfirm = vi.fn();
const { root, container } = renderDialog(onCancel, onConfirm);
const { root, container } = renderDialog(onCancel, vi.fn());
try {
await act(async () => {});
act(() => {
Expand All @@ -66,6 +65,17 @@ describe("ConfirmDialog", () => {
);
});
expect(onCancel).toHaveBeenCalledTimes(1);
} finally {
act(() => root.unmount());
container.remove();
}
});

it("confirms on click", async () => {
const onConfirm = vi.fn();
const { root, container } = renderDialog(vi.fn(), onConfirm);
try {
await act(async () => {});
const confirm = [...document.querySelectorAll("button")].find(
(b) => b.textContent === "Remove",
)!;
Expand All @@ -76,4 +86,75 @@ describe("ConfirmDialog", () => {
container.remove();
}
});

it("plays the outro before the action with animations on", async () => {
vi.useFakeTimers();
localStorage.setItem("monocode.experimentalAnimations", "1");
const onCancel = vi.fn();
const { root, container } = renderDialog(onCancel, vi.fn());
try {
await act(async () => {});
const panel = document.querySelector(".modal-panel")!;
expect(panel).not.toBeNull();

act(() => {
window.dispatchEvent(
new KeyboardEvent("keydown", { key: "Escape", bubbles: true }),
);
});
// Still mounted, playing the outro: no invisible gap where the dialog
// is gone but the action has not run.
expect(document.querySelector('[role="alertdialog"]')).not.toBeNull();
expect(
document.querySelector(".modal-panel-closing"),
).not.toBeNull();
expect(onCancel).not.toHaveBeenCalled();

act(() => {
vi.advanceTimersByTime(150);
});
expect(onCancel).toHaveBeenCalledTimes(1);
} finally {
localStorage.removeItem("monocode.experimentalAnimations");
vi.useRealTimers();
act(() => root.unmount());
container.remove();
}
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.

it("runs the action asked for first when a second one arrives during the outro", async () => {
// The dialog is still mounted for the 150ms of the outro and the window
// keydown listener is still attached, so Escape lands while a confirm is
// already waiting. Confirming something must not become cancelling it.
const onCancel = vi.fn();
const onConfirm = vi.fn();
vi.useFakeTimers();
localStorage.setItem("monocode.experimentalAnimations", "1");
const { root, container } = renderDialog(onCancel, onConfirm);
try {
await act(async () => {});

const confirm = [...document.querySelectorAll("button")].find(
(b) => b.textContent === "Remove",
)!;
act(() => confirm.click());
// A second request, inside the outro and before the timer.
act(() => {
window.dispatchEvent(
new KeyboardEvent("keydown", { key: "Escape", bubbles: true }),
);
});

act(() => {
vi.advanceTimersByTime(150);
});
expect(onConfirm).toHaveBeenCalledTimes(1);
expect(onCancel).not.toHaveBeenCalled();
act(() => root.unmount());
container.remove();
} finally {
localStorage.removeItem("monocode.experimentalAnimations");
vi.useRealTimers();
}
});
});
55 changes: 47 additions & 8 deletions src/chrome/ConfirmDialog.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import { useEffect, useRef, type ReactNode } from "react";
import { useCallback, useEffect, useRef, type ReactNode } from "react";
import { useLockOverscroll } from "../hooks/useLockOverscroll";
import {
useExitAnimation,
useExperimentalAnimations,
} from "../hooks/useExitAnimation";
import {
AlertDialog,
AlertDialogDescription,
Expand Down Expand Up @@ -40,6 +44,34 @@ export function ConfirmDialog({
const cancelRef = useRef<HTMLButtonElement>(null);
const previousFocus = useRef<HTMLElement | null>(null);
const lockOverscroll = useLockOverscroll<HTMLDivElement>();
const animationsEnabled = useExperimentalAnimations();
// Nota: docs/notes/frontend.md#enter-exit-behind-flag
const pendingAction = useRef<(() => void) | null>(null);
const actionRef = useRef({ onCancel, onConfirm });
// Nota: docs/notes/frontend.md#effect-written-callbacks
useEffect(() => {
actionRef.current = { onCancel, onConfirm };
}, [onCancel, onConfirm]);
const { closing, requestClose, handleAnimationEnd } = useExitAnimation({
enabled: animationsEnabled,
durationMs: 150,
onExit: () => {
const action = pendingAction.current;
pendingAction.current = null;
action?.();
},
});
const requestAction = useCallback(
(kind: "cancel" | "confirm") => {
// Nota: docs/notes/frontend.md#one-action-per-mount
if (pendingAction.current) return;
pendingAction.current = () => actionRef.current[
kind === "cancel" ? "onCancel" : "onConfirm"
]();
requestClose();
},
[requestClose],
);
Comment thread
coderabbitai[bot] marked this conversation as resolved.

useEffect(() => {
previousFocus.current =
Expand All @@ -60,26 +92,33 @@ export function ConfirmDialog({
if (event.key !== "Escape" || event.defaultPrevented) return;
event.preventDefault();
event.stopPropagation();
onCancel();
requestAction("cancel");
};
window.addEventListener("keydown", onKey, true);
return () => window.removeEventListener("keydown", onKey, true);
}, [onCancel]);
}, [requestAction]);

return (
<AlertDialog
open
onOpenChange={(open) => {
if (!open) onCancel();
if (!open) requestAction("cancel");
}}
>
<AlertDialogPortal>
<div className="modal-backdrop absolute inset-0 bg-black/40" />
<div
className={`modal-backdrop absolute inset-0 bg-black/40${
closing ? " modal-backdrop-closing" : ""
}`}
/>
<AlertDialogPopup
initialFocus={() => cancelRef.current ?? false}
finalFocus={false}
>
<AlertDialogPanel>
<AlertDialogPanel
className={closing ? "modal-panel-closing" : "modal-panel"}
onAnimationEnd={closing ? handleAnimationEnd : undefined}
>
<div className="flex shrink-0 flex-col px-4 pt-3">
<AlertDialogTitle className="text-2xl font-semibold leading-tight text-content">
{title}
Expand All @@ -96,14 +135,14 @@ export function ConfirmDialog({
<button
ref={cancelRef}
type="button"
onClick={onCancel}
onClick={() => requestAction("cancel")}
className="rounded-md px-3 py-1.5 text-[12px] text-content/70 hover:bg-content/8 hover:text-content focus-visible:outline-2 focus-visible:outline-accent"
>
{cancelLabel}
</button>
<button
type="button"
onClick={onConfirm}
onClick={() => requestAction("confirm")}
className={`rounded-md px-3 py-1.5 text-[12px] font-medium focus-visible:outline-2 focus-visible:outline-accent ${
danger
? "bg-red-500/20 text-red-300 hover:bg-red-500/30"
Expand Down
78 changes: 78 additions & 0 deletions src/chrome/ImageLightbox.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
// @vitest-environment happy-dom
import { act, createElement } from "react";
import { createRoot, type Root } from "react-dom/client";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { ImageLightbox } from "./ImageLightbox";

let container: HTMLDivElement;
let root: Root;

beforeEach(() => {
vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true);
vi.useFakeTimers();
container = document.createElement("div");
document.body.append(container);
root = createRoot(container);
});

afterEach(() => {
act(() => root.unmount());
container.remove();
localStorage.removeItem("monocode.experimentalAnimations");
vi.unstubAllGlobals();
vi.useRealTimers();
});

function renderLightbox(onClose: () => void) {
act(() => {
root.render(
createElement(ImageLightbox, {
src: "asset://preview.png",
alt: "preview",
onClose,
}),
);
});
}

describe("ImageLightbox", () => {
it("closes immediately with animations off", () => {
const onClose = vi.fn();
renderLightbox(onClose);
expect(document.querySelector('[role="dialog"]')).not.toBeNull();

act(() => {
document
.querySelector<HTMLButtonElement>('button[aria-label="Close image preview"]')!
.click();
});
expect(onClose).toHaveBeenCalledTimes(1);
});

it("enters with the modal language and plays the outro before close", () => {
localStorage.setItem("monocode.experimentalAnimations", "1");
const onClose = vi.fn();
renderLightbox(onClose);

const dialog = document.querySelector('[role="dialog"]')!;
expect(dialog.className).toContain("modal-backdrop");
expect(dialog.querySelector("img")?.className).toContain("modal-panel");

act(() => {
window.dispatchEvent(
new KeyboardEvent("keydown", { key: "Escape", bubbles: true }),
);
});
expect(document.querySelector('[role="dialog"]')).not.toBeNull();
expect(dialog.className).toContain("modal-backdrop-closing");
expect(dialog.querySelector("img")?.className).toContain(
"modal-panel-closing",
);
expect(onClose).not.toHaveBeenCalled();

act(() => {
vi.advanceTimersByTime(150);
});
expect(onClose).toHaveBeenCalledTimes(1);
});
});
Loading
Loading