fix: UI polish foundation — shared utils, component tweaks, accessibility pass#71
Merged
TerrifiedBug merged 16 commits intomainfrom Mar 8, 2026
Merged
fix: UI polish foundation — shared utils, component tweaks, accessibility pass#71TerrifiedBug merged 16 commits intomainfrom
TerrifiedBug merged 16 commits intomainfrom
Conversation
DRY — tagBadgeClass and reductionBadgeClass now live in src/lib/badge-variants.ts instead of being duplicated.
…rger tooltips, flat cards
Contributor
Greptile SummaryThis PR is a broad UI polish pass across 47 files, implementing three categories of changes: (1) extracting duplicated badge color utilities into a new Key findings:
Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
BV["src/lib/badge-variants.ts\n(new shared module)"]
BV -->|tagBadgeClass| PP["pipelines/page.tsx"]
BV -->|tagBadgeClass| PS["flow/pipeline-settings.tsx"]
BV -->|reductionBadgeClass| PP
BC["ui/badge.tsx\n+ size='sm' variant"]
BC -->|size='sm'| PP
BC -->|size='sm'| Other["other consumers"]
PH["page-header.tsx\n(new component)"]
PH --> D["page.tsx (Dashboard)"]
PH --> P["pipelines/page.tsx"]
PH --> F["fleet/page.tsx"]
PH --> FN["fleet/[nodeId]/page.tsx"]
PH --> A["analytics/page.tsx"]
PH --> AU["audit/page.tsx"]
PH --> E["environments/page.tsx"]
PH --> ED["environments/[id]/page.tsx"]
PH --> AL["alerts/page.tsx"]
PH --> PR["profile/page.tsx"]
PH --> S["settings/page.tsx"]
PH --> T["templates/page.tsx"]
UI["shadcn/ui tweaks"]
UI --> Card["card.tsx: -shadow-sm"]
UI --> Dialog["dialog.tsx: overlay /80"]
UI --> DM["dropdown-menu.tsx: rounded-md"]
UI --> Sel["select.tsx: rounded-md"]
UI --> TT["tooltip.tsx: text-sm"]
Last reviewed commit: 0f94c80 |
…rror icon, password dialog reset
Clean up dead Bell and BarChart3 imports left after PageHeader migration. Add postinstall script so prisma generate runs automatically after npm/pnpm install.
Owner
Author
|
@greptile review |
- Pipeline editor: keep skeleton loading + correct -mx-6 -my-2 margins - Pipelines list: keep Labels popover column from main, apply Badge size="sm" - Settings: keep single-team display logic from main, add cursor-pointer
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+94
to
97
| <div className="px-3 pb-3 space-y-2 group-data-[collapsible=icon]:hidden"> | ||
| <TeamSelector /> | ||
| <EnvironmentSelector /> | ||
| </div> |
Contributor
There was a problem hiding this comment.
Environment selector inaccessible when sidebar is collapsed to icon mode.
The EnvironmentSelector was moved from the top header bar (always visible) into the sidebar under a group-data-[collapsible=icon]:hidden wrapper. When a user collapses the sidebar to icon-only mode, the EnvironmentSelector becomes entirely hidden with no fallback. Previously the environment selector lived in the header and was always accessible regardless of sidebar state.
Users who collapse the sidebar to save space will have no way to switch environments without first expanding the sidebar. This is a regression from the original UX.
Consider either:
- Keeping the environment selector in the header (restore to previous location)
- Adding a fallback selector in the header when the sidebar is collapsed
- Or updating the UI to show a toast/tooltip indicating users must expand the sidebar to change environments
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/components/app-sidebar.tsx
Line: 94-97
Comment:
Environment selector inaccessible when sidebar is collapsed to icon mode.
The `EnvironmentSelector` was moved from the top header bar (always visible) into the sidebar under a `group-data-[collapsible=icon]:hidden` wrapper. When a user collapses the sidebar to icon-only mode, the `EnvironmentSelector` becomes entirely hidden with no fallback. Previously the environment selector lived in the header and was always accessible regardless of sidebar state.
Users who collapse the sidebar to save space will have no way to switch environments without first expanding the sidebar. This is a regression from the original UX.
Consider either:
- Keeping the environment selector in the header (restore to previous location)
- Adding a fallback selector in the header when the sidebar is collapsed
- Or updating the UI to show a toast/tooltip indicating users must expand the sidebar to change environments
How can I resolve this? If you propose a fix, please make it concise.When sidebar is collapsed, show icon buttons (Users, Layers) that open popovers with the full TeamSelector and EnvironmentSelector. Fixes selectors being inaccessible in collapsed mode.
Comment on lines
+21
to
+28
| case "CRASHED": return "bg-status-error-bg text-status-error-foreground"; | ||
| case "STOPPED": return "bg-status-neutral-bg text-status-neutral-foreground"; | ||
| case "STARTING": | ||
| case "PENDING": return "bg-status-degraded-bg text-status-degraded-foreground"; | ||
| default: return "bg-muted text-muted-foreground"; | ||
| } | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
Unused exported function
statusBadgeClass is exported here but never imported anywhere in the codebase (only tagBadgeClass and reductionBadgeClass are consumed by the pipeline pages). Dead exports in shared utility modules tend to drift out of sync with actual component usage over time. Either wire it up to a consumer in this PR or hold off on adding it until it's needed.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/badge-variants.ts
Line: 21-28
Comment:
**Unused exported function**
`statusBadgeClass` is exported here but never imported anywhere in the codebase (only `tagBadgeClass` and `reductionBadgeClass` are consumed by the pipeline pages). Dead exports in shared utility modules tend to drift out of sync with actual component usage over time. Either wire it up to a consumer in this PR or hold off on adding it until it's needed.
How can I resolve this? If you propose a fix, please make it concise.|
|
||
| export function PageHeader({ title, description, actions, className }: PageHeaderProps) { | ||
| return ( | ||
| <div className={cn("flex items-start justify-between gap-4 mb-6", className)}> |
Contributor
There was a problem hiding this comment.
Hardcoded mb-6 doubles vertical spacing
PageHeader bakes in mb-6 (24 px), but every call site wraps it in a space-y-6 container. In a non-flex/non-grid block context adjacent margins collapse to the larger value (no-op here), but the moment a caller's parent is a flex column the margins stack, producing 48 px between the header and the next element instead of 24 px. The mb-6 is also redundant when the parent already governs spacing via space-y-*, and it prevents overriding the gap without fighting the hardcoded value.
Removing the mb-6 from the component and letting the parent's space-y-* do the work would be the safer approach:
Suggested change
| <div className={cn("flex items-start justify-between gap-4 mb-6", className)}> | |
| <div className={cn("flex items-start justify-between gap-4", className)}> |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/components/page-header.tsx
Line: 12
Comment:
**Hardcoded `mb-6` doubles vertical spacing**
`PageHeader` bakes in `mb-6` (24 px), but every call site wraps it in a `space-y-6` container. In a non-flex/non-grid block context adjacent margins collapse to the larger value (no-op here), but the moment a caller's parent is a flex column the margins stack, producing 48 px between the header and the next element instead of 24 px. The `mb-6` is also redundant when the parent already governs spacing via `space-y-*`, and it prevents overriding the gap without fighting the hardcoded value.
Removing the `mb-6` from the component and letting the parent's `space-y-*` do the work would be the safer approach:
```suggestion
<div className={cn("flex items-start justify-between gap-4", className)}>
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
241
to
+247
| <div className="flex items-center gap-4"> | ||
| <Button variant="ghost" size="icon" onClick={() => router.push("/fleet")} aria-label="Back to fleet"> | ||
| <ArrowLeft className="h-4 w-4" /> | ||
| </Button> | ||
| <div className="flex items-center gap-1.5 text-sm"> | ||
| <Link href="/fleet" className="text-muted-foreground hover:text-foreground transition-colors"> | ||
| Fleet | ||
| </Link> | ||
| <span className="text-muted-foreground">/</span> | ||
| </div> |
Contributor
There was a problem hiding this comment.
Breadcrumb renders a trailing slash with no current-page segment
The breadcrumb div ends with <span>/</span> but the node name <div> that follows it is a sibling in the outer flex items-center gap-4 container, not inside the gap-1.5 breadcrumb box. This means "/" sits 6 px from "Fleet" but 16 px from the node name, making the separator visually disconnected. Compare this to environments/[id]/page.tsx, where the current-page name ({env.name}) is included inside the same gap-1.5 row.
Since the node name div also contains the inline rename UI, the cleanest fix is to include just the static display name as the third breadcrumb segment and keep the rename widget as the page's <h1>. Alternatively, keep the current layout but remove the trailing slash so the breadcrumb reads "Fleet" rather than "Fleet /".
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/(dashboard)/fleet/[nodeId]/page.tsx
Line: 241-247
Comment:
**Breadcrumb renders a trailing slash with no current-page segment**
The breadcrumb div ends with `<span>/</span>` but the node name `<div>` that follows it is a sibling in the outer `flex items-center gap-4` container, not inside the `gap-1.5` breadcrumb box. This means "/" sits 6 px from "Fleet" but 16 px from the node name, making the separator visually disconnected. Compare this to `environments/[id]/page.tsx`, where the current-page name (`{env.name}`) is included inside the same `gap-1.5` row.
Since the node name div also contains the inline rename UI, the cleanest fix is to include just the static display name as the third breadcrumb segment and keep the rename widget as the page's `<h1>`. Alternatively, keep the current layout but remove the trailing slash so the breadcrumb reads "Fleet" rather than "Fleet /".
How can I resolve this? If you propose a fix, please make it concise.
TerrifiedBug
added a commit
that referenced
this pull request
Mar 8, 2026
PR #71 added `"postinstall": "prisma generate"` to package.json. This breaks the Docker build because Stage 1 (deps) only copies package.json and pnpm-lock.yaml before running pnpm install — prisma generate fails when it can't find the schema. Copy prisma/ and prisma.config.ts into the deps stage so the postinstall hook succeeds.
3 tasks
TerrifiedBug
added a commit
that referenced
this pull request
Mar 8, 2026
) PR #71 added `"postinstall": "prisma generate"` to package.json. This breaks the Docker build because Stage 1 (deps) only copies package.json and pnpm-lock.yaml before running pnpm install — prisma generate fails when it can't find the schema. Copy prisma/ and prisma.config.ts into the deps stage so the postinstall hook succeeds.
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.
Summary
Phase 1 of the UI polish full sweep (Vercel/Linear aesthetic).
src/lib/badge-variants.ts): ExtractedtagBadgeClassandreductionBadgeClassfrom duplicated inline functions across 2 files into a single shared modulesmsize variant: Added a compact size option (text-[11px] px-1.5 py-0) to replace ad-hoctext-[10px]hacks in table cellsrounded-sm→rounded-mdon menu items (select + dropdown), darker dialog overlay (bg-black/80), larger tooltip text (text-sm), flat cards (removedshadow-sm)cursor-pointerto all plain interactive buttons,aria-labelto icon-only buttons,transition-colorsto hover states, fixed double labeling in theme toggleTest plan