Skip to content

Commit

Permalink
Fix inconsistent Link UI in Nav block list view editor (#50774)
Browse files Browse the repository at this point in the history
Co-authored-by: Ben Dwyer <ben@scruffian.com>
Co-authored-by: Jerry Jones <jones.jeremydavid@gmail.com>
  • Loading branch information
3 people committed May 19, 2023
1 parent 5584713 commit dc3832f
Show file tree
Hide file tree
Showing 10 changed files with 190 additions and 135 deletions.
5 changes: 3 additions & 2 deletions packages/block-editor/src/components/list-view/appender.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,20 @@
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';

/**
* Internal dependencies
*/
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(
Expand Down
10 changes: 8 additions & 2 deletions packages/block-editor/src/components/list-view/block-contents.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ const ListViewBlockContents = forwardRef(
[ clientId ]
);

const { renderAdditionalBlockUI } = useListViewContext();
const { renderAdditionalBlockUI, insertedBlock, setInsertedBlock } =
useListViewContext();

const isBlockMoveTarget =
blockMovingClientId && selectedBlockInBlockEditor === clientId;
Expand All @@ -66,7 +67,12 @@ const ListViewBlockContents = forwardRef(

return (
<>
{ renderAdditionalBlockUI && renderAdditionalBlockUI( block ) }
{ renderAdditionalBlockUI &&
renderAdditionalBlockUI(
block,
insertedBlock,
setInsertedBlock
) }
<BlockDraggable clientIds={ draggableClientIds }>
{ ( { draggable, onDragStart, onDragEnd } ) => (
<ListViewBlockSelectButton
Expand Down
2 changes: 2 additions & 0 deletions packages/block-editor/src/components/list-view/block.js
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ function ListViewBlock( {
BlockSettingsMenu,
listViewInstanceId,
expandedState,
setInsertedBlock,
} = useListViewContext();

const hasSiblings = siblingBlockCount > 0;
Expand Down Expand Up @@ -339,6 +340,7 @@ function ListViewBlock( {
__experimentalSelectBlock={ updateSelection }
expand={ expand }
expandedState={ expandedState }
setInsertedBlock={ setInsertedBlock }
/>
) }
</TreeGridCell>
Expand Down
8 changes: 8 additions & 0 deletions packages/block-editor/src/components/list-view/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
useRef,
useReducer,
forwardRef,
useState,
} from '@wordpress/element';
import { __ } from '@wordpress/i18n';

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -212,6 +216,8 @@ function ListViewComponent(
BlockSettingsMenu,
listViewInstanceId: instanceId,
renderAdditionalBlockUI,
insertedBlock,
setInsertedBlock,
} ),
[
draggedClientIds,
Expand All @@ -221,6 +227,8 @@ function ListViewComponent(
BlockSettingsMenu,
instanceId,
renderAdditionalBlockUI,
insertedBlock,
setInsertedBlock,
]
);

Expand Down
10 changes: 0 additions & 10 deletions packages/block-editor/src/store/private-selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
30 changes: 1 addition & 29 deletions packages/block-editor/src/store/test/private-selectors.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
/**
* Internal dependencies
*/
import {
isBlockInterfaceHidden,
getLastInsertedBlocksClientIds,
} from '../private-selectors';
import { isBlockInterfaceHidden } from '../private-selectors';

describe( 'private selectors', () => {
describe( 'isBlockInterfaceHidden', () => {
Expand All @@ -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',
] );
} );
} );
} );
43 changes: 0 additions & 43 deletions packages/block-library/src/navigation-link/use-inserted-block.js

This file was deleted.

15 changes: 14 additions & 1 deletion packages/block-library/src/navigation/edit/leaf-more-menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 );

Expand Down Expand Up @@ -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 );
}
Expand Down Expand Up @@ -138,6 +150,7 @@ export default function LeafMoreMenu( props ) {
expanded
expandedState={ props.expandedState }
expand={ props.expand }
setInsertedBlock={ props.setInsertedBlock }
/>
</MenuGroup>
<MenuGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

/**
Expand All @@ -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'" );
Expand Down Expand Up @@ -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 );

Expand All @@ -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 && (
<LinkUI
clientId={ lastInsertedBlockClientId }
link={ insertedBlockAttributes }
onClose={ () => 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 ) }
/>
)
);
Expand Down
Loading

1 comment on commit dc3832f

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flaky tests detected in dc3832f.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5028521914
📝 Reported issues:

Please sign in to comment.