Operations update decorations (#1778)

* initial simple decorations (mark-like), many tests added

* allow decorators to be set by focus, anchor tags - add tests

* handle one more edge case with decorations in hyperscript

* apply prettier cleanup

* apply linting rules

* update changelog

* ensure always normalize decoration ranges

* reapply prettier after latest adjustments

* all operations apply now update decorations with selection

* ranges can now be 'atomic', will invalidate if contents change

* lint, prettier cleanups

* add atomic invalidation tests, update hyperscript usage

* fix linter errors

* minor cleanup

* slight refactor for simplicity

* remove a couple superfluous lines

* update in response to review

* drop unnecessarily committed add'l file

* remove the need for explicit anchor, focus prop on decoration tags

* update hyperscript use to match latest syntax in #1777

* atomic -> isAtomic

examples/search-highlighting/index.js

@@ -56,6 +56,7 @@ class SearchHighlighting extends React.Component {
             focusKey: key,
             focusOffset: offset,
             marks: [{ type: 'highlight' }],
+            atomic: true,
           })
         }
 

packages/slate-hyperscript/src/index.js

@@ -19,9 +19,12 @@ const FOCUS = {}
  */
 
 class DecoratorPoint {
-  constructor(key, marks) {
+  constructor({ key, data }, marks) {
     this._key = key
     this.marks = marks
+    this.attribs = data || {}
+    this.isAtomic = !!this.attribs.atomic
+    delete this.attribs.atomic
     return this
   }
   withPosition = offset => {
@@ -45,6 +48,8 @@ class DecoratorPoint {
       anchorOffset: this.offset,
       focusOffset: focus.offset,
       marks: this.marks,
+      isAtomic: this.isAtomic,
+      ...this.attribs,
     })
   }
 }
@@ -97,7 +102,7 @@ const CREATORS = {
 
   decoration(tagName, attributes, children) {
     if (attributes.key) {
-      return new DecoratorPoint(attributes.key, [{ type: tagName }])
+      return new DecoratorPoint(attributes, [{ type: tagName }])
     }
 
     const nodes = createChildren(children, { key: attributes.key })
@@ -106,6 +111,7 @@ const CREATORS = {
         anchorOffset: 0,
         focusOffset: nodes.reduce((len, n) => len + n.text.length, 0),
         marks: [{ type: tagName }],
+        isAtomic: !!attributes.data.atomic,
       },
     ])
     return nodes

packages/slate/src/models/range.js

