fix(editor): assign new keys to annotations split across blocks

packages/editor/e2e-tests/__tests__/annotations-across-blocks.feature

@@ -46,16 +46,64 @@ Feature: Annotations Across Blocks
     And "bar" has marks "l2"
     And "foo,\n,image,\n,bar" is selected
 
-  # Warning: Possible wrong behaviour
-  # The "bar" link should have a unique key
   Scenario: Splitting an annotation across blocks
     Given the text "foobar"
     And a "link" "l1" around "foobar"
     When the caret is put after "foo"
     And "Enter" is pressed
     Then the text is "foo,\n,bar"
     And "foo" has marks "l1"
-    And "bar" has marks "l1"
+    And "bar" has an annotation different than "l1"
+
+  Scenario: Splitting an annotation across blocks using a selection
+    Given the text "foo bar baz"
+    And a "link" "l1" around "foo bar baz"
+    When "bar" is selected
+    And "Enter" is pressed
+    Then the text is "foo ,\n, baz"
+    And "foo " has marks "l1"
+    And " baz" has an annotation different than "l1"
+
+  Scenario: Splitting a split annotation across blocks
+    Given the text "foo bar baz"
+    And a "link" "l1" around "foo bar baz"
+    And "strong" around "bar"
+    When the caret is put after "foo"
+    And "Enter" is pressed
+    Then the text is "foo,\n, ,bar, baz"
+    And "foo" has marks "l1"
+    And " " has an annotation different than "l1"
+    And "bar" has an annotation different than "l1"
+    And " baz" has an annotation different than "l1"
+    And " " and " baz" have the same marks
+
+  Scenario: Splitting text before annotation doesn't touch the annotation
+    Given the text "foo bar baz"
+    And a "link" "l1" around "baz"
+    When the caret is put before "bar"
+    And "Enter" is pressed
+    Then the text is "foo ,\n,bar ,baz"
+    And "baz" has marks "l1"
+
+  Scenario: Splitting text after annotation doesn't touch the annotation
+    Given the text "foo bar baz"
+    And a "link" "l1" around "foo"
+    When the caret is put after "bar"
+    And "Enter" is pressed
+    Then the text is "foo, bar,\n, baz"
+    And "foo" has marks "l1"
+
+  # Warning: Possible wrong behaviour
+  # "foo" and "bar" should rejoin as one link
+  Scenario: Splitting and merging an annotation across blocks
+    Given the text "foobar"
+    And a "link" "l1" around "foobar"
+    When the caret is put after "foo"
+    And "Enter" is pressed
+    And "Backspace" is pressed
+    Then the text is "foo,bar"
+    And "foo" has marks "l1"
+    And "bar" has an annotation different than "l1"
 
   # Warning: Possible wrong behaviour
   # The " baz" link should have a unique key

packages/editor/e2e-tests/__tests__/gherkin-step-definitions.ts

@@ -281,6 +281,29 @@ export const stepDefinitions = [
       })
     },
   ),
