Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add note history (back/forward) #2819

Merged
merged 45 commits into from
May 15, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
0d10505
Add test for search in feature_NoteHistory
naviji Mar 20, 2020
5ea0f9d
Rename historyAction
naviji Mar 21, 2020
1c5bc1f
Add history limit of 200
naviji Mar 21, 2020
ad16d13
Save history on keydown
naviji Mar 22, 2020
7dcd255
Remove duplicates in history due to deletion
naviji Mar 22, 2020
eceaebc
Add test for adjacent duplicate deletion
naviji Mar 22, 2020
1856638
Fix showAllNotes tests
naviji Mar 23, 2020
b77e762
Add package-lock.json files
naviji Mar 23, 2020
9191be6
Clean up
naviji Mar 25, 2020
4504358
Rename lastSeenNote to currentNote
naviji Mar 25, 2020
626d966
Update goBackWard, goForward functions
naviji Mar 25, 2020
fbce314
Change expect of tests
naviji Mar 30, 2020
b4f5c3d
Merge branch 'master' of https://github.com/laurent22/joplin into his…
naviji Mar 30, 2020
90b7bf9
Fix history corruption
naviji Mar 30, 2020
57a1494
Fix problems when moving
naviji Mar 30, 2020
0ca975b
Put everything in one place
naviji Mar 30, 2020
22e02c1
New actions HISTORY_BACKWARD and HISTORY_FORWARD
naviji Mar 31, 2020
a145df5
Preserve context
naviji Apr 1, 2020
55710cd
Fix note move bug
naviji Apr 1, 2020
27cfa06
Remove redundant historyAction
naviji Apr 1, 2020
87c0de8
Clean up
naviji Apr 1, 2020
8e5d5cf
Fix bug in adjacent deletion
naviji Apr 11, 2020
dd7ee13
Add tests for moving over create, deleted and allnotes
naviji Apr 11, 2020
e3cb65a
Clean up
naviji Apr 11, 2020
2f4655a
Merge from upstream master
naviji Apr 11, 2020
ed7fc8b
Fix bug in goto
naviji Apr 11, 2020
08f30f0
Conflict partial
naviji Apr 12, 2020
9b4fcaa
Fix bug when moving selected note
naviji Apr 14, 2020
a6a5a8f
Failing conflict note test
naviji Apr 14, 2020
1db2c2b
Merge branch 'master' of https://github.com/laurent22/joplin into his…
naviji Apr 15, 2020
ccefd77
Merge branch 'master' of https://github.com/laurent22/joplin into his…
naviji Apr 17, 2020
51f4e2e
Comment out test
naviji Apr 17, 2020
c8bd10e
Comment out test
naviji Apr 17, 2020
281c536
Fix conflict notes test bug
naviji Apr 19, 2020
5894078
Merge from upstream master
naviji Apr 19, 2020
c48a61e
Clean up
naviji Apr 19, 2020
7210a1e
Clean up
naviji Apr 19, 2020
566bea6
Merge branch 'master' of https://github.com/laurent22/joplin into his…
naviji Apr 21, 2020
f625666
Remove package-lock.json
naviji Apr 21, 2020
baed477
Remove old history test file
naviji Apr 21, 2020
bab891b
Clean up
naviji Apr 21, 2020
49a4a30
Fix search bug
naviji Apr 22, 2020
4e00206
Merge from upstream master
naviji May 6, 2020
9972588
Add arrows to WYSIWYG editor
naviji May 7, 2020
ffc01e7
Simplify
naviji May 8, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
455 changes: 455 additions & 0 deletions CliClient/tests/feature_NoteHistory.js

Large diffs are not rendered by default.

238 changes: 237 additions & 1 deletion CliClient/tests/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const { setupDatabaseAndSynchronizer, switchClient, asyncTest, createNTestNotes,
const Folder = require('lib/models/Folder.js');
const Note = require('lib/models/Note.js');
const Tag = require('lib/models/Tag.js');
const { reducer, defaultState, stateUtils } = require('lib/reducer.js');
const { reducer, defaultState, stateUtils, MAX_HISTORY } = require('lib/reducer.js');

function initTestState(folders, selectedFolderIndex, notes, selectedNoteIndexes, tags = null, selectedTagIndex = null) {
let state = defaultState;
Expand Down Expand Up @@ -36,6 +36,33 @@ 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 });
}
return state;
}

function goBackWard(state) {
if (!state.backwardHistoryNotes.length) return state;
state = reducer(state, {
type: 'HISTORY_BACKWARD',
});
return state;
}

function goForward(state) {
if (!state.forwardHistoryNotes.length) return state;
state = reducer(state, {
type: 'HISTORY_FORWARD',
});
return state;
}

