Skip to content

Fix dark-mode styling for skills filter chips#1660

Merged
BunsDev merged 1 commit intomainfrom
okcode/fix-dark-skills-buttons
Apr 13, 2026
Merged

Fix dark-mode styling for skills filter chips#1660
BunsDev merged 1 commit intomainfrom
okcode/fix-dark-skills-buttons

Conversation

@BunsDev
Copy link
Copy Markdown
Member

@BunsDev BunsDev commented Apr 13, 2026

  • Add readable dark-surface and active-state colors to filter chips
  • Cover the toolbar styling with a jsdom test

- Add readable dark-surface and active-state colors to filter chips
- Cover the toolbar styling with a jsdom test
@vercel
Copy link
Copy Markdown
Contributor

vercel bot commented Apr 13, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
clawhub Ready Ready Preview, Comment Apr 13, 2026 6:01pm

@BunsDev BunsDev merged commit aeab23a into main Apr 13, 2026
7 checks passed
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 13, 2026

Greptile Summary

Adds dark-mode class tokens (dark:bg-*, dark:border-*, dark:text-*) to FilterChip in -SkillsToolbar.tsx and backs the change with a new jsdom test that asserts the class strings are present. The implementation is a straightforward CSS-only fix with no data or logic changes.

  • The one P2 note is that the class string now holds conflicting dark:bg-* pairs simultaneously when active is true (base class + active branch), a pattern also pre-existing in light mode. Using the project's cn() helper (src/lib/utils.ts) would let tailwind-merge drop the overridden base class reliably.

Confidence Score: 5/5

Safe to merge — changes are CSS-only with no logic, data, or API impact.

All findings are P2 style suggestions. The conflicting Tailwind utility issue is pre-existing in the component and the PR extends it consistently; it does not introduce a new regression. Tests are correct and well-targeted.

No files require special attention.

Prompt To Fix All With AI
This is a comment left during a code review.
Path: src/routes/skills/-SkillsToolbar.tsx
Line: 263-267

Comment:
**Conflicting Tailwind utilities in active state**

When `active` is `true`, the class string includes both `dark:bg-[rgba(14,28,37,0.84)]` (base) and `dark:bg-[rgba(255,131,95,0.14)]` (active branch) simultaneously — same for the border pair. Without `tailwind-merge`, CSS output order (not class-string order) determines the winner, making the active dark-mode background non-deterministic. The same pre-existing conflict exists in light mode (`bg-[rgba(255,255,255,0.94)]` vs `bg-[color:var(--accent)]/10`). Since the codebase already exports a `cn()` helper (`src/lib/utils.ts`), switching to it here would resolve all these conflicts safely:

```tsx
import { cn } from "../../lib/utils";

//
className={cn(
  "inline-flex min-h-[36px] items-center gap-1.5 rounded-[var(--radius-sm)] border px-3.5 text-xs font-semibold transition-all duration-150",
  active
    ? "border-[color:var(--accent)]/30 bg-[color:var(--accent)]/10 text-[color:var(--accent)] dark:border-[rgba(255,131,95,0.34)] dark:bg-[rgba(255,131,95,0.14)] dark:text-[#ffd5c9]"
    : "border-[rgba(29,59,78,0.22)] bg-[rgba(255,255,255,0.94)] text-[color:var(--ink-soft)] hover:border-[color:var(--border-ui-hover)] hover:text-[color:var(--ink)] dark:border-[rgba(255,255,255,0.12)] dark:bg-[rgba(14,28,37,0.84)] dark:text-[rgba(245,238,232,0.88)] dark:hover:border-[rgba(255,255,255,0.2)] dark:hover:text-[rgba(245,238,232,0.96)]",
)}
```

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "Fix dark-mode styling for skills filter ..." | Re-trigger Greptile

