From 28a2b628bc8718ff571baa738085e3fed408af4e Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 16 Sep 2026 21:30:14 -0700 Subject: [PATCH] fix(native-chat): open the message rail panel on the current message (#21143) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(native-chat): open the message rail panel on the current message The rail's hover panel mounts fresh at scrollTop 0 every time it opens, so in a long thread it showed the top of the conversation instead of where the reader actually is. It already knew which row was current — activeId drives the highlight — it just never scrolled to it. Attach a ref to the current row that calls scrollIntoView({ block: 'nearest' }). Radix unmounts popover content on close, so ref attachment is the open edge; it also re-fires when a different row goes active under an open panel. * fix(native-chat): keep current rail item focused * fix(native-chat): resync rail after list changes * fix(native-chat): own focus across retained rail opens --- .../NativeChatMessageRail.test.tsx | 261 +++++++++++++++++- .../native-chat/NativeChatMessageRail.tsx | 113 +++++--- 2 files changed, 328 insertions(+), 46 deletions(-) diff --git a/src/renderer/src/components/native-chat/NativeChatMessageRail.test.tsx b/src/renderer/src/components/native-chat/NativeChatMessageRail.test.tsx index 37eb4d76e04..729ccb3d0ae 100644 --- a/src/renderer/src/components/native-chat/NativeChatMessageRail.test.tsx +++ b/src/renderer/src/components/native-chat/NativeChatMessageRail.test.tsx @@ -1,7 +1,7 @@ // @vitest-environment happy-dom import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react' import userEvent from '@testing-library/user-event' -import { afterEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { NativeChatMessageRail } from './NativeChatMessageRail' afterEach(cleanup) @@ -12,6 +12,31 @@ const items = Array.from({ length: 3 }, (_, index) => ({ slotIndex: index, hasImages: false })) +const overflowItems = Array.from({ length: 20 }, (_, index) => ({ + id: `overflow-prompt-${index}`, + text: `Overflow prompt ${index}`, + slotIndex: index, + hasImages: false +})) + +function retainClosingPopover(): ReturnType { + const getStyle = window.getComputedStyle.bind(window) + return vi.spyOn(window, 'getComputedStyle').mockImplementation((element, ...args) => { + const style = getStyle(element, ...args) + if (element.getAttribute('data-slot') !== 'popover-content') { + return style + } + return new Proxy(style, { + get: (target, property) => + property === 'animationName' + ? element.getAttribute('data-state') === 'closed' + ? 'exit' + : 'enter' + : // oxlint-disable-next-line anti-slop/no-reflect-get -- Proxy trap passes CSSStyleDeclaration properties through unchanged. + Reflect.get(target, property) + }) + }) +} describe('message rail interaction', () => { it('opens from the keyboard, reaches prompts, jumps, and restores focus', async () => { @@ -19,7 +44,12 @@ describe('message rail interaction', () => { const select = vi.fn() render( @@ -29,12 +59,12 @@ describe('message rail interaction', () => { expect(document.activeElement).toBe(trigger) await user.keyboard('{Enter}') await waitFor(() => - expect(document.activeElement).toBe(screen.getByRole('button', { name: 'Prompt 0' })) + expect(document.activeElement).toBe( + screen.getByRole('button', { name: 'Overflow prompt 12' }) + ) ) - await user.tab() - expect(document.activeElement).toBe(screen.getByRole('button', { name: 'Prompt 1' })) await user.keyboard('{Enter}') - expect(select).toHaveBeenCalledWith(items[1]) + expect(select).toHaveBeenCalledWith(overflowItems[12]) await waitFor(() => expect(document.activeElement).toBe(trigger)) expect(screen.queryByRole('dialog')).toBeNull() await user.keyboard('{Enter}') @@ -43,6 +73,117 @@ describe('message rail interaction', () => { expect(screen.queryByRole('dialog')).toBeNull() }) + it('focuses the current prompt when a hover preview becomes interactive', async () => { + render( + + ) + const trigger = screen.getByRole('button', { name: 'Your messages' }) + fireEvent.pointerEnter(trigger, { pointerType: 'mouse' }) + await screen.findByRole('dialog') + fireEvent.click(trigger) + expect(document.activeElement).toBe(screen.getByRole('button', { name: 'Overflow prompt 12' })) + }) + + it('focuses the first prompt on direct open when no prompt is current', async () => { + const user = userEvent.setup() + render( + + ) + const trigger = screen.getByRole('button', { name: 'Your messages' }) + trigger.focus() + await user.keyboard('{Enter}') + await waitFor(() => + expect(document.activeElement).toBe(screen.getByRole('button', { name: 'Overflow prompt 0' })) + ) + }) + + it('refocuses the current prompt when closed content is reopened before unmount', async () => { + const styleSpy = retainClosingPopover() + const user = userEvent.setup() + try { + render( + + ) + const trigger = screen.getByRole('button', { name: 'Your messages' }) + trigger.focus() + await user.keyboard('{Enter}') + await user.keyboard('{Escape}') + await waitFor(() => + expect( + document.querySelector('[data-slot="popover-content"]')?.getAttribute('data-state') + ).toBe('closed') + ) + + trigger.focus() + fireEvent.click(trigger) + await waitFor(() => + expect(document.activeElement).toBe( + screen.getByRole('button', { name: 'Overflow prompt 12' }) + ) + ) + } finally { + styleSpy.mockRestore() + } + }) + + it('preserves interactive focus when the current prompt changes', async () => { + const user = userEvent.setup() + const { rerender } = render( + + ) + const trigger = screen.getByRole('button', { name: 'Your messages' }) + trigger.focus() + await user.keyboard('{Enter}') + const focusedPrompt = screen.getByRole('button', { name: 'Overflow prompt 12' }) + expect(document.activeElement).toBe(focusedPrompt) + + rerender( + + ) + + expect(document.activeElement).toBe(focusedPrompt) + }) + it('keeps focus in the transcript while a hover preview opens and closes', async () => { render( <> @@ -82,4 +223,112 @@ describe('message rail interaction', () => { fireEvent.wheel(screen.getByRole('button', { name: 'Your messages' }), { deltaY: 7, deltaMode }) expect(element.scrollTop).toBe(expected) }) + + // happy-dom has no layout, so these pin which row the panel scrolls to, not + // the resulting offset. The offset itself only exists in a real browser. + describe('opening position', () => { + const scrolled: Element[] = [] + let scrollIntoView: ReturnType + + beforeEach(() => { + scrolled.length = 0 + scrollIntoView = vi + .spyOn(Element.prototype, 'scrollIntoView') + .mockImplementation(function mockScrollIntoView(this: Element) { + scrolled.push(this) + }) + }) + afterEach(() => scrollIntoView.mockRestore()) + + it('scrolls the panel to the message the reader is on', async () => { + render( + + ) + fireEvent.pointerEnter(screen.getByRole('button', { name: 'Your messages' }), { + pointerType: 'mouse' + }) + await screen.findByRole('dialog') + expect(scrolled).toEqual([screen.getByRole('button', { name: 'Prompt 2' })]) + expect(scrollIntoView).toHaveBeenCalledWith({ block: 'nearest' }) + }) + + it('rechecks the current row when messages are inserted before it', async () => { + const { rerender } = render( + + ) + fireEvent.pointerEnter(screen.getByRole('button', { name: 'Your messages' }), { + pointerType: 'mouse' + }) + await screen.findByRole('dialog') + scrolled.length = 0 + + const shiftedItems = [ + { id: 'older-prompt', text: 'Older prompt', slotIndex: 0, hasImages: false }, + ...items.map((item) => ({ ...item, slotIndex: item.slotIndex + 1 })) + ] + rerender( + + ) + + expect(scrolled).toEqual([screen.getByRole('button', { name: 'Prompt 2' })]) + }) + + it('rechecks the current row when the same number of messages is reordered', async () => { + const { rerender } = render( + + ) + fireEvent.pointerEnter(screen.getByRole('button', { name: 'Your messages' }), { + pointerType: 'mouse' + }) + await screen.findByRole('dialog') + scrolled.length = 0 + + const reorderedItems = [items[2], items[0], items[1]] + rerender( + + ) + + expect(scrolled).toEqual([screen.getByRole('button', { name: 'Prompt 2' })]) + }) + + it('leaves the panel alone when no message is lit', async () => { + render( + + ) + fireEvent.pointerEnter(screen.getByRole('button', { name: 'Your messages' }), { + pointerType: 'mouse' + }) + await screen.findByRole('dialog') + expect(scrolled).toEqual([]) + }) + }) }) diff --git a/src/renderer/src/components/native-chat/NativeChatMessageRail.tsx b/src/renderer/src/components/native-chat/NativeChatMessageRail.tsx index ae8f5446c9f..ca459f33ad7 100644 --- a/src/renderer/src/components/native-chat/NativeChatMessageRail.tsx +++ b/src/renderer/src/components/native-chat/NativeChatMessageRail.tsx @@ -1,7 +1,7 @@ // The rail itself: a column of ticks down the right edge of the transcript, one // per user message, with a hover panel that previews them and jumps on click. -import { memo, useEffect, useRef, useState } from 'react' +import { memo, useEffect, useLayoutEffect, useRef, useState } from 'react' import { Popover, PopoverContent, PopoverTrigger } from '@/components/ui/popover' import { cn } from '@/lib/utils' import { translate } from '@/i18n/i18n' @@ -22,6 +22,65 @@ function railItemLabel(item: NativeChatRailItem): string { : translate('components.native-chat.railEmptyMessage', 'Message') } +type NativeChatMessageRailMode = 'hover' | 'interactive' | null + +function NativeChatMessageRailItems({ + mode, + items, + activeId, + onSelect +}: { + mode: NativeChatMessageRailMode + items: readonly NativeChatRailItem[] + activeId: string | null + onSelect: (item: NativeChatRailItem) => void +}): React.JSX.Element { + const listRef = useRef(null) + const currentItemRef = useRef(null) + const previousMode = useRef(null) + + useLayoutEffect(() => { + const currentItem = activeId === null || items.length === 0 ? null : currentItemRef.current + if (mode !== null) { + currentItem?.scrollIntoView({ block: 'nearest' }) + } + if (mode === 'interactive' && previousMode.current !== 'interactive') { + const focusTarget = currentItem ?? listRef.current?.querySelector('button') + focusTarget?.focus({ preventScroll: true }) + } + previousMode.current = mode + }, [activeId, items, mode]) + + return ( +
    + {items.map((item) => ( +
  • + +
    • + ))} +
+ ) +} + export const NativeChatMessageRail = memo(function NativeChatMessageRail({ rail, scrollRef, @@ -32,10 +91,10 @@ export const NativeChatMessageRail = memo(function NativeChatMessageRail({ onSelect: (item: NativeChatRailItem) => void }): React.JSX.Element | null { // Hover preserves focus; activation enters the focus-managed prompt picker. - const [mode, setMode] = useState<'hover' | 'interactive' | null>(null) + const [mode, setMode] = useState(null) const closeTimer = useRef | null>(null) - const contentRef = useRef(null) const restoreFocus = useRef(false) + const open = mode !== null const cancelClose = (): void => { if (closeTimer.current !== null) { clearTimeout(closeTimer.current) @@ -56,14 +115,13 @@ export const NativeChatMessageRail = memo(function NativeChatMessageRail({ }, [] ) - if (!rail.visible) { return null } return ( { cancelClose() if (open) { @@ -94,7 +152,6 @@ export const NativeChatMessageRail = memo(function NativeChatMessageRail({ event.preventDefault() restoreFocus.current = true setMode('interactive') - contentRef.current?.querySelector('button')?.focus() } }} // The rail overlays the transcript without being inside it, so a wheel @@ -130,7 +187,6 @@ export const NativeChatMessageRail = memo(function NativeChatMessageRail({ { - if (mode === 'hover') { - event.preventDefault() - } - }} + onOpenAutoFocus={(event) => event.preventDefault()} onCloseAutoFocus={(event) => { if (!restoreFocus.current) { event.preventDefault() } }} > -
    - {rail.items.map((item) => ( -
  • - -
    • - ))} -
+ { + onSelect(item) + setMode(null) + }} + /> )