-
Notifications
You must be signed in to change notification settings - Fork 21
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.
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
feat: Improve AI assistant accessibility #248
Conversation
Focusing this description-less, decorative image provide little value for users.
Navigating individual lines of text from a back-and-forth conversation is difficult and overwhelming when there are not discernible, navigable grouping or message author labels.
Improve assistant chat experience while using assistive technologies.
Labeling the element with raw Markdown resulted in announcing disorienting syntax characters.
Without a label, there is no indication that the assistant is actively working to answer a message.
This reverts commit 996b5f7.
Ensure correct ARIA labels are read for the unauthenticated message.
<div className="relative"> | ||
<span className="sr-only"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrapping an element leveraging Tailwind's sr-only
class is necessary to avoid unexpected overflow scrolling for the top level document.
return ( | ||
<svg | ||
aria-hidden={ ariaHidden } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously, the existing aria-hidden
value introduced in #202 was not passed down to the child svg
element.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good note. TIL that one can set the alternative text of a pseudo element by suffixing a slash and string. However, from my testing, this does not work for hiding an element from a screen reader using an empty string value. 😞 Are you able to achieve a different outcome where the tab icon is hidden from the screen reader? |
I haven't managed to hide it from the screen reader. I tried to render the icon in a different way by using the Not sure if it would be feasible but I wonder if we could use |
A good idea. I'm not sure the impact warrants the complexity, currently. My suggestion would be to either: A. Leave it as-is, considering it low-impact; Would you be fine with either of those? If so, do you have a preference? |
Yeah, let's keep it as-is. I don't think it will have a major impact in a11y. |
Fixes https://github.com/Automattic/dotcom-forge/issues/7520.
Proposed Changes
Hide decorative AI assistant icon from screen readers
Selecting this description-less, decorative image provides little value for users.
Communicate message author to screen readers
Navigating individual lines of text from a back-and-forth conversation is difficult and overwhelming when there are not discernible, navigable grouping or message author labels.
Communicate the active assistant "thinking" to screen readers
Without a label, there is no indication that the assistant is actively working to answer a message.
Testing Instructions
Tip
When using VoiceOver...
Decorative assistant icon is not selectable
Message collection navigation is straightforward and comprehensible
Pre-merge Checklist