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

Jump to bottom

Try disabling async mode provider around selected block in ListView #34519

Merged
merged 6 commits into from
Sep 16, 2021
Merged

Try disabling async mode provider around selected block in ListView #34519

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a0c3238
Try disabling async mode provider around selected block in ListView
talldan Sep 3, 2021
7850761
Remove AsyncModeProvider from root of ListView
talldan Sep 3, 2021
398c3c5
Use useCallback to stop provider from constantly updating
talldan Sep 3, 2021
086c14b
Wrap inner part of ListView with AsyncModeProvider
talldan Sep 3, 2021
83ab2ae
Try also making dragged block synchronous
talldan Sep 13, 2021
d17c44f
Avoid rendering entire branch when parent is dragged
talldan Sep 15, 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
18 changes: 4 additions & 14 deletions packages/block-editor/src/components/list-view/block.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import { store as blockEditorStore } from '../../store';
export default function ListViewBlock( {
block,
isSelected,
isDragged,
isBranchSelected,
isLastOfSelectedBranch,
onClick,
Expand All @@ -48,20 +49,9 @@ export default function ListViewBlock( {
const cellRef = useRef( null );
const [ isHovered, setIsHovered ] = useState( false );
const { clientId } = block;
const { isDragging, blockParents } = useSelect(
const blockParents = useSelect(
( select ) => {
const {
isBlockBeingDragged,
isAncestorBeingDragged,
getBlockParents,
} = select( blockEditorStore );

return {
isDragging:
isBlockBeingDragged( clientId ) ||
isAncestorBeingDragged( clientId ),
blockParents: getBlockParents( clientId ),
};
return select( blockEditorStore ).getBlockParents( clientId );
},
[ clientId ]
);
Expand Down Expand Up @@ -128,7 +118,7 @@ export default function ListViewBlock( {
'is-last-of-selected-branch':
withExperimentalPersistentListViewFeatures &&
isLastOfSelectedBranch,
'is-dragging': isDragging,
'is-dragging': isDragged,
} );

return (
Expand Down
31 changes: 19 additions & 12 deletions packages/block-editor/src/components/list-view/branch.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { map, compact } from 'lodash';
/**
* WordPress dependencies
*/
import { Fragment } from '@wordpress/element';
import { AsyncModeProvider } from '@wordpress/data';

/**
* Internal dependencies
Expand All @@ -20,7 +20,6 @@ export default function ListViewBranch( props ) {
const {
blocks,
selectBlock,
selectedBlockClientIds,
showAppender,
showBlockMovers,
showNestedBlocks,
Expand All @@ -32,20 +31,26 @@ export default function ListViewBranch( props ) {
isLastOfBranch = false,
} = props;

const {
expandedState,
expand,
collapse,
draggedClientIds,
selectedClientIds,
} = useListViewContext();

const isTreeRoot = ! parentBlockClientId;
const filteredBlocks = compact( blocks );
const itemHasAppender = ( parentClientId ) =>
showAppender &&
! isTreeRoot &&
isClientIdSelected( parentClientId, selectedBlockClientIds );
isClientIdSelected( parentClientId, selectedClientIds );
const hasAppender = itemHasAppender( parentBlockClientId );
// Add +1 to the rowCount to take the block appender into account.
const blockCount = filteredBlocks.length;
const rowCount = hasAppender ? blockCount + 1 : blockCount;
const appenderPosition = rowCount;

const { expandedState, expand, collapse } = useListViewContext();

return (
<>
{ map( filteredBlocks, ( block, index ) => {
Expand All @@ -63,7 +68,7 @@ export default function ListViewBranch( props ) {

const isSelected = isClientIdSelected(
clientId,
selectedBlockClientIds
selectedClientIds
);
const isSelectedBranch =
isBranchSelected || ( isSelected && hasNestedBranch );
Expand Down Expand Up @@ -93,12 +98,17 @@ export default function ListViewBranch( props ) {
}
};

// Make updates to the selected or dragged blocks synchronous,
// but asynchronous for any other block.
const isDragged = !! draggedClientIds?.includes( clientId );

return (
<Fragment key={ clientId }>
<AsyncModeProvider key={ clientId } value={ ! isSelected }>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could be further enhanced using an IntersectionObserver.

<ListViewBlock
block={ block }
onClick={ selectBlockWithClientId }
onToggleExpanded={ toggleExpanded }
isDragged={ isDragged }
isSelected={ isSelected }
isBranchSelected={ isSelectedBranch }
isLastOfSelectedBranch={ isLastOfSelectedBranch }
Expand All @@ -111,12 +121,9 @@ export default function ListViewBranch( props ) {
path={ updatedPath }
isExpanded={ isExpanded }
/>
{ hasNestedBranch && isExpanded && (
{ hasNestedBranch && isExpanded && ! isDragged && (
<ListViewBranch
blocks={ innerBlocks }
selectedBlockClientIds={
selectedBlockClientIds
}
selectBlock={ selectBlock }
isBranchSelected={ isSelectedBranch }
isLastOfBranch={ isLast }
Expand All @@ -129,7 +136,7 @@ export default function ListViewBranch( props ) {
path={ updatedPath }
/>
) }
</Fragment>
</AsyncModeProvider>
);
} ) }
{ hasAppender && (
Expand Down
47 changes: 30 additions & 17 deletions packages/block-editor/src/components/list-view/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

import { useMergeRefs } from '@wordpress/compose';
import { __experimentalTreeGrid as TreeGrid } from '@wordpress/components';
import { useDispatch } from '@wordpress/data';
import { AsyncModeProvider, useDispatch } from '@wordpress/data';
import {
useCallback,
useEffect,
Expand Down Expand Up @@ -62,7 +62,11 @@ function ListView(
},
ref
) {
const { clientIdsTree, selectedClientIds } = useListViewClientIds(
const {
clientIdsTree,
selectedClientIds,
draggedClientIds,
} = useListViewClientIds(
blocks,
showOnlyCurrentHierarchy,
__experimentalPersistentListViewFeatures
Expand All @@ -86,18 +90,24 @@ function ListView(
isMounted.current = true;
}, [] );

const expand = ( clientId ) => {
if ( ! clientId ) {
return;
}
setExpandedState( { type: 'expand', clientId } );
};
const collapse = ( clientId ) => {
if ( ! clientId ) {
return;
}
setExpandedState( { type: 'collapse', clientId } );
};
const expand = useCallback(
( clientId ) => {
if ( ! clientId ) {
return;
}
setExpandedState( { type: 'expand', clientId } );
},
[ setExpandedState ]
);
const collapse = useCallback(
( clientId ) => {
if ( ! clientId ) {
return;
}
setExpandedState( { type: 'collapse', clientId } );
},
[ setExpandedState ]
);
Comment on lines +93 to +110
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are used in a context provider, so I think they should be memoized to stop the consumers from constantly updating.

const expandRow = ( row ) => {
expand( row?.dataset?.block );
};
Expand All @@ -110,6 +120,8 @@ function ListView(
__experimentalFeatures,
__experimentalPersistentListViewFeatures,
isTreeGridMounted: isMounted.current,
draggedClientIds,
selectedClientIds,
expandedState,
expand,
collapse,
Expand All @@ -118,14 +130,16 @@ function ListView(
__experimentalFeatures,
__experimentalPersistentListViewFeatures,
isMounted.current,
draggedClientIds,
selectedClientIds,
expandedState,
expand,
collapse,
]
);

return (
<>
<AsyncModeProvider value={ true }>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suppose this could be <AsyncModeProvider value>, but that seems a bit unusual.

<ListViewDropIndicator
listViewRef={ elementRef }
blockDropTarget={ blockDropTarget }
Expand All @@ -141,12 +155,11 @@ function ListView(
<ListViewBranch
blocks={ clientIdsTree }
selectBlock={ selectEditorBlock }
selectedBlockClientIds={ selectedClientIds }
{ ...props }
/>
</ListViewContext.Provider>
</TreeGrid>
</>
</AsyncModeProvider>
);
}
export default forwardRef( ListView );
44 changes: 22 additions & 22 deletions packages/block-editor/src/components/list-view/use-list-view-client-ids.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,25 +10,6 @@ import { useSelect } from '@wordpress/data';
import { isClientIdSelected } from './utils';
import { store as blockEditorStore } from '../../store';

const useListViewSelectedClientIds = (
__experimentalPersistentListViewFeatures
) =>
useSelect(
( select ) => {
const {
getSelectedBlockClientId,
getSelectedBlockClientIds,
} = select( blockEditorStore );

if ( __experimentalPersistentListViewFeatures ) {
return getSelectedBlockClientIds();
}

return getSelectedBlockClientId();
},
[ __experimentalPersistentListViewFeatures ]
);

const useListViewClientIdsTree = (
blocks,
selectedClientIds,
Expand Down Expand Up @@ -76,13 +57,32 @@ export default function useListViewClientIds(
showOnlyCurrentHierarchy,
__experimentalPersistentListViewFeatures
) {
const selectedClientIds = useListViewSelectedClientIds(
__experimentalPersistentListViewFeatures
const { selectedClientIds, draggedClientIds } = useSelect(
( select ) => {
const {
getSelectedBlockClientId,
getSelectedBlockClientIds,
getDraggedBlockClientIds,
} = select( blockEditorStore );

if ( __experimentalPersistentListViewFeatures ) {
return {
selectedClientIds: getSelectedBlockClientIds(),
draggedClientIds: getDraggedBlockClientIds(),
};
}

return {
selectedClientIds: getSelectedBlockClientId(),
Copy link
Contributor

@gwwar gwwar Sep 15, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

For other reviewers getSelectedBlockClientIds vs getSelectedBlockClientId

draggedClientIds: getDraggedBlockClientIds(),
};
},
[ __experimentalPersistentListViewFeatures ]
);
const clientIdsTree = useListViewClientIdsTree(
blocks,
selectedClientIds,
showOnlyCurrentHierarchy
);
return { clientIdsTree, selectedClientIds };
return { clientIdsTree, selectedClientIds, draggedClientIds };
}
8 changes: 2 additions & 6 deletions packages/edit-post/src/components/layout/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
EditorSnackbars,
store as editorStore,
} from '@wordpress/editor';
import { AsyncModeProvider, useSelect, useDispatch } from '@wordpress/data';
import { useSelect, useDispatch } from '@wordpress/data';
import { BlockBreadcrumb } from '@wordpress/block-editor';
import { Button, ScrollLock, Popover } from '@wordpress/components';
import { useViewportMatch } from '@wordpress/compose';
Expand Down Expand Up @@ -173,11 +173,7 @@ function Layout( { styles } ) {
return <InserterSidebar />;
}
if ( mode === 'visual' && isListViewOpened ) {
return (
<AsyncModeProvider value="true">
<ListViewSidebar />
</AsyncModeProvider>
);
return <ListViewSidebar />;
}
return null;
};
Expand Down
8 changes: 2 additions & 6 deletions packages/edit-site/src/components/editor/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* WordPress dependencies
*/
import { useEffect, useState, useMemo, useCallback } from '@wordpress/element';
import { AsyncModeProvider, useSelect, useDispatch } from '@wordpress/data';
import { useSelect, useDispatch } from '@wordpress/data';
import {
SlotFillProvider,
Popover,
Expand Down Expand Up @@ -169,11 +169,7 @@ function Editor( { initialSettings, onError } ) {
return <InserterSidebar />;
}
if ( isListViewOpen ) {
return (
<AsyncModeProvider value="true">
<ListViewSidebar />
</AsyncModeProvider>
);
return <ListViewSidebar />;
}
return null;
};
Expand Down