From 65583e4bb424efb44315aae16978e2052cdcb15a Mon Sep 17 00:00:00 2001 From: Ammar Date: Wed, 22 Oct 2025 13:25:09 -0500 Subject: [PATCH] =?UTF-8?q?=F0=9F=A4=96=20Review:=20Move=20Files=20Changed?= =?UTF-8?q?=20to=20top=20and=20remove=20prefix=20subtraction?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Position file tree on top/left instead of bottom/right - Remove common prefix extraction logic - Show full directory structure without automatic prefix subtraction - Users can now collapse/expand the entire tree hierarchy In wide layout, file tree now appears on the left side. In narrow layout, file tree appears at the top. Changed border from border-left to border-right to match new layout. Net -43 LoC (55 deletions, 12 insertions) --- .../RightSidebar/CodeReview/FileTree.tsx | 39 +-- .../RightSidebar/CodeReview/ReviewPanel.tsx | 314 +++++++++--------- 2 files changed, 151 insertions(+), 202 deletions(-) diff --git a/src/components/RightSidebar/CodeReview/FileTree.tsx b/src/components/RightSidebar/CodeReview/FileTree.tsx index bec1c2435..7a18aa979 100644 --- a/src/components/RightSidebar/CodeReview/FileTree.tsx +++ b/src/components/RightSidebar/CodeReview/FileTree.tsx @@ -132,15 +132,6 @@ const TreeHeader = styled.div` gap: 8px; `; -const CommonPrefix = styled.div` - padding: 6px 12px; - background: #1e1e1e; - border-bottom: 1px solid #3e3e42; - font-size: 11px; - color: #888; - font-family: var(--font-monospace); -`; - const EmptyState = styled.div` padding: 20px; color: #888; @@ -194,7 +185,6 @@ const TreeNodeContent: React.FC<{ depth: number; selectedPath: string | null; onSelectFile: (path: string | null) => void; - commonPrefix: string | null; getFileReadStatus?: (filePath: string) => { total: number; read: number } | null; expandStateMap: Record; setExpandStateMap: ( @@ -205,7 +195,6 @@ const TreeNodeContent: React.FC<{ depth, selectedPath, onSelectFile, - commonPrefix, getFileReadStatus, expandStateMap, setExpandStateMap, @@ -317,7 +306,6 @@ const TreeNodeContent: React.FC<{ depth={depth + 1} selectedPath={selectedPath} onSelectFile={onSelectFile} - commonPrefix={commonPrefix} getFileReadStatus={getFileReadStatus} expandStateMap={expandStateMap} setExpandStateMap={setExpandStateMap} @@ -332,7 +320,6 @@ interface FileTreeExternalProps { selectedPath: string | null; onSelectFile: (path: string | null) => void; isLoading?: boolean; - commonPrefix?: string | null; getFileReadStatus?: (filePath: string) => { total: number; read: number } | null; workspaceId: string; } @@ -342,7 +329,6 @@ export const FileTree: React.FC = ({ selectedPath, onSelectFile, isLoading = false, - commonPrefix = null, getFileReadStatus, workspaceId, }) => { @@ -353,42 +339,23 @@ export const FileTree: React.FC = ({ { listener: true } ); - // Find the node at the common prefix path to start rendering from - const startNode = React.useMemo(() => { - if (!commonPrefix || !root) return root; - - // Navigate to the node at the common prefix path - const parts = commonPrefix.split("/"); - let current = root; - - for (const part of parts) { - const child = current.children.find((c) => c.name === part); - if (!child) return root; // Fallback if path not found - current = child; - } - - return current; - }, [root, commonPrefix]); - return ( <> Files Changed {selectedPath && onSelectFile(null)}>Clear filter} - {commonPrefix && {commonPrefix}/} - {isLoading && !startNode ? ( + {isLoading && !root ? ( Loading file tree... - ) : startNode ? ( - startNode.children.map((child) => ( + ) : root ? ( + root.children.map((child) => ( ` `; const HunkList = styled.div` - flex: 1; - min-height: 0; - overflow-y: auto; - padding: 12px; + display: flex; + flex-direction: column; + gap: 12px; `; const FileTreeSection = styled.div` - /* Default: Wide layout - fixed width on right side */ - width: 300px; - flex-shrink: 0; - border-left: 1px solid #3e3e42; + /* Part of scrollable content, not sticky */ + width: 100%; + flex: 0 0 auto; /* Size based on content */ display: flex; flex-direction: column; overflow: hidden; - min-height: 0; - order: 2; /* Come after HunksSection in wide mode */ - - /* Narrow layout: full width, grow to fit full tree, above hunks */ - @container review-panel (max-width: 800px) { - width: 100%; - height: auto; /* Let it grow to show full tree */ - flex: 0 0 auto; /* Fixed size based on content */ - border-left: none; - border-bottom: 1px solid #3e3e42; - order: 0; /* Come before HunksSection in narrow mode */ - } + border-bottom: 1px solid #3e3e42; `; const EmptyState = styled.div` @@ -426,7 +409,6 @@ export const ReviewPanel: React.FC = ({ const [isPanelFocused, setIsPanelFocused] = useState(false); const [refreshTrigger, setRefreshTrigger] = useState(0); const [fileTree, setFileTree] = useState(null); - const [commonPrefix, setCommonPrefix] = useState(null); // Map of hunkId -> toggle function for expand/collapse const toggleExpandFnsRef = useRef void>>(new Map()); @@ -509,13 +491,9 @@ export const ReviewPanel: React.FC = ({ const numstatOutput = numstatResult.data.output ?? ""; const fileStats = parseNumstat(numstatOutput); - // Extract common prefix for display (don't modify paths) - const prefix = extractCommonPrefix(fileStats); - // Build tree with original paths (needed for git commands) const tree = buildFileTree(fileStats); setFileTree(tree); - setCommonPrefix(prefix); } } catch (err) { console.error("Failed to load file tree:", err); @@ -850,133 +828,137 @@ export const ReviewPanel: React.FC = ({ Loading diff... ) : ( - - {truncationWarning && {truncationWarning}} - - - - setSearchState({ ...searchState, input: e.target.value })} - /> - - - setSearchState({ ...searchState, useRegex: !searchState.useRegex }) - } - > - .* - - - {searchState.useRegex ? "Using regex search" : "Using substring search"} - - - - - setSearchState({ ...searchState, matchCase: !searchState.matchCase }) - } - > - Aa - - - {searchState.matchCase - ? "Match case (case-sensitive)" - : "Ignore case (case-insensitive)"} - - - - - - - {hunks.length === 0 ? ( - - No changes found - - No changes found for the selected diff base. -
- Try selecting a different base or make some changes. - - {diagnosticInfo && ( - - Show diagnostic info - - - Command: - {diagnosticInfo.command} - - - Output size: - - {diagnosticInfo.outputLength.toLocaleString()} bytes - - - - Files parsed: - {diagnosticInfo.fileDiffCount} - - - Hunks extracted: - {diagnosticInfo.hunkCount} - - - - )} - - ) : filteredHunks.length === 0 ? ( - - - {debouncedSearchTerm.trim() - ? `No hunks match "${debouncedSearchTerm}". Try a different search term.` - : selectedFilePath - ? `No hunks in ${selectedFilePath}. Try selecting a different file.` - : "No hunks match the current filters. Try adjusting your filter settings."} - - - ) : ( - filteredHunks.map((hunk) => { - const isSelected = hunk.id === selectedHunkId; - const hunkIsRead = isRead(hunk.id); - - return ( - - ); - }) - )} - - - - {/* FileTree positioning handled by CSS order property */} - {(fileTree ?? isLoadingTree) && ( - - {truncationWarning}} + + {/* Search bar - always visible at top, not sticky */} + + + setSearchState({ ...searchState, input: e.target.value })} /> - - )} + + + setSearchState({ ...searchState, useRegex: !searchState.useRegex }) + } + > + .* + + + {searchState.useRegex ? "Using regex search" : "Using substring search"} + + + + + setSearchState({ ...searchState, matchCase: !searchState.matchCase }) + } + > + Aa + + + {searchState.matchCase + ? "Match case (case-sensitive)" + : "Ignore case (case-insensitive)"} + + + + + + {/* Single scrollable area containing both file tree and hunks */} + + {/* FileTree at the top */} + {(fileTree ?? isLoadingTree) && ( + + + + )} + + {/* Hunks below the file tree */} + + + {hunks.length === 0 ? ( + + No changes found + + No changes found for the selected diff base. +
+ Try selecting a different base or make some changes. + + {diagnosticInfo && ( + + Show diagnostic info + + + Command: + {diagnosticInfo.command} + + + Output size: + + {diagnosticInfo.outputLength.toLocaleString()} bytes + + + + Files parsed: + {diagnosticInfo.fileDiffCount} + + + Hunks extracted: + {diagnosticInfo.hunkCount} + + + + )} + + ) : filteredHunks.length === 0 ? ( + + + {debouncedSearchTerm.trim() + ? `No hunks match "${debouncedSearchTerm}". Try a different search term.` + : selectedFilePath + ? `No hunks in ${selectedFilePath}. Try selecting a different file.` + : "No hunks match the current filters. Try adjusting your filter settings."} + + + ) : ( + filteredHunks.map((hunk) => { + const isSelected = hunk.id === selectedHunkId; + const hunkIsRead = isRead(hunk.id); + + return ( + + ); + }) + )} + + + )}