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.

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

List View: render a fixed number of items #35230

Merged
merged 6 commits into from
Oct 27, 2021
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
18 changes: 17 additions & 1 deletion bin/plugin/commands/performance.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ const config = require( '../config' );
* @property {number[]} inserterOpen Average time to open global inserter.
* @property {number[]} inserterSearch Average time to search the inserter.
* @property {number[]} inserterHover Average time to move mouse between two block item in the inserter.
* @property {number[]} listViewOpen Average time to open listView
*/

/**
Expand All @@ -52,7 +53,7 @@ const config = require( '../config' );
* @property {number=} firstContentfulPaint Represents the time when the browser first renders any text or media.
* @property {number=} firstBlock Represents the time when Puppeteer first sees a block selector in the DOM.
* @property {number=} type Average type time.
* @property {number=} minType Minium type time.
* @property {number=} minType Minimum type time.
* @property {number=} maxType Maximum type time.
* @property {number=} focus Average block selection time.
* @property {number=} minFocus Min block selection time.
Expand All @@ -66,6 +67,9 @@ const config = require( '../config' );
* @property {number=} inserterHover Average time to move mouse between two block item in the inserter.
* @property {number=} minInserterHover Min time to move mouse between two block item in the inserter.
* @property {number=} maxInserterHover Max time to move mouse between two block item in the inserter.
* @property {number=} listViewOpen Average time to open list view.
* @property {number=} minListViewOpen Min time to open list view.
* @property {number=} maxListViewOpen Max time to open list view.
*/

/**
Expand Down Expand Up @@ -136,6 +140,9 @@ function curateResults( results ) {
inserterHover: average( results.inserterHover ),
minInserterHover: Math.min( ...results.inserterHover ),
maxInserterHover: Math.max( ...results.inserterHover ),
listViewOpen: average( results.listViewOpen ),
minListViewOpen: Math.min( ...results.listViewOpen ),
maxListViewOpen: Math.max( ...results.listViewOpen ),
};
}

Expand Down Expand Up @@ -378,6 +385,15 @@ async function runPerformanceTests( branches, options ) {
maxInserterHover: rawResults.map(
( r ) => r[ branch ].maxInserterHover
),
listViewOpen: rawResults.map(
( r ) => r[ branch ].listViewOpen
),
minListViewOpen: rawResults.map(
( r ) => r[ branch ].minListViewOpen
),
maxListViewOpen: rawResults.map(
( r ) => r[ branch ].maxListViewOpen
),
},
median
);
Expand Down
3 changes: 2 additions & 1 deletion packages/block-editor/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@

### Performance

- Avoid re-rendering all List View items on block focus [#35706](https://github.com/WordPress/gutenberg/pull/35706). These changes speed up block focus time in large posts by 80% when List View is open.
- Avoid re-rendering all List View items on block focus [#35706](https://github.com/WordPress/gutenberg/pull/35706). When List View is open Block focus time is 4 times faster in large posts.
- Render fixed number of items in List View [#35706](https://github.com/WordPress/gutenberg/pull/35230). Opening List View is 13 times faster in large posts.

### Breaking change

Expand Down
104 changes: 85 additions & 19 deletions packages/block-editor/src/components/list-view/branch.js
Original file line number Diff line number Diff line change
@@ -1,41 +1,94 @@
/**
* External dependencies
*/
import { map, compact } from 'lodash';
import { compact } from 'lodash';

/**
* WordPress dependencies
*/
import { Fragment } from '@wordpress/element';
import { Fragment, memo } from '@wordpress/element';

/**
* Internal dependencies
*/
import ListViewBlock from './block';
import { useListViewContext } from './context';

