Remove block switcher from header, add to multi block controls #7891
Conversation
| aria-label={ __( 'Block Toolbar' ) } | ||
| key="toolbar" | ||
| > | ||
| <BlockSwitcher uids={ multiSelectedBlockUids } /> |
There was a problem hiding this comment.
I think a more appropriate place would be to move to the BlockToolbar component which will make it appear at the right position depending on the preference Fix toolbar to top
There was a problem hiding this comment.
Done! This required that the BlockToolbar became aware of multi-block selection, but it cut down on the repeated toolbars, and is a good point to insert multi-block toolbar controls :)
| @@ -34,7 +33,6 @@ function HeaderToolbar( { hasFixedToolbar, isLargeViewport } ) { | |||
| <EditorHistoryUndo /> | |||
| <EditorHistoryRedo /> | |||
| <TableOfContents /> | |||
| <MultiBlocksSwitcher /> | |||
There was a problem hiding this comment.
I wonder if this MultiBlocksSwitcher is already handling showing the switcher only for multi selection.
We should:
- Delete this component since you duplicated its logic in
block-toolbar - Or Use it in
block-toolbardirectly
| function BlockToolbar( { block, mode } ) { | ||
| if ( ! block || ! block.isValid || mode !== 'visual' ) { | ||
| function BlockToolbar( { block, mode, selectedBlockUIDs } ) { | ||
| if ( selectedBlockUIDs.length < 2 && ( ! block || ! block.isValid || 'visual' !== mode ) ) { |
There was a problem hiding this comment.
- Should we also render nothing if the
selectedBlockUIDsis empty? - Instead of passing the whole
blockobject. It seems we can just get theisValidflag instead as a prop that way this component will only rerender if the flag changes and not other block properties (performance tip)
| return null; | ||
| } | ||
|
|
||
| return ( | ||
| <div className="editor-block-toolbar"> | ||
| <BlockSwitcher uids={ [ block.uid ] } /> | ||
| <BlockSwitcher uids={ selectedBlockUIDs.length > 1 ? selectedBlockUIDs : [ block.uid ] } /> |
There was a problem hiding this comment.
I wonder if we can't normalize this selectedBlockUIDs and block prop with a unique selectedBlockUIDs which would contain the selected blocks (multi-selection or not)
There was a problem hiding this comment.
That would make things a lot cleaner. Thanks!
| return null; | ||
| } | ||
|
|
||
| return ( | ||
| <div className="editor-block-toolbar"> | ||
| <BlockSwitcher uids={ [ block.uid ] } /> | ||
| <BlockSwitcher uids={ blockUIDs } /> | ||
| <BlockControls.Slot /> |
There was a problem hiding this comment.
I wonder if it makes sense to keep the slots in case we want to show multi-selection controls or something like that in the future.
There was a problem hiding this comment.
Yes, we do, and I'm working on that next week in another PR :) So I'll be rebasing to use these changes in that PR
designsimply
added
the
[Feature] Block Multi Selection
Description
Removed the block switcher from the header, and made it appear when selecting multiple blocks.
How has this been tested?
Select multiple blocks and check the switcher is there.