From 567596643cc7aaffb09842811ea633e4ece85359 Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Wed, 9 May 2018 09:53:47 +0100 Subject: [PATCH] Electron: Handle drag and dropping notebooks to change the parent --- CliClient/app/command-rmbook.js | 2 +- CliClient/tests/models_Folder.js | 55 ++++++++++ ElectronClient/app/gui/SideBar.jsx | 101 +++++++++++++----- ReactNativeClient/lib/BaseModel.js | 9 ++ .../lib/components/screens/notes.js | 2 +- .../lib/components/shared/side-menu-shared.js | 3 +- ReactNativeClient/lib/models/Folder.js | 44 ++++++++ docs/donate/index.html | 1 + docs/index.html | 14 +-- joplin.sublime-project | 2 + readme/donate.md | 3 +- 11 files changed, 196 insertions(+), 40 deletions(-) create mode 100644 CliClient/tests/models_Folder.js diff --git a/CliClient/app/command-rmbook.js b/CliClient/app/command-rmbook.js index 6e9c48474..b141be41a 100644 --- a/CliClient/app/command-rmbook.js +++ b/CliClient/app/command-rmbook.js @@ -29,7 +29,7 @@ class Command extends BaseCommand { const folder = await app().loadItem(BaseModel.TYPE_FOLDER, pattern); if (!folder) throw new Error(_('Cannot find "%s".', pattern)); - const ok = force ? true : await this.prompt(_('Delete notebook? All notes within this notebook will also be deleted.'), { booleanAnswerDefault: 'n' }); + const ok = force ? true : await this.prompt(_('Delete notebook? All notes and sub-notebooks within this notebook will also be deleted.'), { booleanAnswerDefault: 'n' }); if (!ok) return; await Folder.delete(folder.id); diff --git a/CliClient/tests/models_Folder.js b/CliClient/tests/models_Folder.js new file mode 100644 index 000000000..01eab8df5 --- /dev/null +++ b/CliClient/tests/models_Folder.js @@ -0,0 +1,55 @@ +require('app-module-path').addPath(__dirname); + +const { time } = require('lib/time-utils.js'); +const { asyncTest, fileContentEqual, setupDatabase, setupDatabaseAndSynchronizer, db, synchronizer, fileApi, sleep, clearDatabase, switchClient, syncTargetId, objectsEqual, checkThrowAsync } = require('test-utils.js'); +const Folder = require('lib/models/Folder.js'); +const Note = require('lib/models/Note.js'); +const BaseModel = require('lib/BaseModel.js'); +const { shim } = require('lib/shim'); + +process.on('unhandledRejection', (reason, p) => { + console.log('Unhandled Rejection at: Promise', p, 'reason:', reason); +}); + +async function allItems() { + let folders = await Folder.all(); + let notes = await Note.all(); + return folders.concat(notes); +} + +describe('models_Folder', function() { + + beforeEach(async (done) => { + await setupDatabaseAndSynchronizer(1); + await switchClient(1); + done(); + }); + + it('should tell if a notebook can be nested under another one', asyncTest(async () => { + let f1 = await Folder.save({ title: "folder1" }); + let f2 = await Folder.save({ title: "folder2", parent_id: f1.id }); + let f3 = await Folder.save({ title: "folder3", parent_id: f2.id }); + let f4 = await Folder.save({ title: "folder4" }); + + expect(await Folder.canNestUnder(f1.id, f2.id)).toBe(false); + expect(await Folder.canNestUnder(f2.id, f2.id)).toBe(false); + expect(await Folder.canNestUnder(f3.id, f1.id)).toBe(true); + expect(await Folder.canNestUnder(f4.id, f1.id)).toBe(true); + expect(await Folder.canNestUnder(f2.id, f3.id)).toBe(false); + expect(await Folder.canNestUnder(f3.id, f2.id)).toBe(true); + expect(await Folder.canNestUnder(f1.id, '')).toBe(true); + expect(await Folder.canNestUnder(f2.id, '')).toBe(true); + })); + + it('should recursively delete notes and sub-notebooks', asyncTest(async () => { + let f1 = await Folder.save({ title: "folder1" }); + let f2 = await Folder.save({ title: "folder2", parent_id: f1.id }); + let n1 = await Note.save({ title: 'note1', parent_id: f2.id }); + + await Folder.delete(f1.id); + + const all = await allItems(); + expect(all.length).toBe(0); + })); + +}); \ No newline at end of file diff --git a/ElectronClient/app/gui/SideBar.jsx b/ElectronClient/app/gui/SideBar.jsx index f85c5427b..f3153076f 100644 --- a/ElectronClient/app/gui/SideBar.jsx +++ b/ElectronClient/app/gui/SideBar.jsx @@ -14,6 +14,48 @@ const MenuItem = bridge().MenuItem; const InteropServiceHelper = require("../InteropServiceHelper.js"); class SideBarComponent extends React.Component { + + + constructor() { + super(); + + this.onFolderDragStart_ = (event) => { + const folderId = event.currentTarget.getAttribute('folderid'); + if (!folderId) return; + + event.dataTransfer.setDragImage(new Image(), 1, 1); + event.dataTransfer.clearData(); + event.dataTransfer.setData('text/x-jop-folder-ids', JSON.stringify([folderId])); + }; + + this.onFolderDragOver_ = (event) => { + if (event.dataTransfer.types.indexOf("text/x-jop-note-ids") >= 0) event.preventDefault(); + if (event.dataTransfer.types.indexOf("text/x-jop-folder-ids") >= 0) event.preventDefault(); + }; + + this.onFolderDrop_ = async (event) => { + const folderId = event.currentTarget.getAttribute('folderid'); + const dt = event.dataTransfer; + if (!dt) return; + + if (dt.types.indexOf("text/x-jop-note-ids") >= 0) { + event.preventDefault(); + + const noteIds = JSON.parse(dt.getData("text/x-jop-note-ids")); + for (let i = 0; i < noteIds.length; i++) { + await Note.moveToFolder(noteIds[i], folderId); + } + } else if (dt.types.indexOf("text/x-jop-folder-ids") >= 0) { + event.preventDefault(); + + const folderIds = JSON.parse(dt.getData("text/x-jop-folder-ids")); + for (let i = 0; i < folderIds.length; i++) { + await Folder.moveToFolder(folderIds[i], folderId); + } + } + }; + } + style() { const theme = themeStyle(this.props.theme); @@ -49,7 +91,7 @@ class SideBarComponent extends React.Component { color: theme.color2, cursor: "default", opacity: 0.8, - fontFamily: theme.fontFamily, + // fontFamily: theme.fontFamily, fontSize: theme.fontSize, textDecoration: "none", paddingRight: 5, @@ -117,7 +159,7 @@ class SideBarComponent extends React.Component { let deleteMessage = ""; if (itemType === BaseModel.TYPE_FOLDER) { - deleteMessage = _("Delete notebook? All notes within this notebook will also be deleted."); + deleteMessage = _("Delete notebook? All notes and sub-notebooks within this notebook will also be deleted."); } else if (itemType === BaseModel.TYPE_TAG) { deleteMessage = _("Remove this tag from all the notes?"); } else if (itemType === BaseModel.TYPE_SEARCH) { @@ -166,6 +208,19 @@ class SideBarComponent extends React.Component { }) ); + // menu.append( + // new MenuItem({ + // label: _("Move"), + // click: async () => { + // this.props.dispatch({ + // type: "WINDOW_COMMAND", + // name: "renameFolder", + // id: itemId, + // }); + // }, + // }) + // ); + menu.append(new MenuItem({ type: "separator" })); const InteropService = require("lib/services/InteropService.js"); @@ -214,40 +269,25 @@ class SideBarComponent extends React.Component { let style = Object.assign({}, this.style().listItem); if (folder.id === Folder.conflictFolderId()) style = Object.assign(style, this.style().conflictFolder); - const onDragOver = (event, folder) => { - if (event.dataTransfer.types.indexOf("text/x-jop-note-ids") >= 0) event.preventDefault(); - }; - - const onDrop = async (event, folder) => { - if (event.dataTransfer.types.indexOf("text/x-jop-note-ids") < 0) return; - event.preventDefault(); - - const noteIds = JSON.parse(event.dataTransfer.getData("text/x-jop-note-ids")); - for (let i = 0; i < noteIds.length; i++) { - await Note.moveToFolder(noteIds[i], folder.id); - } - }; - const itemTitle = Folder.displayTitle(folder); let containerStyle = Object.assign({}, this.style().listItemContainer); - containerStyle.marginLeft = depth * 5; + containerStyle.paddingLeft = containerStyle.paddingLeft + depth * 10; if (selected) containerStyle = Object.assign(containerStyle, this.style().listItemSelected); - const expandIcon = !hasChildren ? null : [+] + let expandLinkStyle = Object.assign({}, this.style().listItemExpandIcon); + let expandIconStyle = { + visibility: hasChildren ? 'visible' : 'hidden', + } + const expandIcon = + const expandLink = hasChildren ? console.info('click')}>{expandIcon} : {expandIcon} return ( -
- { expandIcon } +
+ { expandLink } { - onDragOver(event, folder); - }} - onDrop={event => { - onDrop(event, folder); - }} href="#" data-id={folder.id} data-type={BaseModel.TYPE_FOLDER} @@ -309,11 +349,11 @@ class SideBarComponent extends React.Component { return
; } - makeHeader(key, label, iconName) { + makeHeader(key, label, iconName, extraProps = {}) { const style = this.style().header; const icon = ; return ( -
+
{icon} {label}
@@ -350,7 +390,10 @@ class SideBarComponent extends React.Component { let items = []; - items.push(this.makeHeader("folderHeader", _("Notebooks"), "fa-folder-o")); + items.push(this.makeHeader("folderHeader", _("Notebooks"), "fa-folder-o", { + onDrop: this.onFolderDrop_, + folderid: '', + })); if (this.props.folders.length) { const folderItems = shared.renderFolders(this.props, this.folderItem.bind(this)); diff --git a/ReactNativeClient/lib/BaseModel.js b/ReactNativeClient/lib/BaseModel.js index f84021bee..02c629f0c 100644 --- a/ReactNativeClient/lib/BaseModel.js +++ b/ReactNativeClient/lib/BaseModel.js @@ -44,6 +44,15 @@ class BaseModel { return null; } + // Prefer the use of this function to compare IDs as it handles the case where + // one ID is null and the other is "", in which case they are actually considered to be the same. + static idsEqual(id1, id2) { + if (!id1 && !id2) return true; + if (!id1 && !!id2) return false; + if (!!id1 && !id2) return false; + return id1 === id2; + } + static modelTypeToName(type) { for (let i = 0; i < BaseModel.typeEnum_.length; i++) { const e = BaseModel.typeEnum_[i]; diff --git a/ReactNativeClient/lib/components/screens/notes.js b/ReactNativeClient/lib/components/screens/notes.js index c94a3b2f1..1de762947 100644 --- a/ReactNativeClient/lib/components/screens/notes.js +++ b/ReactNativeClient/lib/components/screens/notes.js @@ -107,7 +107,7 @@ class NotesScreenComponent extends BaseScreenComponent { } deleteFolder_onPress(folderId) { - dialogs.confirm(this, _('Delete notebook? All notes within this notebook will also be deleted.')).then((ok) => { + dialogs.confirm(this, _('Delete notebook? All notes and sub-notebooks within this notebook will also be deleted.')).then((ok) => { if (!ok) return; Folder.delete(folderId).then(() => { diff --git a/ReactNativeClient/lib/components/shared/side-menu-shared.js b/ReactNativeClient/lib/components/shared/side-menu-shared.js index ea89f4b23..728916062 100644 --- a/ReactNativeClient/lib/components/shared/side-menu-shared.js +++ b/ReactNativeClient/lib/components/shared/side-menu-shared.js @@ -1,4 +1,5 @@ const ArrayUtils = require('lib/ArrayUtils'); +const Folder = require('lib/models/Folder'); let shared = {}; @@ -14,7 +15,7 @@ function renderFoldersRecursive_(props, renderItem, items, parentId, depth) { const folders = props.folders; for (let i = 0; i < folders.length; i++) { let folder = folders[i]; - if (folder.parent_id !== parentId) continue; + if (!Folder.idsEqual(folder.parent_id, parentId)) continue; const hasChildren = folderHasChildren_(folders, folder.id); items.push(renderItem(folder, props.selectedFolderId == folder.id && props.notesParentType == 'Folder', hasChildren, depth)); if (hasChildren) items = renderFoldersRecursive_(props, renderItem, items, folder.id, depth + 1); diff --git a/ReactNativeClient/lib/models/Folder.js b/ReactNativeClient/lib/models/Folder.js index f4824b5a9..20188eb39 100644 --- a/ReactNativeClient/lib/models/Folder.js +++ b/ReactNativeClient/lib/models/Folder.js @@ -57,6 +57,11 @@ class Folder extends BaseItem { }); } + static async subFolderIds(parentId) { + const rows = await this.db().selectAll('SELECT id FROM folders WHERE parent_id = ?', [parentId]); + return rows.map(r => r.id); + } + static async noteCount(parentId) { let r = await this.db().selectOne('SELECT count(*) as total FROM notes WHERE is_conflict = 0 AND parent_id = ?', [parentId]); return r ? r.total : 0; @@ -79,6 +84,11 @@ class Folder extends BaseItem { for (let i = 0; i < noteIds.length; i++) { await Note.delete(noteIds[i]); } + + let subFolderIds = await Folder.subFolderIds(folderId); + for (let i = 0; i < subFolderIds.length; i++) { + await Folder.delete(subFolderIds[i]); + } } await super.delete(folderId, options); @@ -101,6 +111,7 @@ class Folder extends BaseItem { return { type_: this.TYPE_FOLDER, id: this.conflictFolderId(), + parent_id: '', title: this.conflictFolderTitle(), updated_time: time.unixMs(), user_updated_time: time.unixMs(), @@ -125,6 +136,39 @@ class Folder extends BaseItem { return this.modelSelectOne('SELECT * FROM folders ORDER BY created_time DESC LIMIT 1'); } + static async canNestUnder(folderId, targetFolderId) { + if (folderId === targetFolderId) return false; + + const conflictFolderId = Folder.conflictFolderId(); + if (folderId == conflictFolderId || targetFolderId == conflictFolderId) return false; + + if (!targetFolderId) return true; + + while (true) { + let folder = await Folder.load(targetFolderId); + if (!folder.parent_id) break; + if (folder.parent_id === folderId) return false; + targetFolderId = folder.parent_id; + } + + return true; + } + + static async moveToFolder(folderId, targetFolderId) { + if (!(await this.canNestUnder(folderId, targetFolderId))) throw new Error(_('Cannot move notebook to this location')); + + // When moving a note to a different folder, the user timestamp is not updated. + // However updated_time is updated so that the note can be synced later on. + + const modifiedFolder = { + id: folderId, + parent_id: targetFolderId, + updated_time: time.unixMs(), + }; + + return Folder.save(modifiedFolder, { autoTimestamp: false }); + } + // These "duplicateCheck" and "reservedTitleCheck" should only be done when a user is // manually creating a folder. They shouldn't be done for example when the folders // are being synced to avoid any strange side-effects. Technically it's possible to diff --git a/docs/donate/index.html b/docs/donate/index.html index 2930e47e6..8a05cc960 100644 --- a/docs/donate/index.html +++ b/docs/donate/index.html @@ -260,6 +260,7 @@
  • Consider rating the app on Google Play or App Store.
  • Create of update a translation.
  • Help with the documentation.
    • +
  • Vote for or review the app on alternativeTo