Skip to content

Commit 52cfd23

Browse files
authored
feat(sig): Report list toolbar as core control (#1532)
## Problem The inbox list toolbar introduced in #1513 added controls _only_ for bulk operations. But the same controls have been missing for _individual_ reports too. In the Cloud version, we had them under a dropdown in the report pane itself. I considered doing the same here, but there's no point when we have a great toolbar - the toolbar should be the individual report controls as well. At the same time, checkboxes take away from the content when shown always - and we don't need them by default for _most_ user interactions here. (Ofc will validate this, but at least I think that's a safe assumption for now that the individual report level is the primary one.) ## Changes Making the report list toolbar always-on, with intuitive logic for selection state synced with the currently-viewed-report state. Unifies the selection state, so that "currently viewed" and "selected" are the same thing. Checkboxes only show up when multiple reports are selected. For multi-select, you can use the "Select all" checkbox or standard desktop Shift/Cmd+click patterns. See in action: <img src="https://res.cloudinary.com/dmukukwp6/image/upload/Clean_Shot_2026_04_08_at_11_58_59_c89bf3657b.gif"> <!-- If you're an agent, only list tests you actually ran. -->
1 parent 25ee01c commit 52cfd23

10 files changed

Lines changed: 633 additions & 231 deletions

File tree

apps/code/src/main/services/git/create-pr-saga.ts

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -72,21 +72,21 @@ export class CreatePrSaga extends Saga<CreatePrSagaInput, CreatePrSagaOutput> {
7272
let { commitMessage, prTitle, prBody } = input;
7373

7474
if (input.branchName) {
75+
const branchName = input.branchName;
7576
const currentBranch = await this.readOnlyStep("get-original-branch", () =>
7677
this.deps.getCurrentBranch(directoryPath),
7778
);
7879

7980
// on retry, do not attempt to re-create the branch
80-
if (currentBranch !== input.branchName) {
81+
if (currentBranch !== branchName) {
8182
this.deps.onProgress(
8283
"creating-branch",
83-
`Creating branch ${input.branchName}...`,
84+
`Creating branch ${branchName}...`,
8485
);
8586

8687
await this.step({
8788
name: "creating-branch",
88-
execute: () =>
89-
this.deps.createBranch(directoryPath, input.branchName!),
89+
execute: () => this.deps.createBranch(directoryPath, branchName),
9090
rollback: async () => {
9191
if (currentBranch) {
9292
await this.deps.checkoutBranch(directoryPath, currentBranch);
@@ -120,6 +120,8 @@ export class CreatePrSaga extends Saga<CreatePrSagaInput, CreatePrSagaOutput> {
120120
throw new Error("Commit message is required.");
121121
}
122122

123+
const finalCommitMessage = commitMessage;
124+
123125
this.deps.onProgress("committing", "Committing changes...");
124126

125127
const preCommitSha = await this.readOnlyStep("get-pre-commit-sha", () =>
@@ -129,10 +131,14 @@ export class CreatePrSaga extends Saga<CreatePrSagaInput, CreatePrSagaOutput> {
129131
await this.step({
130132
name: "committing",
131133
execute: async () => {
132-
const result = await this.deps.commit(directoryPath, commitMessage!, {
133-
stagedOnly: input.stagedOnly,
134-
taskId: input.taskId,
135-
});
134+
const result = await this.deps.commit(
135+
directoryPath,
136+
finalCommitMessage,
137+
{
138+
stagedOnly: input.stagedOnly,
139+
taskId: input.taskId,
140+
},
141+
);
136142
if (!result.success) throw new Error(result.message);
137143
return result;
138144
},

apps/code/src/renderer/features/inbox/components/InboxSignalsTab.tsx

Lines changed: 116 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ import { Box, Flex, ScrollArea } from "@radix-ui/themes";
2626
import type { SignalReportsQueryParams } from "@shared/types";
2727
import { useNavigationStore } from "@stores/navigationStore";
2828
import { useRendererWindowFocusStore } from "@stores/rendererWindowFocusStore";
29-
import { useCallback, useEffect, useMemo, useRef, useState } from "react";
29+
import { useCallback, useEffect, useMemo, useRef } from "react";
30+
import { MultiSelectStack } from "./detail/MultiSelectStack";
3031
import { ReportDetailPane } from "./detail/ReportDetailPane";
3132
import { ReportListPane } from "./list/ReportListPane";
3233
import { SignalsToolbar } from "./list/SignalsToolbar";
@@ -134,39 +135,78 @@ export function InboxSignalsTab() {
134135
[allReports],
135136
);
136137

137-
// ── Selection state ─────────────────────────────────────────────────────
138-
const [selectedReportId, setSelectedReportId] = useState<string | null>(null);
138+
// ── Selection state (unified — store is single source of truth) ─────────
139139
const selectedReportIds = useInboxReportSelectionStore(
140-
(s) => s.selectedReportIds ?? [],
140+
(s) => s.selectedReportIds,
141+
);
142+
const setSelectedReportIds = useInboxReportSelectionStore(
143+
(s) => s.setSelectedReportIds,
141144
);
142145
const toggleReportSelection = useInboxReportSelectionStore(
143146
(s) => s.toggleReportSelection,
144147
);
148+
const selectRange = useInboxReportSelectionStore((s) => s.selectRange);
145149
const pruneSelection = useInboxReportSelectionStore((s) => s.pruneSelection);
150+
const clearSelection = useInboxReportSelectionStore((s) => s.clearSelection);
146151

147-
useEffect(() => {
148-
if (reports.length === 0) {
149-
setSelectedReportId(null);
150-
return;
151-
}
152-
if (!selectedReportId) {
153-
return;
154-
}
155-
const selectedExists = reports.some(
156-
(report) => report.id === selectedReportId,
157-
);
158-
if (!selectedExists) {
159-
setSelectedReportId(null);
160-
}
161-
}, [reports, selectedReportId]);
152+
// Stable refs so callbacks don't need re-registration on every render
153+
const selectedReportIdsRef = useRef(selectedReportIds);
154+
selectedReportIdsRef.current = selectedReportIds;
155+
const reportsRef = useRef(reports);
156+
reportsRef.current = reports;
162157

158+
// Prune selection when visible reports change (e.g. filter/search)
163159
useEffect(() => {
164160
pruneSelection(reports.map((report) => report.id));
165161
}, [reports, pruneSelection]);
166162

167-
const selectedReport = useMemo(
168-
() => reports.find((report) => report.id === selectedReportId) ?? null,
169-
[reports, selectedReportId],
163+
// The report to show in the detail pane (only when exactly 1 is selected)
164+
const selectedReport = useMemo(() => {
165+
if (selectedReportIds.length !== 1) return null;
166+
return reports.find((r) => r.id === selectedReportIds[0]) ?? null;
167+
}, [reports, selectedReportIds]);
168+
169+
// Reports for the multi-select stack (when 2+ selected)
170+
const selectedReports = useMemo(() => {
171+
if (selectedReportIds.length < 2) return [];
172+
const idSet = new Set(selectedReportIds);
173+
return reports.filter((r) => idSet.has(r.id));
174+
}, [reports, selectedReportIds]);
175+
176+
// ── Click handler: plain / cmd / shift ──────────────────────────────────
177+
const handleReportClick = useCallback(
178+
(reportId: string, event: { metaKey: boolean; shiftKey: boolean }) => {
179+
if (event.shiftKey) {
180+
selectRange(
181+
reportId,
182+
reportsRef.current.map((r) => r.id),
183+
);
184+
} else if (event.metaKey) {
185+
toggleReportSelection(reportId);
186+
} else if (
187+
selectedReportIdsRef.current.length === 1 &&
188+
selectedReportIdsRef.current[0] === reportId
189+
) {
190+
// Plain click on the only selected report — deselect it
191+
clearSelection();
192+
} else {
193+
// Plain click — select only this report
194+
setSelectedReportIds([reportId]);
195+
}
196+
},
197+
[selectRange, toggleReportSelection, setSelectedReportIds, clearSelection],
198+
);
199+
200+
// Select-all checkbox
201+
const handleToggleSelectAll = useCallback(
202+
(checked: boolean) => {
203+
if (checked) {
204+
setSelectedReportIds(reportsRef.current.map((r) => r.id));
205+
} else {
206+
clearSelection();
207+
}
208+
},
209+
[setSelectedReportIds, clearSelection],
170210
);
171211

172212
// ── Sidebar resize ─────────────────────────────────────────────────────
@@ -237,10 +277,6 @@ export function InboxSignalsTab() {
237277
const showTwoPaneLayout = hasMountedTwoPaneRef.current;
238278

239279
// ── Arrow-key navigation between reports ──────────────────────────────
240-
const reportsRef = useRef(reports);
241-
reportsRef.current = reports;
242-
const selectedReportIdRef = useRef(selectedReportId);
243-
selectedReportIdRef.current = selectedReportId;
244280
const leftPaneRef = useRef<HTMLDivElement>(null);
245281

246282
const focusListPane = useCallback(() => {
@@ -252,41 +288,46 @@ export function InboxSignalsTab() {
252288
// Auto-focus the list pane when the two-pane layout appears
253289
useEffect(() => {
254290
if (showTwoPaneLayout) {
255-
// Small delay to ensure the ref is mounted after conditional render
256291
focusListPane();
257292
}
258293
}, [focusListPane, showTwoPaneLayout]);
259294

260-
const navigateReport = useCallback((direction: 1 | -1) => {
261-
const list = reportsRef.current;
262-
if (list.length === 0) return;
263-
264-
const currentId = selectedReportIdRef.current;
265-
const currentIndex = currentId
266-
? list.findIndex((r) => r.id === currentId)
267-
: -1;
268-
const nextIndex =
269-
currentIndex === -1
270-
? 0
271-
: Math.max(0, Math.min(list.length - 1, currentIndex + direction));
272-
const nextId = list[nextIndex].id;
273-
274-
setSelectedReportId(nextId);
275-
276-
const container = leftPaneRef.current;
277-
const row = container?.querySelector<HTMLElement>(
278-
`[data-report-id="${nextId}"]`,
279-
);
280-
const stickyHeader = container?.querySelector<HTMLElement>(
281-
"[data-inbox-sticky-header]",
282-
);
283-
284-
if (!row) return;
285-
286-
const stickyHeaderHeight = stickyHeader?.offsetHeight ?? 0;
287-
row.style.scrollMarginTop = `${stickyHeaderHeight}px`;
288-
row.scrollIntoView({ block: "nearest" });
289-
}, []);
295+
const navigateReport = useCallback(
296+
(direction: 1 | -1) => {
297+
const list = reportsRef.current;
298+
if (list.length === 0) return;
299+
300+
// Find the current position based on the last selected report
301+
const currentIds = selectedReportIdsRef.current;
302+
const currentId =
303+
currentIds.length > 0 ? currentIds[currentIds.length - 1] : null;
304+
const currentIndex = currentId
305+
? list.findIndex((r) => r.id === currentId)
306+
: -1;
307+
const nextIndex =
308+
currentIndex === -1
309+
? 0
310+
: Math.max(0, Math.min(list.length - 1, currentIndex + direction));
311+
const nextId = list[nextIndex].id;
312+
313+
setSelectedReportIds([nextId]);
314+
315+
const container = leftPaneRef.current;
316+
const row = container?.querySelector<HTMLElement>(
317+
`[data-report-id="${nextId}"]`,
318+
);
319+
const stickyHeader = container?.querySelector<HTMLElement>(
320+
"[data-inbox-sticky-header]",
321+
);
322+
323+
if (!row) return;
324+
325+
const stickyHeaderHeight = stickyHeader?.offsetHeight ?? 0;
326+
row.style.scrollMarginTop = `${stickyHeaderHeight}px`;
327+
row.scrollIntoView({ block: "nearest" });
328+
},
329+
[setSelectedReportIds],
330+
);
290331

291332
// Window-level keyboard handler so arrow keys work regardless of which
292333
// pane has focus — only suppressed inside interactive widgets.
@@ -310,14 +351,17 @@ export function InboxSignalsTab() {
310351
} else if (e.key === "ArrowUp") {
311352
e.preventDefault();
312353
navigateReport(-1);
313-
} else if (e.key === " " && selectedReportIdRef.current) {
354+
} else if (
355+
e.key === "Escape" &&
356+
selectedReportIdsRef.current.length > 0
357+
) {
314358
e.preventDefault();
315-
toggleReportSelection(selectedReportIdRef.current);
359+
clearSelection();
316360
}
317361
};
318362
window.addEventListener("keydown", handler);
319363
return () => window.removeEventListener("keydown", handler);
320-
}, [navigateReport, toggleReportSelection]);
364+
}, [navigateReport, clearSelection]);
321365

322366
const searchDisabledReason =
323367
!hasReports && !searchQuery.trim()
@@ -366,11 +410,7 @@ export function InboxSignalsTab() {
366410
) {
367411
return;
368412
}
369-
if (
370-
target.closest(
371-
"[data-report-id], button, [role='checkbox']",
372-
)
373-
) {
413+
if (target.closest("[data-report-id], button")) {
374414
focusListPane();
375415
}
376416
}}
@@ -387,9 +427,7 @@ export function InboxSignalsTab() {
387427
}
388428
if (
389429
target !== leftPaneRef.current &&
390-
target.closest(
391-
"[data-report-id], button, [role='checkbox']",
392-
)
430+
target.closest("[data-report-id], button")
393431
) {
394432
focusListPane();
395433
}
@@ -413,6 +451,8 @@ export function InboxSignalsTab() {
413451
processingCount={processingCount}
414452
pipelinePausedUntil={signalProcessingState?.paused_until}
415453
reports={reports}
454+
effectiveBulkIds={selectedReportIds}
455+
onToggleSelectAll={handleToggleSelectAll}
416456
/>
417457
</Box>
418458
<ReportListPane
@@ -428,9 +468,8 @@ export function InboxSignalsTab() {
428468
hasSignalSources={hasSignalSources}
429469
searchQuery={searchQuery}
430470
hasActiveFilters={hasActiveFilters}
431-
selectedReportId={selectedReportId}
432471
selectedReportIds={selectedReportIds}
433-
onSelectReport={setSelectedReportId}
472+
onReportClick={handleReportClick}
434473
onToggleReportSelection={toggleReportSelection}
435474
/>
436475
</Flex>
@@ -463,10 +502,15 @@ export function InboxSignalsTab() {
463502
position: "relative",
464503
}}
465504
>
466-
{selectedReport ? (
505+
{selectedReports.length > 1 ? (
506+
<MultiSelectStack
507+
reports={selectedReports}
508+
onClearSelection={clearSelection}
509+
/>
510+
) : selectedReport ? (
467511
<ReportDetailPane
468512
report={selectedReport}
469-
onClose={() => setSelectedReportId(null)}
513+
onClose={clearSelection}
470514
/>
471515
) : (
472516
<SelectReportPane />

0 commit comments

Comments
 (0)