From 42b870c6be84f9b2fefcc5bf4813598a23a16de1 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Tue, 7 Aug 2018 13:23:08 +0100 Subject: [PATCH] RichText: Removing/merging richtext should only trigger if the selection is collapsed (#8651) * RichText: Removing/merging richtext should only trigger if the selection is collapsed * Add e2e test to avoid uncollapsed selection to be mergedwq * Code Style improvements per review --- .../editor/src/components/rich-text/index.js | 10 ++++++---- .../splitting-merging.test.js.snap | 20 ++++++++++++++----- test/e2e/specs/splitting-merging.test.js | 20 ++++++++++++++++--- 3 files changed, 38 insertions(+), 12 deletions(-) diff --git a/packages/editor/src/components/rich-text/index.js b/packages/editor/src/components/rich-text/index.js index 06d954e775615..e8c0e187c47d2 100644 --- a/packages/editor/src/components/rich-text/index.js +++ b/packages/editor/src/components/rich-text/index.js @@ -47,7 +47,7 @@ import TokenUI from './tokens/ui'; * Browser dependencies */ -const { Node } = window; +const { Node, getSelection } = window; /** * Zero-width space character used by TinyMCE as a caret landing point for @@ -396,7 +396,7 @@ export class RichText extends Component { * @param {tinymce.EditorEvent} event Keydown event. */ onHorizontalNavigationKeyDown( event ) { - const { focusNode } = window.getSelection(); + const { focusNode } = getSelection(); const { nodeType, nodeValue } = focusNode; if ( nodeType !== Node.TEXT_NODE ) { @@ -437,8 +437,10 @@ export class RichText extends Component { const { keyCode } = event; if ( - ( keyCode === BACKSPACE && isHorizontalEdge( rootNode, true ) ) || - ( keyCode === DELETE && isHorizontalEdge( rootNode, false ) ) + getSelection().isCollapsed && ( + ( keyCode === BACKSPACE && isHorizontalEdge( rootNode, true ) ) || + ( keyCode === DELETE && isHorizontalEdge( rootNode, false ) ) + ) ) { if ( ! this.props.onMerge && ! this.props.onRemove ) { return; diff --git a/test/e2e/specs/__snapshots__/splitting-merging.test.js.snap b/test/e2e/specs/__snapshots__/splitting-merging.test.js.snap index c7ddb36297de9..3a3351fc00b8d 100644 --- a/test/e2e/specs/__snapshots__/splitting-merging.test.js.snap +++ b/test/e2e/specs/__snapshots__/splitting-merging.test.js.snap @@ -1,18 +1,28 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`splitting and merging blocks Should delete an empty first line 1`] = ` +exports[`splitting and merging blocks should delete an empty first line 1`] = ` "

" `; -exports[`splitting and merging blocks Should merge into inline boundary position 1`] = ` +exports[`splitting and merging blocks should merge into inline boundary position 1`] = ` "

Bar

" `; -exports[`splitting and merging blocks Should split and merge paragraph blocks using Enter and Backspace 1`] = ` +exports[`splitting and merging blocks should not merge paragraphs if the selection is not collapsed 1`] = ` +" +

Foo

+ + + +

+" +`; + +exports[`splitting and merging blocks should split and merge paragraph blocks using Enter and Backspace 1`] = ` "

First

@@ -22,13 +32,13 @@ exports[`splitting and merging blocks Should split and merge paragraph blocks us " `; -exports[`splitting and merging blocks Should split and merge paragraph blocks using Enter and Backspace 2`] = ` +exports[`splitting and merging blocks should split and merge paragraph blocks using Enter and Backspace 2`] = ` "

FirstBetweenSecond

" `; -exports[`splitting and merging blocks Should split and merge paragraph blocks using Enter and Backspace 3`] = ` +exports[`splitting and merging blocks should split and merge paragraph blocks using Enter and Backspace 3`] = ` "

First

diff --git a/test/e2e/specs/splitting-merging.test.js b/test/e2e/specs/splitting-merging.test.js index 11c82b90b0fe3..5611dfc86ea11 100644 --- a/test/e2e/specs/splitting-merging.test.js +++ b/test/e2e/specs/splitting-merging.test.js @@ -17,7 +17,7 @@ describe( 'splitting and merging blocks', () => { await newPost(); } ); - it( 'Should split and merge paragraph blocks using Enter and Backspace', async () => { + it( 'should split and merge paragraph blocks using Enter and Backspace', async () => { // Use regular inserter to add paragraph block and text await insertBlock( 'Paragraph' ); await page.keyboard.type( 'FirstSecond' ); @@ -55,7 +55,7 @@ describe( 'splitting and merging blocks', () => { expect( await getEditedPostContent() ).toMatchSnapshot(); } ); - it( 'Should merge into inline boundary position', async () => { + it( 'should merge into inline boundary position', async () => { // Regression Test: Caret should reset to end of inline boundary when // backspacing to delete second paragraph. await insertBlock( 'Paragraph' ); @@ -71,7 +71,7 @@ describe( 'splitting and merging blocks', () => { expect( await getEditedPostContent() ).toMatchSnapshot(); } ); - it( 'Should delete an empty first line', async () => { + it( 'should delete an empty first line', async () => { // Regression Test: When a paragraph block has line break, and the first // line has no text, pressing backspace at the start of the second line // should remove the first. @@ -87,4 +87,18 @@ describe( 'splitting and merging blocks', () => { expect( await getEditedPostContent() ).toMatchSnapshot(); } ); + + it( 'should not merge paragraphs if the selection is not collapsed', async () => { + await insertBlock( 'Paragraph' ); + await page.keyboard.type( 'Foo' ); + await insertBlock( 'Paragraph' ); + await page.keyboard.type( 'Bar' ); + + await page.keyboard.down( 'Shift' ); + await pressTimes( 'ArrowLeft', 3 ); + await page.keyboard.up( 'Shift' ); + await page.keyboard.press( 'Backspace' ); + + expect( await getEditedPostContent() ).toMatchSnapshot(); + } ); } );