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

Unable to dismiss block inserter panel after inserting a V2 block #43090

Closed
mmtr opened this issue Aug 9, 2022 · 12 comments · Fixed by #47926
Closed

Unable to dismiss block inserter panel after inserting a V2 block #43090

mmtr opened this issue Aug 9, 2022 · 12 comments · Fixed by #47926
Assignees
Labels
[Feature] Inserter The main way to insert blocks using the + button in the editing interface [Priority] High Used to indicate top priority items that need quick attention [Type] Bug An existing feature does not function as intended

Comments

@mmtr
Copy link
Contributor

mmtr commented Aug 9, 2022

Description

After using the block inserter panel to insert a V2 block like the Buttons block, it is not possible to dismiss the panel.

Step-by-step reproduction instructions

  • Start a new post.
  • Click on the + icon to open the block inserter panel.
  • Search for Buttons.
  • Select the Buttons block.
  • Click on the x icon to close the block inserter panel.
  • The panel remains visible.

Screenshots, screen recording, code snippet

Screen.Recording.2022-08-10.at.11.55.35.mov

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

@mmtr mmtr added [Type] Bug An existing feature does not function as intended [Feature] Inserter The main way to insert blocks using the + button in the editing interface labels Aug 9, 2022
@annezazu
Copy link
Contributor

Can replicate with Gutenberg Version 13.8.2. This feels like a pretty bad bug considering. Going to flag in #core-editor.

@glendaviesnz
Copy link
Contributor

glendaviesnz commented Aug 17, 2022

Had a quick look to try and see when it was introduced, but was able to replicate as far back as 12.9, so just isolating the cause and fixing in trunk is probably going to be quicker/easier than a git bisect/blame. I probably won't have time to have a closer look until tomorrow, so if someone else wants to take a look in the mean time go for it.

@priethor priethor added the [Priority] High Used to indicate top priority items that need quick attention label Aug 17, 2022
@priethor
Copy link
Contributor

Thanks @glendaviesnz , it would be great if you could have a look at it 🙂

@ntsekouras
Copy link
Contributor

ntsekouras commented Aug 17, 2022

Thanks for reporting this @mmtr!

After using the block inserter panel to insert a V2 block like the Buttons block

What do you mean by V2 block?

**For the below text pay attention to the difference of Buttons(plural) and Button block.

After digging a bit, in order for this to happen, the inserted block needs to satisfy a couple of conditions:

  1. Have innerBlocks and use templateInsertUpdatesSelection: true
  2. Declare allowed blocks that don't contain the parent(for example Buttons allow only the Button block

In the main inserter when we insert the block we select it(in the store) but not focus it, so as the inserter remains open. With the above conditions, when we insert let's say the Buttons block, it inserts a Button block and selects it, making the available blocks in the inserter to be only other Button blocks. This makes the original Buttons block we clicked unmount and the focus is not restored.

@glendaviesnz
Copy link
Contributor

I didn't have much time to look at this today, but the PR that introduced the focus criteria for closing is here, and I confirmed that switching back to a simple toggle fixes the issue of it not closing, but reintroduces the focus trap issue that this PR fixed.

@mmtr
Copy link
Contributor Author

mmtr commented Aug 18, 2022

What do you mean by V2 block?

A block registered using the block API Version 2. That could have been a wrong assumption from my end though, since I couldn't reproduce this on blocks using the Version 1, but maybe because I was missing the other conditions you noted.

@youknowriad
Copy link
Contributor

@ntsekouras That makes sense. It seems that we need some kind of focus loss detection in the inserter when it's supposed to stay focused. That said, it's a bit tricky because we rely on the focus loss happening to actually close the inserter in several situations:

  • Ctrl + click on any block for instance

@Mamaduka
Copy link
Member

Mamaduka commented Dec 7, 2022

Do we have any possible solution for this issue? I noticed that this is causing some E2E test failures.

@ndiego
Copy link
Member

ndiego commented Jan 26, 2023

Confirmed that this remains an issue in Gutenberg trunk (~15.1)

@Mamaduka
Copy link
Member

Adding this to the WP 6.2 project board's todos.

@talldan
Copy link
Contributor

talldan commented Jan 30, 2023

This comment from @youknowriad seems to imply that this code is in place to ensure this doesn't happen:

// This ensures for instance that the focus stays in the inserter when inserting the "buttons" block.
getSelectedBlocksInitialCaretPosition()

Something must have changed that caused it to stop working.

For me, I'd question whether selecting the templated inner button block is the right thing when inserting from the library. I think the parent should stay selected

It can lead to some weird situations, like if you try inserting four quotes in a row from the library, it actually does something weird where the quotes are recursively nested. I'm not sure the quote block should allow other quotes within it anyway, but still, the issue is that the person inserting the block is not necessarily aware of the context change around where blocks are being inserted.

@kevin940726
Copy link
Member

What's stopping us from calling setIsInserterOpened( false ) again just to be sure? It might not be the best solution, but at least I think it works. I can put up a PR and add some comments above referencing this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Inserter The main way to insert blocks using the + button in the editing interface [Priority] High Used to indicate top priority items that need quick attention [Type] Bug An existing feature does not function as intended
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

10 participants