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 ARIA role of transcript when connection is pending #4420

Merged
merged 8 commits into from Nov 9, 2022

Conversation

beyackle2
Copy link
Contributor

@beyackle2 beyackle2 commented Sep 20, 2022

Fixes #4393.

Changelog Entry

Fixed

Description

The <section role="feed"> should not be emptied and should have at least one <article> or role="article".

When the chat is started, it is initially empty. We should not render <section role="feed">.

Design

We tried <section role="feed" aria-busy="true">, however, Accessibility Insights still say the element is violating "WCAG 1.3.1: Ensures elements with an ARIA role that require child roles contain them".

We have 2 options:

  1. Render <section> without role attribute, and set role="feed" when it has contents
  2. Do not render <section role="feed"> if it is empty

We went on the second option as we worry the first one might cause issues in some screen readers.

Specific Changes

  • Not rendering <section role="feed"> when it is empty
  • 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)

@ghost
Copy link

ghost commented Sep 20, 2022

CLA assistant check
All CLA requirements met.

CHANGELOG.md Outdated Show resolved Hide resolved
packages/component/src/BasicTranscript.tsx Outdated Show resolved Hide resolved
packages/component/src/BasicTranscript.tsx Outdated Show resolved Hide resolved
packages/component/src/BasicTranscript.tsx Outdated Show resolved Hide resolved
packages/component/src/BasicTranscript.tsx Outdated Show resolved Hide resolved
packages/component/src/BasicTranscript.tsx Outdated Show resolved Hide resolved
Copy link
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.

Need tests and this will be puuuuurfect. 😽

packages/component/src/BasicTranscript.tsx Outdated Show resolved Hide resolved
packages/component/src/BasicTranscript.tsx Outdated Show resolved Hide resolved
packages/component/src/BasicTranscript.tsx Outdated Show resolved Hide resolved
packages/component/src/BasicTranscript.tsx Outdated Show resolved Hide resolved
@compulim
Copy link
Contributor

image

Looks like aria-busy="true" won't help with the accessibility issue. Few things we could try:

  • Change role to something else until the first message come
  • Inject an invisible "the chat history is empty" in the transcript before the first message

@compulim
Copy link
Contributor

compulim commented Oct 28, 2022

@beyackle2 Few things I changed:

  • Now, it renders <section role="feed"> if there are any contents
  • For tests, I am adding this DOM integrity test to verifyDOMIntegrity.js, which is a tiny version of axe-core
    • Ultimately, we should use axe-core and replace verifyDOMIntegrity.js (outlined in Add Microsoft FastPass or axe-core to our test suite #3716)
    • However, axe-core is using a non-MIT license so we need to do something extra before using it
    • Thus, we have the tiny verifyDOMIntegrity.js as a stopgap for now

Please let me know if it looks good to you.

@compulim compulim changed the title fix ARIA role of transcript when connection is pending Fix ARIA role of transcript when connection is pending Oct 28, 2022
@compulim compulim added this to the release-4.15.5 milestone Oct 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Accessibility] WCAG 1.3.1: Ensures elements with an ARIA role that require child roles contain them
4 participants