[Block Editor]: Restrict delete multi selected blocks shortcut #37595
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ntsekouras
added
[Type] Bug
ntsekouras
requested review from
Mamaduka,
ellatrix,
ramonjd,
andrewserong and
aaronrobertshaw
|
Size Change: +36 B (0%) Total Size: 1.13 MB
ℹ️ View Unchanged
|
ntsekouras
force-pushed
the
try/restrict-delete-multi-selection-shortcut
branch
2 times, most recently
from
87c61b7 to
cf51bcf
Compare
Mamaduka
reviewed
Dec 22, 2021
ntsekouras
force-pushed
the
try/restrict-delete-multi-selection-shortcut
branch
from
cf51bcf to
85dce58
Compare
Mamaduka
approved these changes
Dec 22, 2021
stacimc
approved these changes
Dec 22, 2021
There was a problem hiding this comment.
This tested great for me.
✅ Reproduced the bug on trunk in both the site and post editor
✅ Tested Delete and Esc from within the Add to Reusable blocks menu
✅ Tested Delete and Esc from within Inspector Controls, including nested controls (for example, opening up several layers of modals in the Color controls and using Esc to close each in order)
✅ Tested in both post editor and site editor
aaronrobertshaw
approved these changes
Dec 23, 2021
There was a problem hiding this comment.
Thanks for the fix @ntsekouras 👍
This works great for me.
✅ Can replicate the original issue
✅ Normal deletion of multiple blocks still works as expected
✅ Prevents unintentional deletion of blocks when in inspector controls or modals etc
✅ Solution works in both Site and Post editors.
7 tasks
andrewserong
approved these changes
Dec 23, 2021
There was a problem hiding this comment.
Just another +1 that this is testing well for me, too in the post and site editors. Thanks for the clean fix @ntsekouras! I'll close out a couple of other WIP PRs that sought to fix this issue, too.
8 tasks
7 tasks
ellatrix
reviewed
Dec 23, 2021
|
Bit late to the party, but thanks for this fix @ntsekouras 🙇 |
noisysocks
removed
the
Backport to WP 6.7 Beta/RC
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes: #36513
Fixes: #37316
Currently if the writing flow keyboard shortcuts (BlockEditorKeyboardShortcuts) are registered, they can be also processed in the Inspector controls or in any modals created in a block because they're all nested in the same BlockTools tree.
This results to the above two issues where if we have multi selected blocks and press
escapeordeleteon a modal's or Inspector controls inputs, the blocks are deleted and we're not just removing a character as expected.@ellatrix explained the issue very well here and I'm quoting part of it.
This PR implements the first solution with some tweaks.
From the above solution I added this check only in the specific shortcut for deleting multi selected blocks, as IMO this is the only one that disrupts the expected flow.
In addition I'm checking only for
input and text areabecause we can be in content editable(example paragraph) but we still want the other shortcuts to work likeduplicate, etc...Testing instructions
post editororsite editormulti select some blocks1.1 observe that by pressing
escapeordeleteeven with the block setting open will still work.1.2 either in
Add to Reusable blocksinput or in an input in Inspector controls likecolor, line height, etc..pressescapeordeleteand observe that the blocks are not deleted.Before
before.mov
After
after.mov
Noting that the block editor shortcuts were just merged for site editor and that's why in previous testing from others the above issues couldn't be reproduced consistently between the two editors (post/site).