From fbb35438186b0d0a0cee093ea882695d67150489 Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Wed, 17 Jul 2019 22:42:53 +0100 Subject: [PATCH] Desktop: Fixed race condition when loading a note while another one is still loading. Improved performance when loading large note. --- ElectronClient/app/gui/NoteText.jsx | 181 +++++++++++++++------------- 1 file changed, 94 insertions(+), 87 deletions(-) diff --git a/ElectronClient/app/gui/NoteText.jsx b/ElectronClient/app/gui/NoteText.jsx index a78ddcbfac..9e325a51e6 100644 --- a/ElectronClient/app/gui/NoteText.jsx +++ b/ElectronClient/app/gui/NoteText.jsx @@ -71,6 +71,7 @@ class NoteTextComponent extends React.Component { newNote: null, noteTags: [], showRevisions: false, + loading: false, // If the current note was just created, and the title has never been // changed by the user, this variable contains that note ID. Used @@ -95,6 +96,7 @@ class NoteTextComponent extends React.Component { this.lastSetMarkers_ = ''; this.lastSetMarkersOptions_ = {}; this.selectionRange_ = null; + this.lastComponentUpdateNoteId_ = null; this.noteSearchBar_ = React.createRef(); // Complicated but reliable method to get editor content height @@ -388,6 +390,14 @@ class NoteTextComponent extends React.Component { this.webviewRef().closeDevTools(); } } + + const currentNoteId = this.state.note ? this.state.note.id : null; + if (this.lastComponentUpdateNoteId_ !== currentNoteId) { + const undoManager = this.editor_.editor.getSession().getUndoManager(); + undoManager.reset(); + this.editor_.editor.getSession().setUndoManager(undoManager); + this.lastComponentUpdateNoteId_ = currentNoteId; + } } webviewRef() { @@ -397,6 +407,8 @@ class NoteTextComponent extends React.Component { } async saveIfNeeded(saveIfNewNote = false, options = {}) { + if (this.state.loading) return; + const forceSave = saveIfNewNote && (this.state.note && !this.state.note.id); if (this.scheduleSaveTimeout_) clearTimeout(this.scheduleSaveTimeout_); @@ -447,6 +459,12 @@ class NoteTextComponent extends React.Component { await this.saveIfNeeded(); + const defer = () => { + this.setState({ loading: false }); + } + + this.setState({ loading: true }); + const previousNote = this.state.note ? Object.assign({}, this.state.note) : null; const stateNoteId = this.state.note ? this.state.note.id : null; @@ -470,14 +488,14 @@ class NoteTextComponent extends React.Component { noteTags = await Tag.tagsByNoteId(noteId); this.lastLoadedNoteId_ = noteId; note = noteId ? await Note.load(noteId) : null; - if (noteId !== this.lastLoadedNoteId_) return; // Race condition - current note was changed while this one was loading - if (options.noReloadIfLocalChanges && this.isModified()) return; + if (noteId !== this.lastLoadedNoteId_) return defer(); // Race condition - current note was changed while this one was loading + if (options.noReloadIfLocalChanges && this.isModified()) return defer(); // If the note hasn't been changed, exit now if (this.state.note && note) { let diff = Note.diffObjects(this.state.note, note); delete diff.type_; - if (!Object.getOwnPropertyNames(diff).length) return; + if (!Object.getOwnPropertyNames(diff).length) return defer(); } } @@ -515,22 +533,6 @@ class NoteTextComponent extends React.Component { } if (this.editor_) { - // Calling setValue here does two things: - // 1. It sets the initial value as recorded by the undo manager. If we were to set it instead to "" and wait for the render - // phase to set the value, the initial value would still be "", which means pressing "undo" on a note that has just loaded - // would clear it. - // 2. It resets the undo manager - fixes https://github.com/laurent22/joplin/issues/355 - // Note: calling undoManager.reset() doesn't work - try { - this.editor_.editor.getSession().setValue(note && note.body? note.body : ''); - } catch (error) { - if (error.message === "Cannot read property 'match' of undefined") { - // The internals of Ace Editor throws an exception when creating a new note, - // but that can be ignored. - } else { - console.error(error); - } - } this.editor_.editor.clearSelection(); this.editor_.editor.moveCursorTo(0,0); @@ -595,7 +597,9 @@ class NoteTextComponent extends React.Component { // if (newState.note) await shared.refreshAttachedResources(this, newState.note.body); - this.updateHtml(newState.note ? newState.note.markup_language : null, newState.note ? newState.note.body : ''); + await this.updateHtml(newState.note ? newState.note.markup_language : null, newState.note ? newState.note.body : ''); + + defer(); } async componentWillReceiveProps(nextProps) { @@ -1416,6 +1420,8 @@ class NoteTextComponent extends React.Component { } createToolbarItems(note) { + const markupLanguage = note.markup_language; + const toolbarItems = []; if (note && this.state.folder && ['Search', 'Tag'].includes(this.props.notesParentType)) { toolbarItems.push({ @@ -1428,7 +1434,6 @@ class NoteTextComponent extends React.Component { noteId: note.id, }); }, - // enabled: false, }); } @@ -1451,83 +1456,85 @@ class NoteTextComponent extends React.Component { }); } - toolbarItems.push({ - tooltip: _('Bold'), - iconName: 'fa-bold', - onClick: () => { return this.commandTextBold(); }, - }); + if (note.markup_language === Note.MARKUP_LANGUAGE_MARKDOWN) { + toolbarItems.push({ + tooltip: _('Bold'), + iconName: 'fa-bold', + onClick: () => { return this.commandTextBold(); }, + }); - toolbarItems.push({ - tooltip: _('Italic'), - iconName: 'fa-italic', - onClick: () => { return this.commandTextItalic(); }, - }); + toolbarItems.push({ + tooltip: _('Italic'), + iconName: 'fa-italic', + onClick: () => { return this.commandTextItalic(); }, + }); - toolbarItems.push({ - type: 'separator', - }); + toolbarItems.push({ + type: 'separator', + }); - toolbarItems.push({ - tooltip: _('Hyperlink'), - iconName: 'fa-link', - onClick: () => { return this.commandTextLink(); }, - }); + toolbarItems.push({ + tooltip: _('Hyperlink'), + iconName: 'fa-link', + onClick: () => { return this.commandTextLink(); }, + }); - toolbarItems.push({ - tooltip: _('Code'), - iconName: 'fa-code', - onClick: () => { return this.commandTextCode(); }, - }); + toolbarItems.push({ + tooltip: _('Code'), + iconName: 'fa-code', + onClick: () => { return this.commandTextCode(); }, + }); - toolbarItems.push({ - tooltip: _('Attach file'), - iconName: 'fa-paperclip', - onClick: () => { return this.commandAttachFile(); }, - }); + toolbarItems.push({ + tooltip: _('Attach file'), + iconName: 'fa-paperclip', + onClick: () => { return this.commandAttachFile(); }, + }); - toolbarItems.push({ - type: 'separator', - }); + toolbarItems.push({ + type: 'separator', + }); - toolbarItems.push({ - tooltip: _('Numbered List'), - iconName: 'fa-list-ol', - onClick: () => { return this.commandTextListOl(); }, - }); + toolbarItems.push({ + tooltip: _('Numbered List'), + iconName: 'fa-list-ol', + onClick: () => { return this.commandTextListOl(); }, + }); - toolbarItems.push({ - tooltip: _('Bulleted List'), - iconName: 'fa-list-ul', - onClick: () => { return this.commandTextListUl(); }, - }); + toolbarItems.push({ + tooltip: _('Bulleted List'), + iconName: 'fa-list-ul', + onClick: () => { return this.commandTextListUl(); }, + }); - toolbarItems.push({ - tooltip: _('Checkbox'), - iconName: 'fa-check-square', - onClick: () => { return this.commandTextCheckbox(); }, - }); + toolbarItems.push({ + tooltip: _('Checkbox'), + iconName: 'fa-check-square', + onClick: () => { return this.commandTextCheckbox(); }, + }); - toolbarItems.push({ - tooltip: _('Heading'), - iconName: 'fa-header', - onClick: () => { return this.commandTextHeading(); }, - }); + toolbarItems.push({ + tooltip: _('Heading'), + iconName: 'fa-header', + onClick: () => { return this.commandTextHeading(); }, + }); - toolbarItems.push({ - tooltip: _('Horizontal Rule'), - iconName: 'fa-ellipsis-h', - onClick: () => { return this.commandTextHorizontalRule(); }, - }); + toolbarItems.push({ + tooltip: _('Horizontal Rule'), + iconName: 'fa-ellipsis-h', + onClick: () => { return this.commandTextHorizontalRule(); }, + }); - toolbarItems.push({ - tooltip: _('Insert Date Time'), - iconName: 'fa-calendar-plus-o', - onClick: () => { return this.commandDateTime(); }, - }); + toolbarItems.push({ + tooltip: _('Insert Date Time'), + iconName: 'fa-calendar-plus-o', + onClick: () => { return this.commandDateTime(); }, + }); - toolbarItems.push({ - type: 'separator', - }); + toolbarItems.push({ + type: 'separator', + }); + } if (note && this.props.watchedNoteFiles.indexOf(note.id) >= 0) { toolbarItems.push({ @@ -1814,7 +1821,7 @@ class NoteTextComponent extends React.Component { const toolbarItems = this.createToolbarItems(note); - const toolbar = markupLanguage !== Note.MARKUP_LANGUAGE_MARKDOWN ? null : @@ -1853,7 +1860,7 @@ class NoteTextComponent extends React.Component { delete editorRootStyle.fontSize; const editor =