Button: Stabilize __experimentalIsFocusable prop
#62282
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mirka
added
[Type] Enhancement
|
I'd kind of wish |
It did come up, with the two major downsides being breaking spec of an extremely standard prop/attribute, and having to use a mixed type. I wouldn't go as far as to say it's a nonstarter, but breaking spec is a big one for me, personally. To add some context about going the "additional prop" route, I added a note in the PR description about how Ariakit adopts this pattern. |
mirka
force-pushed
the
button-focusable-disabled
branch
from
00d5e97 to
8070a05
Compare
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Flaky tests detected in e622a3f. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/9785502033
|
tyxla
approved these changes
Jul 4, 2024
| @@ -159,7 +161,7 @@ export function UnforwardedButton( | |||
| 'has-icon': !! icon, | |||
| } ); | |||
|
|
|||
| const trulyDisabled = disabled && ! isFocusable; | |||
| const trulyDisabled = disabled && ! accessibleWhenDisabled; | |||
There was a problem hiding this comment.
Nice to see this getting clearer now 👏
| @@ -619,6 +619,14 @@ describe( 'Button', () => { | |||
| 'mixed' | |||
| ); | |||
| } ); | |||
|
|
|||
| it( 'should not break when the legacy __experimentalIsFocusable prop is passed', () => { | |||
ciampo
added
the
Needs Dev Note
Co-authored-by: Marin Atanasov <[email protected]>
…3107) * Replace `__experimentalIsFocusable` with `accessibleWhenDisabled` * Replace a few more remaining instances * Revert "Replace a few more remaining instances" This reverts commit 406ef77. --------- Co-authored-by: Marin Atanasov <[email protected]>
# Conflicts: # packages/editor/src/components/post-actions/actions.js
|
Size Change: +69 B (0%) Total Size: 1.75 MB
ℹ️ View Unchanged
|
mirka
added
has dev note
This was referenced Jul 4, 2024
carstingaxion
pushed a commit
to carstingaxion/gutenberg
that referenced
this pull request
Jul 18, 2024
* Button: Stabilize `__experimentalIsFocusable` prop * Add changelog * Tweak changelog Co-authored-by: Marin Atanasov <[email protected]> * Replace `__experimentalIsFocusable` with `accessibleWhenDisabled` (WordPress#63107) * Replace `__experimentalIsFocusable` with `accessibleWhenDisabled` * Replace a few more remaining instances * Revert "Replace a few more remaining instances" This reverts commit 406ef77. --------- Co-authored-by: Marin Atanasov <[email protected]> * Fix new violation --------- Co-authored-by: mirka <[email protected]> Co-authored-by: tyxla <[email protected]> Co-authored-by: stokesman <[email protected]>
97 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Stabilizes the
__experimentalIsFocusableprop on the Button component into aaccessibleWhenDisabledprop.TODO
no-restricted-syntaxrule (Components: Fix inaccessible disabledButtons #62306)__experimentalIsFocusablewithaccessibleWhenDisabled#63107)__experimentalIsFocusablewithaccessibleWhenDisabled#63107)Why?
We are tightening our standards (#62080) around disabled Button accessibility.
The
accessibleWhenDisablednaming and pattern aligns with what Ariakit is doing, which gives some confidence that this is sustainable and scalable across a component library.How?
__experimentalIsFocusableprop.Testing Instructions
__experimentalIsFocusablewithaccessibleWhenDisabled#63107 is merged into this branch.✍️ Dev Note
The
Buttoncomponent had a__experimentalIsFocusableprop to keep the button focusable when disabled. This prop is now stabilized and renamed toaccessibleWhenDisabled. (Existing usages of__experimentalIsFocusablewill continue to work indefinitely.)In most cases, it is recommended to set this to
true. Disabling a control without maintaining focusability can cause accessibility issues, by hiding their presence from screen reader users, or by preventing focus from returning to a trigger element.Learn more about the focusability of disabled controls in the WAI-ARIA Authoring Practices Guide.