feat: preserve scroll position per file in editor (#349)
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Brennan Benson
GitHub
parent
499bde7b2e
commit
001846ccc3
|
|
@ -0,0 +1,213 @@
|
|||
# Design: Preserve Scroll Position Per File in Editor
|
||||
|
||||
## Context
|
||||
|
||||
When navigating between file tabs, the scroll position resets to the top because each editor component unmounts and remounts. The cursor line is already tracked per file via `editorCursorLine` in the store, but scroll position is not. This causes a poor UX when users switch between files frequently.
|
||||
|
||||
## Approach
|
||||
|
||||
Use a module-scoped `Map<string, number>` with LRU eviction, following the proven pattern from `CombinedDiffViewer` (lines 39–58). The scroll cache is extracted into a shared utility at `src/renderer/src/lib/scroll-cache.ts` so all viewer components share one bounded Map.
|
||||
|
||||
**Why not Zustand?** Scroll position updates at high frequency during user interaction. Putting it in Zustand means every write spreads a new object (`{ ...s.editorScrollTop, [fileId]: scrollTop }`), which (a) causes React re-render notifications even though no component subscribes to scroll position for rendering, and (b) generates object allocation churn on every scroll event. A module-scoped Map avoids both problems: zero re-renders, zero GC pressure, O(1) reads and writes. CombinedDiffViewer already validates this approach in production.
|
||||
|
||||
**Why LRU at 20 entries?** Without a cap, the Map grows unboundedly as unique file IDs accumulate across a session. 20 entries covers typical tab working sets with headroom. This matches `CombinedDiffViewer`'s existing cap and means no explicit cleanup is needed when files are closed — eviction is automatic.
|
||||
|
||||
## Files to Modify
|
||||
|
||||
### 1. Shared Utility: `src/renderer/src/lib/scroll-cache.ts` (new file)
|
||||
|
||||
Extract `setWithLRU` from `CombinedDiffViewer` into a shared module, and expose a single scroll cache Map:
|
||||
|
||||
```ts
|
||||
const CACHE_MAX_ENTRIES = 20
|
||||
|
||||
// Why: Module-scoped Maps grow unboundedly as unique file keys accumulate.
|
||||
// Cap them with a simple LRU eviction: after each set, if the map exceeds
|
||||
// this limit, delete the oldest entry (Maps iterate in insertion order).
|
||||
export function setWithLRU<K, V>(map: Map<K, V>, key: K, value: V): void {
|
||||
// Re-insert to refresh insertion order (move to end).
|
||||
map.delete(key)
|
||||
map.set(key, value)
|
||||
if (map.size > CACHE_MAX_ENTRIES) {
|
||||
const oldestKey = map.keys().next().value
|
||||
if (oldestKey !== undefined) {
|
||||
map.delete(oldestKey)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Why: A single shared Map for scroll positions across all editor components.
|
||||
// Module-scoped so it survives component unmount/remount without triggering
|
||||
// React re-renders (unlike Zustand, which would broadcast state changes on
|
||||
// every scroll event even though no component renders from scroll position).
|
||||
export const scrollTopCache = new Map<string, number>()
|
||||
```
|
||||
|
||||
After extracting, update `CombinedDiffViewer` to import `setWithLRU` from this module instead of defining it inline. `CombinedDiffViewer`'s own `combinedDiffViewStateCache` and `combinedDiffScrollTopCache` stay local since their value types are component-specific.
|
||||
|
||||
---
|
||||
|
||||
### 2. MonacoEditor: `src/renderer/src/components/editor/MonacoEditor.tsx`
|
||||
|
||||
**Save scroll position:**
|
||||
|
||||
In `handleMount` (line ~62), add a throttled scroll listener:
|
||||
|
||||
```ts
|
||||
import { scrollTopCache, setWithLRU } from '@renderer/lib/scroll-cache'
|
||||
|
||||
// Why: Writing to the Map at 60fps (every scroll frame) is unnecessary since
|
||||
// we only need the final position when the user stops scrolling or switches
|
||||
// tabs. A trailing throttle of ~150ms captures the resting position while
|
||||
// avoiding excessive writes.
|
||||
let scrollThrottleTimer: ReturnType<typeof setTimeout> | null = null
|
||||
|
||||
editorInstance.onDidScrollChange((e) => {
|
||||
if (scrollThrottleTimer !== null) clearTimeout(scrollThrottleTimer)
|
||||
scrollThrottleTimer = setTimeout(() => {
|
||||
setWithLRU(scrollTopCache, filePath, e.scrollTop)
|
||||
scrollThrottleTimer = null
|
||||
}, 150)
|
||||
})
|
||||
```
|
||||
|
||||
Also snapshot the current position synchronously in the cleanup/unmount path (before the timeout fires) so tab switches always capture the latest value — same pattern as `CombinedDiffViewer`'s `updateCachedScrollPosition()` call in its cleanup return.
|
||||
|
||||
**Restore scroll position:**
|
||||
|
||||
In the `else` branch (line ~118) where there is NO pending reveal:
|
||||
|
||||
```ts
|
||||
const savedScrollTop = scrollTopCache.get(filePath)
|
||||
if (savedScrollTop !== undefined) {
|
||||
// Why: Monaco renders synchronously, so a single RAF is sufficient to
|
||||
// wait for the layout pass. Unlike react-markdown or Tiptap, there is
|
||||
// no async content loading that would require a retry loop.
|
||||
requestAnimationFrame(() => editorInstance.setScrollTop(savedScrollTop))
|
||||
}
|
||||
```
|
||||
|
||||
**Key edge case:** When `pendingEditorReveal` exists (search-result navigation), skip scroll restoration — `performReveal` handles its own scroll.
|
||||
|
||||
---
|
||||
|
||||
### 3. MarkdownPreview: `src/renderer/src/components/editor/MarkdownPreview.tsx`
|
||||
|
||||
**Mode-scoped cache key:**
|
||||
|
||||
```ts
|
||||
// Why: Each markdown viewing mode (source/rich/preview) produces different
|
||||
// DOM structures and content heights. A scroll position saved in source mode
|
||||
// (a code block at line 500) has no meaningful correspondence in preview mode
|
||||
// (rendered HTML at a completely different height). Using mode-scoped keys
|
||||
// means each mode remembers its own position independently.
|
||||
const scrollCacheKey = `${filePath}:preview`
|
||||
```
|
||||
|
||||
**Save scroll position:**
|
||||
|
||||
Add a throttled scroll listener on `rootRef.current` (the scrollable div, line 192) via `useLayoutEffect`:
|
||||
|
||||
```ts
|
||||
useLayoutEffect(() => {
|
||||
const container = rootRef.current
|
||||
if (!container) return
|
||||
|
||||
let throttleTimer: ReturnType<typeof setTimeout> | null = null
|
||||
|
||||
const onScroll = (): void => {
|
||||
if (throttleTimer !== null) clearTimeout(throttleTimer)
|
||||
throttleTimer = setTimeout(() => {
|
||||
setWithLRU(scrollTopCache, scrollCacheKey, container.scrollTop)
|
||||
throttleTimer = null
|
||||
}, 150)
|
||||
}
|
||||
|
||||
container.addEventListener('scroll', onScroll, { passive: true })
|
||||
return () => {
|
||||
// Snapshot final position synchronously before detach.
|
||||
setWithLRU(scrollTopCache, scrollCacheKey, container.scrollTop)
|
||||
if (throttleTimer !== null) clearTimeout(throttleTimer)
|
||||
container.removeEventListener('scroll', onScroll)
|
||||
}
|
||||
}, [scrollCacheKey])
|
||||
```
|
||||
|
||||
**Restore scroll position (RAF retry loop):**
|
||||
|
||||
react-markdown renders asynchronously — content may not be in the DOM when the layout effect first runs, so `scrollHeight` is still small and `scrollTop` gets clamped to 0. Use `CombinedDiffViewer`'s RAF retry pattern (lines 353–388) to keep attempting until content has loaded:
|
||||
|
||||
```ts
|
||||
useLayoutEffect(() => {
|
||||
const container = rootRef.current
|
||||
const targetScrollTop = scrollTopCache.get(scrollCacheKey)
|
||||
if (!container || targetScrollTop === undefined) return
|
||||
|
||||
let frameId = 0
|
||||
let attempts = 0
|
||||
|
||||
// Why: react-markdown renders asynchronously, so scrollHeight may still be
|
||||
// too small on the first frame. Retry up to 30 frames (~500ms at 60fps) to
|
||||
// accommodate content loading. This matches CombinedDiffViewer's proven
|
||||
// pattern for dynamic-height content restoration.
|
||||
const tryRestore = (): void => {
|
||||
const maxScrollTop = Math.max(0, container.scrollHeight - container.clientHeight)
|
||||
const nextScrollTop = Math.min(targetScrollTop, maxScrollTop)
|
||||
container.scrollTop = nextScrollTop
|
||||
|
||||
if (Math.abs(container.scrollTop - targetScrollTop) <= 1 || maxScrollTop >= targetScrollTop) {
|
||||
return
|
||||
}
|
||||
|
||||
attempts += 1
|
||||
if (attempts < 30) {
|
||||
frameId = window.requestAnimationFrame(tryRestore)
|
||||
}
|
||||
}
|
||||
|
||||
tryRestore()
|
||||
return () => window.cancelAnimationFrame(frameId)
|
||||
}, [scrollCacheKey])
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### 4. RichMarkdownEditor: `src/renderer/src/components/editor/RichMarkdownEditor.tsx`
|
||||
|
||||
Same approach as MarkdownPreview with two differences:
|
||||
|
||||
1. **Cache key:** `${filePath}:rich`
|
||||
2. **Scroll container:** The scrollable container is `<EditorContent>` with `overflow-auto` (line 334). Wrap in a div with a ref to get the scroll container, move `overflow-auto` to wrapper.
|
||||
|
||||
The RAF retry loop is needed here too — Tiptap renders asynchronously as it hydrates its ProseMirror document, so `scrollHeight` may be undersized on the initial frame.
|
||||
|
||||
Save/restore logic is identical to MarkdownPreview, substituting the container ref and cache key.
|
||||
|
||||
---
|
||||
|
||||
### 5. DiffViewer — Deferred
|
||||
|
||||
DiffViewer doesn't currently receive `filePath` and diff views are typically opened for brief review. Deferring to avoid scope creep. `CombinedDiffViewer` already has its own scroll cache; if DiffViewer needs one later it can import from `scroll-cache.ts`.
|
||||
|
||||
## Edge Cases
|
||||
|
||||
| Case | Behavior |
|
||||
| --------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
|
||||
| Pending reveal (search nav) | Scroll restoration skipped; `performReveal` takes priority |
|
||||
| First open (no saved position) | `undefined` in cache → no restoration, starts at top |
|
||||
| Markdown mode switch (source → preview) | Mode-scoped keys (`path:source`, `path:preview`, `path:rich`) mean each mode preserves its own position independently. No cross-mode confusion. |
|
||||
| Scroll at 0 | Always restore (including 0) to guard against Monaco auto-scroll behavior |
|
||||
| External file content changes | **Known trade-off:** When external changes add/remove content above the viewport, the saved scroll position points to different content. Monaco clamps `scrollTop` if it exceeds the new document length, but does not adjust for insertions above the viewport. HTML containers (`MarkdownPreview`, `RichMarkdownEditor`) behave the same way — the browser clamps `scrollTop` to `scrollHeight - clientHeight`. This is acceptable: scroll position is a best-effort hint, not a semantic anchor. Fixing this would require mapping scroll offsets to content anchors (like line numbers), which is out of scope. |
|
||||
| LRU eviction (>20 files) | Oldest scroll entry is evicted. User sees top-of-file on return to a very old tab — same as a fresh open. No data corruption or memory leak. |
|
||||
| Close file, reopen | LRU may or may not still have the entry. If present, position restores. If evicted, starts at top. No explicit cleanup needed. |
|
||||
| Async content (react-markdown, Tiptap) | RAF retry loop (up to 30 attempts) handles content that renders after the initial layout pass. Falls back to best-effort clamped position if content never reaches the target height. |
|
||||
|
||||
## Verification
|
||||
|
||||
1. Open a code file, scroll down ~50%, switch to another tab, switch back → verify position preserved
|
||||
2. Open a markdown file in preview mode, scroll down, switch tabs, return → verify position preserved
|
||||
3. Open a markdown file in rich mode, scroll down, switch tabs, return → verify position preserved
|
||||
4. Switch a markdown file from source to preview mode → verify each mode has independent scroll position (scroll in source, switch to preview, preview starts at its own saved position or top)
|
||||
5. Use Cmd+Shift+F to search, click a result → verify it scrolls to the match (NOT saved position)
|
||||
6. Open 25+ files to trigger LRU eviction, return to the earliest file → verify it starts at top gracefully
|
||||
7. `pnpm run typecheck` passes
|
||||
|
|
@ -7,6 +7,7 @@ import React, { useState, useEffect, useCallback, useRef, useLayoutEffect } from
|
|||
import type { editor as monacoEditor } from 'monaco-editor'
|
||||
import { useAppStore } from '@/store'
|
||||
import { joinPath } from '@/lib/path'
|
||||
import { setWithLRU } from '@/lib/scroll-cache'
|
||||
import '@/lib/monaco-setup'
|
||||
import { Button } from '@/components/ui/button'
|
||||
import type { OpenFile } from '@/store/slices/editor'
|
||||
|
|
@ -39,24 +40,6 @@ type CachedCombinedDiffViewState = {
|
|||
const combinedDiffViewStateCache = new Map<string, CachedCombinedDiffViewState>()
|
||||
const combinedDiffScrollTopCache = new Map<string, number>()
|
||||
|
||||
// Why: Module-scoped Maps grow unboundedly as unique file.ids accumulate.
|
||||
// Cap them with a simple LRU eviction: after each set, if the map exceeds
|
||||
// this limit, delete the oldest entry (Maps iterate in insertion order).
|
||||
const CACHE_MAX_ENTRIES = 20
|
||||
|
||||
function setWithLRU<K, V>(map: Map<K, V>, key: K, value: V): void {
|
||||
// Re-insert to refresh insertion order (move to end).
|
||||
map.delete(key)
|
||||
map.set(key, value)
|
||||
if (map.size > CACHE_MAX_ENTRIES) {
|
||||
// The first key in the Map is the oldest entry.
|
||||
const oldestKey = map.keys().next().value
|
||||
if (oldestKey !== undefined) {
|
||||
map.delete(oldestKey)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
export default function CombinedDiffViewer({ file }: { file: OpenFile }): React.JSX.Element {
|
||||
const settings = useAppStore((s) => s.settings)
|
||||
const gitStatusByWorktree = useAppStore((s) => s.gitStatusByWorktree)
|
||||
|
|
|
|||
|
|
@ -70,7 +70,12 @@ export function EditorContent({
|
|||
activeFile.diffSource === 'combined-branch')
|
||||
|
||||
const renderMonacoEditor = (fc: FileContent): React.JSX.Element => (
|
||||
// Why: Without a key, React reuses the same MonacoEditor instance when
|
||||
// switching tabs, just updating props. That means useLayoutEffect cleanup
|
||||
// (which snapshots scroll position) never fires. Keying on activeFile.id
|
||||
// forces unmount/remount so the scroll cache captures the outgoing position.
|
||||
<MonacoEditor
|
||||
key={activeFile.id}
|
||||
filePath={activeFile.filePath}
|
||||
relativePath={activeFile.relativePath}
|
||||
content={editBuffers[activeFile.id] ?? fc.content}
|
||||
|
|
@ -103,7 +108,9 @@ export function EditorContent({
|
|||
|
||||
if (renderMode === 'rich-editor') {
|
||||
return (
|
||||
// Why: same remount reasoning as MonacoEditor — see renderMonacoEditor.
|
||||
<RichMarkdownEditor
|
||||
key={activeFile.id}
|
||||
content={currentContent}
|
||||
filePath={activeFile.filePath}
|
||||
onContentChange={handleContentChange}
|
||||
|
|
@ -123,7 +130,11 @@ export function EditorContent({
|
|||
to that renderer preserves readable preview mode instead of forcing the
|
||||
user out of preview entirely. Source mode remains available for edits. */}
|
||||
<div className="min-h-0 flex-1">
|
||||
<MarkdownPreview content={currentContent} filePath={activeFile.filePath} />
|
||||
<MarkdownPreview
|
||||
key={activeFile.id}
|
||||
content={currentContent}
|
||||
filePath={activeFile.filePath}
|
||||
/>
|
||||
</div>
|
||||
</div>
|
||||
)
|
||||
|
|
@ -169,7 +180,7 @@ export function EditorContent({
|
|||
}
|
||||
|
||||
if (isCombinedDiff) {
|
||||
return <CombinedDiffViewer file={activeFile} />
|
||||
return <CombinedDiffViewer key={activeFile.id} file={activeFile} />
|
||||
}
|
||||
|
||||
if (activeFile.mode === 'edit') {
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import React, { useCallback, useEffect, useRef, useState } from 'react'
|
||||
import React, { useCallback, useEffect, useLayoutEffect, useRef, useState } from 'react'
|
||||
import Markdown from 'react-markdown'
|
||||
import remarkGfm from 'remark-gfm'
|
||||
import remarkFrontmatter from 'remark-frontmatter'
|
||||
|
|
@ -9,6 +9,7 @@ import { Button } from '@/components/ui/button'
|
|||
import { Input } from '@/components/ui/input'
|
||||
import { useAppStore } from '@/store'
|
||||
import { computeEditorFontSize } from '@/lib/editor-font-zoom'
|
||||
import { scrollTopCache, setWithLRU } from '@/lib/scroll-cache'
|
||||
import { getMarkdownPreviewLinkTarget } from './markdown-preview-links'
|
||||
import { useLocalImageSrc } from './useLocalImageSrc'
|
||||
import {
|
||||
|
|
@ -42,6 +43,79 @@ export default function MarkdownPreview({
|
|||
settings?.theme === 'dark' ||
|
||||
(settings?.theme === 'system' && window.matchMedia('(prefers-color-scheme: dark)').matches)
|
||||
|
||||
// Why: Each markdown viewing mode (source/rich/preview) produces different
|
||||
// DOM structures and content heights. A scroll position saved in source mode
|
||||
// has no meaningful correspondence in preview mode. Using mode-scoped keys
|
||||
// means each mode remembers its own position independently.
|
||||
const scrollCacheKey = `${filePath}:preview`
|
||||
|
||||
// Save scroll position with trailing throttle and synchronous unmount snapshot.
|
||||
useLayoutEffect(() => {
|
||||
const container = rootRef.current
|
||||
if (!container) {
|
||||
return
|
||||
}
|
||||
|
||||
let throttleTimer: ReturnType<typeof setTimeout> | null = null
|
||||
|
||||
const onScroll = (): void => {
|
||||
if (throttleTimer !== null) {
|
||||
clearTimeout(throttleTimer)
|
||||
}
|
||||
throttleTimer = setTimeout(() => {
|
||||
setWithLRU(scrollTopCache, scrollCacheKey, container.scrollTop)
|
||||
throttleTimer = null
|
||||
}, 150)
|
||||
}
|
||||
|
||||
container.addEventListener('scroll', onScroll, { passive: true })
|
||||
return () => {
|
||||
// Snapshot final position synchronously before detach.
|
||||
setWithLRU(scrollTopCache, scrollCacheKey, container.scrollTop)
|
||||
if (throttleTimer !== null) {
|
||||
clearTimeout(throttleTimer)
|
||||
}
|
||||
container.removeEventListener('scroll', onScroll)
|
||||
}
|
||||
}, [scrollCacheKey])
|
||||
|
||||
// Restore scroll position with RAF retry loop for async react-markdown content.
|
||||
useLayoutEffect(() => {
|
||||
const container = rootRef.current
|
||||
const targetScrollTop = scrollTopCache.get(scrollCacheKey)
|
||||
if (!container || targetScrollTop === undefined) {
|
||||
return
|
||||
}
|
||||
|
||||
let frameId = 0
|
||||
let attempts = 0
|
||||
|
||||
// Why: react-markdown renders asynchronously, so scrollHeight may still be
|
||||
// too small on the first frame. Retry up to 30 frames (~500ms at 60fps) to
|
||||
// accommodate content loading. This matches CombinedDiffViewer's proven
|
||||
// pattern for dynamic-height content restoration.
|
||||
const tryRestore = (): void => {
|
||||
const maxScrollTop = Math.max(0, container.scrollHeight - container.clientHeight)
|
||||
const nextScrollTop = Math.min(targetScrollTop, maxScrollTop)
|
||||
container.scrollTop = nextScrollTop
|
||||
|
||||
if (Math.abs(container.scrollTop - targetScrollTop) <= 1 || maxScrollTop >= targetScrollTop) {
|
||||
return
|
||||
}
|
||||
|
||||
attempts += 1
|
||||
if (attempts < 30) {
|
||||
frameId = window.requestAnimationFrame(tryRestore)
|
||||
}
|
||||
}
|
||||
|
||||
tryRestore()
|
||||
return () => window.cancelAnimationFrame(frameId)
|
||||
// Why: content is included so the restore loop re-triggers when markdown
|
||||
// content arrives or changes (e.g., async file load), since scrollHeight
|
||||
// depends on rendered content and may not be large enough until then.
|
||||
}, [scrollCacheKey, content])
|
||||
|
||||
const moveToMatch = useCallback((direction: 1 | -1) => {
|
||||
const matches = matchesRef.current
|
||||
if (matches.length === 0) {
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import React, { useRef, useCallback, useEffect, useState } from 'react'
|
||||
import React, { useRef, useCallback, useEffect, useLayoutEffect, useState } from 'react'
|
||||
import Editor, { type OnMount } from '@monaco-editor/react'
|
||||
import type { editor } from 'monaco-editor'
|
||||
import { Copy, ExternalLink } from 'lucide-react'
|
||||
|
|
@ -9,6 +9,7 @@ import {
|
|||
DropdownMenuTrigger
|
||||
} from '@/components/ui/dropdown-menu'
|
||||
import { useAppStore } from '@/store'
|
||||
import { scrollTopCache, setWithLRU } from '@/lib/scroll-cache'
|
||||
import '@/lib/monaco-setup'
|
||||
import { setupContextualCopy } from './setup-contextual-copy'
|
||||
import { computeEditorFontSize } from '@/lib/editor-font-zoom'
|
||||
|
|
@ -39,6 +40,11 @@ export default function MonacoEditor({
|
|||
const editorRef = useRef<editor.IStandaloneCodeEditor | null>(null)
|
||||
const copyToastTimeoutRef = useRef<number | null>(null)
|
||||
const copyHintIntervalRef = useRef<number | null>(null)
|
||||
// Why: The scroll throttle timer must be accessible from useLayoutEffect cleanup
|
||||
// so we can cancel any pending write before synchronously snapshotting the final
|
||||
// scroll position on unmount. Without this, a pending timer could fire after
|
||||
// cleanup and overwrite the correct value with a stale one.
|
||||
const scrollThrottleTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null)
|
||||
const propsRef = useRef({ relativePath, language, onSave })
|
||||
|
||||
useEffect(() => {
|
||||
|
|
@ -96,6 +102,20 @@ export default function MonacoEditor({
|
|||
setEditorCursorLine(filePath, e.position.lineNumber)
|
||||
})
|
||||
|
||||
// Why: Writing to the Map at 60fps (every scroll frame) is unnecessary since
|
||||
// we only need the final position when the user stops scrolling or switches
|
||||
// tabs. A trailing throttle of ~150ms captures the resting position while
|
||||
// avoiding excessive writes.
|
||||
editorInstance.onDidScrollChange((e) => {
|
||||
if (scrollThrottleTimerRef.current !== null) {
|
||||
clearTimeout(scrollThrottleTimerRef.current)
|
||||
}
|
||||
scrollThrottleTimerRef.current = setTimeout(() => {
|
||||
setWithLRU(scrollTopCache, filePath, e.scrollTop)
|
||||
scrollThrottleTimerRef.current = null
|
||||
}, 150)
|
||||
})
|
||||
|
||||
// Intercept right-click on line number gutter to show Radix context menu
|
||||
// (same approach as VSCode: custom menu instead of Monaco's built-in one)
|
||||
editorInstance.onMouseDown((e) => {
|
||||
|
|
@ -122,7 +142,20 @@ export default function MonacoEditor({
|
|||
performReveal(editorInstance, reveal.line, reveal.column, reveal.matchLength)
|
||||
useAppStore.getState().setPendingEditorReveal(null)
|
||||
} else {
|
||||
editorInstance.focus()
|
||||
const savedScrollTop = scrollTopCache.get(filePath)
|
||||
if (savedScrollTop !== undefined) {
|
||||
// Why: Monaco renders synchronously, so a single RAF is sufficient to
|
||||
// wait for the layout pass. Unlike react-markdown or Tiptap, there is
|
||||
// no async content loading that would require a retry loop.
|
||||
// Focus is deferred into the same RAF to avoid a one-frame flash where
|
||||
// the editor is focused at scroll position 0 before restoration.
|
||||
requestAnimationFrame(() => {
|
||||
editorInstance.setScrollTop(savedScrollTop)
|
||||
editorInstance.focus()
|
||||
})
|
||||
} else {
|
||||
editorInstance.focus()
|
||||
}
|
||||
}
|
||||
},
|
||||
[copyShortcutLabel, filePath, setEditorCursorLine]
|
||||
|
|
@ -137,6 +170,26 @@ export default function MonacoEditor({
|
|||
[onContentChange]
|
||||
)
|
||||
|
||||
// Snapshot scroll position synchronously on unmount so tab switches always
|
||||
// capture the latest value, even if the trailing throttle hasn't fired yet.
|
||||
// Why useLayoutEffect: cleanup runs before @monaco-editor/react's useEffect
|
||||
// disposes the editor instance, guaranteeing getScrollTop() reads valid state.
|
||||
useLayoutEffect(() => {
|
||||
return () => {
|
||||
// Why: Cancel any pending throttled scroll write so it cannot fire after
|
||||
// this synchronous snapshot, which would overwrite the correct final
|
||||
// position with a stale intermediate value.
|
||||
if (scrollThrottleTimerRef.current !== null) {
|
||||
clearTimeout(scrollThrottleTimerRef.current)
|
||||
scrollThrottleTimerRef.current = null
|
||||
}
|
||||
const ed = editorRef.current
|
||||
if (ed) {
|
||||
setWithLRU(scrollTopCache, filePath, ed.getScrollTop())
|
||||
}
|
||||
}
|
||||
}, [filePath])
|
||||
|
||||
// Update editor options when settings change
|
||||
useEffect(() => {
|
||||
if (!editorRef.current || !settings) {
|
||||
|
|
|
|||
|
|
@ -1,17 +1,18 @@
|
|||
import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react'
|
||||
import React, { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react'
|
||||
import { EditorContent, useEditor } from '@tiptap/react'
|
||||
import type { Editor } from '@tiptap/react'
|
||||
import { ImageIcon, List, ListOrdered, Quote } from 'lucide-react'
|
||||
import { toast } from 'sonner'
|
||||
import { cn } from '@/lib/utils'
|
||||
import { useAppStore } from '@/store'
|
||||
import { scrollTopCache, setWithLRU } from '@/lib/scroll-cache'
|
||||
import { RichMarkdownToolbarButton } from './RichMarkdownToolbarButton'
|
||||
import { isMarkdownPreviewFindShortcut } from './markdown-preview-search'
|
||||
import { extractIpcErrorMessage, getImageCopyDestination } from './rich-markdown-image-utils'
|
||||
import { encodeRawMarkdownHtmlForRichEditor } from './raw-markdown-html'
|
||||
import { createRichMarkdownExtensions } from './rich-markdown-extensions'
|
||||
import { slashCommands } from './rich-markdown-commands'
|
||||
import type { SlashCommand } from './rich-markdown-commands'
|
||||
import { runSlashCommand, slashCommands, syncSlashMenu } from './rich-markdown-commands'
|
||||
import type { SlashCommand, SlashMenuState } from './rich-markdown-commands'
|
||||
import { RichMarkdownSearchBar } from './RichMarkdownSearchBar'
|
||||
import { useRichMarkdownSearch } from './useRichMarkdownSearch'
|
||||
|
||||
|
|
@ -22,14 +23,6 @@ type RichMarkdownEditorProps = {
|
|||
onSave: (content: string) => void
|
||||
}
|
||||
|
||||
type SlashMenuState = {
|
||||
query: string
|
||||
from: number
|
||||
to: number
|
||||
left: number
|
||||
top: number
|
||||
}
|
||||
|
||||
const richMarkdownExtensions = createRichMarkdownExtensions({
|
||||
includePlaceholder: true
|
||||
})
|
||||
|
|
@ -42,6 +35,7 @@ export default function RichMarkdownEditor({
|
|||
}: RichMarkdownEditorProps): React.JSX.Element {
|
||||
const rootRef = useRef<HTMLDivElement | null>(null)
|
||||
const editorFontZoomLevel = useAppStore((s) => s.editorFontZoomLevel)
|
||||
const scrollContainerRef = useRef<HTMLDivElement | null>(null)
|
||||
const [slashMenu, setSlashMenu] = useState<SlashMenuState | null>(null)
|
||||
const [selectedCommandIndex, setSelectedCommandIndex] = useState(0)
|
||||
const isMac = navigator.userAgent.includes('Mac')
|
||||
|
|
@ -157,6 +151,77 @@ export default function RichMarkdownEditor({
|
|||
useEffect(() => {
|
||||
editorRef.current = editor ?? null
|
||||
}, [editor])
|
||||
|
||||
const scrollCacheKey = `${filePath}:rich`
|
||||
|
||||
// Save scroll position with trailing throttle and synchronous unmount snapshot.
|
||||
useLayoutEffect(() => {
|
||||
const container = scrollContainerRef.current
|
||||
if (!container) {
|
||||
return
|
||||
}
|
||||
|
||||
let throttleTimer: ReturnType<typeof setTimeout> | null = null
|
||||
|
||||
const onScroll = (): void => {
|
||||
if (throttleTimer !== null) {
|
||||
clearTimeout(throttleTimer)
|
||||
}
|
||||
throttleTimer = setTimeout(() => {
|
||||
setWithLRU(scrollTopCache, scrollCacheKey, container.scrollTop)
|
||||
throttleTimer = null
|
||||
}, 150)
|
||||
}
|
||||
|
||||
container.addEventListener('scroll', onScroll, { passive: true })
|
||||
return () => {
|
||||
// Snapshot final position synchronously before detach.
|
||||
setWithLRU(scrollTopCache, scrollCacheKey, container.scrollTop)
|
||||
if (throttleTimer !== null) {
|
||||
clearTimeout(throttleTimer)
|
||||
}
|
||||
container.removeEventListener('scroll', onScroll)
|
||||
}
|
||||
}, [scrollCacheKey])
|
||||
|
||||
// Restore scroll position with RAF retry loop for async Tiptap content.
|
||||
useLayoutEffect(() => {
|
||||
const container = scrollContainerRef.current
|
||||
const targetScrollTop = scrollTopCache.get(scrollCacheKey)
|
||||
if (!container || targetScrollTop === undefined) {
|
||||
return
|
||||
}
|
||||
|
||||
let frameId = 0
|
||||
let attempts = 0
|
||||
|
||||
// Why: Tiptap renders asynchronously as it hydrates its ProseMirror document,
|
||||
// so scrollHeight may be undersized on the initial frame. Retry up to 30
|
||||
// frames (~500ms at 60fps) to accommodate content loading. This matches
|
||||
// CombinedDiffViewer's proven pattern for dynamic-height content restoration.
|
||||
const tryRestore = (): void => {
|
||||
const maxScrollTop = Math.max(0, container.scrollHeight - container.clientHeight)
|
||||
const nextScrollTop = Math.min(targetScrollTop, maxScrollTop)
|
||||
container.scrollTop = nextScrollTop
|
||||
|
||||
if (Math.abs(container.scrollTop - targetScrollTop) <= 1 || maxScrollTop >= targetScrollTop) {
|
||||
return
|
||||
}
|
||||
|
||||
attempts += 1
|
||||
if (attempts < 30) {
|
||||
frameId = window.requestAnimationFrame(tryRestore)
|
||||
}
|
||||
}
|
||||
|
||||
tryRestore()
|
||||
return () => window.cancelAnimationFrame(frameId)
|
||||
// Why: `editor` is included so the effect re-runs when the Tiptap editor
|
||||
// instance becomes available (non-null). With `immediatelyRender: false`,
|
||||
// editor is null on the first render, so the retry loop would start before
|
||||
// content is mounted and exhaust its 30 frames before Tiptap hydrates.
|
||||
}, [scrollCacheKey, editor])
|
||||
|
||||
// Why: the custom Image extension reads filePath from editor.storage to resolve
|
||||
// relative image src values to file:// URLs for display. After updating the
|
||||
// stored path we dispatch a no-op transaction so ProseMirror re-renders image
|
||||
|
|
@ -337,7 +402,9 @@ export default function RichMarkdownEditor({
|
|||
query={searchQuery}
|
||||
searchInputRef={searchInputRef}
|
||||
/>
|
||||
<EditorContent editor={editor} className="min-h-0 flex-1 overflow-auto" />
|
||||
<div ref={scrollContainerRef} className="min-h-0 flex-1 overflow-auto">
|
||||
<EditorContent editor={editor} />
|
||||
</div>
|
||||
{slashMenu && filteredSlashCommands.length > 0 ? (
|
||||
<div
|
||||
className="rich-markdown-slash-menu"
|
||||
|
|
@ -377,63 +444,3 @@ export default function RichMarkdownEditor({
|
|||
</div>
|
||||
)
|
||||
}
|
||||
|
||||
function syncSlashMenu(
|
||||
editor: Editor,
|
||||
root: HTMLDivElement | null,
|
||||
setSlashMenu: React.Dispatch<React.SetStateAction<SlashMenuState | null>>
|
||||
): void {
|
||||
if (!root || editor.view.composing || !editor.isEditable) {
|
||||
setSlashMenu(null)
|
||||
return
|
||||
}
|
||||
|
||||
const { state, view } = editor
|
||||
const { selection } = state
|
||||
if (!selection.empty) {
|
||||
setSlashMenu(null)
|
||||
return
|
||||
}
|
||||
|
||||
const { $from } = selection
|
||||
if (!$from.parent.isTextblock) {
|
||||
setSlashMenu(null)
|
||||
return
|
||||
}
|
||||
|
||||
const blockTextBeforeCursor = $from.parent.textBetween(0, $from.parentOffset, '\0', '\0')
|
||||
const slashMatch = blockTextBeforeCursor.match(/^\s*\/([a-z0-9-]*)$/i)
|
||||
if (!slashMatch) {
|
||||
setSlashMenu(null)
|
||||
return
|
||||
}
|
||||
|
||||
const slashOffset = blockTextBeforeCursor.lastIndexOf('/')
|
||||
const start = selection.from - ($from.parentOffset - slashOffset)
|
||||
const coords = view.coordsAtPos(selection.from)
|
||||
const rect = root.getBoundingClientRect()
|
||||
|
||||
setSlashMenu({
|
||||
query: slashMatch[1] ?? '',
|
||||
from: start,
|
||||
to: selection.from,
|
||||
left: coords.left - rect.left,
|
||||
top: coords.bottom - rect.top + 8
|
||||
})
|
||||
}
|
||||
|
||||
function runSlashCommand(
|
||||
editor: Editor,
|
||||
slashMenu: SlashMenuState,
|
||||
command: SlashCommand,
|
||||
onImageCommand?: () => void
|
||||
): void {
|
||||
editor.chain().focus().deleteRange({ from: slashMenu.from, to: slashMenu.to }).run()
|
||||
// Why: image insertion cannot rely on window.prompt() in Electron, so this
|
||||
// command is rerouted into the editor's local image picker flow.
|
||||
if (command.id === 'image' && onImageCommand) {
|
||||
onImageCommand()
|
||||
return
|
||||
}
|
||||
command.run(editor)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -2,6 +2,14 @@ import React from 'react'
|
|||
import type { Editor } from '@tiptap/react'
|
||||
import { Heading1, Heading2, Heading3, ImageIcon, List, ListOrdered, Quote } from 'lucide-react'
|
||||
|
||||
export type SlashMenuState = {
|
||||
query: string
|
||||
from: number
|
||||
to: number
|
||||
left: number
|
||||
top: number
|
||||
}
|
||||
|
||||
export type SlashCommandId =
|
||||
| 'text'
|
||||
| 'heading-1'
|
||||
|
|
@ -24,6 +32,27 @@ export type SlashCommand = {
|
|||
run: (editor: Editor) => void
|
||||
}
|
||||
|
||||
/**
|
||||
* Executes a slash command by first deleting the typed slash text, then
|
||||
* delegating to the command's run method. Image is special-cased because
|
||||
* window.prompt() is not supported in Electron's renderer process.
|
||||
*/
|
||||
export function runSlashCommand(
|
||||
editor: Editor,
|
||||
slashMenu: { from: number; to: number },
|
||||
command: SlashCommand,
|
||||
onImageCommand?: () => void
|
||||
): void {
|
||||
editor.chain().focus().deleteRange({ from: slashMenu.from, to: slashMenu.to }).run()
|
||||
// Why: image insertion cannot rely on window.prompt() in Electron, so this
|
||||
// command is rerouted into the editor's local image picker flow.
|
||||
if (command.id === 'image' && onImageCommand) {
|
||||
onImageCommand()
|
||||
return
|
||||
}
|
||||
command.run(editor)
|
||||
}
|
||||
|
||||
export const slashCommands: SlashCommand[] = [
|
||||
{
|
||||
id: 'text',
|
||||
|
|
@ -144,3 +173,51 @@ export const slashCommands: SlashCommand[] = [
|
|||
}
|
||||
}
|
||||
]
|
||||
|
||||
/**
|
||||
* Inspects the editor selection to decide whether the slash-command menu
|
||||
* should be open (and where to position it), or dismissed.
|
||||
*/
|
||||
export function syncSlashMenu(
|
||||
editor: Editor,
|
||||
root: HTMLDivElement | null,
|
||||
setSlashMenu: React.Dispatch<React.SetStateAction<SlashMenuState | null>>
|
||||
): void {
|
||||
if (!root || editor.view.composing || !editor.isEditable) {
|
||||
setSlashMenu(null)
|
||||
return
|
||||
}
|
||||
|
||||
const { state, view } = editor
|
||||
const { selection } = state
|
||||
if (!selection.empty) {
|
||||
setSlashMenu(null)
|
||||
return
|
||||
}
|
||||
|
||||
const { $from } = selection
|
||||
if (!$from.parent.isTextblock) {
|
||||
setSlashMenu(null)
|
||||
return
|
||||
}
|
||||
|
||||
const blockTextBeforeCursor = $from.parent.textBetween(0, $from.parentOffset, '\0', '\0')
|
||||
const slashMatch = blockTextBeforeCursor.match(/^\s*\/([a-z0-9-]*)$/i)
|
||||
if (!slashMatch) {
|
||||
setSlashMenu(null)
|
||||
return
|
||||
}
|
||||
|
||||
const slashOffset = blockTextBeforeCursor.lastIndexOf('/')
|
||||
const start = selection.from - ($from.parentOffset - slashOffset)
|
||||
const coords = view.coordsAtPos(selection.from)
|
||||
const rect = root.getBoundingClientRect()
|
||||
|
||||
setSlashMenu({
|
||||
query: slashMatch[1] ?? '',
|
||||
from: start,
|
||||
to: selection.from,
|
||||
left: coords.left - rect.left,
|
||||
top: coords.bottom - rect.top + 8
|
||||
})
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,112 @@
|
|||
import { describe, expect, it, beforeEach } from 'vitest'
|
||||
import { setWithLRU, scrollTopCache } from './scroll-cache'
|
||||
|
||||
beforeEach(() => {
|
||||
scrollTopCache.clear()
|
||||
})
|
||||
|
||||
describe('setWithLRU', () => {
|
||||
it('inserts a new entry into the map', () => {
|
||||
const map = new Map<string, number>()
|
||||
setWithLRU(map, 'a', 1)
|
||||
expect(map.get('a')).toBe(1)
|
||||
expect(map.size).toBe(1)
|
||||
})
|
||||
|
||||
it('updates an existing entry', () => {
|
||||
const map = new Map<string, number>()
|
||||
setWithLRU(map, 'a', 1)
|
||||
setWithLRU(map, 'a', 2)
|
||||
expect(map.get('a')).toBe(2)
|
||||
expect(map.size).toBe(1)
|
||||
})
|
||||
|
||||
it('evicts the oldest entry when exceeding the default limit', () => {
|
||||
const map = new Map<string, number>()
|
||||
for (let i = 0; i <= 20; i++) {
|
||||
setWithLRU(map, `key-${i}`, i)
|
||||
}
|
||||
// 21 inserts with default limit of 20 → oldest (key-0) evicted
|
||||
expect(map.size).toBe(20)
|
||||
expect(map.has('key-0')).toBe(false)
|
||||
expect(map.has('key-1')).toBe(true)
|
||||
expect(map.has('key-20')).toBe(true)
|
||||
})
|
||||
|
||||
it('evicts the oldest entry when exceeding a custom limit', () => {
|
||||
const map = new Map<string, number>()
|
||||
setWithLRU(map, 'a', 1, 3)
|
||||
setWithLRU(map, 'b', 2, 3)
|
||||
setWithLRU(map, 'c', 3, 3)
|
||||
setWithLRU(map, 'd', 4, 3)
|
||||
expect(map.size).toBe(3)
|
||||
expect(map.has('a')).toBe(false)
|
||||
expect(map.get('d')).toBe(4)
|
||||
})
|
||||
|
||||
it('refreshes insertion order when updating an existing key', () => {
|
||||
const map = new Map<string, number>()
|
||||
setWithLRU(map, 'a', 1, 3)
|
||||
setWithLRU(map, 'b', 2, 3)
|
||||
setWithLRU(map, 'c', 3, 3)
|
||||
// Touch 'a' to move it to the end
|
||||
setWithLRU(map, 'a', 10, 3)
|
||||
// Now 'b' is the oldest — inserting 'd' should evict 'b', not 'a'
|
||||
setWithLRU(map, 'd', 4, 3)
|
||||
expect(map.has('a')).toBe(true)
|
||||
expect(map.has('b')).toBe(false)
|
||||
expect(map.has('c')).toBe(true)
|
||||
expect(map.has('d')).toBe(true)
|
||||
expect(map.get('a')).toBe(10)
|
||||
})
|
||||
|
||||
it('does not evict when at exactly the limit', () => {
|
||||
const map = new Map<string, number>()
|
||||
setWithLRU(map, 'a', 1, 3)
|
||||
setWithLRU(map, 'b', 2, 3)
|
||||
setWithLRU(map, 'c', 3, 3)
|
||||
expect(map.size).toBe(3)
|
||||
expect(map.has('a')).toBe(true)
|
||||
})
|
||||
|
||||
it('works with a limit of 1', () => {
|
||||
const map = new Map<string, number>()
|
||||
setWithLRU(map, 'a', 1, 1)
|
||||
expect(map.size).toBe(1)
|
||||
setWithLRU(map, 'b', 2, 1)
|
||||
expect(map.size).toBe(1)
|
||||
expect(map.has('a')).toBe(false)
|
||||
expect(map.has('b')).toBe(true)
|
||||
})
|
||||
|
||||
it('evicts only one entry per insert even when far over limit', () => {
|
||||
const map = new Map<string, number>()
|
||||
// Pre-fill with 5 entries
|
||||
for (let i = 0; i < 5; i++) {
|
||||
map.set(`key-${i}`, i)
|
||||
}
|
||||
// Insert with a limit of 3 — only evicts one, leaving 5 entries
|
||||
// (LRU eviction is per-insert, not bulk)
|
||||
setWithLRU(map, 'new', 99, 3)
|
||||
expect(map.size).toBe(5)
|
||||
expect(map.has('key-0')).toBe(false)
|
||||
expect(map.has('new')).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
describe('scrollTopCache', () => {
|
||||
it('is an empty Map on import', () => {
|
||||
expect(scrollTopCache).toBeInstanceOf(Map)
|
||||
expect(scrollTopCache.size).toBe(0)
|
||||
})
|
||||
|
||||
it('works with setWithLRU for mode-scoped keys', () => {
|
||||
setWithLRU(scrollTopCache, '/path/to/file.ts', 100)
|
||||
setWithLRU(scrollTopCache, '/path/to/file.ts:preview', 200)
|
||||
setWithLRU(scrollTopCache, '/path/to/file.ts:rich', 300)
|
||||
expect(scrollTopCache.get('/path/to/file.ts')).toBe(100)
|
||||
expect(scrollTopCache.get('/path/to/file.ts:preview')).toBe(200)
|
||||
expect(scrollTopCache.get('/path/to/file.ts:rich')).toBe(300)
|
||||
expect(scrollTopCache.size).toBe(3)
|
||||
})
|
||||
})
|
||||
|
|
@ -0,0 +1,35 @@
|
|||
// Why: 20 entries covers a typical working set of open/recently-viewed files.
|
||||
// Eviction only means losing a scroll position (user sees top of file), not a
|
||||
// correctness bug, so a conservative cap is fine.
|
||||
const CACHE_MAX_ENTRIES = 20
|
||||
|
||||
// Why: Module-scoped Maps grow unboundedly as unique file keys accumulate.
|
||||
// Cap them with a simple LRU eviction: after each set, if the map exceeds
|
||||
// this limit, delete the oldest entry (Maps iterate in insertion order).
|
||||
// `maxEntries` is optional so consumers with their own Maps (like
|
||||
// CombinedDiffViewer) can use different limits.
|
||||
export function setWithLRU<K, V>(
|
||||
map: Map<K, V>,
|
||||
key: K,
|
||||
value: V,
|
||||
maxEntries: number = CACHE_MAX_ENTRIES
|
||||
): void {
|
||||
// Re-insert to refresh insertion order (move to end).
|
||||
map.delete(key)
|
||||
map.set(key, value)
|
||||
if (map.size > maxEntries) {
|
||||
// Why: Use the iterator's `.done` property rather than checking
|
||||
// `value !== undefined`, because K could legitimately be `undefined`
|
||||
// in a generic Map — the undefined check would skip valid evictions.
|
||||
const first = map.keys().next()
|
||||
if (!first.done) {
|
||||
map.delete(first.value)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Why: A single shared Map for scroll positions across all editor components.
|
||||
// Module-scoped so it survives component unmount/remount without triggering
|
||||
// React re-renders (unlike Zustand, which would broadcast state changes on
|
||||
// every scroll event even though no component renders from scroll position).
|
||||
export const scrollTopCache = new Map<string, number>()
|
||||
Loading…
Reference in New Issue