Skip to content

Fix(app)/tui modal list overflow#99

Merged
andrewmelchor merged 2 commits intomainfrom
fix(app)/tui-modal-list-overflow
Feb 28, 2026
Merged

Fix(app)/tui modal list overflow#99
andrewmelchor merged 2 commits intomainfrom
fix(app)/tui-modal-list-overflow

Conversation

@andrewmelchor
Copy link
Copy Markdown
Member

Summary

Improve the TUI dialog experience by fixing picker list overflow so content stays visually contained within the modal, even when there are many matches. Picker dialogs now scroll cleanly while keeping the active selection in view during keyboard navigation, which makes browsing and selecting items more stable, predictable, and consistent with the expected UX.

@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Feb 28, 2026

Greptile Summary

This PR successfully fixes TUI modal list overflow by replacing box elements with proper scrollbox components and centralizing scroll management logic. The refactoring introduces a new useScrollToIndex hook that handles automatic scrolling to keep selected items in view across five picker/list components.

Key changes:

  • Created centralized useScrollToIndex hook with reactive tracking via createEffect
  • Replaced box with maxHeight={10} with scrollbox with height={10} for proper scrollable containers
  • Added flexShrink={0} to list items to prevent unwanted compression
  • Migrated scrollRef management from direct variable assignment to SolidJS signals (createSignal)
  • Maintained all existing UX behaviors including pending confirmations, fuzzy search, and keyboard navigation

The execution-list.tsx component maintains dual scrolling behavior (auto-scroll to bottom during runs + scroll to selection) which appears intentional for the test execution workflow.

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk - it's a well-executed refactoring with consistent patterns and proper reactive implementations
  • The changes follow consistent patterns across all affected components, use proper SolidJS reactive primitives, and centralize duplicated logic into a reusable hook. The refactoring maintains all existing functionality while fixing the overflow issue. Code quality is high with clear separation of concerns.
  • No files require special attention - all changes follow the same pattern and are straightforward

Important Files Changed

Filename Overview
packages/app/src/tui/hooks/use-scroll-to-index.ts New hook that centralizes scroll-to-index logic for list components; clean implementation with proper reactive tracking
packages/app/src/tui/components/dialog-select.tsx Converted from box+maxHeight to scrollbox+height, added flexShrink={0} to items, integrated useScrollToIndex hook
packages/app/src/tui/components/execution-list.tsx Refactored to use useScrollToIndex hook while maintaining auto-scroll-to-bottom behavior during test runs; dual scrolling effects are intentional
packages/app/src/tui/components/file-request-picker.tsx Converted to scrollbox with fixed height and integrated useScrollToIndex hook; maintains existing picker behavior

Last reviewed commit: 2ab5ccf

@andrewmelchor andrewmelchor merged commit b51beac into main Feb 28, 2026
2 checks passed
@andrewmelchor andrewmelchor deleted the fix(app)/tui-modal-list-overflow branch February 28, 2026 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant