Skip to content

fix: 16 CSS correctness issues found in systematic audit - #155

Merged
jackgranatowski merged 2 commits into
mainfrom
claude/slashed-css-review-JmwHr
May 29, 2026
Merged

fix: 16 CSS correctness issues found in systematic audit#155
jackgranatowski merged 2 commits into
mainfrom
claude/slashed-css-review-JmwHr

Conversation

@jackgranatowski

@jackgranatowski jackgranatowski commented May 29, 2026

Copy link
Copy Markdown
Contributor

Cascade / specificity

  • layout.css: replace physical left with logical-safe approach for
    .sf-imposter — logical inset-inline-start:50% maps to right:50% in
    RTL, causing translate(-50%) to shift away from center; use physical
    left:50% to match translate's physical x-axis

pointer-events / cursor

  • states.css: remove cursor:not-allowed from .is-disabled — pointer-
    events:none on the same element prevents any cursor from rendering,
    making the declaration dead code

Token & variable hygiene

  • tokens.css: change scroll-timeline-range token defaults from 0%/100%
    to entry 0%/cover 30% — the previous values made the var() fallbacks
    in motion.css permanently unreachable dead code and produced full-
    view-timeline animations rather than the intended entrance-only window
  • forms.css: add ,solid fallback to three var(--sf-focus-ring-style)
    outlines — every other focus-ring rule in the codebase provides this
    fallback (accessibility.css:28/182/269, states.css:313)
  • forms.css: add var(--sf-color-action) fallback to two hover rules
    that reference --sf-color-action-hover, which is defined only in the
    optional tokens.palette.css; without the fallback, button hover
    silently removes the background if the palette is not loaded

Physical vs logical properties

  • reset.css: media elements max-width→max-inline-size, height→block-size
  • layout.css: .sf-frame img/video width→inline-size, height→block-size
  • layout.css: .sf-cover--max min-height/max-height → min/max-block-size
  • accessibility.css: touch target min-width/min-height →
    min-inline-size/min-block-size

Vendor-prefix stylelint disable comments (project convention)

  • macros.css: add comment before -webkit-box-orient
  • macros.css: add comment before each -webkit-line-clamp (×3 rules)
  • macros.css: add comment before -webkit-tap-highlight-color

theme-example.css reference corrections

  • base-light value changed from 0.99→0.97 lightness; 0.99 degenerates
    the +0.02/+0.04 surface scale (bg and raised both clamp to white),
    which tokens.css explicitly documents and warns against
  • --sf-border-style-soft reference corrected solid→dashed (actual default)
  • z-index reference table corrected: --sf-z-raised 20→1, --sf-z-mid
    50→100, --sf-z-high 100→500, --sf-z-top 500→900

tokens.components.css (commented)

  • --sf-badge-radius fallback 999px→9999px to match project convention

https://claude.ai/code/session_01D8NLJpDvBWoi6RDfmNgifm

Summary by CodeRabbit

  • Bug Fixes

    • Improved touch target sizing and accessibility support with logical CSS properties.
    • Enhanced disabled state styling with improved text selection handling.
  • Style

    • Updated form focus ring styling with fallback color support.
    • Adjusted scroll animation timeline ranges for better visual timing.
    • Refined theme example values including colors and z-index scales.
    • Improved badge border-radius fallback.

Review Change Stack

Cascade / specificity
- layout.css: replace physical left with logical-safe approach for
  .sf-imposter — logical inset-inline-start:50% maps to right:50% in
  RTL, causing translate(-50%) to shift away from center; use physical
  left:50% to match translate's physical x-axis

pointer-events / cursor
- states.css: remove cursor:not-allowed from .is-disabled — pointer-
  events:none on the same element prevents any cursor from rendering,
  making the declaration dead code

Token & variable hygiene
- tokens.css: change scroll-timeline-range token defaults from 0%/100%
  to entry 0%/cover 30% — the previous values made the var() fallbacks
  in motion.css permanently unreachable dead code and produced full-
  view-timeline animations rather than the intended entrance-only window
