[lexical-yjs] Bug Fix: clean up dangling text after undo in collabora…

…tion (#6670) Co-authored-by: James Fitzsimmons <[email protected]> Co-authored-by: James Fitzsimmons <[email protected]> Co-authored-by: Bob Ippolito <[email protected]>
facebook · Nov 5, 2024 · dd65f59 · dd65f59
1 parent fc1bea0
commit dd65f59
Show file tree

Hide file tree

Showing 2 changed files with 175 additions and 14 deletions.
diff --git a/packages/lexical-playground/__tests__/e2e/Collaboration.spec.mjs b/packages/lexical-playground/__tests__/e2e/Collaboration.spec.mjs
@@ -230,4 +230,161 @@ test.describe('Collaboration', () => {
       focusPath: [1, 1, 0],
     });
   });
+
+  test('Remove dangling text from YJS when there is no preceding text node', async ({
+    isRichText,
+    page,
+    isCollab,
+    browserName,
+  }) => {
+    test.skip(!isCollab);
+
+    // Left collaborator types two paragraphs of text
+    await focusEditor(page);
+    await page.keyboard.type('Line 1');
+    await page.keyboard.press('Enter');
+    await sleep(1050); // default merge interval is 1000, add 50ms as overhead due to CI latency.
+    await page.keyboard.type('This is a test. ');
+
+    // Right collaborator types at the end of paragraph 2
+    await sleep(1050);
+    await page
+      .frameLocator('iframe[name="right"]')
+      .locator('[data-lexical-editor="true"]')
+      .focus();
+    await page.keyboard.press('ArrowDown'); // Move caret to end of paragraph 2
+    await page.keyboard.press('ArrowDown');
+    await page.keyboard.type('Word');
+
+    await assertHTML(
+      page,
+      html`
+        <p
+          class="PlaygroundEditorTheme__paragraph PlaygroundEditorTheme__ltr"
+          dir="ltr">
+          <span data-lexical-text="true">Line 1</span>
+        </p>
+        <p
+          class="PlaygroundEditorTheme__paragraph PlaygroundEditorTheme__ltr"
+          dir="ltr">
+          <span data-lexical-text="true">This is a test. Word</span>
+        </p>
+      `,
+    );
+
+    // Left collaborator undoes their text in the second paragraph.
+    await sleep(50);
+    await page.frameLocator('iframe[name="left"]').getByLabel('Undo').click();
+
+    // The undo also removed the text node from YJS.
+    // Check that the dangling text from right user was also removed.
+    await assertHTML(
+      page,
+      html`
+        <p
+          class="PlaygroundEditorTheme__paragraph PlaygroundEditorTheme__ltr"
+          dir="ltr">
+          <span data-lexical-text="true">Line 1</span>
+        </p>
+        <p class="PlaygroundEditorTheme__paragraph"><br /></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">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);
+
+    // 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">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, '');
             }