Skip to content

Commit 1d9f5e3

Browse files
committed
fix(ui): resolve editor shortcut conflict
1 parent 81580ad commit 1d9f5e3

7 files changed

Lines changed: 112 additions & 27 deletions

File tree

src/ui/components/chrome/HelpDialog.tsx

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,8 @@ export function HelpDialog({
4646
["1 / 2 / 0", "split / stack / auto"],
4747
["s / t", "sidebar / theme"],
4848
["a", "toggle AI notes"],
49-
["e", "toggle unchanged context"],
49+
["z", "toggle unchanged context"],
5050
["l / w / m", "lines / wrap / metadata"],
51-
["e", "open file in $EDITOR"],
5251
],
5352
},
5453
{

src/ui/components/ui-components.test.tsx

Lines changed: 72 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1919,8 +1919,8 @@ describe("UI components", () => {
19191919
"1 / 2 / 0 split / stack / auto",
19201920
"s / t sidebar / theme",
19211921
"a toggle AI notes",
1922+
"z toggle unchanged context",
19221923
"l / w / m lines / wrap / metadata",
1923-
"e open file in $EDITOR",
19241924
"Review",
19251925
"/ focus file filter",
19261926
"c create review note",
@@ -2368,6 +2368,77 @@ describe("UI components", () => {
23682368
expect(expandableFrame).toContain("▾");
23692369
});
23702370

2371+
test("PierreDiffView hides add-note affordances on collapsed and hunk-header rows", async () => {
2372+
const expandable = createExpandableContextDiffFile("meta-hover", "meta-hover.ts");
2373+
const file = {
2374+
...expandable.file,
2375+
sourceFetcher: createTestSourceFetcher(() => expandable.after),
2376+
};
2377+
const theme = resolveTheme("midnight", null);
2378+
const setup = await testRender(
2379+
<PierreDiffView
2380+
file={file}
2381+
layout="split"
2382+
theme={theme}
2383+
width={120}
2384+
selectedHunkIndex={0}
2385+
scrollable={false}
2386+
onStartUserNoteAtHunk={() => {}}
2387+
onToggleGap={() => {}}
2388+
/>,
2389+
{ width: 120, height: 40 },
2390+
);
2391+
2392+
try {
2393+
await act(async () => {
2394+
await setup.renderOnce();
2395+
});
2396+
2397+
const frame = setup.captureCharFrame();
2398+
const frameLines = frame.split("\n");
2399+
const collapsedY = frameLines.findIndex((line) => line.includes("unchanged lines"));
2400+
const hunkHeaderY = frameLines.findIndex((line) => line.includes("@@"));
2401+
const codeY = frameLines.findIndex((line) => line.includes("line 5 modified"));
2402+
expect(collapsedY).toBeGreaterThanOrEqual(0);
2403+
expect(hunkHeaderY).toBeGreaterThanOrEqual(0);
2404+
expect(codeY).toBeGreaterThanOrEqual(0);
2405+
2406+
await act(async () => {
2407+
await setup.mockMouse.moveTo(4, collapsedY);
2408+
await setup.renderOnce();
2409+
});
2410+
expect(setup.captureCharFrame()).not.toContain("[+]");
2411+
2412+
await act(async () => {
2413+
await setup.mockMouse.moveTo(4, hunkHeaderY);
2414+
await setup.renderOnce();
2415+
});
2416+
expect(setup.captureCharFrame()).not.toContain("[+]");
2417+
2418+
let codeHoverFrame = "";
2419+
for (const y of [codeY, codeY + 1]) {
2420+
for (const x of [4, 16, 48, 76]) {
2421+
await act(async () => {
2422+
await setup.mockMouse.moveTo(x, y);
2423+
await setup.renderOnce();
2424+
});
2425+
codeHoverFrame = setup.captureCharFrame();
2426+
if (codeHoverFrame.includes("[+]")) {
2427+
break;
2428+
}
2429+
}
2430+
if (codeHoverFrame.includes("[+]")) {
2431+
break;
2432+
}
2433+
}
2434+
expect(codeHoverFrame).toContain("[+]");
2435+
} finally {
2436+
await act(async () => {
2437+
setup.renderer.destroy();
2438+
});
2439+
}
2440+
});
2441+
23712442
test("PierreDiffView toggles a collapsed gap when clicked", async () => {
23722443
const expandable = createExpandableContextDiffFile("expand-click", "expand-click.ts");
23732444
const file = {

src/ui/diff/PierreDiffView.tsx

Lines changed: 31 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,15 @@ export interface ActiveAddNoteAffordance {
3131
target?: UserNoteLineTarget;
3232
}
3333

34+
type AddNoteTargetRow = Extract<DiffRow, { type: "split-line" | "stack-line" }>;
35+
36+
/** Return whether a diff row can be used as an inline user-note target. */
37+
function isAddNoteTargetRow(row: DiffRow): row is AddNoteTargetRow {
38+
return row.type === "split-line" || row.type === "stack-line";
39+
}
40+
3441
/** Resolve the note insertion target represented by a visible add-note affordance. */
35-
function addNoteAffordanceForRow(row: DiffRow): ActiveAddNoteAffordance {
42+
function addNoteAffordanceForRow(row: AddNoteTargetRow): ActiveAddNoteAffordance {
3643
if (row.type === "split-line") {
3744
return {
3845
hunkIndex: row.hunkIndex,
@@ -45,19 +52,15 @@ function addNoteAffordanceForRow(row: DiffRow): ActiveAddNoteAffordance {
4552
};
4653
}
4754

48-
if (row.type === "stack-line") {
49-
return {
50-
hunkIndex: row.hunkIndex,
51-
target:
52-
row.cell.newLineNumber !== undefined
53-
? { side: "new", line: row.cell.newLineNumber }
54-
: row.cell.oldLineNumber !== undefined
55-
? { side: "old", line: row.cell.oldLineNumber }
56-
: undefined,
57-
};
58-
}
59-
60-
return { hunkIndex: row.hunkIndex };
55+
return {
56+
hunkIndex: row.hunkIndex,
57+
target:
58+
row.cell.newLineNumber !== undefined
59+
? { side: "new", line: row.cell.newLineNumber }
60+
: row.cell.oldLineNumber !== undefined
61+
? { side: "old", line: row.cell.oldLineNumber }
62+
: undefined,
63+
};
6164
}
6265

6366
/** Render a file diff in split or stack mode, with inline agent notes inserted between diff rows. */
@@ -331,6 +334,10 @@ export function PierreDiffView({
331334
);
332335
}
333336

337+
const addNoteAffordance = isAddNoteTargetRow(plannedRow.row)
338+
? addNoteAffordanceForRow(plannedRow.row)
339+
: null;
340+
334341
return (
335342
<box key={plannedRow.key} id={rowId} style={{ width: "100%", flexDirection: "column" }}>
336343
<DiffRowView
@@ -347,10 +354,18 @@ export function PierreDiffView({
347354
copySelectedSide={copySelectedSide}
348355
anchorId={plannedRow.anchorId}
349356
noteGuideSide={plannedRow.noteGuideSide}
350-
showAddNoteBadge={hoveredRowKey === plannedRow.key && Boolean(onStartUserNoteAtHunk)}
357+
showAddNoteBadge={
358+
addNoteAffordance !== null &&
359+
hoveredRowKey === plannedRow.key &&
360+
Boolean(onStartUserNoteAtHunk)
361+
}
351362
onHoverRow={() => {
352363
onHover?.();
353-
activateHoveredRow(plannedRow.key, addNoteAffordanceForRow(plannedRow.row));
364+
if (addNoteAffordance) {
365+
activateHoveredRow(plannedRow.key, addNoteAffordance);
366+
} else {
367+
clearHoveredRow();
368+
}
354369
}}
355370
onStartUserNoteAtHunk={onStartUserNoteAtHunk}
356371
onToggleGap={gapToggleHandler}

src/ui/diff/renderRows.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1134,6 +1134,7 @@ function renderHeaderRow(
11341134
backgroundColor: theme.panelAlt,
11351135
}}
11361136
onMouseMove={() => onHoverRow?.(row.key)}
1137+
onMouseOver={() => onHoverRow?.(row.key)}
11371138
onMouseUp={handleCollapsedClick}
11381139
>
11391140
<text>
@@ -1165,6 +1166,7 @@ function renderHeaderRow(
11651166
backgroundColor: theme.panelAlt,
11661167
}}
11671168
onMouseMove={() => onHoverRow?.(row.key)}
1169+
onMouseOver={() => onHoverRow?.(row.key)}
11681170
>
11691171
<box
11701172
style={{ width: Math.max(0, width - badgeWidth), height: 1 }}

src/ui/hooks/useAppKeyboardShortcuts.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,6 @@ export function useAppKeyboardShortcuts({
109109
toggleLineNumbers,
110110
toggleLineWrap,
111111
toggleSidebar,
112-
triggerEditSelectedFile,
113112
triggerRefreshCurrentInput,
114113
}: UseAppKeyboardShortcutsOptions) {
115114
const activeMenuIdRef = useRef(activeMenuId);
@@ -453,7 +452,7 @@ export function useAppKeyboardShortcuts({
453452
return;
454453
}
455454

456-
if (key.name === "e" || key.sequence === "e") {
455+
if (key.name === "z" || key.sequence === "z") {
457456
runAndCloseMenu(toggleGapForSelectedHunk);
458457
return;
459458
}

src/ui/lib/appMenus.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,6 @@ export function buildAppMenus({
8585
{
8686
kind: "item",
8787
label: "Open file in editor",
88-
hint: "e",
8988
action: triggerEditSelectedFile,
9089
},
9190
];

test/pty/ui-integration.test.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,7 @@ describe("live UI integration", () => {
274274
expect(initial).toContain("▾ 1 unchanged line");
275275
expect(initial).not.toContain("hiddenLine01");
276276

277-
await session.press("e");
277+
await session.press("z");
278278
const expanded = await harness.waitForSnapshot(
279279
session,
280280
(text) => text.includes("Hide 1 unchanged line") && text.includes("hiddenLine01"),
@@ -283,7 +283,7 @@ describe("live UI integration", () => {
283283

284284
expect(expanded).toContain("hiddenLine01");
285285

286-
await session.press("e");
286+
await session.press("z");
287287
const collapsed = await harness.waitForSnapshot(
288288
session,
289289
(text) => text.includes("▾ 1 unchanged line") && !text.includes("hiddenLine01"),
@@ -1666,7 +1666,7 @@ describe("live UI integration", () => {
16661666
});
16671667

16681668
expect(initial).toContain("aaa-collapsed.ts");
1669-
expect(initial).toContain("··· 362 unchanged lines ···");
1669+
expect(initial).toContain(" 362 unchanged lines");
16701670
expect(initial).not.toContain("366 - export const line366 = 366;");
16711671

16721672
await session.scrollDown(1);
@@ -1708,12 +1708,12 @@ describe("live UI integration", () => {
17081708
const restored = await harness.waitForSnapshot(
17091709
session,
17101710
(text) =>
1711-
text.includes("··· 362 unchanged lines ···") &&
1711+
text.includes(" 362 unchanged lines") &&
17121712
harness.countMatches(text, /aaa-collapsed\.ts/g) === initialHeaderCount,
17131713
5_000,
17141714
);
17151715

1716-
expect(restored).toContain("··· 362 unchanged lines ···");
1716+
expect(restored).toContain(" 362 unchanged lines");
17171717
expect(restored).not.toContain("366 - export const line366 = 366;");
17181718
expect(harness.countMatches(restored, /aaa-collapsed\.ts/g)).toBe(initialHeaderCount);
17191719
} finally {

0 commit comments

Comments
 (0)