Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 84 additions & 21 deletions src/components/RightSidebar/CodeReview/HunkViewer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,19 @@ import styled from "@emotion/styled";
import type { DiffHunk } from "@/types/review";
import { SelectableDiffRenderer } from "../../shared/DiffRenderer";
import { Tooltip, TooltipWrapper } from "../../Tooltip";
import { usePersistedState } from "@/hooks/usePersistedState";
import { getReviewExpandStateKey } from "@/constants/storage";
import { KEYBINDS, formatKeybind } from "@/utils/ui/keybinds";

interface HunkViewerProps {
hunk: DiffHunk;
hunkId: string;
workspaceId: string;
isSelected?: boolean;
isRead?: boolean;
onClick?: (e: React.MouseEvent<HTMLElement>) => void;
onToggleRead?: (e: React.MouseEvent<HTMLElement>) => void;
onRegisterToggleExpand?: (hunkId: string, toggleFn: () => void) => void;
onReviewNote?: (note: string) => void;
}

Expand All @@ -27,6 +32,12 @@ const HunkContainer = styled.div<{ isSelected: boolean; isRead: boolean }>`
cursor: pointer;
transition: all 0.2s ease;

/* Remove default focus ring - keyboard navigation uses isSelected state */
&:focus,
&:focus-visible {
outline: none;
}

${(props) =>
props.isRead &&
`
Expand Down Expand Up @@ -165,7 +176,17 @@ const ToggleReadButton = styled.button`
`;

