From 1faf2c6ecdaf18d083f8218af521d5ebef084a1f Mon Sep 17 00:00:00 2001 From: zadam Date: Mon, 18 Mar 2019 21:59:53 +0100 Subject: [PATCH] fixes for some note moving edge cases, closes #454 --- src/public/javascripts/services/branches.js | 94 ++++++++++++++----- src/public/javascripts/services/tree.js | 3 +- .../javascripts/services/tree_builder.js | 10 +- src/public/javascripts/services/tree_cache.js | 16 +++- .../javascripts/services/tree_keybindings.js | 4 +- 5 files changed, 92 insertions(+), 35 deletions(-) diff --git a/src/public/javascripts/services/branches.js b/src/public/javascripts/services/branches.js index 062515f34..a9ab8df83 100644 --- a/src/public/javascripts/services/branches.js +++ b/src/public/javascripts/services/branches.js @@ -20,7 +20,11 @@ async function moveBeforeNode(nodesToMove, beforeNode) { return; } - await changeNode(nodeToMove, node => node.moveTo(beforeNode, 'before')); + await changeNode( + node => node.moveTo(beforeNode, 'before'), + nodeToMove, + beforeNode.data.noteId, + null); } } @@ -42,7 +46,11 @@ async function moveAfterNode(nodesToMove, afterNode) { return; } - await changeNode(nodeToMove, node => node.moveTo(afterNode, 'after')); + await changeNode( + node => node.moveTo(afterNode, 'after'), + nodeToMove, + null, + afterNode.data.noteId); } } @@ -57,20 +65,14 @@ async function moveToNode(nodesToMove, toNode) { return; } - await changeNode(nodeToMove, async node => { - // first expand which will force lazy load and only then move the node - // if this is not expanded before moving, then lazy load won't happen because it already contains node - // this doesn't work if this isn't a folder yet, that's why we expand second time below - await toNode.setExpanded(true); + await changeNode(async node => { + // first expand which will force lazy load and only then move the node + // if this is not expanded before moving, then lazy load won't happen because it already contains node + // this doesn't work if this isn't a folder yet, that's why we expand second time below + await toNode.setExpanded(true); - node.moveTo(toNode); - - toNode.folder = true; - toNode.renderTitle(); - - // this expands the note in case it become the folder only after the move - await toNode.setExpanded(true); - }); + node.moveTo(toNode); + }, nodeToMove); } } @@ -131,22 +133,72 @@ async function moveNodeUpInHierarchy(node) { node.getParent().renderTitle(); } - await changeNode(node, node => node.moveTo(node.getParent(), 'after')); + await changeNode( + node => node.moveTo(node.getParent(), 'after'), + node); } -async function changeNode(node, func) { - utils.assertArguments(node.data.parentNoteId, node.data.noteId); +async function changeNode(func, node, beforeNoteId = null, afterNoteId = null) { + utils.assertArguments(func, node); const childNoteId = node.data.noteId; - const oldParentNoteId = node.data.parentNoteId; + const thisOldParentNode = node.getParent(); + // this will move the node the user is directly operating on to the desired location + // note that there might be other instances of this note in the tree and those are updated through + // force reloading. We could simplify our lives by just reloading this one as well, but that leads + // to flickering and not good user experience. Current solution leads to no-flicker experience in most + // cases (since cloning is not used that often) and correct for multi-clone use cases await func(node); - const newParentNoteId = node.data.parentNoteId = utils.isTopLevelNode(node) ? 'root' : node.getParent().data.noteId; + const thisNewParentNode = node.getParent(); - await treeCache.moveNote(childNoteId, oldParentNoteId, newParentNoteId); + node.data.parentNoteId = thisNewParentNode.data.noteId; + + await treeCache.moveNote(childNoteId, thisOldParentNode.data.noteId, thisNewParentNode.data.noteId, beforeNoteId, afterNoteId); treeService.setCurrentNotePathToHash(node); + + if (!thisOldParentNode.getChildren() || thisOldParentNode.getChildren().length === 0) { + thisOldParentNode.folder = false; + thisOldParentNode.renderTitle(); + } + + if (!thisNewParentNode.folder) { + thisNewParentNode.folder = true; + thisNewParentNode.renderTitle(); + } + + if (!thisNewParentNode.isExpanded()) { + // this expands the note in case it become the folder only after the move + await thisNewParentNode.setExpanded(true); + } + + for (const newParentNode of treeService.getNodesByNoteId(thisNewParentNode.data.noteId)) { + if (newParentNode.key === thisNewParentNode.key) { + // this one has been handled above specifically + continue; + } + + newParentNode.load(true); // force reload to show up new note + newParentNode.folder = true; + newParentNode.renderTitle(); + } + + for (const oldParentNode of treeService.getNodesByNoteId(thisOldParentNode.data.noteId)) { + if (oldParentNode.key === thisOldParentNode.key) { + // this one has been handled above specifically + continue; + } + + await oldParentNode.load(true); // force reload to show up new note + + if (oldParentNode.getChildren().length === 0) { + oldParentNode.folder = false; + } + + oldParentNode.renderTitle(); + } } export default { diff --git a/src/public/javascripts/services/tree.js b/src/public/javascripts/services/tree.js index de7c45f25..c55dcd674 100644 --- a/src/public/javascripts/services/tree.js +++ b/src/public/javascripts/services/tree.js @@ -756,5 +756,6 @@ export default { loadTree, treeInitialized, setExpandedToServer, - getHashValueFromAddress + getHashValueFromAddress, + getNodesByNoteId }; \ No newline at end of file diff --git a/src/public/javascripts/services/tree_builder.js b/src/public/javascripts/services/tree_builder.js index a7b196abf..349e14e1d 100644 --- a/src/public/javascripts/services/tree_builder.js +++ b/src/public/javascripts/services/tree_builder.js @@ -86,18 +86,12 @@ async function prepareNode(branch) { extraClasses: await getExtraClasses(note), icon: await getIcon(note), refKey: note.noteId, - expanded: branch.isExpanded || hoistedNoteId === note.noteId + expanded: branch.isExpanded || hoistedNoteId === note.noteId, + lazy: true }; if (note.hasChildren() || note.type === 'search') { node.folder = true; - - if (node.expanded && note.type !== 'search') { - node.children = await prepareRealBranch(note); - } - else { - node.lazy = true; - } } return node; diff --git a/src/public/javascripts/services/tree_cache.js b/src/public/javascripts/services/tree_cache.js index 077c7996d..000de702b 100644 --- a/src/public/javascripts/services/tree_cache.js +++ b/src/public/javascripts/services/tree_cache.js @@ -151,7 +151,7 @@ class TreeCache { } /* Move note from one parent to another. */ - async moveNote(childNoteId, oldParentNoteId, newParentNoteId) { + async moveNote(childNoteId, oldParentNoteId, newParentNoteId, beforeNoteId, afterNoteId) { utils.assertArguments(childNoteId, oldParentNoteId, newParentNoteId); if (oldParentNoteId === newParentNoteId) { @@ -172,8 +172,18 @@ class TreeCache { // add new associations treeCache.parents[childNoteId].push(newParentNoteId); - treeCache.children[newParentNoteId] = treeCache.children[newParentNoteId] || []; // this might be first child - treeCache.children[newParentNoteId].push(childNoteId); + const children = treeCache.children[newParentNoteId] = treeCache.children[newParentNoteId] || []; // this might be first child + + // we try to put the note into precise order which might be used again by lazy-loaded nodes + if (beforeNoteId && children.includes(beforeNoteId)) { + children.splice(children.indexOf(beforeNoteId), 0, childNoteId); + } + else if (afterNoteId && children.includes(afterNoteId)) { + children.splice(children.indexOf(afterNoteId) + 1, 0, childNoteId); + } + else { + children.push(childNoteId); + } } } diff --git a/src/public/javascripts/services/tree_keybindings.js b/src/public/javascripts/services/tree_keybindings.js index b5c63ca73..16281ee14 100644 --- a/src/public/javascripts/services/tree_keybindings.js +++ b/src/public/javascripts/services/tree_keybindings.js @@ -20,7 +20,7 @@ const keyBindings = { return false; }, "ctrl+down": node => { - let afterNode = node.getNextSibling(); + const afterNode = node.getNextSibling(); if (afterNode !== null) { treeChangesService.moveAfterNode([node], afterNode); } @@ -33,7 +33,7 @@ const keyBindings = { return false; }, "ctrl+right": node => { - let toNode = node.getPrevSibling(); + const toNode = node.getPrevSibling(); if (toNode !== null) { treeChangesService.moveToNode([node], toNode);