Comment on lines +263 to 267
className={`inline-flex min-h-[36px] items-center gap-1.5 rounded-[var(--radius-sm)] border border-[rgba(29,59,78,0.22)] bg-[rgba(255,255,255,0.94)] px-3.5 text-xs font-semibold transition-all duration-150 dark:border-[rgba(255,255,255,0.12)] dark:bg-[rgba(14,28,37,0.84)] ${
active
? "border-[color:var(--accent)]/30 bg-[color:var(--accent)]/10 text-[color:var(--accent)]"
: "text-[color:var(--ink-soft)] hover:border-[color:var(--border-ui-hover)] hover:text-[color:var(--ink)] dark:text-[rgba(245,238,232,0.88)] dark:hover:text-[rgba(245,238,232,0.96)]"
? "border-[color:var(--accent)]/30 bg-[color:var(--accent)]/10 text-[color:var(--accent)] dark:border-[rgba(255,131,95,0.34)] dark:bg-[rgba(255,131,95,0.14)] dark:text-[#ffd5c9]"
: "text-[color:var(--ink-soft)] hover:border-[color:var(--border-ui-hover)] hover:text-[color:var(--ink)] dark:text-[rgba(245,238,232,0.88)] dark:hover:border-[rgba(255,255,255,0.2)] dark:hover:text-[rgba(245,238,232,0.96)]"
}`}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Conflicting Tailwind utilities in active state

When active is true, the class string includes both dark:bg-[rgba(14,28,37,0.84)] (base) and dark:bg-[rgba(255,131,95,0.14)] (active branch) simultaneously — same for the border pair. Without tailwind-merge, CSS output order (not class-string order) determines the winner, making the active dark-mode background non-deterministic. The same pre-existing conflict exists in light mode (bg-[rgba(255,255,255,0.94)] vs bg-[color:var(--accent)]/10). Since the codebase already exports a cn() helper (src/lib/utils.ts), switching to it here would resolve all these conflicts safely:

import { cn } from "../../lib/utils";

// …
className={cn(
  "inline-flex min-h-[36px] items-center gap-1.5 rounded-[var(--radius-sm)] border px-3.5 text-xs font-semibold transition-all duration-150",
  active
    ? "border-[color:var(--accent)]/30 bg-[color:var(--accent)]/10 text-[color:var(--accent)] dark:border-[rgba(255,131,95,0.34)] dark:bg-[rgba(255,131,95,0.14)] dark:text-[#ffd5c9]"
    : "border-[rgba(29,59,78,0.22)] bg-[rgba(255,255,255,0.94)] text-[color:var(--ink-soft)] hover:border-[color:var(--border-ui-hover)] hover:text-[color:var(--ink)] dark:border-[rgba(255,255,255,0.12)] dark:bg-[rgba(14,28,37,0.84)] dark:text-[rgba(245,238,232,0.88)] dark:hover:border-[rgba(255,255,255,0.2)] dark:hover:text-[rgba(245,238,232,0.96)]",
)}
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/routes/skills/-SkillsToolbar.tsx
Line: 263-267

Comment:
**Conflicting Tailwind utilities in active state**

When `active` is `true`, the class string includes both `dark:bg-[rgba(14,28,37,0.84)]` (base) and `dark:bg-[rgba(255,131,95,0.14)]` (active branch) simultaneously — same for the border pair. Without `tailwind-merge`, CSS output order (not class-string order) determines the winner, making the active dark-mode background non-deterministic. The same pre-existing conflict exists in light mode (`bg-[rgba(255,255,255,0.94)]` vs `bg-[color:var(--accent)]/10`). Since the codebase already exports a `cn()` helper (`src/lib/utils.ts`), switching to it here would resolve all these conflicts safely:

```tsx
import { cn } from "../../lib/utils";

//
className={cn(
  "inline-flex min-h-[36px] items-center gap-1.5 rounded-[var(--radius-sm)] border px-3.5 text-xs font-semibold transition-all duration-150",
  active
    ? "border-[color:var(--accent)]/30 bg-[color:var(--accent)]/10 text-[color:var(--accent)] dark:border-[rgba(255,131,95,0.34)] dark:bg-[rgba(255,131,95,0.14)] dark:text-[#ffd5c9]"
    : "border-[rgba(29,59,78,0.22)] bg-[rgba(255,255,255,0.94)] text-[color:var(--ink-soft)] hover:border-[color:var(--border-ui-hover)] hover:text-[color:var(--ink)] dark:border-[rgba(255,255,255,0.12)] dark:bg-[rgba(14,28,37,0.84)] dark:text-[rgba(245,238,232,0.88)] dark:hover:border-[rgba(255,255,255,0.2)] dark:hover:text-[rgba(245,238,232,0.96)]",
)}
```

How can I resolve this? If you propose a fix, please make it concise.

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.

1 participant