Skip to content
Open
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
15 changes: 15 additions & 0 deletions src/ui/activity.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,21 @@ describe("Activity", () => {
mounted.unmount();
});

test("a focused step sliding out of the latest ten hands focus to the control", () => {
const steps = Array.from({ length: 10 }, (_, index) => call(index));
const mounted = mount(<Activity steps={steps} defaultOpen streaming />);
const oldest = mounted.container.querySelector("li[data-step] button");
if (!(oldest instanceof HTMLButtonElement))
throw new Error("expected a control in the oldest step");
act(() => oldest.focus());
mounted.render(
<Activity steps={[...steps, call(10)]} defaultOpen streaming />,
);
expect(shownSteps(mounted.container)).not.toContain("files__read_0");
expect(document.activeElement?.textContent).toBe("Show 1 earlier step");
mounted.unmount();
});

test("ten or fewer steps need no control", () => {
const steps = Array.from({ length: 10 }, (_, index) => call(index));
const mounted = mount(<Activity steps={steps} defaultOpen />);
Expand Down
36 changes: 35 additions & 1 deletion src/ui/activity.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,8 @@ function stepKey(step: ActivityStep, index: number): string {
/**
* The run's steps, while open: the latest ten, with a control for the rest.
* A finished thought with no text is left out, as `Reasoning` shows nothing
* for it.
* for it. When a new step pushes the step holding focus behind the control,
* focus moves to the control.
*/
export function ActivityContent({
renderStep,
Expand Down Expand Up @@ -300,7 +301,10 @@ function ActivitySteps({
}) {
const [showAll, setShowAll] = useState(false);
const listRef = useRef<HTMLOListElement | null>(null);
const showEarlierRef = useRef<HTMLButtonElement | null>(null);
const revealed = useRef(false);
// The key of the step holding focus, while focus is in the list.
const focusedStep = useRef<string | null>(null);
const hidden = showAll ? 0 : Math.max(0, steps.length - SHOWN_STEPS);

// The control disappears once pressed, so focus moves to the first step
Expand All @@ -311,6 +315,21 @@ function ActivitySteps({
listRef.current?.querySelector<HTMLElement>("li[data-step]")?.focus();
}, [showAll]);

// A new step slides the oldest shown one behind the control. If focus
// went with it, it moves to the control, which now counts that step.
useLayoutEffect(() => {
const key = focusedStep.current;
if (key === null) return;
const active = document.activeElement;
if (active !== null && active !== document.body) return;
const behind = steps
.slice(0, hidden)
.some(({ step, index }) => stepKey(step, index) === key);
if (!behind) return;
focusedStep.current = null;
showEarlierRef.current?.focus();
});

return (
<ol
ref={listRef}
Expand All @@ -320,10 +339,24 @@ function ActivitySteps({
"ms-1 mt-1 flex flex-col gap-1 border-s-2 border-border ps-3",
className,
)}
onFocus={(event) => {
focusedStep.current =
event.target
.closest("li[data-step]")
?.getAttribute("data-step-key") ?? null;
}}
onBlur={(event) => {
// A step going away with focus in it does not reach here: React
// ignores blur while it commits, so the effect still sees the key.
const next = event.relatedTarget;
if (next === null || !event.currentTarget.contains(next))
focusedStep.current = null;
}}
>
{hidden > 0 ? (
<li>
<button
ref={showEarlierRef}
type="button"
onClick={() => {
revealed.current = true;
Expand All @@ -339,6 +372,7 @@ function ActivitySteps({
<li
key={stepKey(step, index)}
data-step={step.kind}
data-step-key={stepKey(step, index)}
tabIndex={-1}
className="min-w-0 rounded-md"
>
Expand Down
36 changes: 36 additions & 0 deletions src/ui/message-parts.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,42 @@ describe("MessageParts focus after a decision", () => {
outside.remove();
});

test("focus that left for the page before the decision shows stays there", () => {
const decisions: ToolApprovalDecision[] = [];
const props = (parts: readonly ConversationPart[]) =>
inMessage({ parts, onDecide: (_, decision) => decisions.push(decision) });
const { container, render } = mount(props(RUN_THEN_WAIT));
press(button(container, "Approve"));
// A click on blank page: focus leaves to nothing.
act(() => (document.activeElement as HTMLElement).blur());
const [decision] = decisions;
if (decision === undefined) throw new Error("expected a decision");
render(props(decideIn(RUN_THEN_WAIT, "send", decision)));
expect(document.activeElement).toBe(document.body);
});

test("a decision recorded while the window is away still moves focus", () => {
const decisions: ToolApprovalDecision[] = [];
const props = (parts: readonly ConversationPart[]) =>
inMessage({ parts, onDecide: (_, decision) => decisions.push(decision) });
const { container, render } = mount(props(RUN_THEN_WAIT));
press(button(container, "Approve"));
// Another tab or window takes focus: the button blurs to nothing.
const hasFocus = document.hasFocus;
document.hasFocus = () => false;
try {
act(() => (document.activeElement as HTMLElement).blur());
const [decision] = decisions;
if (decision === undefined) throw new Error("expected a decision");
render(props(decideIn(RUN_THEN_WAIT, "send", decision)));
} finally {
document.hasFocus = hasFocus;
}
expect(
(document.activeElement as HTMLElement | null)?.dataset["slot"],
).toBe("activity-trigger");
});

test("a host's renderApproval gets the handoff through decide", () => {
const { container } = mount(
<Host
Expand Down
11 changes: 7 additions & 4 deletions src/ui/message-parts.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -489,13 +489,16 @@ export function MessageParts({
};

const untrackFocus = (event: FocusEvent<HTMLDivElement>) => {
// Focus leaving for the page or outside ends both watches. A card's
// buttons, or a batch's header, going away with the call does not reach
// here: React ignores blur while it commits, so the watch survives until
// the effect reads it. A decision stays watched until `parts` shows it.
// Focus leaving for the page or outside ends every watch, a decision
// `parts` does not show yet included, unless the window lost focus to
// another tab or window: the person has not left the buttons then. A
// card's buttons, or a batch's header, going away with the call does not
// reach here: React ignores blur while it commits, so the watch survives
// until the effect reads it.
const next = event.relatedTarget;
if (next === null || !event.currentTarget.contains(next)) {
focused.current = null;
if (document.hasFocus()) decided.current = null;
batchFocused.current = null;
}
onBlur?.(event);
Expand Down
11 changes: 8 additions & 3 deletions src/ui/tool-group.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@ import { ToolApproval, type ToolApprovalDecision } from "./tool-approval.js";
export type ToolGroupLabels = {
/** Every call has finished. */
readonly ran: (count: number) => string;
/** Some calls are still queued or running. */
/**
* Some calls are still queued or running. `done` here and in `held`
* leaves out failed calls, which `failed` counts.
*/
readonly running: (count: number, done: number) => string;
/** None is queued or running, but some wait on a person. */
readonly held: (count: number, done: number) => string;
Expand Down Expand Up @@ -124,6 +127,8 @@ export const ToolGroup = forwardRef<HTMLDivElement, ToolGroupProps>(
).length;
const failed = countFailedTools(part.tools);
const count = part.tools.length;
// Finished without failing, so each call counts once in the header.
const done = count - active - waiting - failed;

const rootRef = useRef<HTMLDivElement | null>(null);
const focusInside = useRef(false);
Expand Down Expand Up @@ -229,9 +234,9 @@ export const ToolGroup = forwardRef<HTMLDivElement, ToolGroupProps>(
/>
<span className="min-w-0 flex-1 truncate">
{active > 0
? labels.running(count, count - active - waiting)
? labels.running(count, done)
: waiting > 0
? labels.held(count, count - waiting)
? labels.held(count, done)
: labels.ran(count)}
{waiting > 0 ? <> · {labels.waiting(waiting)}</> : null}
{failed > 0 ? (
Expand Down
41 changes: 41 additions & 0 deletions src/ui/tool.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,47 @@ describe("ToolGroup", () => {
mounted.unmount();
});

test("a failed call is counted as failed, not also as done", () => {
const running = mount(
<ToolGroup
part={batch([
tool({ toolCallId: "a" }),
tool({ toolCallId: "b", status: "error" }),
tool({ toolCallId: "c", status: "running" }),
])}
/>,
);
expect(
running.container.querySelector("[data-slot='tool-group-header']")
?.textContent,
).toBe("Running 3 at once · 1 done · 1 failed");
running.unmount();

const held = mount(
<ToolGroup
part={batch([
tool({ toolCallId: "a" }),
tool({ toolCallId: "b", status: "error" }),
tool({
toolCallId: "c",
status: "approval-requested",
approval: {
id: "x",
status: "pending",
scope: null,
timeoutAt: null,
},
}),
])}
/>,
);
expect(
held.container.querySelector("[data-slot='tool-group-header']")
?.textContent,
).toBe("3 at once · 1 done · 1 waiting for approval · 1 failed");
held.unmount();
});

test("a call waiting on a person is not counted as done", () => {
const pending = {
id: "x",
Expand Down
Loading