Skip to content

Clean up css#6944

Merged
DOsinga merged 12 commits intomainfrom
cleanup-css
Feb 5, 2026
Merged

Clean up css#6944
DOsinga merged 12 commits intomainfrom
cleanup-css

Conversation

@DOsinga
Copy link
Collaborator

@DOsinga DOsinga commented Feb 4, 2026

Summary

Normalized all CSS variables to a consistent Tailwind v4-based semantic naming system, removing mixed patterns from shadcn/ui
copy-paste components and legacy naming conventions.

What Changed

1. Unified Naming Patterns

Consolidated three different naming systems into one consistent Tailwind v4 semantic system:

Before: Mixed patterns

  • shadcn/ui: bg-background, border-border, text-foreground
  • Double-prefix: text-text-default, bg-background-muted
  • Legacy camelCase: var(--text-standard), textStandard

After: Consistent semantic naming

  • bg-default, bg-muted, text-default, text-muted, border-default

Example: bg-backgroundbg-default (259 instances across components)

2. Removed Unused Variables (6)

Dropped variables with no usage:

  • --background-strong
  • --border-input
  • --background-success, --background-warning
  • --border-success, --border-warning

Example: --background-strong had 0 uses → removed from CSS

3. Fixed Undefined Variables (5)

Replaced classes that referenced non-existent CSS variables:

  • bg-accent-primarybg-accent
  • text-placeholdertext-muted
  • var(--text-standard)var(--text-default)

Example: text-placeholder was undefined → changed to text-muted (same color value)

4. Consolidated Redundant Variables (2)

