feat(core): expand Relative Color Syntax across components#37
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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. 📝 WalkthroughWalkthroughRefactors component CSS to derive interactive-state colors using OKLCH where supported, consolidates per-variant button hover/active logic, and adds feature-guarded tint/transition enhancements across cards, inputs, modals, and tooltips. Also includes small docs/site formatting and syntactic JS cleanups. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/core/src/components/button.css`:
- Around line 64-69: The `@supports` block currently only checks for RCS via
"oklch(from red l c h)" but uses light-dark(), so in browsers that support RCS
but not light-dark() the declaration will fail; update the feature test to
require both features (e.g. add an and condition that checks light-dark support
like (color: light-dark(red, blue))) or remove/replace the light-dark() usage
with a compatible single-mode derivation; modify the `@supports` line and
corresponding rule that references light-dark() accordingly so the condition and
declaration use matching, supported features.
In `@packages/core/src/components/card.css`:
- Around line 133-137: The hover darken rule uses oklch(from var(--_card-border)
...) which can fail when --_card-border is transparent for the
[s-card="elevated"] and [s-card][s-surface="floating"] variants; update the
styles so the interactive rule never feeds "transparent" into oklch: either set
a non‑transparent fallback value for --_card-border on those selectors (adjust
the declarations on [s-card="elevated"] and [s-card][s-surface="floating"] to
assign a low‑alpha neutral oklch/rgb fallback) or change the hover rule to use a
fallback in the var() (e.g. var(--_card-border, <non‑transparent‑oklch>)) so the
`@supports` block and the border-color: oklch(from var(--_card-border) calc(l -
0.1) c h) expression always receive a valid non‑transparent color.
♻️ Duplicate comments (1)
packages/core/src/components/button.css (1)
79-84: Samelight-dark()support concern applies here.Apply the same combined feature test for the active state to maintain consistency.
🧹 Nitpick comments (1)
packages/core/src/components/tooltip.css (1)
66-69: Edge case: negative lightness on dark backgrounds.
calc(l - 0.1)can produce negative values when the tooltip background is already very dark (lightness < 0.1). While browsers typically clamp this, using explicit clamping makes intent clearer and ensures predictable behavior:♻️ Suggested fix
`@supports` (color: oklch(from red l c h)) { - border: 1px solid oklch(from var(--_tooltip-bg) calc(l - 0.1) c h); + border: 1px solid oklch(from var(--_tooltip-bg) max(calc(l - 0.1), 0.05) c h); }
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/core/src/components/tooltip.css`:
- Around line 76-78: In packages/core/src/components/tooltip.css the rule
setting min-width: 100% can exceed --_tooltip-max-width for wide triggers;
change the min-width to a clamped value that picks the smaller of 100% and
var(--_tooltip-max-width) (use the CSS min() or clamp() approach) so the tooltip
never forces a width larger than the configured max, and if the original
behavior to allow exceeding max-width was intentional, add a short comment in
the component header documenting that decision and rationale.
🧹 Nitpick comments (2)
apps/docs/src/pages/demo.astro (1)
1005-1021: Consider consistent null handling in slider event handlers.The code uses optional chaining (
?.) when adding event listeners to sliders, but then uses non-null assertions (!) when updating the value display elements. If you're being defensive about the slider existing, the same caution could apply to the value spans:document.getElementById('primary-slider')?.addEventListener('input', (e) => { const value = (e.target as HTMLInputElement).value; document.documentElement.style.setProperty('--shift-hue-primary', value); - document.getElementById('primary-value')!.textContent = value; + const valueEl = document.getElementById('primary-value'); + if (valueEl) valueEl.textContent = value; });Since this is a self-contained demo page where all elements are defined in the same file, the current approach is unlikely to cause issues in practice.
packages/core/src/components/button.css (1)
221-225: Clarify the comment—this hides text, not "passes" contrast checks.The implementation is correct: setting text color to match the background visually hides the label while the spinner displays. However, the comment is misleading—this doesn't improve accessibility contrast; it exploits a 1:1 ratio to avoid automated tool warnings while hiding text. Consider rewording for accuracy.
✏️ Suggested comment clarification
-/* Loading state - use bg color for text to pass a11y contrast checks */ +/* Loading state - hide text by matching bg color (avoids a11y tool false positives vs transparent) */
1 participant
Summary
Expand Relative Color Syntax (RCS) usage across core components to derive hover states, focus rings, borders, and backdrops from base colors. This enables custom colors to "just work" with proper state variations.
Components updated:
l*0.92) and active (l*0.85) states from--_btn-bgRelated Issues
Part of 2026 CSS modernization effort (OKLCH, RCS, light-dark())
Checklist
bun run lintpassesbun run buildsucceedsType of Change
Additional Notes
@supportsfor progressive enhancement