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

Focus is lost after selecting option from "Select Menu" dropdown #38169

Closed
tellthemachines opened this issue Jan 24, 2022 · 9 comments
Closed
Assignees
Labels
[Block] Navigation Affects the Navigation Block [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Dev Ready for, and needs developer efforts [Type] Bug An existing feature does not function as intended

Comments

@tellthemachines
Copy link
Contributor

tellthemachines commented Jan 24, 2022

Description

In the Navigation Block toolbar, the "Select Menu" button opens a dropdown from where a different menu can be selected. Selecting a menu from that dropdown causes the dropdown to close, but when it closes keyboard focus is reset to the document body.

Focus should instead go back to the "Select Menu" button in the toolbar, or perhaps to the block itself.

Possibly due to #39044

Edit: Now it only happens when creating a new menu or importing from a classic one.

Step-by-step reproduction instructions

  1. Create a Navigation block, and make sure the site has more than one navigation menu already.
  2. In the Navigation block toolbar, click "Select Menu" and choose a menu from the dropdown.
  3. Note focus is no longer on the block when the dropdown closes.

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@tellthemachines tellthemachines added [Type] Bug An existing feature does not function as intended [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Block] Navigation Affects the Navigation Block [a11y] Keyboard & Focus labels Jan 24, 2022
@tellthemachines
Copy link
Contributor Author

Looks like what's happening is the toolbar re-renders when the nav block contents are replaced, causing the button that focus should be returned to to disappear from the DOM.

@getdave
Copy link
Contributor

getdave commented Feb 28, 2022

FYI that this is being looked at in #38907 (comment).

We decided not to fix in that PR as this Issue is too complex.

@getdave
Copy link
Contributor

getdave commented Mar 1, 2022

Whoever works on a fix here should be sure to include an e2e test to ensure the Select menu... is not unmounted during loading.

@getdave
Copy link
Contributor

getdave commented Mar 9, 2022

I tested this by removing the RecursionProvider from the block. The focus loss issue still occurs. It's something else...

@alexstine
Copy link
Contributor

This was a fairly complicated issue. I think this happens when the block changes in to loading state. I made a fix but it probably isn't pretty. Hopefully it may serve as a guide moving forward. I think this happens when the placeholder dynamically changes and focus is not forced. The block itself has some interesting focus issues even outside of this issue. 👎

@talldan
Copy link
Contributor

talldan commented Apr 19, 2022

I tested this by removing the RecursionProvider from the block. The focus loss issue still occurs. It's something else...

@getdave That's not a very scientific test, because there can be more than one cause of a bug. That's exactly the case in the navigation block, lots of DOM elements are unmounted/remounted, and that's one cause.

In a past attempt to solve this I removed as much code that causes unmounting/remounting as I could and the Recursion Provider was still an issue.

@alexstine alexstine added Needs Dev Ready for, and needs developer efforts and removed [Status] In Progress Tracking issues with work in progress labels Apr 19, 2022
@getdave
Copy link
Contributor

getdave commented Apr 25, 2022

@getdave That's not a very scientific test, because there can be more than one cause of a bug.

I guess I didn't include my nuance here. What I meant to say was there doesn't appear to be a single cause here.

@draganescu
Copy link
Contributor

I think #42987 solved this by incorporating #42956 but I am not confident enough to close the issue.

@cbravobernal
Copy link
Contributor

cbravobernal commented Sep 6, 2022

Tested and is working fine!

@cbravobernal cbravobernal removed the [Status] In Progress Tracking issues with work in progress label Sep 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Navigation Affects the Navigation Block [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Dev Ready for, and needs developer efforts [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants