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

Dmirano/4551 ffp screen reader bug #4640

Merged
merged 9 commits into from Mar 30, 2023

Conversation

mirano-darren
Copy link
Contributor

@mirano-darren mirano-darren commented Mar 27, 2023

Resolves #4551, #4405

Description

Resolves screen reader bug reading out each choice last. The problem was because of the aria components exclusive to the cms design system's ChoiceList component. The solution was to create our own ChoiceList wrapper, and removing these aria tags from the components. Also replaced everywhere we use the ChoiseList component, which resolves #4405 as well.

Chromatic Link

https://www.chromatic.com/build?appId=61d5b948cf6f17003a12bf77&number=1319

Significant changes or possible side effects

Automated test cases written

Given When Then Type (jest, tap, cypress)
ChoiceList component selected or not selected component renders correctly jest
ChoiceList component component is rendered component does not contain aria-live tags jest

Steps to manually verify the bug fix (windows only)

  1. Navigate to the Budget and FFP page, ffp split field
  2. Open up a screen reader (can't be the apple screen reader since the bug doesn't happen)
  3. Verify the choices get read out loud first

Steps to manually verify the ChoiceList changes

  1. Verify ChoiceList components work as before
  2. ChoiceList fields affected:
  • Update Type fields (MMIS)
  • Medicaid Business Areas (MMIS)
  • Budget and FFP page: ffp split field (MMIS)
  • Assurances and Compliance
  1. Also verify ChoiceList story is correct

This pull request is ready to code review when

  • Automated tests are updated (and all tests are passing)
  • New automated test cases are documented above
  • Chromatic link added above
  • Pull request has been labeled, if applicable with feature, content, bug,
    tests, refactor
  • Associated OpenAPI documentation has been updated
  • The experience passes a basic manual accessibility audit (keyboard nav,
    screenreader, text scaling) OR an exemption is documented

This pull request is ready to test when

  • Code has been reviewed by someone other than the original author

This pull request is ready to review when QA has

  • Verified the functionality related to the change
  • Verified that the change works with Narrator on Windows
  • Verified that the change works with VoiceOver on Mac
  • Verified all updated pages with the WAVE tool
  • Verified tab and keyboard navigation functionality

This pull request can be merged when

  • Design has approved the experience
  • Product has approved the experience

@mirano-darren mirano-darren added the maintenance PR label for code maintenance and misc changes label Mar 27, 2023
@codecov
Copy link

codecov bot commented Mar 27, 2023

Codecov Report

Merging #4640 (35c3f8e) into main (fd80377) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #4640   +/-   ##
=======================================
  Coverage   94.34%   94.35%           
=======================================
  Files         279      280    +1     
  Lines        8864     8873    +9     
  Branches     1787     1787           
=======================================
+ Hits         8363     8372    +9     
  Misses        477      477           
  Partials       24       24           
Flag Coverage Δ
api ∅ <ø> (∅)
common 99.33% <ø> (ø)
web 94.08% <100.00%> (+<0.01%) ⬆️
Impacted Files Coverage Δ
web/src/components/ApdUpdate.js 97.22% <ø> (ø)
web/src/components/MedicaidBusinessAreas.js 100.00% <ø> (ø)
.../src/pages/apd/activities/ffp/MatchRateSelector.js 100.00% <ø> (ø)
...pages/apd/activities/overview/FundingSourceForm.js 95.00% <ø> (ø)
...rc/pages/apd/apd-overview/ApdOverviewMMISFields.js 100.00% <ø> (ø)
...surances-and-compliance/AssurancesAndCompliance.js 87.67% <ø> (ø)
web/src/pages/apd/new/HitechUpdateStatus.js 100.00% <ø> (ø)
web/src/pages/apd/new/MmisUpdateStatus.js 100.00% <ø> (ø)
web/src/pages/login/SelectAffiliation.js 97.29% <ø> (ø)
web/src/components/ChoiceList.js 100.00% <100.00%> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fd80377...35c3f8e. Read the comment docs.

@cms-eapd-bot
Copy link

cms-eapd-bot commented Mar 27, 2023

This deploy was cleaned up.

Copy link
Contributor

@thetif thetif left a comment

Choose a reason for hiding this comment

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

great work!

@mirano-darren mirano-darren added bug needs triage, then squashing and removed maintenance PR label for code maintenance and misc changes labels Mar 28, 2023
Copy link
Contributor

@tbolt tbolt left a comment

Choose a reason for hiding this comment

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

Nice solution wrapping the design system component 👍

Copy link
Contributor

@amyd11 amyd11 left a comment

Choose a reason for hiding this comment

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

I wasn't able to manually test this with my mac since I wasn't able to replicate the bug with the ChromeVox screen reader, but all the code changes you made look good! Nice job

@jeromeleecms jeromeleecms added the sev3 Experience is flawed but not unusable. label Mar 28, 2023
Copy link
Contributor

@jeromeleecms jeromeleecms left a comment

Choose a reason for hiding this comment

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

This "sounds" great! Much easier to follow.

@mirano-darren mirano-darren merged commit 734a081 into main Mar 30, 2023
12 checks passed
@mirano-darren mirano-darren deleted the dmirano/4551-ffp-screen-reader-bug branch March 30, 2023 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug needs triage, then squashing sev3 Experience is flawed but not unusable.
Projects
None yet
6 participants