RichText: don't use DOM to add line padding

WordPress · Feb 20, 2019 · 4116973 · 4116973
1 parent 9f5f1c3
commit 4116973
Show file tree

Hide file tree

Showing 7 changed files with 53 additions and 66 deletions.
diff --git a/packages/e2e-tests/specs/__snapshots__/writing-flow.test.js.snap b/packages/e2e-tests/specs/__snapshots__/writing-flow.test.js.snap
@@ -72,7 +72,7 @@ exports[`adding blocks should create valid paragraph blocks when rapidly pressin
 
 exports[`adding blocks should insert line break at end 1`] = `
 "<!-- wp:paragraph -->
-<p>a<br><br></p>
+<p>a<br></p>
 <!-- /wp:paragraph -->"
 `;
 
@@ -90,7 +90,7 @@ exports[`adding blocks should insert line break at start 1`] = `
 
 exports[`adding blocks should insert line break in empty container 1`] = `
 "<!-- wp:paragraph -->
-<p><br><br></p>
+<p><br></p>
 <!-- /wp:paragraph -->"
 `;
 

diff --git a/packages/e2e-tests/specs/blocks/__snapshots__/list.test.js.snap b/packages/e2e-tests/specs/blocks/__snapshots__/list.test.js.snap
@@ -146,13 +146,13 @@ exports[`List should indent and outdent level 2 3`] = `
 
 exports[`List should insert a line break on shift+enter 1`] = `
 "<!-- wp:list -->
-<ul><li>a<br><br></li></ul>
+<ul><li>a<br></li></ul>
 <!-- /wp:list -->"
 `;
 
 exports[`List should insert a line break on shift+enter in a non trailing list item 1`] = `
 "<!-- wp:list -->
-<ul><li>a</li><li>b<br><br></li><li>c</li></ul>
+<ul><li>a</li><li>b<br></li><li>c</li></ul>
 <!-- /wp:list -->"
 `;
 

diff --git a/packages/rich-text/src/insert-line-break.js b/packages/rich-text/src/insert-line-break.js
@@ -3,32 +3,14 @@
  */
 
 import { insert } from './insert';
-import { LINE_SEPARATOR } from './special-characters';
 
 /**
- * Inserts a line break at the given or selected position. Inserts two line
- * breaks if at the end of a line.
+ * Inserts a line break at the given or selected position.
  *
  * @param {Object} value Value to modify.
  *
- * @return {Object} The value with the line break(s) inserted.
+ * @return {Object} The value with the line break inserted.
  */
 export function insertLineBreak( value ) {
-	const { text, end } = value;
-	const length = text.length;
-
-	let toInsert = '\n';
-
-	// If the caret is at the end of the text, and there is no
-	// trailing line break or no text at all, we have to insert two
-	// line breaks in order to create a new line visually and place
-	// the caret there.
-	if (
-		( end === length || text[ end ] === LINE_SEPARATOR ) &&
-		( text[ end - 1 ] !== '\n' || length === 0 )
-	) {
-		toInsert = '\n\n';
-	}
-
-	return insert( value, toInsert );
+	return insert( value, '\n' );
 }
diff --git a/packages/rich-text/src/test/__snapshots__/to-dom.js.snap b/packages/rich-text/src/test/__snapshots__/to-dom.js.snap
@@ -134,6 +134,9 @@ exports[`recordToDom should handle br 1`] = `
   
   <br />
   
+  <br
+    data-rich-text-padding="true"
+  />
 </body>
 `;
 
@@ -147,6 +150,9 @@ exports[`recordToDom should handle br with formatting 1`] = `
     
   </em>
   
+  <br
+    data-rich-text-padding="true"
+  />
 </body>
 `;
 
@@ -197,12 +203,14 @@ exports[`recordToDom should handle middle empty list value 1`] = `
     <br
       data-rich-text-padding="true"
     />
+    
   </li>
   <li>
     
     <br
       data-rich-text-padding="true"
     />
+    
   </li>
   <li>
     
@@ -276,6 +284,7 @@ exports[`recordToDom should handle multiline value with empty 1`] = `
 exports[`recordToDom should handle nested empty list value 1`] = `
 <body>
   <li>
+    
     <br
       data-rich-text-padding="true"
     />

diff --git a/packages/rich-text/src/test/helpers/index.js b/packages/rich-text/src/test/helpers/index.js
@@ -459,8 +459,8 @@ export const spec = [
 			endOffset: 0,
 			endContainer: element.querySelector( 'ul > li' ),
 		} ),