function createExpectedState(items, keepIndexes, selectedIndexes) {
const expected = { items: [], selectedIds: [] };

Expand Down Expand Up @@ -345,4 +372,213 @@ 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 });
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([]);
}));

it('should remember the last seen note of a notebook', 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); // last seen note is notes[4]
// go to second folder
state = reducer(state, { type: 'FOLDER_SELECT', id: folders[1].id });
state = goToNote(notes, [5], state);
state = goToNote(notes, [6], state);

// return to first folder
state = reducer(state, { type: 'FOLDER_SELECT', id: folders[0].id });

expect(state.lastSelectedNotesIds.Folder[folders[0].id]).toEqual([notes[4].id]);

// return to second folder
state = reducer(state, { type: 'FOLDER_SELECT', id: folders[1].id });
expect(state.lastSelectedNotesIds.Folder[folders[1].id]).toEqual([notes[6].id]);

}));

it('should ensure that history is free of adjacent duplicates', asyncTest(async () => {
naviji marked this conversation as resolved.
Show resolved Hide resolved
// 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]);

// backward = 0 1 2 3 2 3 2 3 2 3 2
// forward =
// current = 3
state = goToNote(notes, [1], state);
state = goToNote(notes, [2], state);
state = goToNote(notes, [3], state);
state = goToNote(notes, [2], state);
state = goToNote(notes, [3], state);
state = goToNote(notes, [2], state);
state = goToNote(notes, [3], state);
state = goToNote(notes, [2], state);
state = goToNote(notes, [3], state);
state = goToNote(notes, [2], state);
state = goToNote(notes, [3], state);

naviji marked this conversation as resolved.
Show resolved Hide resolved
// backward = 0 1 2 3 2 3 2 3 2 3
// forward = 3
// current = 2
state = goBackWard(state);

// backward = 0 1 2 3 2 3 2 3 2
// forward = 3 2
// current = 3
state = goBackWard(state);

// backward = 0 1 2 3 2 3 2 3
// forward = 3 2 3
// current = 2
state = goBackWard(state);

// backward = 0 1 2 3 2 3 2
// forward = 3 2 3 2
// current = 3
state = goBackWard(state);

expect(state.backwardHistoryNotes.map(n=>n.id)).toEqual([notes[0], notes[1], notes[2], notes[3], notes[2], notes[3], notes[2]].map(n=>n.id));
expect(state.forwardHistoryNotes.map(n=>n.id)).toEqual([notes[3], notes[2], notes[3], notes[2]].map(n=>n.id));
expect(state.selectedNoteIds).toEqual([notes[3].id]);

// delete third note
state = reducer(state, { type: 'NOTE_DELETE', id: notes[2].id });

// if adjacent duplicates not removed
// backward = 0 1 3 3
// forward = 3 3
// current = 3

// if adjacent duplicates are removed
// backward = 0 1 3
// forward = 3
// current = 3

// Expected: adjacent duplicates are removed and latest history does not contain current note
// backward = 0 1
// forward =
// current = 3
expect(state.backwardHistoryNotes.map(x => x.id)).toEqual([notes[0].id, notes[1].id]);
expect(state.forwardHistoryNotes.map(x => x.id)).toEqual([]);
expect(state.selectedNoteIds).toEqual([notes[3].id]);
}));

