feat: add Tier-1 sRGB color fallbacks for pre-Chrome-123 / pre-Safari-17.5 engines - #254
Merged
Merged
Conversation
…-17.5 engines Implements the "gate at declaration site" pattern to fix IACVT failures on engines lacking light-dark(), oklch(from…), @Property, or color-mix() support. Architecture: - core/tokens.color-fallbacks.css: generated sRGB defaults in @layer slashed.tokens (light) and @layer slashed.themes (dark), loaded before core/tokens.css in all 10 bundles - core/tokens.css: all light-dark() / oklch(from…) / color-mix() declarations wrapped in @supports blocks, making sRGB defaults the Old_Engine fallbacks - scripts/gen-color-fallbacks.js: build-time generator using culori for perceptually accurate OKLCH→sRGB conversion with full formula replay - scripts/verify-color-tokens.js: validates source token inventory Test suite (P1–P11 + fast-check, 195 unit tests + 376 Playwright): - P1: sRGB validity; P2: coverage/uniqueness; P3: source ordering - P4: @Property mirror drift; P5: gamut mapping; P6: idempotence - P7: Old_Engine cascade simulation; P8: Modern_Engine baseline - P9: perceptual ΔE_OK ≤ 0.12; P10: WCAG 2.1 AA contrast - P11: override propagation (old vs modern engine) https://claude.ai/code/session_01QqME6SfLKZZkJD2W8Tgb3P
…ions Eliminates the build-time generator in favour of CSS hsl() + calc() expressions that recompute automatically when users override brand channel variables, with no build step required on any browser from Chrome 49+. Changes: - core/tokens.color-fallbacks.css: 343-line generated file → 80-line static file using --sf-*-h/s/l channel variables + hsl(var() calc()) derivations - Removes scripts/gen-color-fallbacks.js and scripts/verify-color-tokens.js - Removes gen:fallbacks script from build; build is now just node scripts/bundle.js - Removes CI drift-check step (no generated file to drift) - Dark mode fallbacks removed — handled exclusively by modern CSS (@supports gates) - Drops 5 tests tied to the generator (P4/P5/P6/P9/idempotence/verify) - Updates P1/P3/P7/P10/P11 for the new HSL-based architecture To apply a brand colour on any browser: :root { --sf-primary-h: 340; --sf-primary-s: 80%; --sf-primary-l: 50%; } https://claude.ai/code/session_01QqME6SfLKZZkJD2W8Tgb3P
|
Warning Review limit reached
More reviews will be available in 12 minutes and 1 second. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR implements a comprehensive CSS color fallback system. A new ChangesColor fallback and feature gating
Sequence Diagram(s)sequenceDiagram
participant Browser as Older Browser
participant Fallback as core/tokens.color-fallbacks.css
participant Modern as core/tokens.css `@supports` gates
participant Computed as Computed style
Browser->>Fallback: Load bundle (no `@supports` support)
Fallback->>Browser: Apply HSL fallback tokens
Browser->>Modern: Encounter `@supports` gates
Modern->>Modern: Conditions fail
Computed->>Browser: Use fallback values
participant ModernBrowser as Modern Browser
participant CSS as dist/slashed.full.css
ModernBrowser->>Fallback: Load bundle
Fallback->>ModernBrowser: Apply HSL tokens (temporary)
ModernBrowser->>Modern: Evaluate `@supports` gates
Modern->>Modern: light-dark() supported
Modern->>ModernBrowser: Apply light-dark() values
Computed->>ModernBrowser: Use modern computed values
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
- Normalize calc() operator spacing in tokens.color-fallbacks.css to satisfy stylelint function-calc-no-unspaced-operator - Remove unused depth variable in tier1-p2-coverage.test.js - Escape all regex metacharacters (not just hyphens) when building token-name patterns in tier1-p3-ordering.test.js, per CodeQL incomplete-string-escaping finding https://claude.ai/code/session_01QqME6SfLKZZkJD2W8Tgb3P
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@core/tokens.color-fallbacks.css`:
- Line 53: The CSS token --sf-color-raised is hardcoded to 100% lightness which
prevents overrides of --sf-base-l from propagating; update the declaration so
its lightness uses the base lightness variable (e.g., replace the literal 100%
with var(--sf-base-l) or a dedicated fallback like var(--sf-raised-l,
var(--sf-base-l))) so changes to --sf-base-l recompute --sf-color-raised and
preserve theme overrides.
In `@tests/tier1-p1-srgb-validity.test.js`:
- Around line 23-33: The validator currently allows oklch(...) via
STATIC_OKLCH_RE inside isValidSRGB, which contradicts the Tier‑1 fallback
contract; remove the STATIC_OKLCH_RE branch from isValidSRGB (and any related
uses of STATIC_OKLCH_RE) so that oklch(...) values are no longer treated as
valid sRGB fallbacks, and update the surrounding comment to reflect that oklch
is disallowed for fallbacks.
In `@tests/tier1-p11-overrides.test.js`:
- Around line 13-22: The test uses a JS mock function resolveTokens to simulate
HSL derivation, so it never verifies the real fallback logic in
core/tokens.color-fallbacks.css; replace or augment the test to parse and
evaluate the actual CSS fallback contract instead of (or in addition to) the
synthetic resolveTokens output by reading core/tokens.color-fallbacks.css,
extracting the H/S/L formulas, computing the derived values from those formulas,
and asserting against the expected tokens to ensure regressions in the real CSS
are caught (use resolveTokens only as a comparator or remove it once CSS parsing
is in place).
In `@tests/tier1-p3-ordering.test.js`:
- Around line 69-70: The ordering check is passing the wrong pattern to
findDeclarations by appending ':' (using token + ':'), which doubles the colon
because findDeclarations already appends '\s*:'; update the two calls that use
token + ':' (both occurrences around the ordering check) to pass token alone so
findDeclarations builds the correct regex, ensuring the declaration-count
invariant is validated correctly (search for the calls to findDeclarations and
replace token + ':' with token).
In `@tests/tier1-p8-modern.spec.js`:
- Around line 85-86: Remove the silent skip when a baseline entry is missing:
replace the "if (!expected) continue" behavior with an explicit assertion that
fails when expected is undefined (e.g., assert/expect that expected is defined
with a message including token), then still compare actual to expected as
before; apply this change for both occurrences that check expected (the lines
using variables token, expected, actual) so missing baseline tokens cause test
failures rather than being skipped.
🪄 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: 7bd2b48a-50c5-464d-972b-1bb9c77f96cf
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (15)
bundle.config.jsoncore/tokens.color-fallbacks.csscore/tokens.cssdocs/tokens.mdoptional/legacy.csspackage.jsontests/baseline-modern.jsontests/bundle-size.spec.jstests/tier1-p1-srgb-validity.test.jstests/tier1-p10-contrast.test.jstests/tier1-p11-overrides.test.jstests/tier1-p2-coverage.test.jstests/tier1-p3-ordering.test.jstests/tier1-p7-oldengine.test.jstests/tier1-p8-modern.spec.js
- core/tokens.color-fallbacks.css: stop hardcoding --sf-color-raised to 100% lightness; derive it as base-l + 4% (matching the modern oklch(from … calc(l + 0.04) …) formula) so overriding --sf-base-l propagates to it like every other surface token - tier1-p3-ordering.test.js: findDeclarations() already appends `\s*:` to the pattern, so passing `token + ':'` built a regex requiring a double colon that never matched real declarations — the ordering invariant was silently never checked. Pass the bare token name instead - tier1-p1-srgb-validity.test.js: drop the oklch() allowance from the sRGB validator — the fallback contract is HSL/hex/rgb only, so an oklch() value here would mean a modern expression leaked unguarded - tier1-p11-overrides.test.js: replace the synthetic resolveTokens() mock (which simulated tokens like --sf-color-primary-hover/-subtle/-strong that don't exist in the fallback file, using made-up formulas) with a parser that extracts the real hsl(var(...) calc(...)) derivations from core/tokens.color-fallbacks.css and validates override propagation against the shipped formulas — this version also caught the --sf-color-raised regression above - tier1-p8-modern.spec.js: replace the silent `if (!expected) continue` with an explicit assertion, so a token added to TIER1_TOKENS without regenerating the baseline fails loudly instead of going unchecked; regenerated baseline-modern.json (byte-identical — confirms the fallback-formula fix has no effect on modern-engine resolution) https://claude.ai/code/session_01QqME6SfLKZZkJD2W8Tgb3P
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
3 participants
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.
Implements the "gate at declaration site" pattern to fix IACVT failures on
engines lacking light-dark(), oklch(from…), @Property, or color-mix() support.
Architecture:
slashed.tokens (light) and @layer slashed.themes (dark), loaded before
core/tokens.css in all 10 bundles
wrapped in @supports blocks, making sRGB defaults the Old_Engine fallbacks
perceptually accurate OKLCH→sRGB conversion with full formula replay
Test suite (P1–P11 + fast-check, 195 unit tests + 376 Playwright):
https://claude.ai/code/session_01QqME6SfLKZZkJD2W8Tgb3P
Summary by CodeRabbit
Release Notes
New Features
Improvements
Documentation