docs: add API audit checklist for token/class classification - #647
Conversation
Add a manual-review checklist grouping the full public API surface (747 tokens + 323 classes) into three tiers: foundation, useful-optional, and niche. Large uniform families are grouped; per-element decisions (classes, component variants, state hooks) are listed individually. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CAsUv13o2SAi7Tnsx93EtV
…xt--subtle The name '--secondary' collided conceptually with the brand palette role (--sf-color-secondary) and with its own --sf-color-text--on-secondary, making it read as 'text in the secondary brand colour' when it actually means de-emphasised, lower-emphasis text. Rename to --subtle, which pairs cleanly with the existing --muted tier and carries no brand-role ambiguity. BREAKING CHANGE: the PUBLIC token --sf-color-text--secondary is renamed to --sf-color-text--subtle. Its value, derivation and role are unchanged. Stable-id ledger retires id 314 (removed) and assigns id 801 to the new name. Covers source (core/tokens.css, core/themes.css, core/macros.css), the regenerated docs/registry/configurator artifacts, hand-written docs, the user manual, the demo + landing page, configurator preview sources, and the test baselines/snapshots. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CAsUv13o2SAi7Tnsx93EtV
…e-first guidance Full audit of .sf-is-* against internal consumers, configurator exposure, tier classification, and comparable frameworks (Open Props, Pico.css, Bulma, Automatic.css). Three classes had no design defense and a clear native replacement, so they're gone: - .sf-is-hidden duplicated [hidden], which core/reset.css already hardens to the same `display: none !important`. - .sf-is-readonly duplicated :read-only and incorrectly blocked text selection a real read-only field should allow. - .sf-is-busy was a single cursor:progress declaration with no distinct visual; [aria-busy="true"] alone covers the same ground. Everything else in the file stays: active/open/collapsed/expanded/pressed/ current are documented BEM consumer-API flags (configurator-exposed in MiscPanel.svelte, tiered in token-tiers.js, tested), and danger/pending are explicitly disambiguated from error/loading in docs/states.md's own "Disambiguating the overlaps" section — none of these are the accidental duplicates they first looked like. Added docs/states.md § "Prefer native state": a table of native pseudo- classes/attributes/ARIA states to reach for before adding a .sf-is-* class, plus the removed three as the worked precedent. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CAsUv13o2SAi7Tnsx93EtV
…te visibility helpers Second pass on .sf-is-* after holding the six previously-defended survivors (active, open, collapsed, expanded, pressed, current, danger, pending) to a stricter bar: "documented and intentional" is a different question from "needed at this stage of a pre-1.0, lean framework." None of them clear "most consumers need this today": - .sf-is-active/.sf-is-open/.sf-is-collapsed/.sf-is-pressed/.sf-is-current only ever set an inheritable --sf-is-* custom property for a hypothetical consumer's own calc() to branch on, with zero consumers anywhere in this codebase or in any of the four comparable frameworks surveyed (Open Props, Pico.css, Bulma, Automatic.css ship nothing like it). - .sf-is-expanded was pure vocabulary duplication of .sf-is-open (identical rule, no distinct behaviour). - .sf-is-danger had a live correctness bug (only ever worked on form fields via --sf-field-*, never the "destructive-action button" case its own docs described) and duplicated .sf-is-invalid/.sf-is-error's implementation. - .sf-is-pending was two lines cheap to hand-roll, with .sf-is-loading already covering the common "mask with a spinner" case. All eight removed, along with the now-orphaned --sf-is-active/-current/ -pressed/-open @Property registrations and the --sf-current-font-weight / --sf-state-pending-opacity tokens that lost their only consumer. .sf-is-invisible/.sf-is-visible were relocated, not cut: single-property helpers with no runtime condition of their own (unlike .sf-is-selected, which paints something), moved to optional/utilities.css as .sf-invisible/ .sf-visible — matching Bulma's precedent of treating visibility as a helper, not a state. demo/index.html's own mobile-nav drawer used .sf-is-open as a plain JS-toggled hook (its own #nav-overlay/#demo-nav rules, independent of states.css's flag mechanism) — renamed to a demo-local .is-open class, which doubles as a worked example of "write your own toggle class" guidance. user-manual/system-kolorow.md's stepper example updated the same way. Added docs/states.md § "Prefer native state" with a table of native pseudo-classes/attributes/ARIA states to reach for before adding a .sf-is-* class, and a documented history of both removal rounds. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CAsUv13o2SAi7Tnsx93EtV
|
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 (1)
📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughThe PR renames the secondary text token to subtle, removes deprecated state tokens and classes, adds visibility utilities, and updates core CSS, configurator, demos, generated catalogs, documentation, registries, and tests. ChangesAPI consolidation
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
Greptile SummaryThis PR audits and reduces the framework's public token and state-class API. The main changes are:
Confidence Score: 4/5Published CSS APIs can silently stop styling existing consumer interfaces after an upgrade.
core/tokens.css, core/states.css, and optional/utilities.css Important Files Changed
Reviews (1): Last reviewed commit: "refactor(states)!: remove speculative in..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| --sf-color-text--subtle: light-dark( | ||
| oklch(from var(--sf-color-neutral-source-light) clamp(0.15, calc(l - 0.25 - var(--sf-contrast-bias)), 0.45) c h), | ||
| oklch(from var(--sf-color-neutral) clamp(0.55, calc(l + 0.1 + var(--sf-contrast-bias)), 0.90) c h) | ||
| ); |
There was a problem hiding this comment.
Existing consumer rules using var(--sf-color-text--secondary) become invalid after this rename because the old public token is no longer defined. Those elements fall back to an inherited or initial color instead of the secondary text color.
| --sf-color-text--subtle: light-dark( | |
| oklch(from var(--sf-color-neutral-source-light) clamp(0.15, calc(l - 0.25 - var(--sf-contrast-bias)), 0.45) c h), | |
| oklch(from var(--sf-color-neutral) clamp(0.55, calc(l + 0.1 + var(--sf-contrast-bias)), 0.90) c h) | |
| ); | |
| --sf-color-text--subtle: light-dark( | |
| oklch(from var(--sf-color-neutral-source-light) clamp(0.15, calc(l - 0.25 - var(--sf-contrast-bias)), 0.45) c h), | |
| oklch(from var(--sf-color-neutral) clamp(0.55, calc(l + 0.1 + var(--sf-contrast-bias)), 0.90) c h) | |
| ); | |
| --sf-color-text--secondary: var(--sf-color-text--subtle); |
Context Used: CLAUDE.md (source)
| /* No visibility classes here: .sf-is-hidden duplicated [hidden] (removed — | ||
| core/reset.css hardens that attribute identically). .sf-invisible / | ||
| .sf-visible moved to optional/utilities.css as single-property helpers | ||
| (same call as Bulma's is-hidden/is-invisible living under "helpers", | ||
| not component/state docs) — still the hook docs/roadmap.md's | ||
| reveal-on-scroll pattern targets, just renamed and relocated. */ |
There was a problem hiding this comment.
The old visibility classes were available through core/states.css, but their renamed replacements now exist only in optional/utilities.css, which the supported slashed/optimal entry point does not include. After upgrading, optimal-bundle consumers get no hiding behavior from either their existing .sf-is-invisible class or the new .sf-invisible class unless they add another stylesheet.
Context Used: CLAUDE.md (source)
|
|
||
| /* -- ACTIVE / SELECTED / CURRENT -- */ | ||
|
|
||
| .sf-is-active { | ||
|
|
||
| --sf-is-active: 1; | ||
| } | ||
| /* -- SELECTED / HIGHLIGHTED -- | ||
| No .sf-is-active / .sf-is-open / .sf-is-collapsed / .sf-is-expanded / | ||
| .sf-is-current / .sf-is-pressed here: these only ever set an | ||
| inheritable --sf-is-* custom property for a hypothetical consumer | ||
| calc() to branch on, with zero consumer anywhere in this codebase or | ||
| its demo. Speculative "might be useful one day" surface, removed — | ||
| see docs/states.md § "Prefer native state". Reach for [aria-current], |
There was a problem hiding this comment.
Existing components that toggle .sf-is-active, .sf-is-current, .sf-is-open, .sf-is-pressed, .sf-is-collapsed, or .sf-is-expanded still change their DOM classes, but this removal no longer updates the corresponding inherited state property or visual state. Navigation, toggle, and disclosure UI built against these published classes can therefore stop reflecting its current state after an upgrade.
Context Used: CLAUDE.md (source)
| /* No .sf-is-busy / .sf-is-pending here: both were a couple of | ||
| property-value declarations (cursor:progress, plus opacity for | ||
| pending's "optimistic UI" dimming) with no consumer and no native gap | ||
| they filled — [aria-busy="true"] covers the semantic, and the visual | ||
| is two lines to hand-roll in your own component CSS if you want it. */ |
There was a problem hiding this comment.
Consumers that apply the published .sf-is-busy or .sf-is-pending classes during an in-flight request now get neither the progress cursor nor pending opacity. The request still runs, but the user loses the only framework-provided visual indication that the action is being processed.
Context Used: CLAUDE.md (source)
| } | ||
|
|
||
| .sf-is-pressed { | ||
|
|
||
| --sf-is-pressed: 1; | ||
| background-color: var(--sf-color-bg--selected); | ||
| } | ||
|
|
||
| /* -- OPEN / CLOSED -- */ | ||
|
|
||
| .sf-is-open { | ||
| --sf-is-open: 1; | ||
| } | ||
|
|
||
| .sf-is-collapsed { | ||
| --sf-is-open: 0; | ||
| } | ||
|
|
||
| .sf-is-expanded { | ||
| --sf-is-open: 1; | ||
| } | ||
|
|
||
| /* -- VALIDATION / FEEDBACK -- */ | ||
| /* -- VALIDATION / FEEDBACK -- | ||
| No .sf-is-danger here: identical implementation to .sf-is-invalid/ | ||
| .sf-is-error (both only ever wrote --sf-field-*, so it never worked as | ||
| the "destructive-action context" its own docs described outside a form | ||
| field) and, per docs/migration.md, visual variants belong in your own | ||
| component CSS anyway. Use .sf-is-invalid/.sf-is-error, or your own |
There was a problem hiding this comment.
Existing validation or destructive UI using .sf-is-danger no longer sets --sf-field-border-color and --sf-field-text-color. Those elements silently revert to their normal colors, so an error or destructive state can appear neutral after an upgrade.
Context Used: CLAUDE.md (source)
| /* -- INTERACTIVITY -- | ||
| No .sf-is-readonly here: it duplicated :read-only for real form controls, | ||
| and its pointer-events:none blocked text selection that a genuinely | ||
| read-only field should still allow — reach for :read-only, or | ||
| [aria-readonly] on a non-native widget, instead. */ | ||
|
|
||
| /* cursor: not-allowed inherits to children with pointer-events: auto — |
There was a problem hiding this comment.
An existing element using .sf-is-hidden is no longer removed from layout because this PR deletes the class without a compatibility rule. On upgrade, content previously hidden by the framework can become visible until each consumer migrates its markup to the native hidden attribute.
Context Used: CLAUDE.md (source)
…review-classify-3f1a2r # Conflicts: # docs/llm-guide.md
|
Re: the Greptile P1 findings on
No compatibility shims were omitted by oversight — for a pre-1.0 framework we're trading a clean break now against carrying speculative/duplicate surface area indefinitely. Happy to discuss further if there's a case for keeping any specific one, but no code changes planned from these six comments. Generated by Claude Code |
…(deps): bump js-yaml 4.2.0 → 4.3.0 Migration guide entries for this branch's two breaking changes: the --sf-color-text--secondary → --sf-color-text--subtle rename, and the .sf-is-* state-class reduction (11 removed, 2 relocated to optional/utilities.css, 6 now-orphaned tokens removed alongside). Satisfies the "Check migration docs for breaking changes" CI gate. js-yaml bump resolves GHSA-52cp-r559-cp3m (quadratic CPU via YAML merge-key chains), a transitive dependency of stylelint -> cosmiconfig already present on main — unrelated to this branch's changes, fixed via plain `npm audit fix` (no --force, same major, lockfile-only). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CAsUv13o2SAi7Tnsx93EtV
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
docs/macros.md (1)
382-382: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAlign the value assignment.
Adding a few spaces before
oklchwill align this value with the other token definitions in this block.✨ Proposed fix
- --sf-color-text--subtle: oklch(from var(--my-fg) l c h / 0.70); + --sf-color-text--subtle: oklch(from var(--my-fg) l c h / 0.70);🤖 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 `@docs/macros.md` at line 382, Align the value assignment for --sf-color-text--subtle with the other token definitions by adding the necessary spaces before the oklch expression.docs/llm-guide.md (1)
221-221: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAlign the trailing comment.
Adding a few spaces here will align this comment with the adjacent tokens for a cleaner look.
✨ Proposed fix
- --sf-color-text--subtle /* Between muted and primary */ + --sf-color-text--subtle /* Between muted and primary */🤖 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 `@docs/llm-guide.md` at line 221, Update the trailing comment on --sf-color-text--subtle by adding the necessary spacing before the comment so it aligns with the adjacent token comments. Preserve the variable and comment text unchanged.
🤖 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 `@docs/llm-guide.md`:
- Line 221: Update the trailing comment on --sf-color-text--subtle by adding the
necessary spacing before the comment so it aligns with the adjacent token
comments. Preserve the variable and comment text unchanged.
In `@docs/macros.md`:
- Line 382: Align the value assignment for --sf-color-text--subtle with the
other token definitions by adding the necessary spaces before the oklch
expression.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d2a3c854-d76a-49af-9843-695cc5aaaa2d
⛔ Files ignored due to path filters (4)
configurator/src/data/api-index.generated.jsonis excluded by!**/*.generated.*configurator/src/data/classes.generated.jsonis excluded by!**/*.generated.*configurator/src/data/token-registry.generated.jsonis excluded by!**/*.generated.*dist/css-custom-data.jsonis excluded by!**/dist/**
📒 Files selected for processing (42)
README.mdbadges/badge-optimal.jsonconfigurator/src/components/panels/EffectsPanel.svelteconfigurator/src/components/panels/MiscPanel.svelteconfigurator/src/components/panels/WcagPanel.svelteconfigurator/src/data/domain-patterns.jsonconfigurator/src/lib/preview/sections.tsconfigurator/src/lib/preview/skin.tscore/macros.csscore/states.csscore/themes.csscore/tokens.cssdemo/index.htmldocs/api-audit-checklist.mddocs/api-index.jsondocs/api-index.mddocs/architecture.mddocs/classes.mddocs/llm-guide.mddocs/macros.mddocs/ref-allowlist.jsondocs/registry.jsondocs/roadmap.mddocs/states.mddocs/theming.mddocs/token-annotations.jsondocs/token-index.jsondocs/token-index.mddocs/tokens.mdindex.htmloptional/utilities.cssscripts/check-macro-catalog.jsscripts/token-tiers.jstests/a11y-fixture.htmltests/baseline-modern.jsontests/behavior.spec.jstests/demo-visual.spec.jstests/states-full.spec.jstests/tier1-p8-modern.spec.jstests/token-api.snapshot.jsontoken-registry.jsonuser-manual/system-kolorow.md
💤 Files with no reviewable changes (5)
- configurator/src/data/domain-patterns.json
- scripts/check-macro-catalog.js
- scripts/token-tiers.js
- configurator/src/components/panels/EffectsPanel.svelte
- tests/demo-visual.spec.js
…umns Addresses two CodeRabbit nitpicks: the rename from --secondary to --subtle left the value/comment column misaligned in docs/macros.md's customize-example and docs/llm-guide.md's token list. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CAsUv13o2SAi7Tnsx93EtV
|
@coderabbitai review |
✅ Action performedReview finished.
|
2 participants
https://claude.ai/code/session_01CAsUv13o2SAi7Tnsx93EtV
Summary by CodeRabbit
New Features
.sf-invisibleand.sf-visibleutilities for visibility control without changing layout.--sf-color-text--subtletext token.Changes
--sf-color-text--subtleacross themes, previews, and examples.Documentation