Skip to content

Accessibility updates [uswds-site 1227] #4197

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

Merged
merged 28 commits into from
May 17, 2021
Merged

Accessibility updates [uswds-site 1227] #4197

merged 28 commits into from
May 17, 2021

Conversation

thisisdano
Copy link
Contributor

@thisisdano thisisdano commented May 14, 2021

Summary

This adjusts some of the component code templates to allow the final output code to pass accessibility tests on our website

Requirements

This is a companion to uswds/uswds-site#1227
Approve and merge this PR first. Then link the uswds-site PR to the develop branch.

Details

See code comments

@thisisdano thisisdano changed the title Accessibility updates Accessibility updates [uswds-site 1227] May 17, 2021
@thisisdano thisisdano requested a review from mejiaj May 17, 2021 03:30
@@ -247,7 +247,7 @@ We recommend using a **minifier** like [csso](https://github.com/css/csso) to co

### Use another framework or package manager

If you’re using another framework or package manager that doesn’t support `npm`, you can find the source files in this repository and use them in your project. Otherwise, we recommend that you follow the [download instructions](#download). Please note that the core team [isn’t responsible for all frameworks’ implementations](https://github.com/uswds/uswds/issues/877).
If you’re using another framework or package manager that doesn’t support `npm`, you can find the source files in this repository and use them in your project. Otherwise, we recommend that you follow the [download instructions](#download-and-install). Please note that the core team [isn’t responsible for all frameworks’ implementations](https://github.com/uswds/uswds/issues/877).
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fix internal link

@@ -5,6 +5,7 @@ context:
banner:
text: "An official website of the United States government"
action: "Here’s how you know"
aria_label: "Official government website"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use explicit ARIA label metadata...

@@ -1,6 +1,6 @@
<ul class="usa-button-group">
<li class="usa-button-group__item">
<a href="#" class="usa-button usa-button--outline">Back</a>
<a href="javascript:void(0);" class="usa-button usa-button--outline">Back</a>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Avoid em empty hash links, use javascript:void(0); instead

Copy link
Contributor

Choose a reason for hiding this comment

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

Does this mean we have to update the href's in breadcrumb?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess so... I wonder why those weren't flagged as errors....

@@ -36,6 +38,8 @@ variants:
Un <strong>candado</strong> ($LOCK_IMG) o <strong>https://</strong> significa que usted se conectó de forma segura a un sitio web $TLD. Comparta información sensible sólo en sitios web oficiales y seguros.
- name: dot-mil
context:
banner:
aria_label: "Official government website,"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

... and use trailing commas as a hack to provide legible ARIA labels but still make them unique

@@ -1,21 +1,21 @@
<div class="usa-summary-box" role="complementary">
<div class="usa-summary-box">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Remove complementary role for content inside a main element

Copy link
Contributor

Choose a reason for hiding this comment

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

Does this appear as a region in Voiceover or do we need to add an aria-label?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does not have an explicit region now. I can give it one

<div class="usa-summary-box__body">
<h3 class="usa-summary-box__heading">Key information</h3>
<div class="usa-summary-box__text">
<ul class="usa-list">
<li>If you are under a winter storm warning,
<a class="usa-summary-box__link" href="#usa-anchor-find-shelter">find shelter</a>
<a class="usa-summary-box__link" href="javascript:void(0);">find shelter</a>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't use fake anchor links

@@ -1,4 +1,4 @@
<div class="usa-table-container--scrollable">
<div class="usa-table-container--scrollable" tabindex="0">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Assure that scrollable elements are focusable

@@ -1,6 +1,6 @@
<div class="grid-row grid-gap">
<div class="mobile-lg:grid-col-4">
<h6 class="margin-0">Unordered list</h6>
<h3 class="site-preview-heading margin-0">Unordered list</h3>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use proper heading level

</section>
</div>
<div class="usa-line-length-example">
<h3 class="site-preview-heading margin-top-0">Line length</h3>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use proper heading level and only include sample text within the usa-prose

@thisisdano thisisdano marked this pull request as ready for review May 17, 2021 04:42
Copy link
Contributor

@mejiaj mejiaj left a comment

Choose a reason for hiding this comment

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

A few questions on Identifier and Breadcrumb href. Everything else looks good.

@@ -1,6 +1,6 @@
<ul class="usa-button-group">
<li class="usa-button-group__item">
<a href="#" class="usa-button usa-button--outline">Back</a>
<a href="javascript:void(0);" class="usa-button usa-button--outline">Back</a>
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this mean we have to update the href's in breadcrumb?

@@ -1,21 +1,21 @@
<div class="usa-summary-box" role="complementary">
<div class="usa-summary-box">
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this appear as a region in Voiceover or do we need to add an aria-label?

@thisisdano thisisdano merged commit 2650f15 into develop May 17, 2021
@thisisdano thisisdano deleted the dw-site-pa11y branch May 17, 2021 21:26
mejiaj pushed a commit that referenced this pull request Jun 24, 2021
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.

2 participants