From 65cbb6e388c839c7945fa97c0e495fb7691bdc47 Mon Sep 17 00:00:00 2001 From: mic704b Date: Sun, 19 Jan 2020 00:53:00 +1100 Subject: [PATCH] Desktop: Maintain selection when non-selected note is deleted (#2290) * Fix jump of focus following deletion action. Applies to notes, folders and tags. * Add tests for reducer item delete handling. * Add comments. * Clean up. --- CliClient/tests/reducer.js | 350 +++++++++++++++++++++++++++++++ ReactNativeClient/lib/reducer.js | 57 +++-- 2 files changed, 394 insertions(+), 13 deletions(-) create mode 100644 CliClient/tests/reducer.js diff --git a/CliClient/tests/reducer.js b/CliClient/tests/reducer.js new file mode 100644 index 0000000000..343bb6958b --- /dev/null +++ b/CliClient/tests/reducer.js @@ -0,0 +1,350 @@ +/* eslint-disable no-unused-vars */ + +require('app-module-path').addPath(__dirname); +const {setupDatabaseAndSynchronizer, switchClient, asyncTest } = require('test-utils.js'); +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'); + +async function createNTestFolders(n) { + let folders = []; + for (let i = 0; i < n; i++) { + let folder = await Folder.save({ title: 'folder' }); + folders.push(folder); + } + return folders; +} + +async function createNTestNotes(n, folder) { + let notes = []; + for (let i = 0; i < n; i++) { + let note = await Note.save({ title: 'note', parent_id: folder.id }); + notes.push(note); + } + return notes; +} + +async function createNTestTags(n) { + let tags = []; + for (let i = 0; i < n; i++) { + let tag = await Tag.save({ title: 'tag' }); + tags.push(tag); + } + return tags; +} + +function initTestState(folders, selectedFolderIndex, notes, selectedIndexes, tags=null, selectedTagIndex=null) { + let state = defaultState; + if (folders != null) { + state = reducer(state, { type: 'FOLDER_UPDATE_ALL', items: folders }); + } + if (selectedFolderIndex != null) { + state = reducer(state, { type: 'FOLDER_SELECT', id: folders[selectedFolderIndex].id }); + } + if (notes != null) { + state = reducer(state, { type: 'NOTE_UPDATE_ALL', notes: notes, noteSource: 'test' }); + } + if (selectedIndexes != null) { + let selectedIds = []; + for (let i = 0; i < selectedIndexes.length; i++) { + selectedIds.push(notes[selectedIndexes[i]].id); + } + state = reducer(state, { type: 'NOTE_SELECT', ids: selectedIds }); + } + if (tags != null) { + state = reducer(state, { type: 'TAG_UPDATE_ALL', items: tags }); + } + if (selectedTagIndex != null) { + state = reducer(state, { type: 'TAG_SELECT', id: tags[selectedTagIndex].id }); + } + + return state; +} + +function createExpectedState(items, keepIndexes, selectedIndexes) { + let expected = { items: [], selectedIds: []}; + + for (let i = 0; i < selectedIndexes.length; i++) { + expected.selectedIds.push(items[selectedIndexes[i]].id); + } + for (let i = 0; i < keepIndexes.length; i++) { + expected.items.push(items[keepIndexes[i]]); + } + return expected; +} + +function getIds(items, indexes=null) { + let ids = []; + for (let i = 0; i < items.length; i++) { + if (indexes == null || i in indexes) { + ids.push(items[i].id); + } + } + return ids; +} + +let insideBeforeEach = false; + +describe('Reducer', function() { + + beforeEach(async (done) => { + insideBeforeEach = true; + + await setupDatabaseAndSynchronizer(1); + await switchClient(1); + + done(); + + insideBeforeEach = false; + }); + + // tests for NOTE_DELETE + it('should delete selected note', asyncTest(async () => { + // create 1 folder + let folders = await createNTestFolders(1); + // create 5 notes + let notes = await createNTestNotes(5, folders[0]); + // select the 1st folder and the 3rd note + let state = initTestState(folders, 0, notes, [2]); + + // test action + // delete the third note + state = reducer(state, {type: 'NOTE_DELETE', id: notes[2].id}); + + // expect that the third note is missing, and the 4th note is now selected + let expected = createExpectedState(notes, [0,1,3,4], [3]); + + // check the ids of all the remaining notes + expect(getIds(state.notes)).toEqual(getIds(expected.items)); + // check the ids of the selected notes + expect(state.selectedNoteIds).toEqual(expected.selectedIds); + })); + + it('should delete selected note at top', asyncTest(async () => { + let folders = await createNTestFolders(1); + let notes = await createNTestNotes(5, folders[0]); + let state = initTestState(folders, 0, notes, [1]); + + // test action + state = reducer(state, {type: 'NOTE_DELETE', id: notes[0].id}); + + let expected = createExpectedState(notes, [1,2,3,4], [1]); + + expect(getIds(state.notes)).toEqual(getIds(expected.items)); + expect(state.selectedNoteIds).toEqual(expected.selectedIds); + })); + + it('should delete last remaining note', asyncTest(async () => { + let folders = await createNTestFolders(1); + let notes = await createNTestNotes(1, folders[0]); + let state = initTestState(folders, 0, notes, [0]); + + // test action + state = reducer(state, {type: 'NOTE_DELETE', id: notes[0].id}); + + let expected = createExpectedState(notes, [], []); + + expect(getIds(state.notes)).toEqual(getIds(expected.items)); + expect(state.selectedNoteIds).toEqual(expected.selectedIds); + })); + + it('should delete selected note at bottom', asyncTest(async () => { + let folders = await createNTestFolders(1); + let notes = await createNTestNotes(5, folders[0]); + let state = initTestState(folders, 0, notes, [4]); + + // test action + state = reducer(state, {type: 'NOTE_DELETE', id: notes[4].id}); + + let expected = createExpectedState(notes, [0,1,2,3], [3]); + + expect(getIds(state.notes)).toEqual(getIds(expected.items)); + expect(state.selectedNoteIds).toEqual(expected.selectedIds); + })); + + it('should delete note when a note below is selected', asyncTest(async () => { + let folders = await createNTestFolders(1); + let notes = await createNTestNotes(5, folders[0]); + let state = initTestState(folders, 0, notes, [3]); + + // test action + state = reducer(state, {type: 'NOTE_DELETE', id: notes[1].id}); + + let expected = createExpectedState(notes, [0,2,3,4], [3]); + + expect(getIds(state.notes)).toEqual(getIds(expected.items)); + expect(state.selectedNoteIds).toEqual(expected.selectedIds); + })); + + it('should delete note when a note above is selected', asyncTest(async () => { + let folders = await createNTestFolders(1); + let notes = await createNTestNotes(5, folders[0]); + let state = initTestState(folders, 0, notes, [1]); + + // test action + state = reducer(state, {type: 'NOTE_DELETE', id: notes[3].id}); + + let expected = createExpectedState(notes, [0,1,2,4], [1]); + + expect(getIds(state.notes)).toEqual(getIds(expected.items)); + expect(state.selectedNoteIds).toEqual(expected.selectedIds); + })); + + it('should delete selected notes', asyncTest(async () => { + let folders = await createNTestFolders(1); + let notes = await createNTestNotes(5, folders[0]); + let state = initTestState(folders, 0, notes, [1,2]); + + // test action + state = reducer(state, {type: 'NOTE_DELETE', id: notes[1].id}); + state = reducer(state, {type: 'NOTE_DELETE', id: notes[2].id}); + + let expected = createExpectedState(notes, [0,3,4], [3]); + + expect(getIds(state.notes)).toEqual(getIds(expected.items)); + expect(state.selectedNoteIds).toEqual(expected.selectedIds); + })); + + it('should delete note when a notes below it are selected', asyncTest(async () => { + let folders = await createNTestFolders(1); + let notes = await createNTestNotes(5, folders[0]); + let state = initTestState(folders, 0, notes, [3,4]); + + // test action + state = reducer(state, {type: 'NOTE_DELETE', id: notes[1].id}); + + let expected = createExpectedState(notes, [0,2,3,4], [3,4]); + + expect(getIds(state.notes)).toEqual(getIds(expected.items)); + expect(state.selectedNoteIds).toEqual(expected.selectedIds); + })); + + it('should delete note when a notes above it are selected', asyncTest(async () => { + let folders = await createNTestFolders(1); + let notes = await createNTestNotes(5, folders[0]); + let state = initTestState(folders, 0, notes, [1,2]); + + // test action + state = reducer(state, {type: 'NOTE_DELETE', id: notes[3].id}); + + let expected = createExpectedState(notes, [0,1,2,4], [1,2]); + + expect(getIds(state.notes)).toEqual(getIds(expected.items)); + expect(state.selectedNoteIds).toEqual(expected.selectedIds); + })); + + it('should delete notes at end', asyncTest(async () => { + let folders = await createNTestFolders(1); + let notes = await createNTestNotes(5, folders[0]); + let state = initTestState(folders, 0, notes, [3,4]); + + // test action + state = reducer(state, {type: 'NOTE_DELETE', id: notes[3].id}); + state = reducer(state, {type: 'NOTE_DELETE', id: notes[4].id}); + + let expected = createExpectedState(notes, [0,1,2], [2]); + + expect(getIds(state.notes)).toEqual(getIds(expected.items)); + expect(state.selectedNoteIds).toEqual(expected.selectedIds); + })); + + it('should delete notes when non-contiguous selection', asyncTest(async () => { + let folders = await createNTestFolders(1); + let notes = await createNTestNotes(5, folders[0]); + let state = initTestState(folders, 0, notes, [0,2,4]); + + // test action + state = reducer(state, {type: 'NOTE_DELETE', id: notes[0].id}); + state = reducer(state, {type: 'NOTE_DELETE', id: notes[2].id}); + state = reducer(state, {type: 'NOTE_DELETE', id: notes[4].id}); + + let expected = createExpectedState(notes, [1,3], [1]); + + expect(getIds(state.notes)).toEqual(getIds(expected.items)); + expect(state.selectedNoteIds).toEqual(expected.selectedIds); + })); + + // tests for FOLDER_DELETE + it('should delete selected notebook', asyncTest(async () => { + let folders = await createNTestFolders(5); + let notes = await createNTestNotes(5, folders[0]); + let state = initTestState(folders, 2, notes, [2]); + + // test action + state = reducer(state, {type: 'FOLDER_DELETE', id: folders[2].id}); + + let expected = createExpectedState(folders, [0,1,3,4], [3]); + + expect(getIds(state.folders)).toEqual(getIds(expected.items)); + expect(state.selectedFolderId).toEqual(expected.selectedIds[0]); + })); + + it('should delete notebook when a book above is selected', asyncTest(async () => { + let folders = await createNTestFolders(5); + let notes = await createNTestNotes(5, folders[0]); + let state = initTestState(folders, 1, notes, [2]); + + // test action + state = reducer(state, {type: 'FOLDER_DELETE', id: folders[2].id}); + + let expected = createExpectedState(folders, [0,1,3,4], [1]); + + expect(getIds(state.folders)).toEqual(getIds(expected.items)); + expect(state.selectedFolderId).toEqual(expected.selectedIds[0]); + })); + + it('should delete notebook when a book below is selected', asyncTest(async () => { + let folders = await createNTestFolders(5); + let notes = await createNTestNotes(5, folders[0]); + let state = initTestState(folders, 4, notes, [2]); + + // test action + state = reducer(state, {type: 'FOLDER_DELETE', id: folders[2].id}); + + let expected = createExpectedState(folders, [0,1,3,4], [4]); + + expect(getIds(state.folders)).toEqual(getIds(expected.items)); + expect(state.selectedFolderId).toEqual(expected.selectedIds[0]); + })); + + // tests for TAG_DELETE + it('should delete selected tag', asyncTest(async () => { + let tags = await createNTestTags(5); + let state = initTestState(null, null, null, null, tags, [2]); + + // test action + state = reducer(state, {type: 'TAG_DELETE', id: tags[2].id}); + + let expected = createExpectedState(tags, [0,1,3,4], [3]); + + expect(getIds(state.tags)).toEqual(getIds(expected.items)); + expect(state.selectedTagId).toEqual(expected.selectedIds[0]); + })); + + it('should delete tag when a tag above is selected', asyncTest(async () => { + let tags = await createNTestTags(5); + let state = initTestState(null, null, null, null, tags, [2]); + + // test action + state = reducer(state, {type: 'TAG_DELETE', id: tags[4].id}); + + let expected = createExpectedState(tags, [0,1,2,3], [2]); + + expect(getIds(state.tags)).toEqual(getIds(expected.items)); + expect(state.selectedTagId).toEqual(expected.selectedIds[0]); + })); + + it('should delete tag when a tag below is selected', asyncTest(async () => { + let tags = await createNTestTags(5); + let state = initTestState(null, null, null, null, tags, [2]); + + // test action + state = reducer(state, {type: 'TAG_DELETE', id: tags[0].id}); + + let expected = createExpectedState(tags, [1,2,3,4], [2]); + + expect(getIds(state.tags)).toEqual(getIds(expected.items)); + expect(state.selectedTagId).toEqual(expected.selectedIds[0]); + })); +}); diff --git a/ReactNativeClient/lib/reducer.js b/ReactNativeClient/lib/reducer.js index 75592dd35f..d50ce0efc4 100644 --- a/ReactNativeClient/lib/reducer.js +++ b/ReactNativeClient/lib/reducer.js @@ -136,38 +136,69 @@ function folderSetCollapsed(state, action) { // When deleting a note, tag or folder function handleItemDelete(state, action) { const map = { - FOLDER_DELETE: ['folders', 'selectedFolderId'], - NOTE_DELETE: ['notes', 'selectedNoteIds'], - TAG_DELETE: ['tags', 'selectedTagId'], - SEARCH_DELETE: ['searches', 'selectedSearchId'], + FOLDER_DELETE: ['folders', 'selectedFolderId', true], + NOTE_DELETE: ['notes', 'selectedNoteIds', false], + TAG_DELETE: ['tags', 'selectedTagId', true], + SEARCH_DELETE: ['searches', 'selectedSearchId', true], }; const listKey = map[action.type][0]; const selectedItemKey = map[action.type][1]; + const isSingular = map[action.type][2]; + + const selectedItemKeys = isSingular ? [state[selectedItemKey]] : state[selectedItemKey]; + const isSelected = selectedItemKeys.includes(action.id); - let previousIndex = 0; - let newItems = []; const items = state[listKey]; + let newItems = []; + let newSelectedIndexes = []; + for (let i = 0; i < items.length; i++) { let item = items[i]; + if (isSelected) { + // the selected item is deleted so select the following item + // if multiple items are selected then just use the first one + if (selectedItemKeys[0] == item.id) { + newSelectedIndexes.push(newItems.length); + } + } else { + // the selected item/s is not deleted so keep it selected + if (selectedItemKeys.includes(item.id)) { + newSelectedIndexes.push(newItems.length); + } + } if (item.id == action.id) { - previousIndex = i; continue; } newItems.push(item); } + if (newItems.length == 0) { + newSelectedIndexes = []; // no remaining items so no selection + + } else if (newSelectedIndexes.length == 0) { + newSelectedIndexes.push(0); // no selection exists so select the top + + } else { + // when the items at end of list are deleted then select the end + for (let i = 0; i < newSelectedIndexes.length; i++) { + if (newSelectedIndexes[i] >= newItems.length) { + newSelectedIndexes = [newItems.length - 1]; + break; + } + } + } + let newState = Object.assign({}, state); newState[listKey] = newItems; - if (previousIndex >= newItems.length) { - previousIndex = newItems.length - 1; + const newIds = []; + for (let i = 0; i < newSelectedIndexes.length; i++) { + newIds.push(newItems[newSelectedIndexes[i]].id); } + newState[selectedItemKey] = isSingular ? newIds[0] : newIds; - const newId = previousIndex >= 0 ? newItems[previousIndex].id : null; - newState[selectedItemKey] = action.type === 'NOTE_DELETE' ? [newId] : newId; - - if (!newId && newState.notesParentType !== 'Folder') { + if ((newIds.length == 0) && newState.notesParentType !== 'Folder') { newState.notesParentType = 'Folder'; }