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

USWDS - Accessibility: Added button functionality to usa-button links #4385

Merged
merged 18 commits into from May 10, 2023

Conversation

mahoneycm
Copy link
Contributor

@mahoneycm mahoneycm commented Nov 4, 2021

Summary

Added spacebar activation to links styled as buttons. This allows users to trigger these links the same way they would expect to trigger buttons. usa-card buttons have been replaced with links styled as buttons.

This PR has two main goals based on @thisisdano's comments in #3690:

  • Links that are styled as buttons should be triggered by the spacebar without using role="button" 1
  • usa-card buttons should be replaced with links 2

Breaking change

⚠️ This is a breaking change.

usa-card buttons have been replaced with links styled as buttons.

Before:

<div class="usa-card__footer">
    <button type="button" class="usa-button">Visit Florida Keys</button>
</div>

After:

<div class="usa-card__footer">
    <a href="#" class="usa-button">Visit Florida Keys</a>
</div>

Related issue

Closes #3690

Related PRs

Changelog PR →

Preview link

usa-card w/ links styled as buttons

Preview link:

Problem statement

Links styled as buttons should be triggered as such

Solution

Create a simple JS file to target all anchors with the usa-button class and allow them to be triggered with the spacebar

Replace buttons in usa-card with usa-button anchor tags

Testing and review

  1. Visit button-group storybook preview
  2. Turn on screen reader
  3. Tab to "Back" link
  4. Verify the screen reader reads it as a link
  5. Press spacebar key
  6. Verify that the link is followed
  7. Visit usa-card
  8. Verify that all buttons are now links with usa-button class
  9. Verify spacebar & enter button trigger the links
    • It may help to update the links href to verify the page updates

Before opening this PR, make sure you’ve done whichever of these applies to you:

  • Confirm that this code follows the 18F Front End Coding Style Guide and Accessibility Guide.
  • Run git pull origin [base branch] to pull in the most recent updates from your base and check for merge conflicts. (Often, the base branch is develop).
  • Run npm run prettier:js to format any JS updates.
  • Run npm test and confirm that all tests pass.

Additional information

  • Added href="#" to allow links to be targeted through tabbing.
  • Ran npm run build:html & npm run build:storybook for good measure.

Footnotes

  1. https://github.com/uswds/uswds/issues/3690#issuecomment-951091169

  2. https://github.com/uswds/uswds/issues/3690#issuecomment-951095032

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.

Thanks for updating this. Can you resolve conflicts and fix some formatting issues on href?

src/components/11-button-groups/button-groups.njk Outdated Show resolved Hide resolved
src/components/card/_card--basic.njk Outdated Show resolved Hide resolved
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.

@mahoneycm thanks for all your work on this.

Aside from the requested changes, would you mind creating a unit test for it under spec/unit?

src/components/card/card--standard.njk Outdated Show resolved Hide resolved
src/js/components/button.js Outdated Show resolved Hide resolved
src/js/components/button.js Outdated Show resolved Hide resolved
src/js/components/button.js Outdated Show resolved Hide resolved
src/js/components/button.js Outdated Show resolved Hide resolved
@mahoneycm
Copy link
Contributor Author

@thisisdano This was something I worked on a while back for accessibility. Remembered it was wondering if we should readdress?

@JackRyan1989
Copy link

Hi there! There's some interest from Login.gov in having this change implemented. Are there blockers or other considerations preventing this from being merged? Thanks

@mahoneycm mahoneycm requested a review from mejiaj March 14, 2023 15:54
@mahoneycm
Copy link
Contributor Author

@mejiaj Updated this PR for the latest USWDS! I also revised the PR description to make the work and problem more understandable.

This included Dan's request to change usa-card buttons to links but that causes the markup to change. If we want to separate that task, we can remove it from this PR

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.

LGTM, usually we require a unit test for every new script. I'm not sure on this one given the very limited scope.

Copy link
Contributor

@amyleadem amyleadem left a comment

Choose a reason for hiding this comment

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

Looks good!
Confirmed that:

  • links styled with .usa-button are activated by both space and enter
  • links are identified as links in VoiceOver

For good measure, I also searched for any <a> tags with type="button" and couldn't find any.

A couple small things:

  • Could you update the PR summary to match the changelog/release notes format? (Some details outlined here)
  • Could you add details to the "Breaking changes" section so that users will know exactly what markup change they'll need to do?
  • Also, this shouldn't need to block anything, but I'm wondering if we can make a story that explicitly demonstrates links styled as buttons. We have the example in the button-group story, but it is a bit hidden.

@mahoneycm
Copy link
Contributor Author

@amyleadem Updated the PR description with those two changes!

I agree, I would like to highlight links styled as buttons somewhere as well. Do you think that this should be part of this PR or should I open up a new ticket for that work?

Copy link
Contributor

@amyleadem amyleadem 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 updating the descriptions. I think it is fine to address the story as a follow-up. Will you open an issue so we don't lose track?

@mahoneycm
Copy link
Contributor Author

Added changelog PR uswds/uswds-site#2085

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.

Buttons: allow links that look like buttons to be triggered
5 participants