@@ -19,6 +19,7 @@ const DEFAULTS = {
   isBackward: null,
   isFocused: false,
   marks: null,
+  isAtomic: false,
 }
 
 /**
@@ -84,6 +85,7 @@ class Range extends Record(DEFAULTS) {
         isBackward: attrs.isBackward,
         isFocused: attrs.isFocused,
         marks: attrs.marks,
+        isAtomic: attrs.isAtomic,
       }
     }
 
@@ -99,6 +101,7 @@ class Range extends Record(DEFAULTS) {
       if ('isFocused' in attrs) props.isFocused = attrs.isFocused
       if ('marks' in attrs)
         props.marks = attrs.marks == null ? null : Mark.createSet(attrs.marks)
+      if ('isAtomic' in attrs) props.isAtomic = attrs.isAtomic
       return props
     }
 
@@ -123,6 +126,7 @@ class Range extends Record(DEFAULTS) {
       isBackward = null,
       isFocused = false,
       marks = null,
+      isAtomic = false,
     } = object
 
     const range = new Range({
@@ -133,6 +137,7 @@ class Range extends Record(DEFAULTS) {
       isBackward,
       isFocused,
       marks: marks == null ? null : new Set(marks.map(Mark.fromJSON)),
+      isAtomic,
     })
 
     return range
@@ -779,6 +784,7 @@ class Range extends Record(DEFAULTS) {
       isFocused: this.isFocused,
       marks:
         this.marks == null ? null : this.marks.toArray().map(m => m.toJSON()),
+      isAtomic: this.isAtomic,
     }
 
     return object

packages/slate/src/models/value.js

@@ -675,6 +675,24 @@ class Value extends Record(DEFAULTS) {
       delete object.selection.focusKey
     }
 
+    if (
+      options.preserveDecorations &&
+      object.decorations &&
+      !options.preserveKeys
+    ) {
+      const { document } = this
+      object.decorations = object.decorations.map(decoration => {
+        const withPath = {
+          ...decoration,
+          anchorPath: document.getPath(decoration.anchorKey),
+          focusPath: document.getPath(decoration.focusKey),
+        }
+        delete withPath.anchorKey
+        delete withPath.focusKey
+        return withPath
+      })
+    }
+
     return object
   }
 

packages/slate/src/operations/apply.js

@@ -10,6 +10,65 @@ import Operation from '../models/operation'
 
 const debug = Debug('slate:operation:apply')
 
+/**
+ * Apply adjustments to affected ranges (selections, decorations);
+ * accepts (value, checking function(range) -> bool, applying function(range) -> range)
+ * returns value with affected ranges updated
+ *
+ * @param {Value} value
+ * @param {Function} checkAffected
+ * @param {Function} adjustRange
+ * @return {Value}
+ */
+
+function applyRangeAdjustments(value, checkAffected, adjustRange) {
+  // check selection, apply adjustment if affected
+  if (value.selection && checkAffected(value.selection)) {
+    value = value.set('selection', adjustRange(value.selection))
+  }
+  if (!value.decorations) return value
+
+  // check all ranges, apply adjustment if affected
+  const decorations = value.decorations
+    .map(
+      decoration =>
+        checkAffected(decoration) ? adjustRange(decoration) : decoration
+    )
+    .filter(decoration => decoration.anchorKey !== null)
+  return value.set('decorations', decorations)
+}
+
+/**
+ * clear any atomic ranges (in decorations) if they contain the point (key, offset, offset-end?)
+ * specified
+ *
+ * @param {Value} value
+ * @param {String} key
+ * @param {Number} offset
+ * @param {Number?} offsetEnd
+ * @return {Value}
+ */
+
+function clearAtomicRangesIfContains(value, key, offset, offsetEnd = null) {
+  return applyRangeAdjustments(
+    value,
+    range => {
+      if (!range.isAtomic) return false
+      const { startKey, startOffset, endKey, endOffset } = range
+      return (
+        (startKey == key &&
+          startOffset < offset &&
+          (endKey != key || endOffset > offset)) ||
+        (offsetEnd &&
+          startKey == key &&
+          startOffset < offsetEnd &&
+          (endKey != key || endOffset > offsetEnd))
+      )
+    },
+    range => range.deselect()
+  )
+}
+
 /**
  * Applying functions.
  *
@@ -65,23 +124,37 @@ const APPLIERS = {
 
   insert_text(value, operation) {
     const { path, offset, text, marks } = operation
-    let { document, selection } = value
-    const { anchorKey, focusKey, anchorOffset, focusOffset } = selection
+    let { document } = value
     let node = document.assertPath(path)
 
     // Update the document
     node = node.insertText(offset, text, marks)
     document = document.updateNode(node)
 
-    // Update the selection
-    if (anchorKey == node.key && anchorOffset >= offset) {
-      selection = selection.moveAnchor(text.length)
-    }
-    if (focusKey == node.key && focusOffset >= offset) {
-      selection = selection.moveFocus(text.length)
-    }
+    value = value.set('document', document)
+
+    // if insert happens within atomic ranges, clear
+    value = clearAtomicRangesIfContains(value, node.key, offset)
+
+    // Update the selection, decorations
+    value = applyRangeAdjustments(
+      value,
+      ({ anchorKey, anchorOffset, isBackward, isAtomic }) =>
+        anchorKey == node.key &&
+        (anchorOffset > offset ||
+          (anchorOffset == offset && (!isAtomic || !isBackward))),
+      range => range.moveAnchor(text.length)
+    )
+
+    value = applyRangeAdjustments(
+      value,
+      ({ focusKey, focusOffset, isBackward, isAtomic }) =>
+        focusKey == node.key &&
+        (focusOffset > offset ||
+          (focusOffset == offset && (!isAtomic || isBackward))),
+      range => range.moveFocus(text.length)
+    )
 
-    value = value.set('document', document).set('selection', selection)
     return value
   },
 
@@ -98,7 +171,7 @@ const APPLIERS = {
     const withPath = path
       .slice(0, path.length - 1)
       .concat([path[path.length - 1] - 1])
-    let { document, selection } = value
+    let { document } = value
     const one = document.assertPath(withPath)
     const two = document.assertPath(path)
     let parent = document.getParent(one.key)
@@ -108,36 +181,31 @@ const APPLIERS = {
     // Perform the merge in the document.
     parent = parent.mergeNode(oneIndex, twoIndex)
     document = document.updateNode(parent)
+    value = value.set('document', document)
 
-    // If the nodes are text nodes and the selection is inside the second node
-    // update it to refer to the first node instead.
     if (one.object == 'text') {
-      const { anchorKey, anchorOffset, focusKey, focusOffset } = selection
-      let normalize = false
-
-      if (anchorKey == two.key) {
-        selection = selection.moveAnchorTo(
-          one.key,
-          one.text.length + anchorOffset
-        )
-        normalize = true
-      }
-
-      if (focusKey == two.key) {
-        selection = selection.moveFocusTo(
-          one.key,
-          one.text.length + focusOffset
-        )
-        normalize = true
-      }
-
-      if (normalize) {
-        selection = selection.normalize(document)
-      }
+      value = applyRangeAdjustments(
+        value,
+        // If the nodes are text nodes and the range is inside the second node:
+        ({ anchorKey, focusKey }) =>
+          anchorKey == two.key || focusKey == two.key,
+        // update it to refer to the first node instead:
+        range => {
+          if (range.anchorKey == two.key)
+            range = range.moveAnchorTo(
+              one.key,
+              one.text.length + range.anchorOffset
+            )
+          if (range.focusKey == two.key)
+            range = range.moveFocusTo(
+              one.key,
+              one.text.length + range.focusOffset
+            )
+          return range.normalize(document)
+        }
+      )
     }
 
-    // Update the document and selection.
-    value = value.set('document', document).set('selection', selection)
     return value
   },
 
@@ -222,44 +290,38 @@ const APPLIERS = {
   remove_node(value, operation) {
     const { path } = operation
     let { document, selection } = value
-    const { startKey, endKey } = selection
     const node = document.assertPath(path)
 
-    // If the selection is set, check to see if it needs to be updated.
-    if (selection.isSet) {
-      const hasStartNode = node.hasNode(startKey)
-      const hasEndNode = node.hasNode(endKey)
+    if (selection.isSet || value.decorations !== null) {
       const first = node.object == 'text' ? node : node.getFirstText() || node
       const last = node.object == 'text' ? node : node.getLastText() || node
       const prev = document.getPreviousText(first.key)
       const next = document.getNextText(last.key)
 
-      // If the start point was in this node, update it to be just before/after.
-      if (hasStartNode) {
-        if (prev) {
-          selection = selection.moveStartTo(prev.key, prev.text.length)
-        } else if (next) {
-          selection = selection.moveStartTo(next.key, 0)
-        } else {
-          selection = selection.deselect()
-        }
-      }
+      value = applyRangeAdjustments(
+        value,
+        // If the start or end point was in this node
+        ({ startKey, endKey }) =>
+          node.hasNode(startKey) || node.hasNode(endKey),
+        // update it to be just before/after
+        range => {
+          const { startKey, endKey } = range
 
-      // If the end point was in this node, update it to be just before/after.
-      if (selection.isSet && hasEndNode) {
-        if (prev) {
-          selection = selection.moveEndTo(prev.key, prev.text.length)
-        } else if (next) {
-          selection = selection.moveEndTo(next.key, 0)
-        } else {
-          selection = selection.deselect()
+          if (node.hasNode(startKey)) {
+            range = prev
+              ? range.moveStartTo(prev.key, prev.text.length)
+              : next ? range.moveStartTo(next.key, 0) : range.deselect()
+          }
+          if (node.hasNode(endKey)) {
+            range = prev
+              ? range.moveEndTo(prev.key, prev.text.length)
+              : next ? range.moveEndTo(next.key, 0) : range.deselect()
+          }
+          // If the range wasn't deselected, normalize it.
+          if (range.isSet) return range.normalize(document)
+          return range
         }
-      }
-
-      // If the selection wasn't deselected, normalize it.
-      if (selection.isSet) {
-        selection = selection.normalize(document)
-      }
+      )
     }
 
     // Remove the node from the document.
@@ -268,8 +330,8 @@ const APPLIERS = {
     parent = parent.removeNode(index)
     document = document.updateNode(parent)
 
-    // Update the document and selection.
-    value = value.set('document', document).set('selection', selection)
+    // Update the document and range.
+    value = value.set('document', document)
     return value
   },
 
@@ -285,29 +347,47 @@ const APPLIERS = {
     const { path, offset, text } = operation
     const { length } = text
     const rangeOffset = offset + length
-    let { document, selection } = value
-    const { anchorKey, focusKey, anchorOffset, focusOffset } = selection
+    let { document } = value
+
     let node = document.assertPath(path)
 
-    if (anchorKey == node.key) {
-      if (anchorOffset >= rangeOffset) {
-        selection = selection.moveAnchor(-length)
-      } else if (anchorOffset > offset) {
-        selection = selection.moveAnchorTo(anchorKey, offset)
-      }
-    }
+    // if insert happens within atomic ranges, clear
+    value = clearAtomicRangesIfContains(
+      value,
+      node.key,
+      offset,
+      offset + length
+    )
 
-    if (focusKey == node.key) {
-      if (focusOffset >= rangeOffset) {
-        selection = selection.moveFocus(-length)
-      } else if (focusOffset > offset) {
-        selection = selection.moveFocusTo(focusKey, offset)
-      }
-    }
+    value = applyRangeAdjustments(
+      value,
+      // if anchor of range is here
+      ({ anchorKey }) => anchorKey == node.key,
+      // adjust if it is in or past the removal range
+      range =>
+        range.anchorOffset >= rangeOffset
+          ? range.moveAnchor(-length)
+          : range.anchorOffset > offset
+            ? range.moveAnchorTo(range.anchorKey, offset)
+            : range
+    )
+
+    value = applyRangeAdjustments(
+      value,
+      // if focus of range is here
+      ({ focusKey }) => focusKey == node.key,
+      // adjust if it is in or past the removal range
+      range =>
+        range.focusOffset >= rangeOffset
+          ? range.moveFocus(-length)
+          : range.focusOffset > offset
+            ? range.moveFocusTo(range.focusKey, offset)
+            : range
+    )
 
     node = node.removeText(offset, length)
     document = document.updateNode(node)
-    value = value.set('document', document).set('selection', selection)
+    value = value.set('document', document)
     return value
   },
 
@@ -400,7 +480,7 @@ const APPLIERS = {
 
   split_node(value, operation) {
     const { path, position, properties } = operation
-    let { document, selection } = value
+    let { document } = value
 
     // Calculate a few things...
     const node = document.assertPath(path)
@@ -416,32 +496,37 @@ const APPLIERS = {
       }
     }
     document = document.updateNode(parent)
-
-    // Determine whether we need to update the selection...
-    const { startKey, endKey, startOffset, endOffset } = selection
     const next = document.getNextText(node.key)
-    let normalize = false
 
-    // If the start point is after or equal to the split, update it.
-    if (node.key == startKey && position <= startOffset) {
-      selection = selection.moveStartTo(next.key, startOffset - position)
-      normalize = true
-    }
+    value = applyRangeAdjustments(
+      value,
+      // check if range is affected
+      ({ startKey, startOffset, endKey, endOffset }) =>
+        (node.key == startKey && position <= startOffset) ||
+        (node.key == endKey && position <= endOffset),
+      // update its start / end as needed
+      range => {
+        const { startKey, startOffset, endKey, endOffset } = range
+        let normalize = false
 
-    // If the end point is after or equal to the split, update it.
-    if (node.key == endKey && position <= endOffset) {
-      selection = selection.moveEndTo(next.key, endOffset - position)
-      normalize = true
-    }
+        if (node.key == startKey && position <= startOffset) {
+          range = range.moveStartTo(next.key, startOffset - position)
+          normalize = true
+        }
 
-    // Normalize the selection if we changed it, since the methods we use might
-    // leave it in a non-normalized value.
-    if (normalize) {
-      selection = selection.normalize(document)
-    }
+        if (node.key == endKey && position <= endOffset) {
+          range = range.moveEndTo(next.key, endOffset - position)
+          normalize = true
+        }
+
+        // Normalize the selection if we changed it
+        if (normalize) return range.normalize(document)
+        return range
+      }
+    )
 
     // Return the updated value.
-    value = value.set('document', document).set('selection', selection)
+    value = value.set('document', document)
     return value
   },
 }

packages/slate/test/helpers/h.js

@@ -34,6 +34,9 @@ const h = createHyperscript({
     u: 'underline',
     fontSize: 'font-size',
   },
+  decorators: {
+    highlight: 'highlight',
+  },
 })
 
 /**

packages/slate/test/operations/apply/update-decorations/atomic-invalidation-delete.js

@@ -0,0 +1,61 @@
+/** @jsx h */
+
+import h from '../../../helpers/h'
+
+export default [
+  {
+    type: 'remove_text',
+    path: [0, 0],
+    offset: 13,
+    text: 'on',
+    marks: [],
+  },
+  {
+    type: 'remove_text',
+    path: [1, 0],
+    offset: 0,
+    text: 'This ',
+    marks: [],
+  },
+  {
+    type: 'remove_text',
+    path: [2, 0],
+    offset: 10,
+    text: 'ation',
+    marks: [],
+  },
+]
+
+export const input = (
+  <value>
+    <document>
+      <paragraph>
+        This <highlight atomic>decoration</highlight> should be invalid,{' '}
+        <highlight atomic>this</highlight> one shouldn't.
+      </paragraph>
+      <paragraph>
+        This <highlight atomic>decoration</highlight> will be fine.
+      </paragraph>
+      <paragraph>
+        This <highlight>decoration</highlight> can be altered, since non-atomic.
+      </paragraph>
+    </document>
+  </value>
+)
+
+export const output = (
+  <value>
+    <document>
+      <paragraph>
+        This decorati should be invalid, <highlight atomic>this</highlight> one
+        shouldn't.
+      </paragraph>
+      <paragraph>
+        <highlight atomic>decoration</highlight> will be fine.
+      </paragraph>
+      <paragraph>
+        This <highlight>decor</highlight> can be altered, since non-atomic.
+      </paragraph>
+    </document>
+  </value>
+)

