TUI: per-agent quick-jump numbers + title-band modal system - #141
Conversation
The card redesign dropped the per-agent index column. Re-add it as a muted [N] cell after the status dot, sourced from each quick-jump entry's digit so the displayed number matches the jump key. Applies to both the worktree-grouped layout and the flat no-worktree fallback. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Introduce a `modalBand` theme token (tinted full-width window-chrome header) and rebuild `renderOverlayBox` around it: a centered, intent-tinted rounded border with a title band above a separator rule and the padded body. Route every modal through this one primitive — tool picker, launch options, name agent, create service, worktree list, graveyard/delete confirms, busy, error, switcher, migrate, teammate picker, and the two-pane notification panel. This replaces the tool-picker's bespoke solid-fill renderBox (which was also not centered) and unifies the dialog system on the design language. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
More reviews will be available in 1 hour, 27 minutes, and 44 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughRefactors ChangesOverlay Rendering Infrastructure and Migration
Dashboard Quick-Jump Index Rendering
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/tui/render/theme.ts (1)
106-112: 💤 Low valueConsider reusing
padVisibleto reduce duplication.The truncation/padding logic here duplicates what
padVisiblealready does. The function could be simplified:export function modalBand(label: string, tone: BandTone, width: number): string { const text = ` ${label}`; const w = Math.max(0, width); - const visible = visibleWidth(text); - const filled = visible >= w ? truncateAnsi(text, w) : `${text}${" ".repeat(w - visible)}`; + const filled = padVisible(text, w); return `${BAND_SGR[tone]}${filled}${RESET}`; }This eliminates the subtle difference (using
>=vs>for the truncation branch) and keeps one canonical implementation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/tui/render/theme.ts` around lines 106 - 112, The modalBand function contains truncation and padding logic that duplicates functionality already provided by the padVisible function. Replace the manual truncation and padding logic in modalBand with a call to padVisible, passing the label text and width parameters. This will eliminate code duplication, remove the subtle difference in the truncation condition (using >= vs >), and ensure consistent behavior by reusing the single canonical implementation.src/tui/render/box.ts (1)
32-37: 💤 Low valueUse
visibleWidthfor the band label measurement for consistency.The body lines use
visibleWidth(line)to measure width, butbandLabeluses.length. WhilebandLabelcurrently contains no ANSI codes, usingvisibleWidthwould be consistent with the pattern used for body lines and defensive against future changes whereiconmight include styling.- const measuredContentWidth = Math.max(bandLabel.length + 1, 0, ...body.map((line) => visibleWidth(line))); + const measuredContentWidth = Math.max(visibleWidth(bandLabel) + 1, 0, ...body.map((line) => visibleWidth(line)));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/tui/render/box.ts` around lines 32 - 37, The bandLabel measurement in the measuredContentWidth calculation uses .length while body lines use visibleWidth() for width measurement. Replace bandLabel.length with visibleWidth(bandLabel) in the Math.max call where measuredContentWidth is calculated to maintain consistency with the body line measurement pattern and be defensive against future ANSI code additions to the icon variable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/multiplexer/tool-picker.ts`:
- Around line 111-128: The tool picker overlay does not explicitly handle the
case when the tools array is empty, resulting in an interface that shows action
hints without any tools to select. Add a check before the body construction loop
in the tool-picker.ts function to detect when tools.length is zero, and if so,
insert an appropriate message into the body array (such as "No tools enabled" or
similar explanation) to clearly communicate to the user why the list is empty
and no selections are available.
---
Nitpick comments:
In `@src/tui/render/box.ts`:
- Around line 32-37: The bandLabel measurement in the measuredContentWidth
calculation uses .length while body lines use visibleWidth() for width
measurement. Replace bandLabel.length with visibleWidth(bandLabel) in the
Math.max call where measuredContentWidth is calculated to maintain consistency
with the body line measurement pattern and be defensive against future ANSI code
additions to the icon variable.
In `@src/tui/render/theme.ts`:
- Around line 106-112: The modalBand function contains truncation and padding
logic that duplicates functionality already provided by the padVisible function.
Replace the manual truncation and padding logic in modalBand with a call to
padVisible, passing the label text and width parameters. This will eliminate
code duplication, remove the subtle difference in the truncation condition
(using >= vs >), and ensure consistent behavior by reusing the single canonical
implementation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 88b651f2-8656-4ba3-91b3-250629105f1b
📒 Files selected for processing (9)
src/multiplexer/tool-picker.test.tssrc/multiplexer/tool-picker.tssrc/tui/render/box.test.tssrc/tui/render/box.tssrc/tui/render/theme.tssrc/tui/screens/dashboard-renderers.test.tssrc/tui/screens/dashboard-renderers.tssrc/tui/screens/overlay-renderers.tssrc/tui/screens/subscreen-renderers.ts
- Tool picker now shows an explicit "No enabled tools" state instead of bare action hints when no tools are enabled (CodeRabbit). - modalBand reuses padVisible instead of duplicating truncate/pad logic. - renderOverlayBox measures the band label with visibleWidth for consistency. - Guard the notification panel against a degenerate height on tiny terminals. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Summary
Two TUI design-language follow-ups.
1. Restore per-agent quick-jump numbers
The card redesign dropped the per-agent index column inside worktree groups, which made it hard to eyeball-jump to a specific agent. Re-added as a muted
[N]cell after the status dot, sourced from each quick-jump entry digit so the number you see is the number you type (worktree digit + agent digit). Applies to the worktree-grouped layout and the flat no-worktree fallback.2. Redesign the modal/dialog system (Direction B — title band)
The old dialogs were ugly and off-center: the tool-picker had its own solid-fill
renderBoxthat wasn't centered, and modals lacked a coherent identity.modalBand(label, tone, width)theme token — a tinted, full-width window-chrome header bar (info/danger), joining the existingpill/chip/keycap/cardprimitives.renderOverlayBoxrebuilt around it: centered, intent-tinted rounded border → title band → separator rule → padded body. Danger variant auto-adds the ⚠ glyph; the busy overlay passes its spinner as the band icon.renderBox(the off-center offender).Test plan
[1]/[2]within a worktree group.box.test.tsand the tool-picker overlay tests for the band structure (obsolete solid-fill assertion removed).yarn typecheck,yarn lint, fullyarn vitest(1263 tests), andyarn buildall green.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Style