From fe76803f608ba31a3e134601960c0bd3f7cbcf4b Mon Sep 17 00:00:00 2001 From: tellthemachines Date: Thu, 30 Sep 2021 14:37:24 +1000 Subject: [PATCH] Insert Links by default in Navigation block (#34899) * Insert Links by default in Navigation block * Fix errors. * Open links by default with submenus * Insert page item by default. * Fix re-rendering loop. * Actually fix re-rendering loop. * Switch back to inserting generic link. * Separate direct insert from default block props * Syntactic embellishment * Update e2e tests and add a new test for Navigation block. * Try direct insert as a match function. --- .../src/components/inner-blocks/index.js | 4 ++ .../use-nested-settings-update.js | 37 +++++++---- .../src/components/inserter/index.js | 18 +++++- packages/block-editor/src/store/selectors.js | 33 ++++++++++ .../src/navigation-submenu/edit.js | 4 ++ packages/block-library/src/navigation/edit.js | 12 ++++ .../__snapshots__/navigation.test.js.snap | 10 +++ .../experiments/blocks/navigation.test.js | 61 ++++++++++++++----- .../experiments/navigation-editor.test.js | 5 -- 9 files changed, 149 insertions(+), 35 deletions(-) diff --git a/packages/block-editor/src/components/inner-blocks/index.js b/packages/block-editor/src/components/inner-blocks/index.js index dcf07e7391fc8..5a8bc95238fd5 100644 --- a/packages/block-editor/src/components/inner-blocks/index.js +++ b/packages/block-editor/src/components/inner-blocks/index.js @@ -42,6 +42,8 @@ function UncontrolledInnerBlocks( props ) { const { clientId, allowedBlocks, + __experimentalDefaultBlock, + __experimentalDirectInsert, template, templateLock, wrapperRef, @@ -57,6 +59,8 @@ function UncontrolledInnerBlocks( props ) { useNestedSettingsUpdate( clientId, allowedBlocks, + __experimentalDefaultBlock, + __experimentalDirectInsert, templateLock, captureToolbars, orientation, diff --git a/packages/block-editor/src/components/inner-blocks/use-nested-settings-update.js b/packages/block-editor/src/components/inner-blocks/use-nested-settings-update.js index 8a276be0a69e4..8412b221c9b53 100644 --- a/packages/block-editor/src/components/inner-blocks/use-nested-settings-update.js +++ b/packages/block-editor/src/components/inner-blocks/use-nested-settings-update.js @@ -18,21 +18,26 @@ import { getLayoutType } from '../../layouts'; * the block-editor store, then the store is updated with the new settings which * came from props. * - * @param {string} clientId The client ID of the block to update. - * @param {string[]} allowedBlocks An array of block names which are permitted - * in inner blocks. - * @param {string} [templateLock] The template lock specified for the inner - * blocks component. (e.g. "all") - * @param {boolean} captureToolbars Whether or children toolbars should be shown - * in the inner blocks component rather than on - * the child block. - * @param {string} orientation The direction in which the block - * should face. - * @param {Object} layout The layout object for the block container. + * @param {string} clientId The client ID of the block to update. + * @param {string[]} allowedBlocks An array of block names which are permitted + * in inner blocks. + * @param {?Array} __experimentalDefaultBlock The default block to insert: [ blockName, { blockAttributes } ]. + * @param {?Function|boolean} __experimentalDirectInsert If a default block should be inserted directly by the + * appender. + * @param {string} [templateLock] The template lock specified for the inner + * blocks component. (e.g. "all") + * @param {boolean} captureToolbars Whether or children toolbars should be shown + * in the inner blocks component rather than on + * the child block. + * @param {string} orientation The direction in which the block + * should face. + * @param {Object} layout The layout object for the block container. */ export default function useNestedSettingsUpdate( clientId, allowedBlocks, + __experimentalDefaultBlock, + __experimentalDirectInsert, templateLock, captureToolbars, orientation, @@ -83,6 +88,14 @@ export default function useNestedSettingsUpdate( newSettings.orientation = layoutType.getOrientation( layout ); } + if ( __experimentalDefaultBlock !== undefined ) { + newSettings.__experimentalDefaultBlock = __experimentalDefaultBlock; + } + + if ( __experimentalDirectInsert !== undefined ) { + newSettings.__experimentalDirectInsert = __experimentalDirectInsert; + } + if ( ! isShallowEqual( blockListSettings, newSettings ) ) { updateBlockListSettings( clientId, newSettings ); } @@ -90,6 +103,8 @@ export default function useNestedSettingsUpdate( clientId, blockListSettings, _allowedBlocks, + __experimentalDefaultBlock, + __experimentalDirectInsert, templateLock, parentLock, captureToolbars, diff --git a/packages/block-editor/src/components/inserter/index.js b/packages/block-editor/src/components/inserter/index.js index c4231235f6035..d08ac5ff272a0 100644 --- a/packages/block-editor/src/components/inserter/index.js +++ b/packages/block-editor/src/components/inserter/index.js @@ -102,6 +102,7 @@ class Inserter extends Component { disabled, blockTitle, hasSingleBlockType, + directInsertBlock, toggleProps, hasItems, renderToggle = defaultRenderToggle, @@ -113,6 +114,7 @@ class Inserter extends Component { disabled: disabled || ! hasItems, blockTitle, hasSingleBlockType, + directInsertBlock, toggleProps, } ); } @@ -168,12 +170,13 @@ class Inserter extends Component { const { position, hasSingleBlockType, + directInsertBlock, insertOnlyAllowedBlock, __experimentalIsQuick: isQuick, onSelectOrClose, } = this.props; - if ( hasSingleBlockType ) { + if ( hasSingleBlockType || directInsertBlock?.length ) { return this.renderToggle( { onToggle: insertOnlyAllowedBlock } ); } @@ -202,6 +205,7 @@ export default compose( [ getBlockRootClientId, hasInserterItems, __experimentalGetAllowedBlocks, + __experimentalGetDirectInsertBlock, } = select( blockEditorStore ); const { getBlockVariations } = select( blocksStore ); @@ -210,6 +214,10 @@ export default compose( [ const allowedBlocks = __experimentalGetAllowedBlocks( rootClientId ); + const directInsertBlock = __experimentalGetDirectInsertBlock( + rootClientId + ); + const hasSingleBlockType = size( allowedBlocks ) === 1 && size( @@ -226,6 +234,7 @@ export default compose( [ hasSingleBlockType, blockTitle: allowedBlockType ? allowedBlockType.title : '', allowedBlockType, + directInsertBlock, rootClientId, }; } ), @@ -238,10 +247,11 @@ export default compose( [ isAppender, hasSingleBlockType, allowedBlockType, + directInsertBlock, onSelectOrClose, } = ownProps; - if ( ! hasSingleBlockType ) { + if ( ! hasSingleBlockType && ! directInsertBlock?.length ) { return; } @@ -274,7 +284,9 @@ export default compose( [ const { insertBlock } = dispatch( blockEditorStore ); - const blockToInsert = createBlock( allowedBlockType.name ); + const blockToInsert = directInsertBlock?.length + ? createBlock( ...directInsertBlock ) + : createBlock( allowedBlockType.name ); insertBlock( blockToInsert, getInsertionIndex(), rootClientId ); diff --git a/packages/block-editor/src/store/selectors.js b/packages/block-editor/src/store/selectors.js index 8e9f3792b0e0d..69d63048403ca 100644 --- a/packages/block-editor/src/store/selectors.js +++ b/packages/block-editor/src/store/selectors.js @@ -1792,6 +1792,39 @@ export const __experimentalGetAllowedBlocks = createSelector( ] ); +/** + * Returns the block to be directly inserted by the block appender. + * + * @param {Object} state Editor state. + * @param {?string} rootClientId Optional root client ID of block list. + * + * @return {?Array} The block type to be directly inserted. + */ +export const __experimentalGetDirectInsertBlock = createSelector( + ( state, rootClientId = null ) => { + if ( ! rootClientId ) { + return; + } + const defaultBlock = + state.blockListSettings[ rootClientId ]?.__experimentalDefaultBlock; + const directInsert = + state.blockListSettings[ rootClientId ]?.__experimentalDirectInsert; + if ( ! defaultBlock || ! directInsert ) { + return; + } + if ( typeof directInsert === 'function' ) { + return directInsert( getBlock( state, rootClientId ) ) + ? defaultBlock + : null; + } + return defaultBlock; + }, + ( state, rootClientId ) => [ + state.blockListSettings[ rootClientId ], + state.blocks.tree[ rootClientId ], + ] +); + const checkAllowListRecursive = ( blocks, allowedBlockTypes ) => { if ( isBoolean( allowedBlockTypes ) ) { return allowedBlockTypes; diff --git a/packages/block-library/src/navigation-submenu/edit.js b/packages/block-library/src/navigation-submenu/edit.js index 90e713893832e..7588cd6b579a8 100644 --- a/packages/block-library/src/navigation-submenu/edit.js +++ b/packages/block-library/src/navigation-submenu/edit.js @@ -50,6 +50,8 @@ import { name } from './block.json'; const ALLOWED_BLOCKS = [ 'core/navigation-link', 'core/navigation-submenu' ]; +const DEFAULT_BLOCK = [ 'core/navigation-link' ]; + const MAX_NESTING = 5; /** @@ -504,6 +506,8 @@ export default function NavigationSubmenuEdit( { }, { allowedBlocks: ALLOWED_BLOCKS, + __experimentalDefaultBlock: DEFAULT_BLOCK, + __experimentalDirectInsert: true, renderAppender: isSelected || ( isImmediateParentOfSelectedBlock && diff --git a/packages/block-library/src/navigation/edit.js b/packages/block-library/src/navigation/edit.js index b9e9fd8e53184..d5ef92a4c894f 100644 --- a/packages/block-library/src/navigation/edit.js +++ b/packages/block-library/src/navigation/edit.js @@ -50,6 +50,16 @@ const ALLOWED_BLOCKS = [ 'core/navigation-submenu', ]; +const DEFAULT_BLOCK = [ 'core/navigation-link' ]; + +const DIRECT_INSERT = ( block ) => { + return block.innerBlocks.every( + ( { name } ) => + name === 'core/navigation-link' || + name === 'core/navigation-submenu' + ); +}; + const LAYOUT = { type: 'default', alignments: [], @@ -164,6 +174,8 @@ function Navigation( { }, { allowedBlocks: ALLOWED_BLOCKS, + __experimentalDefaultBlock: DEFAULT_BLOCK, + __experimentalDirectInsert: DIRECT_INSERT, orientation: attributes.orientation, renderAppender: CustomAppender || appender, diff --git a/packages/e2e-tests/specs/experiments/blocks/__snapshots__/navigation.test.js.snap b/packages/e2e-tests/specs/experiments/blocks/__snapshots__/navigation.test.js.snap index f067002d11dac..1614ace7f64a4 100644 --- a/packages/e2e-tests/specs/experiments/blocks/__snapshots__/navigation.test.js.snap +++ b/packages/e2e-tests/specs/experiments/blocks/__snapshots__/navigation.test.js.snap @@ -44,6 +44,16 @@ exports[`Navigation Creating from existing Pages allows a navigation block to be " `; +exports[`Navigation Shows the quick inserter when the block contains non-navigation specific blocks 1`] = ` +" + + + + + +" +`; + exports[`Navigation allows an empty navigation block to be created and manually populated using a mixture of internal and external links 1`] = ` " diff --git a/packages/e2e-tests/specs/experiments/blocks/navigation.test.js b/packages/e2e-tests/specs/experiments/blocks/navigation.test.js index 40449589463c6..48859634ee45c 100644 --- a/packages/e2e-tests/specs/experiments/blocks/navigation.test.js +++ b/packages/e2e-tests/specs/experiments/blocks/navigation.test.js @@ -246,17 +246,6 @@ async function createEmptyNavBlock() { await startEmptyButton.click(); } -async function addLinkBlock() { - // Using 'click' here checks for regressions of https://github.com/WordPress/gutenberg/issues/18329, - // an issue where the block appender requires two clicks. - await page.click( '.wp-block-navigation .block-list-appender' ); - - const [ linkButton ] = await page.$x( - "//*[contains(@class, 'block-editor-inserter__quick-inserter')]//*[text()='Custom Link']" - ); - await linkButton.click(); -} - async function toggleSidebar() { await page.click( '.edit-post-header__settings button[aria-label="Settings"]' @@ -414,7 +403,7 @@ describe( 'Navigation', () => { await createEmptyNavBlock(); - await addLinkBlock(); + await page.click( '.wp-block-navigation .block-list-appender' ); // Add a link to the Link block. await updateActiveNavigationLink( { @@ -425,7 +414,7 @@ describe( 'Navigation', () => { await showBlockToolbar(); - await addLinkBlock(); + await page.click( '.wp-block-navigation .block-list-appender' ); // After adding a new block, search input should be shown immediately. // Verify that Escape would close the popover. @@ -477,7 +466,7 @@ describe( 'Navigation', () => { await createEmptyNavBlock(); - await addLinkBlock(); + await page.click( '.wp-block-navigation .block-list-appender' ); // Add a link to the Link block. await updateActiveNavigationLink( { @@ -487,7 +476,7 @@ describe( 'Navigation', () => { await showBlockToolbar(); - await addLinkBlock(); + await page.click( '.wp-block-navigation .block-list-appender' ); // Wait for URL input to be focused await page.waitForSelector( @@ -548,7 +537,7 @@ describe( 'Navigation', () => { // Create an empty nav block. await createEmptyNavBlock(); - await addLinkBlock(); + await page.click( '.wp-block-navigation .block-list-appender' ); // Wait for URL input to be focused await page.waitForSelector( @@ -642,6 +631,46 @@ describe( 'Navigation', () => { expect( navSubmenusLength ).toEqual( navButtonTogglesLength ); } ); + it( 'Shows the quick inserter when the block contains non-navigation specific blocks', async () => { + // Add the navigation block. + await insertBlock( 'Navigation' ); + + // Create an empty nav block. + await page.waitForSelector( '.wp-block-navigation-placeholder' ); + + await createEmptyNavBlock(); + + // Add a Link block first. + await page.click( '.wp-block-navigation .block-list-appender' ); + + // Add a link to the Link block. + await updateActiveNavigationLink( { + url: 'https://wordpress.org', + label: 'WP', + type: 'url', + } ); + + // Now add a different block type. + await insertBlock( 'Site Title' ); + + // Now try inserting another Link block via the quick inserter. + await page.click( '.wp-block-navigation .block-list-appender' ); + + const [ linkButton ] = await page.$x( + "//*[contains(@class, 'block-editor-inserter__quick-inserter')]//*[text()='Custom Link']" + ); + await linkButton.click(); + + await updateActiveNavigationLink( { + url: 'https://wordpress.org/news/', + label: 'WP News', + type: 'url', + } ); + + // Expect a Navigation block with two links and a Site Title. + expect( await getEditedPostContent() ).toMatchSnapshot(); + } ); + // The following tests are unstable, roughly around when https://github.com/WordPress/wordpress-develop/pull/1412 // landed. The block manually tests well, so let's skip to unblock other PRs and immediately follow up. cc @vcanales it.skip( 'loads frontend code only if the block is present', async () => { diff --git a/packages/e2e-tests/specs/experiments/navigation-editor.test.js b/packages/e2e-tests/specs/experiments/navigation-editor.test.js index cfeadac5ea5b7..78349d25eeee6 100644 --- a/packages/e2e-tests/specs/experiments/navigation-editor.test.js +++ b/packages/e2e-tests/specs/experiments/navigation-editor.test.js @@ -380,11 +380,6 @@ describe( 'Navigation editor', () => { ); await appender.click(); - const linkInserterItem = await page.waitForXPath( - '//button[@role="option"]//span[.="Custom Link"]' - ); - await linkInserterItem.click(); - await page.waitForSelector( 'input[aria-label="URL"]' ); // The link suggestions should be searchable.