fix: autofocus and preserve scroll/cursor when toggling source mode#135
Merged
fix: autofocus and preserve scroll/cursor when toggling source mode#135
Conversation
When switching between formatted and markdown source views, the editor now automatically focuses the target view and preserves the approximate scroll position and cursor location using block-index mapping. Closes #117 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughCaptures top-visible and cursor markdown block indices when toggling source mode, persists them in a transition ref, and restores focus, caret, and container/textarea scroll positions synchronously when entering or exiting the markdown textarea/source view. Also disables textarea word-wrap. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Editor as EditorComponent
participant PM as ProseMirror
participant TA as Textarea
participant Ref as transitionRef
User->>Editor: click toggleSourceMode()
alt Entering source mode
Editor->>PM: posAtCoords(top of container) -> map to topBlockIndex
Editor->>PM: current selection -> cursorBlockIndex
Editor->>Ref: store {topBlockIndex, cursorBlockIndex, mdSnapshot}
Editor->>TA: set sourceContent, enable sourceMode
TA->>TA: useLayoutEffect -> focus textarea, set caret to cursorBlockIndex, scroll to topBlockIndex
else Exiting source mode
Editor->>TA: read scrollTop, selectionStart -> getMarkdownBlockOffsets() -> top/cursor indices
Editor->>Ref: store {topBlockIndex, cursorBlockIndex, mdSnapshot}
Editor->>PM: parse markdown -> update document, disable sourceMode
PM->>PM: requestAnimationFrame -> focus editor at blockIndexToPos(cursorBlockIndex)
PM->>Editor: scroll container to align topBlockIndex
PM->>Ref: clear transitionRef
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/editor/Editor.tsx`:
- Around line 1903-1919: The current scroll restore calculates topBlockIndex
from textarea.scrollTop / lineHeight which breaks when lines wrap; replace that
logic in the block using textarea, sourceContent and getMarkdownBlockOffsets by
measuring the visual character offset via a hidden mirror element (create a div
that copies textarea styles, width and padding, set its textContent to
sourceContent up to incremental char positions or use Range measurement to map
scrollTop to the nearest char index) and use that char index to compute
topBlockIndex against blockOffsets; alternatively you can disable soft-wrap on
the textarea (wrap="off") if wrapping is unacceptable. Apply the same
mirror-based measurement or wrap="off" change to the other identical region
around the code referenced (the block at 1977-1983).
- Around line 450-478: getMarkdownBlockOffsets mis-detects fence boundaries
because inCodeFence is toggled before examining the current line and it only
starts blocks after blank lines, so headings or fence openings are missed;
update the logic in getMarkdownBlockOffsets to (1) detect a fence opener when
not inCodeFence and treat that current line as a block start (push pos) then set
inCodeFence = true, (2) when inCodeFence only toggle inCodeFence off after
checking the line so the closing fence isn’t treated as the block start, and (3)
treat ATX headings (lines starting with '#') and other top-level markers as
block starts regardless of prevBlank; adjust the checks around prevBlank,
inCodeFence, offsets, and pos in the loop to implement these rules.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 98d826d9-e196-4c6b-a4c3-8ba0c5d87d6d
📒 Files selected for processing (1)
src/components/editor/Editor.tsx
- Fix getMarkdownBlockOffsets: separate open/close fence handling so closing fences aren't mis-detected as block starts; recognize ATX headings as block starts regardless of preceding blank lines - Add wrap="off" to source textarea so scrollTop/lineHeight maps exactly to logical lines (eliminates soft-wrap mismatch) - Extract blockIndexToPos helper, deduplicate getMarkdownBlockOffsets calls in exit path Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/editor/Editor.tsx`:
- Around line 2002-2022: The requestAnimationFrame scheduled when exiting source
mode (the callback that calls requestAnimationFrame and uses
editor.commands.focus, blockIndexToPos, editor.view.coordsAtPos and container)
is not cancelled which can run after unmount or after sourceMode changes; store
the RAF id when you call requestAnimationFrame and cancel it in the
component/effect cleanup (use cancelAnimationFrame(id)) and guard the callback
to ensure editor and container are still mounted/non-null before calling
editor.commands.focus or editor.view.coordsAtPos.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2e00128c-38db-4183-8f14-ca85b0b26cf4
📒 Files selected for processing (1)
src/components/editor/Editor.tsx
Cancel the requestAnimationFrame scheduled when exiting source mode if the effect re-runs or the component unmounts. Guard the callback against a disconnected editor view. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/editor/Editor.tsx (1)
1976-1981: Inconsistent early return may cause React warnings.Line 1981 returns
undefined(implicit void) while lines 1969-1971 and 2029-2031 return cleanup functions. While functionally safe here (sincerafIdhasn't been set yet), React's rules of hooks recommend effects return either nothing or a cleanup function consistently for readability and to avoid potential future refactoring bugs.♻️ Suggested fix for consistent return
if (sourceMode) { // Just entered source mode — focus textarea and scroll to anchor block const textarea = container?.querySelector( "textarea", ) as HTMLTextAreaElement | null; - if (!textarea) return; + if (!textarea) return () => {};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/editor/Editor.tsx` around lines 1976 - 1981, The effect branch that handles sourceMode currently does an early `if (!textarea) return;` which returns undefined while other branches return cleanup functions; change that early return to return a no-op cleanup function so the effect consistently returns a cleanup function in all branches (e.g., replace the `if (!textarea) return;` with a check that returns `() => {}`), keeping references to sourceMode, textarea and the existing rafId/cleanup logic intact so later refactors won't introduce hook-warning inconsistencies.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/editor/Editor.tsx`:
- Around line 1976-1981: The effect branch that handles sourceMode currently
does an early `if (!textarea) return;` which returns undefined while other
branches return cleanup functions; change that early return to return a no-op
cleanup function so the effect consistently returns a cleanup function in all
branches (e.g., replace the `if (!textarea) return;` with a check that returns
`() => {}`), keeping references to sourceMode, textarea and the existing
rafId/cleanup logic intact so later refactors won't introduce hook-warning
inconsistencies.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8990a324-286f-4884-bf52-d89104864b28
📒 Files selected for processing (1)
src/components/editor/Editor.tsx
Clamp blockIndexToPos lower bound for empty docs, return consistent cleanup function from useLayoutEffect early exit, and fall back to end of markdown (instead of start) when cursor is past the last block. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implementation
getMarkdownBlockOffsets()helper maps between TipTap top-level nodes and markdown blocks (blank-line separated, code-fence aware)useLayoutEffecthandles textarea focus/scroll immediately after DOM commitrequestAnimationFramedefers TipTap focus/scroll to afterEditorContentremounts its ProseMirror viewCloses #117
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
New Features