+  Then(
+    '{string} has an annotation different than {key}',
+    async ({editorA, keyMap}: Context, text: string, key: string) => {
+      await getEditorTextMarks(editorA, text).then((marks) => {
+        const annotations =
+          marks?.filter((mark) => mark !== 'em' && mark !== 'strong') ?? []
+
+        expect(annotations.length).toBeGreaterThan(0)
+        expect(
+          annotations.some((annotation) => annotation === keyMap.get(key)),
+        ).toBeFalsy()
+      })
+    },
+  ),
+  Then(
+    '{string} and {string} have the same marks',
+    async ({editorA}: Context, textA: string, textB: string) => {
+      const marksA = await getEditorTextMarks(editorA, textA)
+      const marksB = await getEditorTextMarks(editorA, textB)
+
+      expect(marksA).toEqual(marksB)
+    },
+  ),
   Then('{string} has no marks', async ({editorA}: Context, text: string) => {
     await getEditorTextMarks(editorA, text).then((marks) =>
       expect(marks).toEqual([]),

packages/editor/src/editor/plugins/createWithInsertBreak.ts

@@ -1,3 +1,4 @@
+import {isEqual} from 'lodash'
 import {Editor, Node, Path, Range, Transforms} from 'slate'
 import {
   type PortableTextMemberSchemaTypes,
@@ -7,14 +8,15 @@ import {type SlateTextBlock, type VoidElement} from '../../types/slate'
 
 export function createWithInsertBreak(
   types: PortableTextMemberSchemaTypes,
+  keyGenerator: () => string,
 ): (editor: PortableTextSlateEditor) => PortableTextSlateEditor {
   return function withInsertBreak(
     editor: PortableTextSlateEditor,
   ): PortableTextSlateEditor {
     const {insertBreak} = editor
 
     editor.insertBreak = () => {
-      if (!editor.selection || Range.isExpanded(editor.selection)) {
+      if (!editor.selection) {
         insertBreak()
         return
       }
@@ -25,10 +27,10 @@ export function createWithInsertBreak(
         | VoidElement
 
       if (editor.isTextBlock(focusBlock)) {
-        const [, end] = Range.edges(editor.selection)
+        const [start, end] = Range.edges(editor.selection)
         const isEndAtStartOfNode = Editor.isStart(editor, end, end.path)
 
-        if (isEndAtStartOfNode) {
+        if (isEndAtStartOfNode && Range.isCollapsed(editor.selection)) {
           const focusDecorators = editor.isTextSpan(focusBlock.children[0])
             ? (focusBlock.children[0].marks ?? []).filter((mark) =>
                 types.decorators.some((decorator) => decorator.value === mark),
@@ -50,6 +52,113 @@ export function createWithInsertBreak(
           editor.onChange()
           return
         }
+
+        const isStartAtEndOfNode = Editor.isEnd(editor, start, start.path)
+        const isInTheMiddleOfNode = !isEndAtStartOfNode && !isStartAtEndOfNode
+
+        if (isInTheMiddleOfNode) {
+          Editor.withoutNormalizing(editor, () => {
+            if (!editor.selection) {
+              return
+            }
+
+            Transforms.splitNodes(editor, {
+              at: editor.selection,
+            })
+
+            const [nextNode, nextNodePath] = Editor.node(
+              editor,
+              Path.next(focusBlockPath),
+              {depth: 1},
+            )
+
+            /**
+             * Assign new keys to markDefs that are now split across two blocks
+             */
+            if (
+              editor.isTextBlock(nextNode) &&
+              nextNode.markDefs &&
+              nextNode.markDefs.length > 0
+            ) {
+              const newMarkDefKeys = new Map<string, string>()
+
+              const prevNodeSpans = Array.from(
+                Node.children(editor, focusBlockPath),
+              )
+                .map((entry) => entry[0])
+                .filter((node) => editor.isTextSpan(node))
+              const children = Node.children(editor, nextNodePath)
+
+              for (const [child, childPath] of children) {
+                if (!editor.isTextSpan(child)) {
+                  continue
+                }
+
+                const marks = child.marks ?? []
+
+                // Go through the marks of the span and figure out if any of
+                // them refer to annotations that are also present in the
+                // previous block
+                for (const mark of marks) {
+                  if (
+                    types.decorators.some(
+                      (decorator) => decorator.value === mark,
+                    )
+                  ) {
+                    continue
+                  }
+
+                  if (
+                    prevNodeSpans.some((prevNodeSpan) =>
+                      prevNodeSpan.marks?.includes(mark),
+                    ) &&
+                    !newMarkDefKeys.has(mark)
+                  ) {
+                    // This annotation is both present in the previous block
+                    // and this block, so let's assign a new key to it
+                    newMarkDefKeys.set(mark, keyGenerator())
+                  }
+                }
+
+                const newMarks = marks.map(
+                  (mark) => newMarkDefKeys.get(mark) ?? mark,
+                )
+
+                // No need to update the marks if they are the same
+                if (!isEqual(marks, newMarks)) {
+                  Transforms.setNodes(
+                    editor,
+                    {marks: newMarks},
+                    {
+                      at: childPath,
+                    },
+                  )
+                }
+              }
+
+              // Time to update all the markDefs that need a new key because
+              // they've been split across blocks
+              const newMarkDefs = nextNode.markDefs.map((markDef) => ({
+                ...markDef,
+                _key: newMarkDefKeys.get(markDef._key) ?? markDef._key,
+              }))
+
+              // No need to update the markDefs if they are the same
+              if (!isEqual(nextNode.markDefs, newMarkDefs)) {
+                Transforms.setNodes(
+                  editor,
+                  {markDefs: newMarkDefs},
+                  {
+                    at: nextNodePath,
+                    match: (node) => editor.isTextBlock(node),
+                  },
+                )
+              }
+            }
+          })
+          editor.onChange()
+          return
+        }
       }
 
       insertBreak()

packages/editor/src/editor/plugins/index.ts

@@ -95,7 +95,7 @@ export const withPlugins = <T extends Editor>(
 
   const withPlaceholderBlock = createWithPlaceholderBlock()
 
-  const withInsertBreak = createWithInsertBreak(schemaTypes)
+  const withInsertBreak = createWithInsertBreak(schemaTypes, keyGenerator)
 
   const withUtils = createWithUtils({
     keyGenerator,