packages/slate/test/operations/apply/update-decorations/atomic-invalidation-insert.js

@@ -0,0 +1,61 @@
+/** @jsx h */
+
+import h from '../../../helpers/h'
+
+export default [
+  {
+    type: 'insert_text',
+    path: [0, 0],
+    offset: 6,
+    text: 'x',
+    marks: [],
+  },
+  {
+    type: 'insert_text',
+    path: [1, 0],
+    offset: 5,
+    text: 'small ',
+    marks: [],
+  },
+  {
+    type: 'remove_text',
+    path: [2, 0],
+    offset: 10,
+    text: 'ation',
+    marks: [],
+  },
+]
+
+export const input = (
+  <value>
+    <document>
+      <paragraph>
+        This <highlight atomic>decoration</highlight> should be invalid,{' '}
+        <highlight atomic>this</highlight> one shouldn't.
+      </paragraph>
+      <paragraph>
+        This <highlight atomic>decoration</highlight> will be fine.
+      </paragraph>
+      <paragraph>
+        This <highlight>decoration</highlight> can be altered, since non-atomic.
+      </paragraph>
+    </document>
+  </value>
+)
+
+export const output = (
+  <value>
+    <document>
+      <paragraph>
+        This dxecoration should be invalid, <highlight atomic>this</highlight>{' '}
+        one shouldn't.
+      </paragraph>
+      <paragraph>
+        This small <highlight atomic>decoration</highlight> will be fine.
+      </paragraph>
+      <paragraph>
+        This <highlight>decor</highlight> can be altered, since non-atomic.
+      </paragraph>
+    </document>
+  </value>
+)

