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
38 changes: 38 additions & 0 deletions e2e/conversation-map.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,44 @@ test.describe("conversation map", () => {
expect(await page.evaluate(() => window.scrollY)).toBe(0);
});

test("moving along the rail by keyboard scrolls only the rail", async ({
page,
}) => {
await page.goto("/conversation-map-tall-page");
await expect(current(page)).toHaveAccessibleName("Question 79");
await expect
.poll(() => railState(page))
.toEqual({ overflows: true, scrolled: true, showsCurrent: true });
// Focus the tab stop without scrolling, so the page is still at the top
// and the map below the fold.
await tick(page, "Question 79").evaluate((element) =>
(element as HTMLElement).focus({ preventScroll: true }),
);
/** Whether the rail shows the focused tick. */
const showsFocused = () =>
page
.locator("[data-slot='conversation-map-rail']")
.evaluate((element) => {
const view = element.getBoundingClientRect();
const box = document.activeElement?.getBoundingClientRect();
return (
box !== undefined &&
box.top >= view.top - 1 &&
box.bottom <= view.bottom + 1
);
});

await page.keyboard.press("Home");
await expect(tick(page, "Question 0")).toBeFocused();
await expect.poll(showsFocused).toBe(true);
expect(await page.evaluate(() => window.scrollY)).toBe(0);

await page.keyboard.press("End");
await expect(tick(page, "Question 79")).toBeFocused();
await expect.poll(showsFocused).toBe(true);
expect(await page.evaluate(() => window.scrollY)).toBe(0);
});

test("a rail that shrinks keeps the current tick in view", async ({
page,
}) => {
Expand Down
39 changes: 39 additions & 0 deletions src/ui/conversation-earlier.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -213,4 +213,43 @@ describe("ConversationEarlier", () => {
container.querySelector("[role='log']"),
);
});

test("a retry that ends the history leaves no focus to restore later", () => {
const { container, rerender } = render(
<Marker stalled onReachStart={() => {}} />,
);
const button = container.querySelector("button");
if (button === null) throw new Error("expected the retry button");
button.focus();
// The retry brings the last page in the same commit: the marker goes.
act(() => button.click());
rerender(<Marker hasEarlier={false} onReachStart={() => {}} />);
expect(marker(container)).toBeNull();
act(() => (document.activeElement as HTMLElement | null)?.blur());
expect(document.activeElement).toBe(document.body);

// Older items turn up again, and a later load stalls and recovers.
rerender(<Marker stalled onReachStart={() => {}} />);
rerender(<Marker loading onReachStart={() => {}} />);
expect(document.activeElement).toBe(document.body);
});

test("a retry that ends the history while stalled leaves no focus to restore later", () => {
const { container, rerender } = render(
<Marker stalled onReachStart={() => {}} />,
);
const button = container.querySelector("button");
if (button === null) throw new Error("expected the retry button");
button.focus();
// Stalled throughout: only the marker's coming and going can reset it.
act(() => button.click());
rerender(<Marker stalled hasEarlier={false} onReachStart={() => {}} />);
expect(marker(container)).toBeNull();
act(() => (document.activeElement as HTMLElement | null)?.blur());
expect(document.activeElement).toBe(document.body);

rerender(<Marker stalled onReachStart={() => {}} />);
rerender(<Marker loading onReachStart={() => {}} />);
expect(document.activeElement).toBe(document.body);
});
});
10 changes: 7 additions & 3 deletions src/ui/conversation-earlier.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,14 @@ export const ConversationEarlier = forwardRef<

// A retry swaps the button for the loading text: hold focus on the marker
// rather than let it fall to the body, and hand it back to the button if
// the retry stalls again.
// the retry stalls again. A retry that ends the history takes the marker
// with it, leaving nothing to restore.
useLayoutEffect(() => {
const element = markerRef.current;
if (element === null) return;
if (element === null) {
retried.current = false;
return;
}
if (stalled) {
if (document.activeElement === element) {
element.querySelector("button")?.focus();
Expand All @@ -86,7 +90,7 @@ export const ConversationEarlier = forwardRef<
if (active === null || active === document.body) {
element.focus({ preventScroll: true });
}
}, [stalled]);
}, [stalled, hasEarlier]);