export const HunkViewer = React.memo<HunkViewerProps>(
({ hunk, hunkId, isSelected, isRead = false, onClick, onToggleRead, onReviewNote }) => {
({
hunk,
hunkId,
workspaceId,
isSelected,
isRead = false,
onClick,
onToggleRead,
onRegisterToggleExpand,
onReviewNote,
}) => {
// Parse diff lines (memoized - only recompute if hunk.content changes)
// Must be done before state initialization to determine initial collapse state
const { lineCount, additions, deletions, isLargeHunk } = React.useMemo(() => {
Expand All @@ -179,23 +200,69 @@ export const HunkViewer = React.memo<HunkViewerProps>(
};
}, [hunk.content]);

// Collapse by default if marked as read OR if hunk has >200 lines
const [isExpanded, setIsExpanded] = useState(() => !isRead && !isLargeHunk);
// Persist manual expand/collapse state across remounts per workspace
// Maps hunkId -> isExpanded for user's manual preferences
// Enable listener to synchronize updates across all HunkViewer instances
const [expandStateMap, setExpandStateMap] = usePersistedState<Record<string, boolean>>(
getReviewExpandStateKey(workspaceId),
{},
{ listener: true }
);

// Auto-collapse when marked as read, auto-expand when unmarked (but respect large hunk threshold)
// Check if user has manually set expand state for this hunk
const hasManualState = hunkId in expandStateMap;
const manualExpandState = expandStateMap[hunkId];

// Determine initial expand state (priority: manual > read status > size)
const [isExpanded, setIsExpanded] = useState(() => {
if (hasManualState) {
return manualExpandState;
}
return !isRead && !isLargeHunk;
});

// Auto-collapse when marked as read, auto-expand when unmarked (unless user manually set)
React.useEffect(() => {
// Don't override manual expand/collapse choices
if (hasManualState) {
return;
}

if (isRead) {
setIsExpanded(false);
} else if (!isLargeHunk) {
setIsExpanded(true);
}
// Note: When unmarking as read, large hunks remain collapsed
}, [isRead, isLargeHunk]);
}, [isRead, isLargeHunk, hasManualState]);

const handleToggleExpand = (e: React.MouseEvent) => {
e.stopPropagation();
setIsExpanded(!isExpanded);
};
// Sync local state with persisted state when it changes
React.useEffect(() => {
if (hasManualState) {
setIsExpanded(manualExpandState);
}
}, [hasManualState, manualExpandState]);

const handleToggleExpand = React.useCallback(
(e?: React.MouseEvent) => {
e?.stopPropagation();
const newExpandState = !isExpanded;
setIsExpanded(newExpandState);
// Persist manual expand/collapse choice
setExpandStateMap((prev) => ({
...prev,
[hunkId]: newExpandState,
}));
},
[isExpanded, hunkId, setExpandStateMap]
);

// Register toggle method with parent component
React.useEffect(() => {
if (onRegisterToggleExpand) {
onRegisterToggleExpand(hunkId, handleToggleExpand);
}
}, [hunkId, onRegisterToggleExpand, handleToggleExpand]);

const handleToggleRead = (e: React.MouseEvent<HTMLButtonElement>) => {
e.stopPropagation();
Expand All @@ -214,13 +281,6 @@ export const HunkViewer = React.memo<HunkViewerProps>(
role="button"
tabIndex={0}
data-hunk-id={hunkId}
onKeyDown={(e) => {
if (e.key === "Enter" || e.key === " ") {
e.preventDefault();
// Cast to MouseEvent-like for onClick handler
onClick?.(e as unknown as React.MouseEvent<HTMLElement>);
}
}}
>
<HunkHeader>
{isRead && (
Expand All @@ -247,12 +307,12 @@ export const HunkViewer = React.memo<HunkViewerProps>(
<ToggleReadButton
data-hunk-id={hunkId}
onClick={handleToggleRead}
aria-label="Mark as read (m)"
aria-label={`Mark as read (${formatKeybind(KEYBINDS.TOGGLE_HUNK_READ)})`}
>
{isRead ? "â—‹" : "â—‰"}
</ToggleReadButton>
<Tooltip align="right" position="top">
Mark as read (m)
Mark as read ({formatKeybind(KEYBINDS.TOGGLE_HUNK_READ)})
</Tooltip>
</TooltipWrapper>
)}
Expand Down Expand Up @@ -283,12 +343,15 @@ export const HunkViewer = React.memo<HunkViewerProps>(
</HunkContent>
) : (
<CollapsedIndicator onClick={handleToggleExpand}>
{isRead && "Hunk marked as read. "}Click to expand ({lineCount} lines)
{isRead && "Hunk marked as read. "}Click to expand ({lineCount} lines) or press{" "}
{formatKeybind(KEYBINDS.TOGGLE_HUNK_COLLAPSE)}
</CollapsedIndicator>
)}

{isLargeHunk && isExpanded && !isPureRename && (
<CollapsedIndicator onClick={handleToggleExpand}>Click to collapse</CollapsedIndicator>
{hasManualState && isExpanded && !isPureRename && (
<CollapsedIndicator onClick={handleToggleExpand}>
Click here or press {formatKeybind(KEYBINDS.TOGGLE_HUNK_COLLAPSE)} to collapse
</CollapsedIndicator>
)}
</HunkContainer>
);
Expand Down
17 changes: 16 additions & 1 deletion src/components/RightSidebar/CodeReview/ReviewPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* Displays diff hunks for viewing changes in the workspace
*/

import React, { useState, useEffect, useMemo, useCallback } from "react";
import React, { useState, useEffect, useMemo, useCallback, useRef } from "react";
import styled from "@emotion/styled";
import { HunkViewer } from "./HunkViewer";
import { ReviewControls } from "./ReviewControls";
Expand Down Expand Up @@ -286,6 +286,8 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
const [refreshTrigger, setRefreshTrigger] = useState(0);
const [fileTree, setFileTree] = useState<FileTreeNode | null>(null);
const [commonPrefix, setCommonPrefix] = useState<string | null>(null);
// Map of hunkId -> toggle function for expand/collapse
const toggleExpandFnsRef = useRef<Map<string, () => void>>(new Map());

// Persist file filter per workspace
const [selectedFilePath, setSelectedFilePath] = usePersistedState<string | null>(
Expand Down Expand Up @@ -539,6 +541,10 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
[handleToggleRead]
);

const handleRegisterToggleExpand = useCallback((hunkId: string, toggleFn: () => void) => {
toggleExpandFnsRef.current.set(hunkId, toggleFn);
}, []);

// Calculate stats
const stats = useMemo(() => {
const total = hunks.length;
Expand Down Expand Up @@ -598,6 +604,13 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
// Toggle read state of selected hunk
e.preventDefault();
handleToggleRead(selectedHunkId);
} else if (matchesKeybind(e, KEYBINDS.TOGGLE_HUNK_COLLAPSE)) {
// Toggle expand/collapse state of selected hunk
e.preventDefault();
const toggleFn = toggleExpandFnsRef.current.get(selectedHunkId);
if (toggleFn) {
toggleFn();
}
}
};

Expand Down Expand Up @@ -698,10 +711,12 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
key={hunk.id}
hunk={hunk}
hunkId={hunk.id}
workspaceId={workspaceId}
isSelected={isSelected}
isRead={hunkIsRead}
onClick={handleHunkClick}
onToggleRead={handleHunkToggleRead}
onRegisterToggleExpand={handleRegisterToggleExpand}
onReviewNote={onReviewNote}
/>
);
Expand Down
62 changes: 49 additions & 13 deletions src/constants/storage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,23 +84,43 @@ export function getCompactContinueMessageKey(workspaceId: string): string {
return `compactContinueMessage:${workspaceId}`;
}

/**
* Get the localStorage key for hunk expand/collapse state in Review tab
* Stores user's manual expand/collapse preferences per hunk
* Format: "reviewExpandState:{workspaceId}"
*/
export function getReviewExpandStateKey(workspaceId: string): string {
return `reviewExpandState:${workspaceId}`;
}

/**
* List of workspace-scoped key functions that should be copied on fork and deleted on removal
* Note: Excludes ephemeral keys like getCompactContinueMessageKey
*/
const PERSISTENT_WORKSPACE_KEY_FUNCTIONS: Array<(workspaceId: string) => string> = [
getModelKey,
getInputKey,
getModeKey,
getThinkingLevelKey,
getAutoRetryKey,
getRetryStateKey,
getReviewExpandStateKey,
];

/**
* Additional ephemeral keys to delete on workspace removal (not copied on fork)
*/
const EPHEMERAL_WORKSPACE_KEY_FUNCTIONS: Array<(workspaceId: string) => string> = [
getCancelledCompactionKey,
getCompactContinueMessageKey,
];

/**
* Copy all workspace-specific localStorage keys from source to destination workspace
* This includes: model, input, mode, thinking level, auto-retry, retry state
* This includes: model, input, mode, thinking level, auto-retry, retry state, review expand state
*/
export function copyWorkspaceStorage(sourceWorkspaceId: string, destWorkspaceId: string): void {
// List of key-generating functions to copy
// Note: We deliberately skip getCompactContinueMessageKey as it's ephemeral
const keyFunctions: Array<(workspaceId: string) => string> = [
getModelKey,
getInputKey,
getModeKey,
getThinkingLevelKey,
getAutoRetryKey,
getRetryStateKey,
];

for (const getKey of keyFunctions) {
for (const getKey of PERSISTENT_WORKSPACE_KEY_FUNCTIONS) {
const sourceKey = getKey(sourceWorkspaceId);
const destKey = getKey(destWorkspaceId);
const value = localStorage.getItem(sourceKey);
Expand All @@ -109,3 +129,19 @@ export function copyWorkspaceStorage(sourceWorkspaceId: string, destWorkspaceId:
}
}
}

/**
* Delete all workspace-specific localStorage keys for a workspace
* Should be called when a workspace is deleted to prevent orphaned data
*/
export function deleteWorkspaceStorage(workspaceId: string): void {
const allKeyFunctions = [
...PERSISTENT_WORKSPACE_KEY_FUNCTIONS,
...EPHEMERAL_WORKSPACE_KEY_FUNCTIONS,
];

for (const getKey of allKeyFunctions) {
const key = getKey(workspaceId);
localStorage.removeItem(key);
}
}
4 changes: 4 additions & 0 deletions src/hooks/useWorkspaceManagement.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { useState, useEffect, useCallback } from "react";
import type { FrontendWorkspaceMetadata } from "@/types/workspace";
import type { WorkspaceSelection } from "@/components/ProjectSidebar";
import type { ProjectConfig } from "@/config";
import { deleteWorkspaceStorage } from "@/constants/storage";

interface UseWorkspaceManagementProps {
selectedWorkspace: WorkspaceSelection | null;
Expand Down Expand Up @@ -118,6 +119,9 @@ export function useWorkspaceManagement({
): Promise<{ success: boolean; error?: string }> => {
const result = await window.api.workspace.remove(workspaceId, options);
if (result.success) {
// Clean up workspace-specific localStorage keys
deleteWorkspaceStorage(workspaceId);

// Backend has already updated the config - reload projects to get updated state
const projectsList = await window.api.projects.list();
const loadedProjects = new Map<string, ProjectConfig>(projectsList);
Expand Down
14 changes: 12 additions & 2 deletions src/utils/ui/keybinds.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,15 @@ export function formatKeybind(keybind: Keybind): string {
if (keybind.meta) parts.push("Meta");
}

// Add the key (capitalize single letters)
const key = keybind.key.length === 1 ? keybind.key.toUpperCase() : keybind.key;
// Add the key (handle special cases, then capitalize single letters)
let key: string;
if (keybind.key === " ") {
key = "Space";
} else if (keybind.key.length === 1) {
key = keybind.key.toUpperCase();
} else {
key = keybind.key;
}
parts.push(key);

return isMac() ? parts.join("\u00B7") : parts.join("+"); // · on Mac, + elsewhere
Expand Down Expand Up @@ -257,4 +264,7 @@ export const KEYBINDS = {

/** Mark selected hunk as read/unread in Code Review panel */
TOGGLE_HUNK_READ: { key: "m" },

/** Toggle hunk expand/collapse in Code Review panel */
TOGGLE_HUNK_COLLAPSE: { key: " " },
} as const;