packages/slate/test/operations/apply/update-decorations/delete-before.js

@@ -0,0 +1,33 @@
+/** @jsx h */
+
+import h from '../../../helpers/h'
+
+export default [
+  {
+    type: 'remove_text',
+    path: [0, 0],
+    offset: 2,
+    text: ' there',
+    marks: [],
+  },
+]
+
+export const input = (
+  <value>
+    <document>
+      <paragraph>
+        Hi<cursor /> there <highlight>you</highlight> person
+      </paragraph>
+    </document>
+  </value>
+)
+
+export const output = (
+  <value>
+    <document>
+      <paragraph>
+        Hi<cursor /> <highlight>you</highlight> person
+      </paragraph>
+    </document>
+  </value>
+)

packages/slate/test/operations/apply/update-decorations/insert-before.js

@@ -0,0 +1,33 @@
+/** @jsx h */
+
+import h from '../../../helpers/h'
+
+export default [
+  {
+    type: 'insert_text',
+    path: [0, 0],
+    offset: 2,
+    text: ' added',
+    marks: [],
+  },
+]
+
+export const input = (
+  <value>
+    <document>
+      <paragraph>
+        Hi<cursor /> there <highlight>you</highlight>
+      </paragraph>
+    </document>
+  </value>
+)
+
+export const output = (
+  <value>
+    <document>
+      <paragraph>
+        Hi added<cursor /> there <highlight>you</highlight>
+      </paragraph>
+    </document>
+  </value>
+)

