fix: problems during editing (cursor jumping)

2026-02-27 17:41:50 +01:00
parent 8051e716a1
commit 84376cda01
4 changed files with 331 additions and 30 deletions
--- 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<T>(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<string, string> = {
  en: 'en-US',
  de: 'de-DE',
@@ -191,6 +201,9 @@ export const PostEditor: React.FC<PostEditorProps> = ({ postId }) => {
  const [showMediaSearch, setShowMediaSearch] = useState(false);
  const [metadataExpanded, setMetadataExpanded] = useState(true);
  const editorRef = useRef<unknown>(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<PostEditorProps> = ({ 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<PostEditorProps> = ({ 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<PostEditorProps> = ({ 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<PostEditorProps> = ({ 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<PostData>);
@@ -867,9 +886,10 @@ export const PostEditor: React.FC<PostEditorProps> = ({ postId }) => {
          
          {editorMode === 'markdown' && (
            <MonacoEditor
+              key={monacoResetToken}
              height="100%"
              language="markdown-with-macros"
-              value={content}
+              defaultValue={content}
              onChange={(value) => setContent(value || '')}
              onMount={handleEditorDidMount}
              beforeMount={handleEditorWillMount}
--- 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<LightboxProps> = ({

 // Hook to extract images from markdown content
 export function useMarkdownImages(content: string): LightboxImage[] {
-  const [images, setImages] = useState<LightboxImage[]>([]);
-
-  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
--- a/src/renderer/store/appStore.ts
+++ b/src/renderer/store/appStore.ts
@@ -382,9 +382,10 @@ export const useAppStore = create<AppState>()(
      }),
      
      // 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);
--- a/tests/renderer/components/EditorAutoSaveCursor.test.tsx
+++ 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 <div data-testid="monaco-editor" data-value={props.value} data-default-value={props.defaultValue} />;
+  },
+}));
+
+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: () => <div data-testid="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<string, unknown> = {}) => ({
+  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<string, EventListener[]>;
+
+  beforeEach(() => {
+    lastMonacoValue = undefined;
+    lastMonacoDefaultValue = undefined;
+    lastMonacoKey = undefined;
+    monacoOnChange = null;
+    eventHandlers = new Map();
+    vi.clearAllMocks();
+
+    const neverSettles = new Promise<never>(() => {});
+
+    (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<string>(),
+      isLoading: false,
+    });
+  });
+
+  afterEach(() => {
+    useAppStore.setState({
+      dirtyPosts: new Set<string>(),
+    });
+  });
+
+  /** 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(<PostEditor postId="post-1" />);
+
+    // 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(<PostEditor postId="post-1" />);
+
+    // 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(<PostEditor postId="post-1" />);
+
+    // 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();
+  });
+});