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

Ctrl+Alt+Backspace on reusable block also deletes block above #9940

Closed
annestk opened this issue Sep 16, 2018 · 4 comments · Fixed by #10008
Closed

Ctrl+Alt+Backspace on reusable block also deletes block above #9940

annestk opened this issue Sep 16, 2018 · 4 comments · Fixed by #10008
Assignees
Labels
[Feature] Synced Patterns Related to synced patterns (formerly reusable blocks) [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Testing Needs further testing to be confirmed. [Type] Bug An existing feature does not function as intended

Comments

@annestk
Copy link

annestk commented Sep 16, 2018

Describe the bug
When using the remove block keyboard shortcut on a reusable block, the block above is also deleted. It doesn't happen when I select 'remove block' from the ellipsis, or when using the keyboard shortcut on other blocks.
It only happens with reusable blocks (not a specific one) and regardless of the type of block above it (including another reusable block).

To Reproduce
Steps to reproduce the behavior:

  1. Add any block
  2. Add a reusable block
  3. Press ctrl+alt+backspace
  4. Both the reusable block and the block above is deleted

Expected behavior
Only the reusable block should be removed

Screenshots
ezgif-1-6aca517b7e.gif

Console error
I'm seeing the following errors, though I don't know if it's related

Uncaught Error: only one instance of babel-polyfill is allowed
at Object. (commons-820.min.js?ver=8.2:1)
at Object. (commons-820.min.js?ver=8.2:1)
at e (commons-820.min.js?ver=8.2:1)
at Object. (commons-820.min.js?ver=8.2:6)
at e (commons-820.min.js?ver=8.2:1)
at commons-820.min.js?ver=8.2:1
at commons-820.min.js?ver=8.2:1
twp-templates.js?ver=1.1.5:101 Uncaught TypeError: Cannot read property 'replace' of undefined
at Object.t.refreshTitle (twp-templates.js?ver=1.1.5:101)
at refreshTemplateVars (twp-templates.js?ver=1.1.5:333)

Desktop (please complete the following information):

  • OS: Windows 10.0.17134
  • Chrome 68.0.3440.106
  • Gutenberg 3.8 (I don't remember if it happened with earlier versions too - I might not have noticed)
@designsimply designsimply 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). [Feature] Synced Patterns Related to synced patterns (formerly reusable blocks) Needs Testing Needs further testing to be confirmed. labels Sep 17, 2018
@youknowriad
Copy link
Contributor

This not an easy issue to solve :)

  • Clicking "Backspace" when the wrapper of a block is selected, removes the block
  • Clicking "ctrl + alt + backspace" when the a block is selected (no matter where the focus is) removes the block

Which means if the wrapper of a block is selected (it happens in reusable blocks but also in other blocks like images...), and you hit "ctrl + alt + backspace", you trigger both callbacks removing two blocks.

What do you think would be the best fix here?

  • I wasn't able to "stopPropagation"
  • Maybe we can add a check on the first callback to ensure "ctrl + alt" are not pressed?

Thoughs @aduth @talldan

@talldan
Copy link
Contributor

talldan commented Sep 18, 2018

Hey @youknowriad

Maybe we can add a check on the first callback to ensure "ctrl + alt" are not pressed?

That's how I ended up solving the issue in RichText ...
https://github.com/WordPress/gutenberg/blob/master/packages/editor/src/components/rich-text/index.js#L419

But hopefully that won't be needed in BlockListBlock - because of #9036, the shortcut will be changed to something other than backspace, so that seems like the best way to solve it. (This reminds me I need to work on that again).

@talldan talldan self-assigned this Sep 18, 2018
@talldan
Copy link
Contributor

talldan commented Sep 18, 2018

I'll work on something now to get the ball rolling on that again.

@talldan
Copy link
Contributor

talldan commented Sep 18, 2018

Thanks for the report @annestk! As briefly mentioned above, it looks like this shortcut is triggering two actions to remove blocks, and so two blocks are removed.

There's already a plan to change the shortcut to something else as there's a separate issue with this shortcut for linux users (#9036). I've opened pull request #10008 to do that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Synced Patterns Related to synced patterns (formerly reusable blocks) [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Testing Needs further testing to be confirmed. [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants