Skip to content
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

Merged
merged 7 commits into from
Jun 17, 2024

Conversation

dcalhoun
Copy link
Member

@dcalhoun dcalhoun commented Jun 13, 2024

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...

  • Option+Control+Shift+[Up|Right|Down|Left] will navigate between/into/out of content "groups."
  • Option+Control+[Up|Right|Down|Left] will navigate between individual content elements, depending on the context and settings.
  • Option+Control+Space will "activate" the selected element, triggering the relevant event handler.

Decorative assistant icon is not selectable

  1. Enable a screen reader (e.g., VoiceOver).
  2. Navigate to the "Assistant" tab.
  3. Move selection to elements before and after the automatically focused input.
  4. Verify the assistant icon is skipped over and not selectable.

Message collection navigation is straightforward and comprehensible

  1. Enable a screen reader (e.g., VoiceOver).
  2. Navigate to the "Assistant" tab.
  3. Interact with the assistant — send messages, move selection between various messages, etc.
  4. Verify the message navigation is straightforward and comprehensible, providing clear communication of each message and the author, while also allowing navigation of content within each message.

Pre-merge Checklist

  • Have you checked for TypeScript, React or other console errors?

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.
Ensure correct ARIA labels are read for the unauthenticated message.
@dcalhoun dcalhoun added the [Type] Enhancement Improvement upon an existing feature label Jun 13, 2024
@dcalhoun dcalhoun self-assigned this Jun 13, 2024
Comment on lines +170 to +171
<div className="relative">
<span className="sr-only">
Copy link
Member Author

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 }
Copy link
Member Author

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.

@dcalhoun dcalhoun marked this pull request as ready for review June 13, 2024 19:17
@dcalhoun dcalhoun requested review from a team June 13, 2024 19:17
Copy link
Contributor

@fluiddot fluiddot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🎊 !

I noticed that when navigating inside the group that represents the tab button, the icon is accessible by screen readers:

Screenshot 2024-06-17 at 12 22 47

I understand this is a minor issue, as it can only be focused when navigating within the group.

src/components/content-tab-assistant.tsx Show resolved Hide resolved
@dcalhoun
Copy link
Member Author

dcalhoun commented Jun 17, 2024

I noticed that when navigating inside the group that represents the tab button, the icon is accessible by screen readers

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?

@fluiddot
Copy link
Contributor

fluiddot commented Jun 17, 2024

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 icon tab property, but the component only renders the icon in that case (reference).

Not sure if it would be feasible but I wonder if we could use isAccessibilitySupportEnabled to determine when to apply the class name components-tab-panel__tabs--assistant that renders the icon.

@dcalhoun
Copy link
Member Author

Not sure if it would be feasible but I wonder if we could use isAccessibilitySupportEnabled to determine when to apply the class name components-tab-panel__tabs--assistant that renders the icon.

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;
B. Or adding a descriptive alternative text, e.g., Studio Assistant icon.

Would you be fine with either of those? If so, do you have a preference?

@fluiddot
Copy link
Contributor

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.

@dcalhoun dcalhoun merged commit 6e4da02 into trunk Jun 17, 2024
16 of 17 checks passed
@dcalhoun dcalhoun deleted the feat/improve-ai-assistant-accessibility branch June 17, 2024 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Enhancement Improvement upon an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants