From 04d3d026274d01b6a61d62f22b9d7033597db379 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=87=8C?= Date: Fri, 22 May 2026 21:28:28 +0800 Subject: [PATCH] fix(web): Replace anti-pattern of assigning role="button" to entire message div (#665) * fix(web): replace message-level aria-expanded with explicit toggle button Replace the anti-pattern of assigning role=\"button\" and aria-expanded to the entire message content div. Instead, show an explicit \"Show info\" / \"Hide info\" button next to the timestamp. Also add a visible background container to MessageMetadata so users can actually see when metadata expands/collapses. Remove now-unused metadataToggle.ts and its tests. * fix(web): include turnCount in hasMetadata and pass it through codexReview branch --- .../messages/AssistantMessage.tsx | 86 +++++------ .../messages/MessageMetadata.tsx | 2 +- .../AssistantChat/messages/UserMessage.tsx | 45 +++--- .../messages/metadataToggle.test.ts | 138 ------------------ .../AssistantChat/messages/metadataToggle.ts | 29 ---- 5 files changed, 58 insertions(+), 242 deletions(-) delete mode 100644 web/src/components/AssistantChat/messages/metadataToggle.test.ts delete mode 100644 web/src/components/AssistantChat/messages/metadataToggle.ts diff --git a/web/src/components/AssistantChat/messages/AssistantMessage.tsx b/web/src/components/AssistantChat/messages/AssistantMessage.tsx index b04bc998..3cb2baf5 100644 --- a/web/src/components/AssistantChat/messages/AssistantMessage.tsx +++ b/web/src/components/AssistantChat/messages/AssistantMessage.tsx @@ -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' @@ -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' @@ -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) => { - if (isNestedInteractiveEvent(event)) return - setShowMetadata((open) => !open) - }, []) const messageId = useAssistantState(({ message }) => message.id) const isCliOutput = useAssistantState(({ message }) => { const custom = message.metadata.custom as Partial | undefined @@ -66,14 +61,7 @@ export function HappyAssistantMessage() { || (typeof durationMs === 'number' && durationMs >= 0) || usage != null || (messageModel != null && messageModel !== '') - - const onMetadataKeyDown = useCallback((event: KeyboardEvent) => { - 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' @@ -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'} )} @@ -106,7 +94,6 @@ export function HappyAssistantMessage() { usage={usage} model={messageModel ?? null} turnCount={turnCount} - className="mt-1" /> )} @@ -120,17 +107,20 @@ export function HappyAssistantMessage() { className={`${rootClass} ${copyText ? 'group/msg' : ''} scroll-mt-4`} >
-
+
-
+
+ {hasMetadata && ( + + )}
{showMetadata && ( )}
@@ -167,17 +157,20 @@ export function HappyAssistantMessage() { id={getConversationMessageAnchorId(messageId)} className={`${rootClass} ${copyText ? 'group/msg' : ''} scroll-mt-4`} > -
+
-
+
+ {hasMetadata && ( + + )}
{showMetadata && ( )}
@@ -200,17 +192,20 @@ export function HappyAssistantMessage() { className={`${rootClass} ${copyText ? 'group/msg' : ''} scroll-mt-4`} >
-
+
-
+
+ {hasMetadata && ( + + )}
{showMetadata && ( )}
diff --git a/web/src/components/AssistantChat/messages/MessageMetadata.tsx b/web/src/components/AssistantChat/messages/MessageMetadata.tsx index 1b42becf..31358147 100644 --- a/web/src/components/AssistantChat/messages/MessageMetadata.tsx +++ b/web/src/components/AssistantChat/messages/MessageMetadata.tsx @@ -73,7 +73,7 @@ export function MessageMetadata({ invokedAt, durationMs, usage, model, turnCount if (parts.length === 0) return null return ( -
+
{parts.map((part, i) => ( {part} ))} diff --git a/web/src/components/AssistantChat/messages/UserMessage.tsx b/web/src/components/AssistantChat/messages/UserMessage.tsx index 605370cf..8ebe3dc9 100644 --- a/web/src/components/AssistantChat/messages/UserMessage.tsx +++ b/web/src/components/AssistantChat/messages/UserMessage.tsx @@ -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' @@ -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) => { - if (isNestedInteractiveEvent(event)) return - setShowMetadata((open) => !open) - }, []) const role = useAssistantState(({ message }) => message.role) const messageId = useAssistantState(({ message }) => message.id) const text = useAssistantState(({ message }) => { @@ -55,14 +50,6 @@ export function HappyUserMessage() { const hasMetadata = invokedAt != null - const onMetadataKeyDown = useCallback((event: KeyboardEvent) => { - 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 @@ -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'} )}
{showMetadata && invokedAt != null && ( - + )}
@@ -103,12 +90,7 @@ export function HappyUserMessage() { return (
@@ -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 ? @@ -137,11 +116,21 @@ export function HappyUserMessage() {
)}
-
+
+ {hasMetadata && ( + + )}
{showMetadata && invokedAt != null && ( - + )}
diff --git a/web/src/components/AssistantChat/messages/metadataToggle.test.ts b/web/src/components/AssistantChat/messages/metadataToggle.test.ts deleted file mode 100644 index 7862fe63..00000000 --- a/web/src/components/AssistantChat/messages/metadataToggle.test.ts +++ /dev/null @@ -1,138 +0,0 @@ -import { describe, expect, it } from 'vitest' -import type { KeyboardEvent, MouseEvent } from 'react' -import { isNestedInteractiveEvent } from './metadataToggle' - -function makeMouseEvent(target: HTMLElement, currentTarget?: HTMLElement): MouseEvent { - return { target, currentTarget } as unknown as MouseEvent -} - -function makeKeyboardEvent(target: HTMLElement, currentTarget?: HTMLElement): KeyboardEvent { - return { target, currentTarget } as unknown as KeyboardEvent -} - -describe('isNestedInteractiveEvent', () => { - it('returns true when the click target is itself a button', () => { - const button = document.createElement('button') - expect(isNestedInteractiveEvent(makeMouseEvent(button))).toBe(true) - }) - - it('returns true when the click target is nested inside a button (e.g. icon)', () => { - const button = document.createElement('button') - const icon = document.createElement('span') - button.appendChild(icon) - expect(isNestedInteractiveEvent(makeMouseEvent(icon))).toBe(true) - }) - - it('returns true for role="button" elements (Radix triggers, Markdown copy button)', () => { - const div = document.createElement('div') - div.setAttribute('role', 'button') - const inner = document.createElement('span') - div.appendChild(inner) - expect(isNestedInteractiveEvent(makeMouseEvent(inner))).toBe(true) - }) - - it('returns true for anchors and form controls', () => { - const a = document.createElement('a') - const input = document.createElement('input') - const textarea = document.createElement('textarea') - const select = document.createElement('select') - expect(isNestedInteractiveEvent(makeMouseEvent(a))).toBe(true) - expect(isNestedInteractiveEvent(makeMouseEvent(input))).toBe(true) - expect(isNestedInteractiveEvent(makeMouseEvent(textarea))).toBe(true) - expect(isNestedInteractiveEvent(makeMouseEvent(select))).toBe(true) - }) - - it('returns false for plain message body text', () => { - const root = document.createElement('div') - const paragraph = document.createElement('p') - paragraph.textContent = 'Hello' - root.appendChild(paragraph) - expect(isNestedInteractiveEvent(makeMouseEvent(paragraph))).toBe(false) - }) - - it('returns false when target is not an Element', () => { - expect(isNestedInteractiveEvent({ target: null } as unknown as MouseEvent)).toBe(false) - }) - - it('returns true when the click target is an SVG icon inside a button', () => { - // Icon-only controls (copy, retry, code-copy) render an / - // child of the