From 4e7327d238c69c14fe3e5bf98f7e7b1aa65afb7d Mon Sep 17 00:00:00 2001 From: Jason Tamulonis Date: Thu, 28 May 2020 14:54:05 -0700 Subject: [PATCH] Fix inverse of sibling move operation (#3691) * Fix path transform against sibling move operations * Fix my jibberish * Add some comments --- packages/slate/src/interfaces/operation.ts | 15 ++++++++++++--- .../inverse/moveNode/backward-in-parent.js | 9 +++++++++ .../moveNode/child-to-ends-after-parent.js | 9 +++++++++ .../moveNode/child-to-ends-before-parent.js | 9 +++++++++ .../Operation/inverse/moveNode/child-to-parent.js | 12 ++++++++++++ .../moveNode/ends-after-parent-to-child.js | 9 +++++++++ .../moveNode/ends-before-parent-to-child.js | 10 ++++++++++ .../inverse/moveNode/forward-in-parent.js | 9 +++++++++ .../Operation/inverse/moveNode/non-sibling.js | 9 +++++++++ 9 files changed, 88 insertions(+), 3 deletions(-) create mode 100644 packages/slate/test/interfaces/Operation/inverse/moveNode/backward-in-parent.js create mode 100644 packages/slate/test/interfaces/Operation/inverse/moveNode/child-to-ends-after-parent.js create mode 100644 packages/slate/test/interfaces/Operation/inverse/moveNode/child-to-ends-before-parent.js create mode 100644 packages/slate/test/interfaces/Operation/inverse/moveNode/child-to-parent.js create mode 100644 packages/slate/test/interfaces/Operation/inverse/moveNode/ends-after-parent-to-child.js create mode 100644 packages/slate/test/interfaces/Operation/inverse/moveNode/ends-before-parent-to-child.js create mode 100644 packages/slate/test/interfaces/Operation/inverse/moveNode/forward-in-parent.js create mode 100644 packages/slate/test/interfaces/Operation/inverse/moveNode/non-sibling.js diff --git a/packages/slate/src/interfaces/operation.ts b/packages/slate/src/interfaces/operation.ts index c9cc22088..dd43d56de 100755 --- a/packages/slate/src/interfaces/operation.ts +++ b/packages/slate/src/interfaces/operation.ts @@ -228,9 +228,18 @@ export const Operation = { return op } - // We need to get the original path here, but sometimes the `newPath` - // is a younger sibling of (or ends before) the original, and this - // accounts for it. + // If the move happens completely within a single parent the path and + // newPath are stable with respect to each other. + if (Path.isSibling(path, newPath)) { + return { ...op, path: newPath, newPath: path } + } + + // If the move does not happen within a single parent it is possible + // for the move to impact the true path to the location where the node + // was removed from and where it was inserted. We have to adjust for this + // and find the original path. We can accomplish this (only in non-sibling) + // moves by looking at the impact of the move operation on the node + // after the original move path. const inversePath = Path.transform(path, op)! const inverseNewPath = Path.transform(Path.next(path), op)! return { ...op, path: inversePath, newPath: inverseNewPath } diff --git a/packages/slate/test/interfaces/Operation/inverse/moveNode/backward-in-parent.js b/packages/slate/test/interfaces/Operation/inverse/moveNode/backward-in-parent.js new file mode 100644 index 000000000..d731e0281 --- /dev/null +++ b/packages/slate/test/interfaces/Operation/inverse/moveNode/backward-in-parent.js @@ -0,0 +1,9 @@ +import { Operation } from 'slate' + +export const input = { type: 'move_node', path: [0, 2], newPath: [0, 1] } + +export const test = value => { + return Operation.inverse(value) +} + +export const output = { type: 'move_node', path: [0, 1], newPath: [0, 2] } diff --git a/packages/slate/test/interfaces/Operation/inverse/moveNode/child-to-ends-after-parent.js b/packages/slate/test/interfaces/Operation/inverse/moveNode/child-to-ends-after-parent.js new file mode 100644 index 000000000..ac5e12dc1 --- /dev/null +++ b/packages/slate/test/interfaces/Operation/inverse/moveNode/child-to-ends-after-parent.js @@ -0,0 +1,9 @@ +import { Operation } from 'slate' + +export const input = { type: 'move_node', path: [0, 2, 1], newPath: [0, 3] } + +export const test = value => { + return Operation.inverse(value) +} + +export const output = { type: 'move_node', path: [0, 3], newPath: [0, 2, 1] } diff --git a/packages/slate/test/interfaces/Operation/inverse/moveNode/child-to-ends-before-parent.js b/packages/slate/test/interfaces/Operation/inverse/moveNode/child-to-ends-before-parent.js new file mode 100644 index 000000000..04fca646f --- /dev/null +++ b/packages/slate/test/interfaces/Operation/inverse/moveNode/child-to-ends-before-parent.js @@ -0,0 +1,9 @@ +import { Operation } from 'slate' + +export const input = { type: 'move_node', path: [0, 2, 1], newPath: [0, 1] } + +export const test = value => { + return Operation.inverse(value) +} + +export const output = { type: 'move_node', path: [0, 1], newPath: [0, 3, 1] } diff --git a/packages/slate/test/interfaces/Operation/inverse/moveNode/child-to-parent.js b/packages/slate/test/interfaces/Operation/inverse/moveNode/child-to-parent.js new file mode 100644 index 000000000..9a74b3d79 --- /dev/null +++ b/packages/slate/test/interfaces/Operation/inverse/moveNode/child-to-parent.js @@ -0,0 +1,12 @@ +import { Operation } from 'slate' + +// This test covers moving a child to the location of where the current parent is (not becoming its parent). +// When the move happens the child is inserted infront of its old parent causing its former parent's index to shiftp +// back within its former grandparent (now parent). +export const input = { type: 'move_node', path: [0, 2, 1], newPath: [0, 2] } + +export const test = value => { + return Operation.inverse(value) +} + +export const output = { type: 'move_node', path: [0, 2], newPath: [0, 3, 1] } diff --git a/packages/slate/test/interfaces/Operation/inverse/moveNode/ends-after-parent-to-child.js b/packages/slate/test/interfaces/Operation/inverse/moveNode/ends-after-parent-to-child.js new file mode 100644 index 000000000..8c15b0495 --- /dev/null +++ b/packages/slate/test/interfaces/Operation/inverse/moveNode/ends-after-parent-to-child.js @@ -0,0 +1,9 @@ +import { Operation } from 'slate' + +export const input = { type: 'move_node', path: [0, 3], newPath: [0, 2, 1] } + +export const test = value => { + return Operation.inverse(value) +} + +export const output = { type: 'move_node', path: [0, 2, 1], newPath: [0, 3] } diff --git a/packages/slate/test/interfaces/Operation/inverse/moveNode/ends-before-parent-to-child.js b/packages/slate/test/interfaces/Operation/inverse/moveNode/ends-before-parent-to-child.js new file mode 100644 index 000000000..1e8581af2 --- /dev/null +++ b/packages/slate/test/interfaces/Operation/inverse/moveNode/ends-before-parent-to-child.js @@ -0,0 +1,10 @@ +import { Operation } from 'slate' + +export const input = { type: 'move_node', path: [0, 1], newPath: [0, 2, 1] } + +export const test = value => { + return Operation.inverse(value) +} + +// The path has changed here because the removal of [0, 1] caused [0, 2] to shift forward into its location. +export const output = { type: 'move_node', path: [0, 1, 1], newPath: [0, 1] } diff --git a/packages/slate/test/interfaces/Operation/inverse/moveNode/forward-in-parent.js b/packages/slate/test/interfaces/Operation/inverse/moveNode/forward-in-parent.js new file mode 100644 index 000000000..499663647 --- /dev/null +++ b/packages/slate/test/interfaces/Operation/inverse/moveNode/forward-in-parent.js @@ -0,0 +1,9 @@ +import { Operation } from 'slate' + +export const input = { type: 'move_node', path: [0, 1], newPath: [0, 2] } + +export const test = value => { + return Operation.inverse(value) +} + +export const output = { type: 'move_node', path: [0, 2], newPath: [0, 1] } diff --git a/packages/slate/test/interfaces/Operation/inverse/moveNode/non-sibling.js b/packages/slate/test/interfaces/Operation/inverse/moveNode/non-sibling.js new file mode 100644 index 000000000..46a9328d5 --- /dev/null +++ b/packages/slate/test/interfaces/Operation/inverse/moveNode/non-sibling.js @@ -0,0 +1,9 @@ +import { Operation } from 'slate' + +export const input = { type: 'move_node', path: [0, 2], newPath: [1, 0, 0] } + +export const test = value => { + return Operation.inverse(value) +} + +export const output = { type: 'move_node', path: [1, 0, 0], newPath: [0, 2] }