-		startPath: [ 0, 0, 0, 0, 0 ],
-		endPath: [ 0, 0, 0, 0, 0 ],
+		startPath: [ 0, 2, 0, 0, 0 ],
+		endPath: [ 0, 2, 0, 0, 0 ],
 		record: {
 			start: 1,
 			end: 1,
@@ -479,8 +479,8 @@ export const spec = [
 			endOffset: 0,
 			endContainer: element.firstChild.nextSibling,
 		} ),
-		startPath: [ 1, 0, 0 ],
-		endPath: [ 1, 0, 0 ],
+		startPath: [ 1, 2, 0 ],
+		endPath: [ 1, 2, 0 ],
 		record: {
 			start: 1,
 			end: 1,

diff --git a/packages/rich-text/src/to-dom.js b/packages/rich-text/src/to-dom.js
@@ -113,39 +113,6 @@ function remove( node ) {
 	return node.parentNode.removeChild( node );
 }
 
-function createLinePadding( doc ) {
-	const element = doc.createElement( 'br' );
-	element.setAttribute( 'data-rich-text-padding', 'true' );
-	return element;
-}
-
-function padEmptyLines( { element, multilineWrapperTags } ) {
-	const length = element.childNodes.length;
-	const doc = element.ownerDocument;
-
-	for ( let index = 0; index < length; index++ ) {
-		const child = element.childNodes[ index ];
-
-		if ( child.nodeType === TEXT_NODE ) {
-			if ( length === 1 && ! child.nodeValue ) {
-				// Pad if the only child is an empty text node.
-				element.appendChild( createLinePadding( doc ) );
-			}
-		} else {
-			if (
-				multilineWrapperTags &&
-				! child.previousSibling &&
-				multilineWrapperTags.indexOf( child.nodeName.toLowerCase() ) !== -1
-			) {
-				// Pad the line if there is no content before a nested wrapper.
-				element.insertBefore( createLinePadding( doc ), child );
-			}
-
-			padEmptyLines( { element: child, multilineWrapperTags } );
-		}
-	}
-}
-
 function prepareFormats( prepareEditableTree = [], value ) {
 	return prepareEditableTree.reduce( ( accumlator, fn ) => {
 		return fn( accumlator, value.text );
@@ -186,10 +153,6 @@ export function toDom( {
 		isEditableTree,
 	} );
 
-	if ( isEditableTree ) {
-		padEmptyLines( { element: tree, multilineWrapperTags } );
-	}
-
 	return {
 		body: tree,
 		selection: { startPath, endPath },

diff --git a/packages/rich-text/src/to-tree.js b/packages/rich-text/src/to-tree.js
@@ -80,6 +80,14 @@ function getDeepestActiveFormat( value ) {
 	return activeFormats[ selectedFormat - 1 ];
 }
 
+const padding = {
+	type: 'br',
+	attributes: {
+		'data-rich-text-padding': 'true',
+	},
+	object: true,
+};
+
 export function toTree( {
 	value,
 	multilineTag,
@@ -136,6 +144,23 @@ export function toTree( {
 
 		let pointer = getLastChild( tree );
 
+		if ( isEditableTree && character === LINE_SEPARATOR ) {
+			let node = pointer;
+
+			while ( ! isText( node ) ) {
+				node = getLastChild( node );
+			}
+
+			if (
+				! lastCharacter ||
+				lastCharacter === LINE_SEPARATOR ||
+				lastCharacter === '\n'
+			) {
+				append( getParent( node ), padding );
+				append( getParent( node ), '' );
+			}
+		}
+
 		// Set selection for the start of line.
 		if ( lastCharacter === LINE_SEPARATOR ) {
 			let node = pointer;
@@ -232,6 +257,14 @@ export function toTree( {
 			onEndIndex( tree, pointer );
 		}
 
+		if ( i === text.length && isEditableTree && (
+			! lastCharacter ||
+			lastCharacter === LINE_SEPARATOR ||
+			lastCharacter === '\n'
+		) ) {
+			append( getParent( pointer ), padding );
+		}
+
 		lastCharacterFormats = characterFormats;
 		lastCharacter = character;
 	}