fix(portable-text-editor): add autoresolving validations and fix norm…

…alization issues (#6770) * fix(portable-text-editor): add autoresolving validations and fix normalization issue This will add up front validation for orphaned marks and markDefs that can be autoresolved through patches. It will also fix some issues when it comes to adding and removing annotations, which transformations now must be built up using the withoutNormalization scope. * test(portable-text-editor): add validation and normalization tests
sanity-io · May 28, 2024 · c458289 · c458289
1 parent 7efdeeb
commit c458289
Show file tree

Hide file tree

Showing 6 changed files with 216 additions and 110 deletions.
diff --git a/packages/@sanity/portable-text-editor/src/editor/__tests__/PortableTextEditor.test.tsx b/packages/@sanity/portable-text-editor/src/editor/__tests__/PortableTextEditor.test.tsx
@@ -310,7 +310,7 @@ describe('initialization', () => {
         _key: 'abc',
         _type: 'myTestBlockType',
         markDefs: [],
-        children: [{_key: 'def', _type: 'span', text: 'Test', marks: ['invalid']}],
+        children: [{_key: 'def', _type: 'span', marks: []}],
       },
     ]
     const initialSelection: EditorSelection = {
@@ -333,15 +333,15 @@ describe('initialization', () => {
           type: 'invalidValue',
           value: initialValue,
           resolution: {
-            action: 'Remove invalid marks',
+            action: 'Write an empty text property to the object',
             description:
-              "Block with _key 'abc' contains marks (invalid) not supported by the current content model.",
+              "Child with _key 'def' in block with key 'abc' has missing or invalid text property!",
             i18n: {
-              action: 'inputs.portable-text.invalid-value.orphaned-marks.action',
-              description: 'inputs.portable-text.invalid-value.orphaned-marks.description',
+              action: 'inputs.portable-text.invalid-value.invalid-span-text.action',
+              description: 'inputs.portable-text.invalid-value.invalid-span-text.description',
               values: {
                 key: 'abc',
-                orphanedMarks: ['invalid'],
+                childKey: 'def',
               },
             },
             item: {
@@ -351,8 +351,7 @@ describe('initialization', () => {
                 {
                   _key: 'def',
                   _type: 'span',
-                  marks: ['invalid'],
-                  text: 'Test',
+                  marks: [],
                 },
               ],
               markDefs: [],
@@ -367,10 +366,14 @@ describe('initialization', () => {
                   {
                     _key: 'def',
                   },
-                  'marks',
                 ],
                 type: 'set',
-                value: [],
+                value: {
+                  _key: 'def',
+                  _type: 'span',
+                  marks: [],
+                  text: '',
+                },
               },
             ],
           },

diff --git a/packages/@sanity/portable-text-editor/src/editor/__tests__/pteWarningsSelfSolving.test.tsx b/packages/@sanity/portable-text-editor/src/editor/__tests__/pteWarningsSelfSolving.test.tsx
@@ -180,6 +180,123 @@ describe('when PTE would display warnings, instead it self solves', () => {
       }
     })
   })
+
+  it('removes orphaned marks', async () => {
+    const editorRef: RefObject<PortableTextEditor> = createRef()
+    const initialValue = [
+      {
+        _key: 'abc',
+        _type: 'myTestBlockType',
+        style: 'normal',
+        markDefs: [],
+        children: [
+          {
+            _key: 'def',
+            _type: 'span',
+            marks: ['ghi'],
+            text: 'Hello',
+          },
+        ],
+      },
+    ]
+
+    const onChange = jest.fn()
+    render(
+      <PortableTextEditorTester
+        onChange={onChange}
+        ref={editorRef}
+        schemaType={schemaType}
+        value={initialValue}
+      />,
+    )
+    await waitFor(() => {
+      expect(onChange).toHaveBeenCalledWith({type: 'value', value: initialValue})
+      expect(onChange).toHaveBeenCalledWith({type: 'ready'})
+    })
+    await waitFor(() => {
+      if (editorRef.current) {
+        PortableTextEditor.focus(editorRef.current)
+        expect(PortableTextEditor.getValue(editorRef.current)).toEqual([
+          {
+            _key: 'abc',
+            _type: 'myTestBlockType',
+            children: [
+              {
+                _key: 'def',
+                _type: 'span',
+                text: 'Hello',
+                marks: [],
+              },
+            ],
+            markDefs: [],
+            style: 'normal',
+          },
+        ])
+      }
+    })
+  })
+
+  it('removes orphaned marksDefs', async () => {
+    const editorRef: RefObject<PortableTextEditor> = createRef()
+    const initialValue = [
+      {
+        _key: 'abc',
+        _type: 'myTestBlockType',
+        style: 'normal',
+        markDefs: [
+          {
+            _key: 'ghi',
+            _type: 'link',
+            href: 'https://sanity.io',
+          },
+        ],
+        children: [
+          {
+            _key: 'def',
+            _type: 'span',
+            marks: [],
+            text: 'Hello',
+          },
+        ],
+      },
+    ]
+
+    const onChange = jest.fn()
+    render(
+      <PortableTextEditorTester
+        onChange={onChange}
+        ref={editorRef}
+        schemaType={schemaType}
+        value={initialValue}
+      />,
+    )
+    await waitFor(() => {
+      expect(onChange).toHaveBeenCalledWith({type: 'value', value: initialValue})
+      expect(onChange).toHaveBeenCalledWith({type: 'ready'})
+    })
+    await waitFor(() => {
+      if (editorRef.current) {
+        PortableTextEditor.focus(editorRef.current)
+        expect(PortableTextEditor.getValue(editorRef.current)).toEqual([
+          {
+            _key: 'abc',
+            _type: 'myTestBlockType',
+            children: [
+              {
+                _key: 'def',
+                _type: 'span',
+                text: 'Hello',
+                marks: [],
+              },
+            ],
+            markDefs: [],
+            style: 'normal',
+          },
+        ])
+      }
+    })
+  })
+
   it('allows missing .markDefs', async () => {
     const editorRef: RefObject<PortableTextEditor> = createRef()
     const initialValue = [

diff --git a/packages/@sanity/portable-text-editor/src/editor/plugins/createWithEditableAPI.ts b/packages/@sanity/portable-text-editor/src/editor/plugins/createWithEditableAPI.ts
@@ -23,7 +23,6 @@ import {toPortableTextRange, toSlateRange} from '../../utils/ranges'
 import {fromSlateValue, isEqualToEmptyEditor, toSlateValue} from '../../utils/values'
 import {KEY_TO_VALUE_ELEMENT, SLATE_TO_PORTABLE_TEXT_RANGE} from '../../utils/weakMaps'
 import {type PortableTextEditor} from '../PortableTextEditor'
-import {normalizeMarkDefs} from './createWithPortableTextMarkModel'
 
 const debug = debugWithName('API:editable')
 
@@ -517,7 +516,6 @@ export function createWithEditableAPI(
                   })
                 }
               })
-              normalizeMarkDefs(editor, types)
             }
           })
           Editor.normalize(editor)

