Select Mode: Updates to the block toolbar #65485
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -183,6 +183,9 @@ export function BlockSettingsDropdown( { | |
| } | ||
| } | ||
|
|
||
| const shouldShowBlockParentMenuItem = | ||
| ! parentBlockIsSelected && !! firstParentClientId; | ||
|
|
||
| return ( | ||
| <BlockActions | ||
| clientIds={ clientIds } | ||
|
|
@@ -199,124 +202,143 @@ export function BlockSettingsDropdown( { | |
| onRemove, | ||
| onCopy, | ||
| onPasteStyles, | ||
| } ) => { | ||
| // It is possible that some plugins register fills for this menu | ||
| // even if Core doesn't render anything in the block settings menu. | ||
| // in which case, we may want to render the menu anyway. | ||
| // That said for now, we can start more conservative. | ||
| const isEmpty = | ||
| ! canRemove && | ||
| ! canDuplicate && | ||
| ! canInsertBlock && | ||
| isContentOnly; | ||
|
|
||
| if ( isEmpty ) { | ||
| return null; | ||
| } | ||
|
|
||
| return ( | ||
| <DropdownMenu | ||
| icon={ moreVertical } | ||
| label={ __( 'Options' ) } | ||
| className="block-editor-block-settings-menu" | ||
| popoverProps={ POPOVER_PROPS } | ||
| open={ open } | ||
| onToggle={ onToggle } | ||
| noIcons | ||
| { ...props } | ||
| > | ||
| { ( { onClose } ) => ( | ||
| <> | ||
| <MenuGroup> | ||
| <__unstableBlockSettingsMenuFirstItem.Slot | ||
| fillProps={ { onClose } } | ||
| /> | ||
| { shouldShowBlockParentMenuItem && ( | ||
| <BlockParentSelectorMenuItem | ||
| parentClientId={ | ||
| firstParentClientId | ||
| } | ||
| parentBlockType={ parentBlockType } | ||
| /> | ||
| ) } | ||
| { count === 1 && ( | ||
| <BlockHTMLConvertButton | ||
| clientId={ firstBlockClientId } | ||
| /> | ||
| ) } | ||
| { ! isContentOnly && ( | ||
| <CopyMenuItem | ||
| clientIds={ clientIds } | ||
| onCopy={ onCopy } | ||
| shortcut={ displayShortcut.primary( | ||
| 'c' | ||
| ) } | ||
| /> | ||
| ) } | ||
| { canDuplicate && ( | ||
| <MenuItem | ||
| onClick={ pipe( | ||
| onClose, | ||
| onDuplicate, | ||
| updateSelectionAfterDuplicate | ||
| ) } | ||
| shortcut={ shortcuts.duplicate } | ||
| > | ||
| { __( 'Duplicate' ) } | ||
| </MenuItem> | ||
| ) } | ||
| { canInsertBlock && ! isContentOnly && ( | ||
| <> | ||
| <MenuItem | ||
| onClick={ pipe( | ||
| onClose, | ||
| onInsertBefore | ||
| ) } | ||
| shortcut={ | ||
| shortcuts.insertBefore | ||
| } | ||
| > | ||
| { __( 'Add before' ) } | ||
| </MenuItem> | ||
| <MenuItem | ||
| onClick={ pipe( | ||
| onClose, | ||
| onInsertAfter | ||
| ) } | ||
| shortcut={ | ||
| shortcuts.insertAfter | ||
| } | ||
| > | ||
| { __( 'Add after' ) } | ||
| </MenuItem> | ||
| </> | ||
| ) } | ||
| </MenuGroup> | ||
| { canCopyStyles && ! isContentOnly && ( | ||
| <MenuGroup> | ||
| <CopyMenuItem | ||
| clientIds={ clientIds } | ||
| onCopy={ onCopy } | ||
| label={ __( 'Copy styles' ) } | ||
| /> | ||
| <MenuItem onClick={ onPasteStyles }> | ||
| { __( 'Paste styles' ) } | ||
| </MenuItem> | ||
| </MenuGroup> | ||
| ) } | ||
| <BlockSettingsMenuControls.Slot | ||
| fillProps={ { | ||
| onClose, | ||
| count, | ||
| firstBlockClientId, | ||
| } } | ||
| clientIds={ clientIds } | ||
| /> | ||
|
Comment on lines
+310
to
+317
There was a problem hiding this comment. It looks like in some cases this slot will output the menu group, even when it's visually empty. To reproduce, if I use Edit mode and select a list item within TT4's blog home template, the settings menu will only include 
With a bit of commenting out, I think I've identified the three components that are rendering empty fills. They bail before outputting anything visually, however it results in diff --git a/packages/editor/src/components/provider/index.js b/packages/editor/src/components/provider/index.js
index 11b1478d584..627e32e588f 100644
--- a/packages/editor/src/components/provider/index.js
+++ b/packages/editor/src/components/provider/index.js
@@ -327,17 +327,17 @@ export const ExperimentalEditorProvider = withRegistryProvider(
selection={ selection }
settings={ blockEditorSettings }
useSubRegistry={ false }
>
{ children }
{ ! settings.__unstableIsPreviewMode && (
<>
- <PatternsMenuItems />
- <TemplatePartMenuItems />
- <ContentOnlySettingsMenu />
+ {/* <PatternsMenuItems /> */}
+ {/* <TemplatePartMenuItems /> */}
+ {/* <ContentOnlySettingsMenu /> */}
{ mode === 'template-locked' && (
<DisableNonPageContentBlocks />
) }
{ type === 'wp_navigation' && (
<NavigationBlockEditingMode />
) }
<EditorKeyboardShortcuts />That then fixes up the spacing: 
So, in order to fix that spacing, do we need to conditionally render the above components, or update their internals so they don't output Initially I was wondering if we could fix it by using CSS with something like: .components-menu-group:has( > div:empty ) {
display: none;
}But unfortunately that still leaves a space due to the last menu group needing the negative margin in order for the spacing to look correct. And even with 
So, unless there's another CSS-only way to handle it, we might need to lightly refactor some of those components that are outputting empty fills? There was a problem hiding this comment. Unfortunately, it's going to be very hard to solve these things by refactoring the fills. This is not the first item or the only place where we have this issue. I wonder if it's something fundamental to how slot/fills work. If we can't find a CSS-only way for now, I think we should accept a small inconvenience for now. There was a problem hiding this comment. Ok I think I have a solution here but also, don't want to apply it in this PR though because it's potentially impactful. So I'm going to open a separate PR for that. There was a problem hiding this comment. This PR should fix it #65561 |
||
| { typeof children === 'function' | ||
| ? children( { onClose } ) | ||
| : Children.map( ( child ) => | ||
| cloneElement( child, { onClose } ) | ||
| ) } | ||
| { canRemove && ( | ||
| <MenuGroup> | ||
| <MenuItem | ||
| onClick={ pipe( | ||
| onClose, | ||
| onRemove, | ||
| updateSelectionAfterRemove | ||
| ) } | ||
| shortcut={ shortcuts.remove } | ||
| > | ||
| { __( 'Delete' ) } | ||
| </MenuItem> | ||
| </MenuGroup> | ||
| ) } | ||
| </> | ||
| ) } | ||
| </DropdownMenu> | ||
| ); | ||
| } } | ||
| </BlockActions> | ||
| ); | ||
| } | ||
|
|
||
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 is the only change to this file, the isEmpty flag was added and the dropdown is not rendered if empty.