From 722872542d090c3ee4e3c54eec2352e95c752997 Mon Sep 17 00:00:00 2001 From: ramonjd Date: Wed, 20 Oct 2021 15:42:53 +1100 Subject: [PATCH 01/15] Check element ref for active document so we can try to expand the list only when a block is selected outside the block list. Using an `useEffect` to loop through parent nodes, and expand them when necessary. --- .../src/components/list-view/index.js | 26 +++++++++++++++++-- .../list-view/use-list-view-client-ids.js | 7 +++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/packages/block-editor/src/components/list-view/index.js b/packages/block-editor/src/components/list-view/index.js index 4f7c9313348935..7d0ad770125818 100644 --- a/packages/block-editor/src/components/list-view/index.js +++ b/packages/block-editor/src/components/list-view/index.js @@ -27,6 +27,7 @@ import useBlockSelection from './use-block-selection'; import useListViewClientIds from './use-list-view-client-ids'; import useListViewDropZone from './use-list-view-drop-zone'; import { store as blockEditorStore } from '../../store'; +import { hasFocusWithin } from './utils'; const expanded = ( state, action ) => { switch ( action.type ) { @@ -69,6 +70,7 @@ function ListView( clientIdsTree, draggedClientIds, selectedClientIds, + selectedBlockParentIds, } = useListViewClientIds( blocks ); const { visibleBlockCount } = useSelect( @@ -90,12 +92,12 @@ function ListView( const { updateBlockSelection } = useBlockSelection(); const [ expandedState, setExpandedState ] = useReducer( expanded, {} ); - const { ref: dropZoneRef, target: blockDropTarget } = useListViewDropZone(); const elementRef = useRef(); const treeGridRef = useMergeRefs( [ elementRef, dropZoneRef, ref ] ); - const isMounted = useRef( false ); + const hasFocus = hasFocusWithin( elementRef?.current ); + useEffect( () => { isMounted.current = true; }, [] ); @@ -179,6 +181,26 @@ function ListView( ] ); + // If a selection is made outside the block list, + // for example, in the Block Editor, + // try to expand the block list tree. + useEffect( () => { + if ( + ! hasFocus && + Array.isArray( selectedBlockParentIds ) && + selectedBlockParentIds.length + ) { + selectedBlockParentIds.forEach( ( clientId ) => { + if ( ! expandedState[ clientId ] ) { + setExpandedState( { + type: 'expand', + clientId, + } ); + } + } ); + } + }, [ hasFocus, selectedBlockParentIds ] ); + return ( Date: Mon, 25 Oct 2021 18:15:38 +1100 Subject: [PATCH 02/15] Renamed parent client id array to `selectedBlockParentClientIds` --- .../block-editor/src/components/list-view/index.js | 10 +++++----- .../components/list-view/use-list-view-client-ids.js | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/block-editor/src/components/list-view/index.js b/packages/block-editor/src/components/list-view/index.js index 7d0ad770125818..f3530679f99807 100644 --- a/packages/block-editor/src/components/list-view/index.js +++ b/packages/block-editor/src/components/list-view/index.js @@ -70,7 +70,7 @@ function ListView( clientIdsTree, draggedClientIds, selectedClientIds, - selectedBlockParentIds, + selectedBlockParentClientIds, } = useListViewClientIds( blocks ); const { visibleBlockCount } = useSelect( @@ -187,10 +187,10 @@ function ListView( useEffect( () => { if ( ! hasFocus && - Array.isArray( selectedBlockParentIds ) && - selectedBlockParentIds.length + Array.isArray( selectedBlockParentClientIds ) && + selectedBlockParentClientIds.length ) { - selectedBlockParentIds.forEach( ( clientId ) => { + selectedBlockParentClientIds.forEach( ( clientId ) => { if ( ! expandedState[ clientId ] ) { setExpandedState( { type: 'expand', @@ -199,7 +199,7 @@ function ListView( } } ); } - }, [ hasFocus, selectedBlockParentIds ] ); + }, [ hasFocus, selectedBlockParentClientIds ] ); return ( diff --git a/packages/block-editor/src/components/list-view/use-list-view-client-ids.js b/packages/block-editor/src/components/list-view/use-list-view-client-ids.js index 513ff251383d4d..72eac077cfe3ee 100644 --- a/packages/block-editor/src/components/list-view/use-list-view-client-ids.js +++ b/packages/block-editor/src/components/list-view/use-list-view-client-ids.js @@ -25,7 +25,7 @@ export default function useListViewClientIds( blocks ) { selectedClientIds: getSelectedBlockClientIds(), draggedClientIds: getDraggedBlockClientIds(), clientIdsTree: blocks ? blocks : __unstableGetClientIdsTree(), - selectedBlockParentIds: getBlockParents( + selectedBlockParentClientIds: getBlockParents( selectedBlockClientIds[ 0 ], false ), From 502c13c650421f15b21b1c0706039b1c187ae372 Mon Sep 17 00:00:00 2001 From: ramonjd Date: Mon, 1 Nov 2021 14:20:47 +1100 Subject: [PATCH 03/15] Creating a new action type, so we can update the state with one action call. --- .../src/components/list-view/index.js | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/packages/block-editor/src/components/list-view/index.js b/packages/block-editor/src/components/list-view/index.js index f3530679f99807..4400510d90c299 100644 --- a/packages/block-editor/src/components/list-view/index.js +++ b/packages/block-editor/src/components/list-view/index.js @@ -35,6 +35,17 @@ const expanded = ( state, action ) => { return { ...state, ...{ [ action.clientId ]: true } }; case 'collapse': return { ...state, ...{ [ action.clientId ]: false } }; + case 'expandAll': + return { + ...state, + ...action.clientIds.reduce( + ( newState, id ) => ( { + ...newState, + [ id ]: true, + } ), + {} + ), + }; default: return state; } @@ -190,13 +201,9 @@ function ListView( Array.isArray( selectedBlockParentClientIds ) && selectedBlockParentClientIds.length ) { - selectedBlockParentClientIds.forEach( ( clientId ) => { - if ( ! expandedState[ clientId ] ) { - setExpandedState( { - type: 'expand', - clientId, - } ); - } + setExpandedState( { + type: 'expandAll', + clientIds: selectedBlockParentClientIds, } ); } }, [ hasFocus, selectedBlockParentClientIds ] ); From dd830fc1cca51f76956425c4f121ba8bca43e577 Mon Sep 17 00:00:00 2001 From: ramonjd Date: Mon, 1 Nov 2021 14:53:02 +1100 Subject: [PATCH 04/15] Refactoring reducer to accept an array of clientIds. --- .../src/components/list-view/index.js | 42 +++++++------------ 1 file changed, 16 insertions(+), 26 deletions(-) diff --git a/packages/block-editor/src/components/list-view/index.js b/packages/block-editor/src/components/list-view/index.js index 4400510d90c299..06793558301dda 100644 --- a/packages/block-editor/src/components/list-view/index.js +++ b/packages/block-editor/src/components/list-view/index.js @@ -30,25 +30,19 @@ import { store as blockEditorStore } from '../../store'; import { hasFocusWithin } from './utils'; const expanded = ( state, action ) => { - switch ( action.type ) { - case 'expand': - return { ...state, ...{ [ action.clientId ]: true } }; - case 'collapse': - return { ...state, ...{ [ action.clientId ]: false } }; - case 'expandAll': - return { - ...state, - ...action.clientIds.reduce( - ( newState, id ) => ( { - ...newState, - [ id ]: true, - } ), - {} - ), - }; - default: - return state; + if ( Array.isArray( action.clientIds ) ) { + return { + ...state, + ...action.clientIds.reduce( + ( newState, id ) => ( { + ...newState, + [ id ]: action.type === 'expand', + } ), + {} + ), + }; } + return state; }; /** @@ -131,7 +125,7 @@ function ListView( if ( ! clientId ) { return; } - setExpandedState( { type: 'expand', clientId } ); + setExpandedState( { type: 'expand', clientIds: [ clientId ] } ); }, [ setExpandedState ] ); @@ -140,7 +134,7 @@ function ListView( if ( ! clientId ) { return; } - setExpandedState( { type: 'collapse', clientId } ); + setExpandedState( { type: 'collapse', clientIds: [ clientId ] } ); }, [ setExpandedState ] ); @@ -196,13 +190,9 @@ function ListView( // for example, in the Block Editor, // try to expand the block list tree. useEffect( () => { - if ( - ! hasFocus && - Array.isArray( selectedBlockParentClientIds ) && - selectedBlockParentClientIds.length - ) { + if ( ! hasFocus ) { setExpandedState( { - type: 'expandAll', + type: 'expand', clientIds: selectedBlockParentClientIds, } ); } From ce53172287261217f9e40c19972f0cf5781f0379 Mon Sep 17 00:00:00 2001 From: ramonjd Date: Wed, 17 Nov 2021 12:20:02 +1100 Subject: [PATCH 05/15] Only try to expand nested branch items where there are parent ids --- packages/block-editor/src/components/list-view/index.js | 6 +++++- .../src/components/list-view/use-list-view-client-ids.js | 1 - 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/block-editor/src/components/list-view/index.js b/packages/block-editor/src/components/list-view/index.js index 06793558301dda..97c56d0db5f009 100644 --- a/packages/block-editor/src/components/list-view/index.js +++ b/packages/block-editor/src/components/list-view/index.js @@ -190,7 +190,11 @@ function ListView( // for example, in the Block Editor, // try to expand the block list tree. useEffect( () => { - if ( ! hasFocus ) { + if ( + ! hasFocus && + Array.isArray( selectedBlockParentClientIds ) && + selectedBlockParentClientIds.length + ) { setExpandedState( { type: 'expand', clientIds: selectedBlockParentClientIds, diff --git a/packages/block-editor/src/components/list-view/use-list-view-client-ids.js b/packages/block-editor/src/components/list-view/use-list-view-client-ids.js index 72eac077cfe3ee..e009eba4e9aea2 100644 --- a/packages/block-editor/src/components/list-view/use-list-view-client-ids.js +++ b/packages/block-editor/src/components/list-view/use-list-view-client-ids.js @@ -16,7 +16,6 @@ export default function useListViewClientIds( blocks ) { getDraggedBlockClientIds, getSelectedBlockClientIds, __unstableGetClientIdsTree, - getSelectedBlockClientIds, getBlockParents, } = select( blockEditorStore ); const selectedBlockClientIds = getSelectedBlockClientIds(); From 629e78a23fb664e9cd8760b447e2d7d913291187 Mon Sep 17 00:00:00 2001 From: ramonjd Date: Fri, 7 Jan 2022 17:47:08 +1100 Subject: [PATCH 06/15] This commit can potentially be discarded as it might be worth exploring in a new PR. Here we're calculating the top scroll position of the selected block. --- .../src/components/list-view/branch.js | 2 +- .../src/components/list-view/index.js | 50 ++++++++++++++++++- 2 files changed, 49 insertions(+), 3 deletions(-) diff --git a/packages/block-editor/src/components/list-view/branch.js b/packages/block-editor/src/components/list-view/branch.js index 71518a1e289f3f..9ed78740a8e76b 100644 --- a/packages/block-editor/src/components/list-view/branch.js +++ b/packages/block-editor/src/components/list-view/branch.js @@ -28,7 +28,7 @@ import { isClientIdSelected } from './utils'; * @param {Array} draggedClientIds a list of dragged client ids * @return {number} block count */ -function countBlocks( block, expandedState, draggedClientIds ) { +export function countBlocks( block, expandedState, draggedClientIds ) { const isDragged = draggedClientIds?.includes( block.clientId ); if ( isDragged ) { return 0; diff --git a/packages/block-editor/src/components/list-view/index.js b/packages/block-editor/src/components/list-view/index.js index 97c56d0db5f009..81d92431794fa0 100644 --- a/packages/block-editor/src/components/list-view/index.js +++ b/packages/block-editor/src/components/list-view/index.js @@ -16,11 +16,12 @@ import { forwardRef, } from '@wordpress/element'; import { __ } from '@wordpress/i18n'; +import { getScrollContainer } from '@wordpress/dom'; /** * Internal dependencies */ -import ListViewBranch from './branch'; +import ListViewBranch, { countBlocks } from './branch'; import { ListViewContext } from './context'; import ListViewDropIndicator from './drop-indicator'; import useBlockSelection from './use-block-selection'; @@ -45,6 +46,8 @@ const expanded = ( state, action ) => { return state; }; +const BLOCK_LIST_ITEM_HEIGHT = 36; + /** * Wrap `ListViewRows` with `TreeGrid`. ListViewRows is a * recursive component (it renders itself), so this ensures TreeGrid is only @@ -112,7 +115,7 @@ function ListView( // See: https://github.com/WordPress/gutenberg/pull/35230 for additional context. const [ fixedListWindow ] = useFixedWindowList( elementRef, - 36, + BLOCK_LIST_ITEM_HEIGHT, visibleBlockCount, { useWindowing: __experimentalPersistentListViewFeatures, @@ -202,6 +205,49 @@ function ListView( } }, [ hasFocus, selectedBlockParentClientIds ] ); + useEffect( () => { + if ( + ! hasFocus && + Array.isArray( selectedClientIds ) && + selectedClientIds.length + ) { + const scrollContainer = getScrollContainer( elementRef.current ); + + // Grab the selected id. This is the point at which we can + // stop counting blocks in the tree. + let selectedId = selectedClientIds[ 0 ]; + + // If the selected block has parents, get the top-level parent. + if ( + Array.isArray( selectedBlockParentClientIds ) && + selectedBlockParentClientIds.length + ) { + selectedId = selectedBlockParentClientIds[ 0 ]; + } + + // Count expanded blocks in the tree. + let heightFactor = 0; + clientIdsTree.every( ( item ) => { + if ( item?.clientId === selectedId ) { + return false; + } + heightFactor += countBlocks( item, expandedState, [] ); + return true; + } ); + + scrollContainer?.scrollTo( { + top: heightFactor * BLOCK_LIST_ITEM_HEIGHT, + } ); + } + }, [ + hasFocus, + expandedState, + elementRef, + clientIdsTree, + selectedBlockParentClientIds, + selectedClientIds, + ] ); + return ( Date: Fri, 7 Jan 2022 23:10:56 +1100 Subject: [PATCH 07/15] Tracking the selected block in the tree and checking it against the selected block *should* inform us that a block has been selected elsewhere, e.g., the editor. When we know this we can execute the hook logic that expands and scrolls to the selected element in the tree if a block is selected elsewhere. Removing hasFocus. --- .../src/components/list-view/index.js | 84 ++++--------------- 1 file changed, 17 insertions(+), 67 deletions(-) diff --git a/packages/block-editor/src/components/list-view/index.js b/packages/block-editor/src/components/list-view/index.js index 81d92431794fa0..9edb0ea029f4c0 100644 --- a/packages/block-editor/src/components/list-view/index.js +++ b/packages/block-editor/src/components/list-view/index.js @@ -16,19 +16,18 @@ import { forwardRef, } from '@wordpress/element'; import { __ } from '@wordpress/i18n'; -import { getScrollContainer } from '@wordpress/dom'; /** * Internal dependencies */ -import ListViewBranch, { countBlocks } from './branch'; +import ListViewBranch from './branch'; import { ListViewContext } from './context'; import ListViewDropIndicator from './drop-indicator'; import useBlockSelection from './use-block-selection'; import useListViewClientIds from './use-list-view-client-ids'; import useListViewDropZone from './use-list-view-drop-zone'; +import useListViewOpenSelectedItem from './use-list-view-open-selected-item'; import { store as blockEditorStore } from '../../store'; -import { hasFocusWithin } from './utils'; const expanded = ( state, action ) => { if ( Array.isArray( action.clientIds ) ) { @@ -46,7 +45,7 @@ const expanded = ( state, action ) => { return state; }; -const BLOCK_LIST_ITEM_HEIGHT = 36; +export const BLOCK_LIST_ITEM_HEIGHT = 36; /** * Wrap `ListViewRows` with `TreeGrid`. ListViewRows is a @@ -78,7 +77,6 @@ function ListView( clientIdsTree, draggedClientIds, selectedClientIds, - selectedBlockParentClientIds, } = useListViewClientIds( blocks ); const { visibleBlockCount } = useSelect( @@ -100,12 +98,23 @@ function ListView( const { updateBlockSelection } = useBlockSelection(); const [ expandedState, setExpandedState ] = useReducer( expanded, {} ); + const { ref: dropZoneRef, target: blockDropTarget } = useListViewDropZone(); const elementRef = useRef(); const treeGridRef = useMergeRefs( [ elementRef, dropZoneRef, ref ] ); - const isMounted = useRef( false ); - const hasFocus = hasFocusWithin( elementRef?.current ); + const isMounted = useRef( false ); + const { setSelectedTreeId } = useListViewOpenSelectedItem( { + firstSelectedBlockClientId: selectedClientIds[ 0 ], + setExpandedState, + } ); + const selectEditorBlock = useCallback( + ( clientId ) => { + updateBlockSelection( clientId ); + setSelectedTreeId( clientId ); + }, + [ setSelectedTreeId, updateBlockSelection ] + ); useEffect( () => { isMounted.current = true; }, [] ); @@ -189,65 +198,6 @@ function ListView( ] ); - // If a selection is made outside the block list, - // for example, in the Block Editor, - // try to expand the block list tree. - useEffect( () => { - if ( - ! hasFocus && - Array.isArray( selectedBlockParentClientIds ) && - selectedBlockParentClientIds.length - ) { - setExpandedState( { - type: 'expand', - clientIds: selectedBlockParentClientIds, - } ); - } - }, [ hasFocus, selectedBlockParentClientIds ] ); - - useEffect( () => { - if ( - ! hasFocus && - Array.isArray( selectedClientIds ) && - selectedClientIds.length - ) { - const scrollContainer = getScrollContainer( elementRef.current ); - - // Grab the selected id. This is the point at which we can - // stop counting blocks in the tree. - let selectedId = selectedClientIds[ 0 ]; - - // If the selected block has parents, get the top-level parent. - if ( - Array.isArray( selectedBlockParentClientIds ) && - selectedBlockParentClientIds.length - ) { - selectedId = selectedBlockParentClientIds[ 0 ]; - } - - // Count expanded blocks in the tree. - let heightFactor = 0; - clientIdsTree.every( ( item ) => { - if ( item?.clientId === selectedId ) { - return false; - } - heightFactor += countBlocks( item, expandedState, [] ); - return true; - } ); - - scrollContainer?.scrollTo( { - top: heightFactor * BLOCK_LIST_ITEM_HEIGHT, - } ); - } - }, [ - hasFocus, - expandedState, - elementRef, - clientIdsTree, - selectedBlockParentClientIds, - selectedClientIds, - ] ); - return ( Date: Mon, 10 Jan 2022 13:21:50 +1100 Subject: [PATCH 08/15] Extracted scroll and expand logic to hook --- .../list-view/use-list-view-client-ids.js | 6 - .../use-list-view-open-selected-item.js | 139 ++++++++++++++++++ 2 files changed, 139 insertions(+), 6 deletions(-) create mode 100644 packages/block-editor/src/components/list-view/use-list-view-open-selected-item.js diff --git a/packages/block-editor/src/components/list-view/use-list-view-client-ids.js b/packages/block-editor/src/components/list-view/use-list-view-client-ids.js index e009eba4e9aea2..5dafa765f16ea5 100644 --- a/packages/block-editor/src/components/list-view/use-list-view-client-ids.js +++ b/packages/block-editor/src/components/list-view/use-list-view-client-ids.js @@ -16,18 +16,12 @@ export default function useListViewClientIds( blocks ) { getDraggedBlockClientIds, getSelectedBlockClientIds, __unstableGetClientIdsTree, - getBlockParents, } = select( blockEditorStore ); - const selectedBlockClientIds = getSelectedBlockClientIds(); return { selectedClientIds: getSelectedBlockClientIds(), draggedClientIds: getDraggedBlockClientIds(), clientIdsTree: blocks ? blocks : __unstableGetClientIdsTree(), - selectedBlockParentClientIds: getBlockParents( - selectedBlockClientIds[ 0 ], - false - ), }; }, [ blocks ] diff --git a/packages/block-editor/src/components/list-view/use-list-view-open-selected-item.js b/packages/block-editor/src/components/list-view/use-list-view-open-selected-item.js new file mode 100644 index 00000000000000..33aa221e14cb71 --- /dev/null +++ b/packages/block-editor/src/components/list-view/use-list-view-open-selected-item.js @@ -0,0 +1,139 @@ +/** + * WordPress dependencies + */ +import { useLayoutEffect, useEffect, useState } from '@wordpress/element'; +import { getScrollContainer } from '@wordpress/dom'; +import { useSelect } from '@wordpress/data'; + +/** + * Internal dependencies + */ +import { BLOCK_LIST_ITEM_HEIGHT } from './'; +import { countBlocks } from './branch'; +import { store as blockEditorStore } from '../../store'; + +export default function useListViewOpenSelectedItem( { + firstSelectedBlockClientId, + clientIdsTree, + blockListItemHeight = BLOCK_LIST_ITEM_HEIGHT, + scrollContainerElement, + expandedState, + setExpandedState, +} ) { + const [ selectedTreeId, setSelectedTreeId ] = useState( null ); + const scrollContainer = getScrollContainer( scrollContainerElement ); + const { selectedBlockParentClientIds } = useSelect( + ( select ) => { + const { getBlockParents } = select( blockEditorStore ); + return { + selectedBlockParentClientIds: getBlockParents( + firstSelectedBlockClientId, + false + ), + }; + }, + [ firstSelectedBlockClientId ] + ); + + const parentClientIds = + Array.isArray( selectedBlockParentClientIds ) && + selectedBlockParentClientIds.length + ? selectedBlockParentClientIds + : null; + + // Track the expanded state of any parents. + // To calculate the number of expanded items correctly. + let parentExpandedState = null; + if ( parentClientIds ) { + parentExpandedState = expandedState[ parentClientIds[ 0 ] ]; + } + + useEffect( () => { + // If the selectedTreeId is the same as the selected block, + // it means that the block was selected using the block list tree. + if ( selectedTreeId === firstSelectedBlockClientId ) { + return; + } + + // If the selected block has parents, get the top-level parent. + if ( parentClientIds ) { + // If the selected block has parents, + // expand the tree branch. + setExpandedState( { + type: 'expand', + clientIds: selectedBlockParentClientIds, + } ); + } + }, [ firstSelectedBlockClientId ] ); + + useLayoutEffect( () => { + // If the selectedTreeId is the same as the selected block, + // it means that the block was selected using the block list tree. + if ( selectedTreeId === firstSelectedBlockClientId ) { + return; + } + if ( + scrollContainer && + !! firstSelectedBlockClientId && + Array.isArray( clientIdsTree ) && + clientIdsTree.length + ) { + // Grab the selected id. This is the point at which we can + // stop counting blocks in the tree. + let selectedId = firstSelectedBlockClientId; + + // If the selected block has parents, get the top-level parent. + if ( parentClientIds ) { + selectedId = parentClientIds[ 0 ]; + // If the selected block has parents, + // check to see if the selected tree is expanded + // so we can accurately calculate the scroll container top value. + if ( ! parentExpandedState ) { + return; + } + } + + // Count expanded blocks in the tree up until the selected block, + // so we can calculate the scroll container top value. + let listItemHeightFactor = 0; + clientIdsTree.every( ( item ) => { + if ( item?.clientId === selectedId ) { + return false; + } + listItemHeightFactor += countBlocks( item, expandedState, [] ); + return true; + } ); + + // New scroll value is the number of expanded items + // multiplied by the item height + // plus the number of expanded children in the selected block + // multiplied by the item height. + const newScrollTopValue = + listItemHeightFactor * blockListItemHeight + + ( parentClientIds ? parentClientIds.length : 1 ) * + blockListItemHeight; + + const shouldScrollDown = + newScrollTopValue > + scrollContainer.scrollTop + scrollContainer.clientHeight; + + const shouldScrollUp = + newScrollTopValue < scrollContainer.scrollTop; + + if ( ! shouldScrollUp && ! shouldScrollDown ) { + return; + } + + // @TODO This doesn't yet work when nested blocks are selected. + // We're still using the top parent block to calculate/trigger redraw. + // If selected block is already visible in the list prevent scroll. + scrollContainer?.scrollTo( { + top: newScrollTopValue, + } ); + } + }, [ firstSelectedBlockClientId, parentExpandedState ] ); + + return { + setSelectedTreeId, + }; +} From 8f025c5118077b347c407efadec579873c984cd2 Mon Sep 17 00:00:00 2001 From: ramonjd Date: Tue, 1 Feb 2022 15:51:11 +1100 Subject: [PATCH 09/15] Revert scroll to logic to be performed in another PR. --- .../use-list-view-open-selected-item.js | 85 +------------------ 1 file changed, 2 insertions(+), 83 deletions(-) diff --git a/packages/block-editor/src/components/list-view/use-list-view-open-selected-item.js b/packages/block-editor/src/components/list-view/use-list-view-open-selected-item.js index 33aa221e14cb71..2dbc918b3d11c8 100644 --- a/packages/block-editor/src/components/list-view/use-list-view-open-selected-item.js +++ b/packages/block-editor/src/components/list-view/use-list-view-open-selected-item.js @@ -1,27 +1,19 @@ /** * WordPress dependencies */ -import { useLayoutEffect, useEffect, useState } from '@wordpress/element'; -import { getScrollContainer } from '@wordpress/dom'; +import { useEffect, useState } from '@wordpress/element'; import { useSelect } from '@wordpress/data'; /** * Internal dependencies */ -import { BLOCK_LIST_ITEM_HEIGHT } from './'; -import { countBlocks } from './branch'; import { store as blockEditorStore } from '../../store'; export default function useListViewOpenSelectedItem( { firstSelectedBlockClientId, - clientIdsTree, - blockListItemHeight = BLOCK_LIST_ITEM_HEIGHT, - scrollContainerElement, - expandedState, setExpandedState, } ) { const [ selectedTreeId, setSelectedTreeId ] = useState( null ); - const scrollContainer = getScrollContainer( scrollContainerElement ); const { selectedBlockParentClientIds } = useSelect( ( select ) => { const { getBlockParents } = select( blockEditorStore ); @@ -41,13 +33,7 @@ export default function useListViewOpenSelectedItem( { ? selectedBlockParentClientIds : null; - // Track the expanded state of any parents. - // To calculate the number of expanded items correctly. - let parentExpandedState = null; - if ( parentClientIds ) { - parentExpandedState = expandedState[ parentClientIds[ 0 ] ]; - } - + // Expand tree when a block is selected. useEffect( () => { // If the selectedTreeId is the same as the selected block, // it means that the block was selected using the block list tree. @@ -66,73 +52,6 @@ export default function useListViewOpenSelectedItem( { } }, [ firstSelectedBlockClientId ] ); - useLayoutEffect( () => { - // If the selectedTreeId is the same as the selected block, - // it means that the block was selected using the block list tree. - if ( selectedTreeId === firstSelectedBlockClientId ) { - return; - } - if ( - scrollContainer && - !! firstSelectedBlockClientId && - Array.isArray( clientIdsTree ) && - clientIdsTree.length - ) { - // Grab the selected id. This is the point at which we can - // stop counting blocks in the tree. - let selectedId = firstSelectedBlockClientId; - - // If the selected block has parents, get the top-level parent. - if ( parentClientIds ) { - selectedId = parentClientIds[ 0 ]; - // If the selected block has parents, - // check to see if the selected tree is expanded - // so we can accurately calculate the scroll container top value. - if ( ! parentExpandedState ) { - return; - } - } - - // Count expanded blocks in the tree up until the selected block, - // so we can calculate the scroll container top value. - let listItemHeightFactor = 0; - clientIdsTree.every( ( item ) => { - if ( item?.clientId === selectedId ) { - return false; - } - listItemHeightFactor += countBlocks( item, expandedState, [] ); - return true; - } ); - - // New scroll value is the number of expanded items - // multiplied by the item height - // plus the number of expanded children in the selected block - // multiplied by the item height. - const newScrollTopValue = - listItemHeightFactor * blockListItemHeight + - ( parentClientIds ? parentClientIds.length : 1 ) * - blockListItemHeight; - - const shouldScrollDown = - newScrollTopValue > - scrollContainer.scrollTop + scrollContainer.clientHeight; - - const shouldScrollUp = - newScrollTopValue < scrollContainer.scrollTop; - - if ( ! shouldScrollUp && ! shouldScrollDown ) { - return; - } - - // @TODO This doesn't yet work when nested blocks are selected. - // We're still using the top parent block to calculate/trigger redraw. - // If selected block is already visible in the list prevent scroll. - scrollContainer?.scrollTo( { - top: newScrollTopValue, - } ); - } - }, [ firstSelectedBlockClientId, parentExpandedState ] ); - return { setSelectedTreeId, }; From f0eb98248cf1e273658f12f908f611a7c43fa82a Mon Sep 17 00:00:00 2001 From: Ramon Date: Tue, 1 Feb 2022 16:01:12 +1100 Subject: [PATCH 10/15] Revert export --- packages/block-editor/src/components/list-view/branch.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-editor/src/components/list-view/branch.js b/packages/block-editor/src/components/list-view/branch.js index 9ed78740a8e76b..71518a1e289f3f 100644 --- a/packages/block-editor/src/components/list-view/branch.js +++ b/packages/block-editor/src/components/list-view/branch.js @@ -28,7 +28,7 @@ import { isClientIdSelected } from './utils'; * @param {Array} draggedClientIds a list of dragged client ids * @return {number} block count */ -export function countBlocks( block, expandedState, draggedClientIds ) { +function countBlocks( block, expandedState, draggedClientIds ) { const isDragged = draggedClientIds?.includes( block.clientId ); if ( isDragged ) { return 0; From 4049d45938fd983d027b0f65be79f63b584ac2c6 Mon Sep 17 00:00:00 2001 From: ramonjd Date: Wed, 2 Feb 2022 20:31:23 +1100 Subject: [PATCH 11/15] Adding e2e test --- .../specs/editor/various/list-view.test.js | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/packages/e2e-tests/specs/editor/various/list-view.test.js b/packages/e2e-tests/specs/editor/various/list-view.test.js index aef74c82a20921..21b512dd8c94f4 100644 --- a/packages/e2e-tests/specs/editor/various/list-view.test.js +++ b/packages/e2e-tests/specs/editor/various/list-view.test.js @@ -5,6 +5,7 @@ import { createNewPost, insertBlock, getEditedPostContent, + openListView, pressKeyWithModifier, pressKeyTimes, } from '@wordpress/e2e-test-utils'; @@ -20,6 +21,10 @@ async function dragAndDrop( draggableElement, targetElement, offsetY ) { return await page.mouse.dragAndDrop( draggablePoint, targetPoint ); } +async function getBlockListLeafNodes() { + return await page.$$( '.block-editor-list-view-leaf' ); +} + describe( 'List view', () => { beforeAll( async () => { await page.setDragInterception( true ); @@ -97,4 +102,46 @@ describe( 'List view', () => { // https://github.com/WordPress/gutenberg/issues/38763. expect( console ).not.toHaveErrored(); } ); + + it( 'should expand nested list items', async () => { + // Insert some blocks of different types. + await insertBlock( 'Cover' ); + + // Click first color option from the block placeholder's color picker to make the inner blocks appear. + const colorPickerButton = await page.waitForSelector( + '.wp-block-cover__placeholder-background-options .components-circular-option-picker__option-wrapper:first-child button' + ); + await colorPickerButton.click(); + + // Open list view. + await openListView(); + + // Things start off expanded. + expect( await getBlockListLeafNodes() ).toHaveLength( 2 ); + + const blockListExpanders = await page.$$( + '.block-editor-list-view__expander' + ); + + // Collapse the first block + await blockListExpanders[ 0 ].click(); + + // Check that we're collapsed + expect( await getBlockListLeafNodes() ).toHaveLength( 1 ); + + // Click the cover title placeholder. + const coverTitle = await page.waitForSelector( + '.wp-block-cover .wp-block-paragraph' + ); + + await coverTitle.click(); + + // The block list should contain two leafs and the second should be selected (and be a paragraph). + const selectedElementText = await page.$eval( + '.block-editor-list-view-leaf:nth-child(2).is-selected a', + ( element ) => element.innerText + ); + + expect( selectedElementText ).toContain( 'Paragraph' ); + } ); } ); From 874a4579ad9816847e0387b32453f9f4336b2342 Mon Sep 17 00:00:00 2001 From: ramonjd Date: Fri, 18 Feb 2022 16:02:04 +1100 Subject: [PATCH 12/15] Forgot to pass on the correct args to `updateBlockSelection()` --- packages/block-editor/src/components/list-view/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/block-editor/src/components/list-view/index.js b/packages/block-editor/src/components/list-view/index.js index 9edb0ea029f4c0..5f551b94789052 100644 --- a/packages/block-editor/src/components/list-view/index.js +++ b/packages/block-editor/src/components/list-view/index.js @@ -109,8 +109,8 @@ function ListView( setExpandedState, } ); const selectEditorBlock = useCallback( - ( clientId ) => { - updateBlockSelection( clientId ); + ( event, clientId ) => { + updateBlockSelection( event, clientId ); setSelectedTreeId( clientId ); }, [ setSelectedTreeId, updateBlockSelection ] From 33cf482639a15d559c8d1f654a868ddc9f6ade20 Mon Sep 17 00:00:00 2001 From: ramonjd Date: Mon, 21 Feb 2022 10:54:50 +1100 Subject: [PATCH 13/15] Focussing on the cover block before testing the expand element. --- packages/e2e-tests/specs/editor/various/list-view.test.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/e2e-tests/specs/editor/various/list-view.test.js b/packages/e2e-tests/specs/editor/various/list-view.test.js index 21b512dd8c94f4..9c5ac2327631fb 100644 --- a/packages/e2e-tests/specs/editor/various/list-view.test.js +++ b/packages/e2e-tests/specs/editor/various/list-view.test.js @@ -129,6 +129,11 @@ describe( 'List view', () => { // Check that we're collapsed expect( await getBlockListLeafNodes() ).toHaveLength( 1 ); + // Focus the cover block. The paragraph is not focussed by default. + const coverBlock = await page.waitForSelector( '.wp-block-cover' ); + + await coverBlock.focus(); + // Click the cover title placeholder. const coverTitle = await page.waitForSelector( '.wp-block-cover .wp-block-paragraph' @@ -138,7 +143,7 @@ describe( 'List view', () => { // The block list should contain two leafs and the second should be selected (and be a paragraph). const selectedElementText = await page.$eval( - '.block-editor-list-view-leaf:nth-child(2).is-selected a', + '.block-editor-list-view-leaf.is-selected .block-editor-list-view-block-contents', ( element ) => element.innerText ); From 30f65ee8630e6f34f1c6303f833c4ff1ed6f4fa2 Mon Sep 17 00:00:00 2001 From: ramonjd Date: Mon, 21 Feb 2022 13:27:51 +1100 Subject: [PATCH 14/15] Renaming function and file name to make it clearer to folks what it does. --- packages/block-editor/src/components/list-view/index.js | 4 ++-- ...selected-item.js => use-list-view-expand-selected-item.js} | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) rename packages/block-editor/src/components/list-view/{use-list-view-open-selected-item.js => use-list-view-expand-selected-item.js} (96%) diff --git a/packages/block-editor/src/components/list-view/index.js b/packages/block-editor/src/components/list-view/index.js index 5f551b94789052..6ea58ca5004438 100644 --- a/packages/block-editor/src/components/list-view/index.js +++ b/packages/block-editor/src/components/list-view/index.js @@ -26,7 +26,7 @@ import ListViewDropIndicator from './drop-indicator'; import useBlockSelection from './use-block-selection'; import useListViewClientIds from './use-list-view-client-ids'; import useListViewDropZone from './use-list-view-drop-zone'; -import useListViewOpenSelectedItem from './use-list-view-open-selected-item'; +import useListViewExpandSelectedItem from './use-list-view-expand-selected-item'; import { store as blockEditorStore } from '../../store'; const expanded = ( state, action ) => { @@ -104,7 +104,7 @@ function ListView( const treeGridRef = useMergeRefs( [ elementRef, dropZoneRef, ref ] ); const isMounted = useRef( false ); - const { setSelectedTreeId } = useListViewOpenSelectedItem( { + const { setSelectedTreeId } = useListViewExpandSelectedItem( { firstSelectedBlockClientId: selectedClientIds[ 0 ], setExpandedState, } ); diff --git a/packages/block-editor/src/components/list-view/use-list-view-open-selected-item.js b/packages/block-editor/src/components/list-view/use-list-view-expand-selected-item.js similarity index 96% rename from packages/block-editor/src/components/list-view/use-list-view-open-selected-item.js rename to packages/block-editor/src/components/list-view/use-list-view-expand-selected-item.js index 2dbc918b3d11c8..e26c4afed9e670 100644 --- a/packages/block-editor/src/components/list-view/use-list-view-open-selected-item.js +++ b/packages/block-editor/src/components/list-view/use-list-view-expand-selected-item.js @@ -9,7 +9,7 @@ import { useSelect } from '@wordpress/data'; */ import { store as blockEditorStore } from '../../store'; -export default function useListViewOpenSelectedItem( { +export default function useListViewExpandSelectedItem({ firstSelectedBlockClientId, setExpandedState, } ) { From d1a2731d73e06e17e674ac7380a2dda307f3cb46 Mon Sep 17 00:00:00 2001 From: ramonjd Date: Mon, 21 Feb 2022 14:03:45 +1100 Subject: [PATCH 15/15] Linto! --- .../components/list-view/use-list-view-expand-selected-item.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-editor/src/components/list-view/use-list-view-expand-selected-item.js b/packages/block-editor/src/components/list-view/use-list-view-expand-selected-item.js index e26c4afed9e670..09b5e09e4713a3 100644 --- a/packages/block-editor/src/components/list-view/use-list-view-expand-selected-item.js +++ b/packages/block-editor/src/components/list-view/use-list-view-expand-selected-item.js @@ -9,7 +9,7 @@ import { useSelect } from '@wordpress/data'; */ import { store as blockEditorStore } from '../../store'; -export default function useListViewExpandSelectedItem({ +export default function useListViewExpandSelectedItem( { firstSelectedBlockClientId, setExpandedState, } ) {