Skip to content

feat(ui): mark files to hide them from the review stream #277

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.

Sign up for GitHub

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

Open
aldevv wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat(ui): mark files to hide them from the review stream #277

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
22 changes: 22 additions & 0 deletions src/ui/App.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,17 @@ export function App({
const selectedHunkIndex = review.selectedHunkIndex;
const moveToAnnotatedFile = review.moveToAnnotatedFile;
const moveToAnnotatedHunk = review.moveToAnnotatedHunk;
const toggleMarkedFile = review.toggleMarkedFile;
const clearMarkedFiles = review.clearMarkedFiles;
const hiddenByMarkCount = review.hiddenByMarkCount;

/** Toggle the focused file's mark via the global `m` shortcut. */
const toggleSelectedFileMark = useCallback(() => {
if (!selectedFile) {
return;
}
toggleMarkedFile(selectedFile.id);
}, [selectedFile, toggleMarkedFile]);

const jumpToFile = useCallback(
(fileId: string, nextHunkIndex = 0, options?: { alignFileHeaderTop?: boolean }) => {
Expand Down Expand Up @@ -472,6 +483,7 @@ export function App({
buildAppMenus({
activeThemeId: activeTheme.id,
canRefreshCurrentInput,
clearMarkedFiles,
focusFilter,
layoutMode,
moveToAnnotatedFile,
Expand All @@ -492,12 +504,14 @@ export function App({
toggleHunkHeaders,
toggleLineNumbers,
toggleLineWrap,
toggleMarkedFile: toggleSelectedFileMark,
toggleSidebar,
wrapLines,
}),
[
activeTheme.id,
canRefreshCurrentInput,
clearMarkedFiles,
focusFilter,
layoutMode,
moveToAnnotatedFile,
Expand All @@ -517,6 +531,7 @@ export function App({
toggleHunkHeaders,
toggleLineNumbers,
toggleLineWrap,
toggleSelectedFileMark,
toggleSidebar,
wrapLines,
],
Expand All @@ -542,6 +557,7 @@ export function App({
activeMenuId,
activateCurrentMenuItem,
canRefreshCurrentInput,
clearMarkedFiles,
closeHelp,
closeMenu,
cycleTheme,
Expand All @@ -564,6 +580,7 @@ export function App({
toggleHunkHeaders,
toggleLineNumbers,
toggleLineWrap,
toggleMarkedFile: toggleSelectedFileMark,
toggleSidebar,
triggerRefreshCurrentInput,
});
Expand Down Expand Up @@ -673,6 +690,7 @@ export function App({
<>
<SidebarPane
entries={review.sidebarEntries}
hiddenByMarkCount={hiddenByMarkCount}
scrollRef={sidebarScrollRef}
selectedFileId={selectedFile?.id}
textWidth={sidebarTextWidth}
Expand All @@ -682,6 +700,10 @@ export function App({
focusFiles();
jumpToFile(fileId, 0, { alignFileHeaderTop: true });
}}
onUnmarkFile={(fileId) => {
focusFiles();
toggleMarkedFile(fileId);
}}
/>

<PaneDivider
Expand Down
2 changes: 1 addition & 1 deletion src/ui/AppHost.interactions.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,7 @@ describe("App interactions", () => {
expect(frame).toContain("- export const alpha = 1;");

await act(async () => {
await setup.mockInput.typeText("m");
await setup.mockInput.typeText("H");
});
await flush(setup);

Expand Down
3 changes: 2 additions & 1 deletion src/ui/components/chrome/HelpDialog.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,14 @@ export function HelpDialog({
["1 / 2 / 0", "split / stack / auto"],
["s / t", "sidebar / theme"],
["a", "toggle AI notes"],
["l / w / m", "lines / wrap / metadata"],
["l / w / H", "lines / wrap / metadata"],
],
},
{
title: "Review",
items: [
["/", "focus file filter"],
["m / M", "mark file / unmark all"],
["Tab", "toggle files/filter focus"],
["F10", "open menus"],
[canRefresh ? "r / q" : "q", canRefresh ? "reload / quit" : "quit"],
Expand Down
16 changes: 14 additions & 2 deletions src/ui/components/panes/FileListItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { TextAttributes } from "@opentui/core";
import { fileRowId } from "../../lib/ids";
import { sidebarEntryStats, type FileGroupEntry, type FileListEntry } from "../../lib/files";
import { fitText, padText } from "../../lib/text";
Expand Down Expand Up @@ -70,6 +71,11 @@ export function FileListItem({
const iconWidth = icon ? 2 : 0; // icon + space
const statsSectionWidth = statsWidth > 0 ? statsWidth + 1 : 0;
const nameWidth = Math.max(1, textWidth - 1 - iconWidth - statsSectionWidth);
// Marked rows render dimmed and crossed out so the user can scan past them but still
// pick one to unmark.
const nameAttributes = entry.marked ? TextAttributes.STRIKETHROUGH : TextAttributes.NONE;
const nameColor = entry.marked ? theme.muted : theme.text;
const iconColor = entry.marked ? theme.muted : color;

return (
<box
Expand Down Expand Up @@ -98,8 +104,14 @@ export function FileListItem({
backgroundColor: rowBackground,
}}
>
{icon && <text fg={color}>{icon} </text>}
<text fg={theme.text}>{padText(fitText(entry.name, nameWidth), nameWidth)}</text>
{icon && (
<text fg={iconColor} attributes={nameAttributes}>
{icon}{" "}
</text>
)}
<text fg={nameColor} attributes={nameAttributes}>
{padText(fitText(entry.name, nameWidth), nameWidth)}
</text>
{statsSectionWidth > 0 && (
<box
style={{
Expand Down
31 changes: 30 additions & 1 deletion src/ui/components/panes/SidebarPane.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,29 +1,38 @@
import type { ScrollBoxRenderable } from "@opentui/core";
import type { RefObject } from "react";
import { sidebarEntryStatsWidth, type SidebarEntry } from "../../lib/files";
import { fitText } from "../../lib/text";
import type { AppTheme } from "../../themes";
import { FileGroupHeader, FileListItem } from "./FileListItem";

/** Render the file navigation sidebar. */
export function SidebarPane({
entries,
hiddenByMarkCount = 0,
scrollRef,
selectedFileId,
textWidth,
theme,
width,
onSelectFile,
onUnmarkFile,
}: {
entries: SidebarEntry[];
/** When > 0, render a footer hint reminding the user how many files are hidden by marks. */
hiddenByMarkCount?: number;
scrollRef: RefObject<ScrollBoxRenderable | null>;
selectedFileId?: string;
textWidth: number;
theme: AppTheme;
width: number;
onSelectFile: (fileId: string) => void;
/** Called when the user clicks a marked sidebar row, so it can be unmarked instead of selected. */
onUnmarkFile?: (fileId: string) => void;
}) {
const fileEntries = entries.filter((entry) => entry.kind === "file");
const statsWidth = Math.max(0, ...fileEntries.map((entry) => sidebarEntryStatsWidth(entry)));
const showHiddenFooter = hiddenByMarkCount > 0;
const hiddenFooterText = `${hiddenByMarkCount} hidden`;

return (
<box
Expand Down Expand Up @@ -63,12 +72,32 @@ export function SidebarPane({
statsWidth={statsWidth}
textWidth={textWidth}
theme={theme}
onSelect={() => onSelectFile(entry.id)}
onSelect={() => {
// Clicking a marked row should bring the file back rather than re-select a
// hidden file in the diff stream.
if (entry.marked && onUnmarkFile) {
onUnmarkFile(entry.id);
return;
}
onSelectFile(entry.id);
}}
Comment on lines +75 to +83
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot May 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Silent fallthrough to onSelectFile for marked rows

When entry.marked is true but onUnmarkFile is absent, the click falls through to onSelectFile(entry.id), which calls jumpToFile on a file that has been intentionally removed from the review stream. The resulting selectFile call sets selectedFileId to the hidden file, and resolveSelectedFile then surfaces it via the allFiles fallback — displaying a file the user just asked to hide. A no-op guard when onUnmarkFile is absent is safer than silently treating the click as a normal selection.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: src/ui/components/panes/SidebarPane.tsx
Line: 75-83

Comment:
**Silent fallthrough to `onSelectFile` for marked rows**

When `entry.marked` is true but `onUnmarkFile` is absent, the click falls through to `onSelectFile(entry.id)`, which calls `jumpToFile` on a file that has been intentionally removed from the review stream. The resulting `selectFile` call sets `selectedFileId` to the hidden file, and `resolveSelectedFile` then surfaces it via the `allFiles` fallback — displaying a file the user just asked to hide. A no-op guard when `onUnmarkFile` is absent is safer than silently treating the click as a normal selection.

How can I resolve this? If you propose a fix, please make it concise.

/>
),
)}
</box>
</scrollbox>
{showHiddenFooter ? (
<box
style={{
width: "100%",
height: 1,
paddingLeft: 1,
backgroundColor: theme.panel,
}}
>
<text fg={theme.muted}>{fitText(hiddenFooterText, Math.max(1, textWidth))}</text>
</box>
) : null}
</box>
);
}
3 changes: 2 additions & 1 deletion src/ui/components/ui-components.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1615,9 +1615,10 @@ describe("UI components", () => {
"1 / 2 / 0 split / stack / auto",
"s / t sidebar / theme",
"a toggle AI notes",
"l / w / m lines / wrap / metadata",
"l / w / H lines / wrap / metadata",
"Review",
"/ focus file filter",
"m / M mark file / unmark all",
"Tab toggle files/filter focus",
"F10 open menus",
"r / q reload / quit",
Expand Down
18 changes: 17 additions & 1 deletion src/ui/hooks/useAppKeyboardShortcuts.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export interface UseAppKeyboardShortcutsOptions {
activeMenuId: MenuId | null;
activateCurrentMenuItem: () => void;
canRefreshCurrentInput: boolean;
clearMarkedFiles: () => void;
closeHelp: () => void;
closeMenu: () => void;
cycleTheme: () => void;
Expand All @@ -45,6 +46,7 @@ export interface UseAppKeyboardShortcutsOptions {
toggleHunkHeaders: () => void;
toggleLineNumbers: () => void;
toggleLineWrap: () => void;
toggleMarkedFile: () => void;
toggleSidebar: () => void;
triggerRefreshCurrentInput: () => void;
}
Expand All @@ -54,6 +56,7 @@ export function useAppKeyboardShortcuts({
activeMenuId,
activateCurrentMenuItem,
canRefreshCurrentInput,
clearMarkedFiles,
closeHelp,
closeMenu,
cycleTheme,
Expand All @@ -76,6 +79,7 @@ export function useAppKeyboardShortcuts({
toggleHunkHeaders,
toggleLineNumbers,
toggleLineWrap,
toggleMarkedFile,
toggleSidebar,
triggerRefreshCurrentInput,
}: UseAppKeyboardShortcutsOptions) {
Expand Down Expand Up @@ -361,7 +365,19 @@ export function useAppKeyboardShortcuts({
return;
}

if (key.name === "m" || key.sequence === "m") {
// `m` marks the focused file; shift+m clears every active mark. The legacy hunk-metadata
// toggle moved to `H` so the mark binding can stay in the most reachable spot.
if (key.sequence === "M" || (key.name === "m" && key.shift)) {
runAndCloseMenu(clearMarkedFiles);
return;
}

if (key.name === "m" && !key.shift) {
runAndCloseMenu(toggleMarkedFile);
return;
}

if (key.sequence === "H" || (key.name === "h" && key.shift)) {
runAndCloseMenu(toggleHunkHeaders);
return;
}
Expand Down
101 changes: 101 additions & 0 deletions src/ui/hooks/useReviewController.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,107 @@ describe("useReviewController", () => {
}
});

test("toggleMarkedFile hides the file and clearMarkedFiles restores everything", async () => {
const controllerRef: { current: ReviewController | null } = { current: null };
const setup = await testRender(
<ReviewControllerHarness
initialFiles={[
createDiffFile(
"alpha",
"alpha.ts",
"export const alpha = 1;\n",
"export const alpha = 2;\n",
),
createDiffFile("beta", "beta.ts", "export const beta = 1;\n", "export const beta = 2;\n"),
]}
onController={(nextController) => {
controllerRef.current = nextController;
}}
/>,
{ width: 80, height: 4 },
);

try {
await flush(setup);
expect(expectValue(controllerRef.current).visibleFiles.map((file) => file.path)).toEqual([
"alpha.ts",
"beta.ts",
]);
expect(expectValue(controllerRef.current).hiddenByMarkCount).toBe(0);

await act(async () => {
expectValue(controllerRef.current).toggleMarkedFile("alpha");
});
await flush(setup);

expect(expectValue(controllerRef.current).visibleFiles.map((file) => file.path)).toEqual([
"beta.ts",
]);
expect(expectValue(controllerRef.current).hiddenByMarkCount).toBe(1);
expect(expectValue(controllerRef.current).markedFileIds.has("alpha")).toBe(true);
// The sidebar still shows the marked file so the user can unmark it.
expect(
expectValue(controllerRef.current)
.sidebarEntries.filter((entry) => entry.kind === "file")
.map((entry) => entry.id),
).toEqual(["alpha", "beta"]);

await act(async () => {
expectValue(controllerRef.current).clearMarkedFiles();
});
await flush(setup);

expect(expectValue(controllerRef.current).visibleFiles.map((file) => file.path)).toEqual([
"alpha.ts",
"beta.ts",
]);
expect(expectValue(controllerRef.current).hiddenByMarkCount).toBe(0);
expect(expectValue(controllerRef.current).markedFileIds.size).toBe(0);
} finally {
await act(async () => {
setup.renderer.destroy();
});
}
});

test("marking the selected file moves selection to the next visible file", async () => {
const controllerRef: { current: ReviewController | null } = { current: null };
const setup = await testRender(
<ReviewControllerHarness
initialFiles={[
createDiffFile(
"alpha",
"alpha.ts",
"export const alpha = 1;\n",
"export const alpha = 2;\n",
),
createDiffFile("beta", "beta.ts", "export const beta = 1;\n", "export const beta = 2;\n"),
]}
onController={(nextController) => {
controllerRef.current = nextController;
}}
/>,
{ width: 80, height: 4 },
);

try {
await flush(setup);
expect(expectValue(controllerRef.current).selectedFile?.path).toBe("alpha.ts");

await act(async () => {
expectValue(controllerRef.current).toggleMarkedFile("alpha");
});
await flush(setup);

expect(expectValue(controllerRef.current).selectedFile?.path).toBe("beta.ts");
expect(expectValue(controllerRef.current).selectedFileId).toBe("beta");
} finally {
await act(async () => {
setup.renderer.destroy();
});
}
});

test("batch live comments do not mutate state when any target is invalid", async () => {
const controllerRef: { current: ReviewController | null } = { current: null };
const setup = await testRender(
Expand Down
Loading
Loading