Skip to content
This repository has been archived by the owner on May 24, 2024. It is now read-only.

[terra-demographics-banner] Fix improper HTML, console warnings, and add missing translations #3828

Merged
merged 2 commits into from
Jun 27, 2023

Conversation

smason0
Copy link
Contributor

@smason0 smason0 commented Jun 22, 2023

Summary

This is a defect fix involving improper HTML, console warnings, and missing translations for code that was added as part of an accessibility change with #3796.

What was changed:

  1. Fixed issue where more than one div element was between a dl and its child dt/dd pairs. There is now only one div layer between these elements.
  2. Fixed console warnings around the recently added ageTitle and genderTitle internal props.
  3. Added missing translations for non-English locales. Also deleted Arabic and Finnish locales, as those are not currently supported by Terra.

Why it was changed:

  1. The introduction of nested divs with the parent having role="presentation" was was found to functionally work as expected with both JAWS and Voiceover, but created test failures when running Webdriver.io tests and checking for accessibility violations via the axe library. This change should fix any violations, as we are now following proper HTML standards.
  2. Console warnings were causing test/build failures in consuming apps.
  3. We must include translations for all supported locales.

Testing

This change was tested using:

  • WDIO
  • Jest
  • Visual testing (please attach a screenshot or recording)
  • Other (please describe below)
  • No tests are needed

Tested with both JAWS and VoiceOver.

Reviews

In addition to engineering reviews, this PR needs:

  • UX review
  • Accessibility review
  • Functional review

Additional Details

This PR resolves:

UXPLATFORM-9199
UXPLATFORM-9160


Thank you for contributing to Terra.
@cerner/terra

@smason0 smason0 self-assigned this Jun 22, 2023
@github-actions github-actions bot temporarily deployed to preview-pr-3828 June 22, 2023 19:01 Destroyed
@smason0 smason0 marked this pull request as ready for review June 22, 2023 19:23
@smason0 smason0 requested a review from a team as a code owner June 22, 2023 19:23
@smason0 smason0 changed the title Split large banner into two lists + translations [terra-demographics-banner] Fix improper HTML, console warnings, and add missing translations Jun 22, 2023
@@ -30,6 +30,8 @@ export default (props) => {
postMenstrualAgeFullText,
postMenstrualAgeLabel,
preferredFirstName,
ageTitle,
genderTitle,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding these here and in the small view resolve the console warnings.

@mjpalazzo mjpalazzo added ⭐ Accessibility Reviewed Accessibility has been reviewed and approved. and removed Accessibility Review Required labels Jun 26, 2023
@mjpalazzo
Copy link
Contributor

Great work, Steven! JAWS announces the elements of the demographic banner very nicely. I used the Down Arrow to navigate from the Patient Name, which is announced as an H2, to the list of identifying data items. JAWS announced the label and value for each as I used the Down Arrow to go down the list. The other data items in the banner were grouped in a separate list for the full banner examples. For the compact banner, the demographic items and the other items were grouped and announced as 1 list.

@smason0
Copy link
Contributor Author

smason0 commented Jun 26, 2023

Great work, Steven! JAWS announces the elements of the demographic banner very nicely. I used the Down Arrow to navigate from the Patient Name, which is announced as an H2, to the list of identifying data items. JAWS announced the label and value for each as I used the Down Arrow to go down the list. The other data items in the banner were grouped in a separate list for the full banner examples. For the compact banner, the demographic items and the other items were grouped and announced as 1 list.

Thanks for confirming! What you describe is the expected behavior. The large banner now will group the list items based on the visual presentation, as two separate lists. The small view displays the identifiers as a single list, and they are announced as one list. Although the original goal with this fix was to make both large and small banners read as one list or as two, I was unable to technically achieve this without creating visual changes. Therefore, we are maintaining this subtle inconsistency, while also following the WCAG standards.

@adoroshk adoroshk merged commit d5ebfe3 into main Jun 27, 2023
@adoroshk adoroshk deleted the UXPLATFORM-9199 branch June 27, 2023 16:09
@sdadn sdadn added ⭐ UX Reviewed UX Reviewed and approved. and removed UX Review Required labels Jun 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug-fix 📦 terra-demographics-banner ⭐ Accessibility Reviewed Accessibility has been reviewed and approved. ⭐ UX Reviewed UX Reviewed and approved.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants