From 2f393e594895696398d5eeb1ee6421dac6810b3b Mon Sep 17 00:00:00 2001
From: Jos de Jong <wjosdejong@gmail.com>
Date: Wed, 8 Jul 2020 12:49:12 +0200
Subject: [PATCH] Fix pasting properties inline in an object instead of at the
 bottom

---
 src/JSONEditor.svelte         |  28 +++------
 src/JSONNode.svelte           |  23 +++++---
 src/actions.js                |  32 +++++++++++
 src/utils/updateProps.js      | 104 +++++++++++++++++++++++++++-------
 src/utils/updateProps.test.js |   1 +
 5 files changed, 140 insertions(+), 48 deletions(-)

diff --git a/src/JSONEditor.svelte b/src/JSONEditor.svelte
index f6f768d..430b1f4 100644
--- a/src/JSONEditor.svelte
+++ b/src/JSONEditor.svelte
@@ -19,9 +19,7 @@
   import { createHistory } from './history.js'
   import Node from './JSONNode.svelte'
   import { expandSelection } from './selection.js'
-  import { singleton } from './singleton.js'
   import {
-    deleteIn,
     existsIn,
     getIn,
     setIn,
@@ -35,6 +33,7 @@
   import jump from './assets/jump.js/src/jump.js'
   import { syncState } from './utils/syncState.js'
   import { isObject } from './utils/typeUtils.js'
+  import { patchProps } from './utils/updateProps.js'
 
   let divContents
 
@@ -99,25 +98,7 @@
 
     // if a property is renamed (move operation), rename it in the object's props
     // so it maintains its identity and hence its index
-    operations
-      .filter(operation => {
-        return operation.op === 'move' && isEqual(
-          initial(parseJSONPointer(operation.from)),
-          initial(parseJSONPointer(operation.path))
-        )
-      })
-      .forEach(operation => {
-        const pathFrom = parseJSONPointer(operation.from)
-        const to = parseJSONPointer(operation.path)
-        const parentPath = initial(pathFrom)
-        const oldKey = last(pathFrom)
-        const newKey = last(to)
-        const props = getIn(state, parentPath.concat(STATE_PROPS))
-        const index = props.findIndex(item => item.key === oldKey)
-        if (index !== -1) {
-          state = setIn(state, parentPath.concat([STATE_PROPS, index, 'key']), newKey)
-        }
-      })
+    state = patchProps(state, operations)
 
     history.add({
       undo: documentPatchResult.revert,
@@ -303,6 +284,10 @@
     emitOnChange()
   }
 
+  function handleUpdateKey (oldKey, newKey) {
+    // should never be called on the root
+  }
+
   function handleToggleSearch() {
     showSearch = !showSearch
   }
@@ -526,6 +511,7 @@
       state={state}
       searchResult={searchResultWithActive}
       onPatch={handlePatch}
+      onUpdateKey={handleUpdateKey}
       onExpand={handleExpand}
       onLimit={handleLimit}
       onSelect={handleSelect}
diff --git a/src/JSONNode.svelte b/src/JSONNode.svelte
index d5de3a1..cb901fe 100644
--- a/src/JSONNode.svelte
+++ b/src/JSONNode.svelte
@@ -1,5 +1,6 @@
 <script>
-  import { debounce, initial, isEqual, last } from 'lodash-es'
+  import { debounce, isEqual } from 'lodash-es'
+  import { rename } from './actions.js'
   import {
     DEBOUNCE_DELAY,
     DEFAULT_LIMIT,
@@ -24,6 +25,7 @@
   export let state
   export let searchResult
   export let onPatch
+  export let onUpdateKey
   export let onExpand
   export let onLimit
   export let onSelect
@@ -98,16 +100,21 @@
 
   function updateKey () {
     const newKey = getPlainText(domKey)
-    const parentPath = initial(path)
 
-    onPatch([{
-      op: 'move',
-      from: compileJSONPointer(parentPath.concat(key)),
-      path: compileJSONPointer(parentPath.concat(newKey))
-    }])
+    // must be handled by the parent which has knowledge about the other keys
+    onUpdateKey(key, newKey)
   }
   const updateKeyDebounced = debounce(updateKey, DEBOUNCE_DELAY)
 
+  function handleUpdateKey (oldKey, newKey) {
+    const index = props.findIndex(prop => prop.key === oldKey)
+    const nextKeys = (index !== -1)
+      ? props.slice(index + 1).map(prop => prop.key)
+      : []
+
+    onPatch(rename(path, oldKey, newKey, nextKeys))
+  }
+
   function handleKeyInput (event) {
     const newKey = getPlainText(event.target)
     keyClass = getKeyClass(newKey, searchResult)
@@ -342,6 +349,7 @@
             state={state && state[index]}
             searchResult={searchResult ? searchResult[index] : undefined}
             onPatch={onPatch}
+            onUpdateKey={handleUpdateKey}
             onExpand={onExpand}
             onLimit={onLimit}
             onSelect={onSelect}
@@ -409,6 +417,7 @@
             state={state && state[prop.key]}
             searchResult={searchResult ? searchResult[prop.key] : undefined}
             onPatch={onPatch}
+            onUpdateKey={handleUpdateKey}
             onExpand={onExpand}
             onLimit={onLimit}
             onSelect={onSelect}
diff --git a/src/actions.js b/src/actions.js
index 07b3adf..a70273c 100644
--- a/src/actions.js
+++ b/src/actions.js
@@ -97,6 +97,38 @@ export function append (json, path, values) {  // TODO: find a better name and d
   }
 }
 
+/**
+ * Rename an object key
+ *
+ * @param {Path} parentPath
+ * @param {string} oldKey
+ * @param {string} newKey
+ * @param {string[]} nextKeys   A list with all keys *after* the renamed key,
+ *                              used to maintain the position of the renamed key.
+ *                              If not provided, the renamed key will be moved
+ *                              to the bottom of the list with keys.
+ * @returns {Array}
+ */
+export function rename(parentPath, oldKey, newKey, nextKeys) {
+  return [
+    // rename a key
+    {
+      op: 'move',
+      from: compileJSONPointer(parentPath.concat(oldKey)),
+      path: compileJSONPointer(parentPath.concat(newKey))
+    },
+
+    // move all lower down keys so the renamed key will maintain it's position
+    ...nextKeys.map(key => {
+      return {
+        op: 'move',
+        from: compileJSONPointer(parentPath.concat(key)),
+        path: compileJSONPointer(parentPath.concat(key))
+      }
+    })
+  ]
+}
+
 /**
  * Create a JSONPatch for an insert action.
  *
diff --git a/src/utils/updateProps.js b/src/utils/updateProps.js
index 11e9e61..67030d8 100644
--- a/src/utils/updateProps.js
+++ b/src/utils/updateProps.js
@@ -1,26 +1,90 @@
+import initial from 'lodash-es/initial.js'
+import { STATE_PROPS } from '../constants.js'
+import { deleteIn, getIn, insertAt, setIn } from './immutabilityHelpers.js'
+import { parseJSONPointer } from './jsonPointer.js'
 import { isObject } from './typeUtils.js'
-import { uniqueId } from 'lodash-es'
+import { isEqual, last, uniqueId } from 'lodash-es'
 
 export function updateProps (value, prevProps) {
-  if (isObject(value)) {
-    // copy the props that still exist
-    const props = prevProps
-      ? prevProps.filter(item => value[item.key] !== undefined) 
-      : []
-
-    // add new props
-    const prevKeys = new Set(props.map(item => item.key))
-    Object.keys(value).forEach(key => {
-      if (!prevKeys.has(key)) {
-        props.push({
-          id: uniqueId(),
-          key
-        })
-      }
-    })
-
-    return props
-  } else {
+  if (!isObject(value)) {
     return undefined
   }
+
+  // copy the props that still exist
+  const props = prevProps
+    ? prevProps.filter(item => value[item.key] !== undefined)
+    : []
+
+  // add new props
+  const prevKeys = new Set(props.map(item => item.key))
+  Object.keys(value).forEach(key => {
+    if (!prevKeys.has(key)) {
+      props.push({
+        id: uniqueId(),
+        key
+      })
+    }
+  })
+
+  return props
+}
+
+// TODO: write unit tests
+// TODO: split this function in smaller functions
+export function patchProps (state, operations) {
+  let updatedState = state
+
+  operations.map(operation => {
+    if (operation.op === 'move') {
+      if (isEqual(
+        initial(parseJSONPointer(operation.from)),
+        initial(parseJSONPointer(operation.path))
+      )) {
+        // move inside the same object
+        const pathFrom = parseJSONPointer(operation.from)
+        const pathTo = parseJSONPointer(operation.path)
+        const parentPath = initial(pathFrom)
+        const props = getIn(updatedState, parentPath.concat(STATE_PROPS))
+
+        if (props) {
+          const oldKey = last(pathFrom)
+          const newKey = last(pathTo)
+          const index = props.findIndex(item => item.key === oldKey)
+
+          if (index !== -1) {
+            if (oldKey !== newKey) {
+              // A property is renamed. Rename it in the object's props
+              // so it maintains its identity and hence its index
+              updatedState = setIn(updatedState, parentPath.concat([STATE_PROPS, index, 'key']), newKey)
+            } else {
+              // operation.from and operation.path are the same:
+              // property is moved but stays the same -> move it to the end of the props
+              const oldProp = props[index]
+              const updatedProps = insertAt(deleteIn(props, [index]), [props.length - 1], oldProp)
+
+              updatedState = setIn(updatedState, parentPath.concat([STATE_PROPS]), updatedProps)
+            }
+          }
+        }
+      }
+    }
+
+    if (operation.op === 'add') {
+      const path = parseJSONPointer(operation.path)
+      const parentPath = initial(path)
+      const props = getIn(updatedState, parentPath.concat(STATE_PROPS))
+      if (props) {
+        const key = last(path)
+        const newProp = {
+          key,
+          id: uniqueId()
+        }
+        const updatedProps = insertAt(props, [props.length], newProp)
+
+        updatedState = setIn(updatedState, parentPath.concat([STATE_PROPS]), updatedProps)
+      }
+    }
+  })
+
+  return updatedState
 }
diff --git a/src/utils/updateProps.test.js b/src/utils/updateProps.test.js
index 7159812..53262b4 100644
--- a/src/utils/updateProps.test.js
+++ b/src/utils/updateProps.test.js
@@ -9,6 +9,7 @@ describe('updateProps', () => {
 
     const props2 = updateProps({a: 1, b: 2}, props1)
     assert.deepStrictEqual(props2.map(item => item.key), ['b', 'a'])
+    assert.deepStrictEqual(props2[0], props1[0]) // b must still have the same id
   })
 
   it('updateProps (2)', () => {