RichText: Removing/merging richtext should only trigger if the select…

…ion 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
WordPress · Aug 7, 2018 · 42b870c · 42b870c
1 parent d5eb120
commit 42b870c
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 12 deletions.
diff --git 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<KeyboardEvent>} 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
@@ -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`] = `
 "<!-- wp:paragraph -->
 <p></p>
 <!-- /wp:paragraph -->"
 `;
 
-exports[`splitting and merging blocks Should merge into inline boundary position 1`] = `
+exports[`splitting and merging blocks should merge into inline boundary position 1`] = `
 "<!-- wp:paragraph -->
 <p>Bar</p>
 <!-- /wp:paragraph -->"
 `;
 
-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`] = `
+"<!-- wp:paragraph -->
+<p>Foo</p>
+<!-- /wp:paragraph -->
+
+<!-- wp:paragraph -->
+<p></p>
+<!-- /wp:paragraph -->"
+`;
+
+exports[`splitting and merging blocks should split and merge paragraph blocks using Enter and Backspace 1`] = `
 "<!-- wp:paragraph -->
 <p>First</p>
 <!-- /wp:paragraph -->
@@ -22,13 +32,13 @@ exports[`splitting and merging blocks Should split and merge paragraph blocks us
 <!-- /wp:paragraph -->"
 `;
 
-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`] = `
 "<!-- wp:paragraph -->
 <p>FirstBetweenSecond</p>
 <!-- /wp:paragraph -->"
 `;
 
-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`] = `
 "<!-- wp:paragraph -->
 <p><strong>First</strong></p>
 <!-- /wp:paragraph -->

diff --git 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();
+	} );
 } );