feat: fix token bugs, modernize tokens, expand palette, add regression suite - #29
Merged
Merged
Conversation
…n suite Correctness (verified against audits/): - cap every shadow-layer alpha at 0.7 so dark mode no longer renders opaque slabs - repair --sf-shadow-color (was malformed/dead) into a neutral-derived tint and wire it into every shadow; colored shadows are now an opt-in override - link hover/active darken in light, lighten in dark (was reducing contrast) - clamp --sf-color-text--inverse to stay in-gamut on custom neutral overrides Modernization (benchmarked vs Tailwind v4 / Open Props): - full font-weight 100-900 scale; font-feature/variation/optical-sizing tokens - humanist/geometric/slab font stacks; bounce/overshoot easings - text-shadow/drop-shadow, breakpoint, perspective, scroll-timeline tokens - named animation presets (keyframes + .sf-* classes) in motion.css - forced-colors handling in accessibility.css Palette: implement the alias layer in optional/tokens.palette.css — full 50-950 scale, a5-a95 alpha, shade + functional aliases per brand color (drops off-grid a75). Tooling & docs: Playwright regression suite (light+dark + token coverage) wired into CI; slashed.full.css bundle; multi-bundle pre-commit hook; English comments/docs; documented intentional tradeoffs. https://claude.ai/code/session_01MoV2rAESh77QMzsc4kL3fo
|
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 ignored due to path filters (2)
📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds multi-bundle build/support and pre-commit staging, expands core and optional design tokens (colors, shadows, typography, motion), adds Playwright token-regression tests and CI job, and updates README/docs/demo to reflect new bundles and tokens. ChangesBuild System and Bundle Configuration
Token System Expansion and Testing
Documentation and Demo
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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: 4
🧹 Nitpick comments (1)
scripts/bundle.js (1)
60-62: ⚡ Quick winUse relative paths for watch matching instead of basenames.
Line 60-Line 62 strips directory context; if
core/andoptional/ever share a filename, watch filtering becomes ambiguous. Track normalized relative paths (e.g.core/tokens.css) and compare against${dir}/${filename}.Diff proposal
- const collectBasenames = () => - getBundles().flatMap((b) => b.files).map((f) => path.basename(f)); + const collectWatchedPaths = () => + new Set(getBundles().flatMap((b) => b.files).map((f) => path.normalize(f))); - let watchedFiles = []; + let watchedFiles = new Set(); try { - watchedFiles = collectBasenames(); + watchedFiles = collectWatchedPaths(); } catch (err) { console.error(`[watch] Failed to load config: ${err.message}`); } @@ ['core', 'optional'].forEach((dir) => { fs.watch(path.join(ROOT, dir), (event, filename) => { if (!filename) return; - if (watchedFiles.includes(filename)) { + const changedPath = path.normalize(path.join(dir, String(filename))); + if (watchedFiles.has(changedPath)) { console.log(`[watch] ${event}: ${dir}/${filename}`); rebuild(); } @@ console.log('[watch] bundle.config.json changed'); try { - watchedFiles = collectBasenames(); + watchedFiles = collectWatchedPaths(); } catch (err) { console.error(`[watch] Invalid config: ${err.message}`); }Also applies to: 84-91
🤖 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 `@scripts/bundle.js` around lines 60 - 62, The current collectBasenames function loses directory context by returning only path.basename(f), causing ambiguous watch matching; change it to return normalized relative paths instead (e.g., using path.posix.join or path.relative) so entries reflect "dir/filename" like "core/tokens.css", and update any other similar utilities referenced in the diff (e.g., the logic around lines 84-91 that filters watches) to compare against `${dir}/${filename}` rather than basenames; locate collectBasenames and the watch filtering code that uses its output and replace basename usage with path.relative(pathRoot, f) (or equivalent normalization) so watch matching is unambiguous.
🤖 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 @.github/workflows/ci.yml:
- Around line 41-42: Replace the floating action versions with pinned commit
SHAs and disable checkout credential persistence: update the uses entries for
actions/checkout@v4 and actions/setup-node@v4 to specific commit SHAs (e.g.,
actions/checkout@<commit-sha> and actions/setup-node@<commit-sha>) and add the
checkout step input persist-credentials: false to the actions/checkout step so
the workflow does not expose tokens to checked-out code.
In `@docs/color-aliases-design-decisions.md`:
- Around line 113-117: The fenced code block containing the CSS variable
examples (--sf-color-{status}-subtle, --sf-color-{status}-muted,
--sf-color-{status}-strong) needs a language identifier for proper rendering and
markdown linting; update the opening fence from ``` to ```text (or another
appropriate language like ```css) so the block becomes a labeled fenced code
block (e.g., ```text) while keeping the content unchanged.
In `@README.md`:
- Line 71: The README table is missing tokens.layout in the
slashed.essential.css bundle entry; update the table row for
`slashed.essential.css` to include `tokens.layout` (or `tokens.layout.css`)
alongside `layers`, `tokens`, `reset`, `base`, `layout`, `states`, `motion`,
`accessibility`, and `print` so the essential bundle list matches the
quick-start and architecture docs; ensure the token name exactly matches other
references (`tokens.layout`/`tokens.layout.css`) for consistency.
In `@tests/tokens.spec.js`:
- Line 11: The FIXTURE constant is built by string concatenation with path.join
which breaks on Windows and with special characters; replace the construction of
FIXTURE to use pathToFileURL from the url module and take its href (i.e., import
or require pathToFileURL and call pathToFileURL(path.join(__dirname,
'fixture.html')).href) so the file URL is formed correctly; update any tests
referencing FIXTURE accordingly.
---
Nitpick comments:
In `@scripts/bundle.js`:
- Around line 60-62: The current collectBasenames function loses directory
context by returning only path.basename(f), causing ambiguous watch matching;
change it to return normalized relative paths instead (e.g., using
path.posix.join or path.relative) so entries reflect "dir/filename" like
"core/tokens.css", and update any other similar utilities referenced in the diff
(e.g., the logic around lines 84-91 that filters watches) to compare against
`${dir}/${filename}` rather than basenames; locate collectBasenames and the
watch filtering code that uses its output and replace basename usage with
path.relative(pathRoot, f) (or equivalent normalization) so watch matching is
unambiguous.
🪄 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: 046a4f7c-825f-4d3e-8010-782cdf6bd6d2
⛔ Files ignored due to path filters (3)
dist/slashed.essential.cssis excluded by!**/dist/**dist/slashed.full.cssis excluded by!**/dist/**package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (19)
.githooks/pre-commit.github/workflows/ci.yml.gitignoreREADME.mdbundle.config.jsoncore/accessibility.csscore/layout.csscore/motion.csscore/tokens.csscore/tokens.layout.cssdocs/architecture.mddocs/color-aliases-design-decisions.mddocs/demo.htmloptional/tokens.palette.csspackage.jsonplaywright.config.jsscripts/bundle.jstests/fixture.htmltests/tokens.spec.js
Comment on lines
+41
to
+42
| - uses: actions/checkout@v4 | ||
| - uses: actions/setup-node@v4 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify workflow action pinning and checkout credential persistence settings.
rg -n '^\s*-\s*uses:\s*actions/' .github/workflows/ci.yml
rg -n 'persist-credentials' .github/workflows/ci.ymlRepository: codeslash-dev/SLASHED
Length of output: 337
Harden action usage in the new test job.
Lines 41–42 use floating version tags and lack credential persistence restrictions. Pin actions to commit SHAs and disable checkout credential persistence to reduce supply-chain and token exposure risk.
🔐 Suggested hardening
- - uses: actions/checkout@v4
+ - uses: actions/checkout@<pinned-commit-sha>
+ with:
+ persist-credentials: false
- - uses: actions/setup-node@v4
+ - uses: actions/setup-node@<pinned-commit-sha>🧰 Tools
🪛 zizmor (1.25.2)
[warning] 41-41: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 41-41: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 42-42: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 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 @.github/workflows/ci.yml around lines 41 - 42, Replace the floating action
versions with pinned commit SHAs and disable checkout credential persistence:
update the uses entries for actions/checkout@v4 and actions/setup-node@v4 to
specific commit SHAs (e.g., actions/checkout@<commit-sha> and
actions/setup-node@<commit-sha>) and add the checkout step input
persist-credentials: false to the actions/checkout step so the workflow does not
expose tokens to checked-out code.
Custom properties can't be used in @media/@container conditions — their only natural use site — so the tokens couldn't deliver on their purpose. SLASHED is container-query-first (fluid tokens + @container primitives), so they added surface area without value. Removed from core/tokens.css and rebundled. https://claude.ai/code/session_01MoV2rAESh77QMzsc4kL3fo
- shadow tint now inherits the neutral's own chroma (drop the *2 amplifier), so a colourless neutral yields colourless shadows and the tint never reads more saturated than the brand neutral - note that animation presets are decorative and inert under reduced motion, so they must not be the sole show/hide mechanism - reword the architecture breakpoint note now that no breakpoint tokens ship https://claude.ai/code/session_01MoV2rAESh77QMzsc4kL3fo
There was a problem hiding this comment.
🧹 Nitpick comments (1)
core/tokens.css (1)
489-494: ⚡ Quick winUse a darkened brand example for the override.
The
var(--sf-color-primary)example skips the fixedL=0.15darkening, so consumers copying it can end up with bright colored slabs again. Show the opt-in override as a dark-tinted derivation instead.Suggested doc tweak
- tinted shadows: var(--sf-color-primary). */ + tinted shadows: oklch(from var(--sf-color-primary) 0.15 c h). */🤖 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 `@core/tokens.css` around lines 489 - 494, The example override for --sf-shadow-color uses var(--sf-color-primary) which skips the fixed darkening; update the documentation/comment to show the opt-in override as a dark-tinted derivation (i.e. derive from --sf-color-primary but force L=0.15 like the neutral example) so consumers copy an explicitly darkened brand; reference the symbols --sf-shadow-color, --sf-color-neutral, --sf-color-primary and the L=0.15 darkening when describing the correct override form.
🤖 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.
Nitpick comments:
In `@core/tokens.css`:
- Around line 489-494: The example override for --sf-shadow-color uses
var(--sf-color-primary) which skips the fixed darkening; update the
documentation/comment to show the opt-in override as a dark-tinted derivation
(i.e. derive from --sf-color-primary but force L=0.15 like the neutral example)
so consumers copy an explicitly darkened brand; reference the symbols
--sf-shadow-color, --sf-color-neutral, --sf-color-primary and the L=0.15
darkening when describing the correct override form.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: aa52a244-e3b1-461a-8880-b1376df913b0
⛔ Files ignored due to path filters (2)
dist/slashed.essential.cssis excluded by!**/dist/**dist/slashed.full.cssis excluded by!**/dist/**
📒 Files selected for processing (3)
core/motion.csscore/tokens.cssdocs/architecture.md
✅ Files skipped from review due to trivial changes (1)
- docs/architecture.md
3 tasks
- README: list tokens.layout in the essential-bundle contents (it ships in it) - tests: build the fixture URL with pathToFileURL().href (cross-platform) - docs: label bare code fences as text (markdownlint MD040) - tokens: shadow-color override example now derives a dark tint (oklch(from … 0.15 c h)) so consumers don't recreate bright slabs - bundle watch: match normalized relative paths, not basenames CI action SHA-pinning / persist-credentials deferred to #31 (repo-wide). https://claude.ai/code/session_01MoV2rAESh77QMzsc4kL3fo
2 tasks
This was referenced May 29, 2026
Merged
This was referenced Jul 8, 2026
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
2 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.
Correctness (verified against audits/):
wire it into every shadow; colored shadows are now an opt-in override
Modernization (benchmarked vs Tailwind v4 / Open Props):
Palette: implement the alias layer in optional/tokens.palette.css — full 50-950
scale, a5-a95 alpha, shade + functional aliases per brand color (drops off-grid a75).
Tooling & docs: Playwright regression suite (light+dark + token coverage) wired into
CI; slashed.full.css bundle; multi-bundle pre-commit hook; English comments/docs;
documented intentional tradeoffs.
https://claude.ai/code/session_01MoV2rAESh77QMzsc4kL3fo
Summary by CodeRabbit