// Once the last page has landed the marker goes away; if it held focus,
// hand focus to the log so it doesn't fall to the body.
Expand Down
11 changes: 11 additions & 0 deletions src/ui/conversation-map.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,17 @@ describe("ConversationMap", () => {
mounted.unmount();
});

test("a focused tick whose rail goes with its turn kept hands focus to the log", () => {
const mounted = render(<Chat />);
act(() => tick(mounted.container, "Fourth question").focus());
mounted.rerender(<Chat turns={TURNS.slice(3)} items={["q4"]} />);
expect(
mounted.container.querySelector("[data-slot='conversation-map']"),
).toBeNull();
expect(document.activeElement).toBe(scroller(mounted.container));
mounted.unmount();
});

test("scrolling the rail measures the previewed tick in the handler, not in render", () => {
const mounted = render(
<StrictMode>
Expand Down
19 changes: 13 additions & 6 deletions src/ui/conversation-map.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -157,19 +157,21 @@ export const ConversationMap = forwardRef<HTMLElement, ConversationMapProps>(
}, [currentId, hidden, reducedMotion]);

// The focused tick's turn left `turns` (a history window trimmed it, the
// host deleted it): its button is gone, focus fell to the body, and React
// reports no blur for it. Hand focus to the rail's tab stop, or to the log
// once the rail itself is gone.
// host deleted it), or the rail hid with it: its button is gone, focus fell
// to the body, and React reports no blur for it. Hand focus to the rail's
// tab stop, or to the log once the rail itself is gone.
useLayoutEffect(() => {
if (focusedId === null || turnIds.includes(focusedId)) return;
if (focusedId === null || (!hidden && turnIds.includes(focusedId))) {
return;
}
setFocusedId(null);
const active = document.activeElement;
if (active !== null && active !== document.body) return;
const stop = [
...(railRef.current?.querySelectorAll<HTMLElement>(TICK) ?? []),
].find((tick) => tick.tabIndex === 0);
(stop ?? scrollContainer())?.focus({ preventScroll: true });
}, [focusedId, turnIds, scrollContainer]);
}, [focusedId, hidden, turnIds, scrollContainer]);

// Escape dismisses a preview wherever focus is: one the mouse opened
// covers the content while focus stays in the composer or the log.
Expand Down Expand Up @@ -260,7 +262,12 @@ export const ConversationMap = forwardRef<HTMLElement, ConversationMapProps>(
: index + (event.key === "ArrowDown" ? 1 : -1);
// Also at either end, so the keys never scroll the rail or the page.
event.preventDefault();
ticks[next]?.focus();
const rail = railRef.current;
const target = ticks[next];
if (rail === null || target === undefined) return;
// Focus alone would scroll every ancestor to reveal a clipped tick.
keepInView(rail, target, "auto");
target.focus({ preventScroll: true });
};

return (
Expand Down
20 changes: 20 additions & 0 deletions src/ui/conversation-thread.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,26 @@ describe("ConversationThread", () => {
mounted.unmount();
});

test("leaves out the input slot for boolean children", () => {
for (const show of [false, true]) {
const mounted = render(
<ConversationThread
parent={parent}
replies={replies}
renderMessage={renderMessage}
>
{show}
</ConversationThread>,
);
expect(
mounted.container.querySelector(
"[data-slot='conversation-thread-input']",
),
).toBeNull();
mounted.unmount();
}
});

test("gives the root its ref, props and classes, and the policy to the conversation", () => {
const ref = createRef<HTMLDivElement>();
const mounted = render(
Expand Down
4 changes: 3 additions & 1 deletion src/ui/conversation-thread.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,9 @@ export function ConversationThread({
</ConversationContent>
<ConversationScrollButton />
</Conversation>
{children === undefined || children === null ? null : (
{children === undefined ||
children === null ||
typeof children === "boolean" ? null : (
<div data-slot="conversation-thread-input" className="px-4 pb-4">
{children}
</div>
Expand Down
Loading