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
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,34 @@ function setup() {
} satisfies ComponentProps<typeof ComposerPrimaryActions>;
}

it.each([false, true])(
"keeps repeat clicks away from Resume and Send (draft: %s)",
async (draft) => {
it.each(["resume", "draft", "question"] as const)(
"keeps repeat Stop clicks from reaching the action that replaces it (%s)",
async (mode) => {
const props = setup();
const container = document.createElement("div");
Object.assign(container.style, { display: "flex", justifyContent: "flex-end", width: "400px" });
await using screen = await render(<ComposerPrimaryActions {...props} />, { container });
const submit = vi.fn();
const pendingAction =
mode === "question"
? {
questionIndex: 0,
isLastQuestion: true,
canAdvance: true,
isResponding: false,
isComplete: true,
}
: null;
const view = (overrides: Partial<ComponentProps<typeof ComposerPrimaryActions>> = {}) => (
<form
style={{ display: "flex", justifyContent: "flex-end", width: "400px" }}
onSubmit={(event) => {
event.preventDefault();
submit();
}}
>
<ComposerPrimaryActions {...props} pendingAction={pendingAction} {...overrides} />
</form>
);
await using screen = await render(view());
using _timers = fakeTimers();
const stop = screen
.getByRole("button", { name: "Stop generation" })
.element() as HTMLButtonElement;
Expand All @@ -54,35 +75,34 @@ it.each([false, true])(
screen.getByRole("button", { name: "Stopping", exact: true }).element() as HTMLButtonElement
).click(),
);
await expect
.element(screen.getByRole("button", { name: "Stopping", exact: true }))
.toBeDisabled();
expect(screen.getByRole("button", { name: "Stopping", exact: true }).element()).toBeDisabled();
expect(props.onInterrupt).toHaveBeenCalledTimes(1);
await screen.rerender(
<ComposerPrimaryActions
{...props}
isRunning={false}
canInterrupt={false}
canResume
hasSendableContent={draft}
/>,
view({
isRunning: false,
canInterrupt: false,
canResume: true,
hasSendableContent: mode === "draft",
}),
);
await expect
.element(screen.getByRole("button", { name: "Stopped", exact: true }))
.toBeDisabled();
expect(target()).toHaveAttribute("aria-label", "Stopped");
const action = { resume: "Resume thread", draft: "Submit message", question: "Submit" }[mode];
expect(screen.getByRole("button", { name: "Stop generation" }).query()).toBeNull();
expect(target()).toBe(screen.getByRole("button", { name: action, exact: true }).element());
await act(() => {
for (let i = 0; i < 20; i++) target()?.click();
});
await act(() => vi.advanceTimersByTime(499));
await act(() => target()?.click());
expect(props.onResume).not.toHaveBeenCalled();
expect(props.onSubmitMessage).not.toHaveBeenCalled();
const action = screen
.getByRole("button", { name: draft ? "Submit message" : "Resume thread" })
.element() as HTMLButtonElement;
await act(() => action.click());
expect(draft ? props.onSubmitMessage : props.onResume).toHaveBeenCalledTimes(1);
await screen.rerender(<ComposerPrimaryActions {...props} />);
await expect.element(screen.getByRole("button", { name: "Stop generation" })).toBeEnabled();
expect(submit).not.toHaveBeenCalled();
await act(() => vi.advanceTimersByTime(1));
await act(() => target()?.click());
expect(
{ resume: props.onResume, draft: props.onSubmitMessage, question: submit }[mode],
).toHaveBeenCalledTimes(1);
await screen.rerender(view());
expect(screen.getByRole("button", { name: "Stop generation" }).element()).toBeEnabled();
},
);

Expand All @@ -106,7 +126,9 @@ it("allows retry after ten seconds and protects a late stop", async () => {
await screen.rerender(
<ComposerPrimaryActions {...props} isRunning={false} canInterrupt={false} canResume />,
);
expect(screen.getByRole("button", { name: "Stopped", exact: true }).element()).toBeDisabled();
await act(() =>
(screen.getByRole("button", { name: "Resume thread" }).element() as HTMLButtonElement).click(),
);
expect(props.onInterrupt).toHaveBeenCalledTimes(2);
expect(props.onResume).not.toHaveBeenCalled();
});
Expand Down
49 changes: 31 additions & 18 deletions apps/web/src/components/chat/ComposerPrimaryActions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ const formatPendingPrimaryActionLabel = (input: {
};

const STOP_PENDING_MAX_MS = 10_000;
// The action that takes Stop's place ignores clicks this long, so repeat Stop clicks can't resume.
const STOP_LANDED_CLICK_GUARD_MS = 500;

const messageActionPillClassName =
"inline-flex shrink-0 cursor-pointer items-center justify-center gap-1.5 whitespace-nowrap rounded-full bg-message-action font-medium text-base text-message-action-foreground shadow-xs shadow-message-action/24 outline-none hover:bg-message-action-hover focus-visible:ring-2 focus-visible:ring-inset focus-visible:ring-ring focus-visible:ring-offset-1 focus-visible:ring-offset-background disabled:pointer-events-none disabled:opacity-64 disabled:shadow-none sm:text-sm";
Expand Down Expand Up @@ -113,11 +115,16 @@ function useStopPhase(input: {
setStopPhase("landed");
else if (stopPhase === "landed" && canInterrupt) setStopPhase(null);
}
// oxlint-disable-next-line t3code/no-unreasoned-state-sync -- timer that re-enables Stop if the interrupt never settles
// oxlint-disable-next-line t3code/no-unreasoned-state-sync -- timers that re-enable an unsettled Stop and end the landed click guard
useEffect(() => {
if (stopPhase !== "pending") return;
const timeout = setTimeout(() => setStopPhase("timedOut"), STOP_PENDING_MAX_MS);
return () => clearTimeout(timeout);
if (stopPhase === "pending") {
const timeout = setTimeout(() => setStopPhase("timedOut"), STOP_PENDING_MAX_MS);
return () => clearTimeout(timeout);
}
if (stopPhase === "landed") {
const timeout = setTimeout(() => setStopPhase(null), STOP_LANDED_CLICK_GUARD_MS);
return () => clearTimeout(timeout);
}
}, [stopPhase]);
return [stopPhase, setStopPhase] as const;
}
Expand All @@ -137,7 +144,7 @@ function StopGenerationButton({
onStopRequested: () => void;
onInterrupt: () => void;
}) {
const isStopDisabled = stopPhase === "pending" || stopPhase === "landed";
const isStopDisabled = stopPhase === "pending";
return (
<Tooltip>
<TooltipTrigger
Expand All @@ -155,23 +162,15 @@ function StopGenerationButton({
onStopRequested();
onInterrupt();
}}
aria-label={
stopPhase === "landed"
? "Stopped"
: stopPhase === "pending"
? "Stopping"
: "Stop generation"
}
aria-label={stopPhase === "pending" ? "Stopping" : "Stop generation"}
/>
}
>
<svg width="12" height="12" viewBox="0 0 12 12" fill="currentColor" aria-hidden="true">
<rect x="2" y="2" width="8" height="8" rx="1.5" />
</svg>
</TooltipTrigger>
<TooltipPopup>
{stopPhase === "pending" ? "Stopping…" : stopPhase === "landed" ? "Stopped" : "Interrupt"}
</TooltipPopup>
<TooltipPopup>{stopPhase === "pending" ? "Stopping…" : "Interrupt"}</TooltipPopup>
</Tooltip>
);
}
Expand Down Expand Up @@ -283,7 +282,9 @@ export const ComposerPrimaryActions = memo(function ComposerPrimaryActions({
const withStopButton = (action: ReactNode) => (
<>
{action}
{stopPhase === null ? null : renderStopGenerationButton(false)}
{stopPhase === "pending" || stopPhase === "timedOut"
? renderStopGenerationButton(false)
: null}
</>
);

Expand Down Expand Up @@ -319,6 +320,9 @@ export const ComposerPrimaryActions = memo(function ComposerPrimaryActions({
type="submit"
className={cn(messageActionPillClassName, "h-8 sm:h-7", compact ? "px-3" : "px-4")}
{...pointerFocusProps}
onClick={(event) => {
if (stopPhase === "landed") event.preventDefault();
}}
disabled={
!canOperateThread ||
isEnvironmentUnavailable ||
Expand All @@ -337,7 +341,9 @@ export const ComposerPrimaryActions = memo(function ComposerPrimaryActions({
questionIndex: pendingAction.questionIndex,
})}
</button>
{canInterrupt || stopPhase !== null ? renderStopGenerationButton(true) : null}
{canInterrupt || stopPhase === "pending" || stopPhase === "timedOut"
? renderStopGenerationButton(true)
: null}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
</div>
);
}
Expand Down Expand Up @@ -402,7 +408,14 @@ export const ComposerPrimaryActions = memo(function ComposerPrimaryActions({
: "bg-message-action text-message-action-foreground enabled:shadow-message-action/24 hover:bg-message-action-hover disabled:bg-foreground/12 disabled:text-muted-foreground",
)}
{...pointerFocusProps}
onClick={showResume ? onResume : onSubmitMessage}
onClick={(event) => {
if (stopPhase === "landed") {
event.preventDefault();
return;
}
if (showResume) onResume?.();
else onSubmitMessage?.(event);
}}
disabled={
isSendBusy ||
isSendDisabled ||
Expand Down
Loading