- forms.css: add ,solid fallback to three var(--sf-focus-ring-style)
  outlines — every other focus-ring rule in the codebase provides this
  fallback (accessibility.css:28/182/269, states.css:313)
- forms.css: add var(--sf-color-action) fallback to two hover rules
  that reference --sf-color-action-hover, which is defined only in the
  optional tokens.palette.css; without the fallback, button hover
  silently removes the background if the palette is not loaded

Physical vs logical properties
- reset.css: media elements max-width→max-inline-size, height→block-size
- layout.css: .sf-frame img/video width→inline-size, height→block-size
- layout.css: .sf-cover--max min-height/max-height → min/max-block-size
- accessibility.css: touch target min-width/min-height →
  min-inline-size/min-block-size

Vendor-prefix stylelint disable comments (project convention)
- macros.css: add comment before -webkit-box-orient
- macros.css: add comment before each -webkit-line-clamp (×3 rules)
- macros.css: add comment before -webkit-tap-highlight-color

theme-example.css reference corrections
- base-light value changed from 0.99→0.97 lightness; 0.99 degenerates
  the +0.02/+0.04 surface scale (bg and raised both clamp to white),
  which tokens.css explicitly documents and warns against
- --sf-border-style-soft reference corrected solid→dashed (actual default)
- z-index reference table corrected: --sf-z-raised 20→1, --sf-z-mid
  50→100, --sf-z-high 100→500, --sf-z-top 500→900

tokens.components.css (commented)
- --sf-badge-radius fallback 999px→9999px to match project convention

https://claude.ai/code/session_01D8NLJpDvBWoi6RDfmNgifm
@coderabbitai

coderabbitai Bot commented May 29, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@jackgranatowski, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 40 minutes and 4 seconds. 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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: acaabbdd-8507-4c8d-8211-0a50acc99279

📥 Commits

Reviewing files that changed from the base of the PR and between 9cb31a6 and bb1070b.

⛔ Files ignored due to path filters (31)
  • dist/slashed-bricks.zip is excluded by !**/dist/**, !**/*.zip
  • dist/slashed.essential.css is excluded by !**/dist/**
  • dist/slashed.essential.flat.css is excluded by !**/dist/**
  • dist/slashed.essential.flat.min.css is excluded by !**/dist/**
  • dist/slashed.essential.flat.min.css.map is excluded by !**/dist/**, !**/*.map
  • dist/slashed.essential.min.css is excluded by !**/dist/**
  • dist/slashed.essential.min.css.map is excluded by !**/dist/**, !**/*.map
  • dist/slashed.full.css is excluded by !**/dist/**
  • dist/slashed.full.flat.css is excluded by !**/dist/**
  • dist/slashed.full.flat.min.css is excluded by !**/dist/**
  • dist/slashed.full.flat.min.css.map is excluded by !**/dist/**, !**/*.map
  • dist/slashed.full.min.css is excluded by !**/dist/**
  • dist/slashed.full.min.css.map is excluded by !**/dist/**, !**/*.map
  • dist/slashed.optimal-components.css is excluded by !**/dist/**
  • dist/slashed.optimal-components.flat.css is excluded by !**/dist/**
  • dist/slashed.optimal-components.flat.min.css is excluded by !**/dist/**
  • dist/slashed.optimal-components.flat.min.css.map is excluded by !**/dist/**, !**/*.map
  • dist/slashed.optimal-components.min.css is excluded by !**/dist/**
  • dist/slashed.optimal-components.min.css.map is excluded by !**/dist/**, !**/*.map
  • dist/slashed.optimal-utilities.css is excluded by !**/dist/**
  • dist/slashed.optimal-utilities.flat.css is excluded by !**/dist/**
  • dist/slashed.optimal-utilities.flat.min.css is excluded by !**/dist/**
  • dist/slashed.optimal-utilities.flat.min.css.map is excluded by !**/dist/**, !**/*.map
  • dist/slashed.optimal-utilities.min.css is excluded by !**/dist/**
  • dist/slashed.optimal-utilities.min.css.map is excluded by !**/dist/**, !**/*.map
  • dist/slashed.optimal.css is excluded by !**/dist/**
  • dist/slashed.optimal.flat.css is excluded by !**/dist/**
  • dist/slashed.optimal.flat.min.css is excluded by !**/dist/**
  • dist/slashed.optimal.flat.min.css.map is excluded by !**/dist/**, !**/*.map
  • dist/slashed.optimal.min.css is excluded by !**/dist/**
  • dist/slashed.optimal.min.css.map is excluded by !**/dist/**, !**/*.map
