Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lazy render block types in the inserter #33749

Merged
merged 7 commits into from
Jul 30, 2021
Merged

Lazy render block types in the inserter #33749

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b970a7c
Lazy render block types in the inserter
youknowriad Jul 29, 2021
1e7c80e
Fix typo
youknowriad Jul 29, 2021
6a5443b
Remove useless unit test
youknowriad Jul 29, 2021
4f3dae1
Fix chrome rendering issues
youknowriad Jul 30, 2021
a8be13b
More stable e2e tests
youknowriad Jul 30, 2021
bbf1299
Fix collections rendering
youknowriad Jul 30, 2021
e3b089d
Make e2e test more stable
youknowriad Jul 30, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 52 additions & 22 deletions packages/block-editor/src/components/inserter/block-types-tab.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { map, flow, groupBy, orderBy } from 'lodash';
*/
import { __, _x } from '@wordpress/i18n';
import { useMemo, useEffect } from '@wordpress/element';
import { useAsyncList } from '@wordpress/compose';

/**
* Internal dependencies
Expand All @@ -21,6 +22,14 @@ const getBlockNamespace = ( item ) => item.name.split( '/' )[ 0 ];

const MAX_SUGGESTED_ITEMS = 6;

/**
* Shared reference to an empty array for cases where it is important to avoid
* returning a new array reference on every invocation and rerendering the component.
*
* @type {Array}
*/
const EMPTY_ARRAY = [];

export function BlockTypesTab( {
rootClientId,
onInsert,
Expand Down Expand Up @@ -71,6 +80,24 @@ export function BlockTypesTab( {
// Hide block preview on unmount.
useEffect( () => () => onHover( null ), [] );

/**
* The inserter contains a big number of blocks and opening it is a costful operation.
* The rendering is the most costful part of it, in order to improve the responsiveness
* of the "opening" action, these lazy lists allow us to render the inserter category per category,
* once all the categories are rendered, we start rendering the collections and the uncategorized block types.
*/
const currentlyRenderedCategories = useAsyncList( categories );
const didRenderAllCategories =
categories.length === currentlyRenderedCategories.length;

// Async List requires an array
const collectionEntries = useMemo( () => {
return Object.entries( collections );
}, [ collections ] );
const currentlyRenderedCollections = useAsyncList(
didRenderAllCategories ? collectionEntries : EMPTY_ARRAY
);

return (
<InserterListbox>
<div>
Expand All @@ -85,7 +112,7 @@ export function BlockTypesTab( {
</InserterPanel>
) }

{ map( categories, ( category ) => {
{ map( currentlyRenderedCategories, ( category ) => {
const categoryItems = itemsPerCategory[ category.slug ];
if ( ! categoryItems || ! categoryItems.length ) {
return null;
Expand All @@ -106,7 +133,7 @@ export function BlockTypesTab( {
);
} ) }

{ uncategorizedItems.length > 0 && (
{ didRenderAllCategories && uncategorizedItems.length > 0 && (
<InserterPanel
className="block-editor-inserter__uncategorized-blocks-panel"
title={ __( 'Uncategorized' ) }
Expand All @@ -120,27 +147,30 @@ export function BlockTypesTab( {
</InserterPanel>
) }

{ map( collections, ( collection, namespace ) => {
const collectionItems = itemsPerCollection[ namespace ];
if ( ! collectionItems || ! collectionItems.length ) {
return null;
{ map(
currentlyRenderedCollections,
( [ namespace, collection ] ) => {
const collectionItems = itemsPerCollection[ namespace ];
if ( ! collectionItems || ! collectionItems.length ) {
return null;
}

return (
<InserterPanel
key={ namespace }
title={ collection.title }
icon={ collection.icon }
>
<BlockTypesList
items={ collectionItems }
onSelect={ onSelectItem }
onHover={ onHover }
label={ collection.title }
/>
</InserterPanel>
);
}

return (
<InserterPanel
key={ namespace }
title={ collection.title }
icon={ collection.icon }
>
<BlockTypesList
items={ collectionItems }
onSelect={ onSelectItem }
onHover={ onHover }
label={ collection.title }
/>
</InserterPanel>
);
} ) }
) }
</div>
</InserterListbox>
);
Expand Down
150 changes: 0 additions & 150 deletions packages/block-editor/src/components/inserter/test/block-types-tab.js
View file
Open in desktop

This file was deleted.

7 changes: 7 additions & 0 deletions packages/e2e-test-utils/src/get-all-block-inserter-item-titles.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@ import { sortBy, uniq } from 'lodash';
* @return {Promise} Promise resolving with an array containing all inserter item titles.
*/
export async function getAllBlockInserterItemTitles() {
// The inserter render lazy renders the list of blocks
// meaning we should wait for the browser to be completed idle.
// Ideally, we shouldn't use a timeout and instead check the browser is idle for
// a specific duration, but didn't manage to find a simple way to do that.
// eslint-disable-next-line no-restricted-syntax
await page.waitFor( 500 );
Copy link
Contributor

Choose a reason for hiding this comment

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

This is likely to be fragile. Perhaps split up the helper to fetch tiles for a specific section, or wait for all expected categories to appear instead of a wait.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

wait for all expected categories to appear instead of a wait.

This is exactly what I want to do here, but I didn't find a way to do it without introducing test specific code in the production code which I'm not a fan of.


const inserterItemTitles = await page.evaluate( () => {
return Array.from(
document.querySelectorAll(
Expand Down
2 changes: 1 addition & 1 deletion packages/e2e-tests/specs/editor/plugins/register-block-type-hooks.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ describe( 'Register block type hooks', () => {
it( 'has a custom category for Paragraph block', async () => {
await openGlobalBlockInserter();

const widgetsCategory = await page.$(
const widgetsCategory = await page.waitForSelector(
'.block-editor-block-types-list[aria-label="Widgets"]'
);

Expand Down
1 change: 1 addition & 0 deletions packages/e2e-tests/specs/editor/various/inserting-blocks.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,7 @@ describe( 'Inserting blocks', () => {

it( 'shows block preview when hovering over block in inserter', async () => {
await openGlobalBlockInserter();
await page.waitForSelector( '.editor-block-list-item-paragraph' );
await page.focus( '.editor-block-list-item-paragraph' );
const preview = await page.waitForSelector(
'.block-editor-inserter__preview',
Expand Down