Skip to content

Commit b51beac

Browse files
Fix(app)/tui modal list overflow (#99)
* fix(app): constrain picker lists to modal bounds with scrollbox * centralize scroll index logic for tui modal
1 parent a67320c commit b51beac

File tree

7 files changed

+109
-51
lines changed

7 files changed

+109
-51
lines changed

packages/app/src/tui/components/dialog-select.tsx

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
1+
import type { ScrollBoxRenderable } from '@opentui/core';
12
import { useKeyboard } from '@opentui/solid';
23
import fuzzysort from 'fuzzysort';
34
import { createMemo, createSignal, For, type JSX, onMount } from 'solid-js';
45
import type { DialogContextValue } from '../context/dialog';
6+
import { useScrollToIndex } from '../hooks';
57
import { rgba, theme } from '../theme';
68
import { normalizeKey } from '../util/normalize-key';
79

@@ -24,6 +26,7 @@ export type DialogSelectProps<T> = {
2426
export function DialogSelect<T>(props: DialogSelectProps<T>): JSX.Element {
2527
const [query, setQuery] = createSignal('');
2628
const [selectedIndex, setSelectedIndex] = createSignal(0);
29+
const [scrollRef, setScrollRef] = createSignal<ScrollBoxRenderable | undefined>(undefined);
2730
let inputRef: { focus: () => void } | undefined;
2831

2932
const filteredOptions = createMemo(() => {
@@ -87,6 +90,12 @@ export function DialogSelect<T>(props: DialogSelectProps<T>): JSX.Element {
8790
setTimeout(() => inputRef?.focus(), 1);
8891
});
8992

93+
useScrollToIndex({
94+
scrollRef,
95+
selectedIndex: clampedIndex,
96+
itemCount: () => filteredOptions().length
97+
});
98+
9099
return (
91100
<box flexDirection="column" gap={1} paddingBottom={1}>
92101
{/* Title bar with escape hint */}
@@ -121,13 +130,19 @@ export function DialogSelect<T>(props: DialogSelectProps<T>): JSX.Element {
121130
</box>
122131

123132
{/* Options list */}
124-
<box flexDirection="column" maxHeight={10}>
133+
<scrollbox
134+
ref={(r) => {
135+
setScrollRef(r);
136+
}}
137+
height={10}
138+
>
125139
<For each={filteredOptions()}>
126140
{(opt, idx) => {
127141
const isSelected = () => idx() === clampedIndex();
128142
return (
129143
<box
130144
height={1}
145+
flexShrink={0}
131146
paddingLeft={2}
132147
paddingRight={2}
133148
flexDirection="row"
@@ -151,7 +166,7 @@ export function DialogSelect<T>(props: DialogSelectProps<T>): JSX.Element {
151166
<text fg={rgba(theme.textMuted)}>No matches</text>
152167
</box>
153168
)}
154-
</box>
169+
</scrollbox>
155170
</box>
156171
);
157172
}

packages/app/src/tui/components/execution-list.tsx

Lines changed: 15 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { ScrollBoxRenderable } from '@opentui/core';
2-
import { createEffect, For, Show } from 'solid-js';
2+
import { createEffect, createSignal, For, Show } from 'solid-js';
3+
import { useScrollToIndex } from '../hooks';
34
import type { ExecutionStatus, ExecutionSummary } from '../observer-store';
45
import { getMethodColor, rgba, theme } from '../theme';
56
import { formatDuration } from '../util/format';
@@ -28,33 +29,25 @@ function getStatusDisplay(status: ExecutionStatus): { icon: string; color: strin
2829
}
2930

3031
export function ExecutionList(props: ExecutionListProps) {
31-
let scrollRef: ScrollBoxRenderable | undefined;
32+
const [scrollRef, setScrollRef] = createSignal<ScrollBoxRenderable | undefined>(undefined);
3233

33-
// Scroll to selected when it changes
34-
createEffect(() => {
35-
const id = props.selectedId;
36-
if (!scrollRef || !id) return;
37-
38-
const index = props.executions.findIndex((e) => e.reqExecId === id);
39-
if (index < 0) return;
40-
41-
const viewportHeight = scrollRef.height;
42-
const scrollTop = scrollRef.scrollTop;
43-
const scrollBottom = scrollTop + viewportHeight;
44-
45-
if (index < scrollTop) {
46-
scrollRef.scrollBy(index - scrollTop);
47-
} else if (index + 1 > scrollBottom) {
48-
scrollRef.scrollBy(index + 1 - scrollBottom);
49-
}
34+
useScrollToIndex({
35+
scrollRef,
36+
selectedIndex: () => {
37+
const id = props.selectedId;
38+
if (!id) return -1;
39+
return props.executions.findIndex((e) => e.reqExecId === id);
40+
},
41+
itemCount: () => props.executions.length
5042
});
5143

5244
// Auto-scroll to bottom when new executions arrive (if running)
5345
createEffect(() => {
54-
if (!scrollRef || !props.isRunning) return;
46+
const ref = scrollRef();
47+
if (!ref || !props.isRunning) return;
5548
const len = props.executions.length;
5649
if (len > 0) {
57-
scrollRef.scrollTo(len - 1);
50+
ref.scrollTo(len - 1);
5851
}
5952
});
6053

@@ -81,7 +74,7 @@ export function ExecutionList(props: ExecutionListProps) {
8174
</box>
8275
<scrollbox
8376
ref={(r) => {
84-
scrollRef = r;
77+
setScrollRef(r);
8578
}}
8679
flexGrow={1}
8780
paddingLeft={1}

packages/app/src/tui/components/file-request-picker.tsx

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import type { ScrollBoxRenderable } from '@opentui/core';
12
import type { WorkspaceRequest } from '@t-req/sdk/client';
23
import fuzzysort from 'fuzzysort';
34
import {
@@ -11,7 +12,7 @@ import {
1112
Show
1213
} from 'solid-js';
1314
import { useDialog, useStore } from '../context';
14-
import { usePickerNavigation } from '../hooks';
15+
import { usePickerNavigation, useScrollToIndex } from '../hooks';
1516
import { isHttpFile, isRunnableScript, isTestFile } from '../store';
1617
import { rgba, theme } from '../theme';
1718
import { RequestPicker } from './request-picker';
@@ -39,6 +40,7 @@ const CONFIRM_TIMEOUT_MS = 2000;
3940
export function FileRequestPicker(props: FileRequestPickerProps): JSX.Element {
4041
const store = useStore();
4142
const dialog = useDialog();
43+
const [scrollRef, setScrollRef] = createSignal<ScrollBoxRenderable | undefined>(undefined);
4244

4345
const [query, setQuery] = createSignal('');
4446
const [pendingSendId, setPendingSendId] = createSignal<string | undefined>(undefined);
@@ -184,6 +186,12 @@ export function FileRequestPicker(props: FileRequestPickerProps): JSX.Element {
184186
}
185187
});
186188

189+
useScrollToIndex({
190+
scrollRef,
191+
selectedIndex: clampedIndex,
192+
itemCount: () => filteredItems().length
193+
});
194+
187195
// Clear pending send when query or selection changes
188196
createEffect(() => {
189197
query();
@@ -243,7 +251,12 @@ export function FileRequestPicker(props: FileRequestPickerProps): JSX.Element {
243251
</box>
244252

245253
{/* Items list */}
246-
<box flexDirection="column" maxHeight={10}>
254+
<scrollbox
255+
ref={(r) => {
256+
setScrollRef(r);
257+
}}
258+
height={10}
259+
>
247260
<For each={filteredItems()}>
248261
{(item, idx) => {
249262
const isSelected = () => idx() === clampedIndex();
@@ -254,6 +267,7 @@ export function FileRequestPicker(props: FileRequestPickerProps): JSX.Element {
254267
return (
255268
<box
256269
height={1}
270+
flexShrink={0}
257271
paddingLeft={2}
258272
paddingRight={2}
259273
flexDirection="row"
@@ -288,7 +302,7 @@ export function FileRequestPicker(props: FileRequestPickerProps): JSX.Element {
288302
<text fg={rgba(theme.textMuted)}>No matches</text>
289303
</box>
290304
</Show>
291-
</box>
305+
</scrollbox>
292306

293307
{/* Action bar */}
294308
<box height={1} paddingLeft={2} flexDirection="row" gap={2}>

packages/app/src/tui/components/file-tree.tsx

Lines changed: 8 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { ScrollBoxRenderable } from '@opentui/core';
2-
import { createEffect, For, Show } from 'solid-js';
2+
import { createSignal, For, Show } from 'solid-js';
3+
import { useScrollToIndex } from '../hooks';
34
import type { FlatNode } from '../store';
45
import { rgba, theme } from '../theme';
56

@@ -11,26 +12,12 @@ export interface FileTreeProps {
1112
}
1213

1314
export function FileTree(props: FileTreeProps) {
14-
let scrollRef: ScrollBoxRenderable | undefined;
15+
const [scrollRef, setScrollRef] = createSignal<ScrollBoxRenderable | undefined>(undefined);
1516

16-
// Scroll to selection when it changes
17-
createEffect(() => {
18-
const idx = props.selectedIndex;
19-
if (!scrollRef || idx < 0) return;
20-
21-
// Each row is height=1, so index equals Y position in scroll content
22-
const viewportHeight = scrollRef.height;
23-
const scrollTop = scrollRef.scrollTop;
24-
const scrollBottom = scrollTop + viewportHeight;
25-
26-
// Check if selected item is outside visible range
27-
if (idx < scrollTop) {
28-
// Item is above viewport - scroll up
29-
scrollRef.scrollBy(idx - scrollTop);
30-
} else if (idx + 1 > scrollBottom) {
31-
// Item is below viewport - scroll down
32-
scrollRef.scrollBy(idx + 1 - scrollBottom);
33-
}
17+
useScrollToIndex({
18+
scrollRef,
19+
selectedIndex: () => props.selectedIndex,
20+
itemCount: () => props.nodes.length
3421
});
3522

3623
return (
@@ -47,7 +34,7 @@ export function FileTree(props: FileTreeProps) {
4734
</box>
4835
<scrollbox
4936
ref={(r) => {
50-
scrollRef = r;
37+
setScrollRef(r);
5138
}}
5239
flexGrow={1}
5340
paddingLeft={1}

packages/app/src/tui/components/request-picker.tsx

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import type { ScrollBoxRenderable } from '@opentui/core';
12
import type { WorkspaceRequest } from '@t-req/sdk/client';
23
import fuzzysort from 'fuzzysort';
34
import {
@@ -11,7 +12,7 @@ import {
1112
Show
1213
} from 'solid-js';
1314
import { useDialog } from '../context';
14-
import { usePickerNavigation } from '../hooks';
15+
import { usePickerNavigation, useScrollToIndex } from '../hooks';
1516
import { getMethodColor, rgba, theme } from '../theme';
1617

1718
interface RequestItem {
@@ -36,6 +37,7 @@ const CONFIRM_TIMEOUT_MS = 2000;
3637

3738
export function RequestPicker(props: RequestPickerProps): JSX.Element {
3839
const dialog = useDialog();
40+
const [scrollRef, setScrollRef] = createSignal<ScrollBoxRenderable | undefined>(undefined);
3941

4042
const [query, setQuery] = createSignal('');
4143
const [pendingSendId, setPendingSendId] = createSignal<string | undefined>(undefined);
@@ -105,6 +107,12 @@ export function RequestPicker(props: RequestPickerProps): JSX.Element {
105107
}
106108
});
107109

110+
useScrollToIndex({
111+
scrollRef,
112+
selectedIndex: clampedIndex,
113+
itemCount: () => filteredItems().length
114+
});
115+
108116
// Clear pending send when query or selection changes
109117
createEffect(() => {
110118
query();
@@ -156,7 +164,12 @@ export function RequestPicker(props: RequestPickerProps): JSX.Element {
156164
</box>
157165

158166
{/* Request list */}
159-
<box flexDirection="column" maxHeight={10}>
167+
<scrollbox
168+
ref={(r) => {
169+
setScrollRef(r);
170+
}}
171+
height={10}
172+
>
160173
<For each={filteredItems()}>
161174
{(item, idx) => {
162175
const isSelected = () => idx() === clampedIndex();
@@ -165,6 +178,7 @@ export function RequestPicker(props: RequestPickerProps): JSX.Element {
165178
return (
166179
<box
167180
height={1}
181+
flexShrink={0}
168182
paddingLeft={2}
169183
paddingRight={2}
170184
flexDirection="row"
@@ -196,7 +210,7 @@ export function RequestPicker(props: RequestPickerProps): JSX.Element {
196210
<text fg={rgba(theme.textMuted)}>No matches</text>
197211
</box>
198212
</Show>
199-
</box>
213+
</scrollbox>
200214

201215
{/* Action bar */}
202216
<box height={1} paddingLeft={2} flexDirection="row" gap={2}>

packages/app/src/tui/hooks/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ export {
1818
type ScriptRunnerReturn,
1919
useScriptRunner
2020
} from './use-script-runner';
21+
export { type UseScrollToIndexOptions, useScrollToIndex } from './use-scroll-to-index';
2122
export {
2223
type TestRunnerOptions,
2324
type TestRunnerReturn,
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import type { ScrollBoxRenderable } from '@opentui/core';
2+
import { createEffect } from 'solid-js';
3+
4+
export interface UseScrollToIndexOptions {
5+
scrollRef: () => ScrollBoxRenderable | undefined;
6+
selectedIndex: () => number;
7+
itemCount: () => number;
8+
}
9+
10+
/**
11+
* Keep a selected row index visible in a scrollbox with fixed-height rows.
12+
*/
13+
export function useScrollToIndex(opts: UseScrollToIndexOptions): void {
14+
createEffect(() => {
15+
const scrollRef = opts.scrollRef();
16+
const itemCount = opts.itemCount();
17+
const index = opts.selectedIndex();
18+
19+
if (!scrollRef || itemCount === 0 || index < 0) return;
20+
21+
const viewportHeight = scrollRef.height;
22+
const scrollTop = scrollRef.scrollTop;
23+
const scrollBottom = scrollTop + viewportHeight;
24+
25+
if (index < scrollTop) {
26+
scrollRef.scrollBy(index - scrollTop);
27+
return;
28+
}
29+
30+
if (index + 1 > scrollBottom) {
31+
scrollRef.scrollBy(index + 1 - scrollBottom);
32+
}
33+
});
34+
}

0 commit comments

Comments
 (0)