From 84376cda018e9287abd58f9a202cecc9a6d00b0b Mon Sep 17 00:00:00 2001 From: hugo Date: Fri, 27 Feb 2026 17:41:50 +0100 Subject: [PATCH] fix: problems during editing (cursor jumping) --- src/renderer/components/Editor/Editor.tsx | 58 ++-- src/renderer/components/Lightbox/Lightbox.tsx | 11 +- src/renderer/store/appStore.ts | 7 +- .../components/EditorAutoSaveCursor.test.tsx | 285 ++++++++++++++++++ 4 files changed, 331 insertions(+), 30 deletions(-) create mode 100644 tests/renderer/components/EditorAutoSaveCursor.test.tsx diff --git a/src/renderer/components/Editor/Editor.tsx b/src/renderer/components/Editor/Editor.tsx index 414a408..4a96c4a 100644 --- a/src/renderer/components/Editor/Editor.tsx +++ b/src/renderer/components/Editor/Editor.tsx @@ -30,6 +30,16 @@ import documentationContent from '../../../../DOCUMENTATION.md?raw'; import apiDocumentationContent from '../../../../API.md?raw'; import './Editor.css'; +/** Debounce a value so it only updates after `delay` ms of inactivity. */ +function useDebouncedValue(value: T, delay: number): T { + const [debounced, setDebounced] = useState(value); + useEffect(() => { + const id = setTimeout(() => setDebounced(value), delay); + return () => clearTimeout(id); + }, [value, delay]); + return debounced; +} + const UI_DATE_LOCALE: Record = { en: 'en-US', de: 'de-DE', @@ -191,6 +201,9 @@ export const PostEditor: React.FC = ({ postId }) => { const [showMediaSearch, setShowMediaSearch] = useState(false); const [metadataExpanded, setMetadataExpanded] = useState(true); const editorRef = useRef(null); + // Token incremented to signal Monaco that it should re-read its defaultValue. + // This is used instead of controlled `value` to avoid cursor-reset races. + const [monacoResetToken, setMonacoResetToken] = useState(0); const isDirty = checkIsDirty(postId); @@ -211,8 +224,11 @@ export const PostEditor: React.FC = ({ postId }) => { window.electronAPI?.posts.hasPublishedVersion(postId).then(setHasPublishedVersion); }, [postId]); - // Resolve media URLs in content for display - const resolvedContent = useMemo(() => resolveMediaUrls(content, media), [content, media]); + // Debounce content for lightbox-only computations (not time-critical) + const debouncedContent = useDebouncedValue(content, 500); + + // Resolve media URLs in content for display (lightbox only) + const resolvedContent = useMemo(() => resolveMediaUrls(debouncedContent, media), [debouncedContent, media]); // Extract images from resolved content for lightbox const images = useMarkdownImages(resolvedContent); @@ -292,38 +308,39 @@ export const PostEditor: React.FC = ({ postId }) => { }, [postId]); // Reset when post data is loaded or changes + // Only sync local state from post on initial load. + // After initialization, local state is the source of truth — this prevents + // auto-save or manual-save completions from overwriting the user's in-progress + // edits and causing the Monaco editor to reset cursor position. useEffect(() => { - if (post) { + if (post && !isInitialized) { setTitle(post.title); setContent(post.content); setAuthor(post.author || ''); setTags(post.tags); setSelectedCategories(post.categories.length > 0 ? post.categories : ['article']); - if (!isInitialized) { - setMetadataExpanded(post.title === ''); - } + setMetadataExpanded(post.title === ''); markClean(postId); // Mark as initialized AFTER setting local state setIsInitialized(true); } - }, [post, postId, markClean]); + }, [post, postId, markClean, isInitialized]); // Track changes and notify auto-save manager // Only run after form has been initialized from post data useEffect(() => { if (!post || !isInitialized) return; - const tagsChanged = JSON.stringify(tags.slice().sort()) !== JSON.stringify(post.tags.slice().sort()); - const categoriesChanged = JSON.stringify(selectedCategories.slice().sort()) !== JSON.stringify(post.categories.slice().sort()); + + // Short-circuit: check cheap comparisons first (content changes on every keystroke) + const contentChanged = content !== post.content; + const titleChanged = title !== post.title; const authorChanged = author !== (post.author || ''); - const hasChanges = - title !== post.title || - content !== post.content || - authorChanged || - tagsChanged || - categoriesChanged; - + const hasChanges = contentChanged || titleChanged || authorChanged || + JSON.stringify(tags.slice().sort()) !== JSON.stringify(post.tags.slice().sort()) || + JSON.stringify(selectedCategories.slice().sort()) !== JSON.stringify(post.categories.slice().sort()); + if (hasChanges) { - markDirty(postId); + if (!isDirty) markDirty(postId); // Notify auto-save manager with accumulated changes // Convert tags array to comma-separated string for auto-save compatibility autoSaveManager.notifyChange(postId, { @@ -336,7 +353,7 @@ export const PostEditor: React.FC = ({ postId }) => { } else { markClean(postId); } - }, [title, content, author, tags, selectedCategories, post, postId, isInitialized, markDirty, markClean]); + }, [title, content, author, tags, selectedCategories, post, postId, isInitialized, isDirty, markDirty, markClean]); // Handle editor mode change and persist preference const handleEditorModeChange = (mode: EditorMode) => { @@ -419,6 +436,8 @@ export const PostEditor: React.FC = ({ postId }) => { setAuthor(reverted.author || ''); setTags(reverted.tags); setSelectedCategories(reverted.categories.length > 0 ? reverted.categories : ['article']); + // Force Monaco to remount with the reverted content + setMonacoResetToken(prev => prev + 1); // Update local post state so UI reflects the published status setPost(reverted as PostData); updatePost(postId, reverted as Partial); @@ -867,9 +886,10 @@ export const PostEditor: React.FC = ({ postId }) => { {editorMode === 'markdown' && ( setContent(value || '')} onMount={handleEditorDidMount} beforeMount={handleEditorWillMount} diff --git a/src/renderer/components/Lightbox/Lightbox.tsx b/src/renderer/components/Lightbox/Lightbox.tsx index 46e08f8..2737405 100644 --- a/src/renderer/components/Lightbox/Lightbox.tsx +++ b/src/renderer/components/Lightbox/Lightbox.tsx @@ -1,4 +1,4 @@ -import React, { useState, useEffect, useCallback } from 'react'; +import React, { useState, useEffect, useMemo, useCallback } from 'react'; import { useI18n } from '../../i18n'; import './Lightbox.css'; @@ -155,10 +155,7 @@ export const Lightbox: React.FC = ({ // Hook to extract images from markdown content export function useMarkdownImages(content: string): LightboxImage[] { - const [images, setImages] = useState([]); - - useEffect(() => { - // Match markdown image syntax: ![alt](src) + return useMemo(() => { const imageRegex = /!\[([^\]]*)\]\(([^)]+)\)/g; const matches: LightboxImage[] = []; let match; @@ -170,10 +167,8 @@ export function useMarkdownImages(content: string): LightboxImage[] { }); } - setImages(matches); + return matches; }, [content]); - - return images; } // Component to render images with lightbox support diff --git a/src/renderer/store/appStore.ts b/src/renderer/store/appStore.ts index 23faa8c..bb76acc 100644 --- a/src/renderer/store/appStore.ts +++ b/src/renderer/store/appStore.ts @@ -382,9 +382,10 @@ export const useAppStore = create()( }), // Dirty tracking - markDirty: (id) => set((state) => ({ - dirtyPosts: new Set([...state.dirtyPosts, id]), - })), + markDirty: (id) => set((state) => { + if (state.dirtyPosts.has(id)) return state; + return { dirtyPosts: new Set([...state.dirtyPosts, id]) }; + }), markClean: (id) => set((state) => { const newDirtyPosts = new Set(state.dirtyPosts); diff --git a/tests/renderer/components/EditorAutoSaveCursor.test.tsx b/tests/renderer/components/EditorAutoSaveCursor.test.tsx new file mode 100644 index 0000000..b9a641c --- /dev/null +++ b/tests/renderer/components/EditorAutoSaveCursor.test.tsx @@ -0,0 +1,285 @@ +import React from 'react'; +import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest'; +import { render, act } from '@testing-library/react'; + +// Capture the value/defaultValue props passed to the Monaco editor +let lastMonacoValue: string | undefined; +let lastMonacoDefaultValue: string | undefined; +let lastMonacoKey: string | number | undefined; +let monacoOnChange: ((value: string | undefined) => void) | null = null; + +vi.mock('@monaco-editor/react', () => ({ + default: (props: { value?: string; defaultValue?: string; onChange?: (value: string | undefined) => void; key?: string | number }) => { + lastMonacoValue = props.value; + lastMonacoDefaultValue = props.defaultValue; + lastMonacoKey = props.key; + monacoOnChange = props.onChange ?? null; + return
; + }, +})); + +vi.mock('@milkdown/kit/core', () => { + const makeChain = () => { + const chain = { + config: (callback: (ctx: { set: () => void; get: () => { markdownUpdated: () => void } }) => void) => { + callback({ + set: () => {}, + get: () => ({ markdownUpdated: () => {} }), + }); + return chain; + }, + use: () => chain, + }; + return chain; + }; + return { + Editor: { make: makeChain }, + defaultValueCtx: Symbol('defaultValueCtx'), + editorViewCtx: Symbol('editorViewCtx'), + rootCtx: Symbol('rootCtx'), + remarkStringifyOptionsCtx: Symbol('remarkStringifyOptionsCtx'), + remarkPluginsCtx: Symbol('remarkPluginsCtx'), + }; +}); + +vi.mock('@milkdown/kit/preset/commonmark', () => ({ + commonmark: {}, + toggleStrongCommand: { key: 'toggleStrong' }, + toggleEmphasisCommand: { key: 'toggleEmphasis' }, + wrapInBlockquoteCommand: { key: 'wrapInBlockquote' }, + wrapInBulletListCommand: { key: 'wrapInBulletList' }, + wrapInOrderedListCommand: { key: 'wrapInOrderedList' }, + insertHrCommand: { key: 'insertHr' }, + toggleInlineCodeCommand: { key: 'toggleInlineCode' }, + insertImageCommand: { key: 'insertImage' }, + toggleLinkCommand: { key: 'toggleLink' }, +})); + +vi.mock('@milkdown/kit/preset/gfm', () => ({ + gfm: {}, + toggleStrikethroughCommand: { key: 'toggleStrike' }, +})); + +vi.mock('@milkdown/kit/plugin/history', () => ({ + history: {}, + undoCommand: { key: 'undo' }, + redoCommand: { key: 'redo' }, +})); + +vi.mock('@milkdown/kit/plugin/listener', () => ({ + listener: {}, + listenerCtx: Symbol('listenerCtx'), +})); + +vi.mock('@milkdown/kit/plugin/clipboard', () => ({ clipboard: {} })); +vi.mock('@milkdown/kit/plugin/trailing', () => ({ trailing: {} })); +vi.mock('@milkdown/kit/plugin/indent', () => ({ indent: {} })); +vi.mock('@milkdown/kit/plugin/cursor', () => ({ cursor: {} })); + +vi.mock('@milkdown/kit/utils', () => ({ + $node: () => ({}), + $inputRule: () => ({}), + $remark: () => ({}), + $prose: () => ({}), + replaceAll: () => () => {}, + callCommand: () => () => {}, +})); + +vi.mock('@milkdown/react', () => ({ + Milkdown: () =>
, + MilkdownProvider: ({ children }: { children: React.ReactNode }) => <>{children}, + useInstance: () => [false, () => ({ action: (action: unknown) => { + if (typeof action === 'function') { + action({ get: () => ({}) }); + } + } })] as const, + useEditor: (factory: (root: Node) => unknown) => { + factory(document.createElement('div')); + }, +})); + +vi.mock('../../../src/renderer/components/Lightbox', () => ({ + Lightbox: () => null, + useMarkdownImages: () => [], +})); +vi.mock('../../../src/renderer/components/PostLinks', () => ({ PostLinks: () => null })); +vi.mock('../../../src/renderer/components/LinkedMediaPanel', () => ({ LinkedMediaPanel: () => null })); +vi.mock('../../../src/renderer/components/ErrorModal', () => ({ ErrorModal: () => null })); +vi.mock('../../../src/renderer/components/ConfirmDeleteModal', () => ({ ConfirmDeleteModal: () => null })); +vi.mock('../../../src/renderer/components/SettingsView', () => ({ SettingsView: () => null })); +vi.mock('../../../src/renderer/components/TagsView', () => ({ TagsView: () => null })); +vi.mock('../../../src/renderer/components/TagInput', () => ({ TagInput: () => null })); +vi.mock('../../../src/renderer/components/ChatPanel', () => ({ ChatPanel: () => null })); +vi.mock('../../../src/renderer/components/ImportAnalysisView', () => ({ ImportAnalysisView: () => null })); +vi.mock('../../../src/renderer/components/MetadataDiffPanel', () => ({ MetadataDiffPanel: () => null })); +vi.mock('../../../src/renderer/components/GitDiffView/GitDiffView', () => ({ GitDiffView: () => null })); +vi.mock('../../../src/renderer/components/InsertModal', () => ({ InsertModal: () => null })); +vi.mock('../../../src/renderer/components/AISuggestionsModal/AISuggestionsModal', () => ({ + AISuggestionsModal: () => null, +})); +vi.mock('../../../src/renderer/components/Toast', () => ({ + showToast: { + success: vi.fn(), + error: vi.fn(), + }, +})); + +import { PostEditor } from '../../../src/renderer/components/Editor/Editor'; +import { useAppStore } from '../../../src/renderer/store'; + +const INITIAL_CONTENT = '# Hello World\n\nThis is the original content.'; + +const createPost = (overrides: Record = {}) => ({ + id: 'post-1', + title: 'Test Post', + content: INITIAL_CONTENT, + excerpt: '', + slug: 'test-post', + status: 'draft' as const, + tags: [], + categories: ['article'], + featuredImage: null, + publishedAt: null, + createdAt: new Date('2026-02-16T12:00:00.000Z'), + updatedAt: new Date('2026-02-16T12:00:00.000Z'), + author: undefined, + metadata: {}, + seoTitle: undefined, + seoDescription: undefined, + canonicalUrl: undefined, + projectId: 'project-1', + filePath: 'posts/test-post.md', + ...overrides, +}); + +describe('Editor does not reset content on auto-save (cursor stability)', () => { + // Capture event handlers registered via mocked addEventListener + let eventHandlers: Map; + + beforeEach(() => { + lastMonacoValue = undefined; + lastMonacoDefaultValue = undefined; + lastMonacoKey = undefined; + monacoOnChange = null; + eventHandlers = new Map(); + vi.clearAllMocks(); + + const neverSettles = new Promise(() => {}); + + (window as any).addEventListener = vi.fn((event: string, handler: EventListener) => { + if (!eventHandlers.has(event)) eventHandlers.set(event, []); + eventHandlers.get(event)!.push(handler); + }); + (window as any).removeEventListener = vi.fn((event: string, handler: EventListener) => { + const handlers = eventHandlers.get(event); + if (handlers) { + const idx = handlers.indexOf(handler); + if (idx >= 0) handlers.splice(idx, 1); + } + }); + + (window as any).electronAPI.posts.get = vi.fn().mockResolvedValue(createPost()); + (window as any).electronAPI.posts.hasPublishedVersion = vi.fn().mockReturnValue(neverSettles); + (window as any).electronAPI.posts.update = vi.fn().mockResolvedValue(null); + (window as any).electronAPI.posts.getPreviewUrl = vi.fn().mockResolvedValue('http://localhost:4123/preview'); + (window as any).electronAPI.meta.getCategories = vi.fn().mockReturnValue(neverSettles); + + useAppStore.setState({ + preferredEditorMode: 'markdown', + posts: [], + media: [], + dirtyPosts: new Set(), + isLoading: false, + }); + }); + + afterEach(() => { + useAppStore.setState({ + dirtyPosts: new Set(), + }); + }); + + /** Dispatch a synthetic event to all captured handlers */ + function fireEvent(name: string, detail: unknown) { + const handlers = eventHandlers.get(name) ?? []; + const event = { detail } as unknown as Event; + for (const h of handlers) h(event); + } + + it('uses uncontrolled Monaco (defaultValue, not value) to prevent cursor jumps', async () => { + render(); + + // Wait for initial post load and initialization + await act(async () => { + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + }); + + // Monaco must be in uncontrolled mode: defaultValue set, value undefined + expect(lastMonacoDefaultValue).toBe(INITIAL_CONTENT); + expect(lastMonacoValue).toBeUndefined(); + }); + + it('does not reset Monaco when auto-save event updates the post', async () => { + render(); + + // Wait for initial post load and initialization + await act(async () => { + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + }); + + const initialKey = lastMonacoKey; + + // User types more content in the editor + const userEditedContent = INITIAL_CONTENT + '\n\nNew paragraph typed by user.'; + await act(async () => { + monacoOnChange?.(userEditedContent); + }); + + // Simulate auto-save completing with the OLD content (before user's latest edits). + const autoSavedPost = createPost(); // still has INITIAL_CONTENT + await act(async () => { + fireEvent('bds:post-auto-saved', { id: 'post-1', updated: autoSavedPost }); + }); + + // Monaco must NOT have been remounted (key unchanged) — content stays untouched + expect(lastMonacoKey).toBe(initialKey); + // value prop must still be undefined (uncontrolled) + expect(lastMonacoValue).toBeUndefined(); + }); + + it('does not reset Monaco when manual save updates the post', async () => { + // Make update return the saved post data + const savedPost = createPost(); + (window as any).electronAPI.posts.update = vi.fn().mockResolvedValue(savedPost); + + render(); + + // Wait for initialization + await act(async () => { + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + }); + + const initialKey = lastMonacoKey; + + // User edits + const userEditedContent = 'Completely rewritten content'; + await act(async () => { + monacoOnChange?.(userEditedContent); + }); + + // Simulate post state update (as happens after save completes with older content) + await act(async () => { + fireEvent('bds:post-auto-saved', { id: 'post-1', updated: savedPost }); + }); + + // Monaco must NOT have been remounted + expect(lastMonacoKey).toBe(initialKey); + expect(lastMonacoValue).toBeUndefined(); + }); +});