From 5078341c15fc2ddea8381cad612df0091884b0db Mon Sep 17 00:00:00 2001 From: Henry Heino <46334387+personalizedrefrigerator@users.noreply.github.com> Date: Wed, 18 Dec 2024 02:58:36 -0800 Subject: [PATCH] Mobile: Fixes #11264: Fix editor shows nothing when there are no selected note IDs (#11514) --- .../components/ScreenHeader/index.tsx | 9 ++++- .../components/screens/Note/Note.tsx | 14 +------ .../app-mobile/components/screens/Notes.tsx | 1 + packages/app-mobile/utils/appDefaultState.ts | 3 ++ packages/lib/reducer.test.ts | 39 +++++++++++++++++-- packages/lib/reducer.ts | 10 ++++- 6 files changed, 57 insertions(+), 19 deletions(-) diff --git a/packages/app-mobile/components/ScreenHeader/index.tsx b/packages/app-mobile/components/ScreenHeader/index.tsx index e15013640..63a908bdc 100644 --- a/packages/app-mobile/components/ScreenHeader/index.tsx +++ b/packages/app-mobile/components/ScreenHeader/index.tsx @@ -550,7 +550,14 @@ class ScreenHeaderComponent extends PureComponent imp // which can cause some bugs where previously set state to another note would interfere // how the new note should be rendered const NoteScreenWrapper = (props: Props) => { - const lastNonNullNoteIdRef = useRef(props.noteId); - if (props.noteId) { - lastNonNullNoteIdRef.current = props.noteId; - } - - // This keeps the current note open even if it's no longer present in selectedNoteIds. - // This might happen, for example, if the selected note is moved to an unselected - // folder. - const noteId = lastNonNullNoteIdRef.current; - const dialogs = useContext(DialogContext); return ( - + ); }; diff --git a/packages/app-mobile/components/screens/Notes.tsx b/packages/app-mobile/components/screens/Notes.tsx index befda92f1..9f243b367 100644 --- a/packages/app-mobile/components/screens/Notes.tsx +++ b/packages/app-mobile/components/screens/Notes.tsx @@ -35,6 +35,7 @@ interface Props { showCompletedTodos: boolean; noteSelectionEnabled: boolean; + selectedNoteIds: string[]; activeFolderId: string; selectedFolderId: string; selectedTagId: string; diff --git a/packages/app-mobile/utils/appDefaultState.ts b/packages/app-mobile/utils/appDefaultState.ts index d69395d07..880cb9525 100644 --- a/packages/app-mobile/utils/appDefaultState.ts +++ b/packages/app-mobile/utils/appDefaultState.ts @@ -18,5 +18,8 @@ const appDefaultState: AppState = { showPanelsDialog: false, newNoteAttachFileAction: null, ...defaultState, + + // On mobile, it's possible to select notes that aren't in the selected folder/tag/etc. + allowSelectionInOtherFolders: true, }; export default appDefaultState; diff --git a/packages/lib/reducer.test.ts b/packages/lib/reducer.test.ts index 15eaae44e..9517af8dc 100644 --- a/packages/lib/reducer.test.ts +++ b/packages/lib/reducer.test.ts @@ -709,13 +709,19 @@ describe('reducer', () => { // Regression test for #10589. it.each([ - true, false, - ])('should preserve note selection if specified while moving a note (preserveSelection: %j)', async (preserveSelection) => { + [true, false], + [undefined, false], + [undefined, true], + [false, true], + ])('should preserve note selection if specified with an option (preserveSelection: %j, allowSelectionInOtherFolders: %j)', async ( + preserveSelectionOption, allowSelectionInOtherFolders, + ) => { const folders = await createNTestFolders(3); const notes = await createNTestNotes(5, folders[0]); // select the 1st folder and the 1st note let state = initTestState(folders, 0, notes, [0]); + state = { ...state, allowSelectionInOtherFolders }; state = goToNote(notes, [0], state); expect(state.selectedNoteIds).toHaveLength(1); @@ -729,11 +735,15 @@ describe('reducer', () => { await Note.moveToFolder( state.selectedNoteIds[0], folders[1].id, - preserveSelection ? { dispatchOptions: { preserveSelection: true } } : undefined, + { dispatchOptions: { preserveSelection: preserveSelectionOption } }, ); expect(BaseModel.dispatch).toHaveBeenCalled(); - if (preserveSelection) { + + // preserveSelectionOption takes precedence over allowSelectionInOtherFolders + const shouldPreserveSelection = preserveSelectionOption ?? allowSelectionInOtherFolders; + + if (shouldPreserveSelection) { expect(state.selectedNoteIds).toMatchObject([notes[0].id]); } else { expect(state.selectedNoteIds).toMatchObject([notes[1].id]); @@ -743,6 +753,27 @@ describe('reducer', () => { expect(state.selectedFolderId).toBe(folders[0].id); }); + test('when selection is allowed in unselected folders, NOTE_UPDATE_ALL should not remove items from the selection', async () => { + const folders = await createNTestFolders(2); + const notes = await createNTestNotes(2, folders[0]); + + // select the 1st folder and the 1st note + let state = initTestState(folders, 0, notes, [0]); + state = { ...state, allowSelectionInOtherFolders: true }; + state = goToNote(notes, [0], state); + + expect(state.selectedNoteIds).toEqual([notes[0].id]); + expect(state.notes).toHaveLength(2); + + state = reducer(state, { + type: 'NOTE_UPDATE_ALL', + notes: [], + }); + + expect(state.notes).toHaveLength(0); + expect(state.selectedNoteIds).toEqual([notes[0].id]); + }); + // window tests test('switching windows should move state from background to the foreground', async () => { const folders = await createNTestFolders(2); diff --git a/packages/lib/reducer.ts b/packages/lib/reducer.ts index 618681f2b..c0e830cda 100644 --- a/packages/lib/reducer.ts +++ b/packages/lib/reducer.ts @@ -169,6 +169,8 @@ export interface State extends WindowState { mustUpgradeAppMessage: string; mustAuthenticate: boolean; + allowSelectionInOtherFolders: boolean; + // Extra reducer keys go here: pluginService: PluginServiceState; shareService: ShareServiceState; @@ -236,6 +238,7 @@ export const defaultState: State = { lastDeletionNotificationTime: 0, mustUpgradeAppMessage: '', mustAuthenticate: false, + allowSelectionInOtherFolders: false, pluginService: pluginServiceDefaultState, shareService: shareServiceDefaultState, @@ -1080,7 +1083,9 @@ const reducer = produce((draft: Draft = defaultState, action: any) => { draft.notes = action.notes; draft.notesSource = action.notesSource; draft.noteListLastSortTime = Date.now(); // Notes are already sorted when they are set this way. - updateSelectedNotesFromExistingNotes(draft); + if (!draft.allowSelectionInOtherFolders) { + updateSelectedNotesFromExistingNotes(draft); + } break; // Insert the note into the note list if it's new, or @@ -1148,7 +1153,8 @@ const reducer = produce((draft: Draft = defaultState, action: any) => { // For example, if the user drags the current note to a different folder, // a new note should be selected. // In some cases, however, the selection needs to be preserved (e.g. the mobile app). - if (noteFolderHasChanged && !action.preserveSelection) { + const preserveSelection = action.preserveSelection ?? draft.allowSelectionInOtherFolders; + if (noteFolderHasChanged && !preserveSelection) { let newIndex = movedNotePreviousIndex; if (newIndex >= newNotes.length) newIndex = newNotes.length - 1; if (!newNotes.length) newIndex = -1;