From 84c220b5c9f9be277b60e3c4c775b12405c75c98 Mon Sep 17 00:00:00 2001 From: scruffian Date: Tue, 28 Feb 2023 17:03:11 +0000 Subject: [PATCH 1/2] Revert: Navigation: Always create a fallback menu --- .../src/navigation/edit/index.js | 118 +++++------------- .../block-library/src/navigation/index.php | 55 +++----- .../specs/editor/blocks/navigation.spec.js | 15 +-- 3 files changed, 56 insertions(+), 132 deletions(-) diff --git a/packages/block-library/src/navigation/edit/index.js b/packages/block-library/src/navigation/edit/index.js index 8e6c4f9b9f6c8..dfc99d1441215 100644 --- a/packages/block-library/src/navigation/edit/index.js +++ b/packages/block-library/src/navigation/edit/index.js @@ -41,7 +41,7 @@ import { } from '@wordpress/components'; import { __, sprintf } from '@wordpress/i18n'; import { speak } from '@wordpress/a11y'; -import { createBlock, getBlockType } from '@wordpress/blocks'; +import { createBlock } from '@wordpress/blocks'; import { close, Icon } from '@wordpress/icons'; /** @@ -214,55 +214,6 @@ function Navigation( { [ navigationMenus ] ); - const handleUpdateMenu = useCallback( - ( menuId, options = { focusNavigationBlock: false } ) => { - const { focusNavigationBlock } = options; - setRef( menuId ); - if ( focusNavigationBlock ) { - selectBlock( clientId ); - } - }, - [ selectBlock, clientId, setRef ] - ); - - // This useEffect adds snackbar and speak status notices when menus are created. - // If there are no fallback navigation menus then we don't show these messages, - // because this means that we are creating the first, fallback navigation menu. - useEffect( () => { - hideNavigationMenuStatusNotice(); - - if ( fallbackNavigationMenus && isCreatingNavigationMenu ) { - speak( __( `Creating Navigation Menu.` ) ); - } - - if ( createNavigationMenuIsSuccess ) { - handleUpdateMenu( createNavigationMenuPost?.id, { - focusNavigationBlock: true, - } ); - - if ( fallbackNavigationMenus ) { - showNavigationMenuStatusNotice( - __( `Navigation Menu successfully created.` ) - ); - } - } - - if ( createNavigationMenuIsError ) { - showNavigationMenuStatusNotice( - __( 'Failed to create Navigation Menu.' ) - ); - } - }, [ - createNavigationMenuIsError, - createNavigationMenuIsSuccess, - handleUpdateMenu, - hideNavigationMenuStatusNotice, - isCreatingNavigationMenu, - showNavigationMenuStatusNotice, - createNavigationMenuPost?.id, - fallbackNavigationMenus, - ] ); - // Attempt to retrieve and prioritize any existing navigation menu unless: // - the are uncontrolled inner blocks already present in the block. // - the user is creating a new menu. @@ -316,7 +267,8 @@ function Navigation( { ! hasResolvedNavigationMenus || isConvertingClassicMenu || fallbackNavigationMenus?.length > 0 || - hasUnsavedBlocks + hasUnsavedBlocks || + ! classicMenus?.length ) { return; } @@ -325,42 +277,25 @@ function Navigation( { // a classic menu with a `primary` location or slug, // then create a new navigation menu based on it. // Otherwise, use the most recently created classic menu. - if ( classicMenus?.length ) { - const primaryMenus = classicMenus.filter( - ( classicMenu ) => - classicMenu.locations.includes( 'primary' ) || - classicMenu.slug === 'primary' - ); + const primaryMenus = classicMenus.filter( + ( classicMenu ) => + classicMenu.locations.includes( 'primary' ) || + classicMenu.slug === 'primary' + ); - if ( primaryMenus.length ) { - convertClassicMenu( - primaryMenus[ 0 ].id, - primaryMenus[ 0 ].name, - 'publish' - ); - } else { - classicMenus.sort( ( a, b ) => { - return b.id - a.id; - } ); - convertClassicMenu( - classicMenus[ 0 ].id, - classicMenus[ 0 ].name, - 'publish' - ); - } + if ( primaryMenus.length ) { + convertClassicMenu( + primaryMenus[ 0 ].id, + primaryMenus[ 0 ].name, + 'publish' + ); } else { - // If there are no fallback navigation menus and no classic menus, - // then create a new navigation menu. - - // Check that we have a page-list block type. - let defaultBlocks = []; - if ( getBlockType( 'core/page-list' ) ) { - defaultBlocks = [ createBlock( 'core/page-list' ) ]; - } - - createNavigationMenu( - 'Navigation', // TODO - use the template slug in future - defaultBlocks, + classicMenus.sort( ( a, b ) => { + return b.id - a.id; + } ); + convertClassicMenu( + classicMenus[ 0 ].id, + classicMenus[ 0 ].name, 'publish' ); } @@ -394,6 +329,19 @@ function Navigation( { classicMenus?.length === 0 && ! hasUncontrolledInnerBlocks; + useEffect( () => { + if ( isPlaceholder ) { + /** + * this fallback only displays (both in editor and on front) + * the list of pages block if no menu is available as a fallback. + * We don't want the fallback to request a save, + * nor to be undoable, hence we mark it non persistent. + */ + __unstableMarkNextChangeAsNotPersistent(); + replaceInnerBlocks( clientId, [ createBlock( 'core/page-list' ) ] ); + } + }, [ clientId, isPlaceholder, ref ] ); + // "loading" state: // - there is a menu creation process in progress. // - there is a classic menu conversion process in progress. diff --git a/packages/block-library/src/navigation/index.php b/packages/block-library/src/navigation/index.php index 5ec43046d6618..ac52492821675 100644 --- a/packages/block-library/src/navigation/index.php +++ b/packages/block-library/src/navigation/index.php @@ -431,37 +431,6 @@ function block_core_navigation_block_contains_core_navigation( $inner_blocks ) { return false; } -/** - * Create and returns a navigation menu containing a page-list as a fallback. - * - * @return array the newly created navigation menu. - */ -function block_core_navigation_get_default_pages_fallback() { - $registry = WP_Block_Type_Registry::get_instance(); - - // If `core/page-list` is not registered then use empty blocks. - $default_blocks = $registry->is_registered( 'core/page-list' ) ? '' : ''; - - // Create a new navigation menu from the fallback blocks. - $wp_insert_post_result = wp_insert_post( - array( - 'post_content' => $default_blocks, - 'post_title' => _x( 'Navigation', 'Title of a Navigation menu' ), - 'post_name' => 'navigation', - 'post_status' => 'publish', - 'post_type' => 'wp_navigation', - ), - true // So that we can check whether the result is an error. - ); - - if ( is_wp_error( $wp_insert_post_result ) ) { - return; - } - - // Fetch the most recently published navigation which will be the default one created above. - return block_core_navigation_get_most_recently_published_navigation(); -} - /** * Retrieves the appropriate fallback to be used on the front of the * site when there is no menu assigned to the Nav block. @@ -472,7 +441,18 @@ function block_core_navigation_get_default_pages_fallback() { * @return array the array of blocks to be used as a fallback. */ function block_core_navigation_get_fallback_blocks() { - // Get the most recently published Navigation post. + $page_list_fallback = array( + array( + 'blockName' => 'core/page-list', + ), + ); + + $registry = WP_Block_Type_Registry::get_instance(); + + // If `core/page-list` is not registered then return empty blocks. + $fallback_blocks = $registry->is_registered( 'core/page-list' ) ? $page_list_fallback : array(); + + // Default to a list of Pages. $navigation_post = block_core_navigation_get_most_recently_published_navigation(); // If there are no navigation posts then try to find a classic menu @@ -481,15 +461,14 @@ function block_core_navigation_get_fallback_blocks() { $navigation_post = block_core_navigation_maybe_use_classic_menu_fallback(); } - // If there are no navigation posts then default to a list of Pages. - if ( ! $navigation_post ) { - $navigation_post = block_core_navigation_get_default_pages_fallback(); - } - - // Use the first non-empty Navigation as fallback, there should always be one. + // Use the first non-empty Navigation as fallback if available. if ( $navigation_post ) { $parsed_blocks = parse_blocks( $navigation_post->post_content ); $maybe_fallback = block_core_navigation_filter_out_empty_blocks( $parsed_blocks ); + + // Normalizing blocks may result in an empty array of blocks if they were all `null` blocks. + // In this case default to the (Page List) fallback. + $fallback_blocks = ! empty( $maybe_fallback ) ? $maybe_fallback : $fallback_blocks; } // Normalizing blocks may result in an empty array of blocks if they were all `null` blocks. diff --git a/test/e2e/specs/editor/blocks/navigation.spec.js b/test/e2e/specs/editor/blocks/navigation.spec.js index 3ebf62c45ac82..b6e0b832eab5a 100644 --- a/test/e2e/specs/editor/blocks/navigation.spec.js +++ b/test/e2e/specs/editor/blocks/navigation.spec.js @@ -29,7 +29,6 @@ test.describe( test( 'default to a list of pages if there are no menus', async ( { admin, editor, - requestUtils, } ) => { await admin.createNewPost(); await editor.insertBlock( { name: 'core/navigation' } ); @@ -48,14 +47,12 @@ test.describe( // Check the markup of the block is correct. await editor.publishPost(); - const navigationMenus = await requestUtils.getNavigationMenus(); - const latestNavigationMenu = navigationMenus[ 0 ]; - await expect.poll( editor.getBlocks ).toMatchObject( [ - { - name: 'core/navigation', - attributes: { ref: latestNavigationMenu.id }, - }, - ] ); + const content = await editor.getEditedPostContent(); + expect( content ).toBe( + ` + +` + ); } ); test( 'default to my only existing menu', async ( { From 38fc111bc6c6c0ba8dbc2f87ecdc0610648b67f2 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Tue, 7 Mar 2023 08:56:31 +0000 Subject: [PATCH 2/2] Fix lint issues --- .../src/navigation/edit/index.js | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/packages/block-library/src/navigation/edit/index.js b/packages/block-library/src/navigation/edit/index.js index dfc99d1441215..9d61c2339d73c 100644 --- a/packages/block-library/src/navigation/edit/index.js +++ b/packages/block-library/src/navigation/edit/index.js @@ -214,6 +214,17 @@ function Navigation( { [ navigationMenus ] ); + const handleUpdateMenu = useCallback( + ( menuId, options = { focusNavigationBlock: false } ) => { + const { focusNavigationBlock } = options; + setRef( menuId ); + if ( focusNavigationBlock ) { + selectBlock( clientId ); + } + }, + [ selectBlock, clientId, setRef ] + ); + // Attempt to retrieve and prioritize any existing navigation menu unless: // - the are uncontrolled inner blocks already present in the block. // - the user is creating a new menu. @@ -340,7 +351,13 @@ function Navigation( { __unstableMarkNextChangeAsNotPersistent(); replaceInnerBlocks( clientId, [ createBlock( 'core/page-list' ) ] ); } - }, [ clientId, isPlaceholder, ref ] ); + }, [ + clientId, + isPlaceholder, + ref, + __unstableMarkNextChangeAsNotPersistent, + replaceInnerBlocks, + ] ); // "loading" state: // - there is a menu creation process in progress.