From 87491799b0ab895d608e87d4aa41ca241b42b828 Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Fri, 28 Jan 2022 16:07:45 +1100 Subject: [PATCH 01/24] List View: Add multi-select behaviour when shift key is selected --- .../src/components/list-view/block.js | 2 +- .../src/components/list-view/index.js | 11 +++-- .../secondary-sidebar/list-view-sidebar.js | 47 ++++++++++++++++-- .../secondary-sidebar/list-view-sidebar.js | 48 +++++++++++++++++-- 4 files changed, 92 insertions(+), 16 deletions(-) diff --git a/packages/block-editor/src/components/list-view/block.js b/packages/block-editor/src/components/list-view/block.js index af488138fe67b3..d6b2c7b4a245eb 100644 --- a/packages/block-editor/src/components/list-view/block.js +++ b/packages/block-editor/src/components/list-view/block.js @@ -105,7 +105,7 @@ function ListViewBlock( { const selectEditorBlock = useCallback( ( event ) => { event.stopPropagation(); - selectBlock( clientId ); + selectBlock( clientId, event ); }, [ clientId, selectBlock ] ); diff --git a/packages/block-editor/src/components/list-view/index.js b/packages/block-editor/src/components/list-view/index.js index dfe3fbcf94ee30..0a652f968ad3ed 100644 --- a/packages/block-editor/src/components/list-view/index.js +++ b/packages/block-editor/src/components/list-view/index.js @@ -27,7 +27,6 @@ import useListViewClientIds from './use-list-view-client-ids'; import useListViewDropZone from './use-list-view-drop-zone'; import { store as blockEditorStore } from '../../store'; -const noop = () => {}; const expanded = ( state, action ) => { switch ( action.type ) { case 'expand': @@ -57,7 +56,7 @@ const expanded = ( state, action ) => { function ListView( { blocks, - onSelect = noop, + onSelect, __experimentalFeatures, __experimentalPersistentListViewFeatures, __experimentalHideContainerBlockActions, @@ -89,9 +88,11 @@ function ListView( [ draggedClientIds ] ); const selectEditorBlock = useCallback( - ( clientId ) => { - selectBlock( clientId ); - onSelect( clientId ); + ( clientId, event ) => { + if ( ! onSelect ) { + selectBlock( clientId ); + } + onSelect( clientId, event ); }, [ selectBlock, onSelect ] ); diff --git a/packages/edit-post/src/components/secondary-sidebar/list-view-sidebar.js b/packages/edit-post/src/components/secondary-sidebar/list-view-sidebar.js index 9076101bc399bd..e84a5b56ad3ca4 100644 --- a/packages/edit-post/src/components/secondary-sidebar/list-view-sidebar.js +++ b/packages/edit-post/src/components/secondary-sidebar/list-view-sidebar.js @@ -12,7 +12,7 @@ import { useInstanceId, useMergeRefs, } from '@wordpress/compose'; -import { useDispatch } from '@wordpress/data'; +import { useDispatch, useSelect } from '@wordpress/data'; import { __ } from '@wordpress/i18n'; import { closeSmall } from '@wordpress/icons'; import { ESCAPE } from '@wordpress/keycodes'; @@ -25,10 +25,47 @@ import { store as editPostStore } from '../../store'; export default function ListViewSidebar() { const { setIsListViewOpened } = useDispatch( editPostStore ); - const { clearSelectedBlock, selectBlock } = useDispatch( blockEditorStore ); - async function selectEditorBlock( clientId ) { - await clearSelectedBlock(); - selectBlock( clientId, -1 ); + const { getBlockParents, getBlockSelectionStart } = useSelect( + blockEditorStore + ); + + const { clearSelectedBlock, multiSelect, selectBlock } = useDispatch( + blockEditorStore + ); + async function selectEditorBlock( clientId, event ) { + if ( ! event.shiftKey ) { + await clearSelectedBlock(); + selectBlock( clientId, -1 ); + } else if ( event.shiftKey ) { + event.preventDefault(); + + const blockSelectionStart = getBlockSelectionStart(); + + // By checking `blockSelectionStart` to be set, we handle the + // case where we select a single block. We also have to check + // the selectionEnd (clientId) not to be included in the + // `blockSelectionStart`'s parents because the click event is + // propagated. + const startParents = getBlockParents( blockSelectionStart ); + + if ( + blockSelectionStart && + blockSelectionStart !== clientId && + ! startParents?.includes( clientId ) + ) { + const startPath = [ ...startParents, blockSelectionStart ]; + const endPath = [ ...getBlockParents( clientId ), clientId ]; + const depth = Math.min( startPath.length, endPath.length ) - 1; + const start = startPath[ depth ]; + const end = endPath[ depth ]; + + // Handle the case of having selected a parent block and + // then shift+click on a child. + if ( start !== end ) { + multiSelect( start, end ); + } + } + } } const focusOnMountRef = useFocusOnMount( 'firstElement' ); diff --git a/packages/edit-site/src/components/secondary-sidebar/list-view-sidebar.js b/packages/edit-site/src/components/secondary-sidebar/list-view-sidebar.js index 41a28c1033f477..4c151e9ae1c0fa 100644 --- a/packages/edit-site/src/components/secondary-sidebar/list-view-sidebar.js +++ b/packages/edit-site/src/components/secondary-sidebar/list-view-sidebar.js @@ -12,7 +12,7 @@ import { useInstanceId, useMergeRefs, } from '@wordpress/compose'; -import { useDispatch } from '@wordpress/data'; +import { useDispatch, useSelect } from '@wordpress/data'; import { __ } from '@wordpress/i18n'; import { closeSmall } from '@wordpress/icons'; import { ESCAPE } from '@wordpress/keycodes'; @@ -25,10 +25,48 @@ import { store as editSiteStore } from '../../store'; export default function ListViewSidebar() { const { setIsListViewOpened } = useDispatch( editSiteStore ); - const { clearSelectedBlock, selectBlock } = useDispatch( blockEditorStore ); - async function selectEditorBlock( clientId ) { - await clearSelectedBlock(); - selectBlock( clientId, -1 ); + const { getBlockParents, getBlockSelectionStart } = useSelect( + blockEditorStore + ); + + const { clearSelectedBlock, multiSelect, selectBlock } = useDispatch( + blockEditorStore + ); + + async function selectEditorBlock( clientId, event ) { + if ( ! event.shiftKey ) { + await clearSelectedBlock(); + selectBlock( clientId, -1 ); + } else if ( event.shiftKey ) { + event.preventDefault(); + + const blockSelectionStart = getBlockSelectionStart(); + + // By checking `blockSelectionStart` to be set, we handle the + // case where we select a single block. We also have to check + // the selectionEnd (clientId) not to be included in the + // `blockSelectionStart`'s parents because the click event is + // propagated. + const startParents = getBlockParents( blockSelectionStart ); + + if ( + blockSelectionStart && + blockSelectionStart !== clientId && + ! startParents?.includes( clientId ) + ) { + const startPath = [ ...startParents, blockSelectionStart ]; + const endPath = [ ...getBlockParents( clientId ), clientId ]; + const depth = Math.min( startPath.length, endPath.length ) - 1; + const start = startPath[ depth ]; + const end = endPath[ depth ]; + + // Handle the case of having selected a parent block and + // then shift+click on a child. + if ( start !== end ) { + multiSelect( start, end ); + } + } + } } const focusOnMountRef = useFocusOnMount( 'firstElement' ); From 4e758172bbc604cebc14de7dfad741ce0e3443e5 Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Mon, 31 Jan 2022 15:24:10 +1100 Subject: [PATCH 02/24] Ensure shift clicking a block when no blocks are selected selects that first block --- .../secondary-sidebar/list-view-sidebar.js | 15 ++++++++++++--- .../secondary-sidebar/list-view-sidebar.js | 14 +++++++++++--- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/packages/edit-post/src/components/secondary-sidebar/list-view-sidebar.js b/packages/edit-post/src/components/secondary-sidebar/list-view-sidebar.js index e84a5b56ad3ca4..1d2c7dedbe365a 100644 --- a/packages/edit-post/src/components/secondary-sidebar/list-view-sidebar.js +++ b/packages/edit-post/src/components/secondary-sidebar/list-view-sidebar.js @@ -25,13 +25,17 @@ import { store as editPostStore } from '../../store'; export default function ListViewSidebar() { const { setIsListViewOpened } = useDispatch( editPostStore ); - const { getBlockParents, getBlockSelectionStart } = useSelect( - blockEditorStore - ); + const { + getBlockParents, + getBlockSelectionStart, + hasMultiSelection, + hasSelectedBlock, + } = useSelect( blockEditorStore ); const { clearSelectedBlock, multiSelect, selectBlock } = useDispatch( blockEditorStore ); + async function selectEditorBlock( clientId, event ) { if ( ! event.shiftKey ) { await clearSelectedBlock(); @@ -39,6 +43,11 @@ export default function ListViewSidebar() { } else if ( event.shiftKey ) { event.preventDefault(); + if ( ! hasSelectedBlock() && ! hasMultiSelection() ) { + selectBlock( clientId, -1 ); + return; + } + const blockSelectionStart = getBlockSelectionStart(); // By checking `blockSelectionStart` to be set, we handle the diff --git a/packages/edit-site/src/components/secondary-sidebar/list-view-sidebar.js b/packages/edit-site/src/components/secondary-sidebar/list-view-sidebar.js index 4c151e9ae1c0fa..577c9ee4ea1cd6 100644 --- a/packages/edit-site/src/components/secondary-sidebar/list-view-sidebar.js +++ b/packages/edit-site/src/components/secondary-sidebar/list-view-sidebar.js @@ -25,9 +25,12 @@ import { store as editSiteStore } from '../../store'; export default function ListViewSidebar() { const { setIsListViewOpened } = useDispatch( editSiteStore ); - const { getBlockParents, getBlockSelectionStart } = useSelect( - blockEditorStore - ); + const { + getBlockParents, + getBlockSelectionStart, + hasMultiSelection, + hasSelectedBlock, + } = useSelect( blockEditorStore ); const { clearSelectedBlock, multiSelect, selectBlock } = useDispatch( blockEditorStore @@ -40,6 +43,11 @@ export default function ListViewSidebar() { } else if ( event.shiftKey ) { event.preventDefault(); + if ( ! hasSelectedBlock() && ! hasMultiSelection() ) { + selectBlock( clientId, -1 ); + return; + } + const blockSelectionStart = getBlockSelectionStart(); // By checking `blockSelectionStart` to be set, we handle the From cfba501b42ad765b660094a67a979e3becc598e2 Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Mon, 31 Jan 2022 17:19:35 +1100 Subject: [PATCH 03/24] Add support for dragging multiple selected blocks --- .../src/components/list-view/block-contents.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/block-editor/src/components/list-view/block-contents.js b/packages/block-editor/src/components/list-view/block-contents.js index 071cd11cd70b29..94843313d96a00 100644 --- a/packages/block-editor/src/components/list-view/block-contents.js +++ b/packages/block-editor/src/components/list-view/block-contents.js @@ -33,17 +33,21 @@ const ListViewBlockContents = forwardRef( ) => { const { clientId } = block; - const { blockMovingClientId, selectedBlockInBlockEditor } = useSelect( + const { + blockMovingClientId, + selectedBlockInBlockEditor, + selectedBlocks, + } = useSelect( ( select ) => { const { - getBlockRootClientId, hasBlockMovingClientId, getSelectedBlockClientId, + getSelectedBlockClientIds, } = select( blockEditorStore ); return { - rootClientId: getBlockRootClientId( clientId ) || '', blockMovingClientId: hasBlockMovingClientId(), selectedBlockInBlockEditor: getSelectedBlockClientId(), + selectedBlocks: getSelectedBlockClientIds(), }; }, [ clientId ] @@ -57,7 +61,7 @@ const ListViewBlockContents = forwardRef( } ); return ( - + { ( { draggable, onDragStart, onDragEnd } ) => ( Date: Tue, 1 Feb 2022 16:23:13 +1100 Subject: [PATCH 04/24] Remove duplication by moving multi-select behaviour to the ListView component, ensure it also works in the widget editor --- .../src/components/list-view/index.js | 72 +++++++++++++++++-- .../secondary-sidebar/list-view-sidebar.js | 60 +--------------- .../secondary-sidebar/list-view-sidebar.js | 60 +--------------- .../secondary-sidebar/list-view-sidebar.js | 12 +--- 4 files changed, 70 insertions(+), 134 deletions(-) diff --git a/packages/block-editor/src/components/list-view/index.js b/packages/block-editor/src/components/list-view/index.js index 0a652f968ad3ed..a701449fa40dda 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 useListViewClientIds from './use-list-view-client-ids'; import useListViewDropZone from './use-list-view-drop-zone'; import { store as blockEditorStore } from '../../store'; +const noop = () => {}; const expanded = ( state, action ) => { switch ( action.type ) { case 'expand': @@ -56,7 +57,7 @@ const expanded = ( state, action ) => { function ListView( { blocks, - onSelect, + onSelect = noop, __experimentalFeatures, __experimentalPersistentListViewFeatures, __experimentalHideContainerBlockActions, @@ -71,7 +72,16 @@ function ListView( draggedClientIds, selectedClientIds, } = useListViewClientIds( blocks ); - const { selectBlock } = useDispatch( blockEditorStore ); + const { clearSelectedBlock, multiSelect, selectBlock } = useDispatch( + blockEditorStore + ); + const { + getBlockParents, + getBlockSelectionStart, + hasMultiSelection, + hasSelectedBlock, + } = useSelect( blockEditorStore ); + const { visibleBlockCount } = useSelect( ( select ) => { const { getGlobalBlockCount, getClientIdsOfDescendants } = select( @@ -88,13 +98,61 @@ function ListView( [ draggedClientIds ] ); const selectEditorBlock = useCallback( - ( clientId, event ) => { - if ( ! onSelect ) { - selectBlock( clientId ); + async ( clientId, event ) => { + if ( ! event.shiftKey ) { + await clearSelectedBlock(); + selectBlock( clientId, -1 ); + } else if ( event.shiftKey ) { + event.preventDefault(); + + if ( ! hasSelectedBlock() && ! hasMultiSelection() ) { + selectBlock( clientId, -1 ); + return; + } + + const blockSelectionStart = getBlockSelectionStart(); + + // By checking `blockSelectionStart` to be set, we handle the + // case where we select a single block. We also have to check + // the selectionEnd (clientId) not to be included in the + // `blockSelectionStart`'s parents because the click event is + // propagated. + const startParents = getBlockParents( blockSelectionStart ); + + if ( + blockSelectionStart && + blockSelectionStart !== clientId && + ! startParents?.includes( clientId ) + ) { + const startPath = [ ...startParents, blockSelectionStart ]; + const endPath = [ + ...getBlockParents( clientId ), + clientId, + ]; + const depth = + Math.min( startPath.length, endPath.length ) - 1; + const start = startPath[ depth ]; + const end = endPath[ depth ]; + + // Handle the case of having selected a parent block and + // then shift+click on a child. + if ( start !== end ) { + multiSelect( start, end ); + } + } } - onSelect( clientId, event ); + onSelect( clientId ); }, - [ selectBlock, onSelect ] + [ + clearSelectedBlock, + getBlockParents, + getBlockSelectionStart, + hasMultiSelection, + hasSelectedBlock, + multiSelect, + selectBlock, + onSelect, + ] ); const [ expandedState, setExpandedState ] = useReducer( expanded, {} ); diff --git a/packages/edit-post/src/components/secondary-sidebar/list-view-sidebar.js b/packages/edit-post/src/components/secondary-sidebar/list-view-sidebar.js index 1d2c7dedbe365a..cc09ac47ae7ae3 100644 --- a/packages/edit-post/src/components/secondary-sidebar/list-view-sidebar.js +++ b/packages/edit-post/src/components/secondary-sidebar/list-view-sidebar.js @@ -1,10 +1,7 @@ /** * WordPress dependencies */ -import { - __experimentalListView as ListView, - store as blockEditorStore, -} from '@wordpress/block-editor'; +import { __experimentalListView as ListView } from '@wordpress/block-editor'; import { Button } from '@wordpress/components'; import { useFocusOnMount, @@ -12,7 +9,7 @@ import { useInstanceId, useMergeRefs, } from '@wordpress/compose'; -import { useDispatch, useSelect } from '@wordpress/data'; +import { useDispatch } from '@wordpress/data'; import { __ } from '@wordpress/i18n'; import { closeSmall } from '@wordpress/icons'; import { ESCAPE } from '@wordpress/keycodes'; @@ -25,58 +22,6 @@ import { store as editPostStore } from '../../store'; export default function ListViewSidebar() { const { setIsListViewOpened } = useDispatch( editPostStore ); - const { - getBlockParents, - getBlockSelectionStart, - hasMultiSelection, - hasSelectedBlock, - } = useSelect( blockEditorStore ); - - const { clearSelectedBlock, multiSelect, selectBlock } = useDispatch( - blockEditorStore - ); - - async function selectEditorBlock( clientId, event ) { - if ( ! event.shiftKey ) { - await clearSelectedBlock(); - selectBlock( clientId, -1 ); - } else if ( event.shiftKey ) { - event.preventDefault(); - - if ( ! hasSelectedBlock() && ! hasMultiSelection() ) { - selectBlock( clientId, -1 ); - return; - } - - const blockSelectionStart = getBlockSelectionStart(); - - // By checking `blockSelectionStart` to be set, we handle the - // case where we select a single block. We also have to check - // the selectionEnd (clientId) not to be included in the - // `blockSelectionStart`'s parents because the click event is - // propagated. - const startParents = getBlockParents( blockSelectionStart ); - - if ( - blockSelectionStart && - blockSelectionStart !== clientId && - ! startParents?.includes( clientId ) - ) { - const startPath = [ ...startParents, blockSelectionStart ]; - const endPath = [ ...getBlockParents( clientId ), clientId ]; - const depth = Math.min( startPath.length, endPath.length ) - 1; - const start = startPath[ depth ]; - const end = endPath[ depth ]; - - // Handle the case of having selected a parent block and - // then shift+click on a child. - if ( start !== end ) { - multiSelect( start, end ); - } - } - } - } - const focusOnMountRef = useFocusOnMount( 'firstElement' ); const headerFocusReturnRef = useFocusReturn(); const contentFocusReturnRef = useFocusReturn(); @@ -116,7 +61,6 @@ export default function ListViewSidebar() { ] ) } > Date: Tue, 1 Feb 2022 16:37:43 +1100 Subject: [PATCH 05/24] Add inline comments --- packages/block-editor/src/components/list-view/index.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/block-editor/src/components/list-view/index.js b/packages/block-editor/src/components/list-view/index.js index a701449fa40dda..aad19c5102265d 100644 --- a/packages/block-editor/src/components/list-view/index.js +++ b/packages/block-editor/src/components/list-view/index.js @@ -103,8 +103,11 @@ function ListView( await clearSelectedBlock(); selectBlock( clientId, -1 ); } else if ( event.shiftKey ) { + // To handle multiple block selection via the `SHIFT` key, prevent + // the browser default behavior of opening the link in a new window. event.preventDefault(); + // Select a single block if no block is selected yet. if ( ! hasSelectedBlock() && ! hasMultiSelection() ) { selectBlock( clientId, -1 ); return; From b6bb2dbffd1324a422f62446e8248be6b1a7a984 Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Tue, 1 Feb 2022 16:43:07 +1100 Subject: [PATCH 06/24] Update documentation, add changelog entry --- packages/block-editor/CHANGELOG.md | 5 +++++ packages/block-editor/src/components/list-view/README.md | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/block-editor/CHANGELOG.md b/packages/block-editor/CHANGELOG.md index af57ce5bc4bfbf..3da8a4fb7eda6f 100644 --- a/packages/block-editor/CHANGELOG.md +++ b/packages/block-editor/CHANGELOG.md @@ -12,6 +12,11 @@ - Removed unused `@wordpress/block-serialization-default-parser`, `css-mediaquery`, `memize` and `redux-multi` dependencies ([#38388](https://github.com/WordPress/gutenberg/pull/38388)). +### New Features + +- List View now supports selecting and dragging multiple blocks via `SHIFT` clicking items in the list [#38314](https://github.com/WordPress/gutenberg/pull/38314). +>>>>>>> e0937d166f (Update documentation, add changelog entry) + ## 8.1.0 (2022-01-27) ## 8.0.0 (2021-11-07) diff --git a/packages/block-editor/src/components/list-view/README.md b/packages/block-editor/src/components/list-view/README.md index f702c28702f8e1..9552efa5a36f82 100644 --- a/packages/block-editor/src/components/list-view/README.md +++ b/packages/block-editor/src/components/list-view/README.md @@ -4,7 +4,7 @@ The ListView component provides an overview of the hierarchical structure of all Blocks that have child blocks (such as group or column blocks) are presented with the parent at the top and the nested children below. -In addition to presenting the structure of the blocks in the editor, the ListView component lets users navigate to each block by clicking on its line in the hierarchy tree. +In addition to presenting the structure of the blocks in the editor, the ListView component lets users navigate to each block by clicking on its line in the hierarchy tree. Multiple blocks at the same level of nesting can be selected by holding down the `SHIFT` key and clicking blocks within the list. ![List view](https://make.wordpress.org/core/files/2020/08/block-navigation.png) ![View of a group list view](https://make.wordpress.org/core/files/2020/08/view-of-group-block-navigation.png) From 06c731c7cce108d8357d4831bbf8fe683d2dc233 Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Wed, 2 Feb 2022 16:15:23 +1100 Subject: [PATCH 07/24] Add e2e test for multi-select in the list view --- .../various/multi-block-selection.test.js | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js b/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js index c87700e23d3e1f..85070bfe840af7 100644 --- a/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js +++ b/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js @@ -10,6 +10,7 @@ import { clickBlockToolbarButton, clickButton, clickMenuItem, + openListView, saveDraft, transformBlockTo, } from '@wordpress/e2e-test-utils'; @@ -747,4 +748,42 @@ describe( 'Multi-block selection', () => { } ); expect( selectedText ).toEqual( 'Post title' ); } ); + + it( 'should multi-select in the ListView component with shift + click', async () => { + // Create four blocks. + await clickBlockAppender(); + await page.keyboard.type( '1' ); + await page.keyboard.press( 'Enter' ); + await page.keyboard.type( '2' ); + await page.keyboard.press( 'Enter' ); + await page.keyboard.type( '3' ); + await page.keyboard.press( 'Enter' ); + await page.keyboard.type( '4' ); + + // Open up the list view, and get a reference to each of the list items. + await openListView(); + const navButtons = await page.$$( + '.block-editor-list-view-block-select-button' + ); + + // Clicking on the second list item should result in the second block being selected. + await navButtons[ 1 ].click(); + expect( await getSelectedFlatIndices() ).toEqual( 2 ); + + // Shift clicking the fourth list item should result in blocks 2 through 4 being selected. + await page.keyboard.down( 'Shift' ); + await navButtons[ 3 ].click(); + expect( await getSelectedFlatIndices() ).toEqual( [ 2, 3, 4 ] ); + + // With the shift key still held down, clicking the first block should result in + // the first two blocks being selected. + await navButtons[ 0 ].click(); + expect( await getSelectedFlatIndices() ).toEqual( [ 1, 2 ] ); + + // With the shift key up, clicking the fourth block should result in only that block + // being selected. + await page.keyboard.up( 'Shift' ); + await navButtons[ 3 ].click(); + expect( await getSelectedFlatIndices() ).toEqual( 4 ); + } ); } ); From edcff8b4034c4697afddff32f2eafc13be52b9f5 Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Wed, 2 Feb 2022 17:30:31 +1100 Subject: [PATCH 08/24] Remove stray line from changelog Co-authored-by: Robert Anderson --- packages/block-editor/CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/block-editor/CHANGELOG.md b/packages/block-editor/CHANGELOG.md index 3da8a4fb7eda6f..57153e73a29278 100644 --- a/packages/block-editor/CHANGELOG.md +++ b/packages/block-editor/CHANGELOG.md @@ -15,7 +15,6 @@ ### New Features - List View now supports selecting and dragging multiple blocks via `SHIFT` clicking items in the list [#38314](https://github.com/WordPress/gutenberg/pull/38314). ->>>>>>> e0937d166f (Update documentation, add changelog entry) ## 8.1.0 (2022-01-27) From 1362dba5f14d588d01f5230d0e9769d9ba57d2fb Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Thu, 3 Feb 2022 16:19:28 +1100 Subject: [PATCH 09/24] Ensure that clicked on blocks that aren't a part of a selection can still be repositioned --- .../src/components/list-view/block-contents.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/block-editor/src/components/list-view/block-contents.js b/packages/block-editor/src/components/list-view/block-contents.js index 94843313d96a00..8a1c3555d000b0 100644 --- a/packages/block-editor/src/components/list-view/block-contents.js +++ b/packages/block-editor/src/components/list-view/block-contents.js @@ -60,8 +60,16 @@ const ListViewBlockContents = forwardRef( 'is-dropping-before': isBlockMoveTarget, } ); + // Only include all selected blocks if the currently clicked on block + // is one of the selected blocks. This ensures that if a user attempts + // to drag a block that isn't part of the selection, they're still able + // to drag it and rearrange its position. + const draggableClientIds = selectedBlocks.includes( clientId ) + ? selectedBlocks + : [ clientId ]; + return ( - + { ( { draggable, onDragStart, onDragEnd } ) => ( Date: Thu, 3 Feb 2022 17:36:23 +1100 Subject: [PATCH 10/24] Try a naive approach to keyboard handling for shift + up/down to select multiple blocks --- .../src/components/list-view/block-select-button.js | 9 +++++++-- packages/block-editor/src/components/list-view/index.js | 6 +++++- packages/components/src/tree-grid/index.js | 7 ++++--- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/packages/block-editor/src/components/list-view/block-select-button.js b/packages/block-editor/src/components/list-view/block-select-button.js index 7a7f3b99a1a6d9..b7644c1b33d4d8 100644 --- a/packages/block-editor/src/components/list-view/block-select-button.js +++ b/packages/block-editor/src/components/list-view/block-select-button.js @@ -19,7 +19,7 @@ import useBlockDisplayInformation from '../use-block-display-information'; import { getBlockPositionDescription } from './utils'; import BlockTitle from '../block-title'; import ListViewExpander from './expander'; -import { SPACE, ENTER } from '@wordpress/keycodes'; +import { UP, DOWN, SPACE, ENTER } from '@wordpress/keycodes'; function ListViewBlockSelectButton( { @@ -59,7 +59,12 @@ function ListViewBlockSelectButton( }; function onKeyDownHandler( event ) { - if ( event.keyCode === ENTER || event.keyCode === SPACE ) { + if ( + event.keyCode === ENTER || + event.keyCode === SPACE || + ( event.shiftKey && + ( event.keyCode === UP || event.keyCode === DOWN ) ) + ) { event.preventDefault(); onClick( event ); } diff --git a/packages/block-editor/src/components/list-view/index.js b/packages/block-editor/src/components/list-view/index.js index aad19c5102265d..f3383a7ed93c9d 100644 --- a/packages/block-editor/src/components/list-view/index.js +++ b/packages/block-editor/src/components/list-view/index.js @@ -99,7 +99,11 @@ function ListView( ); const selectEditorBlock = useCallback( async ( clientId, event ) => { - if ( ! event.shiftKey ) { + if ( ! event.shiftKey || event.type === 'keydown' ) { + // When this function is called from a 'keydown' event, select + // the block as if it were individually clicked. This allows + // the editor canvas to be responsible for the shift + up/down + // multiple block selection behaviour. await clearSelectedBlock(); selectBlock( clientId, -1 ); } else if ( event.shiftKey ) { diff --git a/packages/components/src/tree-grid/index.js b/packages/components/src/tree-grid/index.js index 34bdeabfebc682..f8cfed5c91eaaa 100644 --- a/packages/components/src/tree-grid/index.js +++ b/packages/components/src/tree-grid/index.js @@ -53,10 +53,11 @@ function TreeGrid( ) { const onKeyDown = useCallback( ( event ) => { - const { keyCode, metaKey, ctrlKey, altKey, shiftKey } = event; + const { keyCode, metaKey, ctrlKey, altKey } = event; - const hasModifierKeyPressed = - metaKey || ctrlKey || altKey || shiftKey; + // The shift key is intentionally absent from the following list, + // to enable shift + up/down to select items from the list. + const hasModifierKeyPressed = metaKey || ctrlKey || altKey; if ( hasModifierKeyPressed || From 62f32e442a0ef6ef54b10459c08e030089ee4595 Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Mon, 7 Feb 2022 17:33:59 +1100 Subject: [PATCH 11/24] Move block selection to its own useBlockSelection hook --- .../src/components/list-view/index.js | 79 +--------------- .../list-view/use-block-selection.js | 89 +++++++++++++++++++ 2 files changed, 93 insertions(+), 75 deletions(-) create mode 100644 packages/block-editor/src/components/list-view/use-block-selection.js diff --git a/packages/block-editor/src/components/list-view/index.js b/packages/block-editor/src/components/list-view/index.js index f3383a7ed93c9d..8d88a2547a2def 100644 --- a/packages/block-editor/src/components/list-view/index.js +++ b/packages/block-editor/src/components/list-view/index.js @@ -6,7 +6,7 @@ import { __experimentalUseFixedWindowList as useFixedWindowList, } from '@wordpress/compose'; import { __experimentalTreeGrid as TreeGrid } from '@wordpress/components'; -import { AsyncModeProvider, useDispatch, useSelect } from '@wordpress/data'; +import { AsyncModeProvider, useSelect } from '@wordpress/data'; import { useCallback, useEffect, @@ -23,11 +23,11 @@ import { __ } from '@wordpress/i18n'; 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 { store as blockEditorStore } from '../../store'; -const noop = () => {}; const expanded = ( state, action ) => { switch ( action.type ) { case 'expand': @@ -57,7 +57,6 @@ const expanded = ( state, action ) => { function ListView( { blocks, - onSelect = noop, __experimentalFeatures, __experimentalPersistentListViewFeatures, __experimentalHideContainerBlockActions, @@ -72,15 +71,6 @@ function ListView( draggedClientIds, selectedClientIds, } = useListViewClientIds( blocks ); - const { clearSelectedBlock, multiSelect, selectBlock } = useDispatch( - blockEditorStore - ); - const { - getBlockParents, - getBlockSelectionStart, - hasMultiSelection, - hasSelectedBlock, - } = useSelect( blockEditorStore ); const { visibleBlockCount } = useSelect( ( select ) => { @@ -97,70 +87,9 @@ function ListView( }, [ draggedClientIds ] ); - const selectEditorBlock = useCallback( - async ( clientId, event ) => { - if ( ! event.shiftKey || event.type === 'keydown' ) { - // When this function is called from a 'keydown' event, select - // the block as if it were individually clicked. This allows - // the editor canvas to be responsible for the shift + up/down - // multiple block selection behaviour. - await clearSelectedBlock(); - selectBlock( clientId, -1 ); - } else if ( event.shiftKey ) { - // To handle multiple block selection via the `SHIFT` key, prevent - // the browser default behavior of opening the link in a new window. - event.preventDefault(); - - // Select a single block if no block is selected yet. - if ( ! hasSelectedBlock() && ! hasMultiSelection() ) { - selectBlock( clientId, -1 ); - return; - } - - const blockSelectionStart = getBlockSelectionStart(); - - // By checking `blockSelectionStart` to be set, we handle the - // case where we select a single block. We also have to check - // the selectionEnd (clientId) not to be included in the - // `blockSelectionStart`'s parents because the click event is - // propagated. - const startParents = getBlockParents( blockSelectionStart ); - if ( - blockSelectionStart && - blockSelectionStart !== clientId && - ! startParents?.includes( clientId ) - ) { - const startPath = [ ...startParents, blockSelectionStart ]; - const endPath = [ - ...getBlockParents( clientId ), - clientId, - ]; - const depth = - Math.min( startPath.length, endPath.length ) - 1; - const start = startPath[ depth ]; - const end = endPath[ depth ]; + const { updateBlockSelection } = useBlockSelection(); - // Handle the case of having selected a parent block and - // then shift+click on a child. - if ( start !== end ) { - multiSelect( start, end ); - } - } - } - onSelect( clientId ); - }, - [ - clearSelectedBlock, - getBlockParents, - getBlockSelectionStart, - hasMultiSelection, - hasSelectedBlock, - multiSelect, - selectBlock, - onSelect, - ] - ); const [ expandedState, setExpandedState ] = useReducer( expanded, {} ); const { ref: dropZoneRef, target: blockDropTarget } = useListViewDropZone(); @@ -255,7 +184,7 @@ function ListView( { + if ( ! event.shiftKey || event.type === 'keydown' ) { + // When this function is called from a 'keydown' event, select + // the block as if it were individually clicked. This allows + // the editor canvas to be responsible for the shift + up/down + // multiple block selection behaviour. + await clearSelectedBlock(); + selectBlock( clientId, -1 ); + } else if ( event.shiftKey ) { + // To handle multiple block selection via the `SHIFT` key, prevent + // the browser default behavior of opening the link in a new window. + event.preventDefault(); + + // Select a single block if no block is selected yet. + if ( ! hasSelectedBlock() && ! hasMultiSelection() ) { + selectBlock( clientId, -1 ); + return; + } + + const blockSelectionStart = getBlockSelectionStart(); + + // By checking `blockSelectionStart` to be set, we handle the + // case where we select a single block. We also have to check + // the selectionEnd (clientId) not to be included in the + // `blockSelectionStart`'s parents because the click event is + // propagated. + const startParents = getBlockParents( blockSelectionStart ); + + if ( + blockSelectionStart && + blockSelectionStart !== clientId && + ! startParents?.includes( clientId ) + ) { + const startPath = [ ...startParents, blockSelectionStart ]; + const endPath = [ + ...getBlockParents( clientId ), + clientId, + ]; + const depth = + Math.min( startPath.length, endPath.length ) - 1; + const start = startPath[ depth ]; + const end = endPath[ depth ]; + + // Handle the case of having selected a parent block and + // then shift+click on a child. + if ( start !== end ) { + multiSelect( start, end ); + } + } + } + }, + [ + clearSelectedBlock, + getBlockParents, + getBlockSelectionStart, + hasMultiSelection, + hasSelectedBlock, + multiSelect, + selectBlock, + ] + ); + + return { + updateBlockSelection, + }; +} From 8fbb905dbf4b1ec723eddc6c24e0ef17200df234 Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Mon, 7 Feb 2022 18:03:22 +1100 Subject: [PATCH 12/24] Refactor start and end id calculation to its own function, add unit tests --- .../list-view/test/use-block-selection.js | 50 +++++++++++++++++++ .../list-view/use-block-selection.js | 33 +++++++++--- 2 files changed, 75 insertions(+), 8 deletions(-) create mode 100644 packages/block-editor/src/components/list-view/test/use-block-selection.js diff --git a/packages/block-editor/src/components/list-view/test/use-block-selection.js b/packages/block-editor/src/components/list-view/test/use-block-selection.js new file mode 100644 index 00000000000000..b2cc4d23ef6f47 --- /dev/null +++ b/packages/block-editor/src/components/list-view/test/use-block-selection.js @@ -0,0 +1,50 @@ +/** + * Internal dependencies + */ +import { getStartAndEndIds } from '../use-block-selection'; + +describe( 'getStartAndEndIds', () => { + it( 'should return start and end when no depth is provided', () => { + const result = getStartAndEndIds( { + blockSelectionStart: 'start-id', + clientId: 'clicked-id', + endParents: [], + startParents: [], + } ); + + expect( result ).toEqual( { start: 'start-id', end: 'clicked-id' } ); + } ); + + it( 'should return deepest start and end when depths match', () => { + const result = getStartAndEndIds( { + blockSelectionStart: 'start-id', + clientId: 'clicked-id', + endParents: [ 'end-1', 'end-2', 'end-3' ], + startParents: [ 'start-1', 'start-2', 'start-3' ], + } ); + + expect( result ).toEqual( { start: 'start-id', end: 'clicked-id' } ); + } ); + + it( 'should return shallower ids when start is shallower', () => { + const result = getStartAndEndIds( { + blockSelectionStart: 'start-id', + clientId: 'clicked-id', + endParents: [ 'end-1', 'end-2', 'end-3' ], + startParents: [ 'start-1' ], + } ); + + expect( result ).toEqual( { start: 'start-id', end: 'end-2' } ); + } ); + + it( 'should return shallower ids when end is shallower', () => { + const result = getStartAndEndIds( { + blockSelectionStart: 'start-id', + clientId: 'clicked-id', + endParents: [ 'end-1', 'end-2' ], + startParents: [ 'start-1', 'start-2', 'start-3' ], + } ); + + expect( result ).toEqual( { start: 'start-3', end: 'clicked-id' } ); + } ); +} ); diff --git a/packages/block-editor/src/components/list-view/use-block-selection.js b/packages/block-editor/src/components/list-view/use-block-selection.js index 9051ae2645bf20..882f0c9bc581a6 100644 --- a/packages/block-editor/src/components/list-view/use-block-selection.js +++ b/packages/block-editor/src/components/list-view/use-block-selection.js @@ -9,6 +9,24 @@ import { useCallback } from '@wordpress/element'; */ import { store as blockEditorStore } from '../../store'; +export function getStartAndEndIds( { + blockSelectionStart, + clientId, + endParents, + startParents, +} ) { + const startPath = [ ...startParents, blockSelectionStart ]; + const endPath = [ ...endParents, clientId ]; + const depth = Math.min( startPath.length, endPath.length ) - 1; + const start = startPath[ depth ]; + const end = endPath[ depth ]; + + return { + start, + end, + }; +} + export default function useBlockSelection() { const { clearSelectedBlock, multiSelect, selectBlock } = useDispatch( blockEditorStore @@ -54,15 +72,14 @@ export default function useBlockSelection() { blockSelectionStart !== clientId && ! startParents?.includes( clientId ) ) { - const startPath = [ ...startParents, blockSelectionStart ]; - const endPath = [ - ...getBlockParents( clientId ), + const endParents = getBlockParents( clientId ); + + const { start, end } = getStartAndEndIds( { + blockSelectionStart, clientId, - ]; - const depth = - Math.min( startPath.length, endPath.length ) - 1; - const start = startPath[ depth ]; - const end = endPath[ depth ]; + endParents, + startParents, + } ); // Handle the case of having selected a parent block and // then shift+click on a child. From 999568ef80a4e3da27661f0a4863c75655764199 Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Tue, 8 Feb 2022 17:43:26 +1100 Subject: [PATCH 13/24] Move utility function to utils file, add doc comment, update usage --- .../list-view/test/use-block-selection.js | 50 ------------------- .../src/components/list-view/test/utils.js | 50 +++++++++++++++++++ .../list-view/use-block-selection.js | 25 ++-------- .../src/components/list-view/utils.js | 31 ++++++++++++ 4 files changed, 85 insertions(+), 71 deletions(-) delete mode 100644 packages/block-editor/src/components/list-view/test/use-block-selection.js create mode 100644 packages/block-editor/src/components/list-view/test/utils.js diff --git a/packages/block-editor/src/components/list-view/test/use-block-selection.js b/packages/block-editor/src/components/list-view/test/use-block-selection.js deleted file mode 100644 index b2cc4d23ef6f47..00000000000000 --- a/packages/block-editor/src/components/list-view/test/use-block-selection.js +++ /dev/null @@ -1,50 +0,0 @@ -/** - * Internal dependencies - */ -import { getStartAndEndIds } from '../use-block-selection'; - -describe( 'getStartAndEndIds', () => { - it( 'should return start and end when no depth is provided', () => { - const result = getStartAndEndIds( { - blockSelectionStart: 'start-id', - clientId: 'clicked-id', - endParents: [], - startParents: [], - } ); - - expect( result ).toEqual( { start: 'start-id', end: 'clicked-id' } ); - } ); - - it( 'should return deepest start and end when depths match', () => { - const result = getStartAndEndIds( { - blockSelectionStart: 'start-id', - clientId: 'clicked-id', - endParents: [ 'end-1', 'end-2', 'end-3' ], - startParents: [ 'start-1', 'start-2', 'start-3' ], - } ); - - expect( result ).toEqual( { start: 'start-id', end: 'clicked-id' } ); - } ); - - it( 'should return shallower ids when start is shallower', () => { - const result = getStartAndEndIds( { - blockSelectionStart: 'start-id', - clientId: 'clicked-id', - endParents: [ 'end-1', 'end-2', 'end-3' ], - startParents: [ 'start-1' ], - } ); - - expect( result ).toEqual( { start: 'start-id', end: 'end-2' } ); - } ); - - it( 'should return shallower ids when end is shallower', () => { - const result = getStartAndEndIds( { - blockSelectionStart: 'start-id', - clientId: 'clicked-id', - endParents: [ 'end-1', 'end-2' ], - startParents: [ 'start-1', 'start-2', 'start-3' ], - } ); - - expect( result ).toEqual( { start: 'start-3', end: 'clicked-id' } ); - } ); -} ); diff --git a/packages/block-editor/src/components/list-view/test/utils.js b/packages/block-editor/src/components/list-view/test/utils.js new file mode 100644 index 00000000000000..78d78a9d90069c --- /dev/null +++ b/packages/block-editor/src/components/list-view/test/utils.js @@ -0,0 +1,50 @@ +/** + * Internal dependencies + */ +import { getCommonDepthClientIds } from '../utils'; + +describe( 'getCommonDepthClientIds', () => { + it( 'should return start and end when no depth is provided', () => { + const result = getCommonDepthClientIds( + 'start-id', + 'clicked-id', + [], + [] + ); + + expect( result ).toEqual( { start: 'start-id', end: 'clicked-id' } ); + } ); + + it( 'should return deepest start and end when depths match', () => { + const result = getCommonDepthClientIds( + 'start-id', + 'clicked-id', + [ 'start-1', 'start-2', 'start-3' ], + [ 'end-1', 'end-2', 'end-3' ] + ); + + expect( result ).toEqual( { start: 'start-id', end: 'clicked-id' } ); + } ); + + it( 'should return shallower ids when start is shallower', () => { + const result = getCommonDepthClientIds( + 'start-id', + 'clicked-id', + [ 'start-1' ], + [ 'end-1', 'end-2', 'end-3' ] + ); + + expect( result ).toEqual( { start: 'start-id', end: 'end-2' } ); + } ); + + it( 'should return shallower ids when end is shallower', () => { + const result = getCommonDepthClientIds( + 'start-id', + 'clicked-id', + [ 'start-1', 'start-2', 'start-3' ], + [ 'end-1', 'end-2' ] + ); + + expect( result ).toEqual( { start: 'start-3', end: 'clicked-id' } ); + } ); +} ); diff --git a/packages/block-editor/src/components/list-view/use-block-selection.js b/packages/block-editor/src/components/list-view/use-block-selection.js index 882f0c9bc581a6..154f2e57421678 100644 --- a/packages/block-editor/src/components/list-view/use-block-selection.js +++ b/packages/block-editor/src/components/list-view/use-block-selection.js @@ -8,24 +8,7 @@ import { useCallback } from '@wordpress/element'; * Internal dependencies */ import { store as blockEditorStore } from '../../store'; - -export function getStartAndEndIds( { - blockSelectionStart, - clientId, - endParents, - startParents, -} ) { - const startPath = [ ...startParents, blockSelectionStart ]; - const endPath = [ ...endParents, clientId ]; - const depth = Math.min( startPath.length, endPath.length ) - 1; - const start = startPath[ depth ]; - const end = endPath[ depth ]; - - return { - start, - end, - }; -} +import { getCommonDepthClientIds } from './utils'; export default function useBlockSelection() { const { clearSelectedBlock, multiSelect, selectBlock } = useDispatch( @@ -74,12 +57,12 @@ export default function useBlockSelection() { ) { const endParents = getBlockParents( clientId ); - const { start, end } = getStartAndEndIds( { + const { start, end } = getCommonDepthClientIds( blockSelectionStart, clientId, - endParents, startParents, - } ); + endParents + ); // Handle the case of having selected a parent block and // then shift+click on a child. diff --git a/packages/block-editor/src/components/list-view/utils.js b/packages/block-editor/src/components/list-view/utils.js index 50bb561e42a8a6..8ef1b23fadb784 100644 --- a/packages/block-editor/src/components/list-view/utils.js +++ b/packages/block-editor/src/components/list-view/utils.js @@ -30,3 +30,34 @@ export const isClientIdSelected = ( clientId, selectedBlockClientIds ) => isArray( selectedBlockClientIds ) && selectedBlockClientIds.length ? selectedBlockClientIds.indexOf( clientId ) !== -1 : selectedBlockClientIds === clientId; + +/** + * From a start and end clientId of potentially different nesting levels, + * return the nearest-depth ids that have a common level of depth in the + * nesting hierarchy. For multiple block selection, this ensure that the + * selection is always at the same nesting level, and not split across + * separate levels. + * + * @param {string} startId The first id of a selection. + * @param {string} endId The end id of a selection, usually one that has been clicked on. + * @param {string[]} startParents An array of ancestor ids for the start id, in descending order. + * @param {string[]} endParents An array of ancestor ids for the end id, in descending order. + * @return {Object} An object containing the start and end ids. + */ +export function getCommonDepthClientIds( + startId, + endId, + startParents, + endParents +) { + const startPath = [ ...startParents, startId ]; + const endPath = [ ...endParents, endId ]; + const depth = Math.min( startPath.length, endPath.length ) - 1; + const start = startPath[ depth ]; + const end = endPath[ depth ]; + + return { + start, + end, + }; +} From d884e0ecf3c307438c76825688400d1cfeeeac24 Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Fri, 11 Feb 2022 14:10:40 +1100 Subject: [PATCH 14/24] Update multiSelect behavior to support keeping focus within the ListView while selecting multiple blocks --- .../data/data-core-block-editor.md | 1 + .../list-view/block-select-button.js | 1 - .../src/components/list-view/block.js | 1 - .../list-view/use-block-selection.js | 104 ++++++++++++------ .../writing-flow/use-multi-selection.js | 11 ++ packages/block-editor/src/store/actions.js | 12 +- packages/block-editor/src/store/reducer.js | 3 +- 7 files changed, 94 insertions(+), 39 deletions(-) diff --git a/docs/reference-guides/data/data-core-block-editor.md b/docs/reference-guides/data/data-core-block-editor.md index 87d88b83966513..4dca8ded6f4326 100644 --- a/docs/reference-guides/data/data-core-block-editor.md +++ b/docs/reference-guides/data/data-core-block-editor.md @@ -1261,6 +1261,7 @@ _Parameters_ - _start_ `string`: First block of the multi selection. - _end_ `string`: Last block of the multiselection. +- _initialPosition_ `number|null`: Optional initial position. Pass as null to skip focus within editor canvas. ### receiveBlocks diff --git a/packages/block-editor/src/components/list-view/block-select-button.js b/packages/block-editor/src/components/list-view/block-select-button.js index b7644c1b33d4d8..c699ed77742acc 100644 --- a/packages/block-editor/src/components/list-view/block-select-button.js +++ b/packages/block-editor/src/components/list-view/block-select-button.js @@ -65,7 +65,6 @@ function ListViewBlockSelectButton( ( event.shiftKey && ( event.keyCode === UP || event.keyCode === DOWN ) ) ) { - event.preventDefault(); onClick( event ); } } diff --git a/packages/block-editor/src/components/list-view/block.js b/packages/block-editor/src/components/list-view/block.js index d6b2c7b4a245eb..1bd95b479e7b50 100644 --- a/packages/block-editor/src/components/list-view/block.js +++ b/packages/block-editor/src/components/list-view/block.js @@ -104,7 +104,6 @@ function ListViewBlock( { const selectEditorBlock = useCallback( ( event ) => { - event.stopPropagation(); selectBlock( clientId, event ); }, [ clientId, selectBlock ] diff --git a/packages/block-editor/src/components/list-view/use-block-selection.js b/packages/block-editor/src/components/list-view/use-block-selection.js index 154f2e57421678..2a4fd0f27a8b74 100644 --- a/packages/block-editor/src/components/list-view/use-block-selection.js +++ b/packages/block-editor/src/components/list-view/use-block-selection.js @@ -3,6 +3,7 @@ */ import { useDispatch, useSelect } from '@wordpress/data'; import { useCallback } from '@wordpress/element'; +import { UP, DOWN } from '@wordpress/keycodes'; /** * Internal dependencies @@ -17,65 +18,104 @@ export default function useBlockSelection() { const { getBlockParents, getBlockSelectionStart, + getBlockSelectionEnd, + getNextBlockClientId, + getPreviousBlockClientId, + getSelectedBlockClientIds, hasMultiSelection, hasSelectedBlock, } = useSelect( blockEditorStore ); const updateBlockSelection = useCallback( async ( clientId, event ) => { - if ( ! event.shiftKey || event.type === 'keydown' ) { - // When this function is called from a 'keydown' event, select - // the block as if it were individually clicked. This allows - // the editor canvas to be responsible for the shift + up/down - // multiple block selection behaviour. + if ( ! event.shiftKey ) { await clearSelectedBlock(); - selectBlock( clientId, -1 ); + selectBlock( clientId, null ); } else if ( event.shiftKey ) { // To handle multiple block selection via the `SHIFT` key, prevent // the browser default behavior of opening the link in a new window. event.preventDefault(); - // Select a single block if no block is selected yet. - if ( ! hasSelectedBlock() && ! hasMultiSelection() ) { - selectBlock( clientId, -1 ); - return; - } + const isKeyPress = + event.type === 'keydown' && + ( event.keyCode === UP || event.keyCode === DOWN ); + const isUp = event.keyCode === UP; + const isDown = event.keyCode === DOWN; + const selectedBlocks = getSelectedBlockClientIds(); + const clientIdWithParents = [ + ...getBlockParents( clientId ), + clientId, + ]; - const blockSelectionStart = getBlockSelectionStart(); + if ( + isKeyPress && + ! selectedBlocks.some( ( blockId ) => + clientIdWithParents.includes( blockId ) + ) + ) { + // Ensure that shift-selecting blocks via the keyboard only + // expands the current selection if hovering over already + // selected blocks. Otherwise, clear the selection so that + // a user can create a new selection entirely by keyboard. + await clearSelectedBlock(); + } - // By checking `blockSelectionStart` to be set, we handle the - // case where we select a single block. We also have to check - // the selectionEnd (clientId) not to be included in the - // `blockSelectionStart`'s parents because the click event is - // propagated. - const startParents = getBlockParents( blockSelectionStart ); + let startTarget = getBlockSelectionStart(); + let endTarget = clientId; + // Handle clicking on a block when no blocks are selected. if ( - blockSelectionStart && - blockSelectionStart !== clientId && - ! startParents?.includes( clientId ) + ! isKeyPress && + ! hasSelectedBlock() && + ! hasMultiSelection() ) { - const endParents = getBlockParents( clientId ); + selectBlock( clientId, null ); + return; + } + + // Handle keyboard behavior for selecting multiple blocks. + if ( isKeyPress ) { + const prevId = getPreviousBlockClientId( clientId ); + const nextId = getNextBlockClientId( clientId ); - const { start, end } = getCommonDepthClientIds( - blockSelectionStart, - clientId, - startParents, - endParents - ); + if ( ! hasSelectedBlock() && ! hasMultiSelection() ) { + // Set the starting point of the selection to the currently + // hovered block, if there are no blocks currently selected. + // This ensures that as the selection is expanded or contracted, + // the starting point of the selection is anchored to that block. + startTarget = clientId; + } - // Handle the case of having selected a parent block and - // then shift+click on a child. - if ( start !== end ) { - multiSelect( start, end ); + if ( isUp && prevId ) { + // If the user presses UP, we want to ensure that the block they're + // moving to is the target for selection, and not the currently hovered one. + endTarget = prevId; + } else if ( isDown && nextId ) { + // If the user presses DOWN, we want to ensure that the block they're + // moving to is the target for selection, and not the currently hovered one. + endTarget = nextId; } } + + const startParents = getBlockParents( startTarget ); + const endParents = getBlockParents( endTarget ); + + const { start, end } = getCommonDepthClientIds( + startTarget, + endTarget, + startParents, + endParents + ); + multiSelect( start, end, null ); } }, [ clearSelectedBlock, getBlockParents, getBlockSelectionStart, + getBlockSelectionEnd, + getNextBlockClientId, + getSelectedBlockClientIds, hasMultiSelection, hasSelectedBlock, multiSelect, diff --git a/packages/block-editor/src/components/writing-flow/use-multi-selection.js b/packages/block-editor/src/components/writing-flow/use-multi-selection.js index 01f30fdae5339a..e8d11d528a440b 100644 --- a/packages/block-editor/src/components/writing-flow/use-multi-selection.js +++ b/packages/block-editor/src/components/writing-flow/use-multi-selection.js @@ -62,6 +62,7 @@ function selector( select ) { getMultiSelectedBlockClientIds, hasMultiSelection, getSelectedBlockClientId, + getSelectedBlocksInitialCaretPosition, } = select( blockEditorStore ); return { @@ -69,11 +70,13 @@ function selector( select ) { multiSelectedBlockClientIds: getMultiSelectedBlockClientIds(), hasMultiSelection: hasMultiSelection(), selectedBlockClientId: getSelectedBlockClientId(), + initialPosition: getSelectedBlocksInitialCaretPosition(), }; } export default function useMultiSelection() { const { + initialPosition, isMultiSelecting, multiSelectedBlockClientIds, hasMultiSelection, @@ -93,6 +96,13 @@ export default function useMultiSelection() { const { ownerDocument } = node; const { defaultView } = ownerDocument; + // Allow initialPosition to bypass focus behavior. This is useful + // for the list view or other areas where we don't want to transfer + // focus to the editor canvas. + if ( initialPosition === undefined || initialPosition === null ) { + return; + } + if ( ! hasMultiSelection || isMultiSelecting ) { if ( ! selectedBlockClientId || isMultiSelecting ) { return; @@ -160,6 +170,7 @@ export default function useMultiSelection() { isMultiSelecting, multiSelectedBlockClientIds, selectedBlockClientId, + initialPosition, ] ); } diff --git a/packages/block-editor/src/store/actions.js b/packages/block-editor/src/store/actions.js index 673fa6ae71f741..0b90784dfb7619 100644 --- a/packages/block-editor/src/store/actions.js +++ b/packages/block-editor/src/store/actions.js @@ -259,10 +259,14 @@ export function stopMultiSelect() { /** * Action that changes block multi-selection. * - * @param {string} start First block of the multi selection. - * @param {string} end Last block of the multiselection. + * @param {string} start First block of the multi selection. + * @param {string} end Last block of the multiselection. + * @param {number|null} initialPosition Optional initial position. Pass as null to skip focus within editor canvas. */ -export const multiSelect = ( start, end ) => ( { select, dispatch } ) => { +export const multiSelect = ( start, end, initialPosition = 0 ) => ( { + select, + dispatch, +} ) => { const startBlockRootClientId = select.getBlockRootClientId( start ); const endBlockRootClientId = select.getBlockRootClientId( end ); @@ -271,7 +275,7 @@ export const multiSelect = ( start, end ) => ( { select, dispatch } ) => { return; } - dispatch( { type: 'MULTI_SELECT', start, end } ); + dispatch( { type: 'MULTI_SELECT', start, end, initialPosition } ); const blockCount = select.getSelectedBlockCount(); diff --git a/packages/block-editor/src/store/reducer.js b/packages/block-editor/src/store/reducer.js index 7aa91bd85b1f02..70600cd12482fd 100644 --- a/packages/block-editor/src/store/reducer.js +++ b/packages/block-editor/src/store/reducer.js @@ -1380,7 +1380,7 @@ export function isSelectionEnabled( state = true, action ) { } /** - * Reducer returning the intial block selection. + * Reducer returning the initial block selection. * * Currently this in only used to restore the selection after block deletion and * pasting new content.This reducer should eventually be removed in favour of setting @@ -1399,6 +1399,7 @@ export function initialPosition( state = null, action ) { return action.initialPosition; } else if ( [ + 'MULTI_SELECT', 'SELECT_BLOCK', 'RESET_SELECTION', 'INSERT_BLOCKS', From 27d63bc0a58e23e85f1fa00b2140b88e5dbc103d Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Fri, 11 Feb 2022 15:19:53 +1100 Subject: [PATCH 15/24] Update unit tests, add additional e2e test for keyboard behaviour --- .../block-editor/src/store/test/actions.js | 1 + .../block-editor/src/store/test/reducer.js | 9 ++++ .../various/multi-block-selection.test.js | 45 +++++++++++++++++++ 3 files changed, 55 insertions(+) diff --git a/packages/block-editor/src/store/test/actions.js b/packages/block-editor/src/store/test/actions.js index 619e65ed019298..f13d7b3f3a8980 100644 --- a/packages/block-editor/src/store/test/actions.js +++ b/packages/block-editor/src/store/test/actions.js @@ -165,6 +165,7 @@ describe( 'actions', () => { type: 'MULTI_SELECT', start, end, + initialPosition: 0, } ); } ); diff --git a/packages/block-editor/src/store/test/reducer.js b/packages/block-editor/src/store/test/reducer.js index 66f3e98533ee88..388b2fb5801f1d 100644 --- a/packages/block-editor/src/store/test/reducer.js +++ b/packages/block-editor/src/store/test/reducer.js @@ -2187,6 +2187,15 @@ describe( 'state', () => { expect( state ).toBe( -1 ); } ); + + it( 'should allow setting null value in multi selection', () => { + const state = initialPosition( undefined, { + type: 'MULTI_SELECT', + initialPosition: null, + } ); + + expect( state ).toBe( null ); + } ); } ); describe( 'isMultiSelecting()', () => { diff --git a/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js b/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js index 85070bfe840af7..9c25e55d6d334e 100644 --- a/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js +++ b/packages/e2e-tests/specs/editor/various/multi-block-selection.test.js @@ -786,4 +786,49 @@ describe( 'Multi-block selection', () => { await navButtons[ 3 ].click(); expect( await getSelectedFlatIndices() ).toEqual( 4 ); } ); + + it( 'should multi-select in the ListView component with shift + up and down keys', async () => { + // Create four blocks. + await clickBlockAppender(); + await page.keyboard.type( '1' ); + await page.keyboard.press( 'Enter' ); + await page.keyboard.type( '2' ); + await page.keyboard.press( 'Enter' ); + await page.keyboard.type( '3' ); + await page.keyboard.press( 'Enter' ); + await page.keyboard.type( '4' ); + + // Open up the list view. The fourth block button will be focused. + await openListView(); + + // Press Up twice to focus over the second block. + await pressKeyTimes( 'ArrowUp', 2 ); + + // Shift + press Down to select the 2nd and 3rd blocks. + await page.keyboard.down( 'Shift' ); + await page.keyboard.press( 'ArrowDown' ); + expect( await getSelectedFlatIndices() ).toEqual( [ 2, 3 ] ); + + // Press Down once more to also select the 4th block. + await page.keyboard.press( 'ArrowDown' ); + expect( await getSelectedFlatIndices() ).toEqual( [ 2, 3, 4 ] ); + + // Press Up three times to adjust the selection to only include the first two blocks. + await pressKeyTimes( 'ArrowUp', 3 ); + expect( await getSelectedFlatIndices() ).toEqual( [ 1, 2 ] ); + + // Raise the shift key + await page.keyboard.up( 'Shift' ); + + // Navigate to the bottom of the list of blocks. + await pressKeyTimes( 'ArrowDown', 3 ); + + // Shift + press UP to select the 3rd and 4th blocks. + // This tests that shift selecting blocks by keyboard that are not adjacent + // to an existing selection resets the selection. + await page.keyboard.down( 'Shift' ); + await page.keyboard.press( 'ArrowUp' ); + await page.keyboard.up( 'Shift' ); + expect( await getSelectedFlatIndices() ).toEqual( [ 3, 4 ] ); + } ); } ); From c5853bcf5dd0e6fd0028b0fff51647ecdd240173 Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Fri, 11 Feb 2022 16:02:15 +1100 Subject: [PATCH 16/24] Revert change to focus for when shift key is not held --- .../src/components/list-view/use-block-selection.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-editor/src/components/list-view/use-block-selection.js b/packages/block-editor/src/components/list-view/use-block-selection.js index 2a4fd0f27a8b74..b6566eeda7021e 100644 --- a/packages/block-editor/src/components/list-view/use-block-selection.js +++ b/packages/block-editor/src/components/list-view/use-block-selection.js @@ -30,7 +30,7 @@ export default function useBlockSelection() { async ( clientId, event ) => { if ( ! event.shiftKey ) { await clearSelectedBlock(); - selectBlock( clientId, null ); + selectBlock( clientId ); } else if ( event.shiftKey ) { // To handle multiple block selection via the `SHIFT` key, prevent // the browser default behavior of opening the link in a new window. From 6cb4fb36a1b0fb38bcc2a26c9cb269a390d13050 Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Mon, 14 Feb 2022 17:23:38 +1100 Subject: [PATCH 17/24] Defer calculation of the next/prev clientId to the TreeGrid component where we can pass the next row that is focused --- .../src/components/list-view/README.md | 2 +- .../list-view/block-select-button.js | 9 +- .../src/components/list-view/block.js | 2 +- .../src/components/list-view/index.js | 30 ++-- .../list-view/use-block-selection.js | 131 ++++++++---------- packages/components/src/tree-grid/index.js | 13 +- 6 files changed, 97 insertions(+), 90 deletions(-) diff --git a/packages/block-editor/src/components/list-view/README.md b/packages/block-editor/src/components/list-view/README.md index 9552efa5a36f82..25fb7b61ac6c0e 100644 --- a/packages/block-editor/src/components/list-view/README.md +++ b/packages/block-editor/src/components/list-view/README.md @@ -23,7 +23,7 @@ Renders a list view with default syles. ```jsx import { ListView } from '@wordpress/block-editor'; -const MyNavigation = () => ; +const MyNavigation = () => ; ``` ## Related components diff --git a/packages/block-editor/src/components/list-view/block-select-button.js b/packages/block-editor/src/components/list-view/block-select-button.js index c699ed77742acc..d6bcf371c55d01 100644 --- a/packages/block-editor/src/components/list-view/block-select-button.js +++ b/packages/block-editor/src/components/list-view/block-select-button.js @@ -19,7 +19,7 @@ import useBlockDisplayInformation from '../use-block-display-information'; import { getBlockPositionDescription } from './utils'; import BlockTitle from '../block-title'; import ListViewExpander from './expander'; -import { UP, DOWN, SPACE, ENTER } from '@wordpress/keycodes'; +import { SPACE, ENTER } from '@wordpress/keycodes'; function ListViewBlockSelectButton( { @@ -59,12 +59,7 @@ function ListViewBlockSelectButton( }; function onKeyDownHandler( event ) { - if ( - event.keyCode === ENTER || - event.keyCode === SPACE || - ( event.shiftKey && - ( event.keyCode === UP || event.keyCode === DOWN ) ) - ) { + if ( event.keyCode === ENTER || event.keyCode === SPACE ) { onClick( event ); } } diff --git a/packages/block-editor/src/components/list-view/block.js b/packages/block-editor/src/components/list-view/block.js index 1bd95b479e7b50..bf9755fd149e6d 100644 --- a/packages/block-editor/src/components/list-view/block.js +++ b/packages/block-editor/src/components/list-view/block.js @@ -104,7 +104,7 @@ function ListViewBlock( { const selectEditorBlock = useCallback( ( event ) => { - selectBlock( clientId, event ); + selectBlock( event, clientId ); }, [ clientId, selectBlock ] ); diff --git a/packages/block-editor/src/components/list-view/index.js b/packages/block-editor/src/components/list-view/index.js index 8d88a2547a2def..7d7d7db7dce535 100644 --- a/packages/block-editor/src/components/list-view/index.js +++ b/packages/block-editor/src/components/list-view/index.js @@ -44,15 +44,14 @@ const expanded = ( state, action ) => { * recursive component (it renders itself), so this ensures TreeGrid is only * present at the very top of the navigation grid. * - * @param {Object} props Components props. - * @param {Array} props.blocks Custom subset of block client IDs to be used instead of the default hierarchy. - * @param {Function} props.onSelect Block selection callback. - * @param {boolean} props.showNestedBlocks Flag to enable displaying nested blocks. - * @param {boolean} props.showBlockMovers Flag to enable block movers - * @param {boolean} props.__experimentalFeatures Flag to enable experimental features. - * @param {boolean} props.__experimentalPersistentListViewFeatures Flag to enable features for the Persistent List View experiment. - * @param {boolean} props.__experimentalHideContainerBlockActions Flag to hide actions of top level blocks (like core/widget-area) - * @param {Object} ref Forwarded ref + * @param {Object} props Components props. + * @param {Array} props.blocks Custom subset of block client IDs to be used instead of the default hierarchy. + * @param {boolean} props.showNestedBlocks Flag to enable displaying nested blocks. + * @param {boolean} props.showBlockMovers Flag to enable block movers + * @param {boolean} props.__experimentalFeatures Flag to enable experimental features. + * @param {boolean} props.__experimentalPersistentListViewFeatures Flag to enable features for the Persistent List View experiment. + * @param {boolean} props.__experimentalHideContainerBlockActions Flag to hide actions of top level blocks (like core/widget-area) + * @param {Object} ref Forwarded ref */ function ListView( { @@ -144,6 +143,18 @@ function ListView( }, [ collapse ] ); + const changeRow = useCallback( + ( event, startRow, endRow ) => { + if ( event.shiftKey ) { + updateBlockSelection( + event, + startRow?.dataset?.block, + endRow?.dataset?.block + ); + } + }, + [ updateBlockSelection ] + ); const contextValue = useMemo( () => ( { @@ -180,6 +191,7 @@ function ListView( ref={ treeGridRef } onCollapseRow={ collapseRow } onExpandRow={ expandRow } + onChangeRow={ changeRow } > { + async ( event, clientId, destinationClientId ) => { if ( ! event.shiftKey ) { await clearSelectedBlock(); selectBlock( clientId ); - } else if ( event.shiftKey ) { - // To handle multiple block selection via the `SHIFT` key, prevent - // the browser default behavior of opening the link in a new window. - event.preventDefault(); + return; + } - const isKeyPress = - event.type === 'keydown' && - ( event.keyCode === UP || event.keyCode === DOWN ); - const isUp = event.keyCode === UP; - const isDown = event.keyCode === DOWN; - const selectedBlocks = getSelectedBlockClientIds(); - const clientIdWithParents = [ - ...getBlockParents( clientId ), - clientId, - ]; + // To handle multiple block selection via the `SHIFT` key, prevent + // the browser default behavior of opening the link in a new window. + event.preventDefault(); - if ( - isKeyPress && - ! selectedBlocks.some( ( blockId ) => - clientIdWithParents.includes( blockId ) - ) - ) { - // Ensure that shift-selecting blocks via the keyboard only - // expands the current selection if hovering over already - // selected blocks. Otherwise, clear the selection so that - // a user can create a new selection entirely by keyboard. - await clearSelectedBlock(); - } + const isKeyPress = + event.type === 'keydown' && + ( event.keyCode === UP || event.keyCode === DOWN ); - let startTarget = getBlockSelectionStart(); - let endTarget = clientId; + // Handle clicking on a block when no blocks are selected, and return early. + if ( + ! isKeyPress && + ! hasSelectedBlock() && + ! hasMultiSelection() + ) { + selectBlock( clientId, null ); + return; + } - // Handle clicking on a block when no blocks are selected. - if ( - ! isKeyPress && - ! hasSelectedBlock() && - ! hasMultiSelection() - ) { - selectBlock( clientId, null ); - return; - } + const selectedBlocks = getSelectedBlockClientIds(); + const clientIdWithParents = [ + ...getBlockParents( clientId ), + clientId, + ]; - // Handle keyboard behavior for selecting multiple blocks. - if ( isKeyPress ) { - const prevId = getPreviousBlockClientId( clientId ); - const nextId = getNextBlockClientId( clientId ); + if ( + isKeyPress && + ! selectedBlocks.some( ( blockId ) => + clientIdWithParents.includes( blockId ) + ) + ) { + // Ensure that shift-selecting blocks via the keyboard only + // expands the current selection if focusing over already + // selected blocks. Otherwise, clear the selection so that + // a user can create a new selection entirely by keyboard. + await clearSelectedBlock(); + } - if ( ! hasSelectedBlock() && ! hasMultiSelection() ) { - // Set the starting point of the selection to the currently - // hovered block, if there are no blocks currently selected. - // This ensures that as the selection is expanded or contracted, - // the starting point of the selection is anchored to that block. - startTarget = clientId; - } + let startTarget = getBlockSelectionStart(); + let endTarget = clientId; - if ( isUp && prevId ) { - // If the user presses UP, we want to ensure that the block they're - // moving to is the target for selection, and not the currently hovered one. - endTarget = prevId; - } else if ( isDown && nextId ) { - // If the user presses DOWN, we want to ensure that the block they're - // moving to is the target for selection, and not the currently hovered one. - endTarget = nextId; - } + // Handle keyboard behavior for selecting multiple blocks. + if ( isKeyPress ) { + if ( ! hasSelectedBlock() && ! hasMultiSelection() ) { + // Set the starting point of the selection to the currently + // focused block, if there are no blocks currently selected. + // This ensures that as the selection is expanded or contracted, + // the starting point of the selection is anchored to that block. + startTarget = clientId; + } + if ( destinationClientId ) { + // If the user presses UP or DOWN, we want to ensure that the block they're + // moving to is the target for selection, and not the currently focused one. + endTarget = destinationClientId; } + } - const startParents = getBlockParents( startTarget ); - const endParents = getBlockParents( endTarget ); + const startParents = getBlockParents( startTarget ); + const endParents = getBlockParents( endTarget ); - const { start, end } = getCommonDepthClientIds( - startTarget, - endTarget, - startParents, - endParents - ); - multiSelect( start, end, null ); - } + const { start, end } = getCommonDepthClientIds( + startTarget, + endTarget, + startParents, + endParents + ); + multiSelect( start, end, null ); }, [ clearSelectedBlock, getBlockParents, getBlockSelectionStart, getBlockSelectionEnd, - getNextBlockClientId, getSelectedBlockClientIds, hasMultiSelection, hasSelectedBlock, diff --git a/packages/components/src/tree-grid/index.js b/packages/components/src/tree-grid/index.js index f8cfed5c91eaaa..4a5a4f97aded7c 100644 --- a/packages/components/src/tree-grid/index.js +++ b/packages/components/src/tree-grid/index.js @@ -45,10 +45,17 @@ function getRowFocusables( rowElement ) { * @param {WPElement} props.children Children to be rendered. * @param {Function} props.onExpandRow Callback to fire when row is expanded. * @param {Function} props.onCollapseRow Callback to fire when row is collapsed. + * @param {Function} props.onChangeRow Callback to fire when moving focus to a different row. * @param {Object} ref A ref to the underlying DOM table element. */ function TreeGrid( - { children, onExpandRow = () => {}, onCollapseRow = () => {}, ...props }, + { + children, + onExpandRow = () => {}, + onCollapseRow = () => {}, + onChangeRow = () => {}, + ...props + }, ref ) { const onKeyDown = useCallback( @@ -217,6 +224,10 @@ function TreeGrid( ); focusablesInNextRow[ nextIndex ].focus(); + // Let consumers know the row that was originally focused, + // and the row that is now in focus. + onChangeRow( event, activeRow, rows[ nextRowIndex ] ); + // Prevent key use for anything else. This ensures Voiceover // doesn't try to handle key navigation. event.preventDefault(); From 38b0e5b27968f7d1ab9d3119a92e055cf0e91da7 Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Tue, 15 Feb 2022 17:33:25 +1100 Subject: [PATCH 18/24] Update initialPosition param to be experimental --- .../data/data-core-block-editor.md | 2 +- packages/block-editor/src/store/actions.js | 22 ++++++++++++------- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/docs/reference-guides/data/data-core-block-editor.md b/docs/reference-guides/data/data-core-block-editor.md index 4dca8ded6f4326..81a8d46c67c33b 100644 --- a/docs/reference-guides/data/data-core-block-editor.md +++ b/docs/reference-guides/data/data-core-block-editor.md @@ -1261,7 +1261,7 @@ _Parameters_ - _start_ `string`: First block of the multi selection. - _end_ `string`: Last block of the multiselection. -- _initialPosition_ `number|null`: Optional initial position. Pass as null to skip focus within editor canvas. +- _\_\_experimentalInitialPosition_ `number|null`: Optional initial position. Pass as null to skip focus within editor canvas. ### receiveBlocks diff --git a/packages/block-editor/src/store/actions.js b/packages/block-editor/src/store/actions.js index 0b90784dfb7619..a467c6e8a7b197 100644 --- a/packages/block-editor/src/store/actions.js +++ b/packages/block-editor/src/store/actions.js @@ -259,14 +259,15 @@ export function stopMultiSelect() { /** * Action that changes block multi-selection. * - * @param {string} start First block of the multi selection. - * @param {string} end Last block of the multiselection. - * @param {number|null} initialPosition Optional initial position. Pass as null to skip focus within editor canvas. + * @param {string} start First block of the multi selection. + * @param {string} end Last block of the multiselection. + * @param {number|null} __experimentalInitialPosition Optional initial position. Pass as null to skip focus within editor canvas. */ -export const multiSelect = ( start, end, initialPosition = 0 ) => ( { - select, - dispatch, -} ) => { +export const multiSelect = ( + start, + end, + __experimentalInitialPosition = 0 +) => ( { select, dispatch } ) => { const startBlockRootClientId = select.getBlockRootClientId( start ); const endBlockRootClientId = select.getBlockRootClientId( end ); @@ -275,7 +276,12 @@ export const multiSelect = ( start, end, initialPosition = 0 ) => ( { return; } - dispatch( { type: 'MULTI_SELECT', start, end, initialPosition } ); + dispatch( { + type: 'MULTI_SELECT', + start, + end, + initialPosition: __experimentalInitialPosition, + } ); const blockCount = select.getSelectedBlockCount(); From 025087f325787daeef92b2060c724e51209ddfbc Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Tue, 15 Feb 2022 17:51:25 +1100 Subject: [PATCH 19/24] Update TreeGrid readme with documentation for the three callback functions --- packages/components/src/tree-grid/README.md | 25 ++++++++++++++++++++- packages/components/src/tree-grid/index.js | 2 +- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/packages/components/src/tree-grid/README.md b/packages/components/src/tree-grid/README.md index 3806d7cc66fd34..4c00869f3dab25 100644 --- a/packages/components/src/tree-grid/README.md +++ b/packages/components/src/tree-grid/README.md @@ -108,10 +108,33 @@ function TreeMenu() { ##### Props -`TreeGrid` accepts no specific props. Any props specified will be passed to the `table` element rendered by `TreeGrid`. +Aside from the documented callback functions, any props specified will be passed to the `table` element rendered by `TreeGrid`. `TreeGrid` should always have children. +###### onChangeRow( event: Event, startRow: HTMLElement, destinationRow: HTMLElement ) + +Callback that fires when focus is shifted from one row to another via the UP and DOWN keys. +The callback is passed the event, the start row element that the focus was on originally, and +the destination row element after the focus has moved. + +- Type: `Function` +- Required: No + +###### onCollapseRow( row: HTMLElement ) + +A callback that passes in the row element to be collapsed. + +- Type: `Function` +- Required: No + +###### onExpandRow( row: HTMLElement ) + +A callback that passes in the row element to be expanded. + +- Type: `Function` +- Required: No + #### TreeGridRow ##### Props diff --git a/packages/components/src/tree-grid/index.js b/packages/components/src/tree-grid/index.js index 4a5a4f97aded7c..6725ded76a5df1 100644 --- a/packages/components/src/tree-grid/index.js +++ b/packages/components/src/tree-grid/index.js @@ -280,7 +280,7 @@ function TreeGrid( event.preventDefault(); } }, - [ onExpandRow, onCollapseRow ] + [ onExpandRow, onCollapseRow, onChangeRow ] ); /* Disable reason: A treegrid is implemented using a table element. */ From c3b6a1e15d6d310c0df7717d255bbb33ed870d8d Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Tue, 15 Feb 2022 17:54:00 +1100 Subject: [PATCH 20/24] Add changelog entry --- packages/components/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index ed497aaa24914d..c0ffe952d24376 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -22,6 +22,7 @@ - `TreeGrid` accessibility: improve browser support for Left Arrow focus to parent row in child row. ([#38639](https://github.com/WordPress/gutenberg/pull/38639)) - `TreeGrid` accessibility: Add Home/End keys for better keyboard navigation. ([#38679](https://github.com/WordPress/gutenberg/pull/38679)) - Add `resolvePoint` prop to `FocalPointPicker` to allow updating the value of the picker after a user interaction ([#38247](https://github.com/WordPress/gutenberg/pull/38247)) +- Add `onChangeRow` callback to the `TreeGrid` component, fired when focus is shifted from one row to another via Up and Down arrow keys. ([#38314](https://github.com/WordPress/gutenberg/pull/38314)) ### Experimental From b321851c72de5dc7f6b3f1a469772520e154467c Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Wed, 16 Feb 2022 15:01:12 +1100 Subject: [PATCH 21/24] Pass in selectedClientIds to dropdown --- .../components/list-view/block-contents.js | 13 ++++-------- .../src/components/list-view/block.js | 21 +++++++++++++++++-- .../src/components/list-view/branch.js | 1 + .../list-view/use-block-selection.js | 2 +- 4 files changed, 25 insertions(+), 12 deletions(-) diff --git a/packages/block-editor/src/components/list-view/block-contents.js b/packages/block-editor/src/components/list-view/block-contents.js index 8a1c3555d000b0..2fb787dac3137e 100644 --- a/packages/block-editor/src/components/list-view/block-contents.js +++ b/packages/block-editor/src/components/list-view/block-contents.js @@ -27,27 +27,22 @@ const ListViewBlockContents = forwardRef( siblingBlockCount, level, isExpanded, + selectedClientIds, ...props }, ref ) => { const { clientId } = block; - const { - blockMovingClientId, - selectedBlockInBlockEditor, - selectedBlocks, - } = useSelect( + const { blockMovingClientId, selectedBlockInBlockEditor } = useSelect( ( select ) => { const { hasBlockMovingClientId, getSelectedBlockClientId, - getSelectedBlockClientIds, } = select( blockEditorStore ); return { blockMovingClientId: hasBlockMovingClientId(), selectedBlockInBlockEditor: getSelectedBlockClientId(), - selectedBlocks: getSelectedBlockClientIds(), }; }, [ clientId ] @@ -64,8 +59,8 @@ const ListViewBlockContents = forwardRef( // is one of the selected blocks. This ensures that if a user attempts // to drag a block that isn't part of the selection, they're still able // to drag it and rearrange its position. - const draggableClientIds = selectedBlocks.includes( clientId ) - ? selectedBlocks + const draggableClientIds = selectedClientIds.includes( clientId ) + ? selectedClientIds : [ clientId ]; return ( diff --git a/packages/block-editor/src/components/list-view/block.js b/packages/block-editor/src/components/list-view/block.js index bf9755fd149e6d..33beedcae07816 100644 --- a/packages/block-editor/src/components/list-view/block.js +++ b/packages/block-editor/src/components/list-view/block.js @@ -48,6 +48,7 @@ function ListViewBlock( { showBlockMovers, path, isExpanded, + selectedClientIds, } ) { const cellRef = useRef( null ); const [ isHovered, setIsHovered ] = useState( false ); @@ -109,6 +110,13 @@ function ListViewBlock( { [ clientId, selectBlock ] ); + const selectDuplicatedBlock = useCallback( + ( newClientId ) => { + selectBlock( undefined, newClientId ); + }, + [ selectBlock ] + ); + const toggleExpanded = useCallback( ( event ) => { event.stopPropagation(); @@ -153,6 +161,14 @@ function ListViewBlock( { ) : __( 'Options' ); + // Only include all selected blocks if the currently clicked on block + // is one of the selected blocks. This ensures that if a user attempts + // to alter a block that isn't part of the selection, they're still able + // to do so. + const dropdownClientIds = selectedClientIds.includes( clientId ) + ? selectedClientIds + : [ clientId ]; + return ( ) } @@ -227,7 +244,7 @@ function ListViewBlock( { { ( { ref, tabIndex, onFocus } ) => ( ) } diff --git a/packages/block-editor/src/components/list-view/branch.js b/packages/block-editor/src/components/list-view/branch.js index 007e20df4fa728..71518a1e289f3f 100644 --- a/packages/block-editor/src/components/list-view/branch.js +++ b/packages/block-editor/src/components/list-view/branch.js @@ -145,6 +145,7 @@ function ListViewBranch( props ) { path={ updatedPath } isExpanded={ isExpanded } listPosition={ nextPosition } + selectedClientIds={ selectedClientIds } /> ) } { ! showBlock && ( diff --git a/packages/block-editor/src/components/list-view/use-block-selection.js b/packages/block-editor/src/components/list-view/use-block-selection.js index e7f9bd8f40a68b..f1c5ee67399e6e 100644 --- a/packages/block-editor/src/components/list-view/use-block-selection.js +++ b/packages/block-editor/src/components/list-view/use-block-selection.js @@ -26,7 +26,7 @@ export default function useBlockSelection() { const updateBlockSelection = useCallback( async ( event, clientId, destinationClientId ) => { - if ( ! event.shiftKey ) { + if ( ! event?.shiftKey ) { await clearSelectedBlock(); selectBlock( clientId ); return; From db31ffdb5ae4c4910894ddeb9fe43cb47f6e2cb7 Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Wed, 16 Feb 2022 15:06:00 +1100 Subject: [PATCH 22/24] Prevent shift+click on expand toggle from opening a new window --- packages/block-editor/src/components/list-view/block.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/block-editor/src/components/list-view/block.js b/packages/block-editor/src/components/list-view/block.js index 33beedcae07816..c4abbde1e1b7b7 100644 --- a/packages/block-editor/src/components/list-view/block.js +++ b/packages/block-editor/src/components/list-view/block.js @@ -119,6 +119,8 @@ function ListViewBlock( { const toggleExpanded = useCallback( ( event ) => { + // Prevent shift+click from opening link in a new window when toggling. + event.preventDefault(); event.stopPropagation(); if ( isExpanded === true ) { collapse( clientId ); From 4f0e0bb25692cf11f85952b722481d885fb1f8d2 Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Wed, 16 Feb 2022 17:36:09 +1100 Subject: [PATCH 23/24] Try announcing deselected blocks --- .../list-view/use-block-selection.js | 47 ++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/packages/block-editor/src/components/list-view/use-block-selection.js b/packages/block-editor/src/components/list-view/use-block-selection.js index f1c5ee67399e6e..b6ac6e1825d417 100644 --- a/packages/block-editor/src/components/list-view/use-block-selection.js +++ b/packages/block-editor/src/components/list-view/use-block-selection.js @@ -1,9 +1,17 @@ +/** + * External dependencies + */ +import { difference } from 'lodash'; + /** * WordPress dependencies */ +import { speak } from '@wordpress/a11y'; +import { __, sprintf } from '@wordpress/i18n'; import { useDispatch, useSelect } from '@wordpress/data'; import { useCallback } from '@wordpress/element'; import { UP, DOWN } from '@wordpress/keycodes'; +import { store as blocksStore } from '@wordpress/blocks'; /** * Internal dependencies @@ -16,6 +24,7 @@ export default function useBlockSelection() { blockEditorStore ); const { + getBlockName, getBlockParents, getBlockSelectionStart, getBlockSelectionEnd, @@ -24,6 +33,8 @@ export default function useBlockSelection() { hasSelectedBlock, } = useSelect( blockEditorStore ); + const { getBlockType } = useSelect( blocksStore ); + const updateBlockSelection = useCallback( async ( event, clientId, destinationClientId ) => { if ( ! event?.shiftKey ) { @@ -97,10 +108,44 @@ export default function useBlockSelection() { startParents, endParents ); - multiSelect( start, end, null ); + await multiSelect( start, end, null ); + + // Announce deselected block, or number of deselected blocks if + // the total number of blocks deselected is greater than one. + const updatedSelectedBlocks = getSelectedBlockClientIds(); + + const selectionDiff = difference( + selectedBlocks, + updatedSelectedBlocks + ); + + let label; + if ( selectionDiff.length === 1 ) { + const title = getBlockType( getBlockName( selectionDiff[ 0 ] ) ) + ?.title; + if ( title ) { + label = sprintf( + /* translators: %s: block name */ + __( '%s deselected.' ), + title + ); + } + } else if ( selectionDiff.length > 1 ) { + label = sprintf( + /* translators: %s: number of deselected blocks */ + __( '%s blocks deselected.' ), + selectionDiff.length + ); + } + + if ( label ) { + speak( label ); + } }, [ clearSelectedBlock, + getBlockName, + getBlockType, getBlockParents, getBlockSelectionStart, getBlockSelectionEnd, From 5511896de7cc60499a24ffece4cefb64c68b1b15 Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Fri, 18 Feb 2022 11:17:06 +1100 Subject: [PATCH 24/24] Rename onChangeRow to onFocusRow, update changelog entry --- packages/block-editor/src/components/list-view/index.js | 4 ++-- packages/components/CHANGELOG.md | 2 +- packages/components/src/tree-grid/README.md | 2 +- packages/components/src/tree-grid/index.js | 8 ++++---- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/block-editor/src/components/list-view/index.js b/packages/block-editor/src/components/list-view/index.js index 7d7d7db7dce535..4f7c9313348935 100644 --- a/packages/block-editor/src/components/list-view/index.js +++ b/packages/block-editor/src/components/list-view/index.js @@ -143,7 +143,7 @@ function ListView( }, [ collapse ] ); - const changeRow = useCallback( + const focusRow = useCallback( ( event, startRow, endRow ) => { if ( event.shiftKey ) { updateBlockSelection( @@ -191,7 +191,7 @@ function ListView( ref={ treeGridRef } onCollapseRow={ collapseRow } onExpandRow={ expandRow } - onChangeRow={ changeRow } + onFocusRow={ focusRow } > {}, onCollapseRow = () => {}, - onChangeRow = () => {}, + onFocusRow = () => {}, ...props }, ref @@ -226,7 +226,7 @@ function TreeGrid( // Let consumers know the row that was originally focused, // and the row that is now in focus. - onChangeRow( event, activeRow, rows[ nextRowIndex ] ); + onFocusRow( event, activeRow, rows[ nextRowIndex ] ); // Prevent key use for anything else. This ensures Voiceover // doesn't try to handle key navigation. @@ -280,7 +280,7 @@ function TreeGrid( event.preventDefault(); } }, - [ onExpandRow, onCollapseRow, onChangeRow ] + [ onExpandRow, onCollapseRow, onFocusRow ] ); /* Disable reason: A treegrid is implemented using a table element. */