export default function ListViewBranch( props ) {
function countBlocks( block, expandedState, draggedClientIds ) {
const isDragged = draggedClientIds?.includes( block.clientId );
if ( isDragged ) {
return 0;
}
const isExpanded = expandedState[ block.clientId ] ?? true;
if ( isExpanded ) {
return (
1 +
block.innerBlocks.reduce(
countReducer( expandedState, draggedClientIds ),
0
)
);
}
return 1;
}
const countReducer = ( expandedState, draggedClientIds ) => (
count,
block
) => {
const isDragged = draggedClientIds?.includes( block.clientId );
if ( isDragged ) {
return count;
}
const isExpanded = expandedState[ block.clientId ] ?? true;
if ( isExpanded && block.innerBlocks.length > 0 ) {
return count + countBlocks( block, expandedState, draggedClientIds );
}
return count + 1;
};
gwwar marked this conversation as resolved.
Show resolved Hide resolved

function ListViewBranch( props ) {
const {
blocks,
selectBlock,
showBlockMovers,
showNestedBlocks,
level = 1,
path = '',
listPosition = 0,
fixedListWindow,
} = props;

const { expandedState, draggedClientIds } = useListViewContext();
const {
expandedState,
draggedClientIds,
__experimentalPersistentListViewFeatures,
} = useListViewContext();

const filteredBlocks = compact( blocks );
const blockCount = filteredBlocks.length;
let nextPosition = listPosition;

return (
<>
{ map( filteredBlocks, ( block, index ) => {
{ filteredBlocks.map( ( block, index ) => {
const { clientId, innerBlocks } = block;

if ( index > 0 ) {
nextPosition += countBlocks(
filteredBlocks[ index - 1 ],
expandedState,
draggedClientIds
);
}

const usesWindowing = __experimentalPersistentListViewFeatures;

const { itemInView } = fixedListWindow;

const blockInView =
! usesWindowing || itemInView( nextPosition );

const position = index + 1;
// This string value is used to trigger an animation change.
// This may be removed if we use a different animation library in the future.
const updatedPath =
path.length > 0
? `${ path }_${ position }`
Expand All @@ -49,20 +102,29 @@ export default function ListViewBranch( props ) {

const isDragged = !! draggedClientIds?.includes( clientId );

const showBlock = isDragged || blockInView;
return (
<Fragment key={ clientId }>
<ListViewBlock
block={ block }
selectBlock={ selectBlock }
isDragged={ isDragged }
level={ level }
position={ position }
rowCount={ blockCount }
siblingBlockCount={ blockCount }
showBlockMovers={ showBlockMovers }
path={ updatedPath }
isExpanded={ isExpanded }
/>
{ showBlock && (
<ListViewBlock
block={ block }
selectBlock={ selectBlock }
isDragged={ isDragged }
level={ level }
position={ position }
rowCount={ blockCount }
siblingBlockCount={ blockCount }
showBlockMovers={ showBlockMovers }
path={ updatedPath }
isExpanded={ isExpanded }
listPosition={ nextPosition }
/>
) }
{ ! showBlock && (
<tr>
<td className="block-editor-list-view-placeholder" />
</tr>
) }
{ hasNestedBlocks && isExpanded && ! isDragged && (
<ListViewBranch
blocks={ innerBlocks }
Expand All @@ -71,6 +133,8 @@ export default function ListViewBranch( props ) {
showNestedBlocks={ showNestedBlocks }
level={ level + 1 }
path={ updatedPath }
listPosition={ nextPosition + 1 }
fixedListWindow={ fixedListWindow }
/>
) }
</Fragment>
Expand All @@ -83,3 +147,5 @@ export default function ListViewBranch( props ) {
ListViewBranch.defaultProps = {
selectBlock: () => {},
};

export default memo( ListViewBranch );
33 changes: 30 additions & 3 deletions packages/block-editor/src/components/list-view/index.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
/**
* WordPress dependencies
*/

import { useMergeRefs } from '@wordpress/compose';
import {
useMergeRefs,
__experimentalUseFixedWindowList as useFixedWindowList,
} from '@wordpress/compose';
import { __experimentalTreeGrid as TreeGrid } from '@wordpress/components';
import { AsyncModeProvider, useDispatch } from '@wordpress/data';
import { AsyncModeProvider, useDispatch, useSelect } from '@wordpress/data';
import {
useCallback,
useEffect,
Expand Down Expand Up @@ -67,6 +69,21 @@ function ListView(
) {
const { clientIdsTree, draggedClientIds } = useListViewClientIds( blocks );
const { selectBlock } = useDispatch( blockEditorStore );
const { visibleBlockCount } = useSelect(
( select ) => {
const { getGlobalBlockCount, getClientIdsOfDescendants } = select(
blockEditorStore
);
const draggedBlockCount =
draggedClientIds?.length > 0
? getClientIdsOfDescendants( draggedClientIds ).length + 1
: 0;
return {
visibleBlockCount: getGlobalBlockCount() - draggedBlockCount,
};
},
[ draggedClientIds ]
);
const selectEditorBlock = useCallback(
( clientId ) => {
selectBlock( clientId );
Expand All @@ -85,6 +102,15 @@ function ListView(
isMounted.current = true;
}, [] );

const [ fixedListWindow ] = useFixedWindowList(
elementRef,
36,
gwwar marked this conversation as resolved.
Show resolved Hide resolved
visibleBlockCount,
{
useWindowing: __experimentalPersistentListViewFeatures,
}
);

const expand = useCallback(
( clientId ) => {
if ( ! clientId ) {
Expand Down Expand Up @@ -158,6 +184,7 @@ function ListView(
selectBlock={ selectEditorBlock }
showNestedBlocks={ showNestedBlocks }
showBlockMovers={ showBlockMovers }
fixedListWindow={ fixedListWindow }
{ ...props }
/>
</ListViewContext.Provider>
Expand Down
6 changes: 6 additions & 0 deletions packages/block-editor/src/components/list-view/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -357,3 +357,9 @@ $block-navigation-max-indent: 8;
box-shadow: none;
}

.block-editor-list-view-placeholder {
padding: 0;
margin: 0;
height: 36px;
}

Loading