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

Mobile: Fixes #11264: Fix editor shows nothing when there are no selected note IDs #11514

Merged
Merged
9 changes: 8 additions & 1 deletion packages/app-mobile/components/ScreenHeader/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -550,7 +550,14 @@ class ScreenHeaderComponent extends PureComponent<ScreenHeaderProps, ScreenHeade

try {
for (let i = 0; i < noteIds.length; i++) {
await Note.moveToFolder(noteIds[i], folderId);
await Note.moveToFolder(
noteIds[i],
folderId,
// By default, the note selection is preserved on mobile when a note is moved to
// a different folder. However, when moving notes from the note list, this shouldn't be
// the case:
{ dispatchOptions: { preserveSelection: false } },
);
}
} catch (error) {
alert(_n('This note could not be moved: %s', 'These notes could not be moved: %s', noteIds.length, error.message));
Expand Down
14 changes: 2 additions & 12 deletions packages/app-mobile/components/screens/Note/Note.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ import { isSupportedLanguage } from '../../../services/voiceTyping/vosk';
import { ChangeEvent as EditorChangeEvent, SelectionRangeChangeEvent, UndoRedoDepthChangeEvent } from '@joplin/editor/events';
import { join } from 'path';
import { Dispatch } from 'redux';
import { RefObject, useContext, useRef } from 'react';
import { RefObject, useContext } from 'react';
import { SelectionRange } from '../../NoteEditor/types';
import { getNoteCallbackUrl } from '@joplin/lib/callbackUrlUtils';
import { AppState } from '../../../utils/types';
Expand Down Expand Up @@ -1615,19 +1615,9 @@ class NoteScreenComponent extends BaseScreenComponent<ComponentProps, State> 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 (
<NoteScreenComponent key={noteId} dialogs={dialogs} {...props} />
<NoteScreenComponent key={props.noteId} dialogs={dialogs} {...props} />
);
};

Expand Down
1 change: 1 addition & 0 deletions packages/app-mobile/components/screens/Notes.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ interface Props {
showCompletedTodos: boolean;
noteSelectionEnabled: boolean;

selectedNoteIds: string[];
activeFolderId: string;
selectedFolderId: string;
selectedTagId: string;
Expand Down
3 changes: 3 additions & 0 deletions packages/app-mobile/utils/appDefaultState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
39 changes: 35 additions & 4 deletions packages/lib/reducer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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]);
Expand All @@ -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);
Expand Down
10 changes: 8 additions & 2 deletions packages/lib/reducer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,8 @@ export interface State extends WindowState {
mustUpgradeAppMessage: string;
mustAuthenticate: boolean;

allowSelectionInOtherFolders: boolean;

// Extra reducer keys go here:
pluginService: PluginServiceState;
shareService: ShareServiceState;
Expand Down Expand Up @@ -236,6 +238,7 @@ export const defaultState: State = {
lastDeletionNotificationTime: 0,
mustUpgradeAppMessage: '',
mustAuthenticate: false,
allowSelectionInOtherFolders: false,

pluginService: pluginServiceDefaultState,
shareService: shareServiceDefaultState,
Expand Down Expand Up @@ -1080,7 +1083,9 @@ const reducer = produce((draft: Draft<State> = 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
Expand Down Expand Up @@ -1148,7 +1153,8 @@ const reducer = produce((draft: Draft<State> = 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;
Expand Down
Loading