-
Notifications
You must be signed in to change notification settings - Fork 556
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 semantics re-introduce #3485
Conversation
|
size-limit report 📦
|
We are re-introducing the remediations iteratively and without any changes in the API. Please see my comment on the issue for more details and the active PRs. https://github.com/github/accessibility-audits/issues/2939#issuecomment-1635386101 Therefore, I am closing this PR. |
Context
ActionList semantic github/accessibility-audits#2939 PRs were reverted because they were causing tests to fail at github/github. This PR re-introduces them (except 1 PR that is not stricken through) with some test and usage fixes required in the upstream dotcom.
Update PRC ActionList implementation to have similar semantics to PVC. #2878NavList: Fix group dividers #3328Fix create-slots utils path in ActionList file #3223Add back check for GroupContext in ActionList.Selection to fix issues introduced to github/github #3267fix(ActionList): update to read from group context if selectionVarian… #3269Refactor(ActionList): ActionList.Item should render content as a button if parent is not interactive. #3284What is changed
1) Update PRC ActionList implementation to have similar semantics to PVC.
Deprecating ActionList.Group is not a final decision - we are exploring remediating the semantics while keeping the API the same
ActionList.Group
.ActionList.Heading
to be used for labelling children in anActionList
.2) Render ActionList.Item as
button
The ActionList.Item content should render an interactive button. Exceptions are when the outermost element of the ActionList.Item is an interactive element. Examples of this include NavList (where ActionList.Items can be render as a button) and in ActionMenu, where the top-level element is made interactive with the menuitem role.
github/github fixes
Integration PR: https://github.com/github/github/pull/278891
ActionList.Item
usages that hasLink
component as a child. GH search (Github stuff only link) and with the new change it generates an inaccessible markup like:I refactored these usages to use
ActionList.LinkItem
commit reference (GitHub stuff only link)ActionList.Item
usages that hasrole=button
attribute and nowActionList.Item
also returns as button, first the role attribute becomes unnecessary and it fails some tests that looks for buttonsgetByRole('button')
because it returns multiple.GH search (Github stuff only link)I removed the
role="button"
attribute from these usages.Screenshots
Please provide before/after screenshots for any visual changes
Merge checklist
Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.