feat: color system DX - dark fallback + theme-aware palette - #25
Conversation
… palette - Add dark-mode fallback to all 11 resolved color tokens so dark mode falls back to the light value when no explicit -dark override is set - Replace literal white/black with var(--sf-color-base)/var(--sf-color-text) in palette color-mix() calls to make tint/shade scale theme-aware (fixes #24) - Update THEMOWANIE documentation to reflect the new 6-token override model
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughCore semantic tokens now support quick-branding via light-only overrides with dark-side fallback; palette numeric scales mix with theme-aware base/text tokens instead of literal white/black; demo page links and documentation add a scoped-themes section and local asset loading. ChangesColor Token Theming & Palette Fallback
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
optional/tokens.palette.css (1)
12-13:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate outdated comment to reflect theme-aware implementation.
The comment still refers to mixing with literal
whiteandblack, but the implementation now mixes withvar(--sf-color-base)andvar(--sf-color-text)respectively.📝 Proposed fix
Token naming: - --sf-color-X-100 … -400 tints (mixed with white) - --sf-color-X-600 … -900 shades (mixed with black) + --sf-color-X-100 … -400 tints (mixed toward base) + --sf-color-X-600 … -900 shades (mixed toward text) --sf-color-X-a10 … -a75 alpha (mixed with transparent)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@optional/tokens.palette.css` around lines 12 - 13, Update the outdated comment in tokens.palette.css that describes color mixing for the variables (--sf-color-X-100 … -400 and --sf-color-X-600 … -900) to reflect the theme-aware implementation: state that tints are created by mixing with var(--sf-color-base) and shades by mixing with var(--sf-color-text) (instead of literal "white" and "black"), and mention that this makes the palette respect current theme colors.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@optional/tokens.palette.css`:
- Around line 115-118: The four base tint tokens (--sf-color-base-100,
--sf-color-base-200, --sf-color-base-300, --sf-color-base-400) currently mix
var(--sf-color-base) with itself which collapses to a single value; change the
second operand to var(--sf-color-text) so the scale ramps from base toward text
(i.e., use color-mix(in oklch, var(--sf-color-text) <percent>,
var(--sf-color-base)) and keep the intended percentages (8%, 20%, 40%, 65%) so
each token progressively shifts from the base color toward the text color).
---
Outside diff comments:
In `@optional/tokens.palette.css`:
- Around line 12-13: Update the outdated comment in tokens.palette.css that
describes color mixing for the variables (--sf-color-X-100 … -400 and
--sf-color-X-600 … -900) to reflect the theme-aware implementation: state that
tints are created by mixing with var(--sf-color-base) and shades by mixing with
var(--sf-color-text) (instead of literal "white" and "black"), and mention that
this makes the palette respect current theme colors.
🪄 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 Plus
Run ID: e2087d73-2b9f-4efe-9707-ae9d6e498b6d
⛔ Files ignored due to path filters (1)
dist/slashed.essential.cssis excluded by!**/dist/**
📒 Files selected for processing (2)
core/tokens.cssoptional/tokens.palette.css
| --sf-color-base-100: color-mix(in oklch, var(--sf-color-base) 8%, var(--sf-color-base)); | ||
| --sf-color-base-200: color-mix(in oklch, var(--sf-color-base) 20%, var(--sf-color-base)); | ||
| --sf-color-base-300: color-mix(in oklch, var(--sf-color-base) 40%, var(--sf-color-base)); | ||
| --sf-color-base-400: color-mix(in oklch, var(--sf-color-base) 65%, var(--sf-color-base)); |
There was a problem hiding this comment.
Base color tints collapse to a single value.
Lines 115–118 mix var(--sf-color-base) with itself, which algebraically reduces to 100% base for all tints (100, 200, 300, 400). This produces no useful tint scale. The PR objective explicitly states: "Special-case the base scale (--sf-color-base-*) so it ramps from base toward text rather than mixing base with itself."
To create a grayscale ramp from base to text, tints should also mix with var(--sf-color-text).
🎨 Proposed fix for base color tints
- --sf-color-base-100: color-mix(in oklch, var(--sf-color-base) 8%, var(--sf-color-base));
- --sf-color-base-200: color-mix(in oklch, var(--sf-color-base) 20%, var(--sf-color-base));
- --sf-color-base-300: color-mix(in oklch, var(--sf-color-base) 40%, var(--sf-color-base));
- --sf-color-base-400: color-mix(in oklch, var(--sf-color-base) 65%, var(--sf-color-base));
+ --sf-color-base-100: color-mix(in oklch, var(--sf-color-base) 8%, var(--sf-color-text));
+ --sf-color-base-200: color-mix(in oklch, var(--sf-color-base) 20%, var(--sf-color-text));
+ --sf-color-base-300: color-mix(in oklch, var(--sf-color-base) 40%, var(--sf-color-text));
+ --sf-color-base-400: color-mix(in oklch, var(--sf-color-base) 65%, var(--sf-color-text));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| --sf-color-base-100: color-mix(in oklch, var(--sf-color-base) 8%, var(--sf-color-base)); | |
| --sf-color-base-200: color-mix(in oklch, var(--sf-color-base) 20%, var(--sf-color-base)); | |
| --sf-color-base-300: color-mix(in oklch, var(--sf-color-base) 40%, var(--sf-color-base)); | |
| --sf-color-base-400: color-mix(in oklch, var(--sf-color-base) 65%, var(--sf-color-base)); | |
| --sf-color-base-100: color-mix(in oklch, var(--sf-color-base) 8%, var(--sf-color-text)); | |
| --sf-color-base-200: color-mix(in oklch, var(--sf-color-base) 20%, var(--sf-color-text)); | |
| --sf-color-base-300: color-mix(in oklch, var(--sf-color-base) 40%, var(--sf-color-text)); | |
| --sf-color-base-400: color-mix(in oklch, var(--sf-color-base) 65%, var(--sf-color-text)); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@optional/tokens.palette.css` around lines 115 - 118, The four base tint
tokens (--sf-color-base-100, --sf-color-base-200, --sf-color-base-300,
--sf-color-base-400) currently mix var(--sf-color-base) with itself which
collapses to a single value; change the second operand to var(--sf-color-text)
so the scale ramps from base toward text (i.e., use color-mix(in oklch,
var(--sf-color-text) <percent>, var(--sf-color-base)) and keep the intended
percentages (8%, 20%, 40%, 65%) so each token progressively shifts from the base
color toward the text color).
--sf-color-base-100 through -400 were mixing var(--sf-color-base) with itself, producing no scale (all values equal to base). Fix: mix var(--sf-color-text) into var(--sf-color-base) at increasing percentages (8%→65%), creating a grayscale that steps from near-base toward text — consistent with the shade scale (600–900) and theme-aware in both light and dark mode. Caught by CodeRabbit review on PR #25. https://claude.ai/code/session_015xq1jY8LgWAuyKQJmCM8L7
This pull request was created by @kiro-agent on behalf of @jackgranatowski 👻
Comment with /kiro fix to address specific feedback or /kiro all to address everything.
Learn about Kiro autonomous agent
Summary
Improves the color system developer experience and fixes issue #24.
Changes
Dark-mode fallback to light values (
core/tokens.css)light-dark(var(--X-light), var(--X-dark, var(--X-light)))-lighttokens, dark mode uses the same value instead of framework defaults-darktokens explicitlyTheme-aware palette (
optional/tokens.palette.css) — fixes fix(palette): tints/shades mix with theme-aware tokens instead of literal white/black #24white→var(--sf-color-base)black→var(--sf-color-text)Updated documentation (
core/tokens.css)DX Model
-lighttokens-light+ 6-darkTrade-off
When only
-lightis set, dark mode uses the same color value. On dark backgrounds, a light-mode-optimized color (e.g.oklch(0.45)) may have suboptimal contrast. This is intentional — it's visible and predictable, and signals to the dev that adding a-darkoverride would improve the dark mode experience.Summary by CodeRabbit
Improvements
Documentation / Demo