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

Fix repetitive narration in Windows Narrator #4323

Merged
merged 6 commits into from Jun 23, 2022

Conversation

compulim
Copy link
Contributor

@compulim compulim commented Jun 22, 2022

Fixes #4315. Fixes #4294. Fixes #4295. Fixes #4325.

Changelog Entry

Fixed

  • Fixes #4315. Cleaned up localization strings for suggested actions, by @compulim, in PR #4323
  • Fixes #4294. Screen reader should not read message twice when navigating in the chat history, by @compulim, in PR #4323
  • Fixes #4295. Screen reader should not read suggested actions twice when message arrive in live region, by @compulim, in PR #4323
  • Fixes #4325. aria-keyshortcuts should use modifier keys according to KeyboardEvent key values spec, by @compulim, in PR #4323

Description

We are fixing repetitive narrations and cleaning up some remaining work from last PR:

  • Cleanup for things around suggested actions
  • Reducing repetitive narrations with Windows Narrator
  • Make aria-keyshortcuts compliant
  • Simplify DOM tree with invisible labels
  • Don't use <ul>/<li>

Design

Suggested actions clean up

(Related to #4315)

  • Added aria-orientation="vertical" for suggested actions in stacked layout, which is a role="toolbar"
  • Clean up localization texts from our last PR
    • Instead of "Suggested Actions container: Has content. Press ALT SHIFT A to select."
    • Simplified as "Message has suggested actions. Press ALT SHIFT A to select."
  • Press ESCAPE key while focus is on suggested actions toolbar, should focus back on the send box

Remove timestamp narration in live region

For live region, timestamps were not required to read because the message just arrived. We stated that in ACCESSIBILITY.md for some time but we did not remove them.

Repetitive narrations in live region

(Related to #4295)

In Windows Narrator, when aria-live="polite" is paired with aria-labelledby, the narrations are repeated. We should not use aria-label or aria-labelledby for anything under live region. This will workaround issues in Windows Narrator.

Repetitive narrations with JAWS

(Related to #4294)

When the end-user move cursor in JAWS, it will narrate <div role="group"> with its content. Then, the end-user continues to move the cursor down, JAWS will narrate its content again.

We should refrain from using role="group", or else, add aria-label/aria-labelledby.

Simplify keyboard shortcuts

(Related to #4325)

According to W3C, aria-keyshortcuts should be "alt+shift+a" or "ctrl+altgraph+a" (for "Ctrl + Option + A" in macOS)

However, when narrating it as accessible name, we do not need + (plus) signs.

Keystroke aria-keyshortcuts Accessible name
Alt + Shift + A "Alt+Shift+a" "Alt Shift A"
Ctrl + Option + A "Ctrl+AltGraph+a" "Ctrl Option A"

Simplify DOM tree with invisible labels

Some components render like this:

<div aria-labelledby={labelId}>
  <ScreenReaderText id={labelId} text="Say something but don't show it." />
  ...
</div>

aria-labelledby with a DOM element is redundant. It should be

<div aria-label="Say something but don't show it.">
  ...
</div>

aria-labelledby are preferred and used for labels that are visible and narrated at the same time. However, if label is not visible but only narrated, we should use aria-label.

Attachments should not use <ul>/<li>

We should refrain from using <ul>/<li> to prevent unnecessary list level advancement. This is stated previously in ACCESSIBILITY.md.

Specific Changes

(Please refer to Design section.)

  • I have added tests and executed them locally
  • I have updated CHANGELOG.md
  • I have updated documentation

Review Checklist

This section is for contributors to review your work.

  • Accessibility reviewed (tab order, content readability, alt text, color contrast)
  • Browser and platform compatibilities reviewed
  • CSS styles reviewed (minimal rules, no z-index)
  • Documents reviewed (docs, samples, live demo)
  • Internationalization reviewed (strings, unit formatting)
  • package.json and package-lock.json reviewed
  • Security reviewed (no data URIs, check for nonce leak)
  • Tests reviewed (coverage, legitimacy)

@compulim compulim added the p0 Must Fix. Release-blocker label Jun 23, 2022
@compulim compulim marked this pull request as ready for review June 23, 2022 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p0 Must Fix. Release-blocker
Projects
None yet
2 participants