ActionList: Allow items to remain focusable when disabled

[TylerJDev]

Allows disabled items in ActionMenu and SelectPanel to remain focusable. This stems from feedback we've received from the Accessibility Team.

Changelog

Integration test PR: https://github.com/github/github/pull/324056 Passes with small modifications (minus flakey tests), requires only one change to a test in Dotcom.

Changed

• Allows ActionList.Item to keep focus if it's within an ActionMenu or SelectPanel context.

Rollout strategy

• [x] Patch release
• [ ] Minor release
• [ ] Major release; if selected, include a written rollout or migration plan
• [ ] None; if selected, include a brief description as to why

Testing & Reviewing

• ActionMenu example
• SelectPanel example (select 3 options for remaining items to be disabled)
• ActionList example (disabled items should not receive focus)

Merge checklist

• [ ] Added/updated tests
• [ ] Added/updated documentation
• [x] Added/updated previews (Storybook)
• [ ] Changes are SSR compatible
• [x] Tested in Chrome
• [ ] Tested in Firefox
• [ ] Tested in Safari
• [ ] Tested in Edge
• [ ] (GitHub staff only) Integration tests pass at github/github (Learn more about how to run integration tests)

[changeset-bot[bot]]

🦋 Changeset detected

Latest commit: cb5468924cae138a3a37290777686959329dc825

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	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

[github-actions[bot]]

size-limit report 📦

Path	Size
packages/react/dist/browser.esm.js	95.91 KB (+0.12% 🔺)
packages/react/dist/browser.umd.js	96.24 KB (+0.05% 🔺)

[TylerJDev]

Should require only one change in Dotcom (a modification to a test), other than that it appears that the tests related to this change pass.

[siddharthkp]

Should require only one change in Dotcom (a modification to a test), other than that it appears that the tests related to this change pass.

That's great!

It looks like we can only make the dotcom change after merging this PR, correct?

[TylerJDev]

@siddharthkp, yup that should be it!

[TylerJDev]

This PR should be ready for a review!

cc: @primer/engineer-reviewers

[siddharthkp]

Hi @TylerJDev,

How do you feel about this PR now that inactive elements are released: Docs | Story

Is focusing a disabled item without feedback still the way to go OR should we replace those instances with inactiveText instead?

[TylerJDev]

How do you feel about this PR now that inactive elements are released: Docs | Story

Is focusing a disabled item without feedback still the way to go OR should we replace those instances with inactiveText instead?

We'd want to keep the two focusable since inactive items can still trigger actions, unlike disabled items. We're mainly making disabled items focusable because they are still present in the accessibility tree even though they are disabled, so if a user goes into a menu that has 8 items including disabled items, they'll know which item(s) are disabled.

[primer-integration[bot]]

:wave: Hi from github/github! Your integration PR is ready: https://github.com/github/github/pull/336021

[siddharthkp]

inactive items can still trigger actions

Oh! Totally didn't expect that!