From c66643cbb96df99c3e4edde1afccbff47a3cb346 Mon Sep 17 00:00:00 2001 From: Ella van Durpe Date: Tue, 21 Jan 2020 15:50:35 +0100 Subject: [PATCH 1/5] Multi select: keep selection after move --- .../block-list/use-multi-selection.js | 31 ++++++++++--------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/packages/block-editor/src/components/block-list/use-multi-selection.js b/packages/block-editor/src/components/block-list/use-multi-selection.js index 3750c9334a1f61..b873bd84f87301 100644 --- a/packages/block-editor/src/components/block-list/use-multi-selection.js +++ b/packages/block-editor/src/components/block-list/use-multi-selection.js @@ -147,7 +147,7 @@ export default function useMultiSelection( ref ) { selectedBlockClientId, ] ); - const onSelectionChange = useCallback( () => { + const onSelectionChange = useCallback( ( { isFinal } ) => { const selection = window.getSelection(); // If no selection is found, end multi selection. @@ -159,6 +159,20 @@ export default function useMultiSelection( ref ) { if ( startClientId.current === clientId ) { selectBlock( clientId ); + + if ( isFinal ) { + toggleRichText( ref.current, true ); + + // If the anchor element contains the selection, set focus back to + // the anchor element. + if ( selection.rangeCount ) { + const { commonAncestorContainer } = selection.getRangeAt( 0 ); + + if ( anchorElement.current.contains( commonAncestorContainer ) ) { + anchorElement.current.focus(); + } + } + } } else { const startPath = [ ...getBlockParents( startClientId.current ), startClientId.current ]; const endPath = [ ...getBlockParents( clientId ), clientId ]; @@ -178,21 +192,8 @@ export default function useMultiSelection( ref ) { // The browser selection won't have updated yet at this point, so wait // until the next animation frame to get the browser selection. rafId.current = window.requestAnimationFrame( () => { - onSelectionChange(); + onSelectionChange( { isFinal: true } ); stopMultiSelect(); - toggleRichText( ref.current, true ); - - const selection = window.getSelection(); - - // If the anchor element contains the selection, set focus back to - // the anchor element. - if ( selection.rangeCount ) { - const { commonAncestorContainer } = selection.getRangeAt( 0 ); - - if ( anchorElement.current.contains( commonAncestorContainer ) ) { - anchorElement.current.focus(); - } - } } ); }, [ onSelectionChange, stopMultiSelect ] ); From b5107597fc742eb6825624193f548c046aecf4d8 Mon Sep 17 00:00:00 2001 From: Ella van Durpe Date: Tue, 21 Jan 2020 16:59:16 +0100 Subject: [PATCH 2/5] Add e2e test --- .../multi-block-selection.test.js.snap | 14 +++++++++++++ .../various/multi-block-selection.test.js | 21 +++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/packages/e2e-tests/specs/editor/various/__snapshots__/multi-block-selection.test.js.snap b/packages/e2e-tests/specs/editor/various/__snapshots__/multi-block-selection.test.js.snap index e3125d7972ea80..73c25f00d43af9 100644 --- a/packages/e2e-tests/specs/editor/various/__snapshots__/multi-block-selection.test.js.snap +++ b/packages/e2e-tests/specs/editor/various/__snapshots__/multi-block-selection.test.js.snap @@ -52,6 +52,20 @@ exports[`Multi-block selection should only trigger multi-selection when at the e " `; +exports[`Multi-block selection should preserve selection on move 1`] = ` +" +

2

+ + + +

3

+ + + +

1

+" +`; + exports[`Multi-block selection should return original focus after failed multi selection attempt 1`] = ` "

2

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 8eaf697fb5d3c0..1d72fea1b50af0 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 @@ -7,6 +7,7 @@ import { pressKeyWithModifier, pressKeyTimes, getEditedPostContent, + clickBlockToolbarButton, } from '@wordpress/e2e-test-utils'; async function getSelectedFlatIndices() { @@ -408,4 +409,24 @@ describe( 'Multi-block selection', () => { expect( await getEditedPostContent() ).toMatchSnapshot(); } ); + + it( 'should preserve selection on move', async () => { + 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 pressKeyWithModifier( 'shift', 'ArrowUp' ); + + await testNativeSelection(); + expect( await getSelectedFlatIndices() ).toEqual( [ 2, 3 ] ); + + await clickBlockToolbarButton( 'Move up' ); + + await testNativeSelection(); + expect( await getSelectedFlatIndices() ).toEqual( [ 1, 2 ] ); + + expect( await getEditedPostContent() ).toMatchSnapshot(); + } ); } ); From afa388386fb3ad0b112e93b8ec62bc318fa7aa88 Mon Sep 17 00:00:00 2001 From: Ella van Durpe Date: Wed, 22 Jan 2020 15:20:10 +0100 Subject: [PATCH 3/5] Change e2e test --- .../various/multi-block-selection.test.js | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) 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 1d72fea1b50af0..cd7b19f264666c 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 @@ -410,14 +410,34 @@ describe( 'Multi-block selection', () => { expect( await getEditedPostContent() ).toMatchSnapshot(); } ); - it( 'should preserve selection on move', async () => { + it( 'should preserve dragged selection on move', async () => { 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 pressKeyWithModifier( 'shift', 'ArrowUp' ); + + const [ coord1, coord2 ] = await page.evaluate( () => { + const elements = Array.from( document.querySelectorAll( '.wp-block-paragraph' ) ); + const rect1 = elements[ 2 ].getBoundingClientRect(); + const rect2 = elements[ 1 ].getBoundingClientRect(); + return [ + { + x: rect1.x + ( rect1.width / 2 ), + y: rect1.y + ( rect1.height / 2 ), + }, + { + x: rect2.x + ( rect2.width / 2 ), + y: rect2.y + ( rect2.height / 2 ), + }, + ]; + } ); + + await page.mouse.move( coord1.x, coord1.y ); + await page.mouse.down(); + await page.mouse.move( coord2.x, coord2.y ); + await page.mouse.up(); await testNativeSelection(); expect( await getSelectedFlatIndices() ).toEqual( [ 2, 3 ] ); From c05badfef3f93b7f764549530e9a8dee6cf82e2e Mon Sep 17 00:00:00 2001 From: Ella van Durpe Date: Wed, 22 Jan 2020 16:00:01 +0100 Subject: [PATCH 4/5] Address feedback --- .../components/block-list/use-multi-selection.js | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/block-editor/src/components/block-list/use-multi-selection.js b/packages/block-editor/src/components/block-list/use-multi-selection.js index b873bd84f87301..1712970e7cbacb 100644 --- a/packages/block-editor/src/components/block-list/use-multi-selection.js +++ b/packages/block-editor/src/components/block-list/use-multi-selection.js @@ -147,7 +147,7 @@ export default function useMultiSelection( ref ) { selectedBlockClientId, ] ); - const onSelectionChange = useCallback( ( { isFinal } ) => { + const onSelectionChange = useCallback( ( { isSelectionEnd } ) => { const selection = window.getSelection(); // If no selection is found, end multi selection. @@ -156,15 +156,19 @@ export default function useMultiSelection( ref ) { } const clientId = getBlockClientId( selection.focusNode ); + const isSingularSelection = startClientId.current === clientId; - if ( startClientId.current === clientId ) { + if ( isSingularSelection ) { selectBlock( clientId ); - if ( isFinal ) { + // If the selection is complete (on mouse up), and no multiple + // blocks have been selected, set focus back to the anchor element + // if the anchor element contains the selection. Additionally, rich + // text elements that were previously disabled can now be enabled + // again. + if ( isSelectionEnd ) { toggleRichText( ref.current, true ); - // If the anchor element contains the selection, set focus back to - // the anchor element. if ( selection.rangeCount ) { const { commonAncestorContainer } = selection.getRangeAt( 0 ); @@ -192,7 +196,7 @@ export default function useMultiSelection( ref ) { // The browser selection won't have updated yet at this point, so wait // until the next animation frame to get the browser selection. rafId.current = window.requestAnimationFrame( () => { - onSelectionChange( { isFinal: true } ); + onSelectionChange( { isSelectionEnd: true } ); stopMultiSelect(); } ); }, [ onSelectionChange, stopMultiSelect ] ); From eee4d83309622b4ba9ddba327c0cda9b3243ab01 Mon Sep 17 00:00:00 2001 From: Ella van Durpe Date: Wed, 22 Jan 2020 16:32:44 +0100 Subject: [PATCH 5/5] Fix snapshots --- .../various/__snapshots__/multi-block-selection.test.js.snap | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/e2e-tests/specs/editor/various/__snapshots__/multi-block-selection.test.js.snap b/packages/e2e-tests/specs/editor/various/__snapshots__/multi-block-selection.test.js.snap index 73c25f00d43af9..68f5047e1bc9c0 100644 --- a/packages/e2e-tests/specs/editor/various/__snapshots__/multi-block-selection.test.js.snap +++ b/packages/e2e-tests/specs/editor/various/__snapshots__/multi-block-selection.test.js.snap @@ -52,7 +52,7 @@ exports[`Multi-block selection should only trigger multi-selection when at the e " `; -exports[`Multi-block selection should preserve selection on move 1`] = ` +exports[`Multi-block selection should preserve dragged selection on move 1`] = ` "

2