Skip to content

refactor: improve chart card spacing and remove unused routing logic#2751

Merged
akshaydeo merged 6 commits intov1.5.0from
04-16-fix_prevent_chart_layout_shifts_during_loading_states
Apr 16, 2026
Merged

refactor: improve chart card spacing and remove unused routing logic#2751
akshaydeo merged 6 commits intov1.5.0from
04-16-fix_prevent_chart_layout_shifts_during_loading_states

Conversation

@impoiler
Copy link
Copy Markdown
Contributor

@impoiler impoiler commented Apr 16, 2026

Summary

Improves chart card layout consistency and removes unnecessary width constraints from date picker components.

Changes

  • Applied className prop to ChartCard loading state for consistent styling
  • Reduced bottom margin in chart card header from mb-3 to mb-2
  • Removed inline marginBottom: 6 style from chart skeleton container
  • Added min-h-5 class to chart header legend for consistent height
  • Removed fixed width constraints from DateTimePickerWithRange button
  • Simplified logs layout by removing unused Outlet and child match logic

Type of change

  • Refactor
  • Bug fix
  • Feature
  • Documentation
  • Chore/CI

Affected areas

  • Core (Go)
  • Transports (HTTP)
  • Providers/Integrations
  • Plugins
  • UI (React)
  • Docs

How to test

Verify chart cards display consistently across different loading states and that date picker components adapt to their container width properly.

# UI
cd ui
pnpm i || npm i
pnpm test || npm test
pnpm build || npm run build

Screenshots/Recordings

If UI changes, add before/after screenshots or short clips.

Breaking changes

  • No
  • Yes

Related issues

Link related issues and discussions. Example: Closes #123

Security considerations

No security implications.

Checklist

  • I read docs/contributing/README.md and followed the guidelines
  • I added/updated tests where appropriate
  • I updated documentation where needed
  • I verified builds succeed (Go and UI)
  • I verified the CI pipeline passes locally if applicable

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 16, 2026

Warning

Rate limit exceeded

@impoiler has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 0 minutes and 47 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 0 minutes and 47 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 415a35b2-d86f-4613-87e9-6ed04cd43d31

📥 Commits

Reviewing files that changed from the base of the PR and between 8222a72 and 1811eca.

📒 Files selected for processing (8)
  • ui/app/workspace/dashboard/components/charts/chartCard.tsx
  • ui/app/workspace/dashboard/utils/chartUtils.ts
  • ui/app/workspace/logs/layout.tsx
  • ui/app/workspace/logs/views/columns.tsx
  • ui/app/workspace/logs/views/logsTable.tsx
  • ui/app/workspace/mcp-logs/views/columns.tsx
  • ui/components/table/draggableColumnHeader.tsx
  • ui/components/ui/datePickerWithRange.tsx
📝 Walkthrough

Walkthrough

Minor UI adjustments: chart loading class merging and header spacing; chart header legend min-height added; table column sizes, truncation, and pinned-width enforcement tightened; loading row height reduced; date-picker trigger width classes removed; import reorder in logs layout.

Changes

Cohort / File(s) Summary
Chart styling
ui/app/workspace/dashboard/components/charts/chartCard.tsx, ui/app/workspace/dashboard/utils/chartUtils.ts
Loading Card now merges incoming className via cn(...); header spacing changed mb-3mb-2; skeleton wrapper no longer includes inline marginBottom; added min-h-5 to CHART_HEADER_LEGEND_CLASS.
Logs layout import reorder
ui/app/workspace/logs/layout.tsx
Reordered imports so @tanstack/react-router imports appear after some local imports; no runtime or route behavior changed.
Logs columns & table rendering
ui/app/workspace/logs/views/columns.tsx, ui/app/workspace/logs/views/logsTable.tsx
Multiple columns given explicit size values; cell renderers add truncate; loading placeholder row height reduced (h-24h-12); pinned cells now apply explicit width/minWidth/maxWidth from cell.column.getSize(); TableCell gains overflow-hidden; some imports reordered.
MCP logs columns
ui/app/workspace/mcp-logs/views/columns.tsx
Timestamp column size increased (180 → 230) and timestamp cell adds truncate; actions column set to size: 72.
Draggable column header sizing
ui/components/table/draggableColumnHeader.tsx
<th> now applies explicit width, minWidth, and maxWidth derived from header.getSize() merged with existing pin styles.
Date picker trigger
ui/components/ui/datePickerWithRange.tsx
Removed conditional fixed-width Tailwind classes from the trigger Button; width is no longer enforced based on predefinedPeriod.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I nibble pixels, trim the space,

