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

docs(progress-bar-expressive): fix noisy status updates on docs page #2312

Merged
merged 5 commits into from
Apr 15, 2024

Conversation

cordeliadillon
Copy link
Contributor

Fixes #2311

  • This PR contains CSS changes
  • This PR does not contain CSS changes

Description

The message elements with role="status" in the expressive progress bar doc examples are reading out constantly when perusing the Skin docs with a screen reader. This update sets aria-live="off" on the rendered examples (but not on the rendered code snippets) so as not to be so disruptive when someone is reading the Skin docs.

Notes

I opted to use aria-live="off" rather than removing role="status", since we use the status role as a selector in the main.js file. Setting an explicit aria-live="off" overrides status's implicit aria-live="polite".

Screenshots

N/A

Checklist

  • I verify the build is in a non-broken state
  • I verify all changes are within scope of the linked issue

Copy link

changeset-bot bot commented Apr 12, 2024

🦋 Changeset detected

Latest commit: 0dc0357

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@ebay/skin Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

@saiponnada saiponnada left a comment

Choose a reason for hiding this comment

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

Looks good to me. One minor suggestion. It's better to add a note to docs on why we added aria-live="off" so that it might be clear to someone reading these understand, why role=status is not announcing the messages

Copy link
Contributor

@agliga agliga left a comment

Choose a reason for hiding this comment

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

LGTM overall

However I see some docs files were changed. Usually those should not be changed on docs updates. You might need to clean your node_modules and update from 17.4.0

@cordeliadillon
Copy link
Contributor Author

Looks good to me. One minor suggestion. It's better to add a note to docs on why we added aria-live="off" so that it might be clear to someone reading these understand, why role=status is not announcing the messages

Great suggestion, @saiponnada! I updated the docs and re-requested your review. :)

Copy link
Contributor

@saiponnada saiponnada left a comment

Choose a reason for hiding this comment

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

Perfect!

@cordeliadillon
Copy link
Contributor Author

@saiponnada @agliga, thanks for the re-review! Feel free to merge when ready. (I don't have merge access.)

@saiponnada saiponnada merged commit 913f85b into 17.4.0 Apr 15, 2024
2 checks passed
@saiponnada saiponnada deleted the 2311-status branch April 15, 2024 22:40
@github-actions github-actions bot mentioned this pull request Apr 16, 2024
@github-actions github-actions bot mentioned this pull request Apr 29, 2024
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.

progress-bar-expressive: expressive progress bar reading aloud too much in docs
4 participants