Chore: Desktop: Editor: Don't update the global Redux state on cursor motion (#13580)

.eslintignore

@@ -1397,6 +1397,7 @@ packages/lib/services/KeymapService_keysRegExp.js
 packages/lib/services/KvStore.js
 packages/lib/services/MigrationService.js
 packages/lib/services/NavService.js
+packages/lib/services/NotePositionService.js
 packages/lib/services/PostMessageService.js
 packages/lib/services/ReportService.test.js
 packages/lib/services/ReportService.js

.gitignore

@@ -1369,6 +1369,7 @@ packages/lib/services/KeymapService_keysRegExp.js
 packages/lib/services/KvStore.js
 packages/lib/services/MigrationService.js
 packages/lib/services/NavService.js
+packages/lib/services/NotePositionService.js
 packages/lib/services/PostMessageService.js
 packages/lib/services/ReportService.test.js
 packages/lib/services/ReportService.js

packages/app-desktop/app.reducer.test.ts

@@ -52,7 +52,7 @@ describe('app.reducer', () => {
 			...createAppDefaultState({}),
 			backgroundWindows: {
 				testWindow: {
-					...createAppDefaultWindowState(null),
+					...createAppDefaultWindowState(),
 					windowId: 'testWindow',
 
 					visibleDialogs: {

packages/app-desktop/app.reducer.ts

@@ -30,17 +30,6 @@ export interface NoteIdToScrollPercent {
 	[noteId: string]: number;
 }
 
-type RichTextEditorSelectionBookmark = unknown;
-
-export interface EditorCursorLocations {
-	readonly richText?: RichTextEditorSelectionBookmark;
-	readonly markdown?: number;
-}
-
-export interface NoteIdToEditorCursorLocations {
-	[noteId: string]: EditorCursorLocations;
-}
-
 export interface VisibleDialogs {
 	[dialogKey: string]: boolean;
 }
@@ -53,9 +42,6 @@ export interface AppWindowState extends WindowState {
 	devToolsVisible: boolean;
 	// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
 	watchedResources: any;
-
-	lastEditorScrollPercents: NoteIdToScrollPercent;
-	lastEditorCursorLocations: NoteIdToEditorCursorLocations;
 }
 
 interface BackgroundWindowStates {
@@ -79,7 +65,7 @@ export interface AppState extends State, AppWindowState {
 	isResettingLayout: boolean;
 }
 
-export const createAppDefaultWindowState = (globalState: AppState|null): AppWindowState => {
+export const createAppDefaultWindowState = (): AppWindowState => {
 	return {
 		...defaultWindowState,
 		visibleDialogs: {},
@@ -88,12 +74,6 @@ export const createAppDefaultWindowState = (globalState: AppState|null): AppWind
 		editorCodeView: true,
 		devToolsVisible: false,
 		watchedResources: {},
-
-		// Maintain the scroll and cursor location for secondary windows separate from the
-		// main window. This prevents scrolling in a secondary window from changing/resetting
-		// the default scroll position in the main window:
-		lastEditorCursorLocations: globalState?.lastEditorCursorLocations ?? {},
-		lastEditorScrollPercents: globalState?.lastEditorScrollPercents ?? {},
 	};
 };
 
@@ -101,7 +81,7 @@ export const createAppDefaultWindowState = (globalState: AppState|null): AppWind
 export function createAppDefaultState(resourceEditWatcherDefaultState: any): AppState {
 	return {
 		...defaultState,
-		...createAppDefaultWindowState(null),
+		...createAppDefaultWindowState(),
 		route: {
 			type: 'NAV_GO',
 			routeName: 'Main',
@@ -307,28 +287,6 @@ export default function(state: AppState, action: any) {
 			}
 			break;
 
-		case 'EDITOR_SCROLL_PERCENT_SET':
-
-			{
-				newState = { ...state };
-				const newPercents = { ...newState.lastEditorScrollPercents };
-				newPercents[action.noteId] = action.percent;
-				newState.lastEditorScrollPercents = newPercents;
-			}
-			break;
-
-		case 'EDITOR_CURSOR_POSITION_SET':
-			{
-				newState = { ...state };
-				const newCursorLocations = { ...newState.lastEditorCursorLocations };
-				newCursorLocations[action.noteId] = {
-					...(newCursorLocations[action.noteId] ?? {}),
-					...action.location,
-				};
-				newState.lastEditorCursorLocations = newCursorLocations;
-			}
-			break;
-
 		case 'NOTE_DEVTOOLS_TOGGLE':
 			newState = { ...state };
 			newState.devToolsVisible = !newState.devToolsVisible;

packages/app-desktop/commands/openNoteInNewWindow.ts

@@ -2,7 +2,7 @@ import { CommandRuntime, CommandDeclaration, CommandContext } from '@joplin/lib/
 import { _ } from '@joplin/lib/locale';
 import { stateUtils } from '@joplin/lib/reducer';
 import Note from '@joplin/lib/models/Note';
-import { AppState, createAppDefaultWindowState } from '../app.reducer';
+import { createAppDefaultWindowState } from '../app.reducer';
 import Setting from '@joplin/lib/models/Setting';
 
 export const declaration: CommandDeclaration = {
@@ -25,7 +25,7 @@ export const runtime = (): CommandRuntime => {
 				folderId: note.parent_id,
 				windowId: `window-${noteId}-${idCounter++}`,
 				defaultAppWindowState: {
-					...createAppDefaultWindowState(context.state as AppState),
+					...createAppDefaultWindowState(),
 					noteVisiblePanes: Setting.value('noteVisiblePanes'),
 					editorCodeView: Setting.value('editor.codeView'),
 				},

packages/app-desktop/gui/NoteEditor/NoteEditor.tsx

@@ -18,7 +18,7 @@ import { NoteEditorProps, FormNote, OnChangeEvent, AllAssetsOptions, NoteBodyEdi
 import CommandService from '@joplin/lib/services/CommandService';
 import Button, { ButtonLevel } from '../Button/Button';
 import eventManager, { EventName } from '@joplin/lib/eventManager';
-import { AppState, EditorCursorLocations } from '../../app.reducer';
+import { AppState } from '../../app.reducer';
 import ToolbarButtonUtils, { ToolbarButtonInfo } from '@joplin/lib/services/commands/ToolbarButtonUtils';
 import { _, _n } from '@joplin/lib/locale';
 import NoteTitleBar from './NoteTitle/NoteTitleBar';
@@ -58,6 +58,7 @@ import useVisiblePluginEditorViewIds from '@joplin/lib/hooks/plugins/useVisibleP
 import useConnectToEditorPlugin from './utils/useConnectToEditorPlugin';
 import getResourceBaseUrl from './utils/getResourceBaseUrl';
 import useInitialCursorLocation from './utils/useInitialCursorLocation';
+import NotePositionService, { EditorCursorLocations } from '@joplin/lib/services/NotePositionService';
 
 const debounce = require('debounce');
 
@@ -333,7 +334,6 @@ function NoteEditorContent(props: NoteEditorProps) {
 	const { scrollWhenReadyRef, clearScrollWhenReady } = useScrollWhenReadyOptions({
 		noteId: formNote.id,
 		selectedNoteHash: props.selectedNoteHash,
-		lastEditorScrollPercents: props.lastEditorScrollPercents,
 		editorRef,
 		editorName: props.bodyEditor,
 	});
@@ -401,23 +401,14 @@ function NoteEditorContent(props: NoteEditorProps) {
 	}, [setShowRevisions]);
 
 	const onScroll = useCallback((event: { percent: number }) => {
-		props.dispatch({
+		const noteId = formNoteRef.current.id;
-			type: 'EDITOR_SCROLL_PERCENT_SET',
+		NotePositionService.instance().updateScrollPosition(noteId, windowId, event.percent);
-			// In callbacks of setTimeout()/setInterval(), props/state cannot be used
+	}, [windowId]);
-			// to refer the current value, since they would be one or more generations old.
-			// For the purpose, useRef value should be used.
-			noteId: formNoteRef.current.id,
-			percent: event.percent,
-		});
-	}, [props.dispatch]);
 
 	const onCursorMotion = useCallback((location: EditorCursorLocations) => {
-		props.dispatch({
+		const noteId = formNoteRef.current.id;
-			type: 'EDITOR_CURSOR_POSITION_SET',
+		NotePositionService.instance().updateCursorPosition(noteId, windowId, location);
-			noteId: formNoteRef.current.id,
+	}, [windowId]);
-			location,
-		});
-	}, [props.dispatch]);
 
 	function renderNoNotes(rootStyle: React.CSSProperties) {
 		const emptyDivStyle = {
@@ -430,7 +421,7 @@ function NoteEditorContent(props: NoteEditorProps) {
 
 	const searchMarkers = useSearchMarkers(showLocalSearch, localSearchMarkerOptions, props.searches, props.selectedSearchId, props.highlightedWords);
 	const initialCursorLocation = useInitialCursorLocation({
-		lastEditorCursorLocations: props.lastEditorCursorLocations, noteId: props.noteId,
+		noteId: props.noteId,
 	});
 
 	const markupLanguage = formNote.markup_language;
@@ -743,8 +734,6 @@ const mapStateToProps = (state: AppState, ownProps: ConnectProps) => {
 		watchedNoteFiles: state.watchedNoteFiles,
 		notesParentType: windowState.notesParentType,
 		selectedNoteTags: windowState.selectedNoteTags,
-		lastEditorScrollPercents: state.lastEditorScrollPercents,
-		lastEditorCursorLocations: state.lastEditorCursorLocations,
 		selectedNoteHash: windowState.selectedNoteHash,
 		searches: state.searches,
 		selectedSearchId: windowState.selectedSearchId,

packages/app-desktop/gui/NoteEditor/utils/types.ts

@@ -14,7 +14,7 @@ import { ScrollbarSize } from '@joplin/lib/models/settings/builtInMetadata';
 import { RefObject, SetStateAction } from 'react';
 import * as React from 'react';
 import { ResourceEntity, ResourceLocalStateEntity } from '@joplin/lib/services/database/types';
-import { EditorCursorLocations, NoteIdToEditorCursorLocations, NoteIdToScrollPercent } from '../../../app.reducer';
+import { EditorCursorLocations } from '@joplin/lib/services/NotePositionService';
 
 export interface AllAssetsOptions {
 	contentMaxWidthTarget?: string;
@@ -41,8 +41,6 @@ export interface NoteEditorProps {
 	notesParentType: string;
 	// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
 	selectedNoteTags: any[];
-	lastEditorScrollPercents: NoteIdToScrollPercent;
-	lastEditorCursorLocations: NoteIdToEditorCursorLocations;
 	selectedNoteHash: string;
 	// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
 	searches: any[];

packages/app-desktop/gui/NoteEditor/utils/useInitialCursorLocation.ts

@@ -1,17 +1,17 @@
-import { useMemo } from 'react';
+import { useContext, useMemo } from 'react';
-import { EditorCursorLocations, NoteIdToEditorCursorLocations } from '../../../app.reducer';
+import { WindowIdContext } from '../../NewWindowOrIFrame';
+import NotePositionService from '@joplin/lib/services/NotePositionService';
 
 interface Props {
-	lastEditorCursorLocations: NoteIdToEditorCursorLocations;
 	noteId: string;
 }
 
-const useInitialCursorLocation = ({ noteId, lastEditorCursorLocations }: Props) => {
+const useInitialCursorLocation = ({ noteId }: Props) => {
-	const lastCursorLocation = lastEditorCursorLocations[noteId];
+	const windowId = useContext(WindowIdContext);
 
-	return useMemo((): EditorCursorLocations => {
+	return useMemo(() => {
-		return lastCursorLocation ?? { };
+		return NotePositionService.instance().getCursorPosition(noteId, windowId);
-	}, [lastCursorLocation]);
+	}, [noteId, windowId]);
 };
 
 export default useInitialCursorLocation;

packages/app-desktop/gui/NoteEditor/utils/useScrollWhenReadyOptions.ts

@@ -1,42 +1,43 @@
-import { RefObject, useCallback, useRef } from 'react';
+import { RefObject, useCallback, useContext, useRef } from 'react';
 import { NoteBodyEditorRef, ScrollOptions, ScrollOptionTypes } from './types';
 import usePrevious from '@joplin/lib/hooks/usePrevious';
-import type { NoteIdToScrollPercent } from '../../../app.reducer';
+import NotePositionService from '@joplin/lib/services/NotePositionService';
 import useNowEffect from '@joplin/lib/hooks/useNowEffect';
+import { WindowIdContext } from '../../NewWindowOrIFrame';
 
 interface Props {
 	noteId: string;
 	editorName: string;
 	selectedNoteHash: string;
-	lastEditorScrollPercents: NoteIdToScrollPercent;
 	editorRef: RefObject<NoteBodyEditorRef>;
 }
 
-const useScrollWhenReadyOptions = ({ noteId, editorName, selectedNoteHash, lastEditorScrollPercents, editorRef }: Props) => {
+const useScrollWhenReadyOptions = ({ noteId, editorName, selectedNoteHash, editorRef }: Props) => {
 	const scrollWhenReadyRef = useRef<ScrollOptions|null>(null);
 
 	const previousNoteId = usePrevious(noteId);
-	const noteIdChanged = noteId !== previousNoteId;
 	const previousEditor = usePrevious(editorName);
-	const editorChanged = editorName !== previousEditor;
+	const windowId = useContext(WindowIdContext);
-	const lastScrollPercentsRef = useRef<NoteIdToScrollPercent>(null);
+
-	lastScrollPercentsRef.current = lastEditorScrollPercents;
 
 	// This needs to be a nowEffect to prevent race conditions
 	useNowEffect(() => {
+		const editorChanged = editorName !== previousEditor;
+		const noteIdChanged = noteId !== previousNoteId;
 		if (!editorChanged && !noteIdChanged) return () => {};
 
+		const lastScrollPercent = NotePositionService.instance().getScrollPercent(noteId, windowId) || 0;
+		scrollWhenReadyRef.current = {
+			type: selectedNoteHash ? ScrollOptionTypes.Hash : ScrollOptionTypes.Percent,
+			value: selectedNoteHash ? selectedNoteHash : lastScrollPercent,
+		};
+
 		if (editorRef.current) {
 			editorRef.current.resetScroll();
 		}
 
-		const lastScrollPercent = lastScrollPercentsRef.current[noteId] || 0;
-		scrollWhenReadyRef.current = {
-			type: selectedNoteHash ? ScrollOptionTypes.Hash : ScrollOptionTypes.Percent,
-			value: selectedNoteHash ? selectedNoteHash : lastScrollPercent,
-		};
 		return () => {};
-	}, [editorChanged, noteIdChanged, noteId, selectedNoteHash, editorRef]);
+	}, [editorName, previousEditor, noteId, previousNoteId, selectedNoteHash, editorRef, windowId]);
 
 	const clearScrollWhenReady = useCallback(() => {
 		scrollWhenReadyRef.current = null;

packages/app-desktop/gui/utils/NoteListUtils.test.ts

@@ -38,7 +38,7 @@ describe('NoteListUtils', () => {
 		const mockStore = {
 			getState: () => {
 				return {
-					...createAppDefaultWindowState(null),
+					...createAppDefaultWindowState(),
 					settings: {},
 				};
 			},

packages/lib/services/NotePositionService.ts

@@ -0,0 +1,66 @@
+
+export interface EditorCursorLocations {
+	readonly richText?: unknown; // Depends on the editor implementation
+	readonly markdown?: number;
+}
+
+type NoteIdAndWindowKey = `note-window:${string}`;
+type NoteIdKey = `note:${string}`;
+
+export default class NotePositionService {
+	private constructor() {}
+
+	private static instance_: NotePositionService;
+	public static instance() {
+		this.instance_ ??= new NotePositionService();
+		return this.instance_;
+	}
+
+	private toFallbackKey_(noteId: string): NoteIdKey {
+		return `note:${noteId}`;
+	}
+
+	private toKey_(noteId: string, windowId?: string): NoteIdAndWindowKey {
+		return `note-window:${windowId}--${noteId}`;
+	}
+
+	private cursorFallback_: Map<NoteIdKey, EditorCursorLocations> = new Map();
+	private cursorLocations_: Map<NoteIdAndWindowKey, EditorCursorLocations> = new Map();
+
+	public getCursorPosition(noteId: string, windowId: string) {
+		// If available, use the cursor position for the current window
+		return this.cursorLocations_.get(this.toKey_(noteId, windowId))
+			// Fall back to the last-set cursor location for all windows
+			?? this.cursorFallback_.get(this.toFallbackKey_(noteId))
+			?? { };
+	}
+
+	public updateCursorPosition(noteId: string, windowId: string, position: EditorCursorLocations) {
+		const key = this.toKey_(noteId, windowId);
+		this.cursorLocations_.set(key, {
+			...this.cursorLocations_.get(key),
+			...position,
+		});
+
+		const fallbackKey = this.toFallbackKey_(noteId);
+		this.cursorFallback_.set(fallbackKey, {
+			...this.cursorFallback_.get(fallbackKey),
+			...position,
+		});
+	}
+
+	private scrollFallback_: Map<NoteIdKey, number> = new Map();
+	private scrollLocations_: Map<NoteIdAndWindowKey, number> = new Map();
+
+	public getScrollPercent(noteId: string, windowId: string) {
+		return this.scrollLocations_.get(this.toKey_(noteId, windowId))
+			?? this.scrollFallback_.get(this.toFallbackKey_(noteId))
+			?? 0;
+	}
+
+	public updateScrollPosition(noteId: string, windowId: string, percent: number) {
+		const key = this.toKey_(noteId, windowId);
+		this.scrollLocations_.set(key, percent);
+		this.scrollFallback_.set(this.toFallbackKey_(noteId), percent);
+	}
+}