From c3d10e3aef4f0e92b3bfdcb0427c0229f9b1003c Mon Sep 17 00:00:00 2001 From: George Mamadashvili Date: Thu, 28 Apr 2022 22:13:54 +0400 Subject: [PATCH 1/2] Navigation: Try improving performance --- packages/block-library/src/navigation-link/edit.js | 13 +++++-------- .../block-library/src/navigation-submenu/edit.js | 12 ++++-------- .../src/navigation/edit/inner-blocks.js | 8 ++++---- 3 files changed, 13 insertions(+), 20 deletions(-) diff --git a/packages/block-library/src/navigation-link/edit.js b/packages/block-library/src/navigation-link/edit.js index fe629a533e7c9..57f505a117676 100644 --- a/packages/block-library/src/navigation-link/edit.js +++ b/packages/block-library/src/navigation-link/edit.js @@ -459,9 +459,9 @@ export default function NavigationLinkEdit( { ( select ) => { const { getBlocks, + getBlockCount, getBlockName, getBlockRootClientId, - getClientIdsOfDescendants, hasSelectedInnerBlock, getSelectedBlockClientId, getBlockParentsByBlockName, @@ -469,9 +469,6 @@ export default function NavigationLinkEdit( { const selectedBlockId = getSelectedBlockClientId(); - const descendants = getClientIdsOfDescendants( [ clientId ] ) - .length; - return { innerBlocks: getBlocks( clientId ), isAtMaxNesting: @@ -490,10 +487,10 @@ export default function NavigationLinkEdit( { clientId, false ), - hasDescendants: !! descendants, - selectedBlockHasDescendants: !! getClientIdsOfDescendants( [ - selectedBlockId, - ] )?.length, + hasDescendants: !! getBlockCount( clientId ), + selectedBlockHasDescendants: !! getBlockCount( + selectedBlockId + ), userCanCreatePages: select( coreStore ).canUser( 'create', 'pages' diff --git a/packages/block-library/src/navigation-submenu/edit.js b/packages/block-library/src/navigation-submenu/edit.js index abbea4c63d8b4..ef7c72e23f013 100644 --- a/packages/block-library/src/navigation-submenu/edit.js +++ b/packages/block-library/src/navigation-submenu/edit.js @@ -317,23 +317,19 @@ export default function NavigationSubmenuEdit( { } = useSelect( ( select ) => { const { - getClientIdsOfDescendants, hasSelectedInnerBlock, getSelectedBlockClientId, getBlockParentsByBlockName, getBlock, + getBlockCount, + getBlockOrder, } = select( blockEditorStore ); let _onlyDescendantIsEmptyLink; const selectedBlockId = getSelectedBlockClientId(); - const descendants = getClientIdsOfDescendants( [ clientId ] ) - .length; - - const selectedBlockDescendants = getClientIdsOfDescendants( [ - selectedBlockId, - ] ); + const selectedBlockDescendants = getBlockOrder( selectedBlockId ); // Check for a single descendant in the submenu. If that block // is a link block in a "placeholder" state with no label then @@ -360,7 +356,7 @@ export default function NavigationSubmenuEdit( { clientId, false ), - hasDescendants: !! descendants, + hasDescendants: !! getBlockCount( clientId ), selectedBlockHasDescendants: !! selectedBlockDescendants?.length, userCanCreatePages: select( coreStore ).canUser( 'create', diff --git a/packages/block-library/src/navigation/edit/inner-blocks.js b/packages/block-library/src/navigation/edit/inner-blocks.js index 4b11cf0cafe5d..c76af873dd6e8 100644 --- a/packages/block-library/src/navigation/edit/inner-blocks.js +++ b/packages/block-library/src/navigation/edit/inner-blocks.js @@ -49,7 +49,7 @@ export default function NavigationInnerBlocks( { } = useSelect( ( select ) => { const { - getClientIdsOfDescendants, + getBlockCount, hasSelectedInnerBlock, getSelectedBlockClientId, } = select( blockEditorStore ); @@ -60,9 +60,9 @@ export default function NavigationInnerBlocks( { clientId, false ), - selectedBlockHasDescendants: !! getClientIdsOfDescendants( [ - selectedBlockId, - ] )?.length, + selectedBlockHasDescendants: !! getBlockCount( + selectedBlockId + ), // This prop is already available but computing it here ensures it's // fresh compared to isImmediateParentOfSelectedBlock. From 8da47fef0d82473c3d1b217b5ab080ede244cf96 Mon Sep 17 00:00:00 2001 From: George Mamadashvili Date: Fri, 29 Apr 2022 08:53:05 +0400 Subject: [PATCH 2/2] Update variable names and cleanup --- .../block-library/src/navigation-link/edit.js | 18 ++++----------- .../src/navigation-submenu/edit.js | 22 +++++++++---------- .../src/navigation/edit/inner-blocks.js | 10 ++++----- 3 files changed, 19 insertions(+), 31 deletions(-) diff --git a/packages/block-library/src/navigation-link/edit.js b/packages/block-library/src/navigation-link/edit.js index 57f505a117676..f50fe3263b22b 100644 --- a/packages/block-library/src/navigation-link/edit.js +++ b/packages/block-library/src/navigation-link/edit.js @@ -452,7 +452,7 @@ export default function NavigationLinkEdit( { isAtMaxNesting, isTopLevelLink, isParentOfSelectedBlock, - hasDescendants, + hasChildren, userCanCreatePages, userCanCreatePosts, } = useSelect( @@ -463,12 +463,9 @@ export default function NavigationLinkEdit( { getBlockName, getBlockRootClientId, hasSelectedInnerBlock, - getSelectedBlockClientId, getBlockParentsByBlockName, } = select( blockEditorStore ); - const selectedBlockId = getSelectedBlockClientId(); - return { innerBlocks: getBlocks( clientId ), isAtMaxNesting: @@ -483,14 +480,7 @@ export default function NavigationLinkEdit( { clientId, true ), - isImmediateParentOfSelectedBlock: hasSelectedInnerBlock( - clientId, - false - ), - hasDescendants: !! getBlockCount( clientId ), - selectedBlockHasDescendants: !! getBlockCount( - selectedBlockId - ), + hasChildren: !! getBlockCount( clientId ), userCanCreatePages: select( coreStore ).canUser( 'create', 'pages' @@ -534,7 +524,7 @@ export default function NavigationLinkEdit( { setIsLinkOpen( true ); } // If block has inner blocks, transform to Submenu. - if ( hasDescendants ) { + if ( hasChildren ) { transformToSubmenu(); } }, [] ); @@ -644,7 +634,7 @@ export default function NavigationLinkEdit( { 'is-editing': isSelected || isParentOfSelectedBlock, 'is-dragging-within': isDraggingWithin, 'has-link': !! url, - 'has-child': hasDescendants, + 'has-child': hasChildren, 'has-text-color': !! textColor || !! customTextColor, [ getColorClassName( 'color', textColor ) ]: !! textColor, 'has-background': !! backgroundColor || customBackgroundColor, diff --git a/packages/block-library/src/navigation-submenu/edit.js b/packages/block-library/src/navigation-submenu/edit.js index ef7c72e23f013..58486607fe578 100644 --- a/packages/block-library/src/navigation-submenu/edit.js +++ b/packages/block-library/src/navigation-submenu/edit.js @@ -309,8 +309,8 @@ export default function NavigationSubmenuEdit( { isTopLevelItem, isParentOfSelectedBlock, isImmediateParentOfSelectedBlock, - hasDescendants, - selectedBlockHasDescendants, + hasChildren, + selectedBlockHasChildren, userCanCreatePages, userCanCreatePosts, onlyDescendantIsEmptyLink, @@ -329,13 +329,13 @@ export default function NavigationSubmenuEdit( { const selectedBlockId = getSelectedBlockClientId(); - const selectedBlockDescendants = getBlockOrder( selectedBlockId ); + const selectedBlockChildren = getBlockOrder( selectedBlockId ); // Check for a single descendant in the submenu. If that block // is a link block in a "placeholder" state with no label then // we can consider as an "empty" link. - if ( selectedBlockDescendants?.length === 1 ) { - const singleBlock = getBlock( selectedBlockDescendants[ 0 ] ); + if ( selectedBlockChildren?.length === 1 ) { + const singleBlock = getBlock( selectedBlockChildren[ 0 ] ); _onlyDescendantIsEmptyLink = singleBlock?.name === 'core/navigation-link' && @@ -356,8 +356,8 @@ export default function NavigationSubmenuEdit( { clientId, false ), - hasDescendants: !! getBlockCount( clientId ), - selectedBlockHasDescendants: !! selectedBlockDescendants?.length, + hasChildren: !! getBlockCount( clientId ), + selectedBlockHasChildren: !! selectedBlockChildren?.length, userCanCreatePages: select( coreStore ).canUser( 'create', 'pages' @@ -477,7 +477,7 @@ export default function NavigationSubmenuEdit( { 'is-editing': isSelected || isParentOfSelectedBlock, 'is-dragging-within': isDraggingWithin, 'has-link': !! url, - 'has-child': hasDescendants, + 'has-child': hasChildren, 'has-text-color': !! textColor || !! customTextColor, [ getColorClassName( 'color', textColor ) ]: !! textColor, 'has-background': !! backgroundColor || customBackgroundColor, @@ -534,9 +534,9 @@ export default function NavigationSubmenuEdit( { renderAppender: isSelected || ( isImmediateParentOfSelectedBlock && - ! selectedBlockHasDescendants ) || + ! selectedBlockHasChildren ) || // Show the appender while dragging to allow inserting element between item and the appender. - hasDescendants + hasChildren ? InnerBlocks.ButtonBlockAppender : false, } @@ -550,7 +550,7 @@ export default function NavigationSubmenuEdit( { } const canConvertToLink = - ! selectedBlockHasDescendants || onlyDescendantIsEmptyLink; + ! selectedBlockHasChildren || onlyDescendantIsEmptyLink; return ( diff --git a/packages/block-library/src/navigation/edit/inner-blocks.js b/packages/block-library/src/navigation/edit/inner-blocks.js index c76af873dd6e8..63ba9fc9e774d 100644 --- a/packages/block-library/src/navigation/edit/inner-blocks.js +++ b/packages/block-library/src/navigation/edit/inner-blocks.js @@ -44,7 +44,7 @@ export default function NavigationInnerBlocks( { } ) { const { isImmediateParentOfSelectedBlock, - selectedBlockHasDescendants, + selectedBlockHasChildren, isSelected, } = useSelect( ( select ) => { @@ -60,9 +60,7 @@ export default function NavigationInnerBlocks( { clientId, false ), - selectedBlockHasDescendants: !! getBlockCount( - selectedBlockId - ), + selectedBlockHasChildren: !! getBlockCount( selectedBlockId ), // This prop is already available but computing it here ensures it's // fresh compared to isImmediateParentOfSelectedBlock. @@ -93,7 +91,7 @@ export default function NavigationInnerBlocks( { // appender. const parentOrChildHasSelection = isSelected || - ( isImmediateParentOfSelectedBlock && ! selectedBlockHasDescendants ); + ( isImmediateParentOfSelectedBlock && ! selectedBlockHasChildren ); const placeholder = useMemo( () => , [] ); @@ -127,7 +125,7 @@ export default function NavigationInnerBlocks( { renderAppender: isSelected || ( isImmediateParentOfSelectedBlock && - ! selectedBlockHasDescendants ) || + ! selectedBlockHasChildren ) || // Show the appender while dragging to allow inserting element between item and the appender. parentOrChildHasSelection ? InnerBlocks.ButtonBlockAppender