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

Action list a11y #1828

Merged
merged 19 commits into from
Feb 28, 2023
Merged

Action list a11y #1828

merged 19 commits into from
Feb 28, 2023

Conversation

camertron
Copy link
Contributor

@camertron camertron commented Feb 15, 2023

Description

This PR addresses accessibility feedback for the NavList and ActionList components as identified in https://github.com/github/primer/issues/1120. Namely it:

  1. renders much clearer markup. Several unnecessary <ul>s have been removed and separators are now <div>s instead of <li>s
  2. adjusts a number of roles
  3. allows customizing the heading level
  4. adds aria-labelledby to sub-list <ul>s. If a heading and an aria-label are both provided, an error is now raised.
  5. renames Section -> Group to align better with the React implementation
  6. removes the show_on_hover attribute from ActionList and NavList items, as only showing buttons on hover is not an accessible pattern The show_on_hover option has been left in. We will address it (if necessary) in a future PR.
  7. ensures sub-lists collapse when the user presses escape
  8. updates the NavList's trailing action preview so that the buttons actually do something
  9. ensures NavLists are wrapped with the <nav> tag

Integration

Does this change require any updates to code in production?

Yes. Since the sections slot has been renamed to groups, any place in dotcom where we're calling with_section will need to change to with_group. In addition, we will need to remove aria-labels from any section/group that also contains a header.

Merge checklist

  • Added/updated tests
  • Added/updated documentation
  • Added/updated previews

@changeset-bot
Copy link

changeset-bot bot commented Feb 15, 2023

🦋 Changeset detected

Latest commit: 7372a32

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@primer/view-components Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@camertron camertron temporarily deployed to review-pr-1828 February 15, 2023 00:31 — with GitHub Actions Inactive
@github-actions github-actions bot added the ruby Pull requests that update Ruby code label Feb 15, 2023
@camertron camertron temporarily deployed to review-pr-1828 February 15, 2023 04:26 — with GitHub Actions Inactive
@camertron camertron temporarily deployed to review-pr-1828 February 15, 2023 05:14 — with GitHub Actions Inactive
@camertron camertron temporarily deployed to github-pages February 15, 2023 05:18 — with GitHub Actions Inactive
@radglob radglob temporarily deployed to review-pr-1828 February 16, 2023 15:54 — with GitHub Actions Inactive
@github-actions github-actions bot added the javascript Pull requests that update Javascript code label Feb 16, 2023
@radglob radglob temporarily deployed to github-pages February 16, 2023 15:58 — with GitHub Actions Inactive
@radglob radglob temporarily deployed to review-pr-1828 February 16, 2023 16:05 — with GitHub Actions Inactive
@radglob radglob temporarily deployed to github-pages February 16, 2023 16:09 — with GitHub Actions Inactive
@radglob radglob temporarily deployed to review-pr-1828 February 17, 2023 15:46 — with GitHub Actions Inactive
@radglob radglob temporarily deployed to github-pages February 17, 2023 15:50 — with GitHub Actions Inactive
@camertron camertron marked this pull request as ready for review February 17, 2023 18:49
@camertron camertron requested review from a team and keithamus February 17, 2023 18:49
@camertron camertron temporarily deployed to github-pages February 17, 2023 23:59 — with GitHub Actions Inactive
@camertron camertron temporarily deployed to preview February 21, 2023 23:54 — with GitHub Actions Inactive
@camertron camertron temporarily deployed to github-pages February 22, 2023 00:00 — with GitHub Actions Inactive
@camertron camertron temporarily deployed to preview February 23, 2023 05:26 — with GitHub Actions Inactive
@camertron camertron temporarily deployed to github-pages February 23, 2023 05:30 — with GitHub Actions Inactive
Copy link
Contributor

@lindseywild lindseywild left a comment

Choose a reason for hiding this comment

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

This is awesome 🤩 Just a few suggestions!

@camertron camertron temporarily deployed to preview February 24, 2023 19:31 — with GitHub Actions Inactive
@camertron camertron temporarily deployed to github-pages February 24, 2023 19:34 — with GitHub Actions Inactive
Copy link
Contributor

@langermank langermank left a comment

Choose a reason for hiding this comment

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

Looks great @camertron !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
javascript Pull requests that update Javascript code ruby Pull requests that update Ruby code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants