From adefb8905a17bc3d2b991fc15a3e1a387d10e0c2 Mon Sep 17 00:00:00 2001 From: Ben Dwyer Date: Mon, 15 May 2023 15:56:30 +0100 Subject: [PATCH] Add new API to allow inserter items to be prioritised (#50510) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Proposing a way to sort items in the block inspector based on allowed blocks * Add inserterPriority API to inner blocks * Sort inserter based on inserterPriority prop from block list settings * Use new inserterPriority API in Nav block * Correct comment Co-authored-by: Andrei Draganescu * Remove redundant prop * Avoid stale inserterPriority * Make sorting function stable * Renaming * Remove redundant comment * remove spacer * Set prioritisedBlocks as empty array when no blockListSettings are found. There are instances in the main inserter search results where the rootClientId is undefined, such as when searching for a block. This means there are no blockListSettings, which ends up in no `proritisedBlocks` property being returned and it crashses the app. * proritise -> prioritize for consistency * Update packages/block-editor/src/components/inner-blocks/use-nested-settings-update.js Co-authored-by: Alex Lende * Renaming constant to match updated name * Add prioritizedKInnerBlocks to the inner-blocks README * lint fix * update comment * update comment * pass the correct props to useNestedSettingsUpdate * Use stable ref Co-authored-by: George Mamadashvili * Register the test Plugin for e2e tests * Register block with prioritzedInserterBlocks set * Add initial test scaffold * Tidy up scaffolded test * Add test for new API It fails :( * Try removing sort from helper Why is this even here? It shouldn’t transfer results like this. * Fix test * Add test to check does not override allowedBlocks when conflicted * Add additional assertion for retaining of correct number of results * Ensure tests reflect target of Quick Inserter * sort allowed blocks on the tests that consume getAllBlockInserterItemTitles * Improve e2e test comment * Update packages/block-editor/src/components/inner-blocks/use-nested-settings-update.js * Update packages/block-editor/src/components/inner-blocks/README.md --------- Co-authored-by: Dave Smith Co-authored-by: Andrei Draganescu Co-authored-by: Jerry Jones Co-authored-by: Alex Lende Co-authored-by: George Mamadashvili Co-authored-by: MaggieCabrera --- .../src/components/inner-blocks/README.md | 5 + .../src/components/inner-blocks/index.js | 2 + .../components/inner-blocks/index.native.js | 16 ++- .../use-nested-settings-update.js | 23 +++- .../src/components/inserter/index.js | 10 +- .../src/components/inserter/quick-inserter.js | 2 - .../src/components/inserter/search-results.js | 55 ++++++++- .../components/off-canvas-editor/appender.js | 26 +---- .../block-library/src/navigation/constants.js | 5 + .../src/navigation/edit/inner-blocks.js | 7 +- .../src/get-all-block-inserter-item-titles.js | 2 +- ...ner-blocks-prioritized-inserter-blocks.php | 28 +++++ .../index.js | 82 +++++++++++++ .../specs/editor/plugins/child-blocks.test.js | 3 +- .../inner-blocks-allowed-blocks.test.js | 5 +- ...blocks-prioritized-inserter-blocks.test.js | 108 ++++++++++++++++++ 16 files changed, 328 insertions(+), 51 deletions(-) create mode 100644 packages/e2e-tests/plugins/inner-blocks-prioritized-inserter-blocks.php create mode 100644 packages/e2e-tests/plugins/inner-blocks-prioritized-inserter-blocks/index.js create mode 100644 packages/e2e-tests/specs/editor/plugins/inner-blocks-prioritized-inserter-blocks.test.js diff --git a/packages/block-editor/src/components/inner-blocks/README.md b/packages/block-editor/src/components/inner-blocks/README.md index eb42da998f0d0c..5ecd9c90898210 100644 --- a/packages/block-editor/src/components/inner-blocks/README.md +++ b/packages/block-editor/src/components/inner-blocks/README.md @@ -180,3 +180,8 @@ For example, a button block, deeply nested in several levels of block `X` that u - **Type:** `Function` - **Default:** - `undefined`. The placeholder is an optional function that can be passed in to be a rendered component placed in front of the appender. This can be used to represent an example state prior to any blocks being placed. See the Social Links for an implementation example. + +### `prioritizedInserterBlocks` + +- **Type:** `Array` +- **Default:** - `undefined`. Determines which block types should be shown in the block inserter. For example, when inserting a block within the Navigation block we specify `core/navigation-link` and `core/navigation-link/page` as these are the most commonly used inner blocks. `prioritizedInserterBlocks` takes an array of the form {blockName}/{variationName}, where {variationName} is optional. diff --git a/packages/block-editor/src/components/inner-blocks/index.js b/packages/block-editor/src/components/inner-blocks/index.js index d83f62cf2b45cd..bf33abad4864f3 100644 --- a/packages/block-editor/src/components/inner-blocks/index.js +++ b/packages/block-editor/src/components/inner-blocks/index.js @@ -45,6 +45,7 @@ function UncontrolledInnerBlocks( props ) { const { clientId, allowedBlocks, + prioritizedInserterBlocks, __experimentalDefaultBlock, __experimentalDirectInsert, template, @@ -62,6 +63,7 @@ function UncontrolledInnerBlocks( props ) { useNestedSettingsUpdate( clientId, allowedBlocks, + prioritizedInserterBlocks, __experimentalDefaultBlock, __experimentalDirectInsert, templateLock, diff --git a/packages/block-editor/src/components/inner-blocks/index.native.js b/packages/block-editor/src/components/inner-blocks/index.native.js index e635230b5c2425..54e168f8ee43f4 100644 --- a/packages/block-editor/src/components/inner-blocks/index.native.js +++ b/packages/block-editor/src/components/inner-blocks/index.native.js @@ -72,9 +72,13 @@ function UncontrolledInnerBlocks( props ) { const { clientId, allowedBlocks, + prioritizedInserterBlocks, + __experimentalDefaultBlock, + __experimentalDirectInsert, template, templateLock, templateInsertUpdatesSelection, + __experimentalCaptureToolbars: captureToolbars, orientation, renderAppender, renderFooterAppender, @@ -95,7 +99,17 @@ function UncontrolledInnerBlocks( props ) { const context = useBlockContext( clientId ); - useNestedSettingsUpdate( clientId, allowedBlocks, templateLock ); + useNestedSettingsUpdate( + clientId, + allowedBlocks, + prioritizedInserterBlocks, + __experimentalDefaultBlock, + __experimentalDirectInsert, + templateLock, + captureToolbars, + orientation, + layout + ); useInnerBlockTemplateSync( clientId, 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 d9518eb303a044..49d2da85688c3f 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 @@ -25,6 +25,7 @@ const pendingSettingsUpdates = new WeakMap(); * @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[]} prioritizedInserterBlocks Block names and/or block variations to be prioritized in the inserter, in the format {blockName}/{variationName}. * @param {?WPDirectInsertBlock} __experimentalDefaultBlock The default block to insert: [ blockName, { blockAttributes } ]. * @param {?Function|boolean} __experimentalDirectInsert If a default block should be inserted directly by the * appender. @@ -40,6 +41,7 @@ const pendingSettingsUpdates = new WeakMap(); export default function useNestedSettingsUpdate( clientId, allowedBlocks, + prioritizedInserterBlocks, __experimentalDefaultBlock, __experimentalDirectInsert, templateLock, @@ -64,13 +66,27 @@ export default function useNestedSettingsUpdate( [ clientId ] ); - // Memoize as inner blocks implementors often pass a new array on every - // render. - const _allowedBlocks = useMemo( () => allowedBlocks, allowedBlocks ); + // Memoize allowedBlocks and prioritisedInnerBlocks based on the contents + // of the arrays. Implementors often pass a new array on every render, + // and the contents of the arrays are just strings, so the entire array + // can be passed as dependencies. + + const _allowedBlocks = useMemo( + () => allowedBlocks, + // eslint-disable-next-line react-hooks/exhaustive-deps + allowedBlocks + ); + + const _prioritizedInserterBlocks = useMemo( + () => prioritizedInserterBlocks, + // eslint-disable-next-line react-hooks/exhaustive-deps + prioritizedInserterBlocks + ); useLayoutEffect( () => { const newSettings = { allowedBlocks: _allowedBlocks, + prioritizedInserterBlocks: _prioritizedInserterBlocks, templateLock: templateLock === undefined || parentLock === 'contentOnly' ? parentLock @@ -130,6 +146,7 @@ export default function useNestedSettingsUpdate( clientId, blockListSettings, _allowedBlocks, + _prioritizedInserterBlocks, __experimentalDefaultBlock, __experimentalDirectInsert, templateLock, diff --git a/packages/block-editor/src/components/inserter/index.js b/packages/block-editor/src/components/inserter/index.js index 4acf5e3746eb88..9c24497e5a9078 100644 --- a/packages/block-editor/src/components/inserter/index.js +++ b/packages/block-editor/src/components/inserter/index.js @@ -150,7 +150,6 @@ class PrivateInserter extends Component { prioritizePatterns, onSelectOrClose, selectBlockOnInsert, - orderInitialBlockItems, } = this.props; if ( isQuick ) { @@ -174,7 +173,6 @@ class PrivateInserter extends Component { isAppender={ isAppender } prioritizePatterns={ prioritizePatterns } selectBlockOnInsert={ selectBlockOnInsert } - orderInitialBlockItems={ orderInitialBlockItems } /> ); } @@ -426,13 +424,7 @@ export const ComposedPrivateInserter = compose( [ ] )( PrivateInserter ); const Inserter = forwardRef( ( props, ref ) => { - return ( - - ); + return ; } ); export default Inserter; diff --git a/packages/block-editor/src/components/inserter/quick-inserter.js b/packages/block-editor/src/components/inserter/quick-inserter.js index 9fe96f091a86e8..540b51a4757e0d 100644 --- a/packages/block-editor/src/components/inserter/quick-inserter.js +++ b/packages/block-editor/src/components/inserter/quick-inserter.js @@ -32,7 +32,6 @@ export default function QuickInserter( { isAppender, prioritizePatterns, selectBlockOnInsert, - orderInitialBlockItems, } ) { const [ filterValue, setFilterValue ] = useState( '' ); const [ destinationRootClientId, onInsertBlocks ] = useInsertionPoint( { @@ -125,7 +124,6 @@ export default function QuickInserter( { isDraggable={ false } prioritizePatterns={ prioritizePatterns } selectBlockOnInsert={ selectBlockOnInsert } - orderInitialBlockItems={ orderInitialBlockItems } /> diff --git a/packages/block-editor/src/components/inserter/search-results.js b/packages/block-editor/src/components/inserter/search-results.js index 6dc85af2653311..b2dc15d586adf9 100644 --- a/packages/block-editor/src/components/inserter/search-results.js +++ b/packages/block-editor/src/components/inserter/search-results.js @@ -6,6 +6,7 @@ import { __, _n, sprintf } from '@wordpress/i18n'; import { VisuallyHidden } from '@wordpress/components'; import { useDebounce, useAsyncList } from '@wordpress/compose'; import { speak } from '@wordpress/a11y'; +import { useSelect } from '@wordpress/data'; /** * Internal dependencies @@ -21,6 +22,7 @@ import useBlockTypesState from './hooks/use-block-types-state'; import { searchBlockItems, searchItems } from './search-items'; import InserterListbox from '../inserter-listbox'; import { orderBy } from '../../utils/sorting'; +import { store as blockEditorStore } from '../../store'; const INITIAL_INSERTER_RESULTS = 9; /** @@ -31,6 +33,24 @@ const INITIAL_INSERTER_RESULTS = 9; */ const EMPTY_ARRAY = []; +const orderInitialBlockItems = ( items, priority ) => { + if ( ! priority ) { + return items; + } + + items.sort( ( { id: aName }, { id: bName } ) => { + // Sort block items according to `priority`. + let aIndex = priority.indexOf( aName ); + let bIndex = priority.indexOf( bName ); + // All other block items should come after that. + if ( aIndex < 0 ) aIndex = priority.length; + if ( bIndex < 0 ) bIndex = priority.length; + return aIndex - bIndex; + } ); + + return items; +}; + function InserterSearchResults( { filterValue, onSelect, @@ -46,10 +66,22 @@ function InserterSearchResults( { shouldFocusBlock = true, prioritizePatterns, selectBlockOnInsert, - orderInitialBlockItems, } ) { const debouncedSpeak = useDebounce( speak, 500 ); + const { prioritizedBlocks } = useSelect( + ( select ) => { + const blockListSettings = + select( blockEditorStore ).getBlockListSettings( rootClientId ); + + return { + prioritizedBlocks: + blockListSettings?.prioritizedInserterBlocks || EMPTY_ARRAY, + }; + }, + [ rootClientId ] + ); + const [ destinationRootClientId, onInsertBlocks ] = useInsertionPoint( { onSelect, rootClientId, @@ -89,10 +121,16 @@ function InserterSearchResults( { if ( maxBlockTypesToShow === 0 ) { return []; } + let orderedItems = orderBy( blockTypes, 'frecency', 'desc' ); - if ( ! filterValue && orderInitialBlockItems ) { - orderedItems = orderInitialBlockItems( orderedItems ); + + if ( ! filterValue && prioritizedBlocks.length ) { + orderedItems = orderInitialBlockItems( + orderedItems, + prioritizedBlocks + ); } + const results = searchBlockItems( orderedItems, blockTypeCategories, @@ -108,8 +146,8 @@ function InserterSearchResults( { blockTypes, blockTypeCategories, blockTypeCollections, - maxBlockTypes, - orderInitialBlockItems, + maxBlockTypesToShow, + prioritizedBlocks, ] ); // Announce search results on change. @@ -124,7 +162,12 @@ function InserterSearchResults( { count ); debouncedSpeak( resultsFoundMessage ); - }, [ filterValue, debouncedSpeak ] ); + }, [ + filterValue, + debouncedSpeak, + filteredBlockTypes, + filteredBlockPatterns, + ] ); const currentShownBlockTypes = useAsyncList( filteredBlockTypes, { step: INITIAL_INSERTER_RESULTS, diff --git a/packages/block-editor/src/components/off-canvas-editor/appender.js b/packages/block-editor/src/components/off-canvas-editor/appender.js index 5f981d5a90ca51..1b91f5bdd76845 100644 --- a/packages/block-editor/src/components/off-canvas-editor/appender.js +++ b/packages/block-editor/src/components/off-canvas-editor/appender.js @@ -4,12 +4,7 @@ import { useInstanceId } from '@wordpress/compose'; import { speak } from '@wordpress/a11y'; import { useSelect } from '@wordpress/data'; -import { - forwardRef, - useState, - useEffect, - useCallback, -} from '@wordpress/element'; +import { forwardRef, useState, useEffect } from '@wordpress/element'; import { __, sprintf } from '@wordpress/i18n'; /** @@ -19,11 +14,6 @@ import { store as blockEditorStore } from '../../store'; import useBlockDisplayTitle from '../block-title/use-block-display-title'; import { ComposedPrivateInserter as PrivateInserter } from '../inserter'; -const prioritizedInserterBlocks = [ - 'core/navigation-link/page', - 'core/navigation-link', -]; - export const Appender = forwardRef( ( { nestingLevel, blockCount, clientId, ...props }, ref ) => { const [ insertedBlock, setInsertedBlock ] = useState( null ); @@ -68,19 +58,6 @@ export const Appender = forwardRef( ); }, [ insertedBlockTitle ] ); - const orderInitialBlockItems = useCallback( ( items ) => { - items.sort( ( { id: aName }, { id: bName } ) => { - // Sort block items according to `prioritizedInserterBlocks`. - let aIndex = prioritizedInserterBlocks.indexOf( aName ); - let bIndex = prioritizedInserterBlocks.indexOf( bName ); - // All other block items should come after that. - if ( aIndex < 0 ) aIndex = prioritizedInserterBlocks.length; - if ( bIndex < 0 ) bIndex = prioritizedInserterBlocks.length; - return aIndex - bIndex; - } ); - return items; - }, [] ); - if ( hideInserter ) { return null; } @@ -110,7 +87,6 @@ export const Appender = forwardRef( setInsertedBlock( maybeInsertedBlock ); } } } - orderInitialBlockItems={ orderInitialBlockItems } />
{ '[data-type="test/child-blocks-restricted-parent"] .block-editor-default-block-appender' ); await openGlobalBlockInserter(); - expect( await getAllBlockInserterItemTitles() ).toEqual( [ + const allowedBlocks = await getAllBlockInserterItemTitles(); + expect( allowedBlocks.sort() ).toEqual( [ 'Child Blocks Child', 'Image', 'Paragraph', diff --git a/packages/e2e-tests/specs/editor/plugins/inner-blocks-allowed-blocks.test.js b/packages/e2e-tests/specs/editor/plugins/inner-blocks-allowed-blocks.test.js index 3eb1ee4775f74b..4431d3bd5802f0 100644 --- a/packages/e2e-tests/specs/editor/plugins/inner-blocks-allowed-blocks.test.js +++ b/packages/e2e-tests/specs/editor/plugins/inner-blocks-allowed-blocks.test.js @@ -50,7 +50,8 @@ describe( 'Allowed Blocks Setting on InnerBlocks', () => { await page.waitForSelector( childParagraphSelector ); await page.click( childParagraphSelector ); await openGlobalBlockInserter(); - expect( await getAllBlockInserterItemTitles() ).toEqual( [ + const allowedBlocks = await getAllBlockInserterItemTitles(); + expect( allowedBlocks.sort() ).toEqual( [ 'Button', 'Gallery', 'List', @@ -75,7 +76,7 @@ describe( 'Allowed Blocks Setting on InnerBlocks', () => { await page.$x( `//button//span[contains(text(), 'List')]` ) )[ 0 ]; await insertButton.click(); - // Select the list wrapper so the image is inserable. + // Select the list wrapper so the image is insertable. await page.keyboard.press( 'ArrowUp' ); await insertBlock( 'Image' ); await closeGlobalBlockInserter(); diff --git a/packages/e2e-tests/specs/editor/plugins/inner-blocks-prioritized-inserter-blocks.test.js b/packages/e2e-tests/specs/editor/plugins/inner-blocks-prioritized-inserter-blocks.test.js new file mode 100644 index 00000000000000..c34e402ea19806 --- /dev/null +++ b/packages/e2e-tests/specs/editor/plugins/inner-blocks-prioritized-inserter-blocks.test.js @@ -0,0 +1,108 @@ +/** + * WordPress dependencies + */ +import { + activatePlugin, + createNewPost, + deactivatePlugin, + getAllBlockInserterItemTitles, + insertBlock, + closeGlobalBlockInserter, +} from '@wordpress/e2e-test-utils'; + +const QUICK_INSERTER_RESULTS_SELECTOR = + '.block-editor-inserter__quick-inserter-results'; + +describe( 'Prioritized Inserter Blocks Setting on InnerBlocks', () => { + beforeAll( async () => { + await activatePlugin( + 'gutenberg-test-innerblocks-prioritized-inserter-blocks' + ); + } ); + + beforeEach( async () => { + await createNewPost(); + } ); + + afterAll( async () => { + await deactivatePlugin( + 'gutenberg-test-innerblocks-prioritized-inserter-blocks' + ); + } ); + + describe( 'Quick inserter', () => { + it( 'uses defaulting ordering if prioritzed blocks setting was not set', async () => { + const parentBlockSelector = + '[data-type="test/prioritized-inserter-blocks-unset"]'; + await insertBlock( 'Prioritized Inserter Blocks Unset' ); + await closeGlobalBlockInserter(); + + await page.waitForSelector( parentBlockSelector ); + + await page.click( + `${ parentBlockSelector } .block-list-appender .block-editor-inserter__toggle` + ); + + await page.waitForSelector( QUICK_INSERTER_RESULTS_SELECTOR ); + + await expect( await getAllBlockInserterItemTitles() ).toHaveLength( + 6 + ); + } ); + + it( 'uses the priority ordering if prioritzed blocks setting is set', async () => { + const parentBlockSelector = + '[data-type="test/prioritized-inserter-blocks-set"]'; + await insertBlock( 'Prioritized Inserter Blocks Set' ); + await closeGlobalBlockInserter(); + + await page.waitForSelector( parentBlockSelector ); + + await page.click( + `${ parentBlockSelector } .block-list-appender .block-editor-inserter__toggle` + ); + + await page.waitForSelector( QUICK_INSERTER_RESULTS_SELECTOR ); + + // Should still be only 6 results regardless of the priority ordering. + const inserterItems = await getAllBlockInserterItemTitles(); + + // Should still be only 6 results regardless of the priority ordering. + expect( inserterItems ).toHaveLength( 6 ); + + expect( inserterItems.slice( 0, 3 ) ).toEqual( [ + 'Audio', + 'Spacer', + 'Code', + ] ); + } ); + + it( 'obeys allowed blocks over prioritzed blocks setting if conflicted', async () => { + const parentBlockSelector = + '[data-type="test/prioritized-inserter-blocks-set-with-conflicting-allowed-blocks"]'; + await insertBlock( + 'Prioritized Inserter Blocks Set With Conflicting Allowed Blocks' + ); + await closeGlobalBlockInserter(); + + await page.waitForSelector( parentBlockSelector ); + + await page.click( + `${ parentBlockSelector } .block-list-appender .block-editor-inserter__toggle` + ); + + await page.waitForSelector( QUICK_INSERTER_RESULTS_SELECTOR ); + + const inserterItems = await getAllBlockInserterItemTitles(); + + expect( inserterItems.slice( 0, 3 ) ).toEqual( [ + 'Spacer', + 'Code', + 'Paragraph', + ] ); + expect( inserterItems ).toEqual( + expect.not.arrayContaining( [ 'Audio' ] ) + ); + } ); + } ); +} );