Migrate submission view to use monaco editor in read-only mode#565
Migrate submission view to use monaco editor in read-only mode#565jon-bell wants to merge 4 commits into
Conversation
|
Important Review skippedToo many files! This PR contains 299 files, which is 149 over the limit of 150. ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (299)
You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis pull request introduces a comprehensive file navigation and code annotation system to the submissions viewer. It adds a file tree sidebar, command palette with fuzzy search, Monaco editor integration with Java language support, inline comment system with rubric annotations, and keyboard shortcuts for efficient navigation and tab management. Changes
Sequence DiagramsequenceDiagram
participant User
participant CommandPalette
participant FileTree
participant CodeFile
participant Monaco
participant RubricMenu
User->>FileTree: Click file / use sidebar
FileTree->>CodeFile: onFileSelect(fileId)
CodeFile->>Monaco: Switch active model/render file
Monaco-->>CodeFile: Editor ready
User->>CommandPalette: Press Cmd+P (file search)
CommandPalette->>CommandPalette: Fuzzy filter files
User->>CommandPalette: Select/Enter file
CommandPalette->>CodeFile: onSelectFile(fileId)
CodeFile->>Monaco: Load & render file
User->>CommandPalette: Press Cmd+Shift+O (symbols)
CommandPalette->>CodeFile: Request currentFileSymbols
CodeFile->>CodeFile: parseJavaFile + buildSymbolIndex
CommandPalette->>CommandPalette: Display symbols
User->>CommandPalette: Select symbol
Monaco->>Monaco: scrollToLine + highlight
User->>Monaco: Right-click (context menu)
Monaco->>RubricMenu: Show rubric options
User->>RubricMenu: Select rubric check
RubricMenu->>CodeFile: Trigger annotation dialog
CodeFile->>CodeFile: AnnotationCommentDialog opens
User->>CodeFile: Submit comment + line range
CodeFile-->>User: Save & display inline comment
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
app/course/[course_id]/assignments/[assignment_id]/submissions/[submissions_id]/files/page.tsx (3)
91-150: Remove unusedFilePickercomponent.The
FilePickercomponent is defined but never used — it's been replaced byFileTreeSidebar. The linter correctly flags this.-function FilePicker({ curFile, onSelect }: { curFile: number; onSelect: (fileId: number) => void }) { - const submission = useSubmission(); - const comments = useSubmissionFileComments({}); - const showCommentsFeature = true; //submission.released !== null || isGraderOrInstructor; - return ( - <Box - maxH="250px" - ... (entire component) - </Box> - ); -}
1173-1173: Remove unused variablefilePickerDisplayIndex.This variable is assigned but never used (FilePicker was replaced).
- const filePickerDisplayIndex = curFileIndex === -1 ? 0 : curFileIndex;
1258-1382: Critical: Hooks called after early returns violate React's Rules of Hooks.The
useEffect(lines 1269, 1293),useCallback(lines 1350, 1358), anduseMemo(line 1371) are called after conditional returns at lines 1258 and 1264. This violates React's requirement that hooks must be called in the same order on every render.This will cause unpredictable behavior and React will throw errors in development mode.
🐛 Required fix: Move all hooks before early returns
Move all the hooks currently after line 1266 to before the early returns. The pattern should be:
+ // Handle sidebar resize - MOVE BEFORE EARLY RETURNS + useEffect(() => { + if (!isResizing) return; + // ... resize logic + }, [isResizing]); + + // Keyboard shortcuts - MOVE BEFORE EARLY RETURNS + useEffect(() => { + const handleKeyDown = (e: KeyboardEvent) => { + // ... shortcut logic + }; + window.addEventListener("keydown", handleKeyDown); + return () => window.removeEventListener("keydown", handleKeyDown); + }, [openFileIds, selectedFileId, handleSelectFile, handleCloseFile]); + + const handleCommandPaletteSelectFile = useCallback(/* ... */); + const handleCommandPaletteSelectSymbol = useCallback(/* ... */); + const currentFileSymbols = useMemo(/* ... */); + if (isLoading) { return <Spinner />; } - const submission = submissionData; - if (!submission) { return <NotFound />; } - // Handle sidebar resize <-- REMOVE FROM HERE - useEffect(() => { ... }, [isResizing]); - - // Keyboard shortcuts <-- REMOVE FROM HERE - useEffect(() => { ... }, [...]); - - // etc...Note: You'll need to adjust the hooks to handle the case where
submissionmight be undefined (e.g., use optional chaining or early-exit guards inside the hooks).components/ui/code-file.tsx (1)
3-10: Remove unused imports to fix linter errors.Several imports are unused and flagged by the linter:
useRubricChecksByRubric,useRubricCriteriaByRubric,useRubricWithParts(lines 6-9)Separator(line 28)chakraComponents,Select,SelectComponentsConfig,SelectInstance(line 31)StudentVisibilityIndicator(line 52)🧹 Remove unused imports
import { useGraderPseudonymousMode, useRubricCheck, - useRubricChecksByRubric, useRubricCriteria, - useRubricCriteriaByRubric, - useRubricWithParts } from "@/hooks/useAssignment";-import { Badge, Box, Button, Flex, HStack, Icon, Separator, Tag, Text, VStack } from "@chakra-ui/react"; +import { Badge, Box, Button, Flex, HStack, Icon, Tag, Text, VStack } from "@chakra-ui/react";-import { chakraComponents, Select, SelectComponentsConfig, SelectInstance } from "chakra-react-select";-import { CommentActions, ReviewRoundTag, StudentVisibilityIndicator } from "./rubric-sidebar"; +import { CommentActions, ReviewRoundTag } from "./rubric-sidebar";Also applies to: 28-28, 31-31, 52-52
🤖 Fix all issues with AI agents
In @components/ui/annotation-comment-dialog.tsx:
- Around line 124-139: The object construction for values uses non-null
assertions on file.class_id and authorProfileId which can crash at runtime;
before building values in annotation-comment-dialog.tsx, add defensive checks
(e.g., if (!file.class_id) or if (!authorProfileId)) and handle them by
returning early or throwing a clear error, or use a safe fallback value if
appropriate; update any callers to handle the early return or propagate the
error so you don't rely on the `!` assertions in values.
In @components/ui/code-file.tsx:
- Line 1010: Remove the unused local variable by deleting the assignment to
browserEvent (the line declaring "const browserEvent = e.event.browserEvent") in
the code handling the event inside the CodeFile component; if the original
intent was to use the native browser event, either reference browserEvent where
needed (e.g., preventDefault/stopPropagation or read properties) or remove the
declaration entirely to eliminate the unused variable warning.
- Around line 989-996: Rename the unused catch parameters to indicate
intentional non-use: change the inner catch parameter `err` to `_err` (in the
map callback surrounding the JSON.parse/transform logic) and change the outer
catch parameter `error` to `_error` (the outer try/catch that returns
`symbols`), so both catch blocks use prefixed names instead of unused
identifiers.
In @components/ui/monaco-rubric-context-menu.tsx:
- Around line 96-99: The computed isDisabled (derived from
existingAnnotationsForCheck, check.max_annotations and check.id) is unused;
update the action-building logic that consumes check to either skip checks that
are disabled (filter out when mapping checks to menu actions) or set a disabled
flag/property on the created action object (e.g., action.disabled = isDisabled
and add an explanatory tooltip/title) so the UI respects max_annotations; locate
where checks are transformed into menu items and apply one of these two fixes
using the isDisabled variable.
🧹 Nitpick comments (14)
app/globals.css (1)
24-28: Emoji glyph may render inconsistently across platforms.Using
content: "💬"is functional but emoji rendering varies by OS/browser. Consider using an SVG icon or a font icon for more consistent visuals if cross-platform consistency is important. Otherwise, this is acceptable for a quick implementation.components/ui/rubric-quick-pick.tsx (1)
59-84: Unusedindexvariable in map callback.The
indexparameter is declared but never used. Consider removing it to keep the code clean.🧹 Proposed fix
- {items.map((action, index) => ( + {items.map((action) => (components/ui/file-tree.tsx (2)
3-3: Unused import:useSubmission.The
useSubmissionhook is imported but never used in this file. Remove it to avoid confusion.🧹 Proposed fix
-import { useSubmission, useSubmissionFileComments } from "@/hooks/useSubmission"; +import { useSubmissionFileComments } from "@/hooks/useSubmission";
83-98:handleClickandhandleDoubleClickhave identical logic.Both handlers perform the same actions. The double-click handler only adds
e.stopPropagation()but the logic is duplicated. Consider extracting to a shared helper or removing the double-click handler if single-click already covers the use case.♻️ Proposed consolidation
+ const performAction = () => { + if (isFile && node.file) { + onFileSelect(node.file.id); + } else if (hasChildren) { + onCollapseChange(node.path, !isCollapsed); + } + }; + const handleClick = () => { - if (isFile && node.file) { - onFileSelect(node.file.id); - } else if (hasChildren) { - onCollapseChange(node.path, !isCollapsed); - } + performAction(); }; const handleDoubleClick = (e: React.MouseEvent) => { e.stopPropagation(); - if (isFile && node.file) { - onFileSelect(node.file.id); - } else if (hasChildren) { - onCollapseChange(node.path, !isCollapsed); - } + performAction(); };components/ui/command-palette.tsx (2)
68-77: Consider using a ref callback instead of setTimeout for focus.The 50ms delay is a pragmatic solution, but it can be flaky in slower environments. A more robust approach is to use a callback ref or
requestAnimationFrame.♻️ Alternative approach
useEffect(() => { if (isOpen) { setQuery(""); setSelectedIndex(0); - // Focus input after a brief delay to ensure modal is rendered - setTimeout(() => { - inputRef.current?.focus(); - }, 50); + // Focus input after paint to ensure modal is rendered + requestAnimationFrame(() => { + inputRef.current?.focus(); + }); } }, [isOpen]);
233-237: Potential key collision for symbols on the same line.The key
${item.symbol.fileId}-${item.symbol.line}could collide if multiple symbols (e.g., method overloads) exist on the same line in the same file. Consider adding the symbol name or index to ensure uniqueness.🔑 Proposed fix
- key={item.type === "file" ? item.file.id : `${item.symbol.fileId}-${item.symbol.line}`} + key={item.type === "file" ? item.file.id : `${item.symbol.fileId}-${item.symbol.line}-${item.symbol.name}`}components/ui/monaco-rubric-context-menu.tsx (2)
168-175: Empty conditional block is dead code.The
if (lastMousePositionRef.current)block contains only a comment and no actual logic. Either remove this check or add the intended logic.🧹 Remove the empty block
const { startLine, endLine } = getSelectionLines(); - // Capture current mouse position when action is triggered - if (lastMousePositionRef.current) { - // Position already captured from mousemove/contextmenu - } onAddComment(startLine, endLine);
277-302: Global mousemove listener adds continuous overhead.Tracking mouse position on every move is expensive. Since you already capture position on
contextmenu(line 285-291) and Monaco'sonContextMenu(line 180-192), the mousemove listener may be unnecessary.♻️ Remove redundant mousemove tracking
useEffect(() => { - const handleMouseMove = (e: MouseEvent) => { - lastMousePositionRef.current = { - top: e.clientY, - left: e.clientX - }; - }; - const handleContextMenu = (e: MouseEvent) => { // Capture position when context menu is triggered lastMousePositionRef.current = { top: e.clientY, left: e.clientX }; }; - // Track mouse movement to always have a recent position - document.addEventListener("mousemove", handleMouseMove, { passive: true }); - // Also capture on context menu (in case mouse hasn't moved) document.addEventListener("contextmenu", handleContextMenu, { passive: true }); return () => { - document.removeEventListener("mousemove", handleMouseMove); document.removeEventListener("contextmenu", handleContextMenu); }; }, []);components/ui/annotation-comment-dialog.tsx (2)
77-95: Consider simplifying focus logic with a single retry approach.The multiple nested timeouts (100ms, 300ms) can be hard to maintain. A cleaner approach might be a single retry loop with requestAnimationFrame.
🔧 Suggested simplification
- const focusInput = () => { - if (messageInputRef.current) { - // For single-line mode (textarea), focus directly - messageInputRef.current.focus(); - } else { - // Fallback: try to find textarea in the dialog - const textarea = document.querySelector('[role="dialog"] textarea') as HTMLTextAreaElement; - if (textarea) { - textarea.focus(); - } - } - }; - - // Try focusing after a short delay to ensure DOM is ready - requestAnimationFrame(() => { - setTimeout(focusInput, 100); - // Also try again after a longer delay as fallback - setTimeout(focusInput, 300); - }); + let attempts = 0; + const maxAttempts = 5; + const tryFocus = () => { + if (messageInputRef.current) { + messageInputRef.current.focus(); + return; + } + const textarea = document.querySelector('[role="dialog"] textarea') as HTMLTextAreaElement; + if (textarea) { + textarea.focus(); + return; + } + if (attempts++ < maxAttempts) { + requestAnimationFrame(tryFocus); + } + }; + requestAnimationFrame(tryFocus);
247-261: Redundant null check inside onClick handler.The
if (onImmediateApply)check on line 252 is redundant since the button only renders whenonImmediateApplyis truthy (line 247).♻️ Remove redundant check
<Button variant="solid" colorPalette="green" onClick={() => { - if (onImmediateApply) { - onImmediateApply(); - } + onImmediateApply(); onClose(); }} disabled={isLoading} >app/course/[course_id]/assignments/[assignment_id]/submissions/[submissions_id]/files/page.tsx (1)
1232-1249: Duplicate Java parsing logic with code-file.tsx.Java file parsing is implemented both here (lines 1232-1249) and in
code-file.tsx(lines 719-738). Consider consolidating into a shared hook to avoid duplication.♻️ Extract to shared hook
// hooks/useJavaSymbols.ts export function useJavaSymbols(files: SubmissionFile[] | undefined) { const [fileSymbols, setFileSymbols] = useState<Map<number, JavaFileSymbols>>(new Map()); useEffect(() => { if (!files) return; const javaFiles = files.filter((f) => f.name.endsWith(".java")); const parsed = new Map<number, JavaFileSymbols>(); for (const file of javaFiles) { try { const symbols = parseJavaFile(file.contents, file.id, file.name); parsed.set(file.id, symbols); } catch (error) { console.warn(`Failed to parse Java file ${file.name}:`, error); } } setFileSymbols(parsed); }, [files]); return fileSymbols; }components/ui/code-file.tsx (3)
719-738: Consider using a proper logger instead of console.warn.Line 731 uses
console.warnwhich triggers linter warnings. While acceptable for debugging, a structured logger would be more consistent.
877-1001: Consider simplifying URI validation logic.The document symbol provider has extensive defensive checks for URI scheme validation (lines 886-912). While robust, this suggests working around Monaco quirks. If these edge cases are truly needed, a comment explaining why would help future maintainers.
If you've encountered specific Monaco URI issues, consider adding a brief comment explaining the scenario, e.g.:
// Monaco sometimes returns URIs without scheme property during hot reload - validate before use
223-1381: Large component could benefit from modularization.The
CodeFilecomponent spans ~1150 lines with multiple concerns:
- Monaco editor setup and lifecycle
- Java language providers (definition, reference, symbols)
- View zone management for inline comments
- Comment dialog state management
Consider extracting into focused modules in a future refactor (e.g.,
useMonacoJavaProviders,useViewZoneManager).
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
app/course/[course_id]/assignments/[assignment_id]/submissions/[submissions_id]/files/page.tsxapp/globals.csscomponents/ui/annotation-comment-dialog.tsxcomponents/ui/code-file.tsxcomponents/ui/command-palette.tsxcomponents/ui/file-tree.tsxcomponents/ui/monaco-rubric-context-menu.tsxcomponents/ui/rubric-quick-pick.tsx
🧰 Additional context used
🧬 Code graph analysis (3)
components/ui/rubric-quick-pick.tsx (1)
components/ui/monaco-rubric-context-menu.tsx (1)
RubricContextMenuAction(12-19)
components/ui/annotation-comment-dialog.tsx (9)
utils/supabase/DatabaseTypes.d.ts (4)
SubmissionFileComment(119-125)SubmissionWithGraderResultsAndFiles(239-245)SubmissionFile(165-165)RubricCheck(686-686)hooks/useSubmission.tsx (1)
useSubmissionController(782-788)hooks/useClassProfiles.tsx (2)
useClassProfiles(24-30)useIsGraderOrInstructor(53-56)hooks/useAssignment.tsx (1)
useGraderPseudonymousMode(71-74)components/ui/toaster.tsx (1)
toaster(5-8)components/ui/code-file.tsx (1)
formatPoints(1767-1772)components/ui/rubric-sidebar.tsx (1)
StudentVisibilityIndicator(696-755)components/ui/message-input.tsx (1)
MessageInput(54-750)components/ui/checkbox.tsx (1)
Checkbox(11-19)
components/ui/command-palette.tsx (2)
utils/supabase/DatabaseTypes.d.ts (1)
SubmissionFile(165-165)components/ui/input.tsx (1)
Input(46-46)
🪛 Biome (2.1.2)
app/course/[course_id]/assignments/[assignment_id]/submissions/[submissions_id]/files/page.tsx
[error] 1269-1269: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 1293-1293: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 1350-1350: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 1358-1358: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 1371-1371: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
🪛 GitHub Actions: Lint
components/ui/code-file.tsx
[error] 3-3: 'useRubricChecksByRubric' is defined but never used. (no-unused-vars)
[error] 8-8: 'useRubricCriteriaByRubric' is defined but never used. (no-unused-vars)
[error] 9-9: 'useRubricWithParts' is defined but never used. (no-unused-vars)
[error] 28-28: 'Separator' is defined but never used. (no-unused-vars)
[error] 31-31: 'chakraComponents' is defined but never used. (no-unused-vars)
[error] 31-31: 'Select' is defined but never used. (no-unused-vars)
[error] 31-31: 'SelectComponentsConfig' is defined but never used. (no-unused-vars)
[error] 31-31: 'SelectInstance' is defined but never used. (no-unused-vars)
[error] 52-52: 'StudentVisibilityIndicator' is defined but never used. (no-unused-vars)
[error] 989-989: 'err' is defined but never used. (no-unused-vars)
[error] 996-996: 'error' is defined but never used. (no-unused-vars)
[error] 1010-1010: 'browserEvent' is assigned a value but never used. (no-unused-vars)
[warning] 731-731: Unexpected console statement. (no-console)
[error] Multiple ESLint errors in code-file.tsx detected (no-unused-vars).
[warning] Need to disable some ESLint rules? See Next.js ESLint docs.
app/course/[course_id]/assignments/[assignment_id]/submissions/[submissions_id]/files/page.tsx
[error] 91-91: 'FilePicker' is defined but never used. (no-unused-vars)
[error] 1173-1173: 'filePickerDisplayIndex' is assigned a value but never used. (no-unused-vars)
[error] 1269-1269: React Hook 'useEffect' is called conditionally. Hooks must be called unconditionally in the same order. (react-hooks/rules-of-hooks)
[error] 1293-1293: React Hook 'useEffect' is called conditionally. Hooks must be called unconditionally in the same order. (react-hooks/rules-of-hooks)
[error] 1350-1350: React Hook 'useCallback' is called conditionally. Hooks must be called in the same order on every render. (react-hooks/rules-of-hooks)
[error] 1358-1358: React Hook 'useCallback' is called conditionally. Hooks must be called in the same order on every render. (react-hooks/rules-of-hooks)
[error] 1371-1371: React Hook 'useMemo' is called conditionally. Hooks must be called in the same order on every render. (react-hooks/rules-of-hooks)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: deploy
🔇 Additional comments (14)
app/globals.css (1)
19-22: Opacity transition without a defined end state.The
.monaco-line-highlightclass has atransition: opacity 2s ease-outbut no change in opacity value is defined in CSS. This suggests the opacity change is controlled via JavaScript (e.g., adding/removing a class or inline style). If that's the case, the implementation is fine. Just ensure the JS side properly triggers the fade by modifying the opacity.components/ui/rubric-quick-pick.tsx (1)
27-35: LGTM on the modal structure and positioning logic.The backdrop + menu pattern with fixed positioning and appropriate zIndex layering is well implemented. The centering via
translate(-50%, -50%)correctly positions the menu at the provided coordinates or viewport center.Also applies to: 37-52
components/ui/file-tree.tsx (2)
26-58: Well-structured tree building logic.The
buildFileTreefunction correctly constructs a hierarchical structure from flat file paths. The path splitting and incremental node creation is clean and handles nested directories properly.
178-211: Clean external/internal state pattern for collapse management.The fallback from
externalCollapsedtointernalCollapsedand the corresponding handler pattern provides good flexibility for both controlled and uncontrolled usage. Well done.components/ui/command-palette.tsx (2)
19-53: Fuzzy match scoring logic is solid.The scoring system with bonuses for consecutive matches, word boundaries, and exact substrings provides good ranking. One minor note: the exact substring bonus (lines 48-50) is applied after the fuzzy match score is potentially zeroed (lines 43-45), meaning a query that exists as an exact substring but wasn't matched by the greedy fuzzy algorithm would still score 2. This is actually a good fallback behavior.
127-148: Keyboard navigation implementation is clean.The handling of Escape, Arrow keys, and Enter is well-structured with proper
preventDefault()calls to avoid scroll behavior on arrow keys.components/ui/monaco-rubric-context-menu.tsx (2)
196-205: Separator using dash characters is a workaround.Monaco doesn't natively support visual separators in context menus, so this is an acceptable workaround. The empty
run()function is intentional.
345-373: Two-level quick pick integration looks good.The flow from selecting a rubric check to choosing "Apply" or "Apply with comment..." is well-structured. The RubricQuickPick components are properly wired with state and callbacks.
components/ui/annotation-comment-dialog.tsx (1)
1-28: LGTM! Clean imports and proper "use client" directive.The imports are well-organized and the component is correctly marked as a client component for Next.js 15.
app/course/[course_id]/assignments/[assignment_id]/submissions/[submissions_id]/files/page.tsx (1)
1194-1230: Approve scroll retry logic with observation.The retry approach (60 attempts × 50ms ≈ 3 seconds) is reasonable for handling async Monaco rendering. The implementation is sound.
components/ui/code-file.tsx (4)
210-244: Clean API design with backward compatibility.Good approach supporting both legacy single-file mode (
fileprop) and new multi-file mode (filesprop) with graceful fallbacks. TheCodeFileHandleprovides a clean imperative API for parent components.
464-508: Good optimization: debounced view zone height updates.The approach of pausing height updates during scrolling (
isScrollingRef) and batching withrequestAnimationFrameis a solid pattern to prevent layout thrashing. The 10px threshold for "significant" height changes is reasonable.
542-582: Nice UX: Line highlight with fade-out animation.The
scrollToLineimplementation correctly reveals the line in center and adds a temporary highlight that fades after 2 seconds. Previous highlights are properly cleaned up before adding new ones.
1105-1107: LGTM - early return is correctly placed after all hooks.Unlike the page.tsx issue, this early return for missing
currentFileis correctly positioned after all hook calls.
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
1 participant
Summary by CodeRabbit
Release Notes
New Features
✏️ Tip: You can customize this high-level summary in your review settings.