RichText: don't use DOM to add line padding (#13850)

* RichText: don't use DOM to add line padding * Clean up * Keep track of user inserted line breaks * Mark isEditableTree param unstable
WordPress · Mar 20, 2019 · 5630e3c · 5630e3c
1 parent ba71cae
commit 5630e3c
Show file tree

Hide file tree

Showing 9 changed files with 91 additions and 94 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/editor/src/components/rich-text/index.js b/packages/editor/src/components/rich-text/index.js
@@ -193,6 +193,7 @@ export class RichText extends Component {
 			multilineTag: this.multilineTag,
 			multilineWrapperTags: this.multilineWrapperTags,
 			prepareEditableTree: this.props.prepareEditableTree,
+			__unstableIsEditableTree: true,
 		} );
 	}
 

diff --git a/packages/rich-text/src/create.js b/packages/rich-text/src/create.js
@@ -108,6 +108,7 @@ export function create( {
 	range,
 	multilineTag,
 	multilineWrapperTags,
+	__unstableIsEditableTree: isEditableTree,
 } = {} ) {
 	if ( typeof text === 'string' && text.length > 0 ) {
 		return {
@@ -128,6 +129,7 @@ export function create( {
 		return createFromElement( {
 			element,
 			range,
+			isEditableTree,
 		} );
 	}
 
@@ -136,6 +138,7 @@ export function create( {
 		range,
 		multilineTag,
 		multilineWrapperTags,
+		isEditableTree,
 	} );
 }
 
@@ -257,6 +260,7 @@ function createFromElement( {
 	multilineTag,
 	multilineWrapperTags,
 	currentWrapperTags = [],
+	isEditableTree,
 } ) {
 	const accumulator = createEmptyValue();
 
@@ -291,7 +295,10 @@ function createFromElement( {
 			continue;
 		}
 
-		if ( node.getAttribute( 'data-rich-text-padding' ) ) {
+		if (
+			node.getAttribute( 'data-rich-text-padding' ) ||
+			( isEditableTree && type === 'br' && ! node.getAttribute( 'data-rich-text-line-break' ) )
+		) {
 			accumulateSelection( accumulator, node, range, createEmptyValue() );
 			continue;
 		}
@@ -433,31 +440,14 @@ function createFromMultilineElement( {
 			continue;
 		}
 
-		let value = createFromElement( {
+		const value = createFromElement( {
 			element: node,
 			range,
 			multilineTag,
 			multilineWrapperTags,
 			currentWrapperTags,
 		} );
 
-		// If a line consists of one single line break (invisible), consider the
-		// line empty, wether this is the browser's doing or not.
-		if ( value.text === '\n' ) {
-			const start = value.start;
-			const end = value.end;
-
-			value = createEmptyValue();
-
-			if ( start !== undefined ) {
-				value.start = 0;
-			}
-
-			if ( end !== undefined ) {
-				value.end = 0;
-			}
-		}
-
 		// Multiline value text should be separated by a double line break.
 		if ( index !== 0 || currentWrapperTags.length > 0 ) {
 			const formats = currentWrapperTags.length > 0 ? [ currentWrapperTags ] : [ , ];
@@ -495,7 +485,7 @@ function getAttributes( { element } ) {
 	for ( let i = 0; i < length; i++ ) {
 		const { name, value } = element.attributes[ i ];
 
-		if ( name === 'data-rich-text-format-boundary' ) {
+		if ( name.indexOf( 'data-rich-text-' ) === 0 ) {
 			continue;
 		}
 

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
@@ -132,8 +132,13 @@ exports[`recordToDom should filter format boundary attributes 1`] = `
 exports[`recordToDom should handle br 1`] = `
 <body>
   
-  <br />
+  <br
+    data-rich-text-line-break="true"
+  />
   
+  <br
+    data-rich-text-padding="true"
+  />
 </body>
 `;
 
@@ -143,27 +148,38 @@ exports[`recordToDom should handle br with formatting 1`] = `
     data-rich-text-format-boundary="true"
   >
     
-    <br />
+    <br
+      data-rich-text-line-break="true"
+    />
     
   </em>
   
+  <br
+    data-rich-text-padding="true"
+  />
 </body>
 `;
 
 exports[`recordToDom should handle br with text 1`] = `
 <body>
   te
-  <br />
+  <br
+    data-rich-text-line-break="true"
+  />
   st
 </body>
 `;
 
 exports[`recordToDom should handle double br 1`] = `
 <body>
   a
-  <br />
+  <br
+    data-rich-text-line-break="true"
+  />
   
-  <br />
+  <br
+    data-rich-text-line-break="true"
+  />
   b
 </body>
 `;
@@ -197,12 +213,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 +294,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"
     />
@@ -294,9 +313,13 @@ exports[`recordToDom should handle nested empty list value 1`] = `
 exports[`recordToDom should handle selection before br 1`] = `
 <body>
   a
-  <br />
+  <br
+    data-rich-text-line-break="true"
+  />
   
-  <br />
+  <br
+    data-rich-text-line-break="true"
+  />
   b
 </body>
 `;

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 },