change isBackward to be computed from paths (#2027)

* change `isBackward` to be computed from paths * remove debuggers * fix lint
2025-08-20 22:21:20 +02:00 · 2018-08-03 14:31:55 -07:00
parent 5a539df0f9
commit b3535e11df
22 changed files with 64 additions and 128 deletions
--- a/packages/slate-hyperscript/test/fixtures/cursor-across-block.js
+++ b/packages/slate-hyperscript/test/fixtures/cursor-across-block.js
@@ -54,7 +54,6 @@ export const output = {
    focusKey: '0',
    focusPath: [0, 0],
    focusOffset: 3,
-    isBackward: false,
    isFocused: true,
    isAtomic: false,
    marks: null,
--- a/packages/slate-hyperscript/test/fixtures/cursor-across-blocks-and-inlines.js
+++ b/packages/slate-hyperscript/test/fixtures/cursor-across-blocks-and-inlines.js
@@ -143,7 +143,6 @@ export const output = {
    focusKey: '4',
    focusPath: [1, 1, 0],
    focusOffset: 1,
-    isBackward: false,
    isFocused: true,
    isAtomic: false,
    marks: null,
--- a/packages/slate-hyperscript/test/fixtures/cursor-across-blocks-end.js
+++ b/packages/slate-hyperscript/test/fixtures/cursor-across-blocks-end.js
@@ -77,7 +77,6 @@ export const output = {
    focusKey: '2',
    focusPath: [1, 0],
    focusOffset: 3,
-    isBackward: false,
    isFocused: true,
    isAtomic: false,
    marks: null,
--- a/packages/slate-hyperscript/test/fixtures/cursor-across-blocks-middle.js
+++ b/packages/slate-hyperscript/test/fixtures/cursor-across-blocks-middle.js
@@ -77,7 +77,6 @@ export const output = {
    focusKey: '2',
    focusPath: [1, 0],
    focusOffset: 1,
-    isBackward: false,
    isFocused: true,
    isAtomic: false,
    marks: null,
--- a/packages/slate-hyperscript/test/fixtures/cursor-across-blocks-start.js
+++ b/packages/slate-hyperscript/test/fixtures/cursor-across-blocks-start.js
@@ -77,7 +77,6 @@ export const output = {
    focusKey: '2',
    focusPath: [1, 0],
    focusOffset: 0,
-    isBackward: false,
    isFocused: true,
    isAtomic: false,
    marks: null,
--- a/packages/slate-hyperscript/test/fixtures/cursor-across-multiple-blocks-end.js
+++ b/packages/slate-hyperscript/test/fixtures/cursor-across-multiple-blocks-end.js
@@ -98,7 +98,6 @@ export const output = {
    focusKey: '4',
    focusPath: [2, 0],
    focusOffset: 5,
-    isBackward: false,
    isFocused: true,
    isAtomic: false,
    marks: null,
--- a/packages/slate-hyperscript/test/fixtures/cursor-across-multiple-blocks-middle.js
+++ b/packages/slate-hyperscript/test/fixtures/cursor-across-multiple-blocks-middle.js
@@ -98,7 +98,6 @@ export const output = {
    focusKey: '4',
    focusPath: [2, 0],
    focusOffset: 1,
-    isBackward: false,
    isFocused: true,
    isAtomic: false,
    marks: null,
--- a/packages/slate-hyperscript/test/fixtures/cursor-across-multiple-blocks-start.js
+++ b/packages/slate-hyperscript/test/fixtures/cursor-across-multiple-blocks-start.js
@@ -98,7 +98,6 @@ export const output = {
    focusKey: '4',
    focusPath: [2, 0],
    focusOffset: 0,
-    isBackward: false,
    isFocused: true,
    isAtomic: false,
    marks: null,
--- a/packages/slate-hyperscript/test/fixtures/cursor-block-end.js
+++ b/packages/slate-hyperscript/test/fixtures/cursor-block-end.js
@@ -54,7 +54,6 @@ export const output = {
    focusKey: '0',
    focusPath: [0, 0],
    focusOffset: 3,
-    isBackward: false,
    isFocused: true,
    isAtomic: false,
    marks: null,
--- a/packages/slate-hyperscript/test/fixtures/cursor-block-middle.js
+++ b/packages/slate-hyperscript/test/fixtures/cursor-block-middle.js
@@ -54,7 +54,6 @@ export const output = {
    focusKey: '0',
    focusPath: [0, 0],
    focusOffset: 1,
-    isBackward: false,
    isFocused: true,
    isAtomic: false,
    marks: null,
--- a/packages/slate-hyperscript/test/fixtures/cursor-block-start.js
+++ b/packages/slate-hyperscript/test/fixtures/cursor-block-start.js
@@ -54,7 +54,6 @@ export const output = {
    focusKey: '0',
    focusPath: [0, 0],
    focusOffset: 0,
-    isBackward: false,
    isFocused: true,
    isAtomic: false,
    marks: null,
--- a/packages/slate-hyperscript/test/fixtures/cursor-custom-block-middle.js
+++ b/packages/slate-hyperscript/test/fixtures/cursor-custom-block-middle.js
@@ -60,7 +60,6 @@ export const output = {
    focusKey: '0',
    focusPath: [0, 0],
    focusOffset: 1,
-    isBackward: false,
    isFocused: true,
    isAtomic: false,
    marks: null,
--- a/packages/slate-hyperscript/test/fixtures/cursor-inline.js
+++ b/packages/slate-hyperscript/test/fixtures/cursor-inline.js
@@ -89,7 +89,6 @@ export const output = {
    focusKey: '0',
    focusPath: [0, 1, 0],
    focusOffset: 1,
-    isBackward: false,
    isFocused: true,
    isAtomic: false,
    marks: null,
--- a/packages/slate-hyperscript/test/fixtures/decoration.js
+++ b/packages/slate-hyperscript/test/fixtures/decoration.js
@@ -64,7 +64,6 @@ export const output = {
      focusKey: '0',
      focusPath: [0, 0],
      focusOffset: 6,
-      isBackward: false,
      isFocused: false,
      isAtomic: false,
      marks: [
--- a/packages/slate-hyperscript/test/fixtures/selection.js
+++ b/packages/slate-hyperscript/test/fixtures/selection.js
@@ -55,7 +55,6 @@ export const output = {
    focusKey: 'a',
    focusPath: [0, 0],
    focusOffset: 2,
-    isBackward: false,
    isFocused: false,
    isAtomic: false,
    marks: null,
--- a/packages/slate/src/changes/at-range.js
+++ b/packages/slate/src/changes/at-range.js
@@ -397,11 +397,7 @@ Changes.deleteBackwardAtRange = (change, range, n = 1, options = {}) => {
  // If the focus offset is farther than the number of characters to delete,
  // just remove the characters backwards inside the current node.
  if (n < focusOffset) {
-    range = range.merge({
-      focusOffset: focusOffset - n,
-      isBackward: true,
-    })
-
+    range = range.merge({ focusOffset: focusOffset - n })
    change.deleteAtRange(range, { normalize })
    return
  }
@@ -424,9 +420,8 @@ Changes.deleteBackwardAtRange = (change, range, n = 1, options = {}) => {
  }

  range = range.merge({
-    focusKey: node.key,
-    focusOffset: offset,
-    isBackward: true,
+    anchorKey: node.key,
+    anchorOffset: offset,
  })

  change.deleteAtRange(range, { normalize })
--- a/packages/slate/src/models/operation.js
+++ b/packages/slate/src/models/operation.js
@@ -295,7 +295,6 @@ class Operation extends Record(DEFAULTS) {
        if ('focusOffset' in value) v.focusOffset = value.focusOffset
        if ('focusPath' in value)
          v.focusPath = value.focusPath && value.focusPath.toJSON()
-        if ('isBackward' in value) v.isBackward = value.isBackward
        if ('isFocused' in value) v.isFocused = value.isFocused
        if ('marks' in value) v.marks = value.marks && value.marks.toJSON()
        value = v
--- a/packages/slate/src/models/range.js
+++ b/packages/slate/src/models/range.js
@@ -20,7 +20,6 @@ const DEFAULTS = {
  focusOffset: null,
  focusPath: null,
  isAtomic: false,
-  isBackward: null,
  isFocused: false,
  marks: null,
 }
@@ -88,7 +87,6 @@ class Range extends Record(DEFAULTS) {
        focusOffset: a.focusOffset,
        focusPath: a.focusPath,
        isAtomic: a.isAtomic,
-        isBackward: a.isBackward,
        isFocused: a.isFocused,
        marks: a.marks,
      }
@@ -103,7 +101,6 @@ class Range extends Record(DEFAULTS) {
      if ('focusOffset' in a) p.focusOffset = a.focusOffset
      if ('focusPath' in a) p.focusPath = PathUtils.create(a.focusPath)
      if ('isAtomic' in a) p.isAtomic = a.isAtomic
-      if ('isBackward' in a) p.isBackward = a.isBackward
      if ('isFocused' in a) p.isFocused = a.isFocused
      if ('marks' in a)
        p.marks = a.marks == null ? null : Mark.createSet(a.marks)
@@ -140,7 +137,6 @@ class Range extends Record(DEFAULTS) {
      focusOffset = null,
      focusPath = null,
      isAtomic = false,
-      isBackward = null,
      isFocused = false,
      marks = null,
    } = object
@@ -153,7 +149,6 @@ class Range extends Record(DEFAULTS) {
      focusOffset,
      focusPath: PathUtils.create(focusPath),
      isAtomic,
-      isBackward,
      isFocused,
      marks: marks == null ? null : new Set(marks.map(Mark.fromJSON)),
    })
@@ -228,6 +223,24 @@ class Range extends Record(DEFAULTS) {
    return !this.isCollapsed
  }

+  /**
+   * Check whether the range is backward.
+   *
+   * @return {Boolean}
+   */
+
+  get isBackward() {
+    if (this.isUnset) return null
+
+    // PERF: if the two keys are the same, we can just use the offsets.
+    if (this.anchorKey === this.focusKey) {
+      return this.anchorOffset > this.focusOffset
+    }
+
+    const isBackward = PathUtils.isBefore(this.focusPath, this.anchorPath)
+    return isBackward
+  }
+
  /**
   * Check whether the range is forward.
   *
@@ -235,30 +248,35 @@ class Range extends Record(DEFAULTS) {
   */

  get isForward() {
-    return this.isBackward == null ? null : !this.isBackward
+    const { isBackward } = this
+    return isBackward == null ? null : !isBackward
  }

  /**
-   * Check whether the range's keys are set.
-   *
-   * @return {Boolean}
-   */
-
-  get isSet() {
-    return (
-      (this.anchorKey != null && this.focusKey != null) ||
-      (this.anchorPath != null && this.focusPath != null)
-    )
-  }
-
-  /**
-   * Check whether the range's keys are not set.
+   * Check whether the range isn't set.
   *
   * @return {Boolean}
   */

  get isUnset() {
-    return !this.isSet
+    return (
+      this.anchorKey == null ||
+      this.anchorOffset == null ||
+      this.anchorPath == null ||
+      this.focusKey == null ||
+      this.focusOffset == null ||
+      this.focusPath == null
+    )
+  }
+
+  /**
+   * Check whether the range is set.
+   *
+   * @return {Boolean}
+   */
+
+  get isSet() {
+    return !this.isUnset
  }

  /**
@@ -495,7 +513,6 @@ class Range extends Record(DEFAULTS) {
      focusOffset: null,
      focusPath: null,
      isFocused: false,
-      isBackward: false,
    })
  }

@@ -513,7 +530,6 @@ class Range extends Record(DEFAULTS) {
      focusKey: this.anchorKey,
      focusOffset: this.anchorOffset,
      focusPath: this.anchorPath,
-      isBackward: this.isBackward == null ? null : !this.isBackward,
    })
  }

@@ -525,13 +541,8 @@ class Range extends Record(DEFAULTS) {
   */

  moveAnchor(n = 1) {
-    const { anchorKey, focusKey, focusOffset, isBackward } = this
    const anchorOffset = this.anchorOffset + n
-    return this.merge({
-      anchorOffset,
-      isBackward:
-        anchorKey == focusKey ? anchorOffset > focusOffset : isBackward,
-    })
+    return this.merge({ anchorOffset })
  }

  /**
@@ -542,13 +553,8 @@ class Range extends Record(DEFAULTS) {
   */

  moveFocus(n = 1) {
-    const { anchorKey, anchorOffset, focusKey, isBackward } = this
    const focusOffset = this.focusOffset + n
-    return this.merge({
-      focusOffset,
-      isBackward:
-        focusKey == anchorKey ? anchorOffset > focusOffset : isBackward,
-    })
+    return this.merge({ focusOffset })
  }

  /**
@@ -560,14 +566,7 @@ class Range extends Record(DEFAULTS) {
   */

  moveAnchorTo(key, offset) {
-    const {
-      anchorKey,
-      focusKey,
-      focusOffset,
-      anchorPath,
-      focusPath,
-      isBackward,
-    } = this
+    const { anchorKey, focusKey, anchorPath, focusPath } = this

    if (typeof key === 'string') {
      const isAnchor = key === anchorKey
@@ -576,9 +575,6 @@ class Range extends Record(DEFAULTS) {
        anchorKey: key,
        anchorPath: isFocus ? focusPath : isAnchor ? anchorPath : null,
        anchorOffset: offset,
-        isBackward: isFocus
-          ? offset > focusOffset
-          : isAnchor ? isBackward : null,
      })
    } else {
      const path = key
@@ -588,9 +584,6 @@ class Range extends Record(DEFAULTS) {
        anchorPath: path,
        anchorKey: isAnchor ? anchorKey : isFocus ? focusKey : null,
        anchorOffset: offset,
-        isBackward: isFocus
-          ? offset > focusOffset
-          : isAnchor ? isBackward : null,
      })
    }
  }
@@ -604,14 +597,7 @@ class Range extends Record(DEFAULTS) {
   */

  moveFocusTo(key, offset) {
-    const {
-      focusKey,
-      anchorKey,
-      anchorOffset,
-      anchorPath,
-      focusPath,
-      isBackward,
-    } = this
+    const { focusKey, anchorKey, anchorPath, focusPath } = this

    if (typeof key === 'string') {
      const isAnchor = key === anchorKey
@@ -620,9 +606,6 @@ class Range extends Record(DEFAULTS) {
        focusKey: key,
        focusPath: isAnchor ? anchorPath : isFocus ? focusPath : null,
        focusOffset: offset,
-        isBackward: isAnchor
-          ? offset < anchorOffset
-          : isFocus ? isBackward : null,
      })
    } else {
      const path = key
@@ -632,9 +615,6 @@ class Range extends Record(DEFAULTS) {
        focusPath: path,
        focusKey: isFocus ? focusKey : isAnchor ? anchorKey : null,
        focusOffset: offset,
-        isBackward: isAnchor
-          ? offset < anchorOffset
-          : isFocus ? isBackward : null,
      })
    }
  }
@@ -647,13 +627,7 @@ class Range extends Record(DEFAULTS) {
   */

  moveAnchorOffsetTo(anchorOffset) {
-    return this.merge({
-      anchorOffset,
-      isBackward:
-        this.anchorKey == this.focusKey
-          ? anchorOffset > this.focusOffset
-          : this.isBackward,
-    })
+    return this.merge({ anchorOffset })
  }

  /**
@@ -664,13 +638,7 @@ class Range extends Record(DEFAULTS) {
   */

  moveFocusOffsetTo(focusOffset) {
-    return this.merge({
-      focusOffset,
-      isBackward:
-        this.anchorKey == this.focusKey
-          ? this.anchorOffset > focusOffset
-          : this.isBackward,
-    })
+    return this.merge({ focusOffset })
  }

  /**
@@ -762,8 +730,8 @@ class Range extends Record(DEFAULTS) {
   */

  moveToRangeOf(start, end = start) {
-    const range = this.isBackward ? this.flip() : this
-    return range.moveAnchorToStartOf(start).moveFocusToEndOf(end)
+    const range = this.moveAnchorToStartOf(start).moveFocusToEndOf(end)
+    return range
  }

  /**
@@ -783,7 +751,6 @@ class Range extends Record(DEFAULTS) {
      focusKey,
      focusOffset,
      focusPath,
-      isBackward,
    } = range

    // If either point in the range is unset, make sure it is fully unset.
@@ -816,7 +783,6 @@ class Range extends Record(DEFAULTS) {
        focusKey: first ? first.key : null,
        focusOffset: first ? 0 : null,
        focusPath: first ? path : null,
-        isBackward: false,
      })
    }

@@ -851,12 +817,6 @@ class Range extends Record(DEFAULTS) {
    anchorPath = node.getPath(anchorKey)
    focusPath = node.getPath(focusKey)

-    // If `isBackward` is not set, derive it.
-    if (isBackward == null) {
-      const result = PathUtils.compare(anchorPath, focusPath)
-      isBackward = result === 0 ? anchorOffset > focusOffset : result === 1
-    }
-
    // Merge in any updated properties.
    return range.merge({
      anchorKey,
@@ -865,7 +825,6 @@ class Range extends Record(DEFAULTS) {
      focusKey,
      focusOffset,
      focusPath,
-      isBackward,
    })
  }

@@ -886,7 +845,6 @@ class Range extends Record(DEFAULTS) {
      focusOffset: this.focusOffset,
      focusPath: this.focusPath && this.focusPath.toArray(),
      isAtomic: this.isAtomic,
-      isBackward: this.isBackward,
      isFocused: this.isFocused,
      marks:
        this.marks == null ? null : this.marks.toArray().map(m => m.toJSON()),
--- a/packages/slate/src/models/value.js
+++ b/packages/slate/src/models/value.js
@@ -114,13 +114,14 @@ class Value extends Record(DEFAULTS) {
      data = data.merge(object.data)
    }

+    selection = document.createRange(selection)
+
    if (selection.isUnset) {
      const text = document.getFirstText()
      if (text) selection = selection.collapseToStartOf(text)
+      selection = document.createRange(selection)
    }

-    selection = document.createRange(selection)
-
    let value = new Value({
      data,
      document,
--- a/packages/slate/src/utils/path-utils.js
+++ b/packages/slate/src/utils/path-utils.js
@@ -1,26 +1,26 @@
 import { List } from 'immutable'

 /**
- * Compare paths `a` and `b` to see which is before or after.
+ * Compare paths `path` and `b` to see which is before or after.
 *
- * @param {List} a
+ * @param {List} path
 * @param {List} b
 * @return {Number|Null}
 */

-function compare(a, b) {
+function compare(path, target) {
  // PERF: if the paths are the same we can exit early.
-  if (a.size !== b.size) return null
+  if (path.size !== target.size) return null

-  for (let i = 0; i < a.size; i++) {
-    const av = a.get(i)
-    const bv = b.get(i)
+  for (let i = 0; i < path.size; i++) {
+    const pv = path.get(i)
+    const tv = target.get(i)

-    // If a's value is ever less than b's, it's before.
-    if (av < bv) return -1
+    // If the path's value is ever less than the target's, it's before.
+    if (pv < tv) return -1

-    // If b's value is ever less than a's, it's after.
-    if (av > bv) return 1
+    // If the target's value is ever less than the path's, it's after.
+    if (pv > tv) return 1
  }

  // Otherwise they were equal the whole way, it's the same.
--- a/packages/slate/test/serializers/raw/serialize/preserve-selection-and-keys.js
+++ b/packages/slate/test/serializers/raw/serialize/preserve-selection-and-keys.js
@@ -49,7 +49,6 @@ export const output = {
    focusKey: '0',
    focusPath: [0, 0],
    focusOffset: 0,
-    isBackward: false,
    isFocused: false,
    marks: null,
    isAtomic: false,
--- a/packages/slate/test/serializers/raw/serialize/preserve-selection.js
+++ b/packages/slate/test/serializers/raw/serialize/preserve-selection.js
@@ -42,7 +42,6 @@ export const output = {
    anchorOffset: 0,
    focusPath: [0, 0],
    focusOffset: 0,
-    isBackward: false,
    isFocused: false,
    marks: null,
    isAtomic: false,