it('should ensure history max limit is maintained', asyncTest(async () => {
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]);

const idx = 0;
for (let i = 0; i < 2 * MAX_HISTORY; i++) {
state = goToNote(notes, [i % 5], state);
}

expect(state.backwardHistoryNotes.length).toEqual(MAX_HISTORY);
expect(state.forwardHistoryNotes.map(x => x.id)).toEqual([]);

for (let i = 0; i < 2 * MAX_HISTORY; i++) {
state = goBackWard(state);
}

expect(state.backwardHistoryNotes).toEqual([]);
expect(state.forwardHistoryNotes.length).toEqual(MAX_HISTORY);

for (let i = 0; i < 2 * MAX_HISTORY; i++) {
state = goForward(state);
}

expect(state.backwardHistoryNotes.length).toEqual(MAX_HISTORY);
expect(state.forwardHistoryNotes.map(x => x.id)).toEqual([]);
}));
});
1 change: 0 additions & 1 deletion ElectronClient/gui/NoteEditor/NoteEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -555,7 +555,6 @@ const mapStateToProps = (state: any) => {
watchedNoteFiles: state.watchedNoteFiles,
windowCommand: state.windowCommand,
notesParentType: state.notesParentType,
historyNotes: state.historyNotes,
selectedNoteTags: state.selectedNoteTags,
lastEditorScrollPercents: state.lastEditorScrollPercents,
selectedNoteHash: state.selectedNoteHash,
Expand Down
1 change: 0 additions & 1 deletion ElectronClient/gui/NoteEditor/utils/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ export interface NoteEditorProps {
windowCommand: any;
folders: any[];
notesParentType: string;
historyNotes: any[];
selectedNoteTags: any[];
lastEditorScrollPercents: any;
selectedNoteHash: string;
Expand Down
4 changes: 0 additions & 4 deletions ElectronClient/gui/NoteEditor/utils/useMessageHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,10 +128,6 @@ export default function useMessageHandler(scrollWhenReady:any, setScrollWhenRead
folderId: item.parent_id,
noteId: item.id,
hash: resourceUrlInfo.hash,
historyNoteAction: {
id: formNote.id,
parent_id: formNote.parent_id,
},
});
} else {
throw new Error(`Unsupported item type: ${item.type_}`);
Expand Down
52 changes: 30 additions & 22 deletions ElectronClient/gui/NoteToolbar/NoteToolbar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,12 @@ interface NoteToolbarProps {
selectedFolderId: string,
folders: any[],
watchedNoteFiles: string[],
backwardHistoryNotes: any[],
forwardHistoryNotes: any[],
notesParentType: string,
note: any,
dispatch: Function,
onButtonClick(event:ButtonClickEvent):void,
historyNotes: any[],
}

function styles_(props:NoteToolbarProps) {
Expand All @@ -37,12 +38,37 @@ function styles_(props:NoteToolbarProps) {
}

function useToolbarItems(props:NoteToolbarProps) {
const { note, selectedFolderId, folders, watchedNoteFiles, notesParentType, dispatch, onButtonClick, historyNotes } = props;
const { note, selectedFolderId, folders, watchedNoteFiles, notesParentType, dispatch
, onButtonClick, backwardHistoryNotes, forwardHistoryNotes } = props;

const toolbarItems = [];

const folder = Folder.byId(folders, selectedFolderId);

toolbarItems.push({
tooltip: _('Back'),
iconName: 'fa-arrow-left',
enabled: (backwardHistoryNotes.length > 0),
onClick: () => {
if (!backwardHistoryNotes.length) return;
props.dispatch({
type: 'HISTORY_BACKWARD',
});
},
});

toolbarItems.push({
tooltip: _('Front'),
iconName: 'fa-arrow-right',
enabled: (forwardHistoryNotes.length > 0),
onClick: () => {
if (!forwardHistoryNotes.length) return;
props.dispatch({
type: 'HISTORY_FORWARD',
});
},
});

if (folder && ['Search', 'Tag', 'SmartFilter'].includes(notesParentType)) {
toolbarItems.push({
title: _('In: %s', substrWithEllipsis(folder.title, 0, 16)),
Expand All @@ -58,25 +84,6 @@ function useToolbarItems(props:NoteToolbarProps) {
});
}

if (historyNotes.length) {
toolbarItems.push({
tooltip: _('Back'),
iconName: 'fa-arrow-left',
onClick: () => {
if (!historyNotes.length) return;

const lastItem = historyNotes[historyNotes.length - 1];

dispatch({
type: 'FOLDER_AND_NOTE_SELECT',
folderId: lastItem.parent_id,
noteId: lastItem.id,
historyNoteAction: 'pop',
});
},
});
}

if (watchedNoteFiles.indexOf(note.id) >= 0) {
toolbarItems.push({
tooltip: _('Click to stop external editing'),
Expand Down Expand Up @@ -149,7 +156,8 @@ const mapStateToProps = (state:any) => {
selectedFolderId: state.selectedFolderId,
folders: state.folders,
watchedNoteFiles: state.watchedNoteFiles,
historyNotes: state.historyNotes,
backwardHistoryNotes: state.backwardHistoryNotes,
forwardHistoryNotes: state.forwardHistoryNotes,
notesParentType: state.notesParentType,
};
};
Expand Down
5 changes: 5 additions & 0 deletions ReactNativeClient/lib/BaseApplication.js
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,11 @@ class BaseApplication {
refreshFolders = true;
}

if (action.type == 'HISTORY_BACKWARD' || action.type == 'HISTORY_FORWARD') {
refreshNotes = true;
refreshNotesUseSelectedNoteId = true;
}

if (action.type == 'FOLDER_SELECT' || action.type === 'FOLDER_DELETE' || action.type === 'FOLDER_AND_NOTE_SELECT' || (action.type === 'SEARCH_UPDATE' && newState.notesParentType === 'Folder')) {
Setting.setValue('activeFolderId', newState.selectedFolderId);
this.currentFolder_ = newState.selectedFolderId ? await Folder.load(newState.selectedFolderId) : null;
Expand Down
Loading