FIX Standardize icon-button labels and tooltips in chat surfaces#1711
Open
romanlutz wants to merge 2 commits intomicrosoft:mainfrom
Open
FIX Standardize icon-button labels and tooltips in chat surfaces#1711romanlutz wants to merge 2 commits intomicrosoft:mainfrom
romanlutz wants to merge 2 commits intomicrosoft:mainfrom
Conversation
The chat bubble styles silently lost the user-bubble background override:
both user and assistant bubbles ended up rendering with the same
colorNeutralBackground3 (rgb(245,245,245)), confirmed via
getComputedStyle on a live page. In light mode this also collapsed
the visual distinction between the assistant bubble and the surrounding
chat area, both of which sit at near-white luminance.
Two contributing causes:
1. The bubble className was built by string concatenation:
`${styles.messageContent} ${isUser ? styles.userMessageContent : ''}`
With Griffel's atomic CSS, conflicting properties on two sibling
classes are not deduped — the user-bubble override could not win.
Switched both bubble class assignments to mergeClasses, which is
the documented way to compose Griffel rule sets safely.
2. The assistant bubble used colorNeutralBackground3, which sits ~5/255
luminance below colorNeutralBackground2 (the chat area). Below the
3:1 WCAG threshold for non-text UI components. Switched the
assistant bubble to colorNeutralBackground1 + a colorNeutralStroke2
border so it pops against the chat area in both themes.
For the user bubble, switched to colorBrandBackground2 (the lighter
brand tint, same token used by activeRow in TargetTable) instead of
the deep colorBrandBackground. The bubble's footer text uses
colorNeutralForeground3 which is unreadable on a deep brand
background; the lighter tint preserves contrast for both the
message body and the dim timestamp/role labels without needing
per-element color overrides.
Tests:
- Added a regression test asserting the user and assistant bubble
containers receive distinct className strings, and that the user
bubble carries at least one style hook the assistant bubble doesn't.
This guards against future re-introduction of the string-concat bug
or accidental removal of the userMessageContent override.
- Existing 44 MessageList tests continue to pass; lint and type-check
are clean.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
An audit of all icon-only buttons in the chat surfaces turned up two
related accessibility gaps:
1. Three buttons in ChatInputArea (Attach files, Toggle converter
panel, Send message) used only the HTML `title` attribute. `title`
is not a reliable accessible name for buttons without visible text:
NVDA and JAWS commonly fall back to it but VoiceOver and several
mobile screen readers do not. The recommended approach (and the
one already used by the sidebar nav and message-action buttons) is
`aria-label` paired with a Fluent UI `<Tooltip>` for sighted users.
2. The conversation-panel toggle in the chat ribbon already had a
`<Tooltip>` but no `aria-label`. Same fix.
3. Several disabled-state tooltips in MessageList collapsed to
identical strings between distinct buttons. For example, when the
active target is single-turn, both the "Branch into new
conversation" and "Branch into new attack" buttons rendered with
the same accessible name "Cannot branch — target is single-turn",
so a screen-reader user could not tell them apart. Same problem
between the two "Cannot add to this attack — ..." copies on the
copy-to-new-conv and branch-conv buttons.
This change:
- Wraps each ChatInputArea icon button in a `<Tooltip>` and replaces
`title=` with `aria-label=`.
- Adds `aria-label="Toggle conversations panel"` to the ChatWindow
ribbon toggle (kept the existing tooltip).
- Rewrites the disabled-state tooltips in MessageList so each of the
four message-action buttons has a unique label that names the action
it would have taken (e.g. "Cannot branch into new conversation —
target is single-turn" vs "Cannot branch into new attack — target
is single-turn"). Also resolves the same-text collision between the
two "Cannot add to this attack — ..." labels.
Tests:
- Added a ChatInputArea test asserting all three icon buttons are
reachable by `getByRole('button', { name: ... })` so a future
regression to `title=`-only labels would fail.
- Added a ChatWindow test asserting the conversations panel toggle is
reachable by accessible name and matches its data-testid.
- Added a MessageList test asserting that, when single-turn,
the four disabled action buttons all have unique non-empty
aria-labels.
- All 564 existing frontend tests continue to pass; lint and
type-check are clean.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes finding documented in
PyRIT-gui-findings/findings/a11y-inconsistent-button-labels/.Summary
An audit of all icon-only buttons in the chat surfaces turned up two related accessibility gaps:
title-only labels inChatInputArea. Three icon buttons (Attach files, Toggle converter panel, Send message) used only the HTMLtitleattribute and noaria-label.titleis not a reliable accessible name for buttons without visible text — NVDA and JAWS commonly fall back to it but VoiceOver and several mobile screen readers do not. The recommended approach (and the one already used by the sidebar nav and message-action buttons) isaria-labelpaired with a Fluent UI<Tooltip>for sighted users.Missing
aria-labelon the conversation-panel toggle. The chat ribbon toggle already had a<Tooltip>but noaria-label. Same fix.Identical disabled-state labels on distinct buttons in
MessageList. Several disabled-state tooltips collapsed to identical strings between distinct buttons. For example, when the active target is single-turn, both the "Branch into new conversation" and "Branch into new attack" buttons rendered with the accessible name"Cannot branch — target is single-turn", so a screen-reader user could not tell them apart. Same problem between the two"Cannot add to this attack — ..."copies on the copy-to-new-conv and branch-conv buttons.What changed
ChatInputAreaicon button in a<Tooltip>and replacedtitle=witharia-label=.aria-label="Toggle conversations panel"to theChatWindowribbon toggle (kept the existing tooltip).MessageListso each of the four message-action buttons has a unique label that names the action it would have taken — e.g."Cannot branch into new conversation — target is single-turn"vs"Cannot branch into new attack — target is single-turn". Also resolved the same-text collision between the two"Cannot add to this attack — ..."labels.Tests
ChatInputAreatest asserting all three icon buttons are reachable bygetByRole('button', { name: ... })so a future regression totitle=-only labels would fail.ChatWindowtest asserting the conversations panel toggle is reachable by accessible name and matches itsdata-testid.MessageListtest asserting that, when single-turn, the four disabled action buttons all have unique non-emptyaria-labels.Screenshots