From d3f9ee9d1d952523f0ece429fd039caf24b3dca0 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Tue, 8 Feb 2022 13:48:38 +0000 Subject: [PATCH 01/11] Encapsulate permissions checks --- packages/block-library/src/navigation/edit/index.js | 6 ------ .../src/navigation/edit/navigation-menu-selector.js | 13 ++++++++----- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/packages/block-library/src/navigation/edit/index.js b/packages/block-library/src/navigation/edit/index.js index 705a46845c105..ddbe453b94662 100644 --- a/packages/block-library/src/navigation/edit/index.js +++ b/packages/block-library/src/navigation/edit/index.js @@ -496,12 +496,6 @@ function Navigation( { onClose(); } } onCreateNew={ startWithEmptyMenu } - canUserCreateNavigation={ - canUserCreateNavigation - } - canUserSwitchNavigation={ - canSwitchNavigationMenu - } /> ) } diff --git a/packages/block-library/src/navigation/edit/navigation-menu-selector.js b/packages/block-library/src/navigation/edit/navigation-menu-selector.js index 168ef06caade9..bcfe046cd305e 100644 --- a/packages/block-library/src/navigation/edit/navigation-menu-selector.js +++ b/packages/block-library/src/navigation/edit/navigation-menu-selector.js @@ -19,14 +19,17 @@ export default function NavigationMenuSelector( { clientId, onSelect, onCreateNew, - canUserCreateNavigation = false, - canUserSwitchNavigation = false, } ) { const { menus: classicMenus, hasMenus: hasClassicMenus, } = useNavigationEntities(); - const { navigationMenus } = useNavigationMenu(); + + const { + navigationMenus, + canUserCreateNavigation, + canSwitchNavigationMenu, + } = useNavigationMenu(); const createNavigationMenu = useCreateNavigationMenu( clientId ); @@ -50,7 +53,7 @@ export default function NavigationMenuSelector( { ); const showSelectMenus = - ( canUserSwitchNavigation || canUserCreateNavigation ) && + ( canSwitchNavigationMenu || canUserCreateNavigation ) && ( navigationMenus?.length || hasClassicMenus ); if ( ! showSelectMenus ) { @@ -60,7 +63,7 @@ export default function NavigationMenuSelector( { return ( <> Date: Tue, 8 Feb 2022 14:06:56 +0000 Subject: [PATCH 02/11] Utilise primary component inside placeholder --- .../src/navigation/edit/index.js | 1 + .../edit/navigation-menu-selector.js | 3 +- .../src/navigation/edit/placeholder/index.js | 32 +++---------------- 3 files changed, 7 insertions(+), 29 deletions(-) diff --git a/packages/block-library/src/navigation/edit/index.js b/packages/block-library/src/navigation/edit/index.js index ddbe453b94662..51f187f9035e6 100644 --- a/packages/block-library/src/navigation/edit/index.js +++ b/packages/block-library/src/navigation/edit/index.js @@ -496,6 +496,7 @@ function Navigation( { onClose(); } } onCreateNew={ startWithEmptyMenu } + showTools={ true } /> ) } diff --git a/packages/block-library/src/navigation/edit/navigation-menu-selector.js b/packages/block-library/src/navigation/edit/navigation-menu-selector.js index bcfe046cd305e..134ac1d98e023 100644 --- a/packages/block-library/src/navigation/edit/navigation-menu-selector.js +++ b/packages/block-library/src/navigation/edit/navigation-menu-selector.js @@ -19,6 +19,7 @@ export default function NavigationMenuSelector( { clientId, onSelect, onCreateNew, + showTools = false, } ) { const { menus: classicMenus, @@ -75,7 +76,7 @@ export default function NavigationMenuSelector( { actionLabel={ __( "Switch to '%s'" ) } /> - { canUserCreateNavigation && ( + { showTools && canUserCreateNavigation && ( { __( 'Create new menu' ) } diff --git a/packages/block-library/src/navigation/edit/placeholder/index.js b/packages/block-library/src/navigation/edit/placeholder/index.js index 2ccac4e6e8617..f7275091478c8 100644 --- a/packages/block-library/src/navigation/edit/placeholder/index.js +++ b/packages/block-library/src/navigation/edit/placeholder/index.js @@ -13,8 +13,7 @@ import useNavigationEntities from '../../use-navigation-entities'; import PlaceholderPreview from './placeholder-preview'; import useNavigationMenu from '../../use-navigation-menu'; import useCreateNavigationMenu from '../use-create-navigation-menu'; -import useConvertClassicMenu from '../../use-convert-classic-menu'; -import ExistingMenusOptions from '../existing-menus-options'; +import NavigationMenuSelector from '../navigation-menu-selector'; export default function NavigationPlaceholder( { clientId, @@ -40,11 +39,8 @@ export default function NavigationPlaceholder( { onFinish( navigationMenu, blocks ); }; - const convertClassicMenu = useConvertClassicMenu( onFinishMenuCreation ); - const { isResolvingPages, - menus, isResolvingMenus, hasMenus, } = useNavigationEntities(); @@ -94,29 +90,9 @@ export default function NavigationPlaceholder( { popoverProps={ { isAlternate: true } } > { () => ( - - convertClassicMenu( - id, - name - ) - } - showClassicMenus={ - canUserCreateNavigation - } + ) } From 11f6d5885c2500d4f3608aff9418565e88d592fe Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Wed, 9 Feb 2022 09:39:00 +0000 Subject: [PATCH 03/11] Inline existing menus options --- .../navigation/edit/existing-menus-options.js | 70 ------------------- .../edit/navigation-menu-selector.js | 70 +++++++++++++------ 2 files changed, 50 insertions(+), 90 deletions(-) delete mode 100644 packages/block-library/src/navigation/edit/existing-menus-options.js diff --git a/packages/block-library/src/navigation/edit/existing-menus-options.js b/packages/block-library/src/navigation/edit/existing-menus-options.js deleted file mode 100644 index a47ae613489bf..0000000000000 --- a/packages/block-library/src/navigation/edit/existing-menus-options.js +++ /dev/null @@ -1,70 +0,0 @@ -/** - * WordPress dependencies - */ -import { MenuGroup, MenuItem } from '@wordpress/components'; -import { __, sprintf } from '@wordpress/i18n'; -import { decodeEntities } from '@wordpress/html-entities'; - -const ExistingMenusOptions = ( { - showNavigationMenus, - showClassicMenus = false, - navigationMenus, - classicMenus, - onSelectNavigationMenu, - onSelectClassicMenu, - actionLabel, -} ) => { - const hasNavigationMenus = !! navigationMenus?.length; - const hasClassicMenus = !! classicMenus?.length; - - /* translators: %s: The name of a menu. */ - const createActionLabel = __( "Create from '%s'" ); - - actionLabel = actionLabel || createActionLabel; - - return ( - <> - { showNavigationMenus && hasNavigationMenus && ( - - { navigationMenus.map( ( menu ) => { - const label = decodeEntities( menu.title.rendered ); - return ( - { - onSelectNavigationMenu( menu ); - } } - key={ menu.id } - aria-label={ sprintf( actionLabel, label ) } - > - { label } - - ); - } ) } - - ) } - { showClassicMenus && hasClassicMenus && ( - - { classicMenus.map( ( menu ) => { - const label = decodeEntities( menu.name ); - return ( - { - onSelectClassicMenu( menu ); - } } - key={ menu.id } - aria-label={ sprintf( - createActionLabel, - label - ) } - > - { label } - - ); - } ) } - - ) } - - ); -}; - -export default ExistingMenusOptions; diff --git a/packages/block-library/src/navigation/edit/navigation-menu-selector.js b/packages/block-library/src/navigation/edit/navigation-menu-selector.js index 134ac1d98e023..c526f87513e43 100644 --- a/packages/block-library/src/navigation/edit/navigation-menu-selector.js +++ b/packages/block-library/src/navigation/edit/navigation-menu-selector.js @@ -2,8 +2,8 @@ * WordPress dependencies */ import { MenuGroup, MenuItem } from '@wordpress/components'; -import { __ } from '@wordpress/i18n'; - +import { __, sprintf } from '@wordpress/i18n'; +import { decodeEntities } from '@wordpress/html-entities'; import { addQueryArgs } from '@wordpress/url'; /** @@ -13,7 +13,6 @@ import useNavigationMenu from '../use-navigation-menu'; import useNavigationEntities from '../use-navigation-entities'; import useConvertClassicMenu from '../use-convert-classic-menu'; import useCreateNavigationMenu from './use-create-navigation-menu'; -import ExistingMenusOptions from './existing-menus-options'; export default function NavigationMenuSelector( { clientId, @@ -21,10 +20,7 @@ export default function NavigationMenuSelector( { onCreateNew, showTools = false, } ) { - const { - menus: classicMenus, - hasMenus: hasClassicMenus, - } = useNavigationEntities(); + const { menus: classicMenus } = useNavigationEntities(); const { navigationMenus, @@ -53,28 +49,62 @@ export default function NavigationMenuSelector( { onFinishMenuCreation ); + const hasNavigationMenus = !! navigationMenus?.length; + const hasClassicMenus = !! classicMenus?.length; + const showNavigationMenus = !! canSwitchNavigationMenu; + const showClassicMenus = !! canUserCreateNavigation; const showSelectMenus = ( canSwitchNavigationMenu || canUserCreateNavigation ) && - ( navigationMenus?.length || hasClassicMenus ); + ( hasNavigationMenus || hasClassicMenus ); if ( ! showSelectMenus ) { return null; } + /* translators: %s: The name of a menu. */ + const actionLabel = __( "Create from '%s'" ); + return ( <> - - convertClassicMenuToBlocks( id, name ) - } - /* translators: %s: The name of a menu. */ - actionLabel={ __( "Switch to '%s'" ) } - /> + { showNavigationMenus && hasNavigationMenus && ( + + { navigationMenus.map( ( menu ) => { + const label = decodeEntities( menu.title.rendered ); + return ( + { + onSelect( menu ); + } } + key={ menu.id } + aria-label={ sprintf( actionLabel, label ) } + > + { label } + + ); + } ) } + + ) } + { showClassicMenus && hasClassicMenus && ( + + { classicMenus.map( ( menu ) => { + const label = decodeEntities( menu.name ); + return ( + { + convertClassicMenuToBlocks( + menu.id, + menu.name + ); + } } + key={ menu.id } + aria-label={ sprintf( actionLabel, label ) } + > + { label } + + ); + } ) } + + ) } { showTools && canUserCreateNavigation && ( From 9cf6f34e329b478de9ba71bafd8eee3622da3003 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Wed, 9 Feb 2022 09:47:03 +0000 Subject: [PATCH 04/11] Account for create vs switch in aria label depending on context --- .../block-library/src/navigation/edit/index.js | 2 ++ .../navigation/edit/navigation-menu-selector.js | 14 ++++++++++---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/packages/block-library/src/navigation/edit/index.js b/packages/block-library/src/navigation/edit/index.js index 51f187f9035e6..d094912c7e933 100644 --- a/packages/block-library/src/navigation/edit/index.js +++ b/packages/block-library/src/navigation/edit/index.js @@ -497,6 +497,8 @@ function Navigation( { } } onCreateNew={ startWithEmptyMenu } showTools={ true } + /* translators: %s: The name of a menu. */ + actionLabel={ __( "Switch to '%s'" ) } /> ) } diff --git a/packages/block-library/src/navigation/edit/navigation-menu-selector.js b/packages/block-library/src/navigation/edit/navigation-menu-selector.js index c526f87513e43..aeb63c049146c 100644 --- a/packages/block-library/src/navigation/edit/navigation-menu-selector.js +++ b/packages/block-library/src/navigation/edit/navigation-menu-selector.js @@ -19,7 +19,13 @@ export default function NavigationMenuSelector( { onSelect, onCreateNew, showTools = false, + actionLabel, } ) { + /* translators: %s: The name of a menu. */ + const createActionLabel = __( "Create from '%s'" ); + + actionLabel = actionLabel || createActionLabel; + const { menus: classicMenus } = useNavigationEntities(); const { @@ -61,9 +67,6 @@ export default function NavigationMenuSelector( { return null; } - /* translators: %s: The name of a menu. */ - const actionLabel = __( "Create from '%s'" ); - return ( <> { showNavigationMenus && hasNavigationMenus && ( @@ -97,7 +100,10 @@ export default function NavigationMenuSelector( { ); } } key={ menu.id } - aria-label={ sprintf( actionLabel, label ) } + aria-label={ sprintf( + createActionLabel, + label + ) } > { label } From c5c3377078cf636dce7c78fb8f9778f0aaec8db2 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Wed, 9 Feb 2022 09:57:49 +0000 Subject: [PATCH 05/11] Remove explicit truthy in prop Props @talldan --- packages/block-library/src/navigation/edit/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-library/src/navigation/edit/index.js b/packages/block-library/src/navigation/edit/index.js index d094912c7e933..d0bd2a82bea7a 100644 --- a/packages/block-library/src/navigation/edit/index.js +++ b/packages/block-library/src/navigation/edit/index.js @@ -496,9 +496,9 @@ function Navigation( { onClose(); } } onCreateNew={ startWithEmptyMenu } - showTools={ true } /* translators: %s: The name of a menu. */ actionLabel={ __( "Switch to '%s'" ) } + showTools /> ) } From eda20d52855f2dd545e98cbabc431b2339c582d1 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 10 Feb 2022 14:13:28 +0000 Subject: [PATCH 06/11] Rename vars to include "Menu" --- .../src/navigation/edit/navigation-menu-selector.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/block-library/src/navigation/edit/navigation-menu-selector.js b/packages/block-library/src/navigation/edit/navigation-menu-selector.js index aeb63c049146c..0be439a6dd607 100644 --- a/packages/block-library/src/navigation/edit/navigation-menu-selector.js +++ b/packages/block-library/src/navigation/edit/navigation-menu-selector.js @@ -30,7 +30,7 @@ export default function NavigationMenuSelector( { const { navigationMenus, - canUserCreateNavigation, + canUserCreateNavigation: canUserCreateNavigationMenu, canSwitchNavigationMenu, } = useNavigationMenu(); @@ -40,7 +40,7 @@ export default function NavigationMenuSelector( { blocks, navigationMenuTitle = null ) => { - if ( ! canUserCreateNavigation ) { + if ( ! canUserCreateNavigationMenu ) { return; } @@ -58,9 +58,9 @@ export default function NavigationMenuSelector( { const hasNavigationMenus = !! navigationMenus?.length; const hasClassicMenus = !! classicMenus?.length; const showNavigationMenus = !! canSwitchNavigationMenu; - const showClassicMenus = !! canUserCreateNavigation; + const showClassicMenus = !! canUserCreateNavigationMenu; const showSelectMenus = - ( canSwitchNavigationMenu || canUserCreateNavigation ) && + ( canSwitchNavigationMenu || canUserCreateNavigationMenu ) && ( hasNavigationMenus || hasClassicMenus ); if ( ! showSelectMenus ) { @@ -112,7 +112,7 @@ export default function NavigationMenuSelector( { ) } - { showTools && canUserCreateNavigation && ( + { showTools && canUserCreateNavigationMenu && ( { __( 'Create new menu' ) } From d2975e8cda321ca7c65ffb13a86dfb591e9f5da6 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 10 Feb 2022 14:16:17 +0000 Subject: [PATCH 07/11] Improve prop naming for clarity --- packages/block-library/src/navigation/edit/index.js | 2 +- .../src/navigation/edit/navigation-menu-selector.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/block-library/src/navigation/edit/index.js b/packages/block-library/src/navigation/edit/index.js index d0bd2a82bea7a..e780ae7372aeb 100644 --- a/packages/block-library/src/navigation/edit/index.js +++ b/packages/block-library/src/navigation/edit/index.js @@ -498,7 +498,7 @@ function Navigation( { onCreateNew={ startWithEmptyMenu } /* translators: %s: The name of a menu. */ actionLabel={ __( "Switch to '%s'" ) } - showTools + showManageActions /> ) } diff --git a/packages/block-library/src/navigation/edit/navigation-menu-selector.js b/packages/block-library/src/navigation/edit/navigation-menu-selector.js index 0be439a6dd607..69270da20651d 100644 --- a/packages/block-library/src/navigation/edit/navigation-menu-selector.js +++ b/packages/block-library/src/navigation/edit/navigation-menu-selector.js @@ -18,7 +18,7 @@ export default function NavigationMenuSelector( { clientId, onSelect, onCreateNew, - showTools = false, + showManageActions = false, actionLabel, } ) { /* translators: %s: The name of a menu. */ @@ -112,7 +112,7 @@ export default function NavigationMenuSelector( { ) } - { showTools && canUserCreateNavigationMenu && ( + { showManageActions && canUserCreateNavigationMenu && ( { __( 'Create new menu' ) } From 7e2c1587a0bb7baf7ff6489a9762b59f95e4f2be Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 11 Feb 2022 16:09:12 +0000 Subject: [PATCH 08/11] Improve conditionals around display of management options --- .../edit/navigation-menu-selector.js | 33 +++++++++++-------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/packages/block-library/src/navigation/edit/navigation-menu-selector.js b/packages/block-library/src/navigation/edit/navigation-menu-selector.js index 69270da20651d..5bf5126536f74 100644 --- a/packages/block-library/src/navigation/edit/navigation-menu-selector.js +++ b/packages/block-library/src/navigation/edit/navigation-menu-selector.js @@ -31,6 +31,7 @@ export default function NavigationMenuSelector( { const { navigationMenus, canUserCreateNavigation: canUserCreateNavigationMenu, + canUserUpdateNavigationEntity: canUserUpdateNavigationMenu, canSwitchNavigationMenu, } = useNavigationMenu(); @@ -112,20 +113,24 @@ export default function NavigationMenuSelector( { ) } - { showManageActions && canUserCreateNavigationMenu && ( - - - { __( 'Create new menu' ) } - - - { __( 'Manage menus' ) } - - - ) } + { showManageActions && + ( canUserCreateNavigationMenu || + canUserUpdateNavigationMenu ) && ( + + { canUserCreateNavigationMenu && ( + + { __( 'Create new menu' ) } + + ) } + + { __( 'Manage menus' ) } + + + ) } ); } From a086afe39a63ce52e5e538a088f5de7c6821ec94 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 11 Feb 2022 16:18:18 +0000 Subject: [PATCH 09/11] Wait for the custom link block --- packages/e2e-tests/specs/editor/blocks/navigation.test.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/e2e-tests/specs/editor/blocks/navigation.test.js b/packages/e2e-tests/specs/editor/blocks/navigation.test.js index 50c28445a7adb..b38aa0849ee30 100644 --- a/packages/e2e-tests/specs/editor/blocks/navigation.test.js +++ b/packages/e2e-tests/specs/editor/blocks/navigation.test.js @@ -119,6 +119,11 @@ async function selectClassicMenu( optionText ) { `//*[contains(@class, 'components-menu-item__item')][ text()="${ optionText }" ]` ); await theOption.click(); + + const navLinkBlockSelector = + '[aria-label="Editor content"][role="region"] [aria-label="Block: Custom Link"]'; + + await page.waitForSelector( navLinkBlockSelector ); } const PLACEHOLDER_ACTIONS_CLASS = 'wp-block-navigation-placeholder__actions'; From 7edc097ea1358ed9cca0305697dd9480665332af Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Mon, 14 Feb 2022 09:38:13 +0000 Subject: [PATCH 10/11] Wait for the menuItems response --- packages/e2e-tests/specs/editor/blocks/navigation.test.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/e2e-tests/specs/editor/blocks/navigation.test.js b/packages/e2e-tests/specs/editor/blocks/navigation.test.js index b38aa0849ee30..ccac0a1f935ab 100644 --- a/packages/e2e-tests/specs/editor/blocks/navigation.test.js +++ b/packages/e2e-tests/specs/editor/blocks/navigation.test.js @@ -120,10 +120,10 @@ async function selectClassicMenu( optionText ) { ); await theOption.click(); - const navLinkBlockSelector = - '[aria-label="Editor content"][role="region"] [aria-label="Block: Custom Link"]'; - - await page.waitForSelector( navLinkBlockSelector ); + await page.waitForResponse( + ( response ) => + response.url().includes( 'menu-items' ) && response.status() === 200 + ); } const PLACEHOLDER_ACTIONS_CLASS = 'wp-block-navigation-placeholder__actions'; From bdb24adf4f354d3c336ffd5434c3079257c7a41a Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Mon, 14 Feb 2022 10:40:50 +0000 Subject: [PATCH 11/11] Improve conditional --- .../edit/navigation-menu-selector.js | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/packages/block-library/src/navigation/edit/navigation-menu-selector.js b/packages/block-library/src/navigation/edit/navigation-menu-selector.js index 5bf5126536f74..4f5b61e71f3ad 100644 --- a/packages/block-library/src/navigation/edit/navigation-menu-selector.js +++ b/packages/block-library/src/navigation/edit/navigation-menu-selector.js @@ -60,6 +60,8 @@ export default function NavigationMenuSelector( { const hasClassicMenus = !! classicMenus?.length; const showNavigationMenus = !! canSwitchNavigationMenu; const showClassicMenus = !! canUserCreateNavigationMenu; + const hasManagePermissions = + canUserCreateNavigationMenu || canUserUpdateNavigationMenu; const showSelectMenus = ( canSwitchNavigationMenu || canUserCreateNavigationMenu ) && ( hasNavigationMenus || hasClassicMenus ); @@ -113,24 +115,22 @@ export default function NavigationMenuSelector( { ) } - { showManageActions && - ( canUserCreateNavigationMenu || - canUserUpdateNavigationMenu ) && ( - - { canUserCreateNavigationMenu && ( - - { __( 'Create new menu' ) } - - ) } - - { __( 'Manage menus' ) } + { showManageActions && hasManagePermissions && ( + + { canUserCreateNavigationMenu && ( + + { __( 'Create new menu' ) } - - ) } + ) } + + { __( 'Manage menus' ) } + + + ) } ); }