-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Fix inserter pattern pagination focus loss #60620
Conversation
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 Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @creative-reinilda. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. 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. |
Size Change: +10 B (0%) Total Size: 1.75 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Side note: I wonder if we should just make __experimentalIsFocusable
the default when disabled
is true. I'd expect that would usually what users want anyway. This will be a breaking change and doesn't match the HTML standard though.
There are legitimate cases where an interactive control needs to be 'truly' disabled. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested it in the following environment.
- OS: Windows 11
- Browser: 124.0.2 (64-bit)
Test Result:
- trunk: Can reproduce the problem.
- This PR: Problem solved for both the Pattern Tab inserter, and the Pattern Modal.
I wonder if we should just make
__experimentalIsFocusable
the default whendisabled
is true
This suggestion seems to be discussed on #56547.
I just cherry-picked this PR to the update/fixes-for-wp-6-5-3 branch to get it included in the next release: 9ad8c97 |
What?
Fixes #55043
Because the pagination buttons should remain focused when paginating, they should use
aria-disabled
instead ofdisabled
.Why?
Applying
disabled
to a focused button causes a focus loss. Because the inserter sidebar is implemented as a dialog, this results in the sidebar closing (though it's strange that it only happens in some browsers).How?
Adds the
__experimentalIsFocusable
prop, which makes theButton
component usearia-disabled
instead ofdisabled
.Testing Instructions
Prerequisite: Use Safari (or firefox, where the issue has also been reported, though I personally couldn't reproduce)
In this PR: the inserter sidebar should stay open and focus should remain on the button
In trunk: Focus is lost and the sidebar closes
Screenshots or screencast
Before
Kapture.2024-04-10.at.12.32.01.mp4
After
Kapture.2024-04-10.at.12.29.16.mp4