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

Don't steal focus when opening browse all blocks #61975

Merged
merged 2 commits into from
May 31, 2024

Conversation

jeryj
Copy link
Contributor

@jeryj jeryj commented May 24, 2024

Fixes #61361

What?

When using the browse all blocks button from adding a new navigation link block, the inserter would steal focus from the popover (correctly) and then the popover would close, stealing focus back.

Why?

Fix using the inserter from the navigabion block.

How?

To prevent this bug, we check to see if focus is still within the popover at the time of closing. If focus is no longer within it, we don't move focus back to the selected block and instead let whatever stole focus keep the focus.

Testing Instructions

  • Use the navigation link inserter (+ button)
  • Select Add Block
  • Select Browse All
  • Focus should be in the inserter and the link inserter popover should be closed

Testing Instructions for Keyboard

  • Use the navigation link inserter (+ button)
  • Select Add Block
  • Select Browse All
  • Focus should be in the inserter and the link inserter popover should be closed

Escape from Adding block

  • Use the navigation link inserter (+ button)
  • Select Add Block
  • Select Browse All
  • Press Escape
  • Focus should be on the previous navigation block

Screenshots or screencast

When using the browse all blocks button from adding a new navigation link block, the inserter would steal focus from the popover (correctly) and then the popover would close, stealing focus back. To prevent this bug, we check to see if focus is still within the popover at the time of closing. If focus is no longer within it, we don't move focus back to the selected block and instead let whatever stole focus keep the focus.
@jeryj jeryj requested a review from ajitbohra as a code owner May 24, 2024 19:32
Copy link

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 props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: jeryj <jeryj@git.wordpress.org>
Co-authored-by: t-hamano <wildworks@git.wordpress.org>
Co-authored-by: talksina <talksina@git.wordpress.org>
Co-authored-by: hagege <hage@git.wordpress.org>
Co-authored-by: annezazu <annezazu@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@jeryj jeryj self-assigned this May 24, 2024
@jeryj jeryj added [Block] Navigation Link Affects the Navigation Link Block [Type] Bug An existing feature does not function as intended labels May 24, 2024
@jeryj jeryj enabled auto-merge (squash) May 28, 2024 14:07
@jeryj jeryj disabled auto-merge May 28, 2024 14:07
@jeryj jeryj enabled auto-merge (squash) May 28, 2024 14:09
@t-hamano t-hamano self-requested a review May 31, 2024 09:28
Copy link
Contributor

@t-hamano t-hamano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I've tested, it works as expected.

Adding E2E tests might help, but since I believe this issue is critical, let's ship this PR to WP6.6.

@jeryj jeryj merged commit 3031559 into trunk May 31, 2024
62 of 63 checks passed
@jeryj jeryj deleted the fix/browse-all-navigation-block branch May 31, 2024 09:29
@github-actions github-actions bot added this to the Gutenberg 18.5 milestone May 31, 2024
carstingaxion pushed a commit to carstingaxion/gutenberg that referenced this pull request Jun 4, 2024
… popover (WordPress#61975)

When using the browse all blocks button from adding a new navigation link block, the inserter would steal focus from the popover (correctly) and then the popover would close, stealing focus back. To prevent this bug, we check to see if focus is still within the popover at the time of closing. If focus is no longer within it, we don't move focus back to the selected block and instead let whatever stole focus keep the focus.
patil-vipul pushed a commit to patil-vipul/gutenberg that referenced this pull request Jun 17, 2024
… popover (WordPress#61975)

When using the browse all blocks button from adding a new navigation link block, the inserter would steal focus from the popover (correctly) and then the popover would close, stealing focus back. To prevent this bug, we check to see if focus is still within the popover at the time of closing. If focus is no longer within it, we don't move focus back to the selected block and instead let whatever stole focus keep the focus.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Navigation Link Affects the Navigation Link Block [Type] Bug An existing feature does not function as intended
Projects
Development

Successfully merging this pull request may close these issues.

Navigation block: browse all blocks auto closes without inserting anything
2 participants