From 6a3e27eb625b69cb6dd155fe0c6d96094581e97d Mon Sep 17 00:00:00 2001 From: zadam Date: Mon, 24 Aug 2020 23:33:27 +0200 Subject: [PATCH] smaller refactorings in note_tree --- src/public/app/services/hoisted_note.js | 8 +- src/public/app/services/link.js | 7 +- src/public/app/services/tab_context.js | 16 +-- src/public/app/services/tree.js | 58 +++++------ src/public/app/widgets/note_tree.js | 125 +++++++++--------------- src/services/keyboard_actions.js | 4 +- 6 files changed, 85 insertions(+), 133 deletions(-) diff --git a/src/public/app/services/hoisted_note.js b/src/public/app/services/hoisted_note.js index 6c2aebc78..e8822ecf3 100644 --- a/src/public/app/services/hoisted_note.js +++ b/src/public/app/services/hoisted_note.js @@ -30,16 +30,16 @@ function isRootNode(node) { async function checkNoteAccess(notePath) { // notePath argument can contain only noteId which is not good when hoisted since // then we need to check the whole note path - const runNotePath = await treeService.getRunPath(notePath); + const resolvedNotePath = await treeService.resolveNotePath(notePath); - if (!runNotePath) { + if (!resolvedNotePath) { console.log("Cannot activate " + notePath); return false; } const hoistedNoteId = getHoistedNoteId(); - if (hoistedNoteId !== 'root' && !runNotePath.includes(hoistedNoteId)) { + if (hoistedNoteId !== 'root' && !resolvedNotePath.includes(hoistedNoteId)) { const confirmDialog = await import('../dialogs/confirm.js'); if (!await confirmDialog.confirm("Requested note is outside of hoisted note subtree and you must unhoist to access the note. Do you want to proceed with unhoisting?")) { @@ -60,4 +60,4 @@ export default { isTopLevelNode, isRootNode, checkNoteAccess -} \ No newline at end of file +} diff --git a/src/public/app/services/link.js b/src/public/app/services/link.js index d5f9c28b1..8879863b9 100644 --- a/src/public/app/services/link.js +++ b/src/public/app/services/link.js @@ -38,13 +38,12 @@ async function createNoteLink(notePath, options = {}) { const $container = $("").append($noteLink); if (showNotePath) { - notePath = await treeService.resolveNotePath(notePath); + const resolvedNotePathSegments = await treeService.resolveNotePathToSegments(notePath); if (notePath) { - const noteIds = notePath.split("/"); - noteIds.pop(); // remove last element + resolvedNotePathSegments.pop(); // remove last element - const parentNotePath = noteIds.join("/").trim(); + const parentNotePath = resolvedNotePathSegments.join("/").trim(); if (parentNotePath) { $container.append($("").text(" (" + await treeService.getNotePathTitle(parentNotePath) + ")")); diff --git a/src/public/app/services/tab_context.js b/src/public/app/services/tab_context.js index 17ade9aae..d47a47721 100644 --- a/src/public/app/services/tab_context.js +++ b/src/public/app/services/tab_context.js @@ -26,25 +26,25 @@ class TabContext extends Component { async setNote(inputNotePath, triggerSwitchEvent = true) { const noteId = treeService.getNoteIdFromNotePath(inputNotePath); - let notePath; + let resolvedNotePath; if ((await treeCache.getNote(noteId)).isDeleted) { // no point in trying to resolve canonical notePath - notePath = inputNotePath; + resolvedNotePath = inputNotePath; } else { - notePath = await treeService.resolveNotePath(inputNotePath); + resolvedNotePath = await treeService.resolveNotePath(inputNotePath); - if (!notePath) { + if (!resolvedNotePath) { console.error(`Cannot resolve note path ${inputNotePath}`); return; } - if (notePath === this.notePath) { + if (resolvedNotePath === this.notePath) { return; } - if (await hoistedNoteService.checkNoteAccess(notePath) === false) { + if (await hoistedNoteService.checkNoteAccess(resolvedNotePath) === false) { return; // note is outside of hoisted subtree and user chose not to unhoist } } @@ -53,7 +53,7 @@ class TabContext extends Component { utils.closeActiveDialog(); - this.notePath = notePath; + this.notePath = resolvedNotePath; this.noteId = noteId; this.autoBookDisabled = false; @@ -62,7 +62,7 @@ class TabContext extends Component { setTimeout(async () => { // we include the note into recent list only if the user stayed on the note at least 5 seconds - if (notePath && notePath === this.notePath) { + if (resolvedNotePath && resolvedNotePath === this.notePath) { await server.post('recent-notes', { noteId: this.note.noteId, notePath: this.notePath diff --git a/src/public/app/services/tree.js b/src/public/app/services/tree.js index ec9e0a1db..4c91b2c8d 100644 --- a/src/public/app/services/tree.js +++ b/src/public/app/services/tree.js @@ -6,25 +6,25 @@ import hoistedNoteService from '../services/hoisted_note.js'; import appContext from "./app_context.js"; /** - * Accepts notePath which might or might not be valid and returns an existing path as close to the original - * notePath as possible. * @return {string|null} */ async function resolveNotePath(notePath) { - const runPath = await getRunPath(notePath); + const runPath = await resolveNotePathToSegments(notePath); return runPath ? runPath.join("/") : null; } /** - * Accepts notePath and tries to resolve it. Part of the path might not be valid because of note moving (which causes + * Accepts notePath which might or might not be valid and returns an existing path as close to the original + * notePath as possible. Part of the path might not be valid because of note moving (which causes * path change) or other corruption, in that case this will try to get some other valid path to the correct note. * * @return {string[]} */ -async function getRunPath(notePath, logErrors = true) { +async function resolveNotePathToSegments(notePath, logErrors = true) { utils.assertArguments(notePath); + // we might get notePath with the tabId suffix, remove it if present notePath = notePath.split("-")[0].trim(); if (notePath.length === 0) { @@ -54,48 +54,38 @@ async function getRunPath(notePath, logErrors = true) { const child = await treeCache.getNote(childNoteId); if (!child) { - console.log("Can't find note " + childNoteId); + console.log(`Can't find note ${childNoteId}`); return; } const parents = child.getParentNotes(); - if (!parents) { - ws.logError("No parents found for " + childNoteId); + if (!parents.length) { + if (logErrors) { + ws.logError(`No parents found for ${childNoteId}`); + } + return; } if (!parents.some(p => p.noteId === parentNoteId)) { if (logErrors) { - console.log(utils.now(), "Did not find parent " + parentNoteId + " for child " + childNoteId); + console.log(utils.now(), `Did not find parent ${parentNoteId} for child ${childNoteId}, available parents: ${parents}`); } - if (parents.length > 0) { - if (logErrors) { - console.log(utils.now(), "Available parents:", parents); + const someNotePath = getSomeNotePath(parents[0]); + + if (someNotePath) { // in case it's root the path may be empty + const pathToRoot = someNotePath.split("/").reverse(); + + for (const noteId of pathToRoot) { + effectivePath.push(noteId); } - const someNotePath = getSomeNotePath(parents[0]); - - if (someNotePath) { // in case it's root the path may be empty - const pathToRoot = someNotePath.split("/").reverse(); - - for (const noteId of pathToRoot) { - effectivePath.push(noteId); - } - - effectivePath.push('root'); - } - - break; + effectivePath.push('root'); } - else { - if (logErrors) { - console.log("No parents so no run path."); - } - return; - } + break; } } @@ -136,7 +126,7 @@ function getSomeNotePath(note) { } async function sortAlphabetically(noteId) { - await server.put('notes/' + noteId + '/sort'); + await server.put(`notes/${noteId}/sort`); } ws.subscribeToMessages(message => { @@ -238,7 +228,7 @@ async function getNoteTitle(noteId, parentNoteId = null) { const branch = treeCache.getBranch(branchId); if (branch && branch.prefix) { - title = branch.prefix + ' - ' + title; + title = `${branch.prefix} - ${title}`; } } } @@ -290,8 +280,8 @@ function parseNotePath(notePath) { export default { sortAlphabetically, resolveNotePath, + resolveNotePathToSegments, getSomeNotePath, - getRunPath, getParentProtectedStatus, getNotePath, getNoteIdFromNotePath, diff --git a/src/public/app/widgets/note_tree.js b/src/public/app/widgets/note_tree.js index 3006871e0..4f99a934b 100644 --- a/src/public/app/widgets/note_tree.js +++ b/src/public/app/widgets/note_tree.js @@ -303,12 +303,12 @@ export default class NoteTreeWidget extends TabAwareWidget { this.$tree.fancytree({ titlesTabbable: true, autoScroll: true, - keyboard: false, // we takover keyboard handling in the hotkeys plugin + keyboard: true, extensions: utils.isMobile() ? ["dnd5", "clones"] : ["hotkeys", "dnd5", "clones"], source: treeData, scrollOfs: { - top: 200, - bottom: 200 + top: 100, + bottom: 100 }, scrollParent: this.$tree, minExpandLevel: 2, // root can't be collapsed @@ -372,10 +372,7 @@ export default class NoteTreeWidget extends TabAwareWidget { })); data.dataTransfer.setData("text", JSON.stringify(notes)); - - // This function MUST be defined to enable dragging for the tree. - // Return false to cancel dragging of node. - return true; + return true; // allow dragging to start }, dragEnter: (node, data) => true, // allow drop on any node dragOver: (node, data) => true, @@ -528,6 +525,38 @@ export default class NoteTreeWidget extends TabAwareWidget { } } + async prepareSearchNoteChildren(note) { + await treeCache.reloadNotes([note.noteId]); + + const newNote = await treeCache.getNote(note.noteId); + + return await this.prepareNormalNoteChildren(newNote); + } + + async prepareNormalNoteChildren(parentNote) { + utils.assertArguments(parentNote); + + const noteList = []; + + const hideArchivedNotes = this.hideArchivedNotes; + + for (const branch of this.getChildBranches(parentNote)) { + if (hideArchivedNotes) { + const note = await branch.getNote(); + + if (note.hasLabel('archived')) { + continue; + } + } + + const node = await this.prepareNode(branch); + + noteList.push(node); + } + + return noteList; + } + getIconClass(note) { const labels = note.getLabels('iconClass'); @@ -631,30 +660,6 @@ export default class NoteTreeWidget extends TabAwareWidget { } } - async prepareNormalNoteChildren(parentNote) { - utils.assertArguments(parentNote); - - const noteList = []; - - const hideArchivedNotes = this.hideArchivedNotes; - - for (const branch of this.getChildBranches(parentNote)) { - if (hideArchivedNotes) { - const note = await branch.getNote(); - - if (note.hasLabel('archived')) { - continue; - } - } - - const node = await this.prepareNode(branch); - - noteList.push(node); - } - - return noteList; - } - getChildBranches(parentNote) { let childBranches = parentNote.getChildBranches(); @@ -678,14 +683,6 @@ export default class NoteTreeWidget extends TabAwareWidget { return childBranches; } - async prepareSearchNoteChildren(note) { - await treeCache.reloadNotes([note.noteId]); - - const newNote = await treeCache.getNote(note.noteId); - - return await this.prepareNormalNoteChildren(newNote); - } - getExtraClasses(note) { utils.assertArguments(note); @@ -810,9 +807,9 @@ export default class NoteTreeWidget extends TabAwareWidget { /** @const {FancytreeNode} */ let parentNode = null; - const runPath = await treeService.getRunPath(notePath, logErrors); + const resolvedNotePathSegments = await treeService.resolveNotePathToSegments(notePath, logErrors); - if (!runPath) { + if (!resolvedNotePathSegments) { if (logErrors) { console.error("Could not find run path for notePath:", notePath); } @@ -820,7 +817,7 @@ export default class NoteTreeWidget extends TabAwareWidget { return; } - for (const childNoteId of runPath) { + for (const childNoteId of resolvedNotePathSegments) { if (childNoteId === hoistedNoteId) { // there must be exactly one node with given hoistedNoteId parentNode = this.getNodesByNoteId(childNoteId)[0]; @@ -870,16 +867,7 @@ export default class NoteTreeWidget extends TabAwareWidget { /** @return {FancytreeNode} */ findChildNode(parentNode, childNoteId) { - let foundChildNode = null; - - for (const childNode of parentNode.getChildren()) { - if (childNode.data.noteId === childNoteId) { - foundChildNode = childNode; - break; - } - } - - return foundChildNode; + return parentNode.getChildren().find(childNode => childNode.data.noteId === childNoteId); } /** @return {FancytreeNode} */ @@ -1154,6 +1142,9 @@ export default class NoteTreeWidget extends TabAwareWidget { } async setExpanded(branchId, isExpanded) { + console.log("expand", isExpanded); + + utils.assertArguments(branchId); const branch = treeCache.getBranch(branchId); @@ -1190,35 +1181,7 @@ export default class NoteTreeWidget extends TabAwareWidget { async getHotKeys() { const actions = await keyboardActionsService.getActionsForScope('note-tree'); - const hotKeyMap = { - // code below shouldn't be necessary normally, however there's some problem with interaction with context menu plugin - // after opening context menu, standard shortcuts don't work, but they are detected here - // so we essentially takeover the standard handling with our implementation. - "left": node => { - node.navigate($.ui.keyCode.LEFT, true); - this.clearSelectedNodes(); - - return false; - }, - "right": node => { - node.navigate($.ui.keyCode.RIGHT, true); - this.clearSelectedNodes(); - - return false; - }, - "up": node => { - node.navigate($.ui.keyCode.UP, true); - this.clearSelectedNodes(); - - return false; - }, - "down": node => { - node.navigate($.ui.keyCode.DOWN, true); - this.clearSelectedNodes(); - - return false; - } - }; + const hotKeyMap = {}; for (const action of actions) { for (const shortcut of action.effectiveShortcuts) { diff --git a/src/services/keyboard_actions.js b/src/services/keyboard_actions.js index 22658e473..663b0e5e2 100644 --- a/src/services/keyboard_actions.js +++ b/src/services/keyboard_actions.js @@ -368,12 +368,12 @@ const DEFAULT_KEYBOARD_ACTIONS = [ }, { actionName: "zoomOut", - defaultShortcuts: ["CommandOrControl+-"], + defaultShortcuts: isElectron ? ["CommandOrControl+-"] : [], scope: "window" }, { actionName: "zoomIn", - defaultShortcuts: ["CommandOrControl+="], + defaultShortcuts: isElectron ? ["CommandOrControl+="] : [], scope: "window" }, {