Fix delete intent (#2367)

* fix delete intent

* refactor deleteAtRange helper logic

* cleanup delete at range commands

* cleanup

packages/slate/src/commands/at-range.js

@@ -5,6 +5,31 @@ import Mark from '../models/mark'
 import Node from '../models/node'
 import TextUtils from '../utils/text-utils'
 
+/**
+ * Ensure that an expanded selection is deleted first, and return the updated
+ * range to account for the deleted part.
+ *
+ * @param {Editor}
+ */
+
+function deleteExpandedAtRange(editor, range) {
+  if (range.isExpanded) {
+    editor.deleteAtRange(range)
+  }
+
+  const { value } = editor
+  const { document } = value
+  const { start, end } = range
+
+  if (document.hasDescendant(start.key)) {
+    range = range.moveToStart()
+  } else {
+    range = range.moveTo(end.key, 0).normalize(document)
+  }
+
+  return range
+}
+
 /**
  * Commands.
  *
@@ -250,61 +275,6 @@ Commands.deleteAtRange = (editor, range) => {
   })
 }
 
-/**
- * Delete backward until the character boundary at a `range`.
- *
- * @param {Editor} editor
- * @param {Range} range
- */
-
-Commands.deleteCharBackwardAtRange = (editor, range) => {
-  const { value } = editor
-  const { document } = value
-  const { start } = range
-  const startBlock = document.getClosestBlock(start.key)
-  const offset = startBlock.getOffset(start.key)
-  const o = offset + start.offset
-  const { text } = startBlock
-  const n = TextUtils.getCharOffsetBackward(text, o)
-  editor.deleteBackwardAtRange(range, n)
-}
-
-/**
- * Delete backward until the line boundary at a `range`.
- *
- * @param {Editor} editor
- * @param {Range} range
- */
-
-Commands.deleteLineBackwardAtRange = (editor, range) => {
-  const { value } = editor
-  const { document } = value
-  const { start } = range
-  const startBlock = document.getClosestBlock(start.key)
-  const offset = startBlock.getOffset(start.key)
-  const o = offset + start.offset
-  editor.deleteBackwardAtRange(range, o)
-}
-
-/**
- * Delete backward until the word boundary at a `range`.
- *
- * @param {Editor} editor
- * @param {Range} range
- */
-
-Commands.deleteWordBackwardAtRange = (editor, range) => {
-  const { value } = editor
-  const { document } = value
-  const { start } = range
-  const startBlock = document.getClosestBlock(start.key)
-  const offset = startBlock.getOffset(start.key)
-  const o = offset + start.offset
-  const { text } = startBlock
-  const n = o === 0 ? 1 : TextUtils.getWordOffsetBackward(text, o)
-  editor.deleteBackwardAtRange(range, n)
-}
-
 /**
  * Delete backward `n` characters at a `range`.
  *
@@ -333,21 +303,17 @@ Commands.deleteBackwardAtRange = (editor, range, n = 1) => {
     return
   }
 
-  const block = document.getClosestBlock(start.key)
+  // If the range is at the start of the document, abort.
-
+  if (start.isAtStartOfNode(document)) {
-  // If the closest is not void, but empty, remove it
-  if (
-    block &&
-    !editor.isVoid(block) &&
-    block.text === '' &&
-    document.nodes.size !== 1
-  ) {
-    editor.removeNodeByKey(block.key)
     return
   }
 
-  // If the range is at the start of the document, abort.
+  const block = document.getClosestBlock(start.key)
-  if (start.isAtStartOfNode(document)) {
+
+  // PERF: If the closest block is empty, remove it. This is just a shortcut,
+  // since merging it would result in the same outcome.
+  if (document.nodes.size !== 1 && block && block.text === '') {
+    editor.removeNodeByKey(block.key)
     return
   }
 
@@ -404,6 +370,30 @@ Commands.deleteBackwardAtRange = (editor, range, n = 1) => {
   editor.deleteAtRange(range)
 }
 
+/**
+ * Delete backward until the character boundary at a `range`.
+ *
+ * @param {Editor} editor
+ * @param {Range} range
+ */
+
+Commands.deleteCharBackwardAtRange = (editor, range) => {
+  if (range.isExpanded) {
+    editor.deleteAtRange(range)
+    return
+  }
+
+  const { value } = editor
+  const { document } = value
+  const { start } = range
+  const startBlock = document.getClosestBlock(start.key)
+  const offset = startBlock.getOffset(start.key)
+  const o = offset + start.offset
+  const { text } = startBlock
+  const n = TextUtils.getCharOffsetBackward(text, o)
+  editor.deleteBackwardAtRange(range, n)
+}
+
 /**
  * Delete forward until the character boundary at a `range`.
  *
@@ -412,6 +402,11 @@ Commands.deleteBackwardAtRange = (editor, range, n = 1) => {
  */
 
 Commands.deleteCharForwardAtRange = (editor, range) => {
+  if (range.isExpanded) {
+    editor.deleteAtRange(range)
+    return
+  }
+
   const { value } = editor
   const { document } = value
   const { start } = range
@@ -423,43 +418,6 @@ Commands.deleteCharForwardAtRange = (editor, range) => {
   editor.deleteForwardAtRange(range, n)
 }
 
-/**
- * Delete forward until the line boundary at a `range`.
- *
- * @param {Editor} editor
- * @param {Range} range
- */
-
-Commands.deleteLineForwardAtRange = (editor, range) => {
-  const { value } = editor
-  const { document } = value
-  const { start } = range
-  const startBlock = document.getClosestBlock(start.key)
-  const offset = startBlock.getOffset(start.key)
-  const o = offset + start.offset
-  editor.deleteForwardAtRange(range, startBlock.text.length - o)
-}
-
-/**
- * Delete forward until the word boundary at a `range`.
- *
- * @param {Editor} editor
- * @param {Range} range
- */
-
-Commands.deleteWordForwardAtRange = (editor, range) => {
-  const { value } = editor
-  const { document } = value
-  const { start } = range
-  const startBlock = document.getClosestBlock(start.key)
-  const offset = startBlock.getOffset(start.key)
-  const o = offset + start.offset
-  const { text } = startBlock
-  const wordOffset = TextUtils.getWordOffsetForward(text, o)
-  const n = wordOffset === 0 ? 1 : wordOffset
-  editor.deleteForwardAtRange(range, n)
-}
-
 /**
  * Delete forward `n` characters at a `range`.
  *
@@ -566,6 +524,99 @@ Commands.deleteForwardAtRange = (editor, range, n = 1) => {
   editor.deleteAtRange(range)
 }
 
+/**
+ * Delete backward until the line boundary at a `range`.
+ *
+ * @param {Editor} editor
+ * @param {Range} range
+ */
+
+Commands.deleteLineBackwardAtRange = (editor, range) => {
+  if (range.isExpanded) {
+    editor.deleteAtRange(range)
+    return
+  }
+
+  const { value } = editor
+  const { document } = value
+  const { start } = range
+  const startBlock = document.getClosestBlock(start.key)
+  const offset = startBlock.getOffset(start.key)
+  const o = offset + start.offset
+  editor.deleteBackwardAtRange(range, o)
+}
+
+/**
+ * Delete forward until the line boundary at a `range`.
+ *
+ * @param {Editor} editor
+ * @param {Range} range
+ */
+
+Commands.deleteLineForwardAtRange = (editor, range) => {
+  if (range.isExpanded) {
+    editor.deleteAtRange(range)
+    return
+  }
+
+  const { value } = editor
+  const { document } = value
+  const { start } = range
+  const startBlock = document.getClosestBlock(start.key)
+  const offset = startBlock.getOffset(start.key)
+  const o = offset + start.offset
+  editor.deleteForwardAtRange(range, startBlock.text.length - o)
+}
+
+/**
+ * Delete backward until the word boundary at a `range`.
+ *
+ * @param {Editor} editor
+ * @param {Range} range
+ */
+
+Commands.deleteWordBackwardAtRange = (editor, range) => {
+  if (range.isExpanded) {
+    editor.deleteAtRange(range)
+    return
+  }
+
+  const { value } = editor
+  const { document } = value
+  const { start } = range
+  const startBlock = document.getClosestBlock(start.key)
+  const offset = startBlock.getOffset(start.key)
+  const o = offset + start.offset
+  const { text } = startBlock
+  const n = o === 0 ? 1 : TextUtils.getWordOffsetBackward(text, o)
+  editor.deleteBackwardAtRange(range, n)
+}
+
+/**
+ * Delete forward until the word boundary at a `range`.
+ *
+ * @param {Editor} editor
+ * @param {Range} range
+ */
+
+Commands.deleteWordForwardAtRange = (editor, range) => {
+  if (range.isExpanded) {
+    editor.deleteAtRange(range)
+    return
+  }
+
+  const { value } = editor
+  const { document } = value
+  const { start } = range
+  const startBlock = document.getClosestBlock(start.key)
+  const offset = startBlock.getOffset(start.key)
+  const o = offset + start.offset
+  const { text } = startBlock
+  const wordOffset = TextUtils.getWordOffsetForward(text, o)
+  const n = wordOffset === 0 ? 1 : wordOffset
+  editor.deleteForwardAtRange(range, n)
+}
+
 /**
  * Insert a `block` node at `range`.
  *
@@ -575,13 +626,9 @@ Commands.deleteForwardAtRange = (editor, range, n = 1) => {
  */
 
 Commands.insertBlockAtRange = (editor, range, block) => {
+  range = deleteExpandedAtRange(editor, range)
   block = Block.create(block)
 
-  if (range.isExpanded) {
-    editor.deleteAtRange(range)
-    range = range.moveToStart()
-  }
-
   const { value } = editor
   const { document } = value
   const { start } = range
@@ -633,16 +680,7 @@ Commands.insertBlockAtRange = (editor, range, block) => {
 
 Commands.insertFragmentAtRange = (editor, range, fragment) => {
   editor.withoutNormalizing(() => {
-    // If the range is expanded, delete it first.
+    range = deleteExpandedAtRange(editor, range)
-    if (range.isExpanded) {
-      editor.deleteAtRange(range)
-
-      if (editor.value.document.getDescendant(range.start.key)) {
-        range = range.moveToStart()
-      } else {
-        range = range.moveTo(range.end.key, 0).normalize(editor.value.document)
-      }
-    }
 
     // If the fragment is empty, there's nothing to do after deleting.
     if (!fragment.nodes.size) return
@@ -795,10 +833,7 @@ Commands.insertInlineAtRange = (editor, range, inline) => {
   inline = Inline.create(inline)
 
   editor.withoutNormalizing(() => {
-    if (range.isExpanded) {
+    range = deleteExpandedAtRange(editor, range)
-      editor.deleteAtRange(range)
-      range = range.moveToStart()
-    }
 
     const { value } = editor
     const { document } = value
@@ -824,32 +859,19 @@ Commands.insertInlineAtRange = (editor, range, inline) => {
  */
 
 Commands.insertTextAtRange = (editor, range, text, marks) => {
+  range = deleteExpandedAtRange(editor, range)
+
   const { value } = editor
   const { document } = value
   const { start } = range
-  let key = start.key
   const offset = start.offset
-  const path = start.path
   const parent = document.getParent(start.key)
 
   if (editor.isVoid(parent)) {
     return
   }
 
-  editor.withoutNormalizing(() => {
+  editor.insertTextByKey(start.key, offset, text, marks)
-    if (range.isExpanded) {
-      editor.deleteAtRange(range)
-
-      const startText = editor.value.document.getNode(path)
-
-      // Update range start after delete
-      if (startText && startText.key !== key) {
-        key = startText.key
-      }
-    }
-
-    editor.insertTextByKey(key, offset, text, marks)
-  })
 }
 
 /**
@@ -951,6 +973,8 @@ Commands.setInlinesAtRange = (editor, range, properties) => {
  */
 
 Commands.splitBlockAtRange = (editor, range, height = 1) => {
+  range = deleteExpandedAtRange(editor, range)
+
   const { start, end } = range
   let { value } = editor
   let { document } = value
@@ -995,10 +1019,7 @@ Commands.splitBlockAtRange = (editor, range, height = 1) => {
  */
 
 Commands.splitInlineAtRange = (editor, range, height = Infinity) => {
-  if (range.isExpanded) {
+  range = deleteExpandedAtRange(editor, range)
-    editor.deleteAtRange(range)
-    range = range.moveToStart()
-  }
 
   const { start } = range
   const { value } = editor

packages/slate/src/commands/with-intent.js

@@ -2,6 +2,23 @@ import Block from '../models/block'
 import Inline from '../models/inline'
 import Mark from '../models/mark'
 
+/**
+ * Ensure that an expanded selection is deleted first using the `editor.delete`
+ * command. This guarantees that it uses the proper semantic "intent" instead of
+ * using `deleteAtRange` under the covers and skipping `delete`.
+ *
+ * @param {Editor}
+ */
+
+function deleteExpanded(editor) {
+  const { value } = editor
+  const { selection } = value
+
+  if (selection.isExpanded) {
+    editor.delete()
+  }
+}
+
 /**
  * Commands.
  *
@@ -10,44 +27,6 @@ import Mark from '../models/mark'
 
 const Commands = {}
 
-/**
- * Mix in the changes that pass through to their at-range equivalents because
- * they don't have any effect on the selection.
- */
-
-const PROXY_TRANSFORMS = [
-  'deleteBackward',
-  'deleteCharBackward',
-  'deleteLineBackward',
-  'deleteWordBackward',
-  'deleteForward',
-  'deleteCharForward',
-  'deleteWordForward',
-  'deleteLineForward',
-  'setBlocks',
-  'setInlines',
-  'splitInline',
-  'unwrapBlock',
-  'unwrapInline',
-  'wrapBlock',
-  'wrapInline',
-]
-
-PROXY_TRANSFORMS.forEach(method => {
-  Commands[method] = (editor, ...args) => {
-    const { value } = editor
-    const { selection } = value
-    const methodAtRange = `${method}AtRange`
-    editor[methodAtRange](selection, ...args)
-
-    if (method.match(/Backward$/)) {
-      editor.moveToStart()
-    } else if (method.match(/Forward$/)) {
-      editor.moveToEnd()
-    }
-  }
-})
-
 /**
  * Add a `mark` to the characters in the current selection.
  *
@@ -77,7 +56,7 @@ Commands.addMark = (editor, mark) => {
  * Add a list of `marks` to the characters in the current selection.
  *
  * @param {Editor} editor
- * @param {Mark} mark
+ * @param {Set<Mark>|Array<Object>} marks
  */
 
 Commands.addMarks = (editor, marks) => {
@@ -95,10 +74,148 @@ Commands.delete = editor => {
   const { selection } = value
   editor.deleteAtRange(selection)
 
-  // Ensure that the selection is collapsed to the start, because in certain
+  // COMPAT: Ensure that the selection is collapsed, because in certain cases
-  // cases when deleting across inline nodes, when splitting the inline node the
+  // when deleting across inline nodes, when splitting the inline node the end
-  // end point of the selection will end up after the split point.
+  // point of the selection will end up after the split point.
-  editor.moveToStart()
+  editor.moveToFocus()
+}
+
+/**
+ * Delete backward `n` characters.
+ *
+ * @param {Editor} editor
+ * @param {Number} n (optional)
+ */
+
+Commands.deleteBackward = (editor, n = 1) => {
+  const { value } = editor
+  const { selection } = value
+
+  if (selection.isExpanded) {
+    editor.delete()
+  } else {
+    editor.deleteBackwardAtRange(selection, n)
+  }
+}
+
+/**
+ * Delete backward one character.
+ *
+ * @param {Editor} editor
+ */
+
+Commands.deleteCharBackward = editor => {
+  const { value } = editor
+  const { selection } = value
+
+  if (selection.isExpanded) {
+    editor.delete()
+  } else {
+    editor.deleteCharBackwardAtRange(selection)
+  }
+}
+
+/**
+ * Delete backward one line.
+ *
+ * @param {Editor} editor
+ */
+
+Commands.deleteLineBackward = editor => {
+  const { value } = editor
+  const { selection } = value
+
+  if (selection.isExpanded) {
+    editor.delete()
+  } else {
+    editor.deleteLineBackwardAtRange(selection)
+  }
+}
+
+/**
+ * Delete backward one word.
+ *
+ * @param {Editor} editor
+ */
+
+Commands.deleteWordBackward = editor => {
+  const { value } = editor
+  const { selection } = value
+
+  if (selection.isExpanded) {
+    editor.delete()
+  } else {
+    editor.deleteWordBackwardAtRange(selection)
+  }
+}
+
+/**
+ * Delete backward `n` characters.
+ *
+ * @param {Editor} editor
+ * @param {Number} n (optional)
+ */
+
+Commands.deleteForward = (editor, n = 1) => {
+  const { value } = editor
+  const { selection } = value
+
+  if (selection.isExpanded) {
+    editor.delete()
+  } else {
+    editor.deleteForwardAtRange(selection, n)
+  }
+}
+
+/**
+ * Delete backward one character.
+ *
+ * @param {Editor} editor
+ */
+
+Commands.deleteCharForward = editor => {
+  const { value } = editor
+  const { selection } = value
+
+  if (selection.isExpanded) {
+    editor.delete()
+  } else {
+    editor.deleteCharForwardAtRange(selection)
+  }
+}
+
+/**
+ * Delete backward one line.
+ *
+ * @param {Editor} editor
+ */
+
+Commands.deleteLineForward = editor => {
+  const { value } = editor
+  const { selection } = value
+
+  if (selection.isExpanded) {
+    editor.delete()
+  } else {
+    editor.deleteLineForwardAtRange(selection)
+  }
+}
+
+/**
+ * Delete backward one word.
+ *
+ * @param {Editor} editor
+ */
+
+Commands.deleteWordForward = editor => {
+  const { value } = editor
+  const { selection } = value
+
+  if (selection.isExpanded) {
+    editor.delete()
+  } else {
+    editor.deleteWordForwardAtRange(selection)
+  }
 }
 
 /**
@@ -109,6 +226,8 @@ Commands.delete = editor => {
  */
 
 Commands.insertBlock = (editor, block) => {
+  deleteExpanded(editor)
+
   block = Block.create(block)
   const { value } = editor
   const { selection } = value
@@ -129,6 +248,8 @@ Commands.insertBlock = (editor, block) => {
 Commands.insertFragment = (editor, fragment) => {
   if (!fragment.nodes.size) return
 
+  deleteExpanded(editor)
+
   let { value } = editor
   let { document, selection } = value
   const { start, end } = selection
@@ -169,6 +290,8 @@ Commands.insertFragment = (editor, fragment) => {
  */
 
 Commands.insertInline = (editor, inline) => {
+  deleteExpanded(editor)
+
   inline = Inline.create(inline)
   const { value } = editor
   const { selection } = value
@@ -188,6 +311,8 @@ Commands.insertInline = (editor, inline) => {
  */
 
 Commands.insertText = (editor, text, marks) => {
+  deleteExpanded(editor)
+
   const { value } = editor
   const { document, selection } = value
   marks = marks || selection.marks || document.getInsertMarksAtRange(selection)
@@ -238,6 +363,32 @@ Commands.replaceMark = (editor, oldMark, newMark) => {
   editor.addMark(newMark)
 }
 
+/**
+ * Set the `properties` of block nodes.
+ *
+ * @param {Editor} editor
+ * @param {Object|String} properties
+ */
+
+Commands.setBlocks = (editor, properties) => {
+  const { value } = editor
+  const { selection } = value
+  editor.setBlocksAtRange(selection, properties)
+}
+
+/**
+ * Set the `properties` of inline nodes.
+ *
+ * @param {Editor} editor
+ * @param {Object|String} properties
+ */
+
+Commands.setInlines = (editor, properties) => {
+  const { value } = editor
+  const { selection } = value
+  editor.setInlinesAtRange(selection, properties)
+}
+
 /**
  * Split the block node at the current selection, to optional `depth`.
  *
@@ -246,6 +397,8 @@ Commands.replaceMark = (editor, oldMark, newMark) => {
  */
 
 Commands.splitBlock = (editor, depth = 1) => {
+  deleteExpanded(editor)
+
   const { value } = editor
   const { selection, document } = value
   const marks = selection.marks || document.getInsertMarksAtRange(selection)
@@ -256,6 +409,20 @@ Commands.splitBlock = (editor, depth = 1) => {
   }
 }
 
+/**
+ * Split the inline nodes to optional `height`.
+ *
+ * @param {Editor} editor
+ * @param {Number} height (optional)
+ */
+
+Commands.splitInline = (editor, height) => {
+  deleteExpanded(editor)
+  const { value } = editor
+  const { selection } = value
+  editor.splitInlineAtRange(selection, height)
+}
+
 /**
  * Add or remove a `mark` from the characters in the current selection,
  * depending on whether it's already there.
@@ -276,6 +443,58 @@ Commands.toggleMark = (editor, mark) => {
   }
 }
 
+/**
+ * Unwrap nodes from a block with `properties`.
+ *
+ * @param {Editor} editor
+ * @param {String|Object} properties
+ */
+
+Commands.unwrapBlock = (editor, properties) => {
+  const { value } = editor
+  const { selection } = value
+  editor.unwrapBlockAtRange(selection, properties)
+}
+
+/**
+ * Unwrap nodes from an inline with `properties`.
+ *
+ * @param {Editor} editor
+ * @param {String|Object} properties
+ */
+
+Commands.unwrapInline = (editor, properties) => {
+  const { value } = editor
+  const { selection } = value
+  editor.unwrapInlineAtRange(selection, properties)
+}
+
+/**
+ * Wrap nodes in a new `block`.
+ *
+ * @param {Editor} editor
+ * @param {Block|Object|String} block
+ */
+
+Commands.wrapBlock = (editor, block) => {
+  const { value } = editor
+  const { selection } = value
+  editor.wrapBlockAtRange(selection, block)
+}
+
+/**
+ * Wrap nodes in a new `inline`.
+ *
+ * @param {Editor} editor
+ * @param {Inline|Object|String} inline
+ */
+
+Commands.wrapInline = (editor, inline) => {
+  const { value } = editor
+  const { selection } = value
+  editor.wrapInlineAtRange(selection, inline)
+}
+
 /**
  * Wrap the current selection with prefix/suffix.
  *

packages/slate/src/plugins/core.js

@@ -1,4 +1,3 @@
-import AtCurrentRange from '../commands/at-current-range'
 import AtRange from '../commands/at-range'
 import ByPath from '../commands/by-path'
 import Commands from './commands'
@@ -8,6 +7,7 @@ import OnValue from '../commands/on-value'
 import Queries from './queries'
 import Schema from './schema'
 import Text from '../models/text'
+import WithIntent from '../commands/with-intent'
 
 /**
  * A plugin that defines the core Slate logic.
@@ -26,12 +26,12 @@ function CorePlugin(options = {}) {
    */
 
   const commands = Commands({
-    ...AtCurrentRange,
     ...AtRange,
     ...ByPath,
     ...OnHistory,
     ...OnSelection,
     ...OnValue,
+    ...WithIntent,
   })
 
   /**

packages/slate/test/commands/at-current-range/delete/across-blocks-inlines.js

@@ -10,14 +10,18 @@ export const input = (
   <value>
     <document>
       <paragraph>
+        <text />
         <link>
           wo<anchor />rd
         </link>
+        <text />
       </paragraph>
       <paragraph>
+        <text />
         <link>
           an<focus />other
         </link>
+        <text />
       </paragraph>
     </document>
   </value>
@@ -27,10 +31,13 @@ export const output = (
   <value>
     <document>
       <paragraph>
+        <text />
+        <link>wo</link>
+        <text />
         <link>
-          wo<cursor />
+          <cursor />other
         </link>
-        <link>other</link>
+        <text />
       </paragraph>
     </document>
   </value>

packages/slate/test/commands/at-current-range/split-block/with-delete-across-blocks-and-inlines.js

@@ -10,14 +10,18 @@ export const input = (
   <value>
     <document>
       <paragraph>
+        <text />
         <link>
           wo<anchor />rd
         </link>
+        <text />
       </paragraph>
       <paragraph>
+        <text />
         <hashtag>
           an<focus />other
         </hashtag>
+        <text />
       </paragraph>
     </document>
   </value>
@@ -27,13 +31,20 @@ export const output = (
   <value>
     <document>
       <paragraph>
+        <text />
         <link>wo</link>
+        <text />
+        <hashtag>
+          <text />
+        </hashtag>
+        <text />
       </paragraph>
       <paragraph>
-        <link />
+        <text />
         <hashtag>
           <cursor />other
         </hashtag>
+        <text />
       </paragraph>
     </document>
   </value>

packages/slate/test/commands/at-current-range/split-block/with-delete-hanging-selection.js

@@ -24,7 +24,7 @@ export const output = (
   <value>
     <document>
       <paragraph>zero</paragraph>
-      <paragraph />
+      <quote />
       <quote>
         <cursor />cat is cute
       </quote>