From 99a2e6efa78f1d16f203752cad194047f75bd1b0 Mon Sep 17 00:00:00 2001 From: hkonsti <45428767+hkonsti@users.noreply.github.com> Date: Wed, 11 Nov 2026 06:52:42 +0000 Subject: [PATCH 1/2] move chat's list rows as they come, go or change places, and name a task opened from the conversation at once --- .../web/src/hooks/use-list-motion.test.tsx | 223 ++++++++++++++++++ packages/web/src/hooks/use-list-motion.ts | 189 +++++++++++++++ packages/web/src/index.css | 35 --- packages/web/src/pages/chat-page.test.tsx | 58 +++-- packages/web/src/pages/chat-page.tsx | 157 +++++------- 5 files changed, 509 insertions(+), 153 deletions(-) create mode 100644 packages/web/src/hooks/use-list-motion.test.tsx create mode 100644 packages/web/src/hooks/use-list-motion.ts diff --git a/packages/web/src/hooks/use-list-motion.test.tsx b/packages/web/src/hooks/use-list-motion.test.tsx new file mode 100644 index 00000000..128b2bec --- /dev/null +++ b/packages/web/src/hooks/use-list-motion.test.tsx @@ -0,0 +1,223 @@ +/// + +import {afterEach, beforeEach, describe, expect, spyOn, test} from 'bun:test'; +import {cleanup, render, waitFor} from '@testing-library/react'; +import {useListMotion} from '@/hooks/use-list-motion'; + +// Where a row is laid out: the test's dom lays nothing out, so each row is told. +type Place = {top: number; height: number}; + +function place({top, height}: Place) { + return (row: HTMLLIElement | null) => { + if (row === null) { + return; + } + Object.defineProperties(row, { + offsetTop: {value: top, configurable: true}, + offsetHeight: {value: height, configurable: true}, + }); + }; +} + +// A list of rows, each `[its key, where it is laid out]`, in the order drawn. +function Rows({rows}: {rows: Array<[string, Place]>}) { + const list = useListMotion(); + return ( + + ); +} + +// What was animated, by the text of the element: its keyframes as the hook gave them. +let moved: Array<{of: string; frames: Keyframe[]}> = []; +const animate = Element.prototype.animate; +let spy: ReturnType | null = null; + +beforeEach(() => { + moved = []; + spy = spyOn(Element.prototype, 'animate').mockImplementation(function ( + this: Element, + frames: Keyframe[] | PropertyIndexedKeyframes | null, + options?: number | KeyframeAnimationOptions, + ) { + moved.push({of: this.textContent ?? '', frames: Array.isArray(frames) ? frames : []}); + return animate.call(this, frames, options); + }); +}); + +afterEach(() => { + cleanup(); + spy?.mockRestore(); + Reflect.deleteProperty(globalThis, 'matchMedia'); +}); + +const agent = {top: 0, height: 56}; + +describe('useListMotion', () => { + test('moves nothing as the list first shows, or as it is drawn again with its rows in place', () => { + const rows: Array<[string, Place]> = [ + ['a', agent], + ['b', {top: 58, height: 56}], + ]; + const {rerender} = render(); + rerender(); + + expect(moved).toEqual([]); + }); + + test('uncovers a row that comes, from its top, as the rows after it slide down from where they were', () => { + const {rerender} = render( + , + ); + rerender( + , + ); + + expect(moved).toEqual([ + {of: 'b', frames: [{transform: 'translateY(-34px)'}, {transform: 'translateY(0)'}]}, + // The row comes down over its content, which moves up within it by as much. + {of: 'task', frames: [{transform: 'translateY(-34px)'}, {transform: 'translateY(0)'}]}, + { + of: 'task', + frames: [ + {transform: 'translateY(34px)', opacity: 0}, + {transform: 'translateY(0)', opacity: 1}, + ], + }, + ]); + }); + + test('keeps a row that goes where it was, out of reach, until the rows after it have slid up over it', async () => { + const {rerender, container} = render( + , + ); + const gone = container.querySelector('[data-row="task"]') as HTMLElement; + rerender( + , + ); + + expect(gone.parentElement).toBe(container.querySelector('ul')); + expect(gone.inert).toBe(true); + expect(gone.getAttribute('aria-hidden')).toBe('true'); + // No longer one of the list's rows, so drawing the list again leaves it be. + expect(gone.hasAttribute('data-row')).toBe(false); + expect(gone.style.position).toBe('absolute'); + expect(gone.style.top).toBe('56px'); + expect(moved).toEqual([ + {of: 'b', frames: [{transform: 'translateY(34px)'}, {transform: 'translateY(0)'}]}, + {of: 'task', frames: [{transform: 'translateY(0)'}, {transform: 'translateY(-34px)'}]}, + { + of: 'task', + frames: [ + {transform: 'translateY(0)', opacity: 1}, + {transform: 'translateY(34px)', opacity: 0}, + ], + }, + ]); + + await waitFor(() => expect(gone.isConnected).toBe(false)); + }); + + test('fades a row in over the one whose place it takes, the rows around them staying put', async () => { + const {rerender, container} = render( + , + ); + const gone = container.querySelector('[data-row="one"]') as HTMLElement; + rerender( + , + ); + + expect(moved).toEqual([ + {of: 'other', frames: [{opacity: 0}, {opacity: 1}]}, + {of: 'one', frames: [{opacity: 1}, {opacity: 0}]}, + ]); + await waitFor(() => expect(gone.isConnected).toBe(false)); + }); + + test('slides rows that change places to their new ones', () => { + const {rerender} = render( + , + ); + rerender( + , + ); + + expect(moved).toEqual([ + {of: 'b', frames: [{transform: 'translateY(58px)'}, {transform: 'translateY(0)'}]}, + {of: 'a', frames: [{transform: 'translateY(-58px)'}, {transform: 'translateY(0)'}]}, + ]); + }); + + test('moves nothing, and keeps no row that went, where the person asked for less motion', () => { + Object.assign(globalThis, {matchMedia: () => ({matches: true})}); + const {rerender, container} = render( + , + ); + rerender( + , + ); + + expect(moved).toEqual([]); + expect(container.querySelectorAll('li')).toHaveLength(2); + }); +}); diff --git a/packages/web/src/hooks/use-list-motion.ts b/packages/web/src/hooks/use-list-motion.ts new file mode 100644 index 00000000..85f08a67 --- /dev/null +++ b/packages/web/src/hooks/use-list-motion.ts @@ -0,0 +1,189 @@ +import {type RefObject, useLayoutEffect, useRef} from 'react'; + +// How long a row takes to move, come or go, and how: fast at first, coming gently to rest. +const MOTION: KeyframeAnimationOptions = {duration: 200, easing: 'cubic-bezier(0.2, 0, 0, 1)'}; + +// A row as it was last laid out, within the list, wherever the list is scrolled to. +type Placed = {element: HTMLElement; top: number; left: number; width: number; height: number}; + +// The list as it was last laid out: its rows by their keys, and the keys in the order drawn. +type Layout = {order: string; rows: Map}; + +// What a row is in the middle of: sliding to a new place, or being uncovered as it comes. +const slides = new WeakMap(); +const wipes = new WeakMap(); + +// Whether the person has not asked for less motion. +function mayMove(): boolean { + return !globalThis.matchMedia?.('(prefers-reduced-motion: reduce)').matches; +} + +function keyOf(row: HTMLElement): string { + return row.dataset.row ?? ''; +} + +// How far from where it is laid out the row is drawn, by a slide under way. +function slideOffset(list: HTMLElement, row: HTMLElement): number { + if (slides.get(row)?.playState !== 'running') { + return 0; + } + const laidOut = + list.getBoundingClientRect().top + list.clientTop - list.scrollTop + row.offsetTop; + return row.getBoundingClientRect().top - laidOut; +} + +// Whether a row came or went where another was or is: the two change places without moving. +function sharesPlace(place: Placed, others: Placed[]): boolean { + return others.some((other) => Math.abs(other.top - place.top) < 1); +} + +// Slides the row from where it was drawn, `by` pixels off, to where it is laid out now. +function slide(row: HTMLElement, by: number): void { + slides.get(row)?.cancel(); + if (Math.abs(by) < 0.5) { + return; + } + // One still being uncovered is there in full from here on. + for (const wipe of wipes.get(row) ?? []) { + wipe.cancel(); + } + slides.set( + row, + row.animate([{transform: `translateY(${by}px)`}, {transform: 'translateY(0)'}], MOTION), + ); +} + +// Uncovers a row from its top, at the pace of the rows below it moving down: the row is a window +// that comes down over its content, which stays where it is by moving the other way within it, +// and fades in. Backwards, the row is covered from its bottom, and stays so. +function wipe(row: HTMLElement, content: Element, backwards: boolean): [Animation, Animation] { + const height = row.offsetHeight; + const window: Keyframe[] = [ + {transform: `translateY(${-height}px)`}, + {transform: 'translateY(0)'}, + ]; + const inside: Keyframe[] = [ + {transform: `translateY(${height}px)`, opacity: 0}, + {transform: 'translateY(0)', opacity: 1}, + ]; + if (!backwards) { + return [row.animate(window, MOTION), content.animate(inside, MOTION)]; + } + const kept: KeyframeAnimationOptions = {...MOTION, fill: 'forwards'}; + return [row.animate(window.toReversed(), kept), content.animate(inside.toReversed(), kept)]; +} + +// A row that came: uncovered where the rows after it make room, or, in the place of one that +// went, faded in over it. +function enter(row: HTMLElement, fade: boolean): void { + const content = row.firstElementChild; + if (fade || content === null) { + row.animate([{opacity: 0}, {opacity: 1}], MOTION); + return; + } + // The row clips its content only while it is a window over it. + row.style.overflow = 'hidden'; + const uncovering = wipe(row, content, false); + const settle = () => { + row.style.overflow = ''; + }; + uncovering[0].onfinish = settle; + uncovering[0].oncancel = settle; + wipes.set(row, uncovering); +} + +// A row that went: kept where it was, over the list, out of reach, until it is covered, or, where +// another came in its place, has faded out. +function leave(list: HTMLElement, place: Placed, fade: boolean): void { + const ghost = place.element; + const content = ghost.firstElementChild; + ghost.removeAttribute('data-row'); + ghost.setAttribute('aria-hidden', 'true'); + ghost.inert = true; + Object.assign(ghost.style, { + position: 'absolute', + top: `${place.top}px`, + left: `${place.left}px`, + width: `${place.width}px`, + height: `${place.height}px`, + margin: '0', + overflow: 'hidden', + }); + list.append(ghost); + const going = + fade || content === null + ? ghost.animate([{opacity: 1}, {opacity: 0}], {...MOTION, fill: 'forwards'}) + : wipe(ghost, content, true)[0]; + going.onfinish = () => ghost.remove(); + going.oncancel = () => ghost.remove(); +} + +/** + * Moves the rows of a list rather than letting them jump. The ref is the list's, which scrolls + * and is what its rows are placed against (`relative`); every row in it says which it is + * (`data-row`, a key no other row of the list has), holds one element, its content, and is in no + * other row. As the list is drawn again with a row more, the row is uncovered while those after it + * slide down to make room; with a row fewer, the row is kept where it was, out of reach, and + * covered while those after it slide up; rows that change places slide to their new ones; and a + * row that takes another's place fades in over it. Nothing moves as the list first shows, or + * where the person asked for less motion. + * + * The rows are laid out where they end up at once, and only drawn elsewhere on the way there + * (transform and opacity), so the browser moves them without the page's thread: a page busy + * drawing what was just opened doesn't hold them up, as it would a row growing in height. + */ +export function useListMotion(): RefObject { + const ref = useRef(null); + const last = useRef(null); + + // After every draw: rows only come, go or change places as the keys drawn do. + useLayoutEffect(() => { + const list = ref.current; + if (list === null) { + return; + } + + const rows = [...list.querySelectorAll('[data-row]')]; + const order = rows.map(keyOf).join('\n'); + const before = last.current; + if (before?.order === order) { + return; + } + + const placed = new Map( + rows.map((row): [string, Placed] => [ + keyOf(row), + { + element: row, + top: row.offsetTop, + left: row.offsetLeft, + width: row.offsetWidth, + height: row.offsetHeight, + }, + ]), + ); + last.current = {order, rows: placed}; + if (before === null || !mayMove()) { + return; + } + + const came = [...placed].filter(([key]) => !before.rows.has(key)).map(([, place]) => place); + const went = [...before.rows].filter(([key]) => !placed.has(key)).map(([, place]) => place); + // Read before anything is moved, so no row is laid out again for the next. + const moves = [...placed].flatMap(([key, {element, top}]) => { + const was = before.rows.get(key); + return was ? [{row: element, by: was.top + slideOffset(list, element) - top}] : []; + }); + for (const {row, by} of moves) { + slide(row, by); + } + for (const place of came) { + enter(place.element, sharesPlace(place, went)); + } + for (const place of went) { + leave(list, place, sharesPlace(place, came)); + } + }); + + return ref; +} diff --git a/packages/web/src/index.css b/packages/web/src/index.css index 312a1ddd..eeef336f 100644 --- a/packages/web/src/index.css +++ b/packages/web/src/index.css @@ -227,38 +227,3 @@ [data-slot='message-scroller-viewport'][data-scrollable-end='false'] { --scroll-fade-b: 0px; } - -/* A task coming into Chat's list under its agent opens its room there, the rows below it moving - down, rather than appearing all at once; one going closes it again, the rows below moving up. - The one going stays closed until it is taken out, at the animation's end. */ -.task-row-enter { - animation: task-row-enter 150ms ease-out; -} - -.task-row-leave { - animation: task-row-enter 150ms ease-out reverse forwards; -} - -@keyframes task-row-enter { - from { - grid-template-rows: 0fr; - opacity: 0; - } - - to { - grid-template-rows: 1fr; - opacity: 1; - } -} - -/* Without motion, one coming in is there at once, and one going is gone at once: its animation, - which takes it out as it ends, ends as it starts. */ -@media (prefers-reduced-motion: reduce) { - .task-row-enter { - animation: none; - } - - .task-row-leave { - animation-duration: 0s; - } -} diff --git a/packages/web/src/pages/chat-page.test.tsx b/packages/web/src/pages/chat-page.test.tsx index af506ca4..3b51f5b6 100644 --- a/packages/web/src/pages/chat-page.test.tsx +++ b/packages/web/src/pages/chat-page.test.tsx @@ -113,6 +113,7 @@ function renderChatPage({ open = 'general', draft = null, answered = () => Promise.resolve(), + taskAnswered = () => Promise.resolve(), initial, }: { personas?: ChatPersona[]; @@ -124,6 +125,8 @@ function renderChatPage({ draft?: string | null; // When the api answers for a conversation: at once, unless a test holds it back. answered?: (personaId: string) => Promise; + // When the api answers for a task's chat: at once, unless a test holds it back. + taskAnswered?: () => Promise; // The address the page opens on, where it is not the open conversation's. initial?: string; } = {}) { @@ -154,12 +157,10 @@ function renderChatPage({ }, { path: '/:org/chat/:id/tasks/:taskId', - loader: later(async () => ({ - task: flights, - personas: listPersonas(), - pinnedTasks, - harnesses, - })), + loader: later(async () => { + await taskAnswered(); + return {task: flights, personas: listPersonas(), pinnedTasks, harnesses}; + }), element: , }, {path: '/:org/activity/tasks/:taskId', element:

