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

crayons-notice accessibility improvements #13273

Conversation

Link2Twenty
Copy link
Contributor

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix (A11y)
  • Optimization
  • Documentation Update

Description

Update crayons-notice where it's used to have a role and/or aria-live

Related Tickets & Documents

closes #12961

QA Instructions, Screenshots, Recordings

N/A

UI accessibility concerns?

N/A

Added tests?

  • Yes
  • No, and this is why: old tests should still work
  • I need help with writing tests

[Forem core team only] How will this change be communicated?

N/A

[optional] Are there any post deployment tasks we need to perform?

N/A

[optional] What gif best describes this PR or how it makes you feel?

Threading a needle

Link2Twenty and others added 23 commits April 12, 2021 11:50
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
Copy link
Contributor

@aitchiss aitchiss left a comment

Choose a reason for hiding this comment

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

Thanks for making those changes!

I think it would be good at some point for us to update the 'notes' section in storybook for the Notice component, to explain when to add aria-live="polite" and when to add role="alert" (https://storybook.forem.com/?path=/story/components-notices-html--default) - I've been trying to gradually add accessibility notes to components as we work on them 🙂

If you fancy a follow up PR feel free to go ahead an add that, or I can do a quick one at some point later on!

@pr-triage pr-triage bot added PR: partially-approved bot applied label for PR's where a single reviewer approves changes and removed PR: unreviewed bot applied label for PR's with no review labels Apr 14, 2021
@aitchiss aitchiss merged commit c9e4227 into forem:master Apr 14, 2021
@pr-triage pr-triage bot added PR: merged bot applied label for PR's that are merged and removed PR: partially-approved bot applied label for PR's where a single reviewer approves changes labels Apr 14, 2021
@Link2Twenty Link2Twenty deleted the Link2Twenty/crayons-notices-accessibility-improvements branch April 20, 2021 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: merged bot applied label for PR's that are merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Accessibility improvements to crayons notices
3 participants