-
Notifications
You must be signed in to change notification settings - Fork 680
feat: Add markdown rendering to file browser #278
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat: Add markdown rendering to file browser #278
Conversation
Claude Code 2.x stores OAuth account info in ~/.claude/.claude.json under the oauthAccount key, rather than the legacy location at ~/.claude/.credentials.json. This change checks the new location first, then falls back to the legacy location for backwards compatibility.
Add ability to reposition the Quick Settings sidebar handle by dragging it vertically on both desktop and mobile devices. The handle combines toggle and drag functionality in a single compact button. Key features: - Drag handle vertically to reposition on screen - Click to toggle sidebar open/closed - Smart gesture detection (5px threshold distinguishes drag from click) - Visual feedback with blue grip icon during drag - Position persists in localStorage across sessions - Constrained to 10-90% of viewport height - Full touch support with proper scroll prevention on mobile - Responsive positioning (top-based on desktop, bottom-based on mobile)
- Extract markdown components from ChatInterface to shared utility - Add three viewing modes for markdown files: Edit, Preview, and Split - Implement side-by-side editor and preview in split mode - Add toggle buttons in header to switch between modes - Save user preferences to localStorage - Support full GitHub Flavored Markdown with LaTeX math - Include comprehensive test markdown file - Bump version to 1.14.0
- Replace broken placeholder.com URLs with Lorem Picsum service - Add multiple image examples to demonstrate markdown image rendering
WalkthroughAdds Markdown rendering and preview/split modes in the code editor, a centralized Markdown utility with copyable code blocks and math support, a draggable, persistent Quick Settings handle, a new Claude credentials lookup order with legacy fallback, and bumps package version. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/components/QuickSettingsPanel.jsx (1)
156-179: Consider cleanup on unmount for body styles.If the component unmounts while dragging, the body styles (position: fixed, overflow: hidden, etc.) won't be reset. Consider adding a cleanup effect.
🔎 Suggested cleanup effect
+ // Cleanup body styles on unmount + useEffect(() => { + return () => { + document.body.style.cursor = ''; + document.body.style.userSelect = ''; + document.body.style.overflow = ''; + document.body.style.position = ''; + document.body.style.width = ''; + }; + }, []); + // Set up global event listeners for drag useEffect(() => {src/components/CodeEditor.jsx (1)
716-759: Consider extracting shared CodeMirror configuration.The CodeMirror configuration in split mode (lines 716-759) and edit mode (lines 771-814) are nearly identical. Consider extracting the shared configuration to reduce duplication.
🔎 Suggested approach
// Extract shared extensions configuration const editorExtensions = useMemo(() => [ ...getLanguageExtension(file.name), ...editorToolbarPanel, ...(file.diffInfo && showDiff && file.diffInfo.old_string !== undefined ? [ unifiedMergeView({ original: file.diffInfo.old_string, mergeControls: false, highlightChanges: true, syntaxHighlightDeletions: false, gutter: true }), ...minimapExtension, ...scrollToFirstChunkExtension ] : []), ...(wordWrap ? [EditorView.lineWrapping] : []) ], [file.name, file.diffInfo, showDiff, minimapExtension, scrollToFirstChunkExtension, wordWrap, editorToolbarPanel]); const editorBasicSetup = useMemo(() => ({ lineNumbers: showLineNumbers, foldGutter: true, dropCursor: false, allowMultipleSelections: false, indentOnInput: true, bracketMatching: true, closeBrackets: true, autocompletion: true, highlightSelectionMatches: true, searchKeymap: true, }), [showLineNumbers]);Then use these in both CodeMirror instances.
Also applies to: 771-814
src/utils/markdownUtils.jsx (1)
184-204: Update ChatInterface.jsx to import and use the shared Markdown component from markdownUtils.jsxChatInterface.jsx (lines 96-111) contains a duplicate Markdown component that should be removed. The new shared component in markdownUtils.jsx provides additional functionality via a
proseprop that automatically handles prose styling, simplifying the multiple usages throughout ChatInterface.jsx where prose classes are currently passed manually in className.Consider refactoring all Markdown usages in ChatInterface.jsx to import and use the centralized version, which will eliminate duplication and improve maintainability.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
TEST_MARKDOWN.mdpackage.jsonserver/routes/cli-auth.jssrc/components/CodeEditor.jsxsrc/components/QuickSettingsPanel.jsxsrc/utils/markdownUtils.jsx
🧰 Additional context used
🧬 Code graph analysis (3)
src/components/CodeEditor.jsx (2)
src/utils/markdownUtils.jsx (3)
Markdown(184-204)Markdown(184-204)content(185-185)src/components/ChatInterface.jsx (2)
Markdown(96-112)content(97-97)
src/components/QuickSettingsPanel.jsx (1)
src/App.jsx (1)
isMobile(53-53)
server/routes/cli-auth.js (1)
server/index.js (3)
path(1327-1327)os(1329-1329)fs(1328-1328)
🔇 Additional comments (10)
package.json (1)
3-3: LGTM!Version bump to 1.14.0 is appropriate for the new markdown rendering feature.
server/routes/cli-auth.js (2)
106-110: LGTM!Adding the
methodfield to the legacy auth path provides consistency with the new 2.x path and aids debugging.
79-94: Claude Code 2.x authentication detection structure is sound, but theoauthAccount.emailAddresspath cannot be verified from the codebase.The fallback logic properly prioritizes the new config location and gracefully falls back to legacy credentials. The empty catch block is appropriate for this fallback behavior. However, the
oauthAccount.emailAddressfield path has no other references in the codebase to confirm it matches the actual Claude Code 2.x configuration structure. Cross-check this field path against the actual Claude Code 2.x config format to ensure correctness.src/components/QuickSettingsPanel.jsx (1)
194-226: LGTM!The combined drag handle and toggle button implementation is well-designed with proper gesture detection, visual feedback, and accessibility attributes. The drag threshold prevents accidental drags on click.
TEST_MARKDOWN.md (1)
1-75: LGTM!Comprehensive test file that covers all major markdown features. Good use of Lorem Picsum for placeholder images instead of broken URLs.
src/components/CodeEditor.jsx (2)
607-647: LGTM!The mode toggle buttons are well-implemented with proper styling, responsive behavior (hidden labels on mobile), and accessible title attributes.
42-60: Hook ordering is correct; code works safely as intended.The
isMarkdownFilememoized value is available when thepreviewModeinitializer runs—both hooks execute synchronously in order. The conditional rendering (isMarkdownFile && previewMode === '...') safely handles cases where the file changes after initialization by short-circuiting to edit mode when conditions don't match, so no inconsistency is visible to the user. The implementation preserves the user's preview mode preference across file switches, which is intentional.src/utils/markdownUtils.jsx (3)
11-19: LGTM!The
normalizeInlineCodeFencesfunction has proper input validation and the regex correctly targets single-line triple-backtick code that should be inline.
119-123: LGTM!The link component properly includes
target="_blank"withrel="noopener noreferrer"for security.
184-204: LGTM!The
Markdownwrapper component is well-designed with proper memoization of plugins and support for prose styling via the typography plugin.
| const [handlePosition, setHandlePosition] = useState(() => { | ||
| const saved = localStorage.getItem('quickSettingsHandlePosition'); | ||
| if (saved) { | ||
| const parsed = JSON.parse(saved); | ||
| return parsed.y; | ||
| } | ||
| return 50; // Default to 50% (middle of screen) | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrap JSON.parse in try-catch to handle corrupted localStorage.
If the stored value is malformed JSON, this will throw and could break component initialization.
🔎 Proposed fix
const [handlePosition, setHandlePosition] = useState(() => {
const saved = localStorage.getItem('quickSettingsHandlePosition');
if (saved) {
- const parsed = JSON.parse(saved);
- return parsed.y;
+ try {
+ const parsed = JSON.parse(saved);
+ return parsed.y ?? 50;
+ } catch {
+ return 50;
+ }
}
return 50; // Default to 50% (middle of screen)
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const [handlePosition, setHandlePosition] = useState(() => { | |
| const saved = localStorage.getItem('quickSettingsHandlePosition'); | |
| if (saved) { | |
| const parsed = JSON.parse(saved); | |
| return parsed.y; | |
| } | |
| return 50; // Default to 50% (middle of screen) | |
| }); | |
| const [handlePosition, setHandlePosition] = useState(() => { | |
| const saved = localStorage.getItem('quickSettingsHandlePosition'); | |
| if (saved) { | |
| try { | |
| const parsed = JSON.parse(saved); | |
| return parsed.y ?? 50; | |
| } catch { | |
| return 50; | |
| } | |
| } | |
| return 50; // Default to 50% (middle of screen) | |
| }); |
🤖 Prompt for AI Agents
In src/components/QuickSettingsPanel.jsx around lines 43 to 50, the JSON.parse
on the saved localStorage value can throw if the data is corrupted; wrap the
parse in a try-catch, return the parsed.y when successful, and fall back to the
default (50) on any error or if parsed.y is missing; also consider removing the
invalid localStorage key inside the catch to avoid repeated failures.
- Extract code component to named CodeBlock function to properly use hooks - Follows React Rules of Hooks by using useState in a proper component - Prevents potential issues with component identity changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/utils/markdownUtils.jsx (2)
181-183: Consider adding lazy loading for images.Images could benefit from
loading="lazy"to improve page performance, especially when rendering markdown files with multiple images.🔎 Proposed enhancement
img: ({ src, alt }) => ( - <img src={src} alt={alt} className="max-w-full h-auto rounded-lg my-2" /> + <img src={src} alt={alt} loading="lazy" className="max-w-full h-auto rounded-lg my-2" /> )
214-223: Optional: Consider extracting shared plugin arrays.The plugin arrays (lines 215-216) are duplicated from the
Markdowncomponent (lines 191-192). While this duplication is minimal, extracting to module-level constants would ensure consistency if the plugin list needs updating.🔎 Proposed refactor
+const REMARK_PLUGINS = [remarkGfm, remarkMath]; +const REHYPE_PLUGINS = [rehypeKatex]; + /** * Markdown component wrapper that provides consistent rendering */ export const Markdown = ({ children, className = '', prose = false }) => { const content = normalizeInlineCodeFences(String(children ?? '')); - const remarkPlugins = useMemo(() => [remarkGfm, remarkMath], []); - const rehypePlugins = useMemo(() => [rehypeKatex], []); + const remarkPlugins = useMemo(() => REMARK_PLUGINS, []); + const rehypePlugins = useMemo(() => REHYPE_PLUGINS, []); // ... }; /** * Hook to use markdown rendering plugins */ export const useMarkdownPlugins = () => { - const remarkPlugins = useMemo(() => [remarkGfm, remarkMath], []); - const rehypePlugins = useMemo(() => [rehypeKatex], []); + const remarkPlugins = useMemo(() => REMARK_PLUGINS, []); + const rehypePlugins = useMemo(() => REHYPE_PLUGINS, []); return { remarkPlugins, rehypePlugins, components: markdownComponents }; };
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/utils/markdownUtils.jsx
🔇 Additional comments (4)
src/utils/markdownUtils.jsx (4)
11-18: LGTM: Handles the target use case correctly.The regex appropriately converts only single-line triple-backtick sequences to inline code. The non-greedy match and exclusion of newlines ensures multi-line code blocks are not affected. Error handling is in place.
21-112: LGTM: CodeBlock extraction properly resolves the Rules of Hooks concern.The component is now a proper named function component with correct hook usage. The clipboard copy implementation includes comprehensive fallbacks, proper error handling, and good accessibility attributes (aria-label, title, focus states).
Based on past review comments, the extraction of CodeBlock to a named component successfully addresses the Rules of Hooks violation.
189-209: LGTM: Well-structured markdown rendering component.The component properly memoizes plugins, normalizes content, and provides a clean API with prose mode support. The string coercion and null handling are appropriate.
1-6: No action needed—markdown library versions are current and secure.The installed versions are all free from known vulnerabilities:
[email protected]— no advisories[email protected]— no advisories[email protected]— no advisories[email protected]— no advisories; KaTeX dependency at0.16.25(patched against CVE-2025-23207)
Summary
Features
✅ Three viewing modes for markdown files:
✅ Full markdown support:
Changes
/src/utils/markdownUtils.jsx- Reusable markdown components/src/components/CodeEditor.jsx- Added preview modesTest Plan
.mdor.markdownfile in the file browserSummary by CodeRabbit
New Features
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.