Fix: clean up dangling text in YJS

facebook · Oct 29, 2024 · 4fbe7fe · 4fbe7fe
1 parent 0d062f6
commit 4fbe7fe
Show file tree

Hide file tree

Showing 2 changed files with 99 additions and 24 deletions.
diff --git a/packages/lexical-playground/__tests__/e2e/Collaboration.spec.mjs b/packages/lexical-playground/__tests__/e2e/Collaboration.spec.mjs
@@ -231,7 +231,7 @@ test.describe('Collaboration', () => {
     });
   });
 
-  test('Undo with two collaborators editing same paragraph', async ({
+  test('Remove dangling text from YJS when there is no preceding text node', async ({
     isRichText,
     page,
     isCollab,
@@ -273,11 +273,12 @@ test.describe('Collaboration', () => {
       `,
     );
 
-    // Left collaborator undoes their second paragraph
-    await sleep(1050);
+    // Left collaborator undoes their text in the second paragraph.
+    await sleep(50);
     await page.frameLocator('iframe[name="left"]').getByLabel('Undo').click();
 
-    // Only left collaborator's text should have been undone
+    // The undo also removed the text node from YJS.
+    // Check that the dangling text from right user was also removed.
     await assertHTML(
       page,
       html`
@@ -286,11 +287,7 @@ test.describe('Collaboration', () => {
           dir="ltr">
           <span data-lexical-text="true">Line 1</span>
         </p>
-        <p
-          class="PlaygroundEditorTheme__paragraph PlaygroundEditorTheme__ltr"
-          dir="ltr">
-          <span data-lexical-text="true">Word</span>
-        </p>
+        <p class="PlaygroundEditorTheme__paragraph"><br /></p>
       `,
     );
 
@@ -310,10 +307,84 @@ test.describe('Collaboration', () => {
           dir="ltr">
           <span data-lexical-text="true">Line 1</span>
         </p>
+        <p class="PlaygroundEditorTheme__paragraph"><br /></p>
+      `,
+    );
+  });
+
+  test('Merge dangling text into preceding text node', async ({
+    isRichText,
+    page,
+    isCollab,
+    browserName,
+  }) => {
+    // test.skip(!isCollab || IS_MAC);
+    test.skip(!isCollab);
+
+    // Left collaborator types two pieces of text in the same paragraph, but with different styling.
+    await focusEditor(page);
+    await page.keyboard.type('normal');
+    await sleep(1050);
+    await toggleBold(page);
+    await page.keyboard.type('bold');
+
+    // Right collaborator types at the end of the paragraph.
+    await sleep(50);
+    await page
+      .frameLocator('iframe[name="right"]')
+      .locator('[data-lexical-editor="true"]')
+      .focus();
+    await page.keyboard.press('ArrowDown'); // Move caret to end of paragraph
+    await page.keyboard.type('BOLD');
+
+    await assertHTML(
+      page,
+      html`
+        <p
+          class="PlaygroundEditorTheme__paragraph PlaygroundEditorTheme__ltr"
+          dir="ltr">
+          <span data-lexical-text="true">normal</span>
+          <strong
+            class="PlaygroundEditorTheme__textBold"
+            data-lexical-text="true">
+            boldBOLD
+          </strong>
+        </p>
+      `,
+    );
+
+    // Left collaborator undoes their bold text.
+    await sleep(50);
+    await page.frameLocator('iframe[name="left"]').getByLabel('Undo').click();
+
+    // The undo also removed bold the text node from YJS.
+    // Check that the dangling text from right user was merged into the preceding text node.
+    await assertHTML(
+      page,
+      html`
+        <p
+          class="PlaygroundEditorTheme__paragraph PlaygroundEditorTheme__ltr"
+          dir="ltr">
+          <span data-lexical-text="true">normalBOLD</span>
+        </p>
+      `,
+    );
+
+    // Left collaborator refreshes their page
+    await page.evaluate(() => {
+      document
+        .querySelector('iframe[name="left"]')
+        .contentDocument.location.reload();
+    });
+
+    // Page content should be the same as before the refresh
+    await assertHTML(
+      page,
+      html`
         <p
           class="PlaygroundEditorTheme__paragraph PlaygroundEditorTheme__ltr"
           dir="ltr">
-          <span data-lexical-text="true">Word</span>
+          <span data-lexical-text="true">normalBOLD</span>
         </p>
       `,
     );

diff --git a/packages/lexical-yjs/src/CollabElementNode.ts b/packages/lexical-yjs/src/CollabElementNode.ts
@@ -157,21 +157,25 @@ export class CollabElementNode {
               nodeIndex !== 0 ? children[nodeIndex - 1] : null;
             const nodeSize = node.getSize();
 
-            if (
-              offset === 0 &&
-              delCount === 1 &&
-              nodeIndex > 0 &&
-              prevCollabNode instanceof CollabTextNode &&
-              length === nodeSize &&
-              // If the node has no keys, it's been deleted
-              Array.from(node._map.keys()).length === 0
-            ) {
-              // Merge the text node with previous.
-              prevCollabNode._text += node._text;
-              children.splice(nodeIndex, 1);
-            } else if (offset === 0 && delCount === nodeSize) {
-              // The entire thing needs removing
+            if (offset === 0 && length === nodeSize) {
+              // Text node has been deleted.
               children.splice(nodeIndex, 1);
+              // If this was caused by an undo from YJS, there could be dangling text.
+              const danglingText = spliceString(
+                node._text,
+                offset,
+                delCount - 1,
+                '',
+              );
+              if (danglingText.length > 0) {
+                if (prevCollabNode instanceof CollabTextNode) {
+                  // Merge the text node with previous.
+                  prevCollabNode._text += danglingText;
+                } else {
+                  // No previous text node to merge into, just delete the text.
+                  this._xmlText.delete(offset, danglingText.length);
+                }
+              }
             } else {
               node._text = spliceString(node._text, offset, delCount, '');
             }