fix max height for card content on traces conversation page#1905
fix max height for card content on traces conversation page#1905shagun-singh-inkeep merged 7 commits intomainfrom
Conversation
🦋 Changeset detectedLatest commit: 1e7d2ee The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No docs changes detected — this PR contains internal UI styling fixes (max-height adjustments for card components) that don't require documentation updates. |
There was a problem hiding this comment.
PR Review Summary
0 Key Findings | Risk: Low
This PR correctly implements max-height constraints on the summary cards in the conversation trace detail page, preventing content overflow. The changes are straightforward CSS styling improvements that follow existing flexbox patterns in the codebase.
🔴❗ Critical (0) ❗🔴
None.
🟠⚠️ Major (0) 🟠⚠️
None.
🟡 Minor (0) 🟡
None.
💭 Consider (2) 💭
- 💭 Consider:
page.tsx:318Scrollbar styling for consistency
💭 1) page.tsx, mcp-breakdown-card.tsx Standardize overflow patterns across all four summary cards
Issue: The four summary cards in the grid use different internal overflow strategies. Duration and Alerts cards have no CardContent overflow handling, AI Calls uses overflow-y-auto directly on CardContent, and MCPBreakdownCard uses overflow-hidden + nested scrollable div.
Why: While this works correctly (Duration/Alerts content is unlikely to overflow), consistent patterns make maintenance easier and future-proof the cards if content grows.
Fix: This is a stylistic consideration. The current approach (only adding scroll behavior where content can grow unbounded) is pragmatic. If desired, standardize by adding flex-shrink-0 to all CardHeaders and flex-1 min-h-0 overflow-y-auto to all CardContents.
Refs:
- page.tsx:262 — Duration card
- page.tsx:373 — Alerts card
✅ APPROVE
Summary: Clean, focused UI fix that achieves its goal. The max-h-[280px] flex flex-col pattern is applied consistently to all four summary cards, and scroll behavior is correctly added where content can grow unbounded (AI Calls, MCP Tool Calls). The Consider items are minor consistency refinements that can be addressed in a follow-up if desired. Ship it! 🚀
Discarded (2)
| Location | Issue | Reason Discarded |
|---|---|---|
mcp-breakdown-card.tsx:103 |
Nested scroll pattern differs from AI Calls card | Both patterns work correctly; this is a low-confidence style preference with no functional impact |
page.tsx:262,373 |
Duration/Alerts cards lack overflow handling | Intentional design — these cards have fixed-height content that won't overflow |
Reviewers (3)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-standards |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
pr-review-consistency |
2 | 0 | 1 | 0 | 1 | 0 | 0 |
pr-review-frontend |
2 | 0 | 0 | 0 | 0 | 0 | 2 |
| Total | 4 | 0 | 1 | 0 | 1 | 0 | 2 |
| <MessageSquare className="h-4 w-4 text-muted-foreground" /> | ||
| </CardHeader> | ||
| <CardContent> | ||
| <CardContent className="flex-1 min-h-0 overflow-y-auto"> |
There was a problem hiding this comment.
💭 Consider: Scrollbar styling for consistency
Issue: This scrollable CardContent doesn't include the scrollbar styling classes used elsewhere in the traces components.
Why: The codebase has an established pattern of styled scrollbars (scrollbar-thin scrollbar-thumb-muted-foreground/30 scrollbar-track-transparent dark:scrollbar-thumb-muted-foreground/50) for a more refined look. Adding them here would match the pattern in activity-details-sidepane.tsx and tool-calls-by-server-card.tsx.
Fix: Consider adding scrollbar classes:
<CardContent className="flex-1 min-h-0 overflow-y-auto scrollbar-thin scrollbar-thumb-muted-foreground/30 scrollbar-track-transparent dark:scrollbar-thumb-muted-foreground/50">Refs:
- activity-details-sidepane.tsx:29 — established scrollbar styling pattern
- tool-calls-by-server-card.tsx:140 — another example in traces
No description provided.