Skip to content

fix(web): Replace anti-pattern of assigning role="button" to entire message div #665

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tiann merged 2 commits into from
May 22, 2026
Merged

fix(web): Replace anti-pattern of assigning role="button" to entire message div #665

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
76b8367
fix(web): replace message-level aria-expanded with explicit toggle bu…
arkylin May 22, 2026
d97eb29
fix(web): include turnCount in hasMetadata and pass it through codexR…
arkylin May 22, 2026
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
86 changes: 40 additions & 46 deletions web/src/components/AssistantChat/messages/AssistantMessage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useCallback, useState, type KeyboardEvent, type MouseEvent } from 'react'
import { useState } from 'react'
import { MessagePrimitive, useAssistantState } from '@assistant-ui/react'
import { MarkdownText } from '@/components/assistant-ui/markdown-text'
import { Reasoning, ReasoningGroup } from '@/components/assistant-ui/reasoning'
Expand All @@ -10,7 +10,6 @@ import type { HappyChatMessageMetadata } from '@/lib/assistant-runtime'
import { getAssistantCopyText } from '@/components/AssistantChat/messages/assistantCopyText'
import { getConversationMessageAnchorId } from '@/chat/outline'
import { MessageMetadata } from '@/components/AssistantChat/messages/MessageMetadata'
import { isNestedInteractiveEvent } from '@/components/AssistantChat/messages/metadataToggle'
import { CodexReviewCard } from '@/components/AssistantChat/messages/CodexReviewCard'
import { MessageTimestamp } from '@/components/AssistantChat/messages/MessageTimestamp'

