Skip to content

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

Merged
thisisdano merged 18 commits into
uswds:developfrom
mahoneycm:cm-button-group-a11y
May 10, 2023
Merged

USWDS - Accessibility: Added button functionality to usa-button links#4385
thisisdano merged 18 commits into
uswds:developfrom
mahoneycm:cm-button-group-a11y

Conversation

@mahoneycm

@mahoneycm mahoneycm commented Nov 4, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

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 ↩

@mahoneycm mahoneycm mentioned this pull request Nov 4, 2021
Closed
4 tasks
mejiaj
mejiaj suggested changes Nov 17, 2021
View reviewed changes

@mejiaj mejiaj left a comment

Copy link
Copy Markdown
Contributor

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?

Comment thread src/components/11-button-groups/button-groups.njk Outdated
Comment thread src/components/card/_card--basic.njk Outdated
mejiaj
mejiaj suggested changes Dec 3, 2021
View reviewed changes

@mejiaj mejiaj left a comment

Copy link
Copy Markdown
Contributor

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?

Comment thread src/components/card/card--standard.njk Outdated
Comment thread src/js/components/button.js Outdated
Comment thread src/js/components/button.js Outdated
Comment thread src/js/components/button.js Outdated
Comment thread src/js/components/button.js Outdated
@mahoneycm

mahoneycm commented Jan 21, 2022

Copy link
Copy Markdown
Contributor Author

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

@JackRyan1989

JackRyan1989 commented Mar 1, 2023

Copy link
Copy Markdown

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

mahoneycm commented Mar 14, 2023

Copy link
Copy Markdown
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

mejiaj
mejiaj approved these changes Mar 31, 2023
View reviewed changes

@mejiaj mejiaj left a comment

Copy link
Copy Markdown
Contributor

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.

@mejiaj mejiaj requested review from amyleadem and thisisdano March 31, 2023 13:55
@aduth aduth mentioned this pull request Mar 31, 2023
Merged
3 tasks
amyleadem
amyleadem suggested changes Apr 19, 2023
View reviewed changes

@amyleadem amyleadem left a comment

Copy link
Copy Markdown
Contributor

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

mahoneycm commented Apr 20, 2023

Copy link
Copy Markdown
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?

amyleadem
amyleadem approved these changes Apr 21, 2023
View reviewed changes

@amyleadem amyleadem left a comment

Copy link
Copy Markdown
Contributor

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 mahoneycm mentioned this pull request Apr 26, 2023
Closed
@mahoneycm mahoneycm mentioned this pull request May 3, 2023
Closed
@mahoneycm

mahoneycm commented May 3, 2023

Copy link
Copy Markdown
Contributor Author

Added changelog PR uswds/uswds-site#2085

@thisisdano thisisdano mentioned this pull request May 8, 2023
Merged
24 tasks
@thisisdano thisisdano merged commit ee90eb4 into uswds:develop May 10, 2023
@thisisdano thisisdano mentioned this pull request Jun 6, 2023
Merged
@mahoneycm mahoneycm mentioned this pull request Aug 14, 2023
Merged
4 tasks
@werdnanoslen werdnanoslen mentioned this pull request Jan 5, 2024
Closed
@aduth aduth mentioned this pull request Mar 1, 2024
Merged
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

3 more reviewers
@mejiaj mejiaj mejiaj approved these changes
@thisisdano thisisdano thisisdano approved these changes
@amyleadem amyleadem amyleadem approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Buttons: allow links that look like buttons to be triggered

5 participants

@mahoneycm @JackRyan1989 @mejiaj @thisisdano @amyleadem