diff --git a/.eslintignore b/.eslintignore index b306353414..594ec904a6 100644 --- a/.eslintignore +++ b/.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 diff --git a/.gitignore b/.gitignore index 4ea7cf3886..3fab7c95a0 100644 --- a/.gitignore +++ b/.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 diff --git a/packages/app-desktop/app.reducer.test.ts b/packages/app-desktop/app.reducer.test.ts index acb7cf78aa..4e6626a9e8 100644 --- a/packages/app-desktop/app.reducer.test.ts +++ b/packages/app-desktop/app.reducer.test.ts @@ -52,7 +52,7 @@ describe('app.reducer', () => { ...createAppDefaultState({}), backgroundWindows: { testWindow: { - ...createAppDefaultWindowState(null), + ...createAppDefaultWindowState(), windowId: 'testWindow', visibleDialogs: { diff --git a/packages/app-desktop/app.reducer.ts b/packages/app-desktop/app.reducer.ts index 10bfb95836..c5a6198a0b 100644 --- a/packages/app-desktop/app.reducer.ts +++ b/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; diff --git a/packages/app-desktop/commands/openNoteInNewWindow.ts b/packages/app-desktop/commands/openNoteInNewWindow.ts index 522e3a7bc6..13d8e3a63c 100644 --- a/packages/app-desktop/commands/openNoteInNewWindow.ts +++ b/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'), }, diff --git a/packages/app-desktop/gui/NoteEditor/NoteEditor.tsx b/packages/app-desktop/gui/NoteEditor/NoteEditor.tsx index e7d0d44609..8443ae39db 100644 --- a/packages/app-desktop/gui/NoteEditor/NoteEditor.tsx +++ b/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({ - type: 'EDITOR_SCROLL_PERCENT_SET', - // In callbacks of setTimeout()/setInterval(), props/state cannot be used - // 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 noteId = formNoteRef.current.id; + NotePositionService.instance().updateScrollPosition(noteId, windowId, event.percent); + }, [windowId]); const onCursorMotion = useCallback((location: EditorCursorLocations) => { - props.dispatch({ - type: 'EDITOR_CURSOR_POSITION_SET', - noteId: formNoteRef.current.id, - location, - }); - }, [props.dispatch]); + const noteId = formNoteRef.current.id; + NotePositionService.instance().updateCursorPosition(noteId, windowId, location); + }, [windowId]); 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, diff --git a/packages/app-desktop/gui/NoteEditor/utils/types.ts b/packages/app-desktop/gui/NoteEditor/utils/types.ts index 662959f4ea..30dd61891a 100644 --- a/packages/app-desktop/gui/NoteEditor/utils/types.ts +++ b/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[]; diff --git a/packages/app-desktop/gui/NoteEditor/utils/useInitialCursorLocation.ts b/packages/app-desktop/gui/NoteEditor/utils/useInitialCursorLocation.ts index d2b5a64bab..f0595ef401 100644 --- a/packages/app-desktop/gui/NoteEditor/utils/useInitialCursorLocation.ts +++ b/packages/app-desktop/gui/NoteEditor/utils/useInitialCursorLocation.ts @@ -1,17 +1,17 @@ -import { useMemo } from 'react'; -import { EditorCursorLocations, NoteIdToEditorCursorLocations } from '../../../app.reducer'; +import { useContext, useMemo } from 'react'; +import { WindowIdContext } from '../../NewWindowOrIFrame'; +import NotePositionService from '@joplin/lib/services/NotePositionService'; interface Props { - lastEditorCursorLocations: NoteIdToEditorCursorLocations; noteId: string; } -const useInitialCursorLocation = ({ noteId, lastEditorCursorLocations }: Props) => { - const lastCursorLocation = lastEditorCursorLocations[noteId]; +const useInitialCursorLocation = ({ noteId }: Props) => { + const windowId = useContext(WindowIdContext); - return useMemo((): EditorCursorLocations => { - return lastCursorLocation ?? { }; - }, [lastCursorLocation]); + return useMemo(() => { + return NotePositionService.instance().getCursorPosition(noteId, windowId); + }, [noteId, windowId]); }; export default useInitialCursorLocation; diff --git a/packages/app-desktop/gui/NoteEditor/utils/useScrollWhenReadyOptions.ts b/packages/app-desktop/gui/NoteEditor/utils/useScrollWhenReadyOptions.ts index ebe59cbfce..59bb4f202b 100644 --- a/packages/app-desktop/gui/NoteEditor/utils/useScrollWhenReadyOptions.ts +++ b/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; } -const useScrollWhenReadyOptions = ({ noteId, editorName, selectedNoteHash, lastEditorScrollPercents, editorRef }: Props) => { +const useScrollWhenReadyOptions = ({ noteId, editorName, selectedNoteHash, editorRef }: Props) => { const scrollWhenReadyRef = useRef(null); const previousNoteId = usePrevious(noteId); - const noteIdChanged = noteId !== previousNoteId; const previousEditor = usePrevious(editorName); - const editorChanged = editorName !== previousEditor; - const lastScrollPercentsRef = useRef(null); - lastScrollPercentsRef.current = lastEditorScrollPercents; + const windowId = useContext(WindowIdContext); + // 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; diff --git a/packages/app-desktop/gui/utils/NoteListUtils.test.ts b/packages/app-desktop/gui/utils/NoteListUtils.test.ts index 8c6ffed7fa..ac288970bb 100644 --- a/packages/app-desktop/gui/utils/NoteListUtils.test.ts +++ b/packages/app-desktop/gui/utils/NoteListUtils.test.ts @@ -38,7 +38,7 @@ describe('NoteListUtils', () => { const mockStore = { getState: () => { return { - ...createAppDefaultWindowState(null), + ...createAppDefaultWindowState(), settings: {}, }; }, diff --git a/packages/lib/services/NotePositionService.ts b/packages/lib/services/NotePositionService.ts new file mode 100644 index 0000000000..5268eeeb84 --- /dev/null +++ b/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 = new Map(); + private cursorLocations_: Map = 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 = new Map(); + private scrollLocations_: Map = 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); + } +}