Conversation
Add a markdown-only TOC sidebar that keeps heading navigation consistent in both source and WYSIWYG modes. Persist TOC visibility, width, and heading depth so the panel survives editor mode switches and app restarts. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a right-side, resizable Table of Contents (TOC) for markdown editors, including parsing and filtering of headings, active-heading detection for BlockNote and CodeMirror, persistent TOC settings store, toolbar toggle, and UI integration with resizable panels. Changes
Sequence DiagramsequenceDiagram
actor User
participant Editor as Editor (CodeMirror/BlockNote)
participant VisibleRange as Visible Range Tracker
participant ActiveCalc as Active Heading Calculator
participant TocPanel as TocPanel
participant TOC as TableOfContents
User->>Editor: Scroll or click
Editor->>VisibleRange: Update visible range / emit
VisibleRange->>ActiveCalc: Provide range
ActiveCalc->>TocPanel: activeHeadingId
TocPanel->>TOC: render headings (active highlighted)
User->>TOC: Click heading item
TOC->>TocPanel: onHeadingClick(id)
TocPanel->>Editor: scrollToLine() or scrollToBlock()
Editor->>User: Jump to heading
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~75 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 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 |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (5)
src/renderer/hooks/use-blocknote.ts (1)
145-147: Simplify element lookup with attribute selectorThe current approach converts a NodeList to an array and uses
find. A direct attribute selector is more concise and efficient.♻️ Proposed simplification
requestAnimationFrame(() => { - const targetElement = Array.from( - editor.domElement?.querySelectorAll<HTMLElement>('[data-node-type="blockContainer"][data-id]') ?? [] - ).find((element) => element.dataset.id === blockId) + const targetElement = editor.domElement?.querySelector<HTMLElement>( + `[data-node-type="blockContainer"][data-id="${blockId}"]` + ) targetElement?.scrollIntoView({ block: 'center' }) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/hooks/use-blocknote.ts` around lines 145 - 147, The lookup for targetElement can be simplified: instead of converting the NodeList to an array and using find, use a direct attribute selector on editor.domElement to query the single element with the matching data-id; update the code that sets targetElement (currently using Array.from(...).find(...)) to call editor.domElement?.querySelector<HTMLElement> with a selector combining data-node-type="blockContainer" and data-id equal to the blockId value so it returns the element directly.src/renderer/components/editor/EditorToolbar.tsx (1)
25-59: Inconsistent button components:<Button>vs native<button>The TOC toggle (lines 25-38) uses the imported
<Button>component withvariant="ghost", while the view mode toggle (lines 40-59) uses a native<button>element with manually applied styles. This creates inconsistency in both code style and potentially visual appearance.Consider using the
<Button>component for both toggles to maintain consistency.♻️ Proposed fix to use Button component consistently
- <button + <Button + variant="ghost" + size="sm" onClick={onToggleViewMode} - className={cn( - 'flex items-center gap-1 px-2 py-0.5 text-xs rounded transition-colors', - 'text-muted-foreground hover:text-foreground hover:bg-secondary' - )} + className="h-6 gap-1 px-2 text-xs text-muted-foreground hover:text-foreground" title={viewMode === 'markdown' ? 'Switch to source mode' : 'Switch to WYSIWYG mode'} > {viewMode === 'markdown' ? ( <> <Code2 size={12} /> <span>Source</span> </> ) : ( <> <Eye size={12} /> <span>Preview</span> </> )} - </button> + </Button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/editor/EditorToolbar.tsx` around lines 25 - 59, Replace the native <button> used for the view-mode toggle with the shared Button component to keep UI and behavior consistent; specifically change the element rendering the view toggle (the block using onToggleViewMode and checking viewMode) to use the imported Button component (same as the TOC toggle) and move the existing className, title, onClick, and children into that Button so it preserves styles and accessibility (aria/state handled like isTocVisible), ensuring the component uses the same props pattern and variant (e.g., variant="ghost" or matching styling).src/renderer/components/editor/EditorPanel.tsx (1)
18-18:useTocSettings()return value is unusedThe hook is called but its return value isn't used. If this is intentional for triggering settings persistence side effects, consider adding a comment to clarify. Otherwise, if a return value is expected, it should be destructured.
- useTocSettings() + // Initialize TOC settings persistence + useTocSettings()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/editor/EditorPanel.tsx` at line 18, The call to useTocSettings() in EditorPanel.tsx currently ignores its return value; either explicitly use/destructure the returned value(s) from useTocSettings (e.g., const { ... } = useTocSettings()) if the hook exposes state or handlers, or, if the hook is only invoked for side effects, add a short inline comment above the call (e.g., // intentionally invoked for side effects: persists TOC settings) to make the intent explicit and avoid linter confusion.src/renderer/components/editor/TableOfContents.test.tsx (1)
7-25: The settings test never exercises the settings callback.
DropdownMenuContentis always rendered here, and the mockedDropdownMenuRadioGroupnever forwardsonValueChange, so the last test only proves the labels exist. It won’t catch a brokenonMaxHeadingLevelChangewire-up inTableOfContents.tsx.Also applies to: 81-95
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/editor/TableOfContents.test.tsx` around lines 7 - 25, Update the mock so the test actually exercises the settings callback: change DropdownMenuRadioGroup to accept an onValueChange prop and render its children while injecting an onSelect handler into DropdownMenuRadioItem instances that calls onValueChange with the item's value; ensure DropdownMenuRadioItem still accepts value and onSelect but when clicked it invokes the injected onSelect so a click triggers the group's onValueChange. This will allow TableOfContents.tsx's onMaxHeadingLevelChange wiring to be exercised by the tests.src/renderer/components/editor/MarkdownEditor.tsx (1)
11-16: Remove or explicitly usefilePathhere.
src/renderer/components/editor/EditorPanel.tsx:73-80still passesfilePath, but this component drops it and only keys its reset path offcontent. That leaves the prop contract misleading and makes same-content file switches easy to mishandle unless a parent remount is doing the reset already.Also applies to: 39-43, 91-103
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/editor/MarkdownEditor.tsx` around lines 11 - 16, MarkdownEditorProps declares filePath but MarkdownEditor ignores it, causing misleading prop contract and incorrect resets when switching between files with identical content; either remove filePath from the prop interface and from EditorPanel or use it inside MarkdownEditor to drive resets/remounts. Fix by updating the MarkdownEditor component (and MarkdownEditorProps) to consume filePath—e.g., include filePath in the component's key or in the useEffect dependency that initializes/reset local state or the editor value—so changes to filePath force the editor to reset even if content is identical; also update EditorPanel usage to match (keep passing filePath if you choose to use it, or stop passing it if you remove the prop).
🤖 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/renderer/components/editor/CodeEditor.tsx`:
- Around line 35-37: The TOC is rendering with default settings before persisted
preferences load; update the component to wait for settings hydration before
reading/using useTocIsVisible, useTocWidth, and setTocWidth. Pull the hydration
flag from the toc settings store or via useTocSettings (e.g., state.hydrated)
and short-circuit the TOC rendering/layout (return null or a neutral
placeholder) until hydrated is true so the editor doesn't paint with
DEFAULT_TOC_SETTINGS and then snap; ensure all places that read
isTocVisible/tocWidth (the uses around useTocIsVisible, useTocWidth,
useTocSettingsStore and the blocks noted at 106-124 and 131-159) are guarded by
that hydration check.
In `@src/renderer/components/editor/MarkdownEditor.tsx`:
- Around line 71-89: The TOC percent sizing uses a hardcoded 10–40% range
instead of the actual pixel limits (TOC_MIN_WIDTH / TOC_MAX_WIDTH), causing
inconsistent clamping; update getTocPanelSizePercent (and where
tocPanelDefaultSize is derived) to compute minPercent = (TOC_MIN_WIDTH /
panelWidth)*100 and maxPercent = (TOC_MAX_WIDTH / panelWidth)*100 and then clamp
widthRatio*100 between these computed percent bounds (use
layoutRef.current?.clientWidth or layoutWidth as before). Also adjust
handleTocResize to clamp the incoming size percent to the same
minPercent/maxPercent before converting to pixels and calling setTocWidth so the
stored pixel width always respects TOC_MIN_WIDTH/TOC_MAX_WIDTH. Ensure
references to ResizablePanel sizing and setTocWidth remain consistent with these
pixel-derived percent limits.
In `@src/renderer/hooks/use-active-heading.ts`:
- Around line 75-99: The observer is storing BlockNote block ids
(element.dataset.id) directly, but the TOC uses heading.id, so create a mapping
from blockId->tocId (e.g., const blockIdToTocId = new Map(headings.map(h =>
[(h.blockId ?? h.id), h.id])) and use it when handling entries in the
IntersectionObserver: translate the dataset id to the TOC id before calling
visibleHeadings.set(...) and before visibleHeadings.delete(...), and ensure
updateActiveHeading continues to call setActiveHeadingId with the translated TOC
id; apply the same translation logic in both the enter (entry.isIntersecting)
and exit branches (lines around 75-99 and 114-120).
In `@src/renderer/hooks/use-blocknote.ts`:
- Around line 139-153: The scrollToBlock handler can call
editor.setTextCursorPosition with a non-existent blockId and throw; update
scrollToBlock to first locate the target element (using
editor.domElement?.querySelectorAll('[data-node-type="blockContainer"][data-id]')
and matching dataset.id) or otherwise verify the node exists before calling
editor.setTextCursorPosition and editor.focus, and wrap the
setTextCursorPosition/scrollIntoView sequence in a try-catch to swallow or log
errors so a missing/deleted heading in the TOC does not cause an uncaught
exception.
In `@src/renderer/hooks/use-toc-settings.ts`:
- Around line 15-27: The code currently treats any failed read the same and
flips setLoaded(true), which lets the subscriber write defaults back to
persistence; change the load logic that calls
persistenceApi.read<TocSettings>(TOC_SETTINGS_KEY) so you only apply
DEFAULT_TOC_SETTINGS and call setLoaded(true) when the read succeeded OR when it
failed with a KEY_NOT_FOUND sentinel (i.e., no value exists). If result.success
is false for any other error, do not set settings or mark loaded (or mark a
separate loadFailed flag) so the subscriber (the effect that watches settings at
the subscriber lines) does not persist defaults back to storage; update that
subscriber to check the same flag (loaded && !loadFailed or only persist when
result.success || KEY_NOT_FOUND) before writing. Ensure you reference
persistenceApi.read, TOC_SETTINGS_KEY, DEFAULT_TOC_SETTINGS, setSettings and
setLoaded (and the subscriber effect) when making these changes.
In `@src/renderer/stores/toc-settings-store.ts`:
- Around line 15-33: The persisted TOC settings must be sanitized before
storing: update clampHeadingLevel and clampWidth to first coerce the input to a
finite number (e.g., using Number(value) and Number.isFinite) and fall back to
DEFAULT_TOC_SETTINGS.maxHeadingLevel / .width when the input is
undefined/NaN/Infinity; also modify useTocSettingsStore's setSettings to
explicitly coerce isVisible to a boolean and pass the sanitized/clamped values
(via clampHeadingLevel and clampWidth) instead of trusting raw settings so
malformed persisted values cannot produce NaN or Infinity that breaks CodeEditor
or heading filtering.
---
Nitpick comments:
In `@src/renderer/components/editor/EditorPanel.tsx`:
- Line 18: The call to useTocSettings() in EditorPanel.tsx currently ignores its
return value; either explicitly use/destructure the returned value(s) from
useTocSettings (e.g., const { ... } = useTocSettings()) if the hook exposes
state or handlers, or, if the hook is only invoked for side effects, add a short
inline comment above the call (e.g., // intentionally invoked for side effects:
persists TOC settings) to make the intent explicit and avoid linter confusion.
In `@src/renderer/components/editor/EditorToolbar.tsx`:
- Around line 25-59: Replace the native <button> used for the view-mode toggle
with the shared Button component to keep UI and behavior consistent;
specifically change the element rendering the view toggle (the block using
onToggleViewMode and checking viewMode) to use the imported Button component
(same as the TOC toggle) and move the existing className, title, onClick, and
children into that Button so it preserves styles and accessibility (aria/state
handled like isTocVisible), ensuring the component uses the same props pattern
and variant (e.g., variant="ghost" or matching styling).
In `@src/renderer/components/editor/MarkdownEditor.tsx`:
- Around line 11-16: MarkdownEditorProps declares filePath but MarkdownEditor
ignores it, causing misleading prop contract and incorrect resets when switching
between files with identical content; either remove filePath from the prop
interface and from EditorPanel or use it inside MarkdownEditor to drive
resets/remounts. Fix by updating the MarkdownEditor component (and
MarkdownEditorProps) to consume filePath—e.g., include filePath in the
component's key or in the useEffect dependency that initializes/reset local
state or the editor value—so changes to filePath force the editor to reset even
if content is identical; also update EditorPanel usage to match (keep passing
filePath if you choose to use it, or stop passing it if you remove the prop).
In `@src/renderer/components/editor/TableOfContents.test.tsx`:
- Around line 7-25: Update the mock so the test actually exercises the settings
callback: change DropdownMenuRadioGroup to accept an onValueChange prop and
render its children while injecting an onSelect handler into
DropdownMenuRadioItem instances that calls onValueChange with the item's value;
ensure DropdownMenuRadioItem still accepts value and onSelect but when clicked
it invokes the injected onSelect so a click triggers the group's onValueChange.
This will allow TableOfContents.tsx's onMaxHeadingLevelChange wiring to be
exercised by the tests.
In `@src/renderer/hooks/use-blocknote.ts`:
- Around line 145-147: The lookup for targetElement can be simplified: instead
of converting the NodeList to an array and using find, use a direct attribute
selector on editor.domElement to query the single element with the matching
data-id; update the code that sets targetElement (currently using
Array.from(...).find(...)) to call editor.domElement?.querySelector<HTMLElement>
with a selector combining data-node-type="blockContainer" and data-id equal to
the blockId value so it returns the element directly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 71cccc62-573c-4506-b775-ededd9f68b36
📒 Files selected for processing (18)
src/renderer/components/editor/CodeEditor.tsxsrc/renderer/components/editor/EditorPanel.tsxsrc/renderer/components/editor/EditorToolbar.tsxsrc/renderer/components/editor/MarkdownEditor.tsxsrc/renderer/components/editor/TableOfContents.test.tsxsrc/renderer/components/editor/TableOfContents.tsxsrc/renderer/components/editor/TocPanel.tsxsrc/renderer/components/ui/resizable.tsxsrc/renderer/hooks/use-active-heading.test.tssrc/renderer/hooks/use-active-heading.tssrc/renderer/hooks/use-blocknote.tssrc/renderer/hooks/use-codemirror.tssrc/renderer/hooks/use-toc-headings.test.tssrc/renderer/hooks/use-toc-headings.tssrc/renderer/hooks/use-toc-settings.tssrc/renderer/stores/toc-settings-store.test.tssrc/renderer/stores/toc-settings-store.tssrc/renderer/types/settings.ts
Harden TOC settings hydration, sizing, and persistence handling so the editor doesn't render with unsanitized defaults or persist bad state. Align BlockNote active heading and navigation behavior with the TOC, and extend tests to cover the review fixes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/renderer/components/editor/CodeEditor.tsx (1)
20-27:⚠️ Potential issue | 🟠 MajorKeep TOC percent bounds within the panel group's valid 0-100 range.
This helper can still produce
maxPercent > 100on narrow windows. That propagates intotocPanelDefaultSizeand the layout sync at Lines 138-145, so the group can be asked to render impossible sizes. Clamp the computed percentages before using them fordefaultSize,minSize,maxSize, orsetLayout.Possible hardening
function getTocPercentBounds(panelWidth: number): { minPercent: number; maxPercent: number } { - const minPercent = (TOC_MIN_WIDTH / panelWidth) * 100 - const maxPercent = (TOC_MAX_WIDTH / panelWidth) * 100 + const safePanelWidth = Math.max(panelWidth, 1) + const minPercent = Math.min(100, (TOC_MIN_WIDTH / safePanelWidth) * 100) + const maxPercent = Math.min(100, (TOC_MAX_WIDTH / safePanelWidth) * 100) return { minPercent, maxPercent: Math.max(minPercent, maxPercent) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/editor/CodeEditor.tsx` around lines 20 - 27, getTocPercentBounds can return values >100 on narrow panels; clamp the computed minPercent and maxPercent to the valid 0–100 range and ensure maxPercent >= minPercent before returning. Update the function getTocPercentBounds to cap minPercent at least 0 and maxPercent at most 100 (and compute maxPercent = Math.max(minPercent, cappedMaxPercent)). Also ensure the callers that use these values for tocPanelDefaultSize and when calling setLayout / setGroupLayout use the clamped results so defaultSize, minSize and maxSize are always within 0–100.src/renderer/components/editor/MarkdownEditor.tsx (1)
18-25:⚠️ Potential issue | 🟠 MajorClamp the computed TOC percent bounds to the panel group's real range.
This helper can still return percentages above 100 on narrow layouts. Once that happens,
tocPanelDefaultSizeand thesetLayout([100 - tocSize, tocSize])sync effect can push invalid sizes into the panel group and collapse the editor pane. Please cap the derived bounds before they flow intodefaultSize/setLayout.Possible hardening
function getTocPercentBounds(panelWidth: number): { minPercent: number; maxPercent: number } { - const minPercent = (TOC_MIN_WIDTH / panelWidth) * 100 - const maxPercent = (TOC_MAX_WIDTH / panelWidth) * 100 + const safePanelWidth = Math.max(panelWidth, 1) + const minPercent = Math.min(100, (TOC_MIN_WIDTH / safePanelWidth) * 100) + const maxPercent = Math.min(100, (TOC_MAX_WIDTH / safePanelWidth) * 100) return { minPercent, maxPercent: Math.max(minPercent, maxPercent) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/editor/MarkdownEditor.tsx` around lines 18 - 25, The getTocPercentBounds function can return values >100 on narrow layouts; clamp the computed minPercent and maxPercent into the valid [0, 100] range before returning so they never flow invalid sizes into defaultSize/setLayout. Update getTocPercentBounds to compute minPercent and maxPercent from TOC_MIN_WIDTH/TOC_MAX_WIDTH and panelWidth, then apply clamping (e.g. minPercent = Math.max(0, Math.min(100, minPercent)); maxPercent = Math.max(minPercent, Math.min(100, maxPercent))) so maxPercent is never below minPercent and neither exceeds 100.
🤖 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/renderer/hooks/use-active-heading.ts`:
- Line 124: The forEach callback currently uses an implicit return
(elements.forEach((element) => observer.observe(element))) which triggers the
useIterableCallbackReturn lint rule; change the arrow function to a block body
so it doesn't implicitly return — e.g., replace the callback with (element) => {
observer.observe(element); } — ensuring the use of elements.forEach and
observer.observe remains the same.
---
Duplicate comments:
In `@src/renderer/components/editor/CodeEditor.tsx`:
- Around line 20-27: getTocPercentBounds can return values >100 on narrow
panels; clamp the computed minPercent and maxPercent to the valid 0–100 range
and ensure maxPercent >= minPercent before returning. Update the function
getTocPercentBounds to cap minPercent at least 0 and maxPercent at most 100 (and
compute maxPercent = Math.max(minPercent, cappedMaxPercent)). Also ensure the
callers that use these values for tocPanelDefaultSize and when calling setLayout
/ setGroupLayout use the clamped results so defaultSize, minSize and maxSize are
always within 0–100.
In `@src/renderer/components/editor/MarkdownEditor.tsx`:
- Around line 18-25: The getTocPercentBounds function can return values >100 on
narrow layouts; clamp the computed minPercent and maxPercent into the valid [0,
100] range before returning so they never flow invalid sizes into
defaultSize/setLayout. Update getTocPercentBounds to compute minPercent and
maxPercent from TOC_MIN_WIDTH/TOC_MAX_WIDTH and panelWidth, then apply clamping
(e.g. minPercent = Math.max(0, Math.min(100, minPercent)); maxPercent =
Math.max(minPercent, Math.min(100, maxPercent))) so maxPercent is never below
minPercent and neither exceeds 100.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 258b36bf-78cf-4288-8672-5867e42b8bd3
📒 Files selected for processing (11)
src/renderer/components/editor/CodeEditor.tsxsrc/renderer/components/editor/EditorPanel.tsxsrc/renderer/components/editor/EditorToolbar.tsxsrc/renderer/components/editor/MarkdownEditor.tsxsrc/renderer/components/editor/TableOfContents.test.tsxsrc/renderer/hooks/use-active-heading.test.tssrc/renderer/hooks/use-active-heading.tssrc/renderer/hooks/use-blocknote.tssrc/renderer/hooks/use-toc-settings.tssrc/renderer/stores/toc-settings-store.test.tssrc/renderer/stores/toc-settings-store.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- src/renderer/hooks/use-blocknote.ts
- src/renderer/stores/toc-settings-store.test.ts
- src/renderer/components/editor/TableOfContents.test.tsx
- src/renderer/components/editor/EditorPanel.tsx
- src/renderer/hooks/use-active-heading.test.ts
Adjust the BlockNote heading observer loop to avoid the iterable callback return lint rule while keeping the TOC heading tracking behavior unchanged. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Closes #61
Summary
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests