-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Background image support: Fix focus loss when resetting background image #55984
Background image support: Fix focus loss when resetting background image #55984
Conversation
Size Change: +36 B (0%) Total Size: 1.7 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.
This works as described! 👍🏻
@@ -271,7 +277,17 @@ function BackgroundImagePanelItem( props ) { | |||
> | |||
{ hasValue && ( | |||
<MenuItem | |||
onClick={ () => resetBackgroundImage( props ) } | |||
onClick={ () => { | |||
const [ toggleButton ] = focus.tabbable.find( |
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.
TIL!
// Focus the toggle button and close the dropdown menu. | ||
// This ensures similar behaviour as to selecting an image, where the dropdown is | ||
// closed and focus is redirected to the dropdown toggle button. | ||
toggleButton?.focus(); |
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 asked ChatGPT if there were any accessibility concerns when programmatically triggering the HTMLElement click() method:
- Ensure that the programmatically triggered click is consistent with keyboard navigation.
- Make sure that the element receiving the click event also receives focus
- ARIA Roles and States: Ensure that the HTML elements have appropriate ARIA roles and states if necessary. (the
aria-expanded
has the right value)
😄
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.
Thank you, @andrewserong!
I totally missed the focus loss issue 🙇
Thanks, folks! 🙂 |
I just cherry-picked this PR to the 6.4-next-point-release branch to get it included in the next release: 223efb0 |
What?
Fix focus loss when resetting a background image in the background image block support (e.g. in a Group block) when resetting via the main dropdown.
Why?
This is most noticeable when navigating via keyboard, but the fix in #55973 that hides the Reset button results in a focus loss when you click the Reset button as the reset button is then no longer rendered once the background image value is cleared. The proposed fix is to deliberately place focus on the toggle button to avoid the focus loss.
How?
Testing Instructions
Testing Instructions for Keyboard
As above, but tab over to the background image toggle and press enter to open the dropdown. After you've added an image, when you hit enter on the Reset option to clear out the value, the dropdown should be closed, and focus should be directed to the toggle button. You should be able to hit enter again to open the dropdown.
Screenshots or screencast
Before
Resetting results in a focus loss with the dropdown still open:
2023-11-09.10.20.53.mp4
After
Resetting closes the dropdown and shifts focus to the toggle button:
2023-11-09.12.28.54.mp4