fix(web): Replace anti-pattern of assigning role="button" to entire message div#665
Conversation
…tton 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.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Findings
None.
Summary
Review mode: initial
No high-confidence issues found in the latest diff. Residual risk: the PR removes the focused metadataToggle regression tests and replaces wrapper toggling with explicit UI buttons, but this runner cannot execute the web test/typecheck suite because bun is unavailable.
Testing
Not run (automation: bun not found in runner).
HAPI Bot
There was a problem hiding this comment.
Pull request overview
This PR improves the chat message metadata disclosure UX and accessibility by removing the “entire message is a button” pattern and replacing it with an explicit “Show info / Hide info” toggle button near the timestamp. It also updates MessageMetadata styling to make the expanded state visually distinct, and removes the now-obsolete event-guard utility and its tests.
Changes:
- Remove
role="button"/aria-expanded/ click+keydown handlers from message containers and add an explicit metadata toggle<button>next to the timestamp. - Restyle
MessageMetadatawith a visible background container (subtle bg + padding + rounded corners). - Delete
metadataToggle.tsandmetadataToggle.test.ts(no longer needed after switching to explicit buttons).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| web/src/components/AssistantChat/messages/UserMessage.tsx | Removes bubble-level “button” behavior and adds an explicit “Show info” toggle near the timestamp for user messages (incl. CLI output variant). |
| web/src/components/AssistantChat/messages/AssistantMessage.tsx | Removes bubble-level “button” behavior and adds an explicit “Show info” toggle near the timestamp across assistant message variants. |
| web/src/components/AssistantChat/messages/MessageMetadata.tsx | Adds a visible background container to the metadata footer for clearer expanded/collapsed affordance. |
| web/src/components/AssistantChat/messages/metadataToggle.ts | Removed: no longer needed after switching to explicit toggle buttons. |
| web/src/components/AssistantChat/messages/metadataToggle.test.ts | Removed: tests for removed helper. |
Comments suppressed due to low confidence (1)
web/src/components/AssistantChat/messages/AssistantMessage.tsx:131
- In the
codexReviewbranch,MessageMetadatais rendered without passingturnCount. Since aggregated footers injectturnCountintometadata.custom, this branch will omit the “N turns” line (and aggregated labeling behavior) even when present. PassturnCount={turnCount}here for consistency with the other branches.
{showMetadata && (
<MessageMetadata
invokedAt={invokedAt}
durationMs={durationMs}
usage={usage}
model={messageModel ?? null}
/>
)}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Findings
None.
Summary
Review mode: follow-up after new commits
No high-confidence issues found in the latest diff. I reviewed the full PR diff, using the previous HAPI Bot review and the compare diff as follow-up context. Residual risk: the removed metadataToggle unit coverage is not replaced by component-level coverage for the new explicit metadata buttons, and this runner cannot execute the web suite because bun is unavailable.
Testing
Not run (automation: bun not found in runner).
HAPI Bot
Fix: Replace anti-pattern of assigning role="button" to entire message div
Problem:
The entire message content div had role="button" and aria-expanded attributes,
which caused several issues:
of selecting text)
Solution:
Testing:
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.
Before
After
