diff --git a/e2e/conversation-map.spec.ts b/e2e/conversation-map.spec.ts index 09c18e04..45c6154f 100644 --- a/e2e/conversation-map.spec.ts +++ b/e2e/conversation-map.spec.ts @@ -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, }) => { diff --git a/src/ui/conversation-earlier.test.tsx b/src/ui/conversation-earlier.test.tsx index 6a8ee333..9bf03395 100644 --- a/src/ui/conversation-earlier.test.tsx +++ b/src/ui/conversation-earlier.test.tsx @@ -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( + {}} />, + ); + 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( {}} />); + 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( {}} />); + rerender( {}} />); + 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( + {}} />, + ); + 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( {}} />); + expect(marker(container)).toBeNull(); + act(() => (document.activeElement as HTMLElement | null)?.blur()); + expect(document.activeElement).toBe(document.body); + + rerender( {}} />); + rerender( {}} />); + expect(document.activeElement).toBe(document.body); + }); }); diff --git a/src/ui/conversation-earlier.tsx b/src/ui/conversation-earlier.tsx index c2774bb5..b74a3dab 100644 --- a/src/ui/conversation-earlier.tsx +++ b/src/ui/conversation-earlier.tsx @@ -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(); @@ -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. diff --git a/src/ui/conversation-map.test.tsx b/src/ui/conversation-map.test.tsx index 4b2fcc6f..cc1ab070 100644 --- a/src/ui/conversation-map.test.tsx +++ b/src/ui/conversation-map.test.tsx @@ -510,6 +510,90 @@ describe("ConversationMap", () => { mounted.unmount(); }); + test("a preview the map hid with does not come back with a second turn", () => { + const add = document.addEventListener.bind(document); + const remove = document.removeEventListener.bind(document); + let keydowns = 0; + const watch = + (delta: number, call: typeof add) => + ( + type: string, + listener: EventListenerOrEventListenerObject, + options?: boolean | AddEventListenerOptions, + ) => { + if (type === "keydown") keydowns += delta; + call(type, listener, options); + }; + document.addEventListener = watch( + 1, + add, + ) as typeof document.addEventListener; + document.removeEventListener = watch( + -1, + remove, + ) as typeof document.removeEventListener; + try { + const mounted = render(); + const before = keydowns; + act(() => { + tick(mounted.container, "Second question").dispatchEvent( + new PointerEvent("pointerover", { + bubbles: true, + pointerType: "mouse", + }), + ); + }); + expect( + mounted.container.querySelector("[role='tooltip']"), + ).not.toBeNull(); + expect(keydowns).toBe(before + 1); + mounted.rerender( + , + ); + expect( + mounted.container.querySelector("[data-slot='conversation-map']"), + ).toBeNull(); + expect(keydowns).toBe(before); + act(() => { + document.dispatchEvent( + new KeyboardEvent("keydown", { key: "Escape", bubbles: true }), + ); + }); + mounted.rerender( + , + ); + expect( + mounted.container.querySelector("[data-slot='conversation-map']"), + ).not.toBeNull(); + expect(mounted.container.querySelector("[role='tooltip']")).toBeNull(); + expect(keydowns).toBe(before); + mounted.unmount(); + } finally { + document.addEventListener = add; + document.removeEventListener = remove; + } + }); + + test("a preview whose turn goes does not come back with it", () => { + const mounted = render(); + act(() => { + tick(mounted.container, "Second question").dispatchEvent( + new PointerEvent("pointerover", { + bubbles: true, + pointerType: "mouse", + }), + ); + }); + expect(mounted.container.querySelector("[role='tooltip']")).not.toBeNull(); + const rest = TURNS.filter((turn) => turn.id !== "q2"); + mounted.rerender( + !id.startsWith("q2"))} />, + ); + mounted.rerender(); + expect(mounted.container.querySelector("[role='tooltip']")).toBeNull(); + mounted.unmount(); + }); + test("a focused tick whose turn goes keeps focus on the rail", () => { const mounted = render(); act(() => tick(mounted.container, "First question").focus()); @@ -534,6 +618,17 @@ describe("ConversationMap", () => { mounted.unmount(); }); + test("a focused tick whose rail goes with its turn kept hands focus to the log", () => { + const mounted = render(); + act(() => tick(mounted.container, "Fourth question").focus()); + mounted.rerender(); + 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( diff --git a/src/ui/conversation-map.tsx b/src/ui/conversation-map.tsx index 258b3171..8d4292ff 100644 --- a/src/ui/conversation-map.tsx +++ b/src/ui/conversation-map.tsx @@ -157,11 +157,17 @@ export const ConversationMap = forwardRef( }, [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. Clear a preview + // whose tick left the same way before moving focus. useLayoutEffect(() => { - if (focusedId === null || turnIds.includes(focusedId)) return; + if (preview !== null && (hidden || !turnIds.includes(preview.id))) { + setPreview(null); + } + if (focusedId === null || (!hidden && turnIds.includes(focusedId))) { + return; + } setFocusedId(null); const active = document.activeElement; if (active !== null && active !== document.body) return; @@ -169,7 +175,7 @@ export const ConversationMap = forwardRef( ...(railRef.current?.querySelectorAll(TICK) ?? []), ].find((tick) => tick.tabIndex === 0); (stop ?? scrollContainer())?.focus({ preventScroll: true }); - }, [focusedId, turnIds, scrollContainer]); + }, [focusedId, hidden, preview, 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. @@ -260,7 +266,12 @@ export const ConversationMap = forwardRef( : 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 ( diff --git a/src/ui/conversation-thread.test.tsx b/src/ui/conversation-thread.test.tsx index 9bc44cd6..ee87ee19 100644 --- a/src/ui/conversation-thread.test.tsx +++ b/src/ui/conversation-thread.test.tsx @@ -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( + + {show} + , + ); + 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(); const mounted = render( diff --git a/src/ui/conversation-thread.tsx b/src/ui/conversation-thread.tsx index e09df17e..32ab9cb6 100644 --- a/src/ui/conversation-thread.tsx +++ b/src/ui/conversation-thread.tsx @@ -117,7 +117,9 @@ export function ConversationThread({ - {children === undefined || children === null ? null : ( + {children === undefined || + children === null || + typeof children === "boolean" ? null : (
{children}