diff --git a/packages/block-editor/src/components/list-view/appender.js b/packages/block-editor/src/components/list-view/appender.js index a006b91e860c20..cb731bbf227a8b 100644 --- a/packages/block-editor/src/components/list-view/appender.js +++ b/packages/block-editor/src/components/list-view/appender.js @@ -4,7 +4,7 @@ import { useInstanceId } from '@wordpress/compose'; import { speak } from '@wordpress/a11y'; import { useSelect } from '@wordpress/data'; -import { forwardRef, useState, useEffect } from '@wordpress/element'; +import { forwardRef, useEffect } from '@wordpress/element'; import { __, sprintf } from '@wordpress/i18n'; /** @@ -12,11 +12,12 @@ import { __, sprintf } from '@wordpress/i18n'; */ import { store as blockEditorStore } from '../../store'; import useBlockDisplayTitle from '../block-title/use-block-display-title'; +import { useListViewContext } from './context'; import Inserter from '../inserter'; export const Appender = forwardRef( ( { nestingLevel, blockCount, clientId, ...props }, ref ) => { - const [ insertedBlock, setInsertedBlock ] = useState( null ); + const { insertedBlock, setInsertedBlock } = useListViewContext(); const instanceId = useInstanceId( Appender ); const { hideInserter } = useSelect( diff --git a/packages/block-editor/src/components/list-view/block-contents.js b/packages/block-editor/src/components/list-view/block-contents.js index a1f5f3562cfd40..37bc35c6a528d6 100644 --- a/packages/block-editor/src/components/list-view/block-contents.js +++ b/packages/block-editor/src/components/list-view/block-contents.js @@ -47,7 +47,8 @@ const ListViewBlockContents = forwardRef( [ clientId ] ); - const { renderAdditionalBlockUI } = useListViewContext(); + const { renderAdditionalBlockUI, insertedBlock, setInsertedBlock } = + useListViewContext(); const isBlockMoveTarget = blockMovingClientId && selectedBlockInBlockEditor === clientId; @@ -66,7 +67,12 @@ const ListViewBlockContents = forwardRef( return ( <> - { renderAdditionalBlockUI && renderAdditionalBlockUI( block ) } + { renderAdditionalBlockUI && + renderAdditionalBlockUI( + block, + insertedBlock, + setInsertedBlock + ) } { ( { draggable, onDragStart, onDragEnd } ) => ( 0; @@ -339,6 +340,7 @@ function ListViewBlock( { __experimentalSelectBlock={ updateSelection } expand={ expand } expandedState={ expandedState } + setInsertedBlock={ setInsertedBlock } /> ) } diff --git a/packages/block-editor/src/components/list-view/index.js b/packages/block-editor/src/components/list-view/index.js index 60c08abaf90895..298e3336ffebcd 100644 --- a/packages/block-editor/src/components/list-view/index.js +++ b/packages/block-editor/src/components/list-view/index.js @@ -16,6 +16,7 @@ import { useRef, useReducer, forwardRef, + useState, } from '@wordpress/element'; import { __ } from '@wordpress/i18n'; @@ -128,6 +129,9 @@ function ListViewComponent( const treeGridRef = useMergeRefs( [ elementRef, dropZoneRef, ref ] ); const isMounted = useRef( false ); + + const [ insertedBlock, setInsertedBlock ] = useState( null ); + const { setSelectedTreeId } = useListViewExpandSelectedItem( { firstSelectedBlockClientId: selectedClientIds[ 0 ], setExpandedState, @@ -212,6 +216,8 @@ function ListViewComponent( BlockSettingsMenu, listViewInstanceId: instanceId, renderAdditionalBlockUI, + insertedBlock, + setInsertedBlock, } ), [ draggedClientIds, @@ -221,6 +227,8 @@ function ListViewComponent( BlockSettingsMenu, instanceId, renderAdditionalBlockUI, + insertedBlock, + setInsertedBlock, ] ); diff --git a/packages/block-editor/src/store/private-selectors.js b/packages/block-editor/src/store/private-selectors.js index 60712e6b8eb6e0..dede6eccccbe50 100644 --- a/packages/block-editor/src/store/private-selectors.js +++ b/packages/block-editor/src/store/private-selectors.js @@ -8,13 +8,3 @@ export function isBlockInterfaceHidden( state ) { return state.isBlockInterfaceHidden; } - -/** - * Gets the client ids of the last inserted blocks. - * - * @param {Object} state Global application state. - * @return {Array|undefined} Client Ids of the last inserted block(s). - */ -export function getLastInsertedBlocksClientIds( state ) { - return state?.lastBlockInserted?.clientIds; -} diff --git a/packages/block-editor/src/store/test/private-selectors.js b/packages/block-editor/src/store/test/private-selectors.js index c5df265f75db35..2c287ceda0f88f 100644 --- a/packages/block-editor/src/store/test/private-selectors.js +++ b/packages/block-editor/src/store/test/private-selectors.js @@ -1,10 +1,7 @@ /** * Internal dependencies */ -import { - isBlockInterfaceHidden, - getLastInsertedBlocksClientIds, -} from '../private-selectors'; +import { isBlockInterfaceHidden } from '../private-selectors'; describe( 'private selectors', () => { describe( 'isBlockInterfaceHidden', () => { @@ -24,29 +21,4 @@ describe( 'private selectors', () => { expect( isBlockInterfaceHidden( state ) ).toBe( false ); } ); } ); - - describe( 'getLastInsertedBlocksClientIds', () => { - it( 'should return undefined if no blocks have been inserted', () => { - const state = { - lastBlockInserted: {}, - }; - - expect( getLastInsertedBlocksClientIds( state ) ).toEqual( - undefined - ); - } ); - - it( 'should return clientIds if blocks have been inserted', () => { - const state = { - lastBlockInserted: { - clientIds: [ '123456', '78910' ], - }, - }; - - expect( getLastInsertedBlocksClientIds( state ) ).toEqual( [ - '123456', - '78910', - ] ); - } ); - } ); } ); diff --git a/packages/block-library/src/navigation-link/use-inserted-block.js b/packages/block-library/src/navigation-link/use-inserted-block.js deleted file mode 100644 index 2644ca2e04f514..00000000000000 --- a/packages/block-library/src/navigation-link/use-inserted-block.js +++ /dev/null @@ -1,43 +0,0 @@ -/** - * WordPress dependencies - */ -import { useSelect, useDispatch } from '@wordpress/data'; -import { store as blockEditorStore } from '@wordpress/block-editor'; - -export const useInsertedBlock = ( insertedBlockClientId ) => { - const { insertedBlockAttributes, insertedBlockName } = useSelect( - ( select ) => { - const { getBlockName, getBlockAttributes } = - select( blockEditorStore ); - - return { - insertedBlockAttributes: getBlockAttributes( - insertedBlockClientId - ), - insertedBlockName: getBlockName( insertedBlockClientId ), - }; - }, - [ insertedBlockClientId ] - ); - - const { updateBlockAttributes } = useDispatch( blockEditorStore ); - - const setInsertedBlockAttributes = ( _updatedAttributes ) => { - if ( ! insertedBlockClientId ) return; - updateBlockAttributes( insertedBlockClientId, _updatedAttributes ); - }; - - if ( ! insertedBlockClientId ) { - return { - insertedBlockAttributes: undefined, - insertedBlockName: undefined, - setInsertedBlockAttributes, - }; - } - - return { - insertedBlockAttributes, - insertedBlockName, - setInsertedBlockAttributes, - }; -}; diff --git a/packages/block-library/src/navigation/edit/leaf-more-menu.js b/packages/block-library/src/navigation/edit/leaf-more-menu.js index f57335ce2bef60..4fe45ac5def83f 100644 --- a/packages/block-library/src/navigation/edit/leaf-more-menu.js +++ b/packages/block-library/src/navigation/edit/leaf-more-menu.js @@ -24,7 +24,13 @@ const BLOCKS_THAT_CAN_BE_CONVERTED_TO_SUBMENU = [ 'core/navigation-submenu', ]; -function AddSubmenuItem( { block, onClose, expandedState, expand } ) { +function AddSubmenuItem( { + block, + onClose, + expandedState, + expand, + setInsertedBlock, +} ) { const { insertBlock, replaceBlock, replaceInnerBlocks } = useDispatch( blockEditorStore ); @@ -69,6 +75,12 @@ function AddSubmenuItem( { block, onClose, expandedState, expand } ) { updateSelectionOnInsert ); } + + // This call sets the local List View state for the "last inserted block". + // This is required for the Nav Block to determine whether or not to display + // the Link UI for this new block. + setInsertedBlock( newLink ); + if ( ! expandedState[ block.clientId ] ) { expand( block.clientId ); } @@ -138,6 +150,7 @@ export default function LeafMoreMenu( props ) { expanded expandedState={ props.expandedState } expand={ props.expand } + setInsertedBlock={ props.setInsertedBlock } /> diff --git a/packages/block-library/src/navigation/edit/menu-inspector-controls.js b/packages/block-library/src/navigation/edit/menu-inspector-controls.js index d4c9506c51d8c0..23f005beb43502 100644 --- a/packages/block-library/src/navigation/edit/menu-inspector-controls.js +++ b/packages/block-library/src/navigation/edit/menu-inspector-controls.js @@ -12,8 +12,7 @@ import { __experimentalHeading as Heading, Spinner, } from '@wordpress/components'; -import { useSelect } from '@wordpress/data'; -import { useState, useEffect } from '@wordpress/element'; +import { useSelect, useDispatch } from '@wordpress/data'; import { __, sprintf } from '@wordpress/i18n'; /** @@ -26,7 +25,6 @@ import useNavigationMenu from '../use-navigation-menu'; import LeafMoreMenu from './leaf-more-menu'; import { updateAttributes } from '../../navigation-link/update-attributes'; import { LinkUI } from '../../navigation-link/link-ui'; -import { useInsertedBlock } from '../../navigation-link/use-inserted-block'; /* translators: %s: The name of a menu. */ const actionLabel = __( "Switch to '%s'" ); @@ -54,40 +52,13 @@ const MainContent = ( { [ clientId ] ); - const [ clientIdWithOpenLinkUI, setClientIdWithOpenLinkUI ] = useState(); - const { lastInsertedBlockClientId } = useSelect( ( select ) => { - const { getLastInsertedBlocksClientIds } = unlock( - select( blockEditorStore ) - ); - const lastInsertedBlocksClientIds = getLastInsertedBlocksClientIds(); - return { - lastInsertedBlockClientId: - lastInsertedBlocksClientIds && lastInsertedBlocksClientIds[ 0 ], - }; - }, [] ); + const { updateBlockAttributes } = useDispatch( blockEditorStore ); - const { - insertedBlockAttributes, - insertedBlockName, - setInsertedBlockAttributes, - } = useInsertedBlock( lastInsertedBlockClientId ); - - const hasExistingLinkValue = insertedBlockAttributes?.url; - - useEffect( () => { - if ( - lastInsertedBlockClientId && - BLOCKS_WITH_LINK_UI_SUPPORT?.includes( insertedBlockName ) && - ! hasExistingLinkValue // don't re-show the Link UI if the block already has a link value. - ) { - setClientIdWithOpenLinkUI( lastInsertedBlockClientId ); - } - }, [ - lastInsertedBlockClientId, - clientId, - insertedBlockName, - hasExistingLinkValue, - ] ); + const setInsertedBlockAttributes = + ( _insertedBlockClientId ) => ( _updatedAttributes ) => { + if ( ! _insertedBlockClientId ) return; + updateBlockAttributes( _insertedBlockClientId, _updatedAttributes ); + }; const { navigationMenu } = useNavigationMenu( currentMenuId ); @@ -109,23 +80,42 @@ const MainContent = ( { 'You have not yet created any menus. Displaying a list of your Pages' ); - const renderLinkUI = ( block ) => { + const renderLinkUI = ( + currentBlock, + lastInsertedBlock, + setLastInsertedBlock + ) => { + const blockSupportsLinkUI = BLOCKS_WITH_LINK_UI_SUPPORT?.includes( + lastInsertedBlock?.name + ); + const currentBlockWasJustInserted = + lastInsertedBlock?.clientId === currentBlock.clientId; + + const shouldShowLinkUIForBlock = + blockSupportsLinkUI && currentBlockWasJustInserted; + return ( - clientIdWithOpenLinkUI === block.clientId && ( + shouldShowLinkUIForBlock && ( setClientIdWithOpenLinkUI( null ) } + clientId={ lastInsertedBlock?.clientId } + link={ lastInsertedBlock?.attributes } + onClose={ () => { + setLastInsertedBlock( null ); + } } hasCreateSuggestion={ false } onChange={ ( updatedValue ) => { updateAttributes( updatedValue, - setInsertedBlockAttributes, - insertedBlockAttributes + setInsertedBlockAttributes( + lastInsertedBlock?.clientId + ), + lastInsertedBlock?.attributes ); - setClientIdWithOpenLinkUI( null ); + setLastInsertedBlock( null ); + } } + onCancel={ () => { + setLastInsertedBlock( null ); } } - onCancel={ () => setClientIdWithOpenLinkUI( null ) } /> ) ); diff --git a/test/e2e/specs/editor/blocks/navigation.spec.js b/test/e2e/specs/editor/blocks/navigation.spec.js index 103e4bc9d57150..835b05e570e992 100644 --- a/test/e2e/specs/editor/blocks/navigation.spec.js +++ b/test/e2e/specs/editor/blocks/navigation.spec.js @@ -522,9 +522,25 @@ test.describe( 'Navigation block', () => { linkControl, } ) => { await admin.createNewPost(); - await requestUtils.createNavigationMenu( navMenuBlocksFixture ); + const { id: menuId } = await requestUtils.createNavigationMenu( + navMenuBlocksFixture + ); - await editor.insertBlock( { name: 'core/navigation' } ); + // Insert x2 blocks as a stress test as several bugs have been found with inserting + // blocks into the navigation block when there are multiple blocks referencing the + // **same** menu. + await editor.insertBlock( { + name: 'core/navigation', + attributes: { + ref: menuId, + }, + } ); + await editor.insertBlock( { + name: 'core/navigation', + attributes: { + ref: menuId, + }, + } ); await editor.openDocumentSettingsSidebar(); @@ -572,11 +588,17 @@ test.describe( 'Navigation block', () => { // Expect to see the Link creation UI be focused. const linkUIInput = linkControl.getSearchInput(); + // Coverage for bug whereby Link UI input would be incorrectly prepopulated. + // It should: + // - be focused - should not be in "preview" mode but rather ready to accept input. + // - be empty - not pre-populated + // See: https://github.com/WordPress/gutenberg/issues/50733 await expect( linkUIInput ).toBeFocused(); + await expect( linkUIInput ).toBeEmpty(); const firstResult = await linkControl.getNthSearchResult( 0 ); - // Grab the text from the first result so we can check it was inserted. + // Grab the text from the first result so we can check (later on) that it was inserted. const firstResultText = await linkControl.getSearchResultText( firstResult ); @@ -821,6 +843,92 @@ test.describe( 'Navigation block', () => { .getByText( 'Top Level Item 1' ) ).toBeVisible(); } ); + + test( `does not display link interface for blocks that have not just been inserted`, async ( { + admin, + page, + editor, + requestUtils, + linkControl, + } ) => { + // Provides coverage for a bug whereby the Link UI would be unexpectedly displayed for the last + // inserted block even if the block had been deselected and then reselected. + // See: https://github.com/WordPress/gutenberg/issues/50601 + + await admin.createNewPost(); + const { id: menuId } = await requestUtils.createNavigationMenu( + navMenuBlocksFixture + ); + + // Insert x2 blocks as a stress test as several bugs have been found with inserting + // blocks into the navigation block when there are multiple blocks referencing the + // **same** menu. + await editor.insertBlock( { + name: 'core/navigation', + attributes: { + ref: menuId, + }, + } ); + await editor.insertBlock( { + name: 'core/navigation', + attributes: { + ref: menuId, + }, + } ); + + await editor.openDocumentSettingsSidebar(); + + const listView = page.getByRole( 'treegrid', { + name: 'Block navigation structure', + description: 'Structure for navigation menu: Test Menu', + } ); + + await listView + .getByRole( 'button', { + name: 'Add block', + } ) + .click(); + + const blockResults = page.getByRole( 'listbox', { + name: 'Blocks', + } ); + + await expect( blockResults ).toBeVisible(); + + const blockResultOptions = blockResults.getByRole( 'option' ); + + // Select the Page Link option. + await blockResultOptions.nth( 0 ).click(); + + // Immediately dismiss the Link UI thereby not populating the `url` attribute + // of the block. + await linkControl.pressCancel(); + + // Get the Inspector Tabs. + const blockSettings = page.getByRole( 'region', { + name: 'Editor settings', + } ); + + // Trigger "unmount" of the List View. + await blockSettings + .getByRole( 'tab', { + name: 'Settings', + } ) + .click(); + + // "Remount" the List View. + // this is where the bug previously occurred. + await blockSettings + .getByRole( 'tab', { + name: 'List View', + } ) + .click(); + + // Check that despite being the last inserted block, the Link UI is not displayed + // in this scenario because it was not **just** inserted into the List View (i.e. + // we have unmounted the list view and then remounted it). + await expect( linkControl.getSearchInput() ).not.toBeVisible(); + } ); } ); } ); @@ -1159,6 +1267,14 @@ class LinkControl { } ); } + async pressCancel() { + const cancelButton = this.page.getByRole( 'button', { + name: 'Cancel', + } ); + + return cancelButton.click(); + } + async getSearchResults() { const searchInput = this.getSearchInput();