Feature/mind map generation#131
Conversation
|
Warning Review limit reached
Next review available in: 1 minute Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds a ChangesMind Map View Mode
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/renderer/src/components/ReaderToolbar.tsx (1)
181-204: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winExpose pressed state on the view-mode toggles.
These buttons are stateful toggles, but assistive tech only gets a changing label/title. Add
aria-pressedfor the raw-text and mind-map buttons so the active view is announced without relying on color. As per path instructions, "Interactive controls need semantic elements, visible focus, and keyboard access" and "Keep components typed, accessible, keyboard-friendly, and resilient to missing preload APIs."Suggested change
{onToggleRawText && ( <button type="button" onClick={onToggleRawText} + aria-pressed={viewMode === MARKDOWN_TOGGLE.RAW} className={`rounded-md p-2 transition-colors hover:bg-accent-bg ${viewMode === MARKDOWN_TOGGLE.RAW ? 'text-accent bg-accent-bg' : 'text-text-muted hover:text-text-base'}`} aria-label={viewMode === MARKDOWN_TOGGLE.RAW ? ICON_TITLE.SHOW_RENDERED_TEXT : ICON_TITLE.SHOW_RAW_TEXT} title={viewMode === MARKDOWN_TOGGLE.RAW ? ICON_TITLE.SHOW_RENDERED_TEXT : ICON_TITLE.SHOW_RAW_TEXT} > <Icons.Code size={17} /> </button> )} @@ {onToggleMindMap && ( <button type="button" onClick={onToggleMindMap} + aria-pressed={viewMode === MARKDOWN_TOGGLE.MINDMAP} className={`rounded-md p-2 transition-colors hover:bg-accent-bg ${viewMode === MARKDOWN_TOGGLE.MINDMAP ? 'text-accent bg-accent-bg' : 'text-text-muted hover:text-text-base'}`} aria-label={viewMode === MARKDOWN_TOGGLE.MINDMAP ? ICON_TITLE.SHOW_RENDERED_TEXT : ICON_TITLE.SHOW_MIND_MAP} title={viewMode === MARKDOWN_TOGGLE.MINDMAP ? ICON_TITLE.SHOW_RENDERED_TEXT : ICON_TITLE.SHOW_MIND_MAP} > <Icons.MindMap size={17} /> </button> )}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/renderer/src/components/ReaderToolbar.tsx` around lines 181 - 204, The raw-text and mind-map toggle buttons in ReaderToolbar are stateful controls but only expose their active state through label/title changes. Update the button props in ReaderToolbar to add aria-pressed based on whether viewMode matches MARKDOWN_TOGGLE.RAW or MARKDOWN_TOGGLE.MINDMAP, so assistive tech can announce the selected view without relying on color alone.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/renderer/__tests__/components/MarkmapViewer.test.tsx`:
- Around line 6-33: The tests in MarkmapViewer should verify the actual render
path, not just the SVG wrapper. Update the MarkmapViewer test cases to assert
that Transformer.transform(), Markmap.create(), setData(), and fit() are called
when markdown is provided, using the mocked Transformer and Markmap symbols to
locate the behavior. For the markdown={undefined} case, assert the early return
by checking those mocks are not called at all.
In `@apps/renderer/src/App.tsx`:
- Around line 244-255: Mind-map mode is still wired into the existing
search/highlight flow even though the visible content is rendered by
MarkmapViewer instead of activeTab.html. Update App.tsx so the search UI and the
scroll/highlight effect are disabled or bypassed when viewMode is
MARKDOWN_TOGGLE.MINDMAP, or otherwise make them aware of the mind-map rendering.
Keep the behavior consistent in the main view logic around useSearch, scroll,
and the activeTab/mode switch so document search never reports navigable matches
when the mind-map view cannot support them.
In `@apps/renderer/src/components/MarkmapViewer.tsx`:
- Around line 9-12: The early return in MarkmapViewer’s useEffect leaves the
previous SVG rendered when markdown becomes empty, so clear the existing
map/instance instead of exiting early. Update the effect that uses svgRef and
the markmap instance to handle the empty markdown case by resetting or clearing
the SVG and showing an empty state, while preserving the normal render path for
valid markdown and the existing loading/error/stale-response handling.
- Around line 15-32: `MarkmapViewer` currently passes user-controlled markdown
directly into `Transformer.transform()`, unlike the safer reader rendering path.
Update `renderMarkmap` to sanitize the markdown before creating the mindmap
tree, stripping raw HTML and dangerous URLs such as javascript: links, or reuse
the same markdown sanitization used elsewhere before calling `Transformer` and
`Markmap.create`.
In `@apps/renderer/src/index.css`:
- Line 221: The comment in the stylesheet violates comment-whitespace-inside
because it lacks a space after the opening delimiter. Update the existing CSS
comment near the markmap SVG dark mode styles so it follows the standard spaced
comment form, and keep the wording aligned with the related styles in index.css.
In `@apps/renderer/src/types/component-types.ts`:
- Around line 212-214: The MarkmapViewerProps contract currently makes markdown
optional even though MarkmapViewer still bails out on falsy markdown, which can
hide missing content as a blank pane. Either keep markdown required in
MarkmapViewerProps, or update the MarkmapViewer component to explicitly handle
loading, empty, error, stale-response, and rejected-promise states before
allowing markdown to be optional.
---
Outside diff comments:
In `@apps/renderer/src/components/ReaderToolbar.tsx`:
- Around line 181-204: The raw-text and mind-map toggle buttons in ReaderToolbar
are stateful controls but only expose their active state through label/title
changes. Update the button props in ReaderToolbar to add aria-pressed based on
whether viewMode matches MARKDOWN_TOGGLE.RAW or MARKDOWN_TOGGLE.MINDMAP, so
assistive tech can announce the selected view without relying on color alone.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5cdec31f-3544-4c44-b93d-20626f2513df
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (11)
apps/renderer/__tests__/components/MarkmapViewer.test.tsxapps/renderer/__tests__/hooks/useViewMode.test.tsapps/renderer/package.jsonapps/renderer/src/App.tsxapps/renderer/src/components/MarkmapViewer.tsxapps/renderer/src/components/ReaderToolbar.tsxapps/renderer/src/hooks/useViewMode.tsapps/renderer/src/index.cssapps/renderer/src/types/component-types.tsapps/renderer/src/utils/constants/icon-contants.tsxapps/renderer/src/utils/constants/markdown-constants.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: build
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Write unit tests for all new features to maintain code quality, adding test cases alongside component source code and ensuring all tests pass via
pnpm vitest
Files:
apps/renderer/__tests__/hooks/useViewMode.test.tsapps/renderer/__tests__/components/MarkmapViewer.test.tsx
apps/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (README.md)
Use React for the renderer UI layer.
Files:
apps/renderer/__tests__/hooks/useViewMode.test.tsapps/renderer/src/utils/constants/icon-contants.tsxapps/renderer/__tests__/components/MarkmapViewer.test.tsxapps/renderer/src/components/MarkmapViewer.tsxapps/renderer/src/utils/constants/markdown-constants.tsapps/renderer/src/hooks/useViewMode.tsapps/renderer/src/components/ReaderToolbar.tsxapps/renderer/src/App.tsxapps/renderer/src/types/component-types.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (README.md)
Use TypeScript for application code.
Files:
apps/renderer/__tests__/hooks/useViewMode.test.tsapps/renderer/src/utils/constants/icon-contants.tsxapps/renderer/__tests__/components/MarkmapViewer.test.tsxapps/renderer/src/components/MarkmapViewer.tsxapps/renderer/src/utils/constants/markdown-constants.tsapps/renderer/src/hooks/useViewMode.tsapps/renderer/src/components/ReaderToolbar.tsxapps/renderer/src/App.tsxapps/renderer/src/types/component-types.ts
**
⚙️ CodeRabbit configuration file
**: # Markdown ReaderA native desktop applicaion for reading Markdown files, built entirely on the JavaScript/TypeScript ecosystem using Electron as the desktop runtime.
markdown-reader is to Markdown what Adobe Acrobat Reader is to PDF:
a dedicated, native, first-class desktop viewer for .md files.Table of Contents
Why Markdown Reader ?
The Problem
Markdown is the most widely used plain-text writing format in the world.
Developers, writers, and teams produce millions of .md files daily.
Yet there is no dedicated desktop reader for Markdown that:
Missing Capability Current Workaround Pain Level Opens .md files natively on double-click No default handler — opens as raw text High Renders beautifully like a document VS Code preview pane feels like a dev tool Medium Has a sidebar TOC like a PDF reader Manual heading scanning Medium Supports all Markdown features GitHub renders some, browsers render none High Works fully offline Must open a browser manually High Feels like a reader, not an editor Every tool adds edit affordances Medium VS Code is an editor. GitHub is a web interface. Obsidian is a note-taking vault.
None of them are just a reader — a clean, dedicated app for opening and
reading Markdown files.The Solution
markdown-reader is a dedicated native desktop Markdown reader:
- Double-click any .md file and it opens in markdown-reader
- Registered as the system default application for .md files on install
- Beautiful readable typography — not a developer tool aesthetic
- Full M...
Files:
apps/renderer/__tests__/hooks/useViewMode.test.tsapps/renderer/src/utils/constants/icon-contants.tsxapps/renderer/__tests__/components/MarkmapViewer.test.tsxapps/renderer/src/components/MarkmapViewer.tsxapps/renderer/package.jsonapps/renderer/src/index.cssapps/renderer/src/utils/constants/markdown-constants.tsapps/renderer/src/hooks/useViewMode.tsapps/renderer/src/components/ReaderToolbar.tsxapps/renderer/src/App.tsxapps/renderer/src/types/component-types.ts
**/*.{test,spec}.{ts,tsx}
⚙️ CodeRabbit configuration file
**/*.{test,spec}.{ts,tsx}: Review tests.
- Cover success and failure paths, especially IPC, filesystem, markdown rendering, search, settings, tabs, and exports.
- Use isolated temp directories for disk tests and clean them up.
- Mock Electron/preload APIs explicitly.
- Prefer Testing Library user-event and getByRole for UI tests.
Files:
apps/renderer/__tests__/hooks/useViewMode.test.tsapps/renderer/__tests__/components/MarkmapViewer.test.tsx
apps/renderer/src/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/renderer/src/**/*.{ts,tsx}: Review as React renderer code.
- Keep components typed, accessible, keyboard-friendly, and resilient to missing preload APIs.
- Effects must have correct dependencies and cleanup.
- Handle loading, empty, error, stale-response, and rejected-promise states.
- Do not import Node-only modules into renderer code.
- Avoid unnecessary derived state, unsafe globals, and broad any types.
Files:
apps/renderer/src/utils/constants/icon-contants.tsxapps/renderer/src/components/MarkmapViewer.tsxapps/renderer/src/utils/constants/markdown-constants.tsapps/renderer/src/hooks/useViewMode.tsapps/renderer/src/components/ReaderToolbar.tsxapps/renderer/src/App.tsxapps/renderer/src/types/component-types.ts
apps/renderer/src/**/{renderer,markdown,utils}/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/renderer/src/**/{renderer,markdown,utils}/**/*.{ts,tsx}: Review markdown rendering carefully.
- Sanitize raw HTML, links, images, Mermaid, KaTeX, anchors, and exported content.
- Block script execution, javascript: URLs, unsafe inline handlers, and unsafe local file references.
- Heading IDs and TOC entries must be stable and collision-safe.
- Mermaid/KaTeX/code highlighting failures should not break the whole document.
- Add tests for unsafe HTML, malformed markdown, links, images, code blocks, Mermaid, and KaTeX when changed.
Files:
apps/renderer/src/utils/constants/icon-contants.tsxapps/renderer/src/utils/constants/markdown-constants.ts
apps/renderer/src/**/*.{css,tsx}
⚙️ CodeRabbit configuration file
apps/renderer/src/**/*.{css,tsx}: Review UI, theme, and accessibility.
- Interactive controls need semantic elements, visible focus, and keyboard access.
- Theme changes must preserve readable contrast in light and dark modes.
- Markdown prose must remain readable for tables, code, blockquotes, links, lists, and images.
- Prefer existing tokens/classes over ad hoc inline styling.
Files:
apps/renderer/src/utils/constants/icon-contants.tsxapps/renderer/src/components/MarkmapViewer.tsxapps/renderer/src/index.cssapps/renderer/src/components/ReaderToolbar.tsxapps/renderer/src/App.tsx
**/package.json
⚙️ CodeRabbit configuration file
**/package.json: Review scripts and dependencies.
- Scripts for lint, typecheck, test, coverage, build, and dist must fail on errors.
- Dependencies should live in the package that imports them.
- Runtime imports must not be placed only in devDependencies.
- Electron, Vite, React, TypeScript, Tailwind, and testing upgrades need compatibility attention.
Files:
apps/renderer/package.json
🪛 Stylelint (17.13.0)
apps/renderer/src/index.css
[error] 221-221: Expected whitespace after "/*" (comment-whitespace-inside)
(comment-whitespace-inside)
🔇 Additional comments (6)
apps/renderer/src/utils/constants/markdown-constants.ts (1)
6-16: LGTM!apps/renderer/package.json (1)
19-27: LGTM!apps/renderer/src/types/component-types.ts (1)
186-189: LGTM!apps/renderer/src/utils/constants/icon-contants.tsx (1)
247-258: LGTM!apps/renderer/src/hooks/useViewMode.ts (1)
3-23: LGTM!apps/renderer/__tests__/hooks/useViewMode.test.ts (1)
1-36: LGTM!
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/renderer/src/components/MarkmapViewer.tsx (1)
44-46: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winShow a real error state when Markmap rendering fails.
The
catchonly logs, so failed imports/transforms leave the viewer blank or stale with no feedback. Store the failure in state and render an error fallback alongside the existing empty-state branch. As per path instructions, "Handle loading, empty, error, stale-response, and rejected-promise states."Also applies to: 56-62
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/renderer/src/components/MarkmapViewer.tsx` around lines 44 - 46, The MarkmapViewer render flow currently only logs failures in the catch block, so the UI stays blank or stale when imports/transforms fail. Update the MarkmapViewer component to capture the error in state from the import/transform path and render a visible error fallback in the main conditional branch alongside the existing empty-state handling. Make sure the loading, empty, error, stale-response, and rejected-promise cases are all handled in the same render logic so failures are surfaced instead of only calling console.error.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/renderer/__tests__/components/MarkmapViewer.test.tsx`:
- Around line 33-51: The MarkmapViewer tests only cover the initial render and
empty markdown case; add coverage for teardown and rejection paths using the
MarkmapViewer component. Extend the existing test file with a rerender scenario
that updates markdown from a value to undefined and asserts the Markmap instance
destroy() path is called, then add a failure-path test that mocks the Markmap
load/transform flow to reject and verifies the viewer renders its error state
instead of crashing.
In `@apps/renderer/src/App.tsx`:
- Line 236: `App.tsx` is passing a no-op handler to `ReaderToolbar` in mind-map
mode, which leaves the search control visible but nonfunctional; update the
`onOpenSearch` prop so it is `undefined` when `viewMode ===
MARKDOWN_TOGGLE.MINDMAP`, or wire an explicit disabled state in `ReaderToolbar`
instead. Use the existing `viewMode`, `MARKDOWN_TOGGLE.MINDMAP`, `openSearch`,
and `ReaderToolbar` props to keep the component typed and keyboard-accessible.
In `@apps/renderer/src/components/MarkmapViewer.tsx`:
- Around line 32-34: The MarkmapViewer transform path is still allowing
dangerous link URLs through because sanitizing the raw markdown string alone
does not stop Transformer.transform() from parsing unsafe hrefs. Update
MarkmapViewer’s markdown handling around Transformer.transform() and setData()
to strip or normalize unsafe link protocols before the transform runs, or reuse
the same markdown rendering/sanitization flow used by the normal reader view so
javascript: and similar URLs cannot reach the mindmap output.
- Around line 11-19: Move the Markmap instance teardown out of the
markdown-empty branch in MarkmapViewer and into the useEffect cleanup. In the
effect that creates/updates the Markmap instance, make the returned cleanup
responsible for calling markmapRef.current.destroy() and resetting
markmapRef.current so listeners and observers are always released, including
when the component unmounts or markdown clears. Keep the existing svgElement
handling, but ensure the cleanup path in MarkmapViewer is the single place that
disposes the instance.
---
Outside diff comments:
In `@apps/renderer/src/components/MarkmapViewer.tsx`:
- Around line 44-46: The MarkmapViewer render flow currently only logs failures
in the catch block, so the UI stays blank or stale when imports/transforms fail.
Update the MarkmapViewer component to capture the error in state from the
import/transform path and render a visible error fallback in the main
conditional branch alongside the existing empty-state handling. Make sure the
loading, empty, error, stale-response, and rejected-promise cases are all
handled in the same render logic so failures are surfaced instead of only
calling console.error.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6ed8a949-3a9a-40c4-a56c-209c7dcd7ed2
📒 Files selected for processing (4)
apps/renderer/__tests__/components/MarkmapViewer.test.tsxapps/renderer/src/App.tsxapps/renderer/src/components/MarkmapViewer.tsxapps/renderer/src/components/ReaderToolbar.tsx
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: build
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Write unit tests for all new features to maintain code quality, adding test cases alongside component source code and ensuring all tests pass via
pnpm vitest
Files:
apps/renderer/__tests__/components/MarkmapViewer.test.tsx
apps/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (README.md)
Use React for the renderer UI layer.
Files:
apps/renderer/__tests__/components/MarkmapViewer.test.tsxapps/renderer/src/components/ReaderToolbar.tsxapps/renderer/src/App.tsxapps/renderer/src/components/MarkmapViewer.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (README.md)
Use TypeScript for application code.
Files:
apps/renderer/__tests__/components/MarkmapViewer.test.tsxapps/renderer/src/components/ReaderToolbar.tsxapps/renderer/src/App.tsxapps/renderer/src/components/MarkmapViewer.tsx
**
⚙️ CodeRabbit configuration file
**: # Markdown ReaderA native desktop applicaion for reading Markdown files, built entirely on the JavaScript/TypeScript ecosystem using Electron as the desktop runtime.
markdown-reader is to Markdown what Adobe Acrobat Reader is to PDF:
a dedicated, native, first-class desktop viewer for .md files.Table of Contents
Why Markdown Reader ?
The Problem
Markdown is the most widely used plain-text writing format in the world.
Developers, writers, and teams produce millions of .md files daily.
Yet there is no dedicated desktop reader for Markdown that:
Missing Capability Current Workaround Pain Level Opens .md files natively on double-click No default handler — opens as raw text High Renders beautifully like a document VS Code preview pane feels like a dev tool Medium Has a sidebar TOC like a PDF reader Manual heading scanning Medium Supports all Markdown features GitHub renders some, browsers render none High Works fully offline Must open a browser manually High Feels like a reader, not an editor Every tool adds edit affordances Medium VS Code is an editor. GitHub is a web interface. Obsidian is a note-taking vault.
None of them are just a reader — a clean, dedicated app for opening and
reading Markdown files.The Solution
markdown-reader is a dedicated native desktop Markdown reader:
- Double-click any .md file and it opens in markdown-reader
- Registered as the system default application for .md files on install
- Beautiful readable typography — not a developer tool aesthetic
- Full M...
Files:
apps/renderer/__tests__/components/MarkmapViewer.test.tsxapps/renderer/src/components/ReaderToolbar.tsxapps/renderer/src/App.tsxapps/renderer/src/components/MarkmapViewer.tsx
**/*.{test,spec}.{ts,tsx}
⚙️ CodeRabbit configuration file
**/*.{test,spec}.{ts,tsx}: Review tests.
- Cover success and failure paths, especially IPC, filesystem, markdown rendering, search, settings, tabs, and exports.
- Use isolated temp directories for disk tests and clean them up.
- Mock Electron/preload APIs explicitly.
- Prefer Testing Library user-event and getByRole for UI tests.
Files:
apps/renderer/__tests__/components/MarkmapViewer.test.tsx
apps/renderer/src/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/renderer/src/**/*.{ts,tsx}: Review as React renderer code.
- Keep components typed, accessible, keyboard-friendly, and resilient to missing preload APIs.
- Effects must have correct dependencies and cleanup.
- Handle loading, empty, error, stale-response, and rejected-promise states.
- Do not import Node-only modules into renderer code.
- Avoid unnecessary derived state, unsafe globals, and broad any types.
Files:
apps/renderer/src/components/ReaderToolbar.tsxapps/renderer/src/App.tsxapps/renderer/src/components/MarkmapViewer.tsx
apps/renderer/src/**/*.{css,tsx}
⚙️ CodeRabbit configuration file
apps/renderer/src/**/*.{css,tsx}: Review UI, theme, and accessibility.
- Interactive controls need semantic elements, visible focus, and keyboard access.
- Theme changes must preserve readable contrast in light and dark modes.
- Markdown prose must remain readable for tables, code, blockquotes, links, lists, and images.
- Prefer existing tokens/classes over ad hoc inline styling.
Files:
apps/renderer/src/components/ReaderToolbar.tsxapps/renderer/src/App.tsxapps/renderer/src/components/MarkmapViewer.tsx
🔇 Additional comments (1)
apps/renderer/src/components/ReaderToolbar.tsx (1)
24-24: LGTM!Also applies to: 188-202
mind-murtaza
left a comment
mind-murtaza
left a comment
There was a problem hiding this comment.
The new feature is well-structured and properly secures against XSS, but contains a memory leak during unmounts and causes unnecessary event re-bindings due to inline function references.
Findings by File
📄 apps/renderer/src/components/MarkmapViewer.tsx — TypeScript (React) · 3 issues
[MAJOR] Dead code & memory leak when cleaning up markmapRef · L10–L25
- What's wrong: The
useEffectreturns early if!svgElementbefore it can ever reach the cleanup logic for!markdown. Since an empty state unmounts the<svg>,svgRef.currentis null, meaningmarkmapRef.current.destroy()is entirely unreachable. Furthermore, there is no cleanup when the component unmounts. - Why it matters: Markmap hooks into DOM events (e.g., D3 zoom/pan listeners) and relies on
destroy()to clean them up. This unreachable cleanup causes a memory leak every time the user switches away from the mindmap view. - Category: Performance (Resource Leaks)
- Fix: Separate the unmount cleanup from the render effect, and check for
!markdownbefore checking the ref.// 1. Clean up on component unmount useEffect(() => { return () => { if (markmapRef.current) { markmapRef.current.destroy(); markmapRef.current = null; } }; }, []); // 2. Render effect useEffect(() => { if (!markdown) { if (markmapRef.current) { markmapRef.current.destroy(); markmapRef.current = null; } return; } const svgElement = svgRef.current; if (!svgElement) return; let isMounted = true; const renderMarkmap = async () => { /* ... */ }; void renderMarkmap(); return () => { isMounted = false; }; }, [markdown]);
[MINOR] Duplicate CSS using inline <style> block · L71–L85
- What's wrong: The component injects a
<style>block to style the mind map nodes, overriding the.markmap-wrapper svg textselector. - Why it matters: Injecting raw
<style>tags from React creates duplicate rules in the DOM and breaks the separation of concerns. The exact same rules were actually added toindex.css(using.markmap-svg) but were left unused because the SVG missing the corresponding class name. - Category: Architecture, Modularization & Separation of Concerns / DRY
- Fix: Add the class
markmap-svgto the SVG and completely remove the<style>block from the component.<svg ref={svgRef} className="w-full h-full flex-1 markmap-svg" style={{ minHeight: '600px' }} />
[NIT] Missing type safety for markmapRef · L4
- What's wrong:
const markmapRef = useRef<any>(null);usesany. - Why it matters: Defeats TypeScript's type-checking and can lead to runtime errors if the Markmap API shape changes in the future.
- Category: Coding Standards & Style
- Fix: Use a type-only import for Markmap so it doesn't bundle synchronously.
import type { Markmap } from 'markmap-view'; // ... const markmapRef = useRef<Markmap | null>(null);
📄 apps/renderer/src/App.tsx — TypeScript (React) · 1 issue
[MINOR] Inline arrow functions cause effect thrashing · L95 & L119 & L176
- What's wrong: Defining inline arrow functions inside the configuration objects, e.g.,
onSearchDocument: viewMode === MARKDOWN_TOGGLE.MINDMAP ? () => {} : openSearch. - Why it matters:
() => {}creates a new function reference every timeApprenders. IfuseMenuEventsoruseShortcutsdepend on these references, they will unnecessarily unbind and rebind all keyboard/menu listeners on every render. - Category: Performance
- Fix: Move the condition inside a memoized handler or use a stable no-op reference outside the component.
const handleOpenSearch = useCallback(() => { if (viewMode !== MARKDOWN_TOGGLE.MINDMAP) { openSearch(); } }, [viewMode, openSearch]); // Then pass handleOpenSearch directly: useMenuEvents({ // ... onSearchDocument: handleOpenSearch, });
📄 apps/renderer/src/index.css — CSS · 1 issue
[MINOR] Dead CSS rules · L221–L229
- What's wrong:
.markmap-svgand related rules were added, but never actually used inMarkmapViewer.tsx. - Why it matters: Dead code bloats the stylesheet and causes confusion.
- Category: Design Principles (YAGNI)
- Fix: Apply
.markmap-svgto the<svg>inMarkmapViewer.tsx(as noted above), and verify the rules are properly structured, or delete them if you prefer the inline approach (though the stylesheet is preferred).
📄 apps/renderer/tests/hooks/useViewMode.test.ts — TypeScript (Vitest) · 1 issue
[NIT] Typo in test description · L88
- What's wrong: Test reads
'should toggle between mindmap and redered content'. - Why it matters: Tests serve as living documentation; typos reduce polish.
- Category: Tooling & Repo Hygiene
- Fix: Fix the typo.
it('should toggle between mindmap and rendered content', () => {
Repo-level Findings
- [MINOR] Network Error Handling: The dynamic imports for Markmap (
await import('markmap-lib')) fail silently with just aconsole.errorif the chunk fails to load over the network. Consider wrapping the component in an Error Boundary or setting an internalerrorstate so the user isn't stuck with an infinitely loading or empty screen.
What's Done Well
- The UI properly disables searching (
openSearch) when in mind-map mode, preventing inconsistent states. - Sanitizing the markdown via
DOMPurifybefore passing it toTransformeris an excellent security practice against XSS inside mind maps! - The use of dynamic imports for
markmap-libandmarkmap-viewkeeps the initial bundle size small until the user actually requests the mind-map view. Great performance optimization.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/renderer/src/components/MarkmapViewer.tsx (1)
90-98: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd an accessible label to the mind-map SVG.
The
<svg>has norole/aria-label, so screen-reader users get no indication of what this region represents. As per path instructions, "Interactive controls need semantic elements, visible focus, and keyboard access."♿ Proposed fix
<svg ref={svgRef} className="w-full h-full flex-1 markmap-svg" style={{ minHeight: '600px' }} + role="img" + aria-label="Mind map of document content" />🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/renderer/src/components/MarkmapViewer.tsx` around lines 90 - 98, The MarkmapViewer SVG is missing accessible semantics, so update the <svg> in MarkmapViewer to expose its purpose to assistive tech by adding an appropriate role and an aria-label (or equivalent accessible name) describing the mind map region. Keep the change localized to the SVG rendered by MarkmapViewer, and ensure the label is specific enough for screen readers to announce what the visualization represents.Source: Path instructions
♻️ Duplicate comments (2)
apps/renderer/src/components/MarkmapViewer.tsx (1)
47-49: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftUnsafe link protocols still reach the mindmap SVG.
DOMPurify.sanitize(markdown)sanitizes the raw markdown text, not the HTML thatTransformer.transform()later derives from parsed markdown links/images. A link like[x](javascript:alert(1))passes through sanitization untouched as plain text and is only turned into a realhrefafterwards inside markmap'sforeignObjectrendering — this is unresolved from a prior review round on this same code.Sanitize/normalize the transformed output (or strip dangerous URL protocols from parsed links) before calling
setData(), rather than sanitizing the markdown string alone.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/renderer/src/components/MarkmapViewer.tsx` around lines 47 - 49, The MarkmapViewer transformation still allows unsafe link protocols because DOMPurify.sanitize(markdown) is applied before Transformer.transform() creates real link hrefs. Update MarkmapViewer’s markdown pipeline so the transformed output from Transformer.transform is sanitized/normalized before setData(), or explicitly strip dangerous URL protocols from parsed links/images after transformation. Keep the fix centered in MarkmapViewer around the Transformer and setData flow.apps/renderer/src/App.tsx (1)
238-238: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winStill passing a no-op search handler to
ReaderToolbar.In mind-map mode this passes
arrow(() => {}) asonOpenSearch, soReaderToolbarstill renders the search button (visible whenever the prop is truthy) but it does nothing. Passundefinedinstead so the button doesn't render, or add an explicit disabled state. As per path instructions, "Keep components typed, accessible, keyboard-friendly."🔧 Proposed fix
- onOpenSearch={viewMode === MARKDOWN_TOGGLE.MINDMAP ? arrow : openSearch} + onOpenSearch={viewMode === MARKDOWN_TOGGLE.MINDMAP ? undefined : openSearch}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/renderer/src/App.tsx` at line 238, The `App` component is still passing a no-op handler (`arrow`) to `ReaderToolbar` in mind-map mode, which leaves the search button visible but inert. Update the `onOpenSearch` prop logic in `App` so it passes `undefined` when `viewMode === MARKDOWN_TOGGLE.MINDMAP`, or add an explicit disabled state in `ReaderToolbar` if that behavior is required. Keep the `ReaderToolbar` prop typing aligned so the search control only renders when a real handler is provided.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/renderer/src/App.tsx`:
- Line 41: Rename the `arrow` no-op helper in `App.tsx` to a clearer
intent-revealing name like `noop`, and update all three call sites that
reference it so the disabled handler usage is obvious. Keep the helper
definition and its usages aligned by using the same identifier throughout the
component.
---
Outside diff comments:
In `@apps/renderer/src/components/MarkmapViewer.tsx`:
- Around line 90-98: The MarkmapViewer SVG is missing accessible semantics, so
update the <svg> in MarkmapViewer to expose its purpose to assistive tech by
adding an appropriate role and an aria-label (or equivalent accessible name)
describing the mind map region. Keep the change localized to the SVG rendered by
MarkmapViewer, and ensure the label is specific enough for screen readers to
announce what the visualization represents.
---
Duplicate comments:
In `@apps/renderer/src/App.tsx`:
- Line 238: The `App` component is still passing a no-op handler (`arrow`) to
`ReaderToolbar` in mind-map mode, which leaves the search button visible but
inert. Update the `onOpenSearch` prop logic in `App` so it passes `undefined`
when `viewMode === MARKDOWN_TOGGLE.MINDMAP`, or add an explicit disabled state
in `ReaderToolbar` if that behavior is required. Keep the `ReaderToolbar` prop
typing aligned so the search control only renders when a real handler is
provided.
In `@apps/renderer/src/components/MarkmapViewer.tsx`:
- Around line 47-49: The MarkmapViewer transformation still allows unsafe link
protocols because DOMPurify.sanitize(markdown) is applied before
Transformer.transform() creates real link hrefs. Update MarkmapViewer’s markdown
pipeline so the transformed output from Transformer.transform is
sanitized/normalized before setData(), or explicitly strip dangerous URL
protocols from parsed links/images after transformation. Keep the fix centered
in MarkmapViewer around the Transformer and setData flow.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2e5d799e-1b4b-47fd-9ce1-9d6637e27cae
📒 Files selected for processing (3)
apps/renderer/__tests__/hooks/useViewMode.test.tsapps/renderer/src/App.tsxapps/renderer/src/components/MarkmapViewer.tsx
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: build
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Write unit tests for all new features to maintain code quality, adding test cases alongside component source code and ensuring all tests pass via
pnpm vitest
Files:
apps/renderer/__tests__/hooks/useViewMode.test.ts
apps/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (README.md)
Use React for the renderer UI layer.
Files:
apps/renderer/__tests__/hooks/useViewMode.test.tsapps/renderer/src/App.tsxapps/renderer/src/components/MarkmapViewer.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (README.md)
Use TypeScript for application code.
Files:
apps/renderer/__tests__/hooks/useViewMode.test.tsapps/renderer/src/App.tsxapps/renderer/src/components/MarkmapViewer.tsx
**
⚙️ CodeRabbit configuration file
**: # Markdown ReaderA native desktop applicaion for reading Markdown files, built entirely on the JavaScript/TypeScript ecosystem using Electron as the desktop runtime.
markdown-reader is to Markdown what Adobe Acrobat Reader is to PDF:
a dedicated, native, first-class desktop viewer for .md files.Table of Contents
Why Markdown Reader ?
The Problem
Markdown is the most widely used plain-text writing format in the world.
Developers, writers, and teams produce millions of .md files daily.
Yet there is no dedicated desktop reader for Markdown that:
Missing Capability Current Workaround Pain Level Opens .md files natively on double-click No default handler — opens as raw text High Renders beautifully like a document VS Code preview pane feels like a dev tool Medium Has a sidebar TOC like a PDF reader Manual heading scanning Medium Supports all Markdown features GitHub renders some, browsers render none High Works fully offline Must open a browser manually High Feels like a reader, not an editor Every tool adds edit affordances Medium VS Code is an editor. GitHub is a web interface. Obsidian is a note-taking vault.
None of them are just a reader — a clean, dedicated app for opening and
reading Markdown files.The Solution
markdown-reader is a dedicated native desktop Markdown reader:
- Double-click any .md file and it opens in markdown-reader
- Registered as the system default application for .md files on install
- Beautiful readable typography — not a developer tool aesthetic
- Full M...
Files:
apps/renderer/__tests__/hooks/useViewMode.test.tsapps/renderer/src/App.tsxapps/renderer/src/components/MarkmapViewer.tsx
**/*.{test,spec}.{ts,tsx}
⚙️ CodeRabbit configuration file
**/*.{test,spec}.{ts,tsx}: Review tests.
- Cover success and failure paths, especially IPC, filesystem, markdown rendering, search, settings, tabs, and exports.
- Use isolated temp directories for disk tests and clean them up.
- Mock Electron/preload APIs explicitly.
- Prefer Testing Library user-event and getByRole for UI tests.
Files:
apps/renderer/__tests__/hooks/useViewMode.test.ts
apps/renderer/src/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/renderer/src/**/*.{ts,tsx}: Review as React renderer code.
- Keep components typed, accessible, keyboard-friendly, and resilient to missing preload APIs.
- Effects must have correct dependencies and cleanup.
- Handle loading, empty, error, stale-response, and rejected-promise states.
- Do not import Node-only modules into renderer code.
- Avoid unnecessary derived state, unsafe globals, and broad any types.
Files:
apps/renderer/src/App.tsxapps/renderer/src/components/MarkmapViewer.tsx
apps/renderer/src/**/*.{css,tsx}
⚙️ CodeRabbit configuration file
apps/renderer/src/**/*.{css,tsx}: Review UI, theme, and accessibility.
- Interactive controls need semantic elements, visible focus, and keyboard access.
- Theme changes must preserve readable contrast in light and dark modes.
- Markdown prose must remain readable for tables, code, blockquotes, links, lists, and images.
- Prefer existing tokens/classes over ad hoc inline styling.
Files:
apps/renderer/src/App.tsxapps/renderer/src/components/MarkmapViewer.tsx
🔇 Additional comments (1)
apps/renderer/__tests__/hooks/useViewMode.test.ts (1)
24-35: LGTM!
2 participants
Description
This PR uses Mark map library to generate map from raw markdown content.
Type of Change
Related Issue
Closes #126
Changes Made
Screenshots (if applicable)
Checklist
Electron main/preload
Renderer UI
ReaderToolbar(alongside Raw text), driven by the newonToggleMindMapprop andmindmapview mode.App.tsxto render one of: raw text (RAW), rendered HTML (default), or the mind map (MINDMAP), and to suppress search UI/actions while in mind-map mode.ICON_TITLE.Markdown rendering
MarkmapViewerto convert provided markdown into a Markmap-rendered SVG by dynamically importingmarkmap-lib,markmap-view, anddompurify.DOMPurify, transforms it viaTransformer.transform, then updates the viewer usingsetData(root)andfit().foreignObjectcontent).Shared packages
Tests
MarkmapViewer(mockingmarkmap-lib/markmap-view) to verify SVG rendering/pipeline calls and the empty-state behavior.useViewModeto verify toggling betweenrendered,raw, and the newmindmapmode.Tooling/CI
Docs
Packaging
markmap-libandmarkmap-viewtoapps/renderer/package.jsondependencies.Breaking changes: None