react icon indicating copy to clipboard operation
react copied to clipboard

PageHeader: Address dom order issues (screen reader experience feedback from sign-off)

Open broccolinisoup opened this issue 1 year ago • 2 comments

Copied from https://github.com/primer/react/pull/3816 (due to merge conflicts)

As a part of addressing accessibility sign-off review comments https://github.com/github/primer/issues/1115#issuecomment-1499501472, this PR updates the stories to ensure the order of the elements are the following;

  1. PageHeader.Title
  2. ContextArea (all elements)
  3. PageHeader.LeadingAction
  4. PageHeader.TrailingAction
  5. PageHeader.Actions

instead of

  1. ContextArea (all elements)
  2. PageHeader.LeadingAction
  3. PageHeader.Title
  4. PageHeader.TrailingAction
  5. PageHeader.Actions

The motivation behind is to make sure the actions on the page that are displayed above or before the main heading (Context area actions and leading action) should come after the main title (PageHeader.Title). With this way, we make sure screen reader users who navigate through heading menus don't miss any actions.

While we updated the dom order in the stories and this is how we will recommend to order the elements, we made sure everything, visually stays the same. (Leveraging CSS grid layout)

sign-off review comment reference

Changelog

New

Changed

  • Layout of the Root element from flex to grid
  • The order of PageHeader's sub components in the stories.

Removed

Rollout strategy

This is technically a breaking change but PageHeader is still a draft component, so we are good to realise this as a patch. There is one instance at github/github to be updated and I'll list the commit here to be included in the release PR.

  • [x] Patch release
  • [ ] Minor release
  • [ ] Major release; if selected, include a written rollout or migration plan

Testing & Reviewing

Merge checklist

  • [ ] Added/updated tests
  • [ ] Added/updated documentation (After I get 👍🏻 on the solution and the direction, I'll update the docs)
  • [ ] Added/updated previews (Storybook)
  • [ ] Changes are SSR compatible
  • [x] Tested in Chrome
  • [ ] Tested in Firefox
  • [ ] Tested in Safari
  • [ ] Tested in Edge

Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.

broccolinisoup avatar Mar 06 '24 01:03 broccolinisoup

🦋 Changeset detected

Latest commit: 7bb732d7024edb4474aef7404ff4e1e29d942a78

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

This PR includes changesets to release 1 package
Name Type
@primer/react Minor

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

changeset-bot[bot] avatar Mar 06 '24 01:03 changeset-bot[bot]

size-limit report 📦

Path Size
packages/react/dist/browser.esm.js 89.34 KB (0%)
packages/react/dist/browser.umd.js 89.68 KB (0%)

github-actions[bot] avatar Mar 06 '24 01:03 github-actions[bot]