Skip to content

Commit f78eced

Browse files
committed
Share the modal scroll-window math in one hook
operator-modal and permission-modal each carried their own copy of FALLBACK_TERMINAL_ROWS, maxRowOffset, and the clamp-and-slice PageUp/PageDown logic. Extract it into useScrollWindow so the offset math is owned in one place.
1 parent 59a91b9 commit f78eced

4 files changed

Lines changed: 106 additions & 26 deletions

File tree

src/tui/components/operator-modal.tsx

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { createMemoizedParseMarkdown } from "../markdown-parser.js";
66
import type { StyledSegment } from "../markdown-parser.js";
77
import { color } from "../theme.js";
88
import { inkPropsForSegment } from "../styled-segment-props.js";
9+
import { FALLBACK_TERMINAL_ROWS, useScrollWindow } from "../hooks/use-scroll-window.js";
910

1011
export type OperatorModalProps = {
1112
question: string;
@@ -17,14 +18,9 @@ export type OperatorModalProps = {
1718
terminalRows?: number;
1819
};
1920

20-
const FALLBACK_TERMINAL_ROWS = 24;
2121
const MIN_QUESTION_ROWS = 2;
2222
const MIN_OPTION_ROWS = 3;
2323

24-
function maxRowOffset(rowCount: number, visibleRows: number): number {
25-
return Math.max(0, rowCount - visibleRows);
26-
}
27-
2824
function segmentProps(seg: StyledSegment): Record<string, unknown> {
2925
return inkPropsForSegment(seg);
3026
}
@@ -153,14 +149,13 @@ export function OperatorModal({
153149
}
154150

155151
const questionScrollable = questionLines.length > questionRows;
156-
const questionMaxOffset = maxRowOffset(questionLines.length, questionRows);
157-
const [questionScrollOffset, setQuestionScrollOffset] = useState(0);
158-
const clampedQuestionOffset = Math.min(questionScrollOffset, questionMaxOffset);
152+
const questionScroll = useScrollWindow(questionLines.length, questionRows);
153+
const clampedQuestionOffset = questionScroll.offset;
159154
const visibleQuestionLines = questionScrollable
160155
? questionLines.slice(clampedQuestionOffset, clampedQuestionOffset + questionRows)
161156
: questionLines;
162-
const questionLinesAbove = clampedQuestionOffset;
163-
const questionLinesBelow = Math.max(0, questionLines.length - clampedQuestionOffset - questionRows);
157+
const questionLinesAbove = questionScroll.above;
158+
const questionLinesBelow = questionScroll.below;
164159

165160
// Only the plain list windows around the selection — grid mode is capped at
166161
// 4 options (2 rows), which always fits.
@@ -206,11 +201,11 @@ export function OperatorModal({
206201
return;
207202
}
208203
if (questionScrollable && key.pageUp) {
209-
setQuestionScrollOffset((o) => Math.max(0, o - questionRows));
204+
questionScroll.pageUp();
210205
return;
211206
}
212207
if (questionScrollable && key.pageDown) {
213-
setQuestionScrollOffset((o) => Math.min(questionMaxOffset, o + questionRows));
208+
questionScroll.pageDown();
214209
return;
215210
}
216211
if (key.ctrl && (key.upArrow || key.downArrow)) return;

src/tui/components/permission-modal.tsx

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { stripTerminalControlSequences } from "../../util/control-char-strip.js"
88
import { isShellCommentOnly } from "../../permission/command.js";
99
import { collapseSegmentPayloads, groupChainSegmentsForDisplay, middleEllipsis } from "../command-display.js";
1010
import type { QueuedApprovalSummary } from "../hooks/use-gates.js";
11+
import { FALLBACK_TERMINAL_ROWS, useScrollWindow } from "../hooks/use-scroll-window.js";
1112

1213
// Bidi controls (RLO, embeddings, isolates) visually reorder the rendered
1314
// command — Trojan Source — and zero-width characters hide payload boundaries,
@@ -65,11 +66,6 @@ const MAX_RENDERED_QUEUE_ENTRIES = 5;
6566
// meaningful plus its scroll indicators), so it is the floor regardless of
6667
// how little the terminal reports.
6768
const MIN_BODY_ROWS = 3;
68-
const FALLBACK_TERMINAL_ROWS = 24;
69-
70-
function maxRowOffset(rowCount: number, visibleRows: number): number {
71-
return Math.max(0, rowCount - visibleRows);
72-
}
7369

7470
export type PermissionModalProps = {
7571
request: PermissionRequest;
@@ -353,17 +349,16 @@ export function PermissionModal({
353349
const bodyViewportRows = bodyScrollable
354350
? Math.max(1, availableBodyRows - 1)
355351
: availableBodyRows;
356-
const bodyMaxOffset = maxRowOffset(bodyRows.length, bodyViewportRows);
357-
// Local, top-pinned scroll state — unlike the transcript's useScroll (which
358-
// pins to the newest/bottom line), an approval body should open showing its
352+
// Top-pinned scroll state — unlike the transcript's useScroll (which pins
353+
// to the newest/bottom line), an approval body should open showing its
359354
// start, with PageDown revealing the rest.
360-
const [bodyScrollOffset, setBodyScrollOffset] = useState(0);
361-
const clampedBodyOffset = Math.min(bodyScrollOffset, bodyMaxOffset);
355+
const bodyScroll = useScrollWindow(bodyRows.length, bodyViewportRows);
356+
const clampedBodyOffset = bodyScroll.offset;
362357
const visibleBodyRows = bodyScrollable
363358
? bodyRows.slice(clampedBodyOffset, clampedBodyOffset + bodyViewportRows)
364359
: bodyRows;
365-
const linesAbove = clampedBodyOffset;
366-
const linesBelow = Math.max(0, bodyRows.length - clampedBodyOffset - bodyViewportRows);
360+
const linesAbove = bodyScroll.above;
361+
const linesBelow = bodyScroll.below;
367362

368363
useInput((input, key) => {
369364
if (key.escape) {
@@ -388,11 +383,11 @@ export function PermissionModal({
388383
}
389384

390385
if (bodyScrollable && key.pageUp) {
391-
setBodyScrollOffset((o) => Math.max(0, o - bodyViewportRows));
386+
bodyScroll.pageUp();
392387
return;
393388
}
394389
if (bodyScrollable && key.pageDown) {
395-
setBodyScrollOffset((o) => Math.min(bodyMaxOffset, o + bodyViewportRows));
390+
bodyScroll.pageDown();
396391
return;
397392
}
398393

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
import { test, expect } from "bun:test";
2+
import { render } from "ink-testing-library";
3+
import type { ReactElement } from "react";
4+
import { Text, useInput } from "ink";
5+
import { useScrollWindow } from "./use-scroll-window.js";
6+
7+
const tick = () => new Promise((resolve) => setTimeout(resolve, 20));
8+
9+
// A minimal harness component so the hook's state (useState-backed) is
10+
// exercised the same way the modals use it: PageUp/PageDown drive offset,
11+
// and above/below report what's scrolled out of view.
12+
function Harness({ rowCount, visibleRows }: { rowCount: number; visibleRows: number }): ReactElement {
13+
const window = useScrollWindow(rowCount, visibleRows);
14+
useInput((_input, key) => {
15+
if (key.pageDown) window.pageDown();
16+
if (key.pageUp) window.pageUp();
17+
});
18+
return <Text>{`offset=${window.offset} above=${window.above} below=${window.below} max=${window.maxOffset}`}</Text>;
19+
}
20+
21+
test("useScrollWindow starts at the top with everything below in view", () => {
22+
const { lastFrame } = render(<Harness rowCount={10} visibleRows={4} />);
23+
expect(lastFrame()).toBe("offset=0 above=0 below=6 max=6");
24+
});
25+
26+
test("useScrollWindow pages down and clamps at maxOffset", async () => {
27+
const { stdin, lastFrame } = render(<Harness rowCount={10} visibleRows={4} />);
28+
stdin.write("\x1B[6~"); // PageDown
29+
await tick();
30+
expect(lastFrame()).toBe("offset=4 above=4 below=2 max=6");
31+
stdin.write("\x1B[6~"); // PageDown again — clamps instead of overshooting
32+
await tick();
33+
expect(lastFrame()).toBe("offset=6 above=6 below=0 max=6");
34+
});
35+
36+
test("useScrollWindow pages back up and clamps at 0", async () => {
37+
const { stdin, lastFrame } = render(<Harness rowCount={10} visibleRows={4} />);
38+
stdin.write("\x1B[6~"); // PageDown
39+
await tick();
40+
stdin.write("\x1B[5~"); // PageUp back to the top
41+
await tick();
42+
expect(lastFrame()).toBe("offset=0 above=0 below=6 max=6");
43+
stdin.write("\x1B[5~"); // PageUp again — clamps instead of going negative
44+
await tick();
45+
expect(lastFrame()).toBe("offset=0 above=0 below=6 max=6");
46+
});
47+
48+
test("useScrollWindow reports maxOffset 0 when content already fits", () => {
49+
const { lastFrame } = render(<Harness rowCount={3} visibleRows={4} />);
50+
expect(lastFrame()).toBe("offset=0 above=0 below=0 max=0");
51+
});

src/tui/hooks/use-scroll-window.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import { useState } from "react";
2+
3+
// Terminals that don't report a size (or report one before the first resize
4+
// event lands) fall back to a conservative row count so a modal still lays
5+
// out something scrollable rather than assuming infinite height.
6+
export const FALLBACK_TERMINAL_ROWS = 24;
7+
8+
function maxRowOffset(rowCount: number, visibleRows: number): number {
9+
return Math.max(0, rowCount - visibleRows);
10+
}
11+
12+
export type ScrollWindow = {
13+
// Current top-of-viewport row, clamped to [0, maxOffset].
14+
offset: number;
15+
maxOffset: number;
16+
// Row counts above/below the visible window, for a "N more above/below"
17+
// scroll indicator.
18+
above: number;
19+
below: number;
20+
pageUp: () => void;
21+
pageDown: () => void;
22+
};
23+
24+
// Top-pinned scroll state shared by the approval modals: a body opens showing
25+
// its start, with PageUp/PageDown clamped to [0, rowCount - visibleRows].
26+
// Both operator-modal and permission-modal had their own copy of this offset
27+
// math and paging logic; this is the single place it's owned now.
28+
export function useScrollWindow(rowCount: number, visibleRows: number): ScrollWindow {
29+
const maxOffset = maxRowOffset(rowCount, visibleRows);
30+
const [rawOffset, setOffset] = useState(0);
31+
const offset = Math.min(rawOffset, maxOffset);
32+
const above = offset;
33+
const below = Math.max(0, rowCount - offset - visibleRows);
34+
35+
const pageUp = (): void => setOffset((o) => Math.max(0, o - visibleRows));
36+
const pageDown = (): void => setOffset((o) => Math.min(maxOffset, o + visibleRows));
37+
38+
return { offset, maxOffset, above, below, pageUp, pageDown };
39+
}

0 commit comments

Comments
 (0)