Bump picker and list visible limits from 10 to 20#245
Conversation
There was a problem hiding this comment.
Pull request overview
Updates default “visible item count” settings in TUI components to show more items by default.
Changes:
- Increase the fallback list viewport height from 10 → 20 when the widget height isn’t set.
- Increase the picker default
maxVisiblefrom 10 → 20.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| internal/tui/workspace/widget/list.go | Adjusts the list widget’s default visible height fallback. |
| internal/tui/picker.go | Increases the picker’s default maximum number of visible items. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 19c0866c14
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="internal/tui/picker.go">
<violation number="1" location="internal/tui/picker.go:240">
P2: One-way ratchet: `maxVisible` is only ever reduced on `WindowSizeMsg` and never restored when the terminal grows. After a resize to a small height, the picker stays permanently cramped even if the terminal is enlarged again.
Recompute both directions by capping against the configured default instead of only shrinking. (If `WithMaxVisible` gains callers later, store the configured value in a dedicated field.)</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Interactive pickers and TUI lists showed too few items by default, requiring excessive scrolling. Doubles both the picker maxVisible default and the list widget pre-layout fallback height.
Handle tea.WindowSizeMsg so the picker never renders more rows than the terminal can display. The default 20 acts as an upper bound; short terminals get clamped down automatically.
Store the configured upper bound in maxVisibleCap so WindowSizeMsg can restore maxVisible when the terminal grows, instead of ratcheting down permanently.
Re-clamp cursor and scrollOffset after maxVisible changes so the highlight stays visible after terminal shrink. Guard WithMaxVisible against non-positive values.
6b3e98f to
eec76c0
Compare
Summary
maxVisiblein the interactive picker from 10 to 20Context
DHH feedback card 9660637689 — pickers showed too few items, requiring excessive scrolling.
WithMaxVisibleexists but no caller overrides it, so bumping the default fixes all pickers at once.Test plan
bin/cipassesSummary by cubic
Show more items in the interactive picker and TUI list by raising the visible limit from 10 to 20 to reduce scrolling.
Picker
maxVisibleclamps to terminal height viatea.WindowSizeMsg, restores up to its cap on resize, re-clamps cursor/scroll offset after resize, andWithMaxVisibleignores non-positive values; the list’s pre-layout fallback height is 20.Written for commit eec76c0. Summary will update on new commits.