Skip to content

docs: CSS source cleanup — comment-only fixes (SL-001..005) - #475

Merged
jackgranatowski merged 3 commits into
claude/pr-469-audit-rebase-ggp0e4from
claude/audit-pr1-css-cleanup
Jul 2, 2026
Merged

jackgranatowski merged 3 commits into
claude/pr-469-audit-rebase-ggp0e4from
claude/audit-pr1-css-cleanup

Conversation

@jackgranatowski

Copy link
Copy Markdown
Contributor

PR1 from the technical-debt audit remediation plan (#469): CSS source cleanup, core/ only. Targets the integration branch claude/pr-469-audit-rebase-ggp0e4, not main — this and subsequent themed PRs will accumulate there before one final PR merges the whole batch to main.

Comment-only — no selector/property/value changes.

Findings addressed

  • SL-001 — cross-link the duplicated dark/light OKLCH clamp() derivation formula between core/tokens.css (inside light-dark()) and core/themes.css (flat re-declaration needed for [data-theme] overrides on non-:root elements). Left unmerged per the audit's own open question about whether the duplication is intentional — deferring that to you rather than guessing.
  • SL-002 — doc-comment above .sf-clickable-parent warning against large (100+ node) grids, since its selectors combine multiple :has() branches.
  • SL-003 — strengthened --sf-is-dark's "internal, don't set directly" comment so it's unmistakable against the immediately-following public --sf-is-active/-current/-pressed/-open block (same naming pattern, opposite contract). Grepped every var(--sf-is-dark) read site first — nothing external sets it.
  • SL-004 — added short section labels to core/layout.css's 11 previously-unlabeled blank-line section dividers.
  • SL-005 — consolidated the "var() not allowed inside @container" note (previously stated at only 1 of 4 @container sites) into one comment near the top of core/layout.css.

A side effect worth calling out

Adding SL-004's /* Section */ label directly above .sf-section shadowed gen-api-index.js's fallback-to-previous-description logic for the uncommented .sf-section--guttered class sitting in the same section — its generated description would have regenerated as the literal string "Section" instead of its existing curated text. Fixed by promoting that description into docs/token-annotations.json's classes overlay (the durable, intended mechanism for exactly this) rather than dropping the section label. Worth knowing about if more section labels get added elsewhere later — same shadowing risk applies to any uncommented class sharing a section with a newly-labeled one.

Verification

  • npm run lint:css, npm run build — clean
  • npm run test:unit — 31/31 pass
  • npm run check:llm-guide, npm run audit:check — pass
  • docs/api-index.json, docs/api-index.md, and the configurator's generated data files are byte-identical to base after the token-annotations.json fix

Generated by Claude Code

Comment-only changes across core/*.css, no selector/property/value diffs
(verified: npm run build's badge-optimal.json is byte-identical).

- SL-001: cross-link the duplicated dark/light OKLCH clamp() derivation
  formula between core/tokens.css:364-373 (inside light-dark()) and
  core/themes.css's SECTION-LEVEL THEMING block (flat re-declaration for
  [data-theme] overrides on non-:root elements). Left unmerged per the
  audit's open question - themes.css genuinely needs flat values outside
  light-dark(), so this may be intentional; deferred to the maintainer.
- SL-002: doc-comment above .sf-clickable-parent warning against applying
  it to large (100+ node) grids, since its selectors combine multiple
  :has() branches.
- SL-003: strengthen --sf-is-dark's "internal, don't set directly"
  comment to be unmistakable against the immediately-following
  --sf-is-active/-current/-pressed/-open block's "public hooks" comment -
  same --sf-is-* naming pattern, opposite contract. Grepped every
  var(--sf-is-dark) read site first to confirm nothing external sets it.
- SL-004: add short section labels to core/layout.css's 11 previously
  unlabeled blank-line section dividers, matching sibling files' style.
- SL-005: consolidate the "var() not allowed inside @container" note
  (previously stated at only 1 of 4 @container sites) into one comment
  near the top of core/layout.css.

Adding SL-004's /* Section */ label directly above .sf-section shadowed
gen-api-index.js's fallback-to-previous-description logic for the
uncommented .sf-section--guttered sitting in the same section (its
description regenerated as the literal string "Section" instead of its
existing curated text). Fixed by promoting that description into
docs/token-annotations.json's classes overlay - the durable, intended
mechanism for exactly this - rather than removing the section label.

Verified: lint:css, build, test:unit, check:llm-guide, audit:check all
pass; docs/api-index.json, docs/api-index.md, and the configurator's
generated data files are byte-identical to base after the fix.
@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: f24800fa-e6ab-4114-9cba-74c3d19cb287

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/audit-pr1-css-cleanup

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Docs: Clarify core CSS comments and preserve generated API annotations (SL-001..005)

📝 Documentation 🕐 10-20 Minutes

Grey Divider

AI Description

• Cross-link the duplicated OKLCH dark derivation formula across tokens.css and themes.css.
• Document :has()-heavy selector performance caveats for .sf-clickable-parent usage.
• Add section labels in layout.css and centralize @container var() limitation notes; keep API docs
 stable.
Diagram

graph TD
  layout["core/layout.css"] --> generator(["gen-api-index.js"]) --> apiindex["docs/api-index.*"]
  tokens["core/tokens.css"] --> generator
  themes["core/themes.css"] --> generator
  a11y["core/accessibility.css"] --> generator
  annotations["docs/token-annotations.json"] --> generator
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Eliminate the SL-001 formula duplication via shared derived tokens
  • ➕ Single source of truth for the OKLCH clamp() derivation
  • ➕ Reduces long-term risk of the two copies drifting
  • ➖ Non-trivial because themes.css needs flat values outside light-dark() for section-level theming
  • ➖ May require rethinking where/when derived dark source tokens are computed and overridden
2. Build-step generation for derived dark tokens
  • ➕ Keeps runtime CSS simpler and avoids duplicating derivation logic in multiple files
  • ➕ Can enforce synchronization automatically
  • ➖ Adds tooling complexity and a new build artifact surface area
  • ➖ Moves away from the current runtime/override-friendly token philosophy

Recommendation: Keep this PR’s approach (comment-only clarification + explicit cross-links + doc overlay fix). It achieves the audit goals without behavior change and avoids prematurely deciding the intentionality of SL-001 duplication. If the team later decides the duplication is accidental, consider a shared-token or build-step approach as a separate, scoped change.

Files changed (5) +43 / -10

Documentation (5) +43 / -10
accessibility.cssAdd performance guidance for :has()-heavy clickable-parent selectors +5/-0

Add performance guidance for :has()-heavy clickable-parent selectors

• Adds a doc comment warning that the two .sf-clickable-parent selector groups each include multiple :has() branches. Guides consumers to avoid applying it to very large grids/lists without profiling.

core/accessibility.css

layout.cssLabel layout sections and centralize @container var() limitation note +16/-3

Label layout sections and centralize @container var() limitation note

• Adds short section header comments to previously unlabeled blocks for easier scanning. Moves the repeated note about var() not being allowed in @container conditions into a single top-of-file explanation.

core/layout.css

themes.cssDocument why OKLCH clamp() derivation is duplicated outside light-dark() +5/-1

Document why OKLCH clamp() derivation is duplicated outside light-dark()

• Extends the SECTION-LEVEL THEMING comment to cross-reference the matching formula in core/tokens.css. Clarifies that themes.css requires flat re-declarations for [data-theme] overrides on non-:root elements.

core/themes.css

tokens.cssClarify --sf-is-dark is internal and link dark derivation duplication +16/-6

Clarify --sf-is-dark is internal and link dark derivation duplication

• Strengthens the comment contract around --sf-is-dark to clearly distinguish it from public --sf-is-* style-query hooks. Adds a cross-reference noting the OKLCH clamp() derivation is duplicated in themes.css for section-level theming.

core/tokens.css

token-annotations.jsonPin sf-section--guttered description via classes overlay +1/-0

Pin sf-section--guttered description via classes overlay

• Adds an explicit classes overlay entry for sf-section--guttered. Prevents generated API docs from accidentally picking up a newly introduced section label as its description fallback.

docs/token-annotations.json

@qodo-code-review

qodo-code-review Bot commented Jul 2, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 6 rules

Grey Divider


Informational

1. Stale tokens.css line cite ✓ Resolved 🐞 Bug ⚙ Maintainability
Description
The new SL-001 comment in core/themes.css references the clamp() formula at
“core/tokens.css:364-373”, but in the current branch that formula lives later in the file, so the
citation is already incorrect and will drift further over time.
Code

core/themes.css[R62-64]

+     SL-001: the clamp() derivation formula below is the same one used inside
+     tokens.css's light-dark() block (core/tokens.css:364-373) — duplicated
+     here, not shared, because this file needs flat values outside light-dark().
Relevance

⭐⭐⭐ High

Team often fixes inaccurate/stale references in comments/paths (accepted in PR #3, #425).

PR-#3
PR-#425

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The added comment cites core/tokens.css:364-373, but the clamp() derivation formula it points to
appears later in the current core/tokens.css (in/after the “@supports gate: light-dark() +
oklch(from …)” block), so the cited range is already off.

core/themes.css[59-65]
core/tokens.css[362-385]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
A comment added in `core/themes.css` cites a specific line range in `core/tokens.css` (`core/tokens.css:364-373`) for the dark clamp() derivation formula. In the current branch, the referenced content is already at a different location, making the citation misleading.

### Issue Context
This is documentation-only, but the goal of SL-001 is to help maintainers keep two duplicated formulas in sync. If the pointer is wrong, it undermines that goal.

### Fix Focus Areas
- core/themes.css[60-65]

### Suggested fix
Update the comment to avoid hardcoded line numbers (e.g., reference a unique heading/comment string to search for, or just `core/tokens.css` without a line range).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread core/themes.css Outdated
claude added 2 commits July 2, 2026 06:13
core/themes.css's cross-link to core/tokens.css cited a specific line
range (364-373) that will drift as either file is edited independently.
Replaced with a stable text anchor (the section's own comment text) to
search for instead, matching how the tokens.css side already referenced
themes.css by section name (SECTION-LEVEL THEMING) rather than by line
number. Caught by Qodo's review on PR 475.
…r commit

A stray 'rm -rf badges' before rebuilding removed the tracked
badge-optimal.json along with the gitignored bundle files, and the
subsequent commit picked up the deletion. Restoring it unchanged - no
content difference, it was regenerated identically by npm run build.
@jackgranatowski
jackgranatowski merged commit 04f2fc5 into claude/pr-469-audit-rebase-ggp0e4 Jul 2, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants