Restored note history feature by Naveen M V <naveenmv7@gmail.com>

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([]);
+
+
+	}));
+
+});

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([]);
+	}));
+
+
+
 });

ElectronClient/gui/MainScreen.jsx

@@ -146,6 +146,7 @@ class MainScreenComponent extends React.Component {
 								this.props.dispatch({
 									type: 'FOLDER_SELECT',
 									id: folder.id,
+									historyAction: 'goto',
 								});
 							}
 						}

ElectronClient/gui/NoteList.jsx

@@ -114,6 +114,7 @@ class NoteListComponent extends React.Component {
 				this.props.dispatch({
 					type: 'NOTE_SELECT',
 					id: item.id,
+					historyAction: 'goto',
 				});
 			}
 		};

ElectronClient/gui/NoteText.jsx

@@ -859,10 +859,7 @@ class NoteTextComponent extends React.Component {
 					folderId: item.parent_id,
 					noteId: item.id,
 					hash: resourceUrlInfo.hash,
-					historyNoteAction: {
+					historyAction: 'goto',
-						id: this.state.note.id,
-						parent_id: this.state.note.parent_id,
-					},
 				});
 			} 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({
-			toolbarItems.push({
+			tooltip: _('Back'),
-				tooltip: _('Back'),
+			iconName: 'fa-arrow-left',
-				iconName: 'fa-arrow-left',
+			enabled: (this.props.backwardHistoryNotes.length > 0),
-				onClick: () => {
+			onClick: () => {
-					if (!this.props.historyNotes.length) return;
+				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({
+		toolbarItems.push({
-						type: 'FOLDER_AND_NOTE_SELECT',
+			tooltip: _('Front'),
-						folderId: lastItem.parent_id,
+			iconName: 'fa-arrow-right',
-						noteId: lastItem.id,
+			enabled: (this.props.forwardHistoryNotes.length > 0),
-						historyNoteAction: 'pop',
+			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,
 	};

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',
 		});
 	}
 

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',
 			});
 		}
 	}

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 });
+				newState = changeSelectedNotes(newState, noteSelectAction);
-
-				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 = [];
-				}
 			}
 			break;
 
@@ -734,14 +785,25 @@ const reducer = (state = defaultState, action) => {
 			break;
 
 		case 'SEARCH_SELECT':
-			newState = Object.assign({}, state);
+			{
-			newState.selectedSearchId = action.id;
+				newState = Object.assign({}, state);
-			if (!action.id) {
+				newState.selectedSearchId = action.id;
-				newState.notesParentType = defaultNotesParentType(state, 'Search');
+				if (!action.id) {
-			} else {
+					newState.notesParentType = defaultNotesParentType(state, 'Search');
-				newState.notesParentType = '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':