packages/slate/test/operations/apply/update-decorations/merge-node.js

@@ -0,0 +1,45 @@
+/** @jsx h */
+
+import h from '../../../helpers/h'
+
+export default [
+  {
+    type: 'merge_node',
+    path: [1],
+    position: 1,
+    properties: {},
+    target: null,
+  },
+  // also merge the resulting leaves
+  {
+    type: 'merge_node',
+    path: [0, 1],
+    position: 1,
+    properties: {},
+    target: null,
+  },
+]
+
+export const input = (
+  <value>
+    <document>
+      <paragraph>
+        The decoration begins<highlight key="1" /> in this paragraph
+      </paragraph>
+      <paragraph>
+        And ends in this soon-to-be merged<highlight key="1" /> one.
+      </paragraph>
+    </document>
+  </value>
+)
+
+export const output = (
+  <value>
+    <document>
+      <paragraph>
+        The decoration begins<highlight key="1" /> in this paragraphAnd ends in
+        this soon-to-be merged<highlight key="1" /> one.
+      </paragraph>
+    </document>
+  </value>
+)

packages/slate/test/operations/apply/update-decorations/remove-node.js

@@ -0,0 +1,34 @@
+/** @jsx h */
+
+import h from '../../../helpers/h'
+
+export default [
+  {
+    type: 'remove_node',
+    path: [0],
+  },
+]
+
+export const input = (
+  <value>
+    <document>
+      <paragraph>
+        The decoration begins<highlight key="1" /> in this soon deleted
+        paragraph
+      </paragraph>
+      <paragraph>
+        And ends in this <highlight key="1" />one.
+      </paragraph>
+    </document>
+  </value>
+)
+
+export const output = (
+  <value>
+    <document>
+      <paragraph>
+        <highlight key="1" />And ends in this <highlight key="1" />one.
+      </paragraph>
+    </document>
+  </value>
+)

packages/slate/test/operations/index.js

@@ -33,9 +33,14 @@ describe('operations', async () => {
               const operations = module.default
               const change = input.change()
               change.applyOperations(operations)
-              const opts = { preserveSelection: true, preserveData: true }
+              const opts = {
+                preserveSelection: true,
+                preserveDecorations: true,
+                preserveData: true,
+              }
               const actual = change.value.toJSON(opts)
               const expected = output.toJSON(opts)
+
               assert.deepEqual(actual, expected)
             })
           }

packages/slate/test/serializers/raw/serialize/preserve-selection-and-keys.js

@@ -48,6 +48,7 @@ export const output = {
     isBackward: false,
     isFocused: false,
     marks: null,
+    isAtomic: false,
   },
 }
 

packages/slate/test/serializers/raw/serialize/preserve-selection.js

@@ -45,6 +45,7 @@ export const output = {
     isBackward: false,
     isFocused: false,
     marks: null,
+    isAtomic: false,
   },
 }