-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Writing flow: gradually 'select all' layers of nested blocks #31859
Conversation
useCallback( | ||
( event ) => { | ||
event.preventDefault(); | ||
multiSelect( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only saw now that these shortcuts are duplicated here, while also being in writing flow. This doesn't make any sense. The select all shortcut should only work from a focussed block upwards. These global shortcuts will also give problems when you focus something outside the block editor and press cmd+a. In other words, it should never be a global behaviour. Additionally it will give problems in the iframe editor when we remove event bubbling through the iframe.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are some related issues. I found #19102 and #22689 in a quick search.
I think registerShortcut
should ideally still be called as I believe that makes the shortcut display in the shortcut help modal. And then the code in useSelectAll
could reference that registered shortcut.
That would also have the side effect of allowing a plugin to unregister the shortcut or change the shortcut keys. This is an interesting one because it uses the same key combination as the browser shortcut, so there's a question over how that would work..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, I modified it to use registerShortcut again.
Size Change: -6 B (0%) Total Size: 1.62 MB
ℹ️ View Unchanged
|
e18b406
to
4133e78
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works really well, thanks!
I also tested unregistering the shortcut and that issue is now fixed too.
4133e78
to
202680d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty good.
c93ee27
to
8012e49
Compare
Description
Fixes #28267.
Fixes #22689.
Allows you to gradually select blocks through layers of nested blocks.
How has this been tested?
Screenshots
Types of changes
Checklist:
*.native.js
files for terms that need renaming or removal).