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

Selected blocks get deleted when creating a reusable block and pressing backspace key #36513

Closed
fluiddot opened this issue Nov 16, 2021 · 7 comments · Fixed by #37595
Closed
Assignees
Labels
[Block] Block The "Reusable Block" Block [Feature] Synced Patterns Related to synced patterns (formerly reusable blocks) [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@fluiddot
Copy link
Contributor

Description

When creating a reusable block via selecting multiple blocks, pressing the backspace key when typing a name in the "Create Reusable block" dialog leads to deleting the selected blocks and therefore the dialog to disappear. This behavior makes it very difficult to create a reusable block as removing characters when setting a name is very common.

Step-by-step reproduction instructions

  1. Add multiple blocks to a post.
  2. Select multiple blocks.
  3. Click on the options button located in the toolbar (the three dots icon).
  4. Click on "Add to Reusable blocks" option.
  5. Select the Name input field.
  6. Type a name and press the backspace key.
  7. Observe that the selected blocks are deleted and the "Create Reusable block" dialog is closed.

Screenshots, screen recording, code snippet

reusable-block-backspace-bug.mp4

Environment info

  • Gutenberg: v11.8.3
  • Editing Toolkit: 3.19495
  • WP AMP: 2.0.5
  • CoBlocks: 2.10.1

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

@fluiddot fluiddot added [Type] Bug An existing feature does not function as intended [Feature] Synced Patterns Related to synced patterns (formerly reusable blocks) [Block] Block The "Reusable Block" Block labels Nov 16, 2021
delowardev added a commit to delowardev/gutenberg that referenced this issue Dec 7, 2021
@hellofromtonya hellofromtonya added the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Dec 21, 2021
@hellofromtonya hellofromtonya linked a pull request Dec 21, 2021 that will close this issue
7 tasks
@hellofromtonya hellofromtonya linked a pull request Dec 21, 2021 that will close this issue
7 tasks
@hellofromtonya
Copy link
Contributor

Copying report from duplicate issue #37539:

Originally reported in WordPress Core's Trac ticket 54652.

Since WordPress 5.9 Beta (works correctly in 5.8.1), backspacing to clear the reusable block's name in the "Create Reusable Block" modal closes the modal and deletes the selected blocks. This is different from WordPress 5.8.1 which clears the name field, but keeps the modal open and does not delete the selected blocks.

The reporter also notes that control Z does not restore the content; however, in my testing, control Z did restore the blocks for me.

Step-by-step reproduction instructions

  1. Add a new post
  2. Add several blocks
  3. Select more than one block
  4. Select options icon in the toolbar
  5. Select "Add to Reusable blocks"
  6. Start to type something in the modal
  7. Backspace to clear the name field

Notice the modal closes and the selected blocks are deleted.

Screenshots, screen recording, code snippet

test-attempt-to-reproduce-54652

Environment info

  • OS: macOS Big Sur
  • Browsers: Chrome, Safari, Edge, and Firefox
  • WordPress: trunk, 5.9 Beta 3, and 5.8.1
  • Theme: TT2 for 5.9, TT1 for 5.8.1
  • Plugins: none and with Gutenberg v 12.1.0 activated
  • Localhost: wp-env and Local

@hellofromtonya
Copy link
Contributor

Test Report from @costdev:

Environment Info
OS: Windows 10
Browser: Chrome
WordPress: trunk
Theme: TT2
Plugins: Gutenberg 12.1

  1. Pressing the Backspace or Delete keys while naming a Reusable Block deletes the content.
  2. Ctrl-Z restores the content after two presses.

Removing this event listener removes the Backspace or Delete behaviour:

gb-37539

  • Removing any other event listener, including (5), doesn't work.
  • It's specifically (4) that seems to be causing the behaviour.

@Mamaduka
Copy link
Member

The issue was introduced via #32323 or with Gutenberg 11.2. React events correctly bubble through portals and are caught by BlockTools.

Short term solution for WP 5.9 could be to modify handleEscapeKeyDown to catch BACKSPACE and DELETE keycodes.

We should probably constrain shortcuts to Modals in the long term, like with tabbing.

/cc @WordPress/gutenberg-core

@ellatrix
Copy link
Member

Sounds to me like key detection in the content should check DOM events instead of React events.

@Mamaduka
Copy link
Member

Mamaduka commented Dec 22, 2021

@ellatrix, that would mean reverting changes from #32323, right? It might be a significant change to be backported this late in the release cycle.

@ntsekouras
Copy link
Contributor

I discussed this with @ellatrix and will have a PR today with a possible solution without reverting.

@ellatrix
Copy link
Member

I think this boils down to the question: do we want shortcuts to work when the options popover in the block toolbar is toggled (which shows some shortcuts). I think yes. In that case this popover is no different from any other thing that uses a portal (iframe, inspector...). There's no way to know where exactly ⇧⌘D (for example) should work. If we want it to work in the block toolbar popover, then it will also work in the inspector and modals created in a block because they're all nested in the same BlockTools tree.

Solution 1: check if the target element is a text area, input, content editable, or event.defaultPrevented is true and return early. In all these cases backspace could be handled elsewhere.

Solution 2: remove "basic" shortcuts like backspace from this event handler and restrict them to only work when the block in the canvas is focused. This would disable these basic shortcuts if the "more options" popover is focussed, but also when focus is in the inspector or a modal created by the block.

Solution 3: More specifically add the keydown handler to certain wrappers: only to the block props (so it works when the block has focus), and the block toolbar. Seems harder for multi block shortcuts, so it may have to be added to block list element. The idea is that there would be multiple instances of event handling: one for focus in the content and one for focus in the block toolbar instead of a single one to catch it all.

@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Dec 22, 2021
@youknowriad youknowriad removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Jan 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Block The "Reusable Block" Block [Feature] Synced Patterns Related to synced patterns (formerly reusable blocks) [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
None yet
6 participants