Rich Text: Only merge RichText on collapsed selection

packages/editor/src/components/rich-text/index.js

@@ -82,6 +82,7 @@ export class RichText extends Component {
 		this.onFocus = this.onFocus.bind( this );
 		this.onChange = this.onChange.bind( this );
 		this.onNodeChange = this.onNodeChange.bind( this );
+		this.onDeleteKeyDown = this.onDeleteKeyDown.bind( this );
 		this.onHorizontalNavigationKeyDown = this.onHorizontalNavigationKeyDown.bind( this );
 		this.onKeyDown = this.onKeyDown.bind( this );
 		this.onKeyUp = this.onKeyUp.bind( this );
@@ -427,41 +428,68 @@ export class RichText extends Component {
 	}
 
 	/**
-	 * Handles a keydown event from TinyMCE.
+	 * Handles a delete key down event to handle merge or removal for a
+	 * collapsed selection where caret is at directional edge: forward for a
+	 * delete key down, reverse for a backspace key down.
 	 *
-	 * @param {KeydownEvent} event The keydown event as triggered by TinyMCE.
+	 * @param {tinymce.EditorEvent<KeyboardEvent>} event Keydown event.
 	 */
-	onKeyDown( event ) {
-		const dom = this.editor.dom;
-		const rootNode = this.editor.getBody();
+	onDeleteKeyDown( event ) {
+		const { onMerge, onRemove } = this.props;
+		if ( ! onMerge && ! onRemove ) {
+			return;
+		}
+
 		const { keyCode } = event;
+		const isReverse = keyCode === BACKSPACE;
 
-		if (
-			getSelection().isCollapsed && (
-				( keyCode === BACKSPACE && isHorizontalEdge( rootNode, true ) ) ||
-				( keyCode === DELETE && isHorizontalEdge( rootNode, false ) )
-			)
-		) {
-			if ( ! this.props.onMerge && ! this.props.onRemove ) {
-				return;
-			}
+		// Only process delete if occurs at uncollapsed edge.
+		const isEdge = (
+			this.editor.selection.isCollapsed() &&
+			isHorizontalEdge( this.editor.getBody(), isReverse )
+		);
+		if ( ! isEdge ) {
+			return;
+		}
 
-			const forward = keyCode === DELETE;
+		let isHandled = false;
 
-			if ( this.props.onMerge ) {
-				this.props.onMerge( forward );
-			}
+		if ( onMerge ) {
+			onMerge( ! isReverse );
+			isHandled = true;
+		}
 
-			if ( this.props.onRemove && this.isEmpty() ) {
-				this.props.onRemove( forward );
-			}
+		if ( onRemove && this.isEmpty() ) {
+			onRemove( ! isReverse );
+			isHandled = true;
+		}
 
-			event.preventDefault();
+		// Only prevent default behaviors if handled.
+		if ( ! isHandled ) {
+			return;
+		}
+
+		event.preventDefault();
+
+		// Calling onMerge() or onRemove() will destroy the editor, so it's
+		// important that we stop other handlers (e.g. ones registered by
+		// TinyMCE) from also handling this event.
+		event.stopImmediatePropagation();
+	}
+
+	/**
+	 * Handles a keydown event from TinyMCE.
+	 *
+	 * @param {KeydownEvent} event The keydown event as triggered by TinyMCE.
+	 */
+	onKeyDown( event ) {
+		const dom = this.editor.dom;
+		const rootNode = this.editor.getBody();
+		const { keyCode } = event;
 
-			// Calling onMerge() or onRemove() will destroy the editor, so it's important
-			// that we stop other handlers (e.g. ones registered by TinyMCE) from
-			// also handling this event.
-			event.stopImmediatePropagation();
+		const isDelete = keyCode === DELETE || keyCode === BACKSPACE;
+		if ( isDelete ) {
+			this.onDeleteKeyDown( event );
 		}
 
 		const isHorizontalNavigation = keyCode === LEFT || keyCode === RIGHT;

test/e2e/specs/__snapshots__/splitting-merging.test.js.snap

@@ -1,18 +1,14 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`splitting and merging blocks Should remove empty paragraph block on backspace 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`] = `
-"<!-- wp:paragraph -->
-<p>Bar</p>
-<!-- /wp:paragraph -->"
-`;
-
-exports[`splitting and merging blocks should not merge paragraphs if the selection is not collapsed 1`] = `
+exports[`splitting and merging blocks should delete wholly-selected block contents 1`] = `
 "<!-- wp:paragraph -->
 <p>Foo</p>
 <!-- /wp:paragraph -->
@@ -22,6 +18,14 @@ exports[`splitting and merging blocks should not merge paragraphs if the selecti
 <!-- /wp:paragraph -->"
 `;
 
+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 remove empty paragraph block on backspace 1`] = `""`;
+
 exports[`splitting and merging blocks should split and merge paragraph blocks using Enter and Backspace 1`] = `
 "<!-- wp:paragraph -->
 <p>First</p>

test/e2e/specs/splitting-merging.test.js

@@ -88,15 +88,43 @@ describe( 'splitting and merging blocks', () => {
 		expect( await getEditedPostContent() ).toMatchSnapshot();
 	} );
 
-	it( 'should not merge paragraphs if the selection is not collapsed', async () => {
+	it( 'should delete wholly-selected block contents', async () => {
+		// Regression Test: When all of a paragraph is selected, pressing
+		// backspace should delete the contents, not merge to previous.
+		//
+		// See: https://github.com/WordPress/gutenberg/issues/8268
 		await insertBlock( 'Paragraph' );
 		await page.keyboard.type( 'Foo' );
 		await insertBlock( 'Paragraph' );
 		await page.keyboard.type( 'Bar' );
 
+		// Select text.
 		await page.keyboard.down( 'Shift' );
 		await pressTimes( 'ArrowLeft', 3 );
 		await page.keyboard.up( 'Shift' );
+
+		// Delete selection.
+		await page.keyboard.press( 'Backspace' );
+		expect( await getEditedPostContent() ).toMatchSnapshot();
+	} );
+
+	it( 'should remove empty paragraph block on backspace', async () => {
+		// Regression Test: In a sole empty paragraph, pressing backspace
+		// should remove the block.
+		//
+		// See: https://github.com/WordPress/gutenberg/pull/8306
+		await insertBlock( 'Paragraph' );
+		await page.keyboard.press( 'Backspace' );
+
+		expect( await getEditedPostContent() ).toMatchSnapshot();
+	} );
+
+	it( 'Should remove empty paragraph block on backspace', async () => {
+		// Regression Test: In a sole empty paragraph, pressing backspace
+		// should remove the block.
+		//
+		// See: https://github.com/WordPress/gutenberg/pull/8306
+		await insertBlock( 'Paragraph' );
 		await page.keyboard.press( 'Backspace' );
 
 		expect( await getEditedPostContent() ).toMatchSnapshot();

test/e2e/support/utils.js

@@ -77,6 +77,34 @@ async function login() {
 	] );
 }
 
+/**
+ * Returns a promise which resolves once it's determined that the active DOM
+ * element is not within a RichText field, or the RichText field's TinyMCE has
+ * completed initialization. This is an unfortunate workaround to address an
+ * issue where TinyMCE takes its time to become ready for user input.
+ *
+ * TODO: This is a code smell, indicating that "too fast" resulting in breakage
+ * could be equally problematic for a fast human. It should be explored whether
+ * all event bindings we assign to TinyMCE to handle could be handled through
+ * the DOM directly instead.
+ *
+ * @return {Promise} Promise resolving once RichText is initialized, or is
+ *                   determined to not be a container of the active element.
+ */
+async function waitForRichTextInitialization() {
+	const isInRichText = await page.evaluate( () => {
+		return !! document.activeElement.closest( '.editor-rich-text__tinymce' );
+	} );
+
+	if ( ! isInRichText ) {
+		return;
+	}
+
+	return page.waitForFunction( () => {
+		return !! document.activeElement.closest( '.mce-content-body' );
+	} );
+}
+
 export async function visitAdmin( adminPath, query ) {
 	await goToWPPath( join( 'wp-admin', adminPath ), query );
 
@@ -206,6 +234,7 @@ export async function insertBlock( searchTerm, panelName = null ) {
 		await panelButton.click();
 	}
 	await page.click( `button[aria-label="${ searchTerm }"]` );
+	await waitForRichTextInitialization();
 }
 
 /**