Merged variables with identical values in both light and dark modes:

  • bg-appbg-default (both were #ffffff light, #22252a dark)
  • bg-cardbg-default (both were #ffffff light, #22252a dark)

Example: bg-card (7 uses) had identical values to bg-default → consolidated

5. Fixed Misused Variables (1)

Corrected variables used for wrong purposes:

  • bg-border-defaultbg-muted (was incorrectly using border color as background)

Example: Dividers in CostTracker.tsx used bg-border-default → changed to bg-muted

Copilot AI review requested due to automatic review settings February 4, 2026 14:31
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR consolidates and simplifies CSS color variable naming conventions across the desktop UI. The changes rename verbose color classes (e.g., text-text-default, bg-background-default) to shorter versions (e.g., text-default, bg-default), improving code readability and maintainability.

Changes:

  • Simplified CSS variable naming in main.css theme configuration
  • Updated all component files to use the new shorter class names
  • Removed unused/redundant CSS variables
  • Unified color token naming across light and dark themes

Reviewed changes

Copilot reviewed 133 out of 133 changed files in this pull request and generated no comments.

Show a summary per file
File Description
ui/desktop/src/styles/main.css Consolidates CSS variables, removes duplicates, creates simplified mappings for background, text, and border colors
ui/desktop/src/suspense-loader.tsx Updates text color class from text-text-muted to text-muted
ui/desktop/src/styles/search.css Updates --text-standard to --text-default and --text-prominent to --text-default
ui/desktop/src/components/ui/*.tsx Updates all UI component files with new shortened class names
ui/desktop/src/components/settings/**/*.tsx Updates all settings component files with new class names
ui/desktop/src/components/sessions/*.tsx Updates session-related components with new class names
ui/desktop/src/components/recipes/*.tsx Updates recipe components with new class names
ui/desktop/src/components/*.tsx Updates all remaining component files with new class names

@DOsinga DOsinga requested a review from zanesq February 4, 2026 16:22
Copy link
Collaborator

@aharvard aharvard left a comment

Choose a reason for hiding this comment

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

This PR introduces some pretty major visual regressions.

We may want to consider a full rewrite of all our colors, starting with the Tailwind configuration and then ensuring that all React components are properly styled.

Image Image

@DOsinga
Copy link
Collaborator Author

DOsinga commented Feb 5, 2026

yeah, I mentioned on discord. it is really a quite small error, but you know, CSS. I have a fix, but just noticed that there is a search.css and a main.css, so checking out if that is useufl

@DOsinga
Copy link
Collaborator Author

DOsinga commented Feb 5, 2026

this should be fixed now

Copilot AI review requested due to automatic review settings February 5, 2026 18:48
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 96 out of 96 changed files in this pull request and generated 4 comments.

<span className="text-xs font-mono">...</span>
</div>
<div className="w-px h-4 bg-border-default mx-2" />
<div className="w-px h-4 mx-2" />
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

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

The border color class was removed from the divider. While the visual appearance might be acceptable due to inherited styles, this makes the divider's appearance depend on CSS cascade rather than being explicit. Consider adding bg-border-default to maintain explicit styling.

Copilot generated this review using guidance from repository custom instructions.
<div
className={cn(
'absolute -top-0.5 -right-0.5 w-2 h-2 rounded-full border border-background-default',
'absolute -top-0.5 -right-0.5 w-2 h-2 rounded-full border-border-default',
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

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

The border class should be border not border-border-default. The border-border-default class doesn't exist - it should be border border-border-default to apply both the border width and color.

Copilot generated this review using guidance from repository custom instructions.
{/* Vertical line segment - full height except last item stops at middle */}
<div
className={`absolute left-0 w-px bg-border-strong ${
className={`absolute left-0 w-px ${
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

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

Border colors were removed from sidebar tree lines. While this might work with inherited styles, it reduces maintainability. Consider explicitly setting bg-border-default to match the semantic naming pattern used elsewhere.

Copilot generated this review using guidance from repository custom instructions.
/>
{/* Horizontal branch line */}
<div className="absolute left-0 w-2 h-px bg-border-strong top-1/2" />
<div className="absolute left-0 w-2 h-px top-1/2" />
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

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

Border colors were removed from sidebar tree lines. While this might work with inherited styles, it reduces maintainability. Consider explicitly setting bg-border-default to match the semantic naming pattern used elsewhere.

Copilot generated this review using guidance from repository custom instructions.
Douwe Osinga added 2 commits February 5, 2026 19:51
Fixed merge conflicts in ToolCallConfirmation.tsx and ToolCallWithResponse.tsx:
- Accepted incoming simplified component structure
- Fixed CSS classes: border-borderSubtle → border-border-default
- Fixed CSS classes: text-textStandard → text-text-default
Copilot AI review requested due to automatic review settings February 5, 2026 18:54
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 96 out of 96 changed files in this pull request and generated 5 comments.

title="Toggle Sidebar"
className={cn(
'hover:after:bg-sidebar-border absolute inset-y-0 z-20 hidden w-4 -translate-x-1/2 transition-all ease-linear group-data-[side=left]:-right-4 group-data-[side=right]:left-0 after:absolute after:inset-y-0 after:left-1/2 after:w-[2px] sm:flex',
'hover:after:border-sidebar-border absolute inset-y-0 z-20 hidden w-4 -translate-x-1/2 transition-all ease-linear group-data-[side=left]:-right-4 group-data-[side=right]:left-0 after:absolute after:inset-y-0 after:left-1/2 after:w-[2px] sm:flex',
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

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

The pseudo-element should use a background property, not border. This line should be hover:after:bg-sidebar-border instead of hover:after:border-sidebar-border. The after element creates a 2px wide vertical bar, which needs a background color, not a border.

Copilot uses AI. Check for mistakes.
Comment on lines 1516 to 1563
@@ -1529,7 +1529,7 @@ export default function ChatInput({
</TooltipTrigger>
<TooltipContent>Attach file</TooltipContent>
</Tooltip>
<div className="w-px h-4 bg-border-default mx-2" />
<div className="w-px h-4 mx-2" />
{/* Model selector, mode selector, alerts, summarize button */}
<div className="flex flex-row items-center">
{/* Cost Tracker */}
@@ -1554,13 +1554,13 @@ export default function ChatInput({
/>
</div>
</Tooltip>
<div className="w-px h-4 bg-border-default mx-2" />
<div className="w-px h-4 mx-2" />
<BottomMenuModeSelection />
<div className="w-px h-4 bg-border-default mx-2" />
<div className="w-px h-4 mx-2" />
<BottomMenuExtensionSelection sessionId={sessionId} />
{sessionId && messages.length > 0 && (
<>
<div className="w-px h-4 bg-border-default mx-2" />
<div className="w-px h-4 mx-2" />
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

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

Vertical dividers are missing their background color. These dividers previously had bg-border-default which was removed, making them invisible. They should have a background class like bg-border-default or an equivalent to remain visible.

Copilot uses AI. Check for mistakes.
{...props}
>
<ScrollAreaPrimitive.ScrollAreaThumb className="relative flex-1 rounded-full bg-border dark:bg-border-dark" />
<ScrollAreaPrimitive.ScrollAreaThumb className="relative flex-1 rounded-full bg-border dark:bg-background-muted" />
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

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

The class bg-border is undefined. There's no --border CSS variable in main.css. This should be bg-border-default or another specific border variant like bg-border-strong.

Copilot uses AI. Check for mistakes.
orientation={orientation}
className={cn(
'bg-border-default shrink-0 data-[orientation=horizontal]:h-px data-[orientation=horizontal]:w-full data-[orientation=vertical]:h-full data-[orientation=vertical]:w-px',
'bg-border shrink-0 data-[orientation=horizontal]:h-px data-[orientation=horizontal]:w-full data-[orientation=vertical]:h-full data-[orientation=vertical]:w-px',
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

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

The class bg-border is undefined. There's no --border CSS variable in main.css. This should be bg-border-default or another specific border variant.

Copilot uses AI. Check for mistakes.
Copy link
Collaborator

Choose a reason for hiding this comment

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

@DOsinga, Copilot is right (surprisingly). The lil lines are missing (right-side of screenshot)

Image

Copy link
Collaborator

Choose a reason for hiding this comment

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

and chat pipes are gone too, same issue i think
Image

data-slot="sidebar-separator"
data-sidebar="separator"
className={cn('bg-border-strong ml-5 my-2 !w-8', className)}
className={cn('bg-border ml-5 my-2 !w-8', className)}
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

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

The class bg-border is undefined. There's no --border CSS variable in main.css. This should be bg-border-default or another specific border variant.

Copilot uses AI. Check for mistakes.
@aharvard
Copy link
Collaborator

aharvard commented Feb 5, 2026

we're getting a double scrollbar now in chat (I think the darker one (the real one) has been hidden up until now for effect so that the fake scroll bar is all users see)

image

Copy link
Collaborator

@aharvard aharvard left a comment

Choose a reason for hiding this comment

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

yay, looking much better — found some minor issues

Douwe Osinga added 4 commits February 5, 2026 20:32
- Changed bg-border to bg-border-default in 4 files:
  - ui/sidebar.tsx (SidebarSeparator)
  - ui/scroll-area.tsx (ScrollAreaThumb)
  - ui/separator.tsx (Separator component)
  - ui/dropdown-menu.tsx (DropdownMenuSeparator)

All border background classes now use properly defined CSS variables.
Fixed CSS classes in ChatInput.tsx after merge:
- border-borderProminent → border-border-strong
- border-borderSubtle → border-border-default
- border-borderStandard → border-border-default
- text-textStandard → text-text-default
- text-textPlaceholder → text-text-muted
- text-textSubtle → text-text-muted
Changed Radix ScrollArea viewport to hide native scrollbar:
- scrollbar-width: auto → none
- Added webkit scrollbar hiding
- Now only the custom Radix scrollbar shows (lighter/styled one)
- Native darker scrollbar is hidden as intended
Changed tree lines from bg-border-default to bg-border-strong:
- Restores original color from main branch
- Makes lines more visible in light mode
- Matches the intended design
Copilot AI review requested due to automatic review settings February 5, 2026 19:44
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 104 out of 106 changed files in this pull request and generated 1 comment.

<div className="flex items-center justify-center h-full text-text-muted translate-y-[1px]">
<span className="text-xs font-mono">...</span>
</div>
<div className="w-px h-4 bg-border-default mx-2" />
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

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

Using bg-border-default to create visual dividers is semantically incorrect. Border variables are colors meant for borders, not backgrounds. This creates several problems:

  1. Accessibility: Using a border color as background can have incorrect contrast ratios
  2. Maintainability: Future theme changes to borders won't be expected to affect these "backgrounds"
  3. Semantics: The class name bg-border-default is confusing and doesn't describe its purpose

The dividers should use bg-muted instead for a proper subtle background, or use proper border utilities like border-r border-border-default for true borders.

Copilot uses AI. Check for mistakes.
Changed SidebarSeparator from bg-border-default to bg-border-strong to
match the tree lines and ensure the separator is visible between
Extensions and Settings menu items.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Copy link
Collaborator

@zanesq zanesq left a comment

Choose a reason for hiding this comment

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

LGTM tested locally and I don't see any visual regressions. Thanks @aharvard for catching those!

Copy link
Collaborator

Choose a reason for hiding this comment

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

@DOsinga Can you remove all these extra docs and files?

@DOsinga DOsinga added this pull request to the merge queue Feb 5, 2026
Merged via the queue into main with commit 589f7c8 Feb 5, 2026
18 checks passed
@DOsinga DOsinga deleted the cleanup-css branch February 5, 2026 23:12
katzdave added a commit that referenced this pull request Feb 6, 2026
…webtoken-10.3.0

* origin/main: (54 commits)
  Switch tetrate tool filtering back to supports_computer_use (#7024)
  feat(ui): add inline rename for chat sessions in sidebar (#6995)
  fix: handle toolnames without underscores (#7015)
  feat(claude-code): use stream-json protocol for persistent sessions (#7029)
  test(providers): add model listing to live provider suite (#7038)
  Agent added too much (#7036)
  fix(deps): bump tree-sitter to 0.26 and set sqlx default-features=false to fix RUSTSEC advisories (#7031)
  feat: add image support and improve error resilience for Codex (#7033)
  fix(providers): Azure OpenAI model listing 404 during configure (#7034)
  fix(deps): bump bat to 0.26.1 to resolve RUSTSEC-2026-0008 (#7021)
  Don't swallow Tetrate errors  (#6998)
  docs: remove hardcoded_stuff links (#7016)
  fix(ui): keep Hub chat input from overlapping SessionInsights on paste (#6719)
  Clean up css (#6944)
  docs: aws bedrock bearer token auth (#6990)
  docs: extended custom provider headers support (#7012)
  feat(cli): add type-to-search filtering to select/multiselect dialogs (#6862)
  feat(ci): add cargo-audit workflow for scanning rust vulnerabilities (#6351)
  feat: add User-Agent header to MCP HTTP requests (#6988)
  chore(deps-dev): bump webpack from 5.102.1 to 5.105.0 in /ui/desktop (#6996)
  ...

# Conflicts:
#	Cargo.lock
kuccello pushed a commit to kuccello/goose that referenced this pull request Feb 7, 2026
Co-authored-by: Douwe Osinga <douwe@squareup.com>
Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
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.

3 participants