-
Notifications
You must be signed in to change notification settings - Fork 99
feature: File explorer #336
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
Conversation
WalkthroughThis update introduces a comprehensive file explorer feature, including a collapsible file tree panel, code preview, and improved navigation in the web application. New React components and hooks manage file tree state, prefetching, and navigation. The layout is restructured for a horizontal split with a file explorer, and related UI components are refactored or replaced for enhanced usability and modularity. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant FileTreePanel
participant ServerActions
participant PureFileTreePanel
participant CodePreviewPanel
participant PureCodePreviewPanel
User->>FileTreePanel: Opens/collapses file tree
FileTreePanel->>ServerActions: getTree(repo, revision)
ServerActions-->>FileTreePanel: File tree data
FileTreePanel->>PureFileTreePanel: Render tree, handle expand/collapse
User->>PureFileTreePanel: Clicks file
PureFileTreePanel->>CodePreviewPanel: Navigate to file path
CodePreviewPanel->>ServerActions: Fetch file source
ServerActions-->>CodePreviewPanel: File source data
CodePreviewPanel->>PureCodePreviewPanel: Render code preview
Poem
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 15
🧹 Nitpick comments (8)
packages/web/src/ee/features/codeNav/components/exploreMenu/referenceList.tsx (1)
148-148
: Consider throttling the mouse enter events for performance.While the prefetching logic is beneficial, frequent mouse movements over list items could trigger excessive API calls. Consider implementing throttling or debouncing to optimize performance, especially for users who quickly move their mouse across many items.
+import { useCallback } from "react"; +import { debounce } from "lodash"; // or implement custom debounce // In the component: +const debouncedPrefetch = useCallback( + debounce((repo: string, revision: string, fileName: string) => { + prefetchFileSource(repo, revision, fileName); + }, 200), + [prefetchFileSource] +); // In the onMouseEnter: -onMouseEnter={() => { - prefetchFileSource(file.repository, revisionName, file.fileName); -}} +onMouseEnter={() => { + debouncedPrefetch(file.repository, revisionName, file.fileName); +}}Also applies to: 156-156, 168-168
packages/web/src/app/[domain]/browse/[...path]/components/treePreviewPanel.tsx (2)
70-72
: Enhance error handling with more specific error messages.The current error handling shows a generic "Error loading tree" message. Consider providing more specific error information to help users understand what went wrong.
if (isFolderContentsError || isRepoInfoError) { - return <div>Error loading tree</div> + return ( + <div className="flex flex-col items-center justify-center p-4 text-center"> + <div className="text-red-600 font-medium">Failed to load repository content</div> + <div className="text-sm text-muted-foreground mt-1"> + {isFolderContentsError && "Could not fetch folder contents. "} + {isRepoInfoError && "Could not fetch repository information. "} + Please try again or contact support if the issue persists. + </div> + </div> + ) }
61-68
: Consider showing more detailed loading states.While the loading spinner is functional, consider showing different loading messages or states for fetching repository info vs folder contents to provide better user feedback.
if (isFolderContentsPending || isRepoInfoPending) { return ( <div className="flex flex-col w-full min-h-full items-center justify-center"> <Loader2 className="w-4 h-4 animate-spin" /> - Loading... + {isRepoInfoPending && isFolderContentsPending + ? "Loading repository..." + : isRepoInfoPending + ? "Loading repository info..." + : "Loading folder contents..." + } </div> ) }packages/web/src/app/[domain]/components/pathHeader.tsx (1)
15-15
: Rename interface to match component name.The interface is named
FileHeaderProps
but the component isPathHeader
. This inconsistency could cause confusion.-interface FileHeaderProps { +interface PathHeaderProps {Also update the usage on line 49:
-}: FileHeaderProps) => { +}: PathHeaderProps) => {packages/web/src/features/fileTree/components/fileTreeItemComponent.tsx (2)
32-54
: Consider using smooth scrolling for better user experience.The instant scrolling behavior might be jarring for users. Consider using smooth scrolling unless there's a specific performance requirement.
scrollIntoView(ref.current, { scrollMode: 'if-needed', block: 'center', - behavior: 'instant', + behavior: 'smooth', // We only want to scroll if the element is hidden vertically
94-102
: Remove unnecessary Fragment wrapper.The Fragment is redundant as it only contains a single conditional element.
> {isCollapseChevronVisible && ( - <> {isCollapsed ? ( <ChevronRightIcon className="w-4 h-4 flex-shrink-0" /> ) : ( <ChevronDownIcon className="w-4 h-4 flex-shrink-0" /> )} - </> )} </div>🧰 Tools
🪛 Biome (1.9.4)
[error] 95-101: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment(lint/complexity/noUselessFragments)
packages/web/src/app/[domain]/browse/layout.tsx (1)
28-34
: Consider using a higher z-index value from a consistent scale.A z-index of 10 might not be sufficient for a sticky header if there are modals or dropdowns. Consider using a higher value from a consistent z-index scale.
- <div className='sticky top-0 left-0 right-0 z-10'> + <div className='sticky top-0 left-0 right-0 z-50'>packages/web/src/app/[domain]/browse/[...path]/components/codePreviewPanel.tsx (1)
21-33
: Consider adding retry and cache configuration to queries.The queries lack retry and cache configuration which could improve reliability and performance.
const { data: fileSourceResponse, isPending: isFileSourcePending, isError: isFileSourceError } = useQuery({ queryKey: ['fileSource', repoName, revisionName, path, domain], queryFn: () => unwrapServiceError(getFileSource({ fileName: path, repository: repoName, branch: revisionName }, domain)), + retry: 2, + staleTime: 5 * 60 * 1000, // 5 minutes }); const { data: repoInfoResponse, isPending: isRepoInfoPending, isError: isRepoInfoError } = useQuery({ queryKey: ['repoInfo', repoName, domain], queryFn: () => unwrapServiceError(getRepoInfoByName(repoName, domain)), + retry: 2, + staleTime: 10 * 60 * 1000, // 10 minutes });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (21)
packages/backend/src/utils.ts
(1 hunks)packages/web/package.json
(1 hunks)packages/web/src/app/[domain]/browse/[...path]/components/codePreviewPanel.tsx
(1 hunks)packages/web/src/app/[domain]/browse/[...path]/components/pureCodePreviewPanel.tsx
(1 hunks)packages/web/src/app/[domain]/browse/[...path]/components/treePreviewPanel.tsx
(1 hunks)packages/web/src/app/[domain]/browse/[...path]/page.tsx
(1 hunks)packages/web/src/app/[domain]/browse/browseStateProvider.tsx
(2 hunks)packages/web/src/app/[domain]/browse/components/bottomPanel.tsx
(2 hunks)packages/web/src/app/[domain]/browse/hooks/useBrowseParams.ts
(1 hunks)packages/web/src/app/[domain]/browse/layout.tsx
(1 hunks)packages/web/src/app/[domain]/components/fileHeader.tsx
(0 hunks)packages/web/src/app/[domain]/components/pathHeader.tsx
(1 hunks)packages/web/src/app/[domain]/search/components/searchResultsPanel/fileMatchContainer.tsx
(2 hunks)packages/web/src/app/[domain]/search/page.tsx
(1 hunks)packages/web/src/ee/features/codeNav/components/exploreMenu/referenceList.tsx
(6 hunks)packages/web/src/features/fileTree/actions.ts
(1 hunks)packages/web/src/features/fileTree/components/fileTreeItemComponent.tsx
(1 hunks)packages/web/src/features/fileTree/components/fileTreePanel.tsx
(1 hunks)packages/web/src/features/fileTree/components/pureFileTreePanel.tsx
(1 hunks)packages/web/src/hooks/usePrefetchFileSource.ts
(1 hunks)packages/web/src/hooks/usePrefetchFolderContents.ts
(1 hunks)
💤 Files with no reviewable changes (1)
- packages/web/src/app/[domain]/components/fileHeader.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
packages/web/src/app/[domain]/browse/[...path]/components/pureCodePreviewPanel.tsx
[error] 197-197: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/web/src/features/fileTree/components/fileTreeItemComponent.tsx
[error] 95-101: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (26)
packages/backend/src/utils.ts (1)
97-98
: Good documentation of technical debt.The comments appropriately highlight code duplication and suggest a concrete solution. Moving shared utilities like
getRepoPath
to a shared package would improve maintainability and reduce duplication.packages/web/src/app/[domain]/search/page.tsx (1)
270-270
: UI consistency improvement.Changing the button variant to "ghost" provides a more subtle appearance for the collapsed filter panel toggle, which aligns well with the overall UI design patterns.
packages/web/src/app/[domain]/browse/components/bottomPanel.tsx (2)
23-25
: Well-defined props interface.The
BottomPanelProps
interface clearly defines the expected prop structure, improving type safety and component documentation.
27-27
: Good flexibility improvement.Making the panel order dynamic instead of hardcoded allows for better layout flexibility and reusability of the component in different panel arrangements.
Also applies to: 101-101
packages/web/src/app/[domain]/search/components/searchResultsPanel/fileMatchContainer.tsx (2)
3-3
: LGTM: Clean component import update.The import change from
FileHeader
toPathHeader
aligns with the broader refactoring mentioned in the AI summary.
95-106
: LGTM: Consistent prop renaming for PathHeader component.The prop changes from
fileName
/fileNameHighlightRange
topath
/pathHighlightRange
are consistent with the newPathHeader
component interface. The remaining props are correctly preserved.packages/web/src/hooks/usePrefetchFileSource.ts (1)
9-25
: LGTM: Well-implemented prefetching hook with proper memoization.The hook follows React Query best practices:
- Proper use of
useCallback
for memoization with correct dependencies- Query key includes all relevant parameters including domain for proper scoping
- Error handling via
unwrapServiceError
wrapper- Clean separation of concerns
packages/web/src/hooks/usePrefetchFolderContents.ts (1)
9-27
: LGTM: Consistent implementation following established patterns.This hook mirrors the structure of
usePrefetchFileSource
with appropriate differences:
- Uses
'tree'
query key for folder contents vs'fileSource'
for files- Calls
getFolderContents
API appropriately- Maintains same error handling and memoization patterns
The implementation is consistent and follows React Query best practices.
packages/web/src/app/[domain]/browse/hooks/useBrowseParams.ts (5)
8-11
: LGTM: Proper error handling for missing browse segment.The error message is descriptive and helps with debugging invalid pathnames.
14-17
: LGTM: Robust pattern matching with clear error messaging.The regex pattern correctly matches the sentinel
/-(tree|blob)/
pattern and provides helpful error context.
19-21
: LGTM: Clean extraction of repository and revision information.The split logic correctly handles both cases - with and without revision specification using the
@
delimiter.
23-40
: LGTM: Proper URL decoding with good documentation.The IIFE pattern cleanly separates the path extraction logic, and the
decodeURIComponent
usage is correctly documented for handling spaces in paths. The switch statement properly handles bothtree
andblob
path types.
42-48
: LGTM: Clean return object structure.The return object provides a clear interface with all necessary browse parameters properly typed.
packages/web/src/ee/features/codeNav/components/exploreMenu/referenceList.tsx (2)
4-4
: LGTM! Consistent migration from FileHeader to PathHeader.The component migration from
FileHeader
toPathHeader
with the prop change fromfileName
topath
aligns with the broader UI refactoring described in the AI summary. The breadcrumb-style path header provides better navigation context.Also applies to: 94-103
12-12
: Well-implemented prefetching optimization.The integration of
usePrefetchFileSource
withonMouseEnter
events is a good UX optimization that will reduce loading times when users click on references. The comment explaining the prefetching strategy is helpful for maintainability.Also applies to: 35-35, 124-130
packages/web/src/app/[domain]/browse/[...path]/page.tsx (2)
8-8
: Clean conditional rendering logic.The simplified routing logic using
pathType
to conditionally renderCodePreviewPanel
orTreePreviewPanel
is clean and maintainable. This approach properly delegates data fetching and error handling to specialized components.Also applies to: 12-16
1-1
:❓ Verification inconclusive
Verify SEO impact of server-side to client-side migration.
The migration from server-side rendering to client-side rendering simplifies the component but may impact SEO and initial page load performance. Since this appears to be a browse/file explorer page, search engine discoverability could be important.
Consider evaluating:
- Whether the file browsing pages need to be indexed by search engines
- Impact on initial load performance, especially for large repositories
- Whether critical file metadata should be server-rendered for better UX
Also applies to: 7-19
🌐 Web query:
What are the SEO implications of migrating from Next.js server components to client components for file browsing pages?
💡 Result:
Migrating from Next.js server components to client components for file browsing pages has several SEO implications to consider:
Server-Side Rendering (SSR) vs Client-Side Rendering (CSR):
- Server components in Next.js are rendered on the server before being sent to the client. This means search engine crawlers receive fully rendered HTML content, which is great for SEO as it ensures content is easily crawlable and indexable.
- Client components, by contrast, are rendered on the client side after the initial HTML is loaded as a shell. This often means search engines initially see minimal content in the HTML and must execute JavaScript to render the full page content.
Crawlability and Indexing:
- Many modern search engines, including Google, can execute JavaScript and index client-rendered content. However, this is not guaranteed for all search engines or bots and can be slower or less reliable.
- If the file browsing pages rely heavily on client components, there is a risk that some search engines may not render and index the content fully, leading to SEO weaknesses.
Page Load Performance and User Experience:
- Server components can improve Time-to-First-Byte (TTFB) and Time-to-Interactive (TTI) because the HTML is ready with content.
- Client components may result in a longer initial blank or loading state, which can negatively affect Core Web Vitals and user experience signals that impact SEO ranking.
Metadata and Structured Data:
- Server components allow easier injection of SEO-critical metadata such as title tags, meta descriptions, and structured data on the server side.
- With client components, you may need to ensure that metadata is handled properly with hydration or client-side libraries, which can be less reliable for SEO.
Consistency and Fallbacks:
- Server components provide a consistent, reliable SEO baseline since the rendered HTML is complete.
- Client components might require more fallback or pre-rendering strategies to ensure crawlers still receive meaningful content.
Recommendations:
- If SEO for file browsing pages is a priority, it is generally better to use server components or hybrid rendering strategies (static generation or SSR) to ensure content is immediately available in the HTML.
- If client components must be used, consider implementing server-side pre-rendering or incremental static regeneration to deliver at least a partial content snapshot to crawlers.
- Test your pages with tools like Google Search Console’s URL Inspection, Lighthouse, and third-party crawler simulators to verify how your client-rendered pages are indexed.
If you'd like, I can also search for the latest best practices or case studies related to this migration and SEO impact. Would you like me to do that?
Assess SEO & Initial Load Impact of CSR Migration
Client-side rendering of the file browser may hinder content crawlability, slow initial rendering, and complicate metadata injection. Please review the following before shipping:
Crawlability & Indexing
• Use Google Search Console’s URL Inspection (and third-party crawler simulators) to confirm bots see full directory listings.
• Decide if browse pages must be fully indexable; if so, CSR alone may be insufficient.Performance & UX
• Benchmark TTFB, LCP and TTI for large repos under CSR.
• Ensure loading states don’t create excessive blank screens that harm Core Web Vitals.Metadata & Structured Data
• Verify that<title>
, meta descriptions, and JSON-LD are present in initial HTML or rendered via SSR/ISR.
• If metadata is client-injected, test that crawlers pick it up.Rendering Strategy
• Consider hybrid approaches (getStaticProps/ISR or getServerSideProps) for critical file metadata.
• If full CSR is required, implement server-side pre-rendering or ISR fallback for SEO.packages/web/src/app/[domain]/browse/browseStateProvider.tsx (2)
14-14
: Good addition of file tree panel state.The
isFileTreePanelCollapsed
state property with a default value offalse
is well-integrated and supports the new file explorer functionality. This follows the established pattern for panel state management.Also applies to: 22-22
37-40
: Good explicit typing of props interface.Defining the
BrowseStateProviderProps
interface explicitly instead of inline typing improves code readability and maintainability.packages/web/src/app/[domain]/browse/[...path]/components/treePreviewPanel.tsx (2)
28-42
: Well-structured data fetching with React Query.The parallel data fetching using separate
useQuery
hooks for repository info and folder contents is properly implemented. The query keys are comprehensive and include all necessary dependencies for cache invalidation.
44-59
: Efficient prefetching strategy with proper memoization.The
onNodeClicked
andonNodeMouseEnter
callbacks are properly memoized withuseCallback
and include all necessary dependencies. The conditional prefetching based on node type (blob vs tree) is a smart optimization.packages/web/src/app/[domain]/components/pathHeader.tsx (1)
76-77
:⚠️ Potential issuePotential issue with duplicate path segments.
The
indexOf
method finds the first occurrence of a substring, which could lead to incorrect highlight calculations if the same segment name appears multiple times in the path (e.g.,/src/components/src/index.js
).Consider using the accumulated position instead:
-const segmentStart = path.indexOf(part, currentPath.length - part.length); +const segmentStart = currentPath.length - part.length + (index > 0 ? 1 : 0); // +1 for '/' separatorLikely an incorrect or invalid review comment.
packages/web/src/features/fileTree/components/fileTreeItemComponent.tsx (1)
56-72
: LGTM! Icon handling is well-implemented.The icon name derivation logic correctly handles both files and folders with appropriate fallback.
packages/web/src/features/fileTree/components/pureFileTreePanel.tsx (1)
66-77
: LGTM! Path expansion logic is well-implemented.The logic correctly expands all parent folders when the path changes, ensuring the current file is visible in the tree.
packages/web/src/app/[domain]/browse/layout.tsx (1)
35-63
: LGTM! Well-structured panel layout.The horizontal-vertical nested panel structure provides good flexibility for the file explorer layout with appropriate sizing constraints.
packages/web/src/features/fileTree/actions.ts (1)
11-19
: Type definitions look good!The type structure is clean and appropriate for representing hierarchical file trees.
packages/web/src/app/[domain]/browse/[...path]/components/pureCodePreviewPanel.tsx
Show resolved
Hide resolved
packages/web/src/app/[domain]/browse/[...path]/components/pureCodePreviewPanel.tsx
Show resolved
Hide resolved
e80ba30
to
9c5cf58
Compare
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: 1
♻️ Duplicate comments (7)
packages/web/src/app/[domain]/browse/[...path]/components/pureCodePreviewPanel.tsx (2)
197-197
: Use optional chaining for safer property access.The condition can be simplified using optional chaining for better readability and safety.
Apply this diff:
-{editorRef && editorRef.view && currentSelection && ( +{editorRef?.view && currentSelection && (🧰 Tools
🪛 Biome (1.9.4)
[error] 197-197: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
59-66
:⚠️ Potential issueAdd validation for parsed integer values.
The
parseInt
function can returnNaN
for invalid input, which could cause issues downstream when creating highlight ranges.Apply this diff to add proper validation:
const [start, end] = highlightRangeQuery.split(',').map((range) => { if (range.includes(':')) { - return range.split(':').map((val) => parseInt(val, 10)); + const parts = range.split(':').map((val) => parseInt(val, 10)); + if (parts.some(isNaN)) { + return null; + } + return parts; } // For line-only format, use column 1 for start and last column for end const line = parseInt(range, 10); - return [line]; + return isNaN(line) ? null : [line]; }); + +if (!start || !end) { + return; +}packages/web/src/app/[domain]/browse/[...path]/components/codePreviewPanel.tsx (2)
80-85
: 🛠️ Refactor suggestionAdd dimensions to Image component to prevent layout shift.
The Next.js Image component should have width and height props to prevent cumulative layout shift.
Apply this diff:
<Image src={codeHostInfo.icon} alt={codeHostInfo.codeHostName} className={cn('w-4 h-4 flex-shrink-0', codeHostInfo.iconClassName)} + width={16} + height={16} />
57-59
: 🛠️ Refactor suggestionImprove error handling with more specific messages.
The generic error message doesn't help users understand what went wrong or provide recovery options.
Apply this diff to provide more specific error handling:
if (isFileSourceError || isRepoInfoError) { - return <div>Error loading file source</div> + return ( + <div className="flex flex-col w-full min-h-full items-center justify-center gap-2"> + <div className="text-red-500"> + {isFileSourceError && "Failed to load file content"} + {isRepoInfoError && "Failed to load repository information"} + </div> + <button + onClick={() => window.location.reload()} + className="px-4 py-2 bg-primary text-primary-foreground rounded-md" + > + Retry + </button> + </div> + ) }packages/web/src/features/fileTree/components/fileTreePanel.tsx (3)
100-100
: Make keyboard shortcut hint cross-platform.The keyboard shortcut hint shows "⌘ B" which is Mac-specific, but the actual hotkey implementation uses "mod+b" which works cross-platform. Consider making the hint dynamic based on the user's platform.
Add a utility to get the platform-specific modifier key:
// In a utils file export const getModifierKey = () => { const isMac = typeof window !== 'undefined' && navigator.platform.toUpperCase().indexOf('MAC') >= 0; return isMac ? '⌘' : 'Ctrl'; };Then update the hint:
-<KeyboardShortcutHint shortcut="⌘ B" /> +<KeyboardShortcutHint shortcut={`${getModifierKey()} B`} />
141-141
: Make keyboard shortcut hint cross-platform.Same issue as line 100 - the keyboard shortcut hint should be dynamic based on the user's platform.
-<KeyboardShortcutHint shortcut="⌘ B" /> +<KeyboardShortcutHint shortcut={`${getModifierKey()} B`} />
185-186
: Fix invalid Tailwind CSS width classes.The skeleton component uses invalid Tailwind width classes like
w-18
. Tailwind CSS doesn't provide these specific width utilities by default.Replace with valid Tailwind classes or use arbitrary values:
-<Skeleton className="h-4 w-18" /> +<Skeleton className="h-4 w-[72px]" /> // or use w-16, w-20, etc.
🧹 Nitpick comments (2)
packages/web/src/features/fileTree/components/fileTreeItemComponent.tsx (2)
56-72
: Consider adding error handling for icon generation.The icon name generation logic performs string manipulation that could be fragile if the
vscode-icons-js
library returns unexpected formats. Consider adding validation or fallback handling.const iconName = useMemo(() => { if (node.type === 'tree') { const icon = getIconForFolder(node.name); if (icon) { + const dotIndex = icon.indexOf('.'); + if (dotIndex === -1) return "vscode-icons:file-type-unknown"; - const iconName = `vscode-icons:${icon.substring(0, icon.indexOf('.')).replaceAll('_', '-')}`; + const iconName = `vscode-icons:${icon.substring(0, dotIndex).replaceAll('_', '-')}`; return iconName; } } else if (node.type === 'blob') { const icon = getIconForFile(node.name); if (icon) { + const dotIndex = icon.indexOf('.'); + if (dotIndex === -1) return "vscode-icons:file-type-unknown"; - const iconName = `vscode-icons:${icon.substring(0, icon.indexOf('.')).replaceAll('_', '-')}`; + const iconName = `vscode-icons:${icon.substring(0, dotIndex).replaceAll('_', '-')}`; return iconName; } } return "vscode-icons:file-type-unknown"; }, [node.name, node.type]);
94-102
: Remove unnecessary Fragment.The React Fragment is redundant since it only wraps a single conditional expression.
<div className="flex flex-row gap-1 cursor-pointer w-4 h-4 flex-shrink-0" > {isCollapseChevronVisible && ( - <> - {isCollapsed ? ( - <ChevronRightIcon className="w-4 h-4 flex-shrink-0" /> - ) : ( - <ChevronDownIcon className="w-4 h-4 flex-shrink-0" /> - )} - </> + isCollapsed ? ( + <ChevronRightIcon className="w-4 h-4 flex-shrink-0" /> + ) : ( + <ChevronDownIcon className="w-4 h-4 flex-shrink-0" /> + ) )} </div>🧰 Tools
🪛 Biome (1.9.4)
[error] 95-101: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment(lint/complexity/noUselessFragments)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (21)
packages/backend/src/utils.ts
(1 hunks)packages/web/package.json
(1 hunks)packages/web/src/app/[domain]/browse/[...path]/components/codePreviewPanel.tsx
(1 hunks)packages/web/src/app/[domain]/browse/[...path]/components/pureCodePreviewPanel.tsx
(1 hunks)packages/web/src/app/[domain]/browse/[...path]/components/treePreviewPanel.tsx
(1 hunks)packages/web/src/app/[domain]/browse/[...path]/page.tsx
(1 hunks)packages/web/src/app/[domain]/browse/browseStateProvider.tsx
(2 hunks)packages/web/src/app/[domain]/browse/components/bottomPanel.tsx
(2 hunks)packages/web/src/app/[domain]/browse/hooks/useBrowseParams.ts
(1 hunks)packages/web/src/app/[domain]/browse/layout.tsx
(1 hunks)packages/web/src/app/[domain]/components/fileHeader.tsx
(0 hunks)packages/web/src/app/[domain]/components/pathHeader.tsx
(1 hunks)packages/web/src/app/[domain]/search/components/searchResultsPanel/fileMatchContainer.tsx
(2 hunks)packages/web/src/app/[domain]/search/page.tsx
(1 hunks)packages/web/src/ee/features/codeNav/components/exploreMenu/referenceList.tsx
(6 hunks)packages/web/src/features/fileTree/actions.ts
(1 hunks)packages/web/src/features/fileTree/components/fileTreeItemComponent.tsx
(1 hunks)packages/web/src/features/fileTree/components/fileTreePanel.tsx
(1 hunks)packages/web/src/features/fileTree/components/pureFileTreePanel.tsx
(1 hunks)packages/web/src/hooks/usePrefetchFileSource.ts
(1 hunks)packages/web/src/hooks/usePrefetchFolderContents.ts
(1 hunks)
💤 Files with no reviewable changes (1)
- packages/web/src/app/[domain]/components/fileHeader.tsx
🚧 Files skipped from review as they are similar to previous changes (15)
- packages/web/src/app/[domain]/search/page.tsx
- packages/backend/src/utils.ts
- packages/web/package.json
- packages/web/src/app/[domain]/search/components/searchResultsPanel/fileMatchContainer.tsx
- packages/web/src/hooks/usePrefetchFolderContents.ts
- packages/web/src/app/[domain]/browse/components/bottomPanel.tsx
- packages/web/src/hooks/usePrefetchFileSource.ts
- packages/web/src/app/[domain]/browse/[...path]/components/treePreviewPanel.tsx
- packages/web/src/ee/features/codeNav/components/exploreMenu/referenceList.tsx
- packages/web/src/app/[domain]/browse/hooks/useBrowseParams.ts
- packages/web/src/app/[domain]/browse/browseStateProvider.tsx
- packages/web/src/app/[domain]/components/pathHeader.tsx
- packages/web/src/features/fileTree/components/pureFileTreePanel.tsx
- packages/web/src/app/[domain]/browse/layout.tsx
- packages/web/src/features/fileTree/actions.ts
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/web/src/app/[domain]/browse/[...path]/components/pureCodePreviewPanel.tsx (3)
packages/web/src/app/[domain]/browse/hooks/useBrowseState.ts (1)
useBrowseState
(6-12)packages/web/src/app/[domain]/browse/hooks/useBrowseNavigation.ts (1)
useBrowseNavigation
(25-59)packages/web/src/components/ui/scroll-area.tsx (1)
ScrollArea
(48-48)
packages/web/src/features/fileTree/components/fileTreeItemComponent.tsx (1)
packages/web/src/features/fileTree/actions.ts (1)
FileTreeItem
(11-15)
🪛 Biome (1.9.4)
packages/web/src/app/[domain]/browse/[...path]/components/pureCodePreviewPanel.tsx
[error] 197-197: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/web/src/features/fileTree/components/fileTreeItemComponent.tsx
[error] 95-101: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (6)
packages/web/src/app/[domain]/browse/[...path]/page.tsx (1)
1-20
: Excellent architectural simplification!The rewrite from a server-side component to a clean client-side component with delegated responsibilities is well-executed. The conditional rendering based on
pathType
is clear and the separation of concerns between parameter extraction (useBrowseParams
) and specialized preview panels improves maintainability.packages/web/src/app/[domain]/browse/[...path]/components/pureCodePreviewPanel.tsx (1)
135-183
: Well-structured callback handlers for code navigation.The
onFindReferences
andonGotoDefinition
callbacks are well-implemented with proper event capturing, state management, and navigation logic. The handling of multiple symbol definitions is particularly thoughtful.packages/web/src/app/[domain]/browse/[...path]/components/codePreviewPanel.tsx (1)
17-46
: Excellent data fetching architecture.The component demonstrates good separation of concerns by handling data fetching with React Query and delegating rendering to
PureCodePreviewPanel
. The use ofuseBrowseParams
for parameter extraction and proper memoization ofcodeHostInfo
shows thoughtful design.packages/web/src/features/fileTree/components/fileTreeItemComponent.tsx (1)
32-54
: LGTM! Well-implemented scroll behavior.The scroll-into-view implementation with custom boundary detection is well thought out and handles the specific needs of keeping active items visible within the parent scroll container.
packages/web/src/features/fileTree/components/fileTreePanel.tsx (2)
47-55
: LGTM! Well-structured data fetching.The React Query implementation is properly structured with appropriate query keys and error handling using
unwrapServiceError
.
57-67
: LGTM! Good keyboard shortcut implementation.The hotkey implementation correctly uses the cross-platform "mod+b" and includes appropriate options for enabling on form tags and content editable elements.
packages/web/src/features/fileTree/components/fileTreePanel.tsx
Outdated
Show resolved
Hide resolved
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: 1
🧹 Nitpick comments (3)
packages/web/src/app/[domain]/components/pathHeader.tsx (3)
21-36
: Fix interface naming inconsistency.The interface is named
FileHeaderProps
but the component isPathHeader
. This inconsistency could cause confusion during maintenance.-interface FileHeaderProps { +interface PathHeaderProps {And update the component props type:
-}: FileHeaderProps) => { +}: PathHeaderProps) => {
109-160
: Consider refactoring the width calculation logic for better maintainability.The current implementation has several areas for improvement:
- Magic number for reserved space (175) should be a named constant
- The temporary DOM element approach could be optimized using canvas measureText or a persistent hidden element
- The complex calculation logic could be extracted into a custom hook
+const RESERVED_SPACE_WIDTH = 175; // Space for copy button and padding + useEffect(() => { const measureSegments = () => { if (!containerRef.current || !breadcrumbsRef.current) return; const containerWidth = containerRef.current.offsetWidth; - const availableWidth = containerWidth - 175; // Reserve space for copy button and padding + const availableWidth = containerWidth - RESERVED_SPACE_WIDTH;Consider extracting this logic into a custom hook like
useBreadcrumbVisibility
for better reusability and testability.
223-315
: Consider breaking down the component for better maintainability.The render method handles multiple UI concerns and the overall component is quite large (300+ lines). While the current implementation works well, consider extracting sub-components for better modularity:
RepositoryInfo
component for lines 225-253BreadcrumbNavigation
component for lines 255-300CopyButton
component for lines 301-312This would improve maintainability, testability, and make the component easier to understand. However, the current implementation is functional and well-structured within a single component.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
CHANGELOG.md
(1 hunks)packages/web/src/app/[domain]/components/pathHeader.tsx
(1 hunks)packages/web/src/lib/newsData.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- CHANGELOG.md
- packages/web/src/lib/newsData.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (4)
packages/web/src/app/[domain]/components/pathHeader.tsx (4)
48-72
: LGTM!The component setup and hook usage is well-structured and appropriate for the breadcrumb functionality.
176-204
: LGTM!The event handlers are well-implemented with good UX considerations like toast notifications, visual feedback for copy operations, and performance optimizations through prefetching on hover.
206-221
: LGTM!The segment highlighting logic is clean and properly handles both light and dark theme styling.
1-315
: Excellent implementation that aligns well with PR objectives.This component successfully implements the breadcrumb navigation feature described in the PR objectives with several standout features:
✅ Fast navigation: Prefetching on hover improves perceived performance
✅ Responsive design: Dynamic breadcrumb collapse handles long paths
✅ Good UX: Copy functionality, visual feedback, and keyboard accessibility
✅ Integration: Works well with the new file explorer architectureThe component addresses the TODO mentioned in the PR description about handling long paths with ellipsis functionality.
…e hierchy across file navigations
…pses. Clicking on the ellipses reveales a dropdown menu with the hidden path parts.
6f4e955
to
2b8fce2
Compare
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: 1
♻️ Duplicate comments (4)
packages/web/src/app/[domain]/browse/[...path]/components/pureCodePreviewPanel.tsx (2)
197-197
: Use optional chaining for safer property access.The condition can be simplified using optional chaining for better readability and safety.
-{editorRef && editorRef.view && currentSelection && ( +{editorRef?.view && currentSelection && (🧰 Tools
🪛 Biome (1.9.4)
[error] 197-197: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
59-66
:⚠️ Potential issueAdd validation for parsed integer values.
The
parseInt
function can returnNaN
for invalid input, which could cause runtime errors when accessing array indices or creating line/column objects.Apply this diff to add proper validation:
const [start, end] = highlightRangeQuery.split(',').map((range) => { if (range.includes(':')) { - return range.split(':').map((val) => parseInt(val, 10)); + const parts = range.split(':').map((val) => parseInt(val, 10)); + if (parts.some(isNaN)) { + return null; + } + return parts; } // For line-only format, use column 1 for start and last column for end const line = parseInt(range, 10); - return [line]; + return isNaN(line) ? null : [line]; }); + +if (!start || !end) { + return; +}packages/web/src/app/[domain]/browse/[...path]/components/codePreviewPanel.tsx (2)
57-59
: 🛠️ Refactor suggestionImprove error handling with more specific messages.
The generic error message doesn't help users understand what went wrong or provide recovery options.
- if (isFileSourceError || isRepoInfoError) { - return <div>Error loading file source</div> - } + if (isFileSourceError || isRepoInfoError) { + return ( + <div className="flex flex-col w-full min-h-full items-center justify-center gap-2"> + <div className="text-red-500"> + {isFileSourceError && "Failed to load file content"} + {isRepoInfoError && "Failed to load repository information"} + </div> + <button + onClick={() => window.location.reload()} + className="px-4 py-2 bg-primary text-primary-foreground rounded-md" + > + Retry + </button> + </div> + ) + }
80-84
: 💡 Verification agent🧩 Analysis chain
Missing width and height props for Next.js Image component.
The Image component should include explicit dimensions to prevent layout shift during loading.
🏁 Script executed:
#!/bin/bash # Verify if the Image component has proper width/height props ast-grep --pattern $'<Image $$$ className={$_} $$$ />'Length of output: 6661
Add explicit width and height to Next.js
<Image>
to prevent layout shiftThe icon in
codePreviewPanel.tsx
currently relies on CSS sizing only. Next.js’s<Image>
requireswidth
andheight
(orfill
+ a positioned parent) so it can reserve space before the image loads. Please update as follows:• File:
packages/web/src/app/[domain]/browse/[...path]/components/codePreviewPanel.tsx
• Lines: ~80–84Suggested diff:
- <Image - src={codeHostInfo.icon} - alt={codeHostInfo.codeHostName} - className={cn('w-4 h-4 flex-shrink-0', codeHostInfo.iconClassName)} - /> + <Image + src={codeHostInfo.icon} + alt={codeHostInfo.codeHostName} + width={16} + height={16} + className={cn('flex-shrink-0', codeHostInfo.iconClassName)} + />If you prefer a fluid layout, you can also wrap the
<Image fill />
in arelative
container with the desired CSS dimensions.
🧹 Nitpick comments (2)
packages/web/src/features/fileTree/components/fileTreeItemComponent.tsx (2)
56-72
: Icon handling logic is correct but consider performance optimization.The memoized icon determination correctly handles both file and folder types with proper fallback. However, the string manipulation could be simplified.
Consider extracting the icon transformation logic into a helper function for better readability:
+const transformIconName = (icon: string): string => { + return `vscode-icons:${icon.substring(0, icon.indexOf('.')).replaceAll('_', '-')}`; +}; const iconName = useMemo(() => { if (node.type === 'tree') { const icon = getIconForFolder(node.name); if (icon) { - const iconName = `vscode-icons:${icon.substring(0, icon.indexOf('.')).replaceAll('_', '-')}`; - return iconName; + return transformIconName(icon); } } else if (node.type === 'blob') { const icon = getIconForFile(node.name); if (icon) { - const iconName = `vscode-icons:${icon.substring(0, icon.indexOf('.')).replaceAll('_', '-')}`; - return iconName; + return transformIconName(icon); } } return "vscode-icons:file-type-unknown"; }, [node.name, node.type]);
11-29
: Consider adding PropTypes or improving TypeScript types for better type safety.The component props are well-defined, but consider making the component more type-safe by defining a proper interface for the props.
Consider defining an interface for better type organization:
+interface FileTreeItemComponentProps { + node: FileTreeItem; + isActive: boolean; + depth: number; + isCollapsed?: boolean; + isCollapseChevronVisible?: boolean; + onClick: () => void; + onMouseEnter: () => void; + parentRef: React.RefObject<HTMLDivElement>; +} -export const FileTreeItemComponent = ({ - node, - isActive, - depth, - isCollapsed = false, - isCollapseChevronVisible = true, - onClick, - onMouseEnter, - parentRef, -}: { - node: FileTreeItem, - isActive: boolean, - depth: number, - isCollapsed?: boolean, - isCollapseChevronVisible?: boolean, - onClick: () => void, - onMouseEnter: () => void, - parentRef: React.RefObject<HTMLDivElement>, -}) => { +export const FileTreeItemComponent = ({ + node, + isActive, + depth, + isCollapsed = false, + isCollapseChevronVisible = true, + onClick, + onMouseEnter, + parentRef, +}: FileTreeItemComponentProps) => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (23)
CHANGELOG.md
(1 hunks)packages/backend/src/utils.ts
(1 hunks)packages/web/package.json
(1 hunks)packages/web/src/app/[domain]/browse/[...path]/components/codePreviewPanel.tsx
(1 hunks)packages/web/src/app/[domain]/browse/[...path]/components/pureCodePreviewPanel.tsx
(1 hunks)packages/web/src/app/[domain]/browse/[...path]/components/treePreviewPanel.tsx
(1 hunks)packages/web/src/app/[domain]/browse/[...path]/page.tsx
(1 hunks)packages/web/src/app/[domain]/browse/browseStateProvider.tsx
(2 hunks)packages/web/src/app/[domain]/browse/components/bottomPanel.tsx
(2 hunks)packages/web/src/app/[domain]/browse/hooks/useBrowseParams.ts
(1 hunks)packages/web/src/app/[domain]/browse/layout.tsx
(1 hunks)packages/web/src/app/[domain]/components/fileHeader.tsx
(0 hunks)packages/web/src/app/[domain]/components/pathHeader.tsx
(1 hunks)packages/web/src/app/[domain]/search/components/searchResultsPanel/fileMatchContainer.tsx
(2 hunks)packages/web/src/app/[domain]/search/page.tsx
(1 hunks)packages/web/src/ee/features/codeNav/components/exploreMenu/referenceList.tsx
(6 hunks)packages/web/src/features/fileTree/actions.ts
(1 hunks)packages/web/src/features/fileTree/components/fileTreeItemComponent.tsx
(1 hunks)packages/web/src/features/fileTree/components/fileTreePanel.tsx
(1 hunks)packages/web/src/features/fileTree/components/pureFileTreePanel.tsx
(1 hunks)packages/web/src/hooks/usePrefetchFileSource.ts
(1 hunks)packages/web/src/hooks/usePrefetchFolderContents.ts
(1 hunks)packages/web/src/lib/newsData.ts
(1 hunks)
💤 Files with no reviewable changes (1)
- packages/web/src/app/[domain]/components/fileHeader.tsx
✅ Files skipped from review due to trivial changes (3)
- packages/backend/src/utils.ts
- packages/web/src/app/[domain]/browse/[...path]/components/treePreviewPanel.tsx
- packages/web/src/lib/newsData.ts
🚧 Files skipped from review as they are similar to previous changes (15)
- packages/web/src/app/[domain]/search/page.tsx
- packages/web/package.json
- CHANGELOG.md
- packages/web/src/app/[domain]/search/components/searchResultsPanel/fileMatchContainer.tsx
- packages/web/src/app/[domain]/browse/components/bottomPanel.tsx
- packages/web/src/hooks/usePrefetchFileSource.ts
- packages/web/src/app/[domain]/browse/hooks/useBrowseParams.ts
- packages/web/src/ee/features/codeNav/components/exploreMenu/referenceList.tsx
- packages/web/src/app/[domain]/browse/browseStateProvider.tsx
- packages/web/src/features/fileTree/components/pureFileTreePanel.tsx
- packages/web/src/features/fileTree/components/fileTreePanel.tsx
- packages/web/src/app/[domain]/components/pathHeader.tsx
- packages/web/src/features/fileTree/actions.ts
- packages/web/src/app/[domain]/browse/layout.tsx
- packages/web/src/hooks/usePrefetchFolderContents.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/web/src/features/fileTree/components/fileTreeItemComponent.tsx (1)
packages/web/src/features/fileTree/actions.ts (1)
FileTreeItem
(11-15)
🪛 Biome (1.9.4)
packages/web/src/app/[domain]/browse/[...path]/components/pureCodePreviewPanel.tsx
[error] 197-197: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/web/src/features/fileTree/components/fileTreeItemComponent.tsx
[error] 95-101: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (8)
packages/web/src/features/fileTree/components/fileTreeItemComponent.tsx (3)
1-10
: LGTM! Clean imports and proper client-side component setup.The imports are well-organized and all dependencies appear to be used appropriately in the component.
32-54
: Auto-scroll implementation looks solid with proper boundary logic.The scroll-into-view effect correctly handles the active state and includes custom boundary logic to only scroll when the element is completely hidden vertically. The dependency array is correct and the boundary function properly checks element positioning.
74-107
: Render logic is well-structured with good accessibility support.The component includes proper keyboard navigation (Enter key support), appropriate ARIA attributes with tabIndex, and responsive styling. The padding calculation for depth-based indentation is clean and the event handlers are properly bound.
🧰 Tools
🪛 Biome (1.9.4)
[error] 95-101: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment(lint/complexity/noUselessFragments)
packages/web/src/app/[domain]/browse/[...path]/page.tsx (1)
7-19
: Clean and well-structured component implementation.The component follows React best practices with clear separation of concerns and straightforward conditional rendering logic based on path type.
packages/web/src/app/[domain]/browse/[...path]/components/pureCodePreviewPanel.tsx (2)
21-27
: Well-defined interface for component props.The interface clearly defines all required props with appropriate types, making the component's API explicit and type-safe.
92-113
: Good use of useMemo for expensive extension array creation.The extensions array is properly memoized with appropriate dependencies, preventing unnecessary re-creation on every render.
packages/web/src/app/[domain]/browse/[...path]/components/codePreviewPanel.tsx (2)
17-28
: Excellent use of React Query for data fetching.The component properly uses React Query with appropriate query keys and error handling, providing a clean separation between data fetching and rendering logic.
90-96
: Clean integration with PureCodePreviewPanel.The component properly passes all necessary props to the pure component, including decoded source and proper fallback for revision name.
packages/web/src/features/fileTree/components/fileTreeItemComponent.tsx
Outdated
Show resolved
Hide resolved
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: 1
♻️ Duplicate comments (5)
packages/web/src/features/fileTree/actions.ts (5)
57-57
: Address the comma delimiter issue in git format string.Using comma as a delimiter in the
--format
string can cause parsing failures if file paths contain commas. This was flagged in previous reviews but remains unresolved.Replace with a safer delimiter like tab character:
- '--format=%(objecttype),%(path)', + '--format=%(objecttype)%09%(path)',And update the parsing logic accordingly:
- const [type, path] = line.split(','); + const [type, path] = line.split('\t');
66-72
: Add validation for git command output parsing.The parsing logic doesn't validate the output format, which could lead to runtime errors if the git command produces unexpected output.
Add validation before destructuring:
const flatList = lines.map(line => { - const [type, path] = line.split(','); + const [type, path] = line.split(','); + if (!type || !path) { + throw new Error(`Invalid git ls-tree output: ${line}`); + } return { type, path, } });
145-154
: Extract common parsing logic to reduce duplication.This parsing logic is nearly identical to the one in
getTree
. The duplication was flagged in previous reviews but remains unaddressed.Create a shared helper function and update both usages:
+const parseGitLsTreeLine = (line: string): { type: string; path: string; name?: string } => { + const [type, path] = line.split('\t'); // Use tab delimiter + if (!type || !path) { + throw new Error(`Invalid git ls-tree output: ${line}`); + } + return { type, path }; +}; - const contents: FileTreeItem[] = lines.map(line => { - const [type, path] = line.split(','); - const name = path.split('/').pop() ?? ''; - - return { - type, - path, - name, - } - }); + const contents: FileTreeItem[] = lines.map(line => { + const { type, path } = parseGitLsTreeLine(line); + const name = path.split('/').pop() ?? ''; + return { type, path, name }; + });
178-186
: Fix incorrect path assignment for intermediate directory nodes.When creating intermediate directory nodes, the path should be constructed from the parts up to the current level, not use the leaf item's full path. This was identified in previous reviews.
if (!next) { + const partialPath = parts.slice(0, i + 1).join('/'); next = { name: part, - path: item.path, + path: partialPath, type: nodeType, children: [], }; current.children.push(next); }
214-234
: Address the code duplication as noted in the TODO.The comment indicates this function is duplicated from the backend's
utils.ts
. Code duplication can lead to maintenance issues and inconsistencies.Since this is marked as a TODO, would you like me to create an issue to track moving this to a shared package? This would ensure consistency and reduce maintenance burden.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/web/src/features/fileTree/actions.ts
(1 hunks)packages/web/src/features/fileTree/components/fileTreeItemComponent.tsx
(1 hunks)packages/web/src/features/fileTree/components/fileTreePanel.tsx
(1 hunks)packages/web/src/features/fileTree/components/pureFileTreePanel.tsx
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/web/src/features/fileTree/components/fileTreeItemComponent.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/web/src/features/fileTree/components/fileTreePanel.tsx
- packages/web/src/features/fileTree/components/pureFileTreePanel.tsx
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
This feature adds a file explorer to Sourcebot:

Features
It's fast 🏎️
file-tree-1.mp4
Collapsible ⬅️
file-tree-2.mp4
Also, breadcrumbs 🍞
file-tree-3.mp4
Icons ✨
Closes #94
Closes #312
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Chores