The task’s activity

}, @@ -708,21 +709,42 @@ describe('ChatPage', () => { ); }); - test('slides the open task in under its agent, and out again as it closes', async () => { - renderChatPage({initial: '/acme/chat/general/tasks/sub-1'}); + test('names a task opened from the conversation at once, in the header and under its agent, while its chat loads, and takes its row away as it closes', async () => { + let arrive = () => undefined; + const held = new Promise((resolve) => { + arrive = () => { + resolve(); + return undefined; + }; + }); + renderChatPage({ + entries: [ + { + id: 'e2:c1', + kind: 'task', + task: {id: 'sub-1', title: 'Find flights'}, + at: '2026-10-21T10:00:00Z', + }, + ], + taskAnswered: () => held, + }); - const general = await screen.findByRole('list', {name: 'General’s tasks'}); - const opened = (await within(general).findByText('Find flights')).closest('li') as HTMLElement; - expect(opened.className).toContain('task-row-enter'); + fireEvent.click(await screen.findByRole('link', {name: 'Find flights'})); + await screen.findByRole('status', {name: 'Loading'}); + const tasks = screen.getByRole('list', {name: 'General’s tasks'}); + const row = within(tasks).getByRole('link', {name: 'Find flights'}); + expect(row.getAttribute('aria-current')).toBe('page'); + expect(screen.getByRole('link', {name: /^General/}).getAttribute('aria-current')).toBeNull(); + expect(within(screen.getByRole('banner')).getByText('Find flights')).toBeTruthy(); - // Back in General's conversation, the task's row stays, out of reach, until it has slid out. - fireEvent.click(screen.getByRole('link', {name: /^General/})); - await waitFor(() => expect(opened.className).toContain('task-row-leave')); - expect(opened.isConnected).toBe(true); - expect(opened.inert).toBe(true); + await act(async () => arrive()); + await screen.findByLabelText('Task'); + expect(within(tasks).getByRole('link', {name: 'Find flights'})).toBe(row); - fireEvent.animationEnd(opened); - await waitFor(() => expect(screen.queryByRole('list', {name: 'General’s tasks'})).toBeNull()); + fireEvent.click(screen.getByRole('link', {name: /^General/})); + await waitFor(() => expect(tasks.isConnected).toBe(false)); + expect(screen.queryByRole('list', {name: 'General’s tasks'})).toBeNull(); + expect(screen.getByRole('link', {name: /^General/}).getAttribute('aria-current')).toBe('page'); }); test('marks nothing read before the agent has answered', async () => { diff --git a/packages/web/src/pages/chat-page.tsx b/packages/web/src/pages/chat-page.tsx index 1cf9e9a6..fe1f0b42 100644 --- a/packages/web/src/pages/chat-page.tsx +++ b/packages/web/src/pages/chat-page.tsx @@ -52,6 +52,7 @@ import {preview} from '@proxy/utils/harness'; import {cn} from '@proxy/ui/lib/utils'; import {useChatDraft} from '@/hooks/use-chat-draft'; import {useLastLoaded, useLoaded} from '@/hooks/use-loaded'; +import {useListMotion} from '@/hooks/use-list-motion'; import {useOrgSlug} from '@/hooks/use-org'; import {useReload} from '@/hooks/use-reload'; import {useReloadWhileHidden} from '@/hooks/use-reload-on-release'; @@ -137,11 +138,12 @@ function lastActive(persona: ChatPersona, last: ChatPersona['lastMessage']): num // A conversation as the page shows it, fresher than the list: its log, and what is being typed. type Shown = {personaId: string; log: ThreadLog; draft: string}; -// A task open in its agent's place, under that agent in the list. -type OpenedTask = {personaId: string; task: ChatTask}; - -// How a task's row in the list moves: in as it opens, out as it closes, or not at all. -type TaskMotion = 'enter' | 'leave' | null; +// The tasks a conversation names: those the persona started in it, and those it wrote to. +function tasksOf(log: ThreadLog): ChatTask[] { + return log.entries.flatMap((entry) => + entry.kind === 'task' || entry.kind === 'sent' ? [entry.task] : [], + ); +} // The personas the person sees, their own and General, in the second sidebar, each with its head, its name and the last // thing said between it and the person, marked where the person hasn't seen it, or what they @@ -151,7 +153,8 @@ type TaskMotion = 'enter' | 'leave' | null; // is from one conversation to the next: the one in the address is lit at once, while it loads. // Under each, the tasks the person pinned in its conversation, the one with something said in it // last first, then, under the open one, the task open in its place where it isn't pinned, lit in -// the agent's stead. +// the agent's stead. A row that comes, goes or changes places moves there (`useListMotion`): a +// task opened or closed, pinned or unpinned, an agent made, deleted or moved up by what was said. function PersonaList({ personas, current, @@ -179,27 +182,7 @@ function PersonaList({ const org = useOrgSlug(); const unread = useUnread(); const [hovered, setHovered] = useState(null); - // The task open in its agent's place where it isn't pinned, shown under the agent while it is. - const opened: OpenedTask | null = - task !== null && !pinned.some((pin) => pin.id === task.id) ? {personaId: current, task} : null; - // The ones open before it, kept a moment as they slide out, so the rows below them move up - // rather than jump. One the person pins stays where it is; one opened again is no longer leaving. - const [leaving, setLeaving] = useState([]); - const [previous, setPrevious] = useState(opened); - if (previous?.task.id !== opened?.task.id) { - setPrevious(opened); - setLeaving((rows) => { - const rest = rows.filter( - (row) => row.task.id !== opened?.task.id && row.task.id !== previous?.task.id, - ); - if (previous === null || pinned.some((pin) => pin.id === previous.task.id)) { - return rest; - } - return [...rest, previous]; - }); - } - const left = (taskId: string) => - setLeaving((rows) => rows.filter((row) => row.task.id !== taskId)); + const list = useListMotion(); // Whether the persona said something the person hasn't seen, as the provider has it. const isUnread = (persona: ChatPersona) => persona.id !== current && unread.unreadIds.has(persona.id); @@ -214,7 +197,7 @@ function PersonaList({ }) .toSorted((a, b) => lastActive(b.persona, b.last) - lastActive(a.persona, a.last)); return ( -
+
{personas.length === 0 && ( No agents yet. )} @@ -225,27 +208,11 @@ function PersonaList({ // The row is lit while its conversation is open, and the task's under it in its place. const lit = open && task === null; const pinnedHere = pinned.filter((candidate) => candidate.personaId === persona.id); - const openTask = opened?.personaId === persona.id ? [opened.task] : []; - // Above the open one, so one closing as another opens gives way to it in place. - const leavingHere = leaving - .filter( - (row) => - row.personaId === persona.id && !pinnedHere.some((pin) => pin.id === row.task.id), - ) - .map((row) => row.task); - // The open task slides in under its agent and out again as it closes; the pinned stay. - const motionOf = (candidate: ChatTask): TaskMotion => { - if (openTask.includes(candidate)) { - return 'enter'; - } - - if (leavingHere.includes(candidate)) { - return 'leave'; - } - - return null; - }; - const tasks = [...pinnedHere, ...leavingHere, ...openTask].map((candidate) => ( + const openTask = + open && task !== null && !pinnedHere.some((candidate) => candidate.id === task.id) + ? [task] + : []; + const tasks = [...pinnedHere, ...openTask].map((candidate) => ( pin.id === candidate.id)} onPin={(pin) => onPin(persona.id, candidate.id, pin)} - motion={motionOf(candidate)} - onLeft={() => left(candidate.id)} /> )); const row = ( @@ -329,23 +294,25 @@ function PersonaList({ onPointerEnter={() => setHovered(persona.id)} onPointerLeave={() => setHovered(null)} > - {persona.mayChange ? ( - - }>{row} - - onEdit(persona)}> - - Edit - - onDelete(persona)}> - - Delete - - - - ) : ( - row - )} +
+ {persona.mayChange ? ( + + }>{row} + + onEdit(persona)}> + + Edit + + onDelete(persona)}> + + Delete + + + + ) : ( + row + )} +
{tasks.length > 0 && (
    {tasks} @@ -360,48 +327,25 @@ function PersonaList({ } // A task in the list, under its agent: its title, opening its chat, lit while it is open, marked -// where it is pinned, and pinned or unpinned from its right-click menu. One that enters slides in, -// its room, gap above it included, opening as it fades in; one that leaves slides out the same -// way, out of reach meanwhile, and says so (`onLeft`) once it is gone. +// where it is pinned, and pinned or unpinned from its right-click menu. The gap above it is the +// row's own, so the room it takes in the list, and gives back, is all its own. function TaskRow({ task, to, lit, pinned, onPin, - motion, - onLeft, }: { task: ChatTask; to: string; lit: boolean; pinned: boolean; onPin: (pinned: boolean) => void; - motion: TaskMotion; - onLeft: () => void; }) { - const leave = motion === 'leave'; return ( - { - if (!leave || event.target !== event.currentTarget) { - return; - } - onLeft(); - }} - /> - } - > -
    + }> +
    known.id === taskId, + ) ?? + null; + // The conversation as it was last shown is fresher than the list while the list is the one + // loaded with a conversation; one loaded with a task since is fresher than it. + const fresher = answer !== null && 'log' in answer ? shown : null; // Pins a task under its agent, or unpins it, and reads the list again with it so. const pinOne = async (personaId: string, pinnedTaskId: string, pin: boolean) => { @@ -517,7 +474,7 @@ export function ChatPage() { const title = () => { if (taskId !== undefined) { - return task === null ? : {task.task.title}; + return named === null ? : {named.title}; } if (name !== undefined) { return {name}; @@ -534,10 +491,10 @@ export function ChatPage() { void pinOne(personaId, pinnedTaskId, pin)} - shown={taskId === undefined ? shown : null} + shown={fresher} recallDraft={recallDraft} onOpen={setFocusRequestedFor} onEdit={setEditing} From 77f4a2983e40f5f0483952746b3046bd4d3245ed Mon Sep 17 00:00:00 2001 From: hkonsti <45428767+hkonsti@users.noreply.github.com> Date: Wed, 11 Nov 2026 07:14:05 +0000 Subject: [PATCH 2/2] follow chat's links in an awaited act in its tests, so a render the click's own act drops can't leave the page unshown --- packages/web/src/pages/chat-page.test.tsx | 31 ++++++++++++++--------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/packages/web/src/pages/chat-page.test.tsx b/packages/web/src/pages/chat-page.test.tsx index 3b51f5b6..737a2803 100644 --- a/packages/web/src/pages/chat-page.test.tsx +++ b/packages/web/src/pages/chat-page.test.tsx @@ -184,6 +184,15 @@ function rowHeads(container: HTMLElement): Element[] { return [...container.querySelectorAll('a [data-slot="live-head"]')]; } +// Follows a link of the page, and waits for React to have drawn where it leads. A click's own +// `act` ends at once: a render under it that reads the page's answer with `use` and isn't done by +// then is dropped, and React, holding on to it as still to come, never draws that page. +async function follow(link: HTMLElement, init?: MouseEventInit): Promise { + await act(async () => { + fireEvent.click(link, init); + }); +} + describe('ChatPage', () => { test('focuses the message input after a conversation is picked from the list', async () => { let arrive = () => undefined; @@ -198,7 +207,7 @@ describe('ChatPage', () => { const initialBox = await screen.findByLabelText('Message'); expect(document.activeElement).not.toBe(initialBox); - fireEvent.click(screen.getByRole('link', {name: /Finance/})); + await follow(screen.getByRole('link', {name: /Finance/})); await screen.findByRole('status', {name: 'Loading'}); await act(async () => arrive()); @@ -214,9 +223,7 @@ describe('ChatPage', () => { const box = await screen.findByLabelText('Message'); expect(document.activeElement).not.toBe(box); - await act(async () => { - fireEvent.click(screen.getByRole('link', {name: /General/})); - }); + await follow(screen.getByRole('link', {name: /General/})); await waitFor(() => { expect(document.activeElement).toBe(box); @@ -300,7 +307,7 @@ describe('ChatPage', () => { fireEvent.pointerEnter(secondRow); expect(secondHead.getAttribute('data-state')).toBe('watching'); fireEvent.pointerUp(secondRow, {pointerType: 'mouse', button: 0}); - fireEvent.click(secondRow); + await follow(secondRow); await waitFor(() => { const openedHead = rowHeads(container)[1]; @@ -313,7 +320,7 @@ describe('ChatPage', () => { const {container} = renderChatPage(); const financeRow = await screen.findByRole('link', {name: /Finance/}); - fireEvent.click(financeRow, {detail: 0}); + await follow(financeRow, {detail: 0}); await waitFor(() => { const openedHead = rowHeads(container)[1]; @@ -329,7 +336,7 @@ describe('ChatPage', () => { fireEvent.pointerEnter(financeRow); fireEvent.pointerUp(financeRow, {pointerType: 'mouse', button: 0}); fireEvent.pointerLeave(financeRow); - fireEvent.click(financeRow); + await follow(financeRow); await waitFor(() => { const openedHead = rowHeads(container)[1]; @@ -349,7 +356,7 @@ describe('ChatPage', () => { renderChatPage({answered: (personaId) => (personaId === 'finance' ? held : Promise.resolve())}); await screen.findByText('What should General do?'); - fireEvent.click(screen.getByRole('link', {name: /Finance/})); + await follow(screen.getByRole('link', {name: /Finance/})); await screen.findByRole('status', {name: 'Loading'}); expect(screen.getByRole('link', {name: /Finance/}).getAttribute('aria-current')).toBe('page'); expect(screen.getByRole('link', {name: /General/}).getAttribute('aria-current')).toBeNull(); @@ -621,7 +628,7 @@ describe('ChatPage', () => { expect(created?.getAttribute('href')).toBe('/acme/chat/general/tasks/sub-1'); expect(sent?.getAttribute('href')).toBe('/acme/chat/general/tasks/sub-1'); - fireEvent.click(sent as HTMLElement); + await follow(sent as HTMLElement); const chat = await screen.findByLabelText('Task'); expect(within(chat).getByText('Three direct flights.')).toBeTruthy(); }); @@ -729,7 +736,7 @@ describe('ChatPage', () => { taskAnswered: () => held, }); - fireEvent.click(await screen.findByRole('link', {name: 'Find flights'})); + await follow(await screen.findByRole('link', {name: 'Find flights'})); await screen.findByRole('status', {name: 'Loading'}); const tasks = screen.getByRole('list', {name: 'General’s tasks'}); const row = within(tasks).getByRole('link', {name: 'Find flights'}); @@ -741,7 +748,7 @@ describe('ChatPage', () => { await screen.findByLabelText('Task'); expect(within(tasks).getByRole('link', {name: 'Find flights'})).toBe(row); - fireEvent.click(screen.getByRole('link', {name: /^General/})); + await follow(screen.getByRole('link', {name: /^General/})); await waitFor(() => expect(tasks.isConnected).toBe(false)); expect(screen.queryByRole('list', {name: 'General’s tasks'})).toBeNull(); expect(screen.getByRole('link', {name: /^General/}).getAttribute('aria-current')).toBe('page'); @@ -794,7 +801,7 @@ describe('ChatPage', () => { const box = await screen.findByLabelText('Message'); fireEvent.change(box, {target: {value: 'Move the pricing review'}}); - fireEvent.click(screen.getByRole('link', {name: /Finance/})); + await follow(screen.getByRole('link', {name: /Finance/})); await screen.findByText('What should Finance do?'); expect(saves()).toEqual([{text: 'Move the pricing review'}]);