📒 Files selected for processing (2)
  • core/states.css
  • docs/tokens.md
📝 Walkthrough

Walkthrough

This PR modernizes CSS across core and optional stylesheets by migrating physical sizing properties to logical equivalents for better internationalization, adding robustness to form control styling through CSS variable fallbacks, adjusting disabled state interactions, and refining design token values and linting practices.

Changes

CSS Properties Modernization and Token Updates

Layer / File(s) Summary
Core sizing modernization — media and touch-target logical properties
core/reset.css, core/accessibility.css
Media element resets and touch-target sizing constraints switched from physical width/height and min-width/min-height to logical inline-size/block-size and min-block-size/min-inline-size properties.
Layout utilities modernization — logical properties with RTL considerations
core/layout.css
Layout utility rules updated to use logical properties: .sf-cover--max uses min/max-block-size, .sf-frame media uses inline-size/block-size, and .sf-imposter centering switches from inset-inline-start to left with clarifying comments on physical vs logical behavior for RTL.
Form control robustness — focus-ring and hover color fallbacks
optional/forms.css
Form control focus and hover states now include CSS variable fallbacks: focus-ring outline defaults --sf-focus-ring-style to solid, and button/input hover backgrounds fall back from --sf-color-action-hover to --sf-color-action when the hover token is unset.
State interactions, linting, and token adjustments
core/states.css, core/macros.css, core/tokens.css, optional/theme-example.css, optional/tokens.components.css
Disabled state removes cursor constraint and adds user-select prevention; scroll-timeline tokens updated to element-tied syntax; stylelint annotations added for vendor-prefixed properties; theme example and component token values adjusted for surface color, border style, z-index scale, and badge radius defaults.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • codeslash-dev/SLASHED#78: Both PRs modify the @media (pointer: coarse) touch-target sizing in core/accessibility.css, changing how the min size is expressed.
  • codeslash-dev/SLASHED#141: The main PR updates core accessibility/layout/reset/states/token CSS rules, and the retrieved PR adds Playwright assertions that directly validate those same classes/CSS custom properties via computed styles.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the primary change: fixing 16 CSS correctness issues discovered through a systematic audit, which aligns with the comprehensive CSS fixes across multiple files.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/slashed-css-review-JmwHr

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 and usage tips.

…d docs

cursor:not-allowed was removed in the previous audit commit on the grounds
that pointer-events:none means it never renders visually on the element
itself. That reasoning is correct but incomplete — the CSS value IS
inherited by child elements that re-enable pointer-events:auto, and the
property is part of the documented .is-disabled API surface (asserted by
tests/states-full.spec.js). Restore it with a comment explaining the intent.

Rebuild dist bundles and regenerate docs/tokens.md (scroll-timeline range
token default values changed to entry 0% / cover 30% in the prior commit).

https://claude.ai/code/session_01D8NLJpDvBWoi6RDfmNgifm
@jackgranatowski
jackgranatowski merged commit c5bb3dd into main May 29, 2026
9 checks passed
@jackgranatowski
jackgranatowski deleted the claude/slashed-css-review-JmwHr branch May 31, 2026 18:01
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