Skip to content

Commit efd2f14

Browse files
authored
🤖 Preserve hunk expand/collapse state and centralize keybinds (#332)
Remembers users' manual expand/collapse choices in Review tab, centralizes keybinds, and fixes Space key to operate on selected hunk. ## Problems 1. When users manually expand or collapse hunks, their choices are lost on remount (e.g., switching workspaces and back) 2. Keybind strings hardcoded in components instead of centralized 3. Space key operated on focused (blue border) hunk instead of selected (yellow border) hunk during j/k navigation ## Solutions ### Persistent Expand/Collapse State - Store per-workspace hunk expand state in localStorage via `usePersistedState` - Key: `reviewExpandState:{workspaceId}` → maps hunkId to isExpanded boolean - Manual choices override automatic collapse logic (read status, size threshold) - Storage copies on workspace fork, clears on workspace deletion - Refactored with `PERSISTENT_WORKSPACE_KEY_FUNCTIONS` array for DRY ### Centralized Keybinds - Added `TOGGLE_HUNK_COLLAPSE` to `KEYBINDS` registry in `src/utils/ui/keybinds.ts` - All keybind displays use `formatKeybind()` for dynamic OS-appropriate formatting - Added special handling for space key to display as "Space" (not invisible whitespace) ### Fixed Space Key Behavior - Removed confusing blue focus ring from HunkContainer - Moved Space key handler from HunkViewer to ReviewPanel level - Space now operates on selected (yellow border) hunk, not focused element - All keyboard shortcuts (j/k/m/Space) consistently operate on same selection state ## Behavior **Priority order for expand state:** 1. **Manual state** (user clicked expand/collapse or pressed Space) → persisted 2. **Read status** (marked as read) → collapsed 3. **Size threshold** (>200 lines) → collapsed 4. **Default** → expanded **Keybind Display:** - Expand indicator: "Click to expand (N lines) or press Space" - Collapse indicator: "Click here or press Space to collapse" - Mark as read: "Mark as read (m)" - All keybinds show OS-appropriate format (e.g., ⌘ on Mac, Ctrl+ on Windows/Linux) **Keyboard Navigation:** - `j/k` - Navigate between hunks (updates yellow border selection) - `m` - Toggle read state on selected hunk - `Space` - Toggle expand/collapse on selected hunk - All operations work on the yellow-bordered (selected) hunk _Generated with `cmux`_
1 parent 0a7e200 commit efd2f14

File tree

5 files changed

+165
-37
lines changed

5 files changed

+165
-37
lines changed

src/components/RightSidebar/CodeReview/HunkViewer.tsx

Lines changed: 84 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,19 @@ import styled from "@emotion/styled";
77
import type { DiffHunk } from "@/types/review";
88
import { SelectableDiffRenderer } from "../../shared/DiffRenderer";
99
import { Tooltip, TooltipWrapper } from "../../Tooltip";
10+
import { usePersistedState } from "@/hooks/usePersistedState";
11+
import { getReviewExpandStateKey } from "@/constants/storage";
12+
import { KEYBINDS, formatKeybind } from "@/utils/ui/keybinds";
1013

1114
interface HunkViewerProps {
1215
hunk: DiffHunk;
1316
hunkId: string;
17+
workspaceId: string;
1418
isSelected?: boolean;
1519
isRead?: boolean;
1620
onClick?: (e: React.MouseEvent<HTMLElement>) => void;
1721
onToggleRead?: (e: React.MouseEvent<HTMLElement>) => void;
22+
onRegisterToggleExpand?: (hunkId: string, toggleFn: () => void) => void;
1823
onReviewNote?: (note: string) => void;
1924
}
2025

@@ -27,6 +32,12 @@ const HunkContainer = styled.div<{ isSelected: boolean; isRead: boolean }>`
2732
cursor: pointer;
2833
transition: all 0.2s ease;
2934
35+
/* Remove default focus ring - keyboard navigation uses isSelected state */
36+
&:focus,
37+
&:focus-visible {
38+
outline: none;
39+
}
40+
3041
${(props) =>
3142
props.isRead &&
3243
`
@@ -165,7 +176,17 @@ const ToggleReadButton = styled.button`
165176
`;
166177

167178
export const HunkViewer = React.memo<HunkViewerProps>(
168-
({ hunk, hunkId, isSelected, isRead = false, onClick, onToggleRead, onReviewNote }) => {
179+
({
180+
hunk,
181+
hunkId,
182+
workspaceId,
183+
isSelected,
184+
isRead = false,
185+
onClick,
186+
onToggleRead,
187+
onRegisterToggleExpand,
188+
onReviewNote,
189+
}) => {
169190
// Parse diff lines (memoized - only recompute if hunk.content changes)
170191
// Must be done before state initialization to determine initial collapse state
171192
const { lineCount, additions, deletions, isLargeHunk } = React.useMemo(() => {
@@ -179,23 +200,69 @@ export const HunkViewer = React.memo<HunkViewerProps>(
179200
};
180201
}, [hunk.content]);
181202

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

185-
// Auto-collapse when marked as read, auto-expand when unmarked (but respect large hunk threshold)
212+
// Check if user has manually set expand state for this hunk
213+
const hasManualState = hunkId in expandStateMap;
214+
const manualExpandState = expandStateMap[hunkId];
215+
216+
// Determine initial expand state (priority: manual > read status > size)
217+
const [isExpanded, setIsExpanded] = useState(() => {
218+
if (hasManualState) {
219+
return manualExpandState;
220+
}
221+
return !isRead && !isLargeHunk;
222+
});
223+
224+
// Auto-collapse when marked as read, auto-expand when unmarked (unless user manually set)
186225
React.useEffect(() => {
226+
// Don't override manual expand/collapse choices
227+
if (hasManualState) {
228+
return;
229+
}
230+
187231
if (isRead) {
188232
setIsExpanded(false);
189233
} else if (!isLargeHunk) {
190234
setIsExpanded(true);
191235
}
192236
// Note: When unmarking as read, large hunks remain collapsed
193-
}, [isRead, isLargeHunk]);
237+
}, [isRead, isLargeHunk, hasManualState]);
194238

195-
const handleToggleExpand = (e: React.MouseEvent) => {
196-
e.stopPropagation();
197-
setIsExpanded(!isExpanded);
198-
};
239+
// Sync local state with persisted state when it changes
240+
React.useEffect(() => {
241+
if (hasManualState) {
242+
setIsExpanded(manualExpandState);
243+
}
244+
}, [hasManualState, manualExpandState]);
245+
246+
const handleToggleExpand = React.useCallback(
247+
(e?: React.MouseEvent) => {
248+
e?.stopPropagation();
249+
const newExpandState = !isExpanded;
250+
setIsExpanded(newExpandState);
251+
// Persist manual expand/collapse choice
252+
setExpandStateMap((prev) => ({
253+
...prev,
254+
[hunkId]: newExpandState,
255+
}));
256+
},
257+
[isExpanded, hunkId, setExpandStateMap]
258+
);
259+
260+
// Register toggle method with parent component
261+
React.useEffect(() => {
262+
if (onRegisterToggleExpand) {
263+
onRegisterToggleExpand(hunkId, handleToggleExpand);
264+
}
265+
}, [hunkId, onRegisterToggleExpand, handleToggleExpand]);
199266

200267
const handleToggleRead = (e: React.MouseEvent<HTMLButtonElement>) => {
201268
e.stopPropagation();
@@ -214,13 +281,6 @@ export const HunkViewer = React.memo<HunkViewerProps>(
214281
role="button"
215282
tabIndex={0}
216283
data-hunk-id={hunkId}
217-
onKeyDown={(e) => {
218-
if (e.key === "Enter" || e.key === " ") {
219-
e.preventDefault();
220-
// Cast to MouseEvent-like for onClick handler
221-
onClick?.(e as unknown as React.MouseEvent<HTMLElement>);
222-
}
223-
}}
224284
>
225285
<HunkHeader>
226286
{isRead && (
@@ -247,12 +307,12 @@ export const HunkViewer = React.memo<HunkViewerProps>(
247307
<ToggleReadButton
248308
data-hunk-id={hunkId}
249309
onClick={handleToggleRead}
250-
aria-label="Mark as read (m)"
310+
aria-label={`Mark as read (${formatKeybind(KEYBINDS.TOGGLE_HUNK_READ)})`}
251311
>
252312
{isRead ? "○" : "◉"}
253313
</ToggleReadButton>
254314
<Tooltip align="right" position="top">
255-
Mark as read (m)
315+
Mark as read ({formatKeybind(KEYBINDS.TOGGLE_HUNK_READ)})
256316
</Tooltip>
257317
</TooltipWrapper>
258318
)}
@@ -283,12 +343,15 @@ export const HunkViewer = React.memo<HunkViewerProps>(
283343
</HunkContent>
284344
) : (
285345
<CollapsedIndicator onClick={handleToggleExpand}>
286-
{isRead && "Hunk marked as read. "}Click to expand ({lineCount} lines)
346+
{isRead && "Hunk marked as read. "}Click to expand ({lineCount} lines) or press{" "}
347+
{formatKeybind(KEYBINDS.TOGGLE_HUNK_COLLAPSE)}
287348
</CollapsedIndicator>
288349
)}
289350

290-
{isLargeHunk && isExpanded && !isPureRename && (
291-
<CollapsedIndicator onClick={handleToggleExpand}>Click to collapse</CollapsedIndicator>
351+
{hasManualState && isExpanded && !isPureRename && (
352+
<CollapsedIndicator onClick={handleToggleExpand}>
353+
Click here or press {formatKeybind(KEYBINDS.TOGGLE_HUNK_COLLAPSE)} to collapse
354+
</CollapsedIndicator>
292355
)}
293356
</HunkContainer>
294357
);

src/components/RightSidebar/CodeReview/ReviewPanel.tsx

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* Displays diff hunks for viewing changes in the workspace
44
*/
55

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

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

544+
const handleRegisterToggleExpand = useCallback((hunkId: string, toggleFn: () => void) => {
545+
toggleExpandFnsRef.current.set(hunkId, toggleFn);
546+
}, []);
547+
542548
// Calculate stats
543549
const stats = useMemo(() => {
544550
const total = hunks.length;
@@ -598,6 +604,13 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
598604
// Toggle read state of selected hunk
599605
e.preventDefault();
600606
handleToggleRead(selectedHunkId);
607+
} else if (matchesKeybind(e, KEYBINDS.TOGGLE_HUNK_COLLAPSE)) {
608+
// Toggle expand/collapse state of selected hunk
609+
e.preventDefault();
610+
const toggleFn = toggleExpandFnsRef.current.get(selectedHunkId);
611+
if (toggleFn) {
612+
toggleFn();
613+
}
601614
}
602615
};
603616

@@ -698,10 +711,12 @@ export const ReviewPanel: React.FC<ReviewPanelProps> = ({
698711
key={hunk.id}
699712
hunk={hunk}
700713
hunkId={hunk.id}
714+
workspaceId={workspaceId}
701715
isSelected={isSelected}
702716
isRead={hunkIsRead}
703717
onClick={handleHunkClick}
704718
onToggleRead={handleHunkToggleRead}
719+
onRegisterToggleExpand={handleRegisterToggleExpand}
705720
onReviewNote={onReviewNote}
706721
/>
707722
);

src/constants/storage.ts

Lines changed: 49 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -84,23 +84,43 @@ export function getCompactContinueMessageKey(workspaceId: string): string {
8484
return `compactContinueMessage:${workspaceId}`;
8585
}
8686

87+
/**
88+
* Get the localStorage key for hunk expand/collapse state in Review tab
89+
* Stores user's manual expand/collapse preferences per hunk
90+
* Format: "reviewExpandState:{workspaceId}"
91+
*/
92+
export function getReviewExpandStateKey(workspaceId: string): string {
93+
return `reviewExpandState:${workspaceId}`;
94+
}
95+
96+
/**
97+
* List of workspace-scoped key functions that should be copied on fork and deleted on removal
98+
* Note: Excludes ephemeral keys like getCompactContinueMessageKey
99+
*/
100+
const PERSISTENT_WORKSPACE_KEY_FUNCTIONS: Array<(workspaceId: string) => string> = [
101+
getModelKey,
102+
getInputKey,
103+
getModeKey,
104+
getThinkingLevelKey,
105+
getAutoRetryKey,
106+
getRetryStateKey,
107+
getReviewExpandStateKey,
108+
];
109+
110+
/**
111+
* Additional ephemeral keys to delete on workspace removal (not copied on fork)
112+
*/
113+
const EPHEMERAL_WORKSPACE_KEY_FUNCTIONS: Array<(workspaceId: string) => string> = [
114+
getCancelledCompactionKey,
115+
getCompactContinueMessageKey,
116+
];
117+
87118
/**
88119
* Copy all workspace-specific localStorage keys from source to destination workspace
89-
* This includes: model, input, mode, thinking level, auto-retry, retry state
120+
* This includes: model, input, mode, thinking level, auto-retry, retry state, review expand state
90121
*/
91122
export function copyWorkspaceStorage(sourceWorkspaceId: string, destWorkspaceId: string): void {
92-
// List of key-generating functions to copy
93-
// Note: We deliberately skip getCompactContinueMessageKey as it's ephemeral
94-
const keyFunctions: Array<(workspaceId: string) => string> = [
95-
getModelKey,
96-
getInputKey,
97-
getModeKey,
98-
getThinkingLevelKey,
99-
getAutoRetryKey,
100-
getRetryStateKey,
101-
];
102-
103-
for (const getKey of keyFunctions) {
123+
for (const getKey of PERSISTENT_WORKSPACE_KEY_FUNCTIONS) {
104124
const sourceKey = getKey(sourceWorkspaceId);
105125
const destKey = getKey(destWorkspaceId);
106126
const value = localStorage.getItem(sourceKey);
@@ -109,3 +129,19 @@ export function copyWorkspaceStorage(sourceWorkspaceId: string, destWorkspaceId:
109129
}
110130
}
111131
}
132+
133+
/**
134+
* Delete all workspace-specific localStorage keys for a workspace
135+
* Should be called when a workspace is deleted to prevent orphaned data
136+
*/
137+
export function deleteWorkspaceStorage(workspaceId: string): void {
138+
const allKeyFunctions = [
139+
...PERSISTENT_WORKSPACE_KEY_FUNCTIONS,
140+
...EPHEMERAL_WORKSPACE_KEY_FUNCTIONS,
141+
];
142+
143+
for (const getKey of allKeyFunctions) {
144+
const key = getKey(workspaceId);
145+
localStorage.removeItem(key);
146+
}
147+
}

src/hooks/useWorkspaceManagement.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { useState, useEffect, useCallback } from "react";
22
import type { FrontendWorkspaceMetadata } from "@/types/workspace";
33
import type { WorkspaceSelection } from "@/components/ProjectSidebar";
44
import type { ProjectConfig } from "@/config";
5+
import { deleteWorkspaceStorage } from "@/constants/storage";
56

67
interface UseWorkspaceManagementProps {
78
selectedWorkspace: WorkspaceSelection | null;
@@ -118,6 +119,9 @@ export function useWorkspaceManagement({
118119
): Promise<{ success: boolean; error?: string }> => {
119120
const result = await window.api.workspace.remove(workspaceId, options);
120121
if (result.success) {
122+
// Clean up workspace-specific localStorage keys
123+
deleteWorkspaceStorage(workspaceId);
124+
121125
// Backend has already updated the config - reload projects to get updated state
122126
const projectsList = await window.api.projects.list();
123127
const loadedProjects = new Map<string, ProjectConfig>(projectsList);

src/utils/ui/keybinds.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -163,8 +163,15 @@ export function formatKeybind(keybind: Keybind): string {
163163
if (keybind.meta) parts.push("Meta");
164164
}
165165

166-
// Add the key (capitalize single letters)
167-
const key = keybind.key.length === 1 ? keybind.key.toUpperCase() : keybind.key;
166+
// Add the key (handle special cases, then capitalize single letters)
167+
let key: string;
168+
if (keybind.key === " ") {
169+
key = "Space";
170+
} else if (keybind.key.length === 1) {
171+
key = keybind.key.toUpperCase();
172+
} else {
173+
key = keybind.key;
174+
}
168175
parts.push(key);
169176

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

258265
/** Mark selected hunk as read/unread in Code Review panel */
259266
TOGGLE_HUNK_READ: { key: "m" },
267+
268+
/** Toggle hunk expand/collapse in Code Review panel */
269+
TOGGLE_HUNK_COLLAPSE: { key: " " },
260270
} as const;

0 commit comments

Comments
 (0)