-
Notifications
You must be signed in to change notification settings - Fork 148
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
USWDS-Site - Fix JAWS accordion button readout #2557
Conversation
- This aligns site code with default component code - This fixes a readout issue in JAWS
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note
The fix in this PR should remove the "Exception" from the status for these test items. Because of that, we can also remove the status_details and github issue details.
css/_uswds-theme-custom-styles.scss
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note
Updating the markup structure caused the original CSS to break. Created the site-accordion-heading as a more reliable method for styling these buttons.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3/25: confirmed with Amy L and Amy C that accordion in preview link read out "expanded" and "collapsed" when using the spacebar and enter key.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking great! Just a few places that are missing the accordion heading classes and then we should be good to go 👍
- Confirm that VoiceOver read out either "Expanded" or "Collapsed" for the accordion buttons, depending on state
- Confirm that there are no visual or functional regressions on any "Component preview", "Component code", or "Pattern preview" accordion buttons
- Confirm that the header and
usa-accordion__buttonHTML structure matches the default USWDS component code - On the [accordion component page](https://federalist-ead78f8d-8948-417c-a957-c21ec5617a57.sites.pages.cloud.gov/preview/uswds/uswds-site/al-jaws-component-accordion/components/accordion/), confirm that the accordion buttons inside the component preview are gray and not blue.
- Confirm that there are no instances of header elements inside
usa-accordion__buttonelements on the site - Confirm the copy in the changelog entry is accurate and free from error.
- Code changes are meaningful and free from error.
- No perceived visual regressions throughout component pages.
- Address page missing accordion heading classes
- Gender identity and sex page missing accordion heading classes
Co-authored-by: mahoneycm <charlie.mahoney@bixal.com>
Co-authored-by: mahoneycm <charlie.mahoney@bixal.com>
|
Thanks for catching those missing header classes, @mahoneycm. This is ready for your re-review. |
Co-authored-by: James Mejia <james.mejia@gsa.gov>
|
@mejiaj Thanks for flagging that this was missing the button type. The |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm! Previous comment resolved
- No perceived visual regressions throughout component pages.
- Fix formatting inconsistencies - Consistent links - Conssitent use of strong elements
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
Shouldn't these all have an updated "Latest updates," then? @mejiaj |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| @@ -2,6 +2,12 @@ title: Accordion accessibility tests | |||
| type: component | |||
| changelogURL: | |||
| items: | |||
| - date: NNNN-NN-NN | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| - date: NNNN-NN-NN | |
| - date: 2024-03-29 |
|
@annepetersen the changes from We could note the change to the accordions though 🤔 — what do you think @amyleadem? |
|
@mejiaj Ah, fair enough! I had this stuck in my memory as one where we had listed Known issues publicly (without looking). Nevermind then |
Summary
usa-accordion__buttonout of the element and instead wrapped the buttons with the headers. This aligns the code structure with the accordion code in USWDS and fixed an issue with the readout of the accordion button in JAWS.site-accordion-headingclass. This allows us to reliably style code/preview accordion buttons.Important
We should update the changelog date before merge
Related issue
Closes #2368
Preview link
Problem statement
The audible announcement of the "expanded" and "collapsed" state of the accordions on the component pages says "to activate press enter". It Should say "Expanded" or "collapsed".
Here is a screen recording of the accordion in use on a component pages vs. the accordion example.
This issue is only evident in JAWS. It announces correctly in NVDA.
Solution
Updating the structure surrounding
usa-accordion__buttonto match the structure in USWDS fixes the JAWS readout.Changes made
usa-accordion__buttonwith the header element, rather than placing the header inside the buttonusa-accordion__headingclass for consistency with USWDSsite-accordion-headingto reliably add styles for the site's preview and code accordions WITHOUT affecting the component preview for accordionBefore/after
Testing and review
Accessibility checks
Dev checks
usa-accordion__buttonHTML structure matches the default USWDS component codetype="buttonusa-accordion__buttonelements on the site