Skip to content

Make Suggested Actions accessible again!#2613

Merged
corinagum merged 14 commits intomicrosoft:masterfrom
corinagum:1780-suggestedActionsA11y
Nov 20, 2019
Merged

Make Suggested Actions accessible again!#2613
corinagum merged 14 commits intomicrosoft:masterfrom
corinagum:1780-suggestedActionsA11y

Conversation

@corinagum
Copy link
Copy Markdown
Contributor

@corinagum corinagum commented Nov 19, 2019

(jk they were never accessible)

Fixes #1780
Fixes #2277
Fixes #2285

Changelog Entry

  • Fixes #1780, #2277, and #2285. Make Suggested Actions accessible, Fix Markdown card in carousel being read multiple times, and label widgets of Connectivity Status and Suggested Actions containers, by @corinagum in PR #2613

Note: How to test Suggested Actions container on Mock Bot

  • use command emptycard or
  • use command suggested-actions

Description

  • Make Suggested Actions container accessible. See Browser Discrepancies below.
  • Fix Markdown card in carousel being read multiple times - previously I recorded this as a nesting issue but was actually a problem with the attachment types inside. This has been resolved with aria-live=" " and aria-hidden={true} inside the Carousel component
  • Connectivity Status: AT will now read "Connectivity Status: Connected/Unable to connect/etc" when it reaches the (Connectivity Status container) - in the UI this component only shows when not connected. Changes in connectivity status will also be announced by the AT, since this is a status widget.
  • Suggested Actions container: This component is hidden until buttons are added inside. At such time, the AT will announce "Suggested Actions container: has content" regardless of where the user is focused on the app, due to this component being a status widget. When the Suggested Actions container is empty, AT will announce "Suggested Actions container: is empty"
  • Japanese localization file updated to latest
  • Please note, "Upload file text" in localization files has been modified. See en-us for changes.

Browser discrepancies

Previously, Suggested Actions were ignored by the AT. This PR fixes that, HOWEVER, there are some idiosyncrasies across browsers that are documented below:

  1. On Chrome & NVDA: When the Suggested Actions container changes, it will read "Suggested actions container: has content. Blue Red Green" twice. I've gone over and over the code with @compulim as my sanity check and I'm confident that this is caused by a Chrome error, not Web Chat's code.
  2. Firefox & NVDA: Throughout the app, the the AT will randomly read extra letters that are not in the aria-label or visual on the UI. For example, it will say "Suggested Actions: has content. ED".
  3. Edge & Narrator: AT will read the change in Suggested Actions container "Suggested Actions: has content. Contains three items", but it will NOT read the buttons inside without navigating to that container. Other browsers will say "Suggestesd Actions container: has content. Blue red Green".
  4. Safari & VoiceOver: AT will read the Connectivity Status and SuggestedActions labels/containers multiple times on start up.

  • Testing Added

@corinagum corinagum marked this pull request as ready for review November 20, 2019 21:27
@coveralls
Copy link
Copy Markdown

coveralls commented Nov 20, 2019

Coverage Status

Coverage increased (+0.2%) to 64.01% when pulling 1388c4f on corinagum:1780-suggestedActionsA11y into 39ab54e on microsoft:master.

@corinagum corinagum changed the title 1780 suggested actions a11y Make Suggested Actions Accessible again! Nov 20, 2019
@corinagum corinagum changed the title Make Suggested Actions Accessible again! Make Suggested Actions accessible again! Nov 20, 2019
Comment thread packages/component/src/Activity/StackedLayout.js
Comment thread packages/component/src/Attachment/TextContent.js Outdated
Comment thread packages/component/src/Attachment/TextContent.js Outdated
Comment thread packages/component/src/Attachment/TextContent.js Outdated
Copy link
Copy Markdown
Contributor

@compulim compulim left a comment

Choose a reason for hiding this comment

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

Automatic approve if everything commented is fixed.

Comment thread packages/component/src/Activity/StackedLayout.js Outdated
Comment thread packages/component/src/Activity/StackedLayout.js Outdated
Comment thread packages/component/src/Activity/StackedLayout.js Outdated
Comment thread packages/component/src/Activity/CarouselFilmStrip.js Outdated
Comment thread packages/component/src/Attachment/TextContent.js Outdated
Comment thread packages/component/src/SendBox/SuggestedActions.js Outdated
Comment thread packages/component/src/SendBox/SuggestedActions.js Outdated
Comment thread packages/component/src/Utils/AbsoluteTime.js Outdated
Comment thread packages/component/src/Utils/AbsoluteTime.js Outdated
Comment thread packages/core/src/reducers/suggestedActions.js Outdated
Comment thread packages/component/src/Localization/ja-JP.js
Comment thread packages/component/src/Utils/AbsoluteTime.js Outdated
Comment thread packages/component/src/Utils/AbsoluteTime.js Outdated
Comment thread packages/component/src/Utils/detectBrowser.js
Comment thread samples/17.b.clear-after-idle/package-lock.json
Comment thread packages/core/src/reducers/suggestedActions.js
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

4 participants