Skip to content

Commit

Permalink
Insert Links by default in Navigation block (#34899)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
tellthemachines committed Sep 30, 2021
1 parent 1e7f837 commit fe76803
Show file tree
Hide file tree
Showing 9 changed files with 149 additions and 35 deletions.
4 changes: 4 additions & 0 deletions packages/block-editor/src/components/inner-blocks/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ function UncontrolledInnerBlocks( props ) {
const {
clientId,
allowedBlocks,
__experimentalDefaultBlock,
__experimentalDirectInsert,
template,
templateLock,
wrapperRef,
Expand All @@ -57,6 +59,8 @@ function UncontrolledInnerBlocks( props ) {
useNestedSettingsUpdate(
clientId,
allowedBlocks,
__experimentalDefaultBlock,
__experimentalDirectInsert,
templateLock,
captureToolbars,
orientation,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -83,13 +88,23 @@ 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 );
}
}, [
clientId,
blockListSettings,
_allowedBlocks,
__experimentalDefaultBlock,
__experimentalDirectInsert,
templateLock,
parentLock,
captureToolbars,
Expand Down
18 changes: 15 additions & 3 deletions packages/block-editor/src/components/inserter/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ class Inserter extends Component {
disabled,
blockTitle,
hasSingleBlockType,
directInsertBlock,
toggleProps,
hasItems,
renderToggle = defaultRenderToggle,
Expand All @@ -113,6 +114,7 @@ class Inserter extends Component {
disabled: disabled || ! hasItems,
blockTitle,
hasSingleBlockType,
directInsertBlock,
toggleProps,
} );
}
Expand Down Expand Up @@ -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 } );
}

Expand Down Expand Up @@ -202,6 +205,7 @@ export default compose( [
getBlockRootClientId,
hasInserterItems,
__experimentalGetAllowedBlocks,
__experimentalGetDirectInsertBlock,
} = select( blockEditorStore );
const { getBlockVariations } = select( blocksStore );

Expand All @@ -210,6 +214,10 @@ export default compose( [

const allowedBlocks = __experimentalGetAllowedBlocks( rootClientId );

const directInsertBlock = __experimentalGetDirectInsertBlock(
rootClientId
);

const hasSingleBlockType =
size( allowedBlocks ) === 1 &&
size(
Expand All @@ -226,6 +234,7 @@ export default compose( [
hasSingleBlockType,
blockTitle: allowedBlockType ? allowedBlockType.title : '',
allowedBlockType,
directInsertBlock,
rootClientId,
};
} ),
Expand All @@ -238,10 +247,11 @@ export default compose( [
isAppender,
hasSingleBlockType,
allowedBlockType,
directInsertBlock,
onSelectOrClose,
} = ownProps;

if ( ! hasSingleBlockType ) {
if ( ! hasSingleBlockType && ! directInsertBlock?.length ) {
return;
}

Expand Down Expand Up @@ -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 );

Expand Down
33 changes: 33 additions & 0 deletions packages/block-editor/src/store/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 4 additions & 0 deletions packages/block-library/src/navigation-submenu/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -504,6 +506,8 @@ export default function NavigationSubmenuEdit( {
},
{
allowedBlocks: ALLOWED_BLOCKS,
__experimentalDefaultBlock: DEFAULT_BLOCK,
__experimentalDirectInsert: true,
renderAppender:
isSelected ||
( isImmediateParentOfSelectedBlock &&
Expand Down
12 changes: 12 additions & 0 deletions packages/block-library/src/navigation/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: [],
Expand Down Expand Up @@ -164,6 +174,8 @@ function Navigation( {
},
{
allowedBlocks: ALLOWED_BLOCKS,
__experimentalDefaultBlock: DEFAULT_BLOCK,
__experimentalDirectInsert: DIRECT_INSERT,
orientation: attributes.orientation,
renderAppender: CustomAppender || appender,

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,16 @@ exports[`Navigation Creating from existing Pages allows a navigation block to be
<!-- /wp:navigation -->"
`;

exports[`Navigation Shows the quick inserter when the block contains non-navigation specific blocks 1`] = `
"<!-- wp:navigation -->
<!-- wp:navigation-link {\\"label\\":\\"WP\\",\\"url\\":\\"https://wordpress.org\\",\\"kind\\":\\"custom\\",\\"isTopLevelLink\\":true} /-->
<!-- wp:site-title /-->
<!-- wp:navigation-link {\\"label\\":\\"WP News\\",\\"url\\":\\"https://wordpress.org/news/\\",\\"kind\\":\\"custom\\",\\"isTopLevelLink\\":true} /-->
<!-- /wp:navigation -->"
`;

exports[`Navigation allows an empty navigation block to be created and manually populated using a mixture of internal and external links 1`] = `
"<!-- wp:navigation -->
<!-- wp:navigation-link {\\"label\\":\\"WP\\",\\"url\\":\\"https://wordpress.org\\",\\"kind\\":\\"custom\\",\\"isTopLevelLink\\":true} /-->
Expand Down
61 changes: 45 additions & 16 deletions packages/e2e-tests/specs/experiments/blocks/navigation.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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"]'
Expand Down Expand Up @@ -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( {
Expand All @@ -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.
Expand Down Expand Up @@ -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( {
Expand All @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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 () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit fe76803

Please sign in to comment.