diff --git a/CliClient/tests/integration_ForwardBackwardNoteHistory.js b/CliClient/tests/integration_ForwardBackwardNoteHistory.js new file mode 100644 index 0000000000..241d21a43a --- /dev/null +++ b/CliClient/tests/integration_ForwardBackwardNoteHistory.js @@ -0,0 +1,138 @@ +require('app-module-path').addPath(__dirname); +const { asyncTest, id, ids, createNTestFolders, createNTestNotes, TestApp } = require('test-utils.js'); + +const { time } = require('lib/time-utils.js'); + + +let testApp = null; + +const goBackWard = (state) => { + const lastItem = state.backwardHistoryNotes[state.backwardHistoryNotes.length - 1]; + testApp.dispatch({ type: 'FOLDER_AND_NOTE_SELECT', noteId: lastItem.id, folderId: lastItem.parent_id, historyAction: 'pop' }); +}; + +const goForward = (state) => { + const lastItem = state.forwardHistoryNotes[state.forwardHistoryNotes.length - 1]; + testApp.dispatch({ type: 'FOLDER_AND_NOTE_SELECT', noteId: lastItem.id, folderId: lastItem.parent_id, historyAction: 'push' }); +}; + +describe('integration_ForwardBackwardNoteHistory', function() { + + beforeEach(async (done) => { + testApp = new TestApp(); + await testApp.start(['--no-welcome']); + done(); + }); + + afterEach(async (done) => { + if (testApp !== null) await testApp.destroy(); + testApp = null; + done(); + }); + + it('should save history when navigating through notes', asyncTest(async () => { + // setup + const folders = await createNTestFolders(2); + await time.msleep(100); + const notes0 = await createNTestNotes(5, folders[0]); + // let notes1 = await createNTestNotes(5, folders[1]); + await time.msleep(100); + + testApp.dispatch({ type: 'FOLDER_SELECT', id: id(folders[0]) }); + await time.msleep(100); + + let state = testApp.store().getState(); + expect(state.backwardHistoryNotes).toEqual([]); + expect(state.forwardHistoryNotes).toEqual([]); + + testApp.dispatch({ type: 'NOTE_SELECT', id: notes0[3].id, historyAction: 'goto' }); + await time.msleep(100); + testApp.dispatch({ type: 'NOTE_SELECT', id: notes0[2].id, historyAction: 'goto' }); + await time.msleep(100); + testApp.dispatch({ type: 'NOTE_SELECT', id: notes0[1].id, historyAction: 'goto' }); + await time.msleep(100); + testApp.dispatch({ type: 'NOTE_SELECT', id: notes0[0].id, historyAction: 'goto' }); + await time.msleep(100); + + state = testApp.store().getState(); + expect(ids(state.backwardHistoryNotes)).toEqual(ids([notes0[4], notes0[3], notes0[2], notes0[1]])); + expect(ids(state.forwardHistoryNotes)).toEqual([]); + + goBackWard(state); + await time.msleep(100); + + state = testApp.store().getState(); + expect(ids(state.backwardHistoryNotes)).toEqual(ids([notes0[4], notes0[3], notes0[2]])); + expect(ids(state.forwardHistoryNotes)).toEqual(ids([notes0[0]])); + + goBackWard(state); + await time.msleep(100); + + state = testApp.store().getState(); + expect(ids(state.backwardHistoryNotes)).toEqual(ids([notes0[4], notes0[3]])); + expect(ids(state.forwardHistoryNotes)).toEqual(ids([notes0[0], notes0[1]])); + + goForward(state); + await time.msleep(100); + + state = testApp.store().getState(); + expect(ids(state.backwardHistoryNotes)).toEqual(ids([notes0[4], notes0[3], notes0[2]])); + expect(ids(state.forwardHistoryNotes)).toEqual(ids([notes0[0]])); + + testApp.dispatch({ type: 'NOTE_SELECT', id: notes0[4].id, historyAction: 'goto' }); + await time.msleep(100); + + state = testApp.store().getState(); + expect(ids(state.backwardHistoryNotes)).toEqual(ids([notes0[4], notes0[3], notes0[2], notes0[1]])); + expect(ids(state.forwardHistoryNotes)).toEqual([]); + + + })); + + + it('should save history when navigating through notebooks', asyncTest(async () => { + const folders = await createNTestFolders(2); + await time.msleep(100); + const notes0 = await createNTestNotes(5, folders[0]); + const notes1 = await createNTestNotes(5, folders[1]); + await time.msleep(100); + + testApp.dispatch({ type: 'FOLDER_SELECT', id: id(folders[0]) }); + await time.msleep(100); + + let state = testApp.store().getState(); + expect(state.backwardHistoryNotes).toEqual([]); + expect(state.forwardHistoryNotes).toEqual([]); + + testApp.dispatch({ type: 'FOLDER_SELECT', id: id(folders[1]), historyAction: 'goto' }); + await time.msleep(100); + + state = testApp.store().getState(); + expect(ids(state.backwardHistoryNotes)).toEqual(ids([notes0[4]])); // notes0[4] was last created + expect(ids(state.forwardHistoryNotes)).toEqual([]); + + testApp.dispatch({ type: 'FOLDER_SELECT', id: id(folders[0]), historyAction: 'goto' }); + await time.msleep(100); + + state = testApp.store().getState(); + expect(ids(state.backwardHistoryNotes)).toEqual(ids([notes0[4], notes1[4]])); + expect(state.forwardHistoryNotes).toEqual([]); + + goBackWard(state); + await time.msleep(100); + + state = testApp.store().getState(); + expect(ids(state.backwardHistoryNotes)).toEqual(ids([notes0[4]])); + expect(ids(state.forwardHistoryNotes)).toEqual(ids([notes0[4]])); + + goForward(state); + await time.msleep(100); + + state = testApp.store().getState(); + expect(ids(state.backwardHistoryNotes)).toEqual(ids([notes0[4], notes1[4]])); + expect(state.forwardHistoryNotes).toEqual([]); + + + })); + +}); diff --git a/CliClient/tests/reducer.js b/CliClient/tests/reducer.js index b17ff93f46..f18faadff4 100644 --- a/CliClient/tests/reducer.js +++ b/CliClient/tests/reducer.js @@ -36,6 +36,41 @@ function initTestState(folders, selectedFolderIndex, notes, selectedNoteIndexes, return state; } +function goToNote(notes, selectedNoteIndexes, state) { + if (selectedNoteIndexes != null) { + const selectedIds = []; + for (let i = 0; i < selectedNoteIndexes.length; i++) { + selectedIds.push(notes[selectedNoteIndexes[i]].id); + } + state = reducer(state, { type: 'NOTE_SELECT', ids: selectedIds, historyAction: 'goto' }); + } + return state; +} + +function goBackWard(state) { + if (!state.backwardHistoryNotes.length) return state; + const lastItem = state.backwardHistoryNotes[state.backwardHistoryNotes.length - 1]; + state = reducer(state, { + type: 'FOLDER_AND_NOTE_SELECT', + noteId: lastItem.id , + folderId: lastItem.parent_id , + historyAction: 'pop', + }); + return state; +} + +function goForward(state) { + if (!state.forwardHistoryNotes.length) return state; + const nextItem = state.forwardHistoryNotes[state.forwardHistoryNotes.length - 1]; + state = reducer(state, { + type: 'FOLDER_AND_NOTE_SELECT', + noteId: nextItem.id , + folderId: nextItem.parent_id , + historyAction: 'push', + }); + return state; +} + function createExpectedState(items, keepIndexes, selectedIndexes) { const expected = { items: [], selectedIds: [] }; @@ -345,4 +380,88 @@ describe('Reducer', function() { expect(state.selectedNoteIds).toEqual(expected.selectedIds); })); + it('should remove deleted note from history', asyncTest(async () => { + + // create 1 folder + const folders = await createNTestFolders(1); + // create 5 notes + const notes = await createNTestNotes(5, folders[0]); + // select the 1st folder and the 1st note + let state = initTestState(folders, 0, notes, [0]); + + // select second note + state = goToNote(notes, [1], state); + // select third note + state = goToNote(notes, [2], state); + // select fourth note + state = goToNote(notes, [3], state); + + // expect history to contain first, second and third note + expect(state.backwardHistoryNotes.length).toEqual(3); + expect(getIds(state.backwardHistoryNotes)).toEqual(getIds(notes.slice(0, 3))); + + // delete third note + state = reducer(state, { type: 'NOTE_DELETE', id: notes[2].id }); + + // expect history to not contain third note + expect(getIds(state.backwardHistoryNotes)).not.toContain(notes[2].id); + })); + + it('should remove all notes of a deleted notebook from history', asyncTest(async () => { + const folders = await createNTestFolders(2); + const notes = []; + for (let i = 0; i < folders.length; i++) { + notes.push(...await createNTestNotes(3, folders[i])); + } + + let state = initTestState(folders, 0, notes.slice(0,3), [0]); + state = goToNote(notes, [1], state); + state = goToNote(notes, [2], state); + + + // go to second folder + state = reducer(state, { type: 'FOLDER_SELECT', id: folders[1].id, historyAction: 'goto' }); + expect(getIds(state.backwardHistoryNotes)).toEqual(getIds(notes.slice(0, 3))); + + // delete the first folder + state = reducer(state, { type: 'FOLDER_DELETE', id: folders[0].id }); + + expect(getIds(state.backwardHistoryNotes)).toEqual([]); + })); + + it('should maintain history correctly when going backward and forward', asyncTest(async () => { + const folders = await createNTestFolders(2); + const notes = []; + for (let i = 0; i < folders.length; i++) { + notes.push(...await createNTestNotes(5, folders[i])); + } + + let state = initTestState(folders, 0, notes.slice(0,5), [0]); + state = goToNote(notes, [1], state); + state = goToNote(notes, [2], state); + state = goToNote(notes, [3], state); + state = goToNote(notes, [4], state); + + expect(getIds(state.backwardHistoryNotes)).toEqual(getIds(notes.slice(0, 4))); + + state = goBackWard(state); + expect(getIds(state.backwardHistoryNotes)).toEqual(getIds(notes.slice(0,3))); + expect(getIds(state.forwardHistoryNotes)).toEqual(getIds(notes.slice(4, 5))); + + state = goBackWard(state); + expect(getIds(state.backwardHistoryNotes)).toEqual(getIds(notes.slice(0,2))); + // because we push the last seen note to stack. + expect(getIds(state.forwardHistoryNotes)).toEqual(getIds([notes[4], notes[3]])); + + state = goForward(state); + expect(getIds(state.backwardHistoryNotes)).toEqual(getIds(notes.slice(0,3))); + expect(getIds(state.forwardHistoryNotes)).toEqual(getIds([notes[4]])); + + state = goForward(state); + expect(getIds(state.backwardHistoryNotes)).toEqual(getIds(notes.slice(0,4))); + expect(getIds(state.forwardHistoryNotes)).toEqual([]); + })); + + + }); diff --git a/ElectronClient/gui/MainScreen.jsx b/ElectronClient/gui/MainScreen.jsx index e57e6c8be5..8d1c6c389b 100644 --- a/ElectronClient/gui/MainScreen.jsx +++ b/ElectronClient/gui/MainScreen.jsx @@ -146,6 +146,7 @@ class MainScreenComponent extends React.Component { this.props.dispatch({ type: 'FOLDER_SELECT', id: folder.id, + historyAction: 'goto', }); } } diff --git a/ElectronClient/gui/NoteList.jsx b/ElectronClient/gui/NoteList.jsx index 28b7b261cd..6cadc081f4 100644 --- a/ElectronClient/gui/NoteList.jsx +++ b/ElectronClient/gui/NoteList.jsx @@ -114,6 +114,7 @@ class NoteListComponent extends React.Component { this.props.dispatch({ type: 'NOTE_SELECT', id: item.id, + historyAction: 'goto', }); } }; diff --git a/ElectronClient/gui/NoteText.jsx b/ElectronClient/gui/NoteText.jsx index 23827df686..48596b16cb 100644 --- a/ElectronClient/gui/NoteText.jsx +++ b/ElectronClient/gui/NoteText.jsx @@ -859,10 +859,7 @@ class NoteTextComponent extends React.Component { folderId: item.parent_id, noteId: item.id, hash: resourceUrlInfo.hash, - historyNoteAction: { - id: this.state.note.id, - parent_id: this.state.note.parent_id, - }, + historyAction: 'goto', }); } else { throw new Error(`Unsupported item type: ${item.type_}`); @@ -1685,24 +1682,39 @@ class NoteTextComponent extends React.Component { }); } - if (this.props.historyNotes.length) { - toolbarItems.push({ - tooltip: _('Back'), - iconName: 'fa-arrow-left', - onClick: () => { - if (!this.props.historyNotes.length) return; + toolbarItems.push({ + tooltip: _('Back'), + iconName: 'fa-arrow-left', + enabled: (this.props.backwardHistoryNotes.length > 0), + onClick: () => { + if (!this.props.backwardHistoryNotes.length) return; + const lastItem = this.props.backwardHistoryNotes[this.props.backwardHistoryNotes.length - 1]; + this.props.dispatch({ + type: 'FOLDER_AND_NOTE_SELECT', + folderId: lastItem.parent_id, + noteId: lastItem.id, - const lastItem = this.props.historyNotes[this.props.historyNotes.length - 1]; + historyAction: 'pop', + }); + }, + }); - this.props.dispatch({ - type: 'FOLDER_AND_NOTE_SELECT', - folderId: lastItem.parent_id, - noteId: lastItem.id, - historyNoteAction: 'pop', - }); - }, - }); - } + toolbarItems.push({ + tooltip: _('Front'), + iconName: 'fa-arrow-right', + enabled: (this.props.forwardHistoryNotes.length > 0), + onClick: () => { + if (!this.props.forwardHistoryNotes.length) return; + const nextItem = this.props.forwardHistoryNotes[this.props.forwardHistoryNotes.length - 1]; + this.props.dispatch({ + type: 'FOLDER_AND_NOTE_SELECT', + folderId: nextItem.parent_id, + noteId: nextItem.id, + + historyAction: 'push', + }); + }, + }); if (note.markup_language === MarkupToHtml.MARKUP_LANGUAGE_MARKDOWN && editorIsVisible) { toolbarItems.push({ @@ -2278,7 +2290,8 @@ const mapStateToProps = state => { watchedNoteFiles: state.watchedNoteFiles, customCss: state.customCss, lastEditorScrollPercents: state.lastEditorScrollPercents, - historyNotes: state.historyNotes, + backwardHistoryNotes: state.backwardHistoryNotes, + forwardHistoryNotes: state.forwardHistoryNotes, templates: state.templates, provisionalNoteIds: state.provisionalNoteIds, }; diff --git a/ElectronClient/gui/SideBar.jsx b/ElectronClient/gui/SideBar.jsx index 35c41ef3b1..ab00bd3ccf 100644 --- a/ElectronClient/gui/SideBar.jsx +++ b/ElectronClient/gui/SideBar.jsx @@ -413,6 +413,7 @@ class SideBarComponent extends React.Component { this.props.dispatch({ type: 'FOLDER_SELECT', id: folder ? folder.id : null, + historyAction: 'goto', }); } diff --git a/ElectronClient/plugins/GotoAnything.jsx b/ElectronClient/plugins/GotoAnything.jsx index 95cd0b61c4..f9e5482460 100644 --- a/ElectronClient/plugins/GotoAnything.jsx +++ b/ElectronClient/plugins/GotoAnything.jsx @@ -218,6 +218,7 @@ class Dialog extends React.PureComponent { type: 'FOLDER_AND_NOTE_SELECT', folderId: item.parent_id, noteId: item.id, + historyAction: 'goto', }); } else if (this.state.listType === BaseModel.TYPE_TAG) { this.props.dispatch({ @@ -228,6 +229,7 @@ class Dialog extends React.PureComponent { this.props.dispatch({ type: 'FOLDER_SELECT', id: item.id, + historyAction: 'goto', }); } } diff --git a/ReactNativeClient/lib/reducer.js b/ReactNativeClient/lib/reducer.js index 1dcda46398..d72a3409af 100644 --- a/ReactNativeClient/lib/reducer.js +++ b/ReactNativeClient/lib/reducer.js @@ -49,7 +49,8 @@ const defaultState = { resourceFetcher: { toFetchCount: 0, }, - historyNotes: [], + backwardHistoryNotes: [], + forwardHistoryNotes: [], plugins: {}, provisionalNoteIds: [], editorNoteStatuses: {}, @@ -104,6 +105,20 @@ stateUtils.lastSelectedNoteIds = function(state) { return output ? output : []; }; +stateUtils.getLastSeenNote = function(state) { + const selectedNoteIds = state.selectedNoteIds; + const notes = state.notes; + if (selectedNoteIds != null && selectedNoteIds.length > 0) { + const currNote = notes.find(note => note.id === selectedNoteIds[0]); + if (currNote != null) { + return { + id: currNote.id, + parent_id: currNote.parent_id, + }; + } + } +}; + function arrayHasEncryptedItems(array) { for (let i = 0; i < array.length; i++) { if (array[i].encryption_applied) return true; @@ -194,6 +209,15 @@ function handleItemDelete(state, action) { const newState = Object.assign({}, state); newState[listKey] = newItems; + if (listKey === 'notes') { + newState.backwardHistoryNotes = newState.backwardHistoryNotes.filter(note => note.id != action.id); + newState.forwardHistoryNotes = newState.forwardHistoryNotes.filter(note => note.id != action.id); + } + if (listKey === 'folders') { + newState.backwardHistoryNotes = newState.backwardHistoryNotes.filter(note => note.parent_id != action.id); + newState.forwardHistoryNotes = newState.forwardHistoryNotes.filter(note => note.parent_id != action.id); + } + const newIds = []; for (let i = 0; i < newSelectedIndexes.length; i++) { newIds.push(newItems[newSelectedIndexes[i]].id); @@ -253,9 +277,25 @@ function defaultNotesParentType(state, exclusion) { function changeSelectedFolder(state, action, options = null) { if (!options) options = {}; - if (!('clearNoteHistory' in options)) options.clearNoteHistory = true; const newState = Object.assign({}, state); + + // Save the last seen note so that back will return to it. + if (action.type === 'FOLDER_SELECT' && action.historyAction == 'goto') { + const backwardHistoryNotes = newState.backwardHistoryNotes.slice(); + let forwardHistoryNotes = newState.forwardHistoryNotes.slice(); + + // Don't update history if going to the same note again. + const lastSeenNote = stateUtils.getLastSeenNote(state); + if (lastSeenNote != null && action.id != lastSeenNote.id) { + forwardHistoryNotes = []; + backwardHistoryNotes.push(Object.assign({}, lastSeenNote)); + } + + newState.backwardHistoryNotes = backwardHistoryNotes; + newState.forwardHistoryNotes = forwardHistoryNotes; + } + newState.selectedFolderId = 'folderId' in action ? action.folderId : action.id; if (!newState.selectedFolderId) { newState.notesParentType = defaultNotesParentType(state, 'Folder'); @@ -265,7 +305,6 @@ function changeSelectedFolder(state, action, options = null) { if (newState.selectedFolderId === state.selectedFolderId && newState.notesParentType === state.notesParentType) return state; - if (options.clearNoteHistory) newState.historyNotes = []; if (options.clearSelectedNoteIds) newState.selectedNoteIds = []; return newState; @@ -285,7 +324,6 @@ function recordLastSelectedNoteIds(state, noteIds) { function changeSelectedNotes(state, action, options = null) { if (!options) options = {}; - if (!('clearNoteHistory' in options)) options.clearNoteHistory = true; let noteIds = []; if (action.id) noteIds = [action.id]; @@ -295,9 +333,39 @@ function changeSelectedNotes(state, action, options = null) { let newState = Object.assign({}, state); if (action.type === 'NOTE_SELECT') { - if (JSON.stringify(newState.selectedNoteIds) === JSON.stringify(noteIds)) return state; newState.selectedNoteIds = noteIds; newState.selectedNoteHash = action.hash ? action.hash : ''; + + const backwardHistoryNotes = newState.backwardHistoryNotes.slice(); + let forwardHistoryNotes = newState.forwardHistoryNotes.slice(); + + // The historyAction property is only used for user-initiated actions and tells how + // the history stack should be handled. That property should not be present for + // programmatic navigation. Possible values are: + // - "goto": When going to a note, but not via the back/forward arrows. + // - "pop": When clicking on the Back arrow + // - "push": When clicking on the Forward arrow + const lastSeenNote = stateUtils.getLastSeenNote(state); + if (action.historyAction == 'goto' && lastSeenNote != null && action.id != lastSeenNote.id) { + forwardHistoryNotes = []; + backwardHistoryNotes.push(Object.assign({}, lastSeenNote)); + } else if (action.historyAction === 'pop' && lastSeenNote != null) { + if (forwardHistoryNotes.length === 0 || lastSeenNote.id != forwardHistoryNotes[forwardHistoryNotes.length - 1].id) { + forwardHistoryNotes.push(Object.assign({}, lastSeenNote)); + } + backwardHistoryNotes.pop(); + } else if (action.historyAction === 'push' && lastSeenNote != null) { + if (backwardHistoryNotes.length === 0 || lastSeenNote.id != backwardHistoryNotes[backwardHistoryNotes.length - 1].id) { + backwardHistoryNotes.push(Object.assign({}, lastSeenNote)); + } + forwardHistoryNotes.pop(); + } + + newState.backwardHistoryNotes = backwardHistoryNotes; + newState.forwardHistoryNotes = forwardHistoryNotes; + + return newState; + } else if (action.type === 'NOTE_SELECT_ADD') { if (!noteIds.length) return state; newState.selectedNoteIds = ArrayUtils.unique(newState.selectedNoteIds.concat(noteIds)); @@ -326,8 +394,6 @@ function changeSelectedNotes(state, action, options = null) { newState = recordLastSelectedNoteIds(newState, newState.selectedNoteIds); - if (options.clearNoteHistory) newState.historyNotes = []; - return newState; } @@ -416,24 +482,9 @@ const reducer = (state = defaultState, action) => { case 'FOLDER_AND_NOTE_SELECT': { - newState = changeSelectedFolder(state, action, { clearNoteHistory: false }); + newState = changeSelectedFolder(state, action); const noteSelectAction = Object.assign({}, action, { type: 'NOTE_SELECT' }); - newState = changeSelectedNotes(newState, noteSelectAction, { clearNoteHistory: false }); - - if (action.historyNoteAction) { - const historyNotes = newState.historyNotes.slice(); - if (typeof action.historyNoteAction === 'object') { - historyNotes.push(Object.assign({}, action.historyNoteAction)); - } else if (action.historyNoteAction === 'pop') { - historyNotes.pop(); - } - newState.historyNotes = historyNotes; - } else if (newState !== state) { - // Clear the note history if folder and selected note have actually been changed. For example - // they won't change if they are already selected. That way, the "Back" button to go to the - // previous note wll stay. - newState.historyNotes = []; - } + newState = changeSelectedNotes(newState, noteSelectAction); } break; @@ -734,14 +785,25 @@ const reducer = (state = defaultState, action) => { break; case 'SEARCH_SELECT': - newState = Object.assign({}, state); - newState.selectedSearchId = action.id; - if (!action.id) { - newState.notesParentType = defaultNotesParentType(state, 'Search'); - } else { - newState.notesParentType = 'Search'; + { + newState = Object.assign({}, state); + newState.selectedSearchId = action.id; + if (!action.id) { + newState.notesParentType = defaultNotesParentType(state, 'Search'); + } else { + newState.notesParentType = 'Search'; + } + + // Update history when searching + const lastSeenNote = stateUtils.getLastSeenNote(state); + if (lastSeenNote != null && (state.backwardHistoryNotes.length === 0 || + state.backwardHistoryNotes[state.backwardHistoryNotes.length - 1].id != lastSeenNote.id)) { + newState.forwardHistoryNotes = []; + newState.backwardHistoryNotes.push(Object.assign({},lastSeenNote)); + } + + newState.selectedNoteIds = []; } - newState.selectedNoteIds = []; break; case 'APP_STATE_SET':