Redesign graveyard screen with shared card components - #149
Merged
Conversation
Render graveyarded worktrees and standalone agents as the dashboard's rounded card() primitive in the muted tone (everything in the graveyard is dead), with keycap resurrect numbers and recency chips — replacing the old flat indented text lines so the screen matches the dashboard. Consolidate the three near-identical footer/help-hint stylers (subscreens' styleFooter, the dashboard's styleHelpGroup/buildHelpLines, overlays' hint rendering) into shared keycapHint/keycapHints/keycapHintLines helpers in theme.ts to prevent future style drift. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThree new theme exports ( ChangeskeycapHint helpers, footer migration, and graveyard card renderer
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/tui/screens/subscreen-renderers.test.ts (1)
8-8: ⚡ Quick winStabilize recency assertions by freezing test time.
Line 8 uses
Date.now()and Line 66 asserts an exact relative-time label ("1w ago"). This can become brittle as recency formatting thresholds evolve. Freeze the clock (or inject a fixednow) so this test stays deterministic.Suggested change
-import { describe, expect, it } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; @@ function renderGraveyard(graveyardIndex = 0): string { @@ describe("renderGraveyardScreen", () => { + beforeEach(() => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-01-10T00:00:00.000Z")); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + it("renders graveyarded worktrees as design-language cards", () => {Also applies to: 64-67
🤖 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/screens/subscreen-renderers.test.ts` at line 8, The `ago` function on line 8 uses `Date.now()` directly, making the test brittle because relative-time label assertions at lines 64-67 depend on the actual current time and will fail as thresholds in the codebase evolve. Freeze the test clock by either mocking Date.now() at the test setup level or refactoring the `ago` function to accept an optional `now` parameter (defaulting to Date.now() for production but allowing injection of a fixed timestamp in tests). This ensures the relative-time assertions like "1w ago" remain deterministic and stable regardless of when the test is run.
🤖 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/tui/screens/subscreen-renderers.ts`:
- Around line 464-465: The cardWidth calculation for two-pane mode at Line 464
uses 58% of cols, but the split-pane layout elsewhere (Line 503) is based on a
56% left-pane assumption, causing the graveyard card to be wider than the list
pane and clip content. Change the Math.max calculation for twoPane cardWidth to
use 56% instead of 58% to align with the split-pane sizing contract used
throughout this renderer.
---
Nitpick comments:
In `@src/tui/screens/subscreen-renderers.test.ts`:
- Line 8: The `ago` function on line 8 uses `Date.now()` directly, making the
test brittle because relative-time label assertions at lines 64-67 depend on the
actual current time and will fail as thresholds in the codebase evolve. Freeze
the test clock by either mocking Date.now() at the test setup level or
refactoring the `ago` function to accept an optional `now` parameter (defaulting
to Date.now() for production but allowing injection of a fixed timestamp in
tests). This ensures the relative-time assertions like "1w ago" remain
deterministic and stable regardless of when the test is run.
🪄 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: 527b30de-ade0-4562-9e18-311a6e3eaf4e
📒 Files selected for processing (6)
src/tui/render/theme.test.tssrc/tui/render/theme.tssrc/tui/screens/dashboard-renderers.tssrc/tui/screens/overlay-renderers.tssrc/tui/screens/subscreen-renderers.test.tssrc/tui/screens/subscreen-renderers.ts
- Render orphan agents/teammates (no worktree group) as loose selectable rows beneath their section header instead of wrapping them in a blank-titled card. - Derive the graveyard two-pane card width from a shared twoPaneLeftWidth() helper (also used by composeTwoPane) so card width matches the list pane by construction rather than a duplicated 0.58 literal. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Owner
Author
|
Concurrent sub-agent review (findings not overlapping the CodeRabbit thread):
|
This was referenced Jun 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Brings the graveyard screen into the dashboard's design language and removes footer/help-styling duplication across the TUI.
Graveyard redesign (Direction B)
card()primitive in the muted tone (everything in the graveyard is dead), replacing the old flat indented text lines.buildGraveyardCards()folds the flat view-model row stream into card blocks: each worktree → a card with its dead agents/services as body rows; each standalone-agent group → its own card.[N]resurrect numbers render as real keycaps on selectable rows, recency shows as a chip in the card summary, dead agents/services keep the muted○/◇status dots.DRY: consolidate footer/help-hint styling
There were three near-identical help/footer stylers. Consolidated into shared
theme.tshelpers —keycapHint,keycapHints,keycapHintLines:styleFooter→keycapHintsstyleHelpGroup+buildHelpLines→keycapHintLineshints()andstyleHelpLine()now build onkeycapHintPreview (ANSI stripped, 120 cols)
Tests
🤖 Generated with Claude Code
Summary by CodeRabbit