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

ActionList: Allow items to remain focusable when disabled #4481

Merged
merged 15 commits into from
Feb 7, 2025

Conversation

TylerJDev
Copy link
Member

@TylerJDev TylerJDev commented Apr 8, 2024

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

  • 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

Merge checklist

Sorry, something went wrong.

Verified

This commit was signed with the committer’s verified signature.
mskelton Mark Skelton
… `disabled`
Copy link

changeset-bot bot commented Apr 8, 2024

🦋 Changeset detected

Latest commit: 09101fa

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

Copy link
Contributor

github-actions bot commented Apr 8, 2024

size-limit report 📦

Path Size
packages/react/dist/browser.esm.js 105.6 KB (+0.08% 🔺)
packages/react/dist/browser.umd.js 106.08 KB (+0.17% 🔺)

@github-actions github-actions bot temporarily deployed to storybook-preview-4481 April 8, 2024 18:01 Inactive
@TylerJDev TylerJDev marked this pull request as ready for review April 9, 2024 15:00
@TylerJDev TylerJDev requested a review from a team as a code owner April 9, 2024 15:00
@TylerJDev TylerJDev requested a review from siddharthkp April 9, 2024 15:00
@siddharthkp siddharthkp added the integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label Apr 9, 2024

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
@github-actions github-actions bot temporarily deployed to storybook-preview-4481 April 11, 2024 13:33 Inactive

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
@github-actions github-actions bot temporarily deployed to storybook-preview-4481 April 24, 2024 14:52 Inactive

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
@github-actions github-actions bot temporarily deployed to storybook-preview-4481 May 8, 2024 11:03 Inactive
@TylerJDev
Copy link
Member Author

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

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
Copy link
Member Author

@siddharthkp, yup that should be it!

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
@github-actions github-actions bot temporarily deployed to storybook-preview-4481 June 21, 2024 17:48 Inactive

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
@github-actions github-actions bot temporarily deployed to storybook-preview-4481 July 12, 2024 20:49 Inactive
@TylerJDev
Copy link
Member Author

This PR should be ready for a review!

cc: @primer/engineer-reviewers

@siddharthkp siddharthkp changed the title Allow items to remain focusable when disabled ActionList: Allow items to remain focusable when disabled Jul 24, 2024
@TylerJDev TylerJDev removed the Stale label Jan 6, 2025

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
@github-actions github-actions bot temporarily deployed to storybook-preview-4481 January 8, 2025 15:49 Inactive

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
@github-actions github-actions bot temporarily deployed to storybook-preview-4481 January 9, 2025 14:01 Inactive
@github-actions github-actions bot added integration-tests: failing Changes in this PR cause breaking changes in gh/gh and removed integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm labels Jan 9, 2025
@siddharthkp siddharthkp removed their request for review January 9, 2025 15:37

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
@github-actions github-actions bot temporarily deployed to storybook-preview-4481 January 28, 2025 14:54 Inactive
@primer-integration
Copy link

👋 Hi from github/github! Your integration PR is ready: https://github.com/github/github/pull/359651

@TylerJDev TylerJDev requested review from a team and mperrotti January 28, 2025 16:13
@primer-integration
Copy link

🔴 golden-jobs completed with status failure.

@TylerJDev TylerJDev enabled auto-merge February 7, 2025 00:54
@TylerJDev TylerJDev requested a review from siddharthkp February 7, 2025 00:55
@TylerJDev TylerJDev dismissed siddharthkp’s stale review February 7, 2025 00:55

No longer needed :)

@TylerJDev TylerJDev disabled auto-merge February 7, 2025 00:55

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
@TylerJDev TylerJDev added this pull request to the merge queue Feb 7, 2025
Merged via the queue into main with commit 6e7da1e Feb 7, 2025
44 checks passed
@TylerJDev TylerJDev deleted the tylerjdev/action-menu-disabled-items branch February 7, 2025 01:45
@primer primer bot mentioned this pull request Feb 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integration-tests: failing Changes in this PR cause breaking changes in gh/gh
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants