mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 08:02:28 +00:00
fix(native-chat): reveal message chrome on keyboard focus, not any focus (#19426)
* fix(native-chat): reveal message chrome on keyboard focus, not any focus `:focus-within` cannot tell a mouse click from a keyboard tab, so clicking any control inside a message row — a tool run's disclosure, most visibly — parked focus there and left the row's copy, scroll, and timestamp chrome showing after the pointer had moved away. The row read as permanently hovered. `:focus-visible` is the distinction the browser already computes, and it is the keyboard focus this reveal was written for in #19218. Measured in the running app: after a real click on a run header, `document.activeElement` is that button and `:focus-visible` is false, while `:focus-within` is true. * refactor(native-chat): spell the keyboard-focus reveal with the first-class has variant Swaps the hand-rolled arbitrary variant `group-[&:has(:focus-visible)]:` for Tailwind's `group-has-[:focus-visible]:`. Every other `has-` variant in the renderer is already spelled in the bracket form, including the same hover-plus-keyboard-focus reveal on the project header actions. Measured in the running app: both spellings compile to an equivalent selector and are identical in every state (idle, mouse click on a control inside the row with the pointer moved away, and each keyboard tab stop). Also asserts the pointer-events half of both reveals, which governs whether the copy button is clickable and had no coverage. * fix(native-chat): preserve message controls on touch devices --------- Co-authored-by: Merge Sim <sim@local>
This commit is contained in:
co-authored by
Merge Sim
parent
d7d21b2c55
commit
6b60c23e07
@@ -17,6 +17,7 @@ const HOVER_REVEAL_FILES = [
|
||||
resolve(__dirname, 'editor/DiffSectionHeader.tsx'),
|
||||
resolve(__dirname, 'github-project/ProjectPicker.tsx'),
|
||||
resolve(__dirname, 'github-project/ProjectRow.tsx'),
|
||||
resolve(__dirname, 'native-chat/NativeChatMessageRow.tsx'),
|
||||
resolve(__dirname, 'right-sidebar/AiVaultSessionRow.tsx'),
|
||||
resolve(__dirname, 'right-sidebar/ChecksPanel.tsx'),
|
||||
resolve(__dirname, 'right-sidebar/local-port-row.tsx'),
|
||||
|
||||
@@ -50,7 +50,7 @@ describe('NativeChatMessageList assistant messages', () => {
|
||||
const controls = copyButton.parentElement
|
||||
|
||||
expect(row).toHaveClass('select-text')
|
||||
expect(controls).toHaveClass('select-none', 'pointer-events-none', 'mt-1')
|
||||
expect(controls).toHaveClass('select-none', 'can-hover:pointer-events-none', 'mt-1')
|
||||
expect(controls).not.toHaveClass('absolute')
|
||||
expect(prose.compareDocumentPosition(controls!)).toBe(Node.DOCUMENT_POSITION_FOLLOWING)
|
||||
})
|
||||
|
||||
@@ -23,7 +23,7 @@ function renderMessage(role: NativeChatMessage['role'], timestamp: number | null
|
||||
)
|
||||
}
|
||||
|
||||
describe('MessageRow hover timestamps', () => {
|
||||
describe('MessageRow control visibility', () => {
|
||||
it('appends time to the existing agent controls and inherits their reveal', () => {
|
||||
renderMessage('assistant')
|
||||
const copy = screen.getByRole('button', { name: 'Copy message' })
|
||||
@@ -31,24 +31,32 @@ describe('MessageRow hover timestamps', () => {
|
||||
const time = screen.getByRole('time')
|
||||
expect(Array.from(copy.parentElement!.children)).toEqual([copy, scroll, time])
|
||||
expect(copy.parentElement).toHaveClass(
|
||||
'opacity-0',
|
||||
'can-hover:opacity-0',
|
||||
'can-hover:pointer-events-none',
|
||||
'group-hover:opacity-100',
|
||||
'group-focus-within:opacity-100'
|
||||
'group-has-[:focus-visible]:opacity-100',
|
||||
'group-hover:pointer-events-auto',
|
||||
'group-has-[:focus-visible]:pointer-events-auto'
|
||||
)
|
||||
expect(copy.parentElement).not.toHaveClass('opacity-0', 'pointer-events-none')
|
||||
expect(time).not.toHaveAttribute('tabindex')
|
||||
copy.focus()
|
||||
expect(copy).toHaveFocus()
|
||||
})
|
||||
|
||||
it('gives user bubbles only a hover/focus timestamp', () => {
|
||||
it('gives user bubbles a timestamp that only hides on hover-capable devices', () => {
|
||||
renderMessage('user')
|
||||
const time = screen.getByRole('time')
|
||||
expect(screen.queryByRole('button')).toBeNull()
|
||||
expect(time).toHaveClass(
|
||||
'opacity-0',
|
||||
'can-hover:opacity-0',
|
||||
'can-hover:pointer-events-none',
|
||||
'group-hover:opacity-100',
|
||||
'group-focus-within:opacity-100'
|
||||
'group-has-[:focus-visible]:opacity-100',
|
||||
'group-hover:pointer-events-auto',
|
||||
'group-has-[:focus-visible]:pointer-events-auto'
|
||||
)
|
||||
expect(time).not.toHaveClass('opacity-0', 'pointer-events-none')
|
||||
expect(time.parentElement).toHaveClass('group')
|
||||
time.focus()
|
||||
expect(time).toHaveFocus()
|
||||
|
||||
@@ -153,7 +153,7 @@ export const MessageRow = memo(function MessageRow({
|
||||
<NativeChatMessageTimestamp
|
||||
timestamp={message.timestamp}
|
||||
focusable
|
||||
className="pointer-events-none select-none opacity-0 transition-opacity group-hover:pointer-events-auto group-hover:opacity-100 group-focus-within:pointer-events-auto group-focus-within:opacity-100"
|
||||
className="select-none transition-opacity can-hover:pointer-events-none can-hover:opacity-0 group-hover:pointer-events-auto group-hover:opacity-100 group-has-[:focus-visible]:pointer-events-auto group-has-[:focus-visible]:opacity-100"
|
||||
/>
|
||||
{deliveryFailed ? (
|
||||
<div className="max-w-[85%] text-[11px] text-destructive/80">
|
||||
@@ -168,7 +168,7 @@ export const MessageRow = memo(function MessageRow({
|
||||
}
|
||||
|
||||
// Plain assistant prose is the copyable unit; reasoning/system asides stay
|
||||
// chrome-free. The controls reveal on hover (and on keyboard focus-within).
|
||||
// chrome-free. Controls reveal on hover/keyboard focus and stay visible on touch.
|
||||
const showControls = !isReasoning && !isSystem && markdown.length > 0
|
||||
|
||||
return (
|
||||
@@ -212,7 +212,7 @@ export const MessageRow = memo(function MessageRow({
|
||||
markdown={markdown}
|
||||
timestamp={message.timestamp}
|
||||
onScrollToTop={scrollToTop}
|
||||
className="pointer-events-none mt-1 -mb-5 w-fit select-none opacity-0 transition-opacity group-hover:pointer-events-auto group-hover:opacity-100 group-focus-within:pointer-events-auto group-focus-within:opacity-100"
|
||||
className="mt-1 -mb-5 w-fit select-none transition-opacity can-hover:pointer-events-none can-hover:opacity-0 group-hover:pointer-events-auto group-hover:opacity-100 group-has-[:focus-visible]:pointer-events-auto group-has-[:focus-visible]:opacity-100"
|
||||
/>
|
||||
) : null}
|
||||
</div>
|
||||
|
||||
Reference in New Issue
Block a user