From d5671a1bdf04675ad539740335692056c1f670d6 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Tue, 7 Aug 2018 12:24:53 +0100 Subject: [PATCH 1/3] RichText: Removing/merging richtext should only trigger if the selection is collapsed --- packages/editor/src/components/rich-text/index.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/editor/src/components/rich-text/index.js b/packages/editor/src/components/rich-text/index.js index 06d954e7756157..7b9f04938f2e16 100644 --- a/packages/editor/src/components/rich-text/index.js +++ b/packages/editor/src/components/rich-text/index.js @@ -437,8 +437,10 @@ export class RichText extends Component { const { keyCode } = event; if ( - ( keyCode === BACKSPACE && isHorizontalEdge( rootNode, true ) ) || - ( keyCode === DELETE && isHorizontalEdge( rootNode, false ) ) + window.getSelection().isCollapsed && ( + ( keyCode === BACKSPACE && isHorizontalEdge( rootNode, true ) ) || + ( keyCode === DELETE && isHorizontalEdge( rootNode, false ) ) + ) ) { if ( ! this.props.onMerge && ! this.props.onRemove ) { return; From 06baca20552c58a1ba7ee3b748a0ae7a7e4e8b45 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Tue, 7 Aug 2018 12:29:50 +0100 Subject: [PATCH 2/3] Add e2e test to avoid uncollapsed selection to be mergedwq --- .../__snapshots__/splitting-merging.test.js.snap | 10 ++++++++++ test/e2e/specs/splitting-merging.test.js | 14 ++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/test/e2e/specs/__snapshots__/splitting-merging.test.js.snap b/test/e2e/specs/__snapshots__/splitting-merging.test.js.snap index c7ddb36297de94..a3b24f5719aaf4 100644 --- a/test/e2e/specs/__snapshots__/splitting-merging.test.js.snap +++ b/test/e2e/specs/__snapshots__/splitting-merging.test.js.snap @@ -12,6 +12,16 @@ exports[`splitting and merging blocks Should merge into inline boundary position " `; +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

diff --git a/test/e2e/specs/splitting-merging.test.js b/test/e2e/specs/splitting-merging.test.js index 11c82b90b0fe39..5fa1c65fe4c648 100644 --- a/test/e2e/specs/splitting-merging.test.js +++ b/test/e2e/specs/splitting-merging.test.js @@ -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(); + } ); } ); From 3cb871474344222b16bdee1bd2bdcaca13e4dd9d Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Tue, 7 Aug 2018 13:07:55 +0100 Subject: [PATCH 3/3] Code Style improvements per review --- packages/editor/src/components/rich-text/index.js | 6 +++--- .../__snapshots__/splitting-merging.test.js.snap | 12 ++++++------ test/e2e/specs/splitting-merging.test.js | 8 ++++---- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/packages/editor/src/components/rich-text/index.js b/packages/editor/src/components/rich-text/index.js index 7b9f04938f2e16..e8c0e187c47d29 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,7 +437,7 @@ export class RichText extends Component { const { keyCode } = event; if ( - window.getSelection().isCollapsed && ( + getSelection().isCollapsed && ( ( keyCode === BACKSPACE && isHorizontalEdge( rootNode, true ) ) || ( keyCode === DELETE && isHorizontalEdge( rootNode, false ) ) ) diff --git a/test/e2e/specs/__snapshots__/splitting-merging.test.js.snap b/test/e2e/specs/__snapshots__/splitting-merging.test.js.snap index a3b24f5719aaf4..3a3351fc00b8da 100644 --- a/test/e2e/specs/__snapshots__/splitting-merging.test.js.snap +++ b/test/e2e/specs/__snapshots__/splitting-merging.test.js.snap @@ -1,18 +1,18 @@ // 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 not merge paragraphs if the selection is not collapsed 1`] = ` +exports[`splitting and merging blocks should not merge paragraphs if the selection is not collapsed 1`] = ` "

Foo

@@ -22,7 +22,7 @@ exports[`splitting and merging blocks Should not merge paragraphs if the selecti " `; -exports[`splitting and merging blocks Should split and merge paragraph blocks using Enter and Backspace 1`] = ` +exports[`splitting and merging blocks should split and merge paragraph blocks using Enter and Backspace 1`] = ` "

First

@@ -32,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 5fa1c65fe4c648..5611dfc86ea113 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. @@ -88,7 +88,7 @@ describe( 'splitting and merging blocks', () => { expect( await getEditedPostContent() ).toMatchSnapshot(); } ); - it( 'Should not merge paragraphs if the selection is not collapsed', async () => { + it( 'should not merge paragraphs if the selection is not collapsed', async () => { await insertBlock( 'Paragraph' ); await page.keyboard.type( 'Foo' ); await insertBlock( 'Paragraph' );