Fixes for `ActionList` semantics

[TylerJDev]

Fixes issue in: https://github.com/github/accessibility-audits/issues/2939

Based off of @broccolinisoup's work in https://github.com/primer/react/pull/3971

Ensures that the default semantics of ActionList.Item rely on <button> instead of <li tabindex="0">. If the ActionList itself should act as a list or menu, <li> will be used with the appropriate roles.

Changed

• Default element type of ActionList.Item is now button
• Minor type changes (i.e. HTMLLIElement to HTMLButtonElement)
• Small ARIA addition to NavList, AutocompleteSuggestions.

Removed

Rollout strategy

• [ ] Patch release
• [x] 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

Integration test PR: https://github.com/github/github/pull/317139

Merge checklist

• [ ] Added/updated tests
• [ ] Added/updated documentation
• [ ] 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: 0298349b0d416915214daa5ba79cfe7ae20864c9

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

[github-actions[bot]]

size-limit report 📦

Path	Size
packages/react/dist/browser.esm.js	89.89 KB (+0.3% 🔺)
packages/react/dist/browser.umd.js	90.2 KB (+0.31% 🔺)

[TylerJDev]

@broccolinisoup, I would love your review on this PR while it's still in draft state. I started from your PR and tried to shape it in a way that would be (mostly) backwards compatible with existing versions, so I'd love any additional input you might have.

[TylerJDev]

Wanted to loop back here to plan a potential rollout. Since this PR consists of a change that would revamp the semantics of ActionList by default, it could be potentially breaking. Here are some ideas I had:

Make new semantics opt-in via a prop

Essentially, we could remove the logic that checks to see if the ActionList is a certain ARIA role (e.g. role="list", role="menu"), and instead give access to the new semantics via a prop that would then render the ActionList with the buttons rather than just list items (e.g. renderButtonItems). This allows existing implementations that are inaccessible due to the current semantics to change to the more accessible markup via the prop.

We could then aim to make the semantics available by default without the prop, potentially in our next major release.

Add prop to all existing ActionList components that disables the new semantics

This would be similar to the rollout of Tooltip v2. The aim would be to slowly reduce the amount of usage of the old semantics, while any new implementations of ActionList would use the new semantics by default. We could slowly enforce usage of the new semantics overtime, by encouraging consumers of ActionList to remove the prop and rely on the new semantics.

Release new semantics as-is

The goal for this PR is to be backwards compatible with existing implementations, meaning that any current ActionList now, should perform the same with the new semantics. It further reduces the amount of risk by limiting what type of ActionList receives the new semantics (i.e. some ARIA roles won't receive the new semantics). We could further gain confidence by testing existing ActionList implementations across dotcom, (there are numerous occurrences, so we wouldn't' be able to test all of them).

I'm open to any additional ideas, or thoughts :grin:

[TylerJDev]

Switching this PR to "ready for review". Now that we've added feature flags, I believe we can smoothly release this change and monitor it 🥳

I've tested briefly in Dotcom and it appears that some of the implementations that this PR would touch work with no issues!

[TylerJDev]

@siddharthkp, @broccolinisoup - Hey y'all! I think this is in a good state for a review! I addressed all of the feedback and we should only need the changes listed in this comment to roll this out in Dotcom (mainly typescript adjustments). Would love one last review from y'all ❤️