Expand All @@ -28,10 +27,6 @@ const MESSAGE_PART_COMPONENTS = {
export function HappyAssistantMessage() {
const { copied, copy } = useCopyToClipboard()
const [showMetadata, setShowMetadata] = useState(false)
const toggleMetadata = useCallback((event: MouseEvent<HTMLElement>) => {
if (isNestedInteractiveEvent(event)) return
setShowMetadata((open) => !open)
}, [])
const messageId = useAssistantState(({ message }) => message.id)
const isCliOutput = useAssistantState(({ message }) => {
const custom = message.metadata.custom as Partial<HappyChatMessageMetadata> | undefined
Expand Down Expand Up @@ -66,14 +61,7 @@ export function HappyAssistantMessage() {
|| (typeof durationMs === 'number' && durationMs >= 0)
|| usage != null
|| (messageModel != null && messageModel !== '')
Comment thread
arkylin marked this conversation as resolved.

const onMetadataKeyDown = useCallback((event: KeyboardEvent<HTMLDivElement>) => {
if (isNestedInteractiveEvent(event)) return
if (event.key === 'Enter' || event.key === ' ') {
event.preventDefault()
setShowMetadata((open) => !open)
}
}, [])
|| (typeof turnCount === 'number' && turnCount >= 2)

const rootClass = toolOnly
? 'py-1 min-w-0 max-w-full overflow-x-hidden'
Expand All @@ -95,7 +83,7 @@ export function HappyAssistantMessage() {
aria-expanded={showMetadata}
className="text-[10px] text-[var(--app-hint)] underline-offset-2 hover:text-[var(--app-fg)] hover:underline"
>
{showMetadata ? 'Hide metadata' : 'Show metadata'}
{showMetadata ? 'Hide info' : 'Show info'}
</button>
)}
</div>
Expand All @@ -106,7 +94,6 @@ export function HappyAssistantMessage() {
usage={usage}
model={messageModel ?? null}
turnCount={turnCount}
className="mt-1"
/>
)}
</MessagePrimitive.Root>
Expand All @@ -120,25 +107,28 @@ export function HappyAssistantMessage() {
className={`${rootClass} ${copyText ? 'group/msg' : ''} scroll-mt-4`}
>
<div className="flex items-start gap-2">
<div
className={hasMetadata ? 'min-w-0 flex-1 cursor-pointer' : 'min-w-0 flex-1'}
onClick={hasMetadata ? toggleMetadata : undefined}
onKeyDown={hasMetadata ? onMetadataKeyDown : undefined}
role={hasMetadata ? 'button' : undefined}
tabIndex={hasMetadata ? 0 : undefined}
aria-expanded={hasMetadata ? showMetadata : undefined}
>
<div className="min-w-0 flex-1">
<CodexReviewCard review={codexReview} />
<div className="mt-1 flex justify-start">
<div className="mt-1 flex items-center gap-2">
<MessageTimestamp className="text-[10px] leading-none text-[var(--app-hint)]" />
{hasMetadata && (
<button
type="button"
onClick={() => setShowMetadata((open) => !open)}
aria-expanded={showMetadata}
className="text-[10px] text-[var(--app-hint)] underline-offset-2 hover:text-[var(--app-fg)] hover:underline"
>
{showMetadata ? 'Hide info' : 'Show info'}
</button>
)}
Comment thread
arkylin marked this conversation as resolved.
</div>
{showMetadata && (
<MessageMetadata
invokedAt={invokedAt}
durationMs={durationMs}
usage={usage}
model={messageModel ?? null}
className="mt-1"
turnCount={turnCount}
/>
)}
</div>
Expand Down Expand Up @@ -167,17 +157,20 @@ export function HappyAssistantMessage() {
id={getConversationMessageAnchorId(messageId)}
className={`${rootClass} ${copyText ? 'group/msg' : ''} scroll-mt-4`}
>
<div
className={hasMetadata ? 'min-w-0 cursor-pointer' : 'min-w-0'}
onClick={hasMetadata ? toggleMetadata : undefined}
onKeyDown={hasMetadata ? onMetadataKeyDown : undefined}
role={hasMetadata ? 'button' : undefined}
tabIndex={hasMetadata ? 0 : undefined}
aria-expanded={hasMetadata ? showMetadata : undefined}
>
<div className="min-w-0">
<MessagePrimitive.Content components={MESSAGE_PART_COMPONENTS} />
<div className="mt-1 flex justify-start">
<div className="mt-1 flex items-center gap-2">
<MessageTimestamp className="text-[10px] leading-none text-[var(--app-hint)]" />
{hasMetadata && (
<button
type="button"
onClick={() => setShowMetadata((open) => !open)}
aria-expanded={showMetadata}
className="text-[10px] text-[var(--app-hint)] underline-offset-2 hover:text-[var(--app-fg)] hover:underline"
>
{showMetadata ? 'Hide info' : 'Show info'}
</button>
)}
</div>
{showMetadata && (
<MessageMetadata
Expand All @@ -186,7 +179,6 @@ export function HappyAssistantMessage() {
usage={usage}
model={messageModel ?? null}
turnCount={turnCount}
className="mt-1"
/>
)}
</div>
Expand All @@ -200,17 +192,20 @@ export function HappyAssistantMessage() {
className={`${rootClass} ${copyText ? 'group/msg' : ''} scroll-mt-4`}
>
<div className="flex items-start gap-2">
<div
className={hasMetadata ? 'min-w-0 flex-1 cursor-pointer' : 'min-w-0 flex-1'}
onClick={hasMetadata ? toggleMetadata : undefined}
onKeyDown={hasMetadata ? onMetadataKeyDown : undefined}
role={hasMetadata ? 'button' : undefined}
tabIndex={hasMetadata ? 0 : undefined}
aria-expanded={hasMetadata ? showMetadata : undefined}
>
<div className="min-w-0 flex-1">
<MessagePrimitive.Content components={MESSAGE_PART_COMPONENTS} />
<div className="mt-1 flex justify-start">
<div className="mt-1 flex items-center gap-2">
<MessageTimestamp className="text-[10px] leading-none text-[var(--app-hint)]" />
{hasMetadata && (
<button
type="button"
onClick={() => setShowMetadata((open) => !open)}
aria-expanded={showMetadata}
className="text-[10px] text-[var(--app-hint)] underline-offset-2 hover:text-[var(--app-fg)] hover:underline"
>
{showMetadata ? 'Hide info' : 'Show info'}
</button>
)}
</div>
{showMetadata && (
<MessageMetadata
Expand All @@ -219,7 +214,6 @@ export function HappyAssistantMessage() {
usage={usage}
model={messageModel ?? null}
turnCount={turnCount}
className="mt-1"
/>
)}
</div>
Expand Down
2 changes: 1 addition & 1 deletion web/src/components/AssistantChat/messages/MessageMetadata.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ export function MessageMetadata({ invokedAt, durationMs, usage, model, turnCount
if (parts.length === 0) return null

return (
<div className={`text-[10px] text-[var(--app-hint)] flex flex-wrap gap-x-2 gap-y-0.5 mt-0.5 px-0.5 leading-tight opacity-60 ${className || ''}`}>
<div className={`text-[10px] text-[var(--app-hint)] bg-[var(--app-subtle-bg)] rounded px-2 py-1.5 flex flex-wrap gap-x-2 gap-y-0.5 mt-1 leading-tight ${className || ''}`}>
{parts.map((part, i) => (
<span key={i} className="whitespace-nowrap">{part}</span>
))}
Expand Down
45 changes: 17 additions & 28 deletions web/src/components/AssistantChat/messages/UserMessage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useCallback, useState, type KeyboardEvent, type MouseEvent } from 'react'
import { useState } from 'react'
import { MessagePrimitive, useAssistantState } from '@assistant-ui/react'
import { useHappyChatContext } from '@/components/AssistantChat/context'
import type { HappyChatMessageMetadata } from '@/lib/assistant-runtime'
Expand All @@ -10,17 +10,12 @@ import { CopyIcon, CheckIcon } from '@/components/icons'
import { useCopyToClipboard } from '@/hooks/useCopyToClipboard'
import { getConversationMessageAnchorId } from '@/chat/outline'
import { MessageMetadata } from '@/components/AssistantChat/messages/MessageMetadata'
import { isNestedInteractiveEvent } from '@/components/AssistantChat/messages/metadataToggle'
import { MessageTimestamp } from '@/components/AssistantChat/messages/MessageTimestamp'

export function HappyUserMessage() {
const ctx = useHappyChatContext()
const { copied, copy } = useCopyToClipboard()
const [showMetadata, setShowMetadata] = useState(false)
const toggleMetadata = useCallback((event: MouseEvent<HTMLElement>) => {
if (isNestedInteractiveEvent(event)) return
setShowMetadata((open) => !open)
}, [])
const role = useAssistantState(({ message }) => message.role)
const messageId = useAssistantState(({ message }) => message.id)
const text = useAssistantState(({ message }) => {
Expand Down Expand Up @@ -55,14 +50,6 @@ export function HappyUserMessage() {

const hasMetadata = invokedAt != null

const onMetadataKeyDown = useCallback((event: KeyboardEvent<HTMLElement>) => {
if (isNestedInteractiveEvent(event)) return
if (event.key === 'Enter' || event.key === ' ') {
event.preventDefault()
setShowMetadata((open) => !open)
}
}, [])

if (role !== 'user') return null
const canRetry = status === 'failed' && typeof localId === 'string' && Boolean(ctx.onRetryMessage)
const onRetry = canRetry ? () => ctx.onRetryMessage!(localId) : undefined
Expand All @@ -85,12 +72,12 @@ export function HappyUserMessage() {
aria-expanded={showMetadata}
className="text-[10px] text-[var(--app-hint)] underline-offset-2 hover:text-[var(--app-fg)] hover:underline"
>
{showMetadata ? 'Hide metadata' : 'Show metadata'}
{showMetadata ? 'Hide info' : 'Show info'}
</button>
)}
</div>
{showMetadata && invokedAt != null && (
<MessageMetadata invokedAt={invokedAt} className="mt-1 justify-end" />
<MessageMetadata invokedAt={invokedAt} />
)}
</div>
</MessagePrimitive.Root>
Expand All @@ -103,12 +90,7 @@ export function HappyUserMessage() {
return (
<MessagePrimitive.Root
id={getConversationMessageAnchorId(messageId)}
className={`${getUserBubbleClassName(status)} group/msg scroll-mt-4 ${hasMetadata ? 'cursor-pointer' : ''}`}
onClick={hasMetadata ? toggleMetadata : undefined}
onKeyDown={hasMetadata ? onMetadataKeyDown : undefined}
role={hasMetadata ? 'button' : undefined}
tabIndex={hasMetadata ? 0 : undefined}
aria-expanded={hasMetadata ? showMetadata : undefined}
className={`${getUserBubbleClassName(status)} group/msg scroll-mt-4`}
>
<div className="flex flex-col gap-1">
<div className="flex items-start gap-2">
Expand All @@ -123,10 +105,7 @@ export function HappyUserMessage() {
type="button"
title="Copy"
className="rounded-md p-0.5 opacity-60 transition-[opacity,background-color] hover:bg-[var(--app-chat-user-chip-bg)] sm:opacity-0 sm:group-hover/msg:opacity-100"
onClick={(event) => {
event.stopPropagation()
copy(text)
}}
onClick={() => copy(text)}
>
{copied
? <CheckIcon className="h-3.5 w-3.5 text-green-500" />
Expand All @@ -137,11 +116,21 @@ export function HappyUserMessage() {
</div>
)}
</div>
<div className="flex justify-end">
<div className="flex justify-end items-center gap-2">
<MessageTimestamp className="text-[10px] leading-none text-[var(--app-hint)]" />
{hasMetadata && (
<button
type="button"
onClick={() => setShowMetadata((open) => !open)}
aria-expanded={showMetadata}
className="text-[10px] text-[var(--app-hint)] underline-offset-2 hover:text-[var(--app-fg)] hover:underline"
>
{showMetadata ? 'Hide info' : 'Show info'}
</button>
)}
</div>
{showMetadata && invokedAt != null && (
<MessageMetadata invokedAt={invokedAt} className="justify-end opacity-60" />
<MessageMetadata invokedAt={invokedAt} />
)}
</div>
</MessagePrimitive.Root>
Expand Down
Loading
Loading