-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Fix navigation editor missing appender not showing appender when no blocks selected #34678
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,7 +14,6 @@ import { | |
Platform, | ||
} from '@wordpress/element'; | ||
import { | ||
InnerBlocks, | ||
__experimentalUseInnerBlocksProps as useInnerBlocksProps, | ||
InspectorControls, | ||
JustifyToolbar, | ||
|
@@ -100,10 +99,14 @@ function Navigation( { | |
setOverlayBackgroundColor, | ||
overlayTextColor, | ||
setOverlayTextColor, | ||
|
||
// These props are used by the navigation editor to override specific | ||
// navigation block settings. | ||
hasSubmenuIndicatorSetting = true, | ||
hasItemJustificationControls = true, | ||
hasColorSettings = true, | ||
customPlaceholder: CustomPlaceholder = null, | ||
customAppender: CustomAppender = null, | ||
} ) { | ||
const [ isPlaceholderShown, setIsPlaceholderShown ] = useState( | ||
! hasExistingNavItems | ||
|
@@ -145,19 +148,24 @@ function Navigation( { | |
|
||
const placeholder = useMemo( () => <PlaceholderPreview />, [] ); | ||
|
||
// When the block is selected itself or has a top level item selected that | ||
// doesn't itself have children, show the standard appender. Else show no | ||
// appender. | ||
const appender = | ||
isSelected || | ||
( isImmediateParentOfSelectedBlock && ! selectedBlockHasDescendants ) | ||
? undefined | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It turns out that |
||
: false; | ||
|
||
const innerBlocksProps = useInnerBlocksProps( | ||
{ | ||
className: 'wp-block-navigation__container', | ||
}, | ||
{ | ||
allowedBlocks: ALLOWED_BLOCKS, | ||
orientation: attributes.orientation, | ||
renderAppender: | ||
( isImmediateParentOfSelectedBlock && | ||
! selectedBlockHasDescendants ) || | ||
isSelected | ||
? InnerBlocks.DefaultAppender | ||
: false, | ||
renderAppender: CustomAppender || appender, | ||
|
||
// Ensure block toolbar is not too far removed from item | ||
// being edited when in vertical mode. | ||
// see: https://github.com/WordPress/gutenberg/pull/34615. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -164,4 +164,12 @@ | |
padding: 0; | ||
} | ||
} | ||
|
||
// Override behavior that hides the navigation block's appender when it's deselected. | ||
.block-editor-block-list__block:not(.is-selected):not(.has-child-selected):not(.block-editor-block-list__layout) { | ||
.block-editor-block-list__layout > .block-list-appender .block-list-appender__toggle { | ||
opacity: unset; | ||
transform: unset; | ||
} | ||
} | ||
Comment on lines
+169
to
+174
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The block editor also has CSS to hide the appender, so that needs to be unset. |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,76 @@ | ||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { useSelect } from '@wordpress/data'; | ||
import { addFilter } from '@wordpress/hooks'; | ||
import { createHigherOrderComponent } from '@wordpress/compose'; | ||
import { | ||
InnerBlocks, | ||
store as blockEditorStore, | ||
} from '@wordpress/block-editor'; | ||
|
||
function CustomAppender() { | ||
return <InnerBlocks.ButtonBlockAppender isToggle />; | ||
} | ||
|
||
function EnhancedNavigationBlock( { blockEdit: BlockEdit, ...props } ) { | ||
const clientId = props.clientId; | ||
const { | ||
noBlockSelected, | ||
isSelected, | ||
isImmediateParentOfSelectedBlock, | ||
selectedBlockHasDescendants, | ||
} = useSelect( | ||
( select ) => { | ||
const { | ||
getClientIdsOfDescendants, | ||
hasSelectedInnerBlock, | ||
getSelectedBlockClientId, | ||
} = select( blockEditorStore ); | ||
const _isImmediateParentOfSelectedBlock = hasSelectedInnerBlock( | ||
clientId, | ||
false | ||
); | ||
const selectedBlockId = getSelectedBlockClientId(); | ||
const _selectedBlockHasDescendants = !! getClientIdsOfDescendants( [ | ||
selectedBlockId, | ||
] )?.length; | ||
|
||
return { | ||
isSelected: selectedBlockId === clientId, | ||
noBlockSelected: ! selectedBlockId, | ||
isImmediateParentOfSelectedBlock: _isImmediateParentOfSelectedBlock, | ||
selectedBlockHasDescendants: _selectedBlockHasDescendants, | ||
}; | ||
}, | ||
[ clientId ] | ||
); | ||
|
||
const customAppender = | ||
noBlockSelected || | ||
isSelected || | ||
( isImmediateParentOfSelectedBlock && ! selectedBlockHasDescendants ) | ||
? CustomAppender | ||
: false; | ||
|
||
return <BlockEdit { ...props } customAppender={ customAppender } />; | ||
} | ||
|
||
const addNavigationEditorCustomAppender = createHigherOrderComponent( | ||
( BlockEdit ) => ( props ) => { | ||
if ( props.name !== 'core/navigation' ) { | ||
return <BlockEdit { ...props } />; | ||
} | ||
|
||
// Use a separate component so that `useSelect` only run on the navigation block. | ||
return <EnhancedNavigationBlock blockEdit={ BlockEdit } { ...props } />; | ||
}, | ||
'withNavigationEditorCustomAppender' | ||
); | ||
|
||
export default () => | ||
addFilter( | ||
'editor.BlockEdit', | ||
'core/edit-navigation/with-navigation-editor-custom-appender', | ||
addNavigationEditorCustomAppender | ||
); |
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's some code smell with this classname, it was added to
BlockListAppender
at some point in the past, but it should really be part of theButtonBlockAppender
component. It's also missing theblock-editor
prefix. I'll work on a separate PR to fix it.