I tuck long text in tidy place,
I size the headers, fix the span,
I hop through imports, change a plan,
Charts and tables dressed with care ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (3 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The linked issue (#123) about Files API Support is completely unrelated to the PR's UI refactoring changes for chart spacing and date picker constraints. Verify the correct linked issue for this PR. The Files API Support issue does not correspond to UI layout refactoring work.
Out of Scope Changes check ⚠️ Warning The PR includes scope creep beyond stated objectives: reordering imports in logs/layout.tsx and column sizing changes in multiple table components exceed the explicit chart card and date picker refactoring goals. Either add these table/column sizing changes to PR objectives or move them to a separate refactoring PR to maintain focused scope.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main changes: refactoring chart card spacing and removing unused routing logic from logs layout.
Description check ✅ Passed The description covers all required sections with sufficient detail: summary, changes, type, affected areas, testing instructions, and checklist.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 04-16-fix_prevent_chart_layout_shifts_during_loading_states

Comment @coderabbitai help to get the list of available commands and usage tips.

@impoiler impoiler self-assigned this Apr 16, 2026
@impoiler impoiler force-pushed the 04-16-fix_prevent_chart_layout_shifts_during_loading_states branch from 44ef84c to 9c87935 Compare April 16, 2026 05:49
@impoiler impoiler force-pushed the 04-16-refactor_collapsed_filter_sidebars branch from 9037d36 to 592a24e Compare April 16, 2026 05:49
@impoiler impoiler changed the title fix: prevent chart layout shifts during loading states refactor: improve chart card spacing and remove unused routing logic Apr 16, 2026
@impoiler impoiler force-pushed the 04-16-refactor_collapsed_filter_sidebars branch from 592a24e to 7c44974 Compare April 16, 2026 08:16
@impoiler impoiler force-pushed the 04-16-fix_prevent_chart_layout_shifts_during_loading_states branch from 9c87935 to 53dccc5 Compare April 16, 2026 08:16
@impoiler impoiler force-pushed the 04-16-refactor_collapsed_filter_sidebars branch from 7c44974 to d33467f Compare April 16, 2026 08:59
@impoiler impoiler force-pushed the 04-16-fix_prevent_chart_layout_shifts_during_loading_states branch from 53dccc5 to 1e816d6 Compare April 16, 2026 08:59
@impoiler impoiler force-pushed the 04-16-refactor_collapsed_filter_sidebars branch from d33467f to 7586110 Compare April 16, 2026 09:24
@impoiler impoiler force-pushed the 04-16-fix_prevent_chart_layout_shifts_during_loading_states branch from 1e816d6 to 491178a Compare April 16, 2026 09:24
@impoiler impoiler force-pushed the 04-16-refactor_collapsed_filter_sidebars branch from 7586110 to a111be6 Compare April 16, 2026 09:25
@impoiler impoiler force-pushed the 04-16-fix_prevent_chart_layout_shifts_during_loading_states branch from 491178a to 0901b65 Compare April 16, 2026 09:25
@impoiler impoiler force-pushed the 04-16-refactor_collapsed_filter_sidebars branch from a111be6 to ebd9f4c Compare April 16, 2026 09:47
@impoiler impoiler force-pushed the 04-16-fix_prevent_chart_layout_shifts_during_loading_states branch from 0901b65 to 94d66c1 Compare April 16, 2026 09:47
@impoiler impoiler marked this pull request as ready for review April 16, 2026 10:07
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 16, 2026

Confidence Score: 5/5

Safe to merge — all changes are cosmetic spacing/class tweaks with no logic or data-flow impact.

No P0 or P1 findings. The className forwarding fix in ChartCard's loading state is the most substantive change and it is correct. Routing logic (Outlet/useChildMatches) remains intact, addressing the concern raised in the previous review thread.

No files require special attention.

Important Files Changed

Filename Overview
ui/app/workspace/dashboard/components/charts/chartCard.tsx Loading state now correctly forwards className prop and uses mb-2 (consistent with non-loading), and skeleton container no longer has inline marginBottom style.
ui/app/workspace/dashboard/utils/chartUtils.ts Added min-h-5 to CHART_HEADER_LEGEND_CLASS to ensure consistent legend height across all chart cards, preventing layout shifts when legend is empty.
ui/app/workspace/logs/layout.tsx Outlet and useChildMatches remain in place, correctly supporting the /workspace/logs/connectors child route; PR description's mention of removing them does not match the current code.
ui/components/ui/datePickerWithRange.tsx Fixed-width constraint removed from DateTimePickerWithRange trigger button, making it adapt to container width; DateTimePicker (single) retains its w-max class as before.
ui/app/workspace/logs/views/logsTable.tsx No functional changes apparent in current state; table with draggable headers, pinning, pagination, and live-update indicators all look correct.
ui/app/workspace/logs/views/columns.tsx Column definitions unchanged; all cell renderers, sizes, and sorting headers are intact.
ui/app/workspace/mcp-logs/views/columns.tsx MCP log column definitions unchanged; status validation, date formatting, and action button look correct.
ui/components/table/draggableColumnHeader.tsx Draggable column header component unchanged; drag-and-drop, pin, and hide logic all intact.

Reviews (7): Last reviewed commit: "fix: prevent chart layout shifts during ..." | Re-trigger Greptile

Comment thread ui/app/workspace/logs/layout.tsx
@impoiler impoiler force-pushed the 04-16-fix_prevent_chart_layout_shifts_during_loading_states branch from 94d66c1 to 391300c Compare April 16, 2026 10:19
@impoiler impoiler force-pushed the 04-16-refactor_collapsed_filter_sidebars branch from ebd9f4c to 39b7f3f Compare April 16, 2026 10:19
@impoiler impoiler force-pushed the 04-16-refactor_collapsed_filter_sidebars branch from 39b7f3f to 8ab3654 Compare April 16, 2026 13:38
@impoiler impoiler force-pushed the 04-16-fix_prevent_chart_layout_shifts_during_loading_states branch from 391300c to 6da298e Compare April 16, 2026 13:38
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@ui/app/workspace/logs/layout.tsx`:
- Line 3: The layout currently always renders <LogsPage /> which prevents nested
routes from mounting; update the layout component (the default export in this
file that currently returns <LogsPage />) to render <Outlet /> instead and move
<LogsPage /> into an index route so child routes like /workspace/logs/connectors
can render; ensure you import Outlet from react-router (or
`@tanstack/react-router` if using its Outlet) and remove the unconditional
<LogsPage /> render, using <Outlet /> in the layout and registering LogsPage as
the index route.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a30e8c81-d92b-4081-9c18-21b7d6add9a1

📥 Commits

Reviewing files that changed from the base of the PR and between 8ab3654 and 6da298e.

📒 Files selected for processing (8)
  • ui/app/workspace/dashboard/components/charts/chartCard.tsx
  • ui/app/workspace/dashboard/utils/chartUtils.ts
  • ui/app/workspace/logs/layout.tsx
  • ui/app/workspace/logs/views/columns.tsx
  • ui/app/workspace/logs/views/logsTable.tsx
  • ui/app/workspace/mcp-logs/views/columns.tsx
  • ui/components/table/draggableColumnHeader.tsx
  • ui/components/ui/datePickerWithRange.tsx
💤 Files with no reviewable changes (1)
  • ui/components/ui/datePickerWithRange.tsx

Comment thread ui/app/workspace/logs/layout.tsx Outdated
@impoiler impoiler force-pushed the 04-16-refactor_collapsed_filter_sidebars branch from 8ab3654 to 45a6993 Compare April 16, 2026 13:58
@impoiler impoiler force-pushed the 04-16-fix_prevent_chart_layout_shifts_during_loading_states branch from 6da298e to 85e7e4b Compare April 16, 2026 13:58
@impoiler impoiler force-pushed the 04-16-refactor_collapsed_filter_sidebars branch from 45a6993 to bb4de48 Compare April 16, 2026 13:59
@impoiler impoiler force-pushed the 04-16-fix_prevent_chart_layout_shifts_during_loading_states branch 2 times, most recently from 35f0b38 to 75d19a0 Compare April 16, 2026 14:16
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
ui/app/workspace/dashboard/components/charts/chartCard.tsx (1)

17-46: Optional: extract shared header/card shell to avoid branch drift.

Both branches duplicate the same card shell + header markup. A small extraction would reduce future divergence risk.

♻️ Suggested refactor
 export function ChartCard({ title, children, headerActions, loading, testId, height = "200px", className }: ChartCardProps) {
-	if (loading) {
-		return (
-			<Card className={cn("min-w-0 rounded-sm p-2 shadow-none", className)} data-testid={testId}>
-				<div className="mb-2 space-y-2">
-					<span className="text-primary pl-2 text-sm font-medium">{title}</span>
-					{headerActions && (
-						<div className="w-full min-w-0" data-testid={testId ? `${testId}-actions` : undefined}>
-							{headerActions}
-						</div>
-					)}
-				</div>
-				<div style={{ height }} data-testid={testId ? `${testId}-chart-skeleton` : undefined}>
-					<Skeleton className="h-full w-full" />
-				</div>
-			</Card>
-		);
-	}
-
 	return (
 		<Card className={cn("min-w-0 rounded-sm p-2 shadow-none", className)} data-testid={testId}>
 			<div className="mb-2 space-y-2">
 				<span className="text-primary pl-2 text-sm font-medium">{title}</span>
 				{headerActions && (
 					<div className="w-full min-w-0" data-testid={testId ? `${testId}-actions` : undefined}>
 						{headerActions}
 					</div>
 				)}
 			</div>
-			<div style={{ height }}>{children}</div>
+			{loading ? (
+				<div style={{ height }} data-testid={testId ? `${testId}-chart-skeleton` : undefined}>
+					<Skeleton className="h-full w-full" />
+				</div>
+			) : (
+				<div style={{ height }}>{children}</div>
+			)}
 		</Card>
 	);
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@ui/app/workspace/dashboard/components/charts/chartCard.tsx` around lines 17 -
46, The Card markup and header block are duplicated in the loading and
non-loading branches of the ChartCard component; extract the shared card shell
and header into a single helper (e.g., a renderCardShell function or a small
subcomponent like ChartCardShell) that accepts props: title, headerActions,
className, testId, height and children (or a loading flag to render Skeleton
when needed) and use that single rendering path inside ChartCard to avoid branch
drift while preserving existing symbols Card, title, headerActions, testId,
height, Skeleton, className and children.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@ui/app/workspace/dashboard/components/charts/chartCard.tsx`:
- Around line 17-46: The Card markup and header block are duplicated in the
loading and non-loading branches of the ChartCard component; extract the shared
card shell and header into a single helper (e.g., a renderCardShell function or
a small subcomponent like ChartCardShell) that accepts props: title,
headerActions, className, testId, height and children (or a loading flag to
render Skeleton when needed) and use that single rendering path inside ChartCard
to avoid branch drift while preserving existing symbols Card, title,
headerActions, testId, height, Skeleton, className and children.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7d56858d-b096-4956-92c2-410737149ecc

📥 Commits

Reviewing files that changed from the base of the PR and between 6da298e and 75d19a0.

📒 Files selected for processing (8)
  • ui/app/workspace/dashboard/components/charts/chartCard.tsx
  • ui/app/workspace/dashboard/utils/chartUtils.ts
  • ui/app/workspace/logs/layout.tsx
  • ui/app/workspace/logs/views/columns.tsx
  • ui/app/workspace/logs/views/logsTable.tsx
  • ui/app/workspace/mcp-logs/views/columns.tsx
  • ui/components/table/draggableColumnHeader.tsx
  • ui/components/ui/datePickerWithRange.tsx
💤 Files with no reviewable changes (1)
  • ui/components/ui/datePickerWithRange.tsx
✅ Files skipped from review due to trivial changes (2)
  • ui/app/workspace/logs/layout.tsx
  • ui/app/workspace/dashboard/utils/chartUtils.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • ui/app/workspace/mcp-logs/views/columns.tsx
  • ui/components/table/draggableColumnHeader.tsx

coderabbitai[bot]
coderabbitai Bot previously approved these changes Apr 16, 2026
akshaydeo
akshaydeo previously approved these changes Apr 16, 2026
Copy link
Copy Markdown
Contributor

akshaydeo commented Apr 16, 2026

Merge activity

@impoiler impoiler force-pushed the 04-16-fix_prevent_chart_layout_shifts_during_loading_states branch from 75d19a0 to 8222a72 Compare April 16, 2026 14:59
@impoiler impoiler force-pushed the 04-16-refactor_collapsed_filter_sidebars branch from bb4de48 to 1a30f40 Compare April 16, 2026 14:59
@impoiler impoiler force-pushed the 04-16-refactor_collapsed_filter_sidebars branch from 1a30f40 to 5de4598 Compare April 16, 2026 15:13
@impoiler impoiler force-pushed the 04-16-fix_prevent_chart_layout_shifts_during_loading_states branch from 8222a72 to 1811eca Compare April 16, 2026 15:13
@akshaydeo akshaydeo changed the base branch from 04-16-refactor_collapsed_filter_sidebars to graphite-base/2751 April 16, 2026 15:22
@akshaydeo akshaydeo changed the base branch from graphite-base/2751 to v1.5.0 April 16, 2026 15:24
@akshaydeo akshaydeo dismissed stale reviews from coderabbitai[bot] and themself April 16, 2026 15:24

The base branch was changed.

@akshaydeo akshaydeo merged commit daa3b09 into v1.5.0 Apr 16, 2026
5 of 13 checks passed
@akshaydeo akshaydeo deleted the 04-16-fix_prevent_chart_layout_shifts_during_loading_states branch April 16, 2026 15:25
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.

2 participants