diff --git a/packages/@sanity/portable-text-editor/src/editor/plugins/createWithInsertData.ts b/packages/@sanity/portable-text-editor/src/editor/plugins/createWithInsertData.ts
@@ -142,7 +142,7 @@ export function createWithInsertData(
           // Validate the result
           const validation = validateValue(parsed, schemaTypes, keyGenerator)
           // Bail out if it's not valid
-          if (!validation.valid) {
+          if (!validation.valid && !validation.resolution?.autoResolve) {
             const errorDescription = `${validation.resolution?.description}`
             change$.next({
               type: 'error',
@@ -321,41 +321,43 @@ function _insertFragment(
   fragment: Descendant[],
   schemaTypes: PortableTextMemberSchemaTypes,
 ) {
-  if (!editor.selection) {
-    return
-  }
-
-  // Ensure that markDefs for any annotations inside this fragment are copied over to the focused text block.
-  const [focusBlock, focusPath] = Editor.node(editor, editor.selection, {depth: 1})
-  if (editor.isTextBlock(focusBlock) && editor.isTextBlock(fragment[0])) {
-    const {markDefs} = focusBlock
-    debug('Mixing markDefs of focusBlock and fragments[0] block', markDefs, fragment[0].markDefs)
-    if (!isEqual(markDefs, fragment[0].markDefs)) {
-      Transforms.setNodes(
-        editor,
-        {
-          markDefs: uniq([...(fragment[0].markDefs || []), ...(markDefs || [])]),
-        },
-        {at: focusPath, mode: 'lowest', voids: false},
-      )
+  editor.withoutNormalizing(() => {
+    if (!editor.selection) {
+      return
+    }
+    // Ensure that markDefs for any annotations inside this fragment are copied over to the focused text block.
+    const [focusBlock, focusPath] = Editor.node(editor, editor.selection, {depth: 1})
+    if (editor.isTextBlock(focusBlock) && editor.isTextBlock(fragment[0])) {
+      const {markDefs} = focusBlock
+      debug('Mixing markDefs of focusBlock and fragments[0] block', markDefs, fragment[0].markDefs)
+      if (!isEqual(markDefs, fragment[0].markDefs)) {
+        Transforms.setNodes(
+          editor,
+          {
+            markDefs: uniq([...(fragment[0].markDefs || []), ...(markDefs || [])]),
+          },
+          {at: focusPath, mode: 'lowest', voids: false},
+        )
+      }
     }
-  }
 
-  const isPasteToEmptyEditor = isEqualToEmptyEditor(editor.children, schemaTypes)
+    const isPasteToEmptyEditor = isEqualToEmptyEditor(editor.children, schemaTypes)
+
+    if (isPasteToEmptyEditor) {
+      // Special case for pasting directly into an empty editor (a placeholder block).
+      // When pasting content starting with multiple empty blocks,
+      // `editor.insertFragment` can potentially duplicate the keys of
+      // the placeholder block because of operations that happen
+      // inside `editor.insertFragment` (involves an `insert_node` operation).
+      // However by splitting the placeholder block first in this situation we are good.
+      Transforms.splitNodes(editor, {at: [0, 0]})
+      editor.insertFragment(fragment)
+      Transforms.removeNodes(editor, {at: [0]})
+    } else {
+      // All other inserts
+      editor.insertFragment(fragment)
+    }
+  })
 
-  if (isPasteToEmptyEditor) {
-    // Special case for pasting directly into an empty editor (a placeholder block).
-    // When pasting content starting with multiple empty blocks,
-    // `editor.insertFragment` can potentially duplicate the keys of
-    // the placeholder block because of operations that happen
-    // inside `editor.insertFragment` (involves an `insert_node` operation).
-    // However by splitting the placeholder block first in this situation we are good.
-    Transforms.splitNodes(editor, {at: [0, 0]})
-    editor.insertFragment(fragment)
-    Transforms.removeNodes(editor, {at: [0]})
-  } else {
-    // All other inserts
-    editor.insertFragment(fragment)
-  }
   editor.onChange()
 }
diff --git a/packages/@sanity/portable-text-editor/src/editor/plugins/createWithPortableTextMarkModel.ts b/packages/@sanity/portable-text-editor/src/editor/plugins/createWithPortableTextMarkModel.ts
@@ -202,14 +202,31 @@ export function createWithPortableTextMarkModel(
           editor.onChange()
         }
       }
-      // Check consistency of markDefs
+      // Check consistency of markDefs (unless we are merging two nodes)
       if (
-        isTextBlock &&
-        editor.operations.some((op) =>
-          ['split_node', 'remove_node', 'remove_text', 'merge_node'].includes(op.type),
+        editor.isTextBlock(node) &&
+        !editor.operations.some(
+          (op) => op.type === 'merge_node' && 'markDefs' in op.properties && op.path.length === 1,
         )
       ) {
-        normalizeMarkDefs(editor, types)
+        const newMarkDefs = (node.markDefs || []).filter((def) => {
+          return node.children.find((child) => {
+            return (
+              Text.isText(child) && Array.isArray(child.marks) && child.marks.includes(def._key)
+            )
+          })
+        })
+        if (node.markDefs && !isEqual(newMarkDefs, node.markDefs)) {
+          debug('Removing markDef not in use')
+          Transforms.setNodes(
+            editor,
+            {
+              markDefs: newMarkDefs,
+            },
+            {at: path},
+          )
+          editor.onChange()
+        }
       }
     }
 
@@ -422,41 +439,3 @@ export function createWithPortableTextMarkModel(
     }
   }
 }
-/**
- * Normalize markDefs, removing those not in use.
- * @internal
- */
-export function normalizeMarkDefs(
-  editor: PortableTextSlateEditor,
-  types: PortableTextMemberSchemaTypes,
-): void {
-  const {selection} = editor
-  if (selection) {
-    const blocks = Editor.nodes(editor, {
-      at: selection,
-      match: (n) => n._type === types.block.name,
-    })
-    for (const [block, path] of blocks) {
-      if (editor.isTextBlock(block)) {
-        const newMarkDefs = (block.markDefs || []).filter((def) => {
-          return block.children.find((child) => {
-            return (
-              Text.isText(child) && Array.isArray(child.marks) && child.marks.includes(def._key)
-            )
-          })
-        })
-        if (!isEqual(newMarkDefs, block.markDefs)) {
-          debug('Removing markDef not in use')
-          Transforms.setNodes(
-            editor,
-            {
-              markDefs: newMarkDefs,
-            },
-            {at: path},
-          )
-          editor.onChange()
-        }
-      }
-    }
-  }
-}