From eda5c02e79c3ee2807b5abc51002ca72af5c6a93 Mon Sep 17 00:00:00 2001
From: DamareYoh <34608378+DamareYoh@users.noreply.github.com>
Date: Mon, 5 Feb 2018 10:16:55 -0800
Subject: [PATCH] Bugfix/undo merge node (#1594)

* fixed build for windows

* fixed issue where undo of merge node does not restore the node back to its original properties

* fixed lint issue

* updated operation docs for additional property on split_node and merge_node

* finished incomplete sentence in the docs.

* updated test to also verify data is restored

* renamed the 'original' property to 'properties' to be more consistent with similar operation interfaces, updated docs

* got rid of extra operations property.

* deserializing properties in merge_node and split_node, passing properties object in splitNodeByKey

* missed committing operations modles.

* updated operations.toJSON for new properties on merge_node and split_node

* fix linting error

* remove outdated comment.

* expanded check for split node inverse to include inline nodes

* partially revert update to test with deletion across inlines
---
 docs/reference/slate/operation.md             |  8 ++++---
 packages/slate/src/changes/by-key.js          | 12 ++++++++++
 .../src/constants/operation-attributes.js     |  2 ++
 packages/slate/src/models/operation.js        | 22 +++++++++++++++++++
 packages/slate/src/operations/apply.js        |  8 ++++++-
 .../test/history/undo/delete-across-blocks.js |  8 ++++---
 .../history/undo/delete-across-inlines.js     |  4 ++--
 7 files changed, 55 insertions(+), 9 deletions(-)

diff --git a/docs/reference/slate/operation.md b/docs/reference/slate/operation.md
index dbee26f88..cbc905b57 100644
--- a/docs/reference/slate/operation.md
+++ b/docs/reference/slate/operation.md
@@ -104,11 +104,12 @@ Insert a new `node` at `path`.
 {
   type: 'merge_node',
   path: Array,
-  position: Number
+  position: Number,
+  properties: Object,
 }
 ```
 
-Merge the node at `path` with its previous sibling. The `position` refers to either the index in the child nodes of the previous sibling in the case of [`Block`](./block.md) or [`Inline`](./inline.md) nodes, and the index in the characters of the previous sibling in the case of [`Text`](./text.md) nodes.
+Merge the node at `path` with its previous sibling. The `position` refers to either the index in the child nodes of the previous sibling in the case of [`Block`](./block.md) or [`Inline`](./inline.md) nodes, and the index in the characters of the previous sibling in the case of [`Text`](./text.md) nodes. The `properties` object contains properties of the merged node in the event that the change is undone.
 
 ### `move_node`
 
@@ -155,10 +156,11 @@ Set new `properties` on the node at `path`.
   path: Array,
   position: Number,
   target: Number,
+  properties: Object,
 }
 ```
 
-Split the node at `path` at `position`. The `position` refers to either the index in the child nodes in the case of [`Block`](./block.md) or [`Inline`](./inline.md) nodes, and the index in the characters in the case of [`Text`](./text.md) nodes. In the case of nested splits, `target` refers to the target path of the child split operation.
+Split the node at `path` at `position`. The `position` refers to either the index in the child nodes in the case of [`Block`](./block.md) or [`Inline`](./inline.md) nodes, and the index in the characters in the case of [`Text`](./text.md) nodes. In the case of nested splits, `target` refers to the target path of the child split operation. The `properties` object contains properties that should be assigned to the new node created after the split operation is complete.
 
 
 ## Value Operations
diff --git a/packages/slate/src/changes/by-key.js b/packages/slate/src/changes/by-key.js
index 49f51e2f6..e9ac3a7d1 100644
--- a/packages/slate/src/changes/by-key.js
+++ b/packages/slate/src/changes/by-key.js
@@ -174,6 +174,7 @@ Changes.mergeNodeByKey = (change, key, options = {}) => {
   const { value } = change
   const { document } = value
   const path = document.getPath(key)
+  const original = document.getDescendant(key)
   const previous = document.getPreviousSibling(key)
 
   if (!previous) {
@@ -187,6 +188,12 @@ Changes.mergeNodeByKey = (change, key, options = {}) => {
     value,
     path,
     position,
+    // for undos to succeed we only need the type and data because
+    // these are the only properties that get changed in the merge operation
+    properties: {
+      type: original.type,
+      data: original.data,
+    },
     target: null,
   })
 
@@ -504,12 +511,17 @@ Changes.splitNodeByKey = (change, key, position, options = {}) => {
   const { value } = change
   const { document } = value
   const path = document.getPath(key)
+  const node = document.getDescendantAtPath(path)
 
   change.applyOperation({
     type: 'split_node',
     value,
     path,
     position,
+    properties: {
+      type: node.type,
+      data: node.data,
+    },
     target,
   })
 
diff --git a/packages/slate/src/constants/operation-attributes.js b/packages/slate/src/constants/operation-attributes.js
index 1a62f0bef..894848804 100644
--- a/packages/slate/src/constants/operation-attributes.js
+++ b/packages/slate/src/constants/operation-attributes.js
@@ -29,6 +29,7 @@ const OPERATION_ATTRIBUTES = {
     'value',
     'path',
     'position',
+    'properties',
     'target',
   ],
   move_node: [
@@ -82,6 +83,7 @@ const OPERATION_ATTRIBUTES = {
     'value',
     'path',
     'position',
+    'properties',
     'target',
   ],
 }
diff --git a/packages/slate/src/models/operation.js b/packages/slate/src/models/operation.js
index 8bb220a0c..2b10a4d1c 100644
--- a/packages/slate/src/models/operation.js
+++ b/packages/slate/src/models/operation.js
@@ -130,6 +130,10 @@ class Operation extends Record(DEFAULTS) {
         v = Value.create(v)
       }
 
+      if (key == 'properties' && type == 'merge_node') {
+        v = Node.createProperties(v)
+      }
+
       if (key == 'properties' && type == 'set_mark') {
         v = Mark.createProperties(v)
       }
@@ -159,6 +163,10 @@ class Operation extends Record(DEFAULTS) {
         v = Value.createProperties(v)
       }
 
+      if (key == 'properties' && type == 'split_node') {
+        v = Node.createProperties(v)
+      }
+
       attrs[key] = v
     }
 
@@ -235,6 +243,13 @@ class Operation extends Record(DEFAULTS) {
         value = value.toJSON()
       }
 
+      if (key == 'properties' && type == 'merge_node') {
+        const v = {}
+        if ('data' in value) v.data = value.data.toJS()
+        if ('type' in value) v.type = value.type
+        value = v
+      }
+
       if (key == 'properties' && type == 'set_mark') {
         const v = {}
         if ('data' in value) v.data = value.data.toJS()
@@ -270,6 +285,13 @@ class Operation extends Record(DEFAULTS) {
         value = v
       }
 
+      if (key == 'properties' && type == 'split_node') {
+        const v = {}
+        if ('data' in value) v.data = value.data.toJS()
+        if ('type' in value) v.type = value.type
+        value = v
+      }
+
       json[key] = value
     }
 
diff --git a/packages/slate/src/operations/apply.js b/packages/slate/src/operations/apply.js
index 9fe5a608e..6be000570 100644
--- a/packages/slate/src/operations/apply.js
+++ b/packages/slate/src/operations/apply.js
@@ -395,7 +395,7 @@ const APPLIERS = {
    */
 
   split_node(value, operation) {
-    const { path, position } = operation
+    const { path, position, properties } = operation
     let { document, selection } = value
 
     // Calculate a few things...
@@ -405,6 +405,12 @@ const APPLIERS = {
 
     // Split the node by its parent.
     parent = parent.splitNode(index, position)
+    if (properties) {
+      const splitNode = parent.nodes.get(index + 1)
+      if (splitNode.object !== 'text') {
+        parent = parent.updateNode(splitNode.merge(properties))
+      }
+    }
     document = document.updateNode(parent)
 
     // Determine whether we need to update the selection...
diff --git a/packages/slate/test/history/undo/delete-across-blocks.js b/packages/slate/test/history/undo/delete-across-blocks.js
index 40fa3db58..73b2ffedd 100644
--- a/packages/slate/test/history/undo/delete-across-blocks.js
+++ b/packages/slate/test/history/undo/delete-across-blocks.js
@@ -12,15 +12,17 @@ export default function (value) {
     .value
 }
 
+// the paragraph and code blocks have some random data
+// to verify that the data objects get restored to what they were after undo
 export const input = (
   <value>
     <document>
-      <paragraph>
+      <paragraph data={{ key: 'value' }}>
         o<anchor />ne
       </paragraph>
-      <paragraph>
+      <code data={{ key2: 'value2' }}>
         tw<focus />o
-      </paragraph>
+      </code>
     </document>
   </value>
 )
diff --git a/packages/slate/test/history/undo/delete-across-inlines.js b/packages/slate/test/history/undo/delete-across-inlines.js
index 483df4994..bd84dae40 100644
--- a/packages/slate/test/history/undo/delete-across-inlines.js
+++ b/packages/slate/test/history/undo/delete-across-inlines.js
@@ -16,10 +16,10 @@ export const input = (
   <value>
     <document>
       <paragraph>
-        <link>o<anchor />ne</link>
+        <link data={{ url: 'https://www.google.com' }}>o<anchor />ne</link>
       </paragraph>
       <paragraph>
-        <link>tw<focus />o</link>
+        <link data={{ url: 'https://www.github.com' }}>tw<focus />o</link>
       </paragraph>
     </document>
   </value>