Skip to content

feat(components): expand staged button/card surface - #442

Merged
jackgranatowski merged 2 commits into
mainfrom
claude/acss-slashed-components-analysis-sj77ci
Jun 28, 2026
Merged

feat(components): expand staged button/card surface#442
jackgranatowski merged 2 commits into
mainfrom
claude/acss-slashed-components-analysis-sj77ci

Conversation

@jackgranatowski

@jackgranatowski jackgranatowski commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Staged (commented until v0.8) additions to the optional component module,
keeping the framework's own mechanisms — design tokens, the OKLCH palette,
container queries instead of breakpoints, and explicit BEM classes instead
of fragile attribute selectors.

Smart-card subcomponents:

  • .sf-card__media (16/9 cover image, inner radius), .sf-card__avatar
    (round), .sf-card__title (unified heading size)
  • nested .sf-btn shrinks via --sf-card-btn-font-size token

Button semantic variants:

  • tokenize base font-size as --sf-btn-font-size
  • add --primary/--neutral (hover via --hover token) and
    --success/--warning/--info (hover via the direction-aware -strong
    triplet)
  • fix --danger hover to use --sf-color-danger-strong instead of
    filter: brightness() for correct light/dark behaviour

Full-width:

  • container-query full-width below 20rem with .sf-btn--auto opt-out
    (threshold is a literal — query conditions cannot read var())

New staged tokens: --sf-card-media-ratio/-radius, --sf-card-heading-size,
--sf-card-btn-font-size, --sf-btn-font-size.

Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01Crh1FKPkmEzMbUQbi2Fi8H

Summary by CodeRabbit

  • New Features
    • Added semantic button variants (primary, neutral, success, warning, info, danger).
    • Introduced full-width button options, including container-query–based sizing for narrow layouts.
    • Enhanced card styling with dedicated subcomponents for media, avatar, and title.
  • Bug Fixes
    • Updated danger button hover behavior to use consistent token-driven visuals.
    • Improved button sizing when placed inside cards for better typography alignment.

@coderabbitai

coderabbitai Bot commented Jun 28, 2026

Copy link
Copy Markdown

Review Change Stack

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 48 minutes and 2 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ 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.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

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: dd01493c-4068-43e1-b807-f6e84c669127

📥 Commits

Reviewing files that changed from the base of the PR and between 2aa447e and 0c5739d.

📒 Files selected for processing (2)
  • optional/components.css
  • optional/tokens.components.css
📝 Walkthrough

Walkthrough

Updates optional component CSS for .sf-btn semantic variants, token-based sizing, and width modifiers, then adds .sf-card subcomponent styling for media, avatar, title, and nested button font sizing. Token comment blocks are expanded to match the new component behavior.

Changes

Button and Card Component Enhancements

Layer / File(s) Summary
Button variants and sizing
optional/components.css, optional/tokens.components.css
Updates .sf-btn docs and base font-size, adds explicit semantic variants with token-driven hover styles, and adds block/container-query width modifiers. Token comments note the new button font-size token and container-query width behavior.
Card subcomponents and nested button override
optional/components.css, optional/tokens.components.css
Updates .sf-card docs, adds styles for .sf-card__media, .sf-card__avatar, and .sf-card__title, and scopes a .sf-btn font-size override inside cards. Token comments add matching card subcomponent notes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

  • Components: second batch #384: This PR extends the .sf-btn and .sf-card component work with semantic button variants, sizing rules, and card subcomponent 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 clearly summarizes the main change: staged button and card surface expansions in components.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/acss-slashed-components-analysis-sj77ci

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

Expand optional component CSS for staged button variants and card subcomponents
✨ Enhancement 🕐 20-40 Minutes

Grey Divider

Description

• Add semantic button variants with tokenized font sizing and consistent hover behavior.
• Introduce container-query full-width buttons with an explicit .sf-btn--auto opt-out.
• Expand smart-card subcomponents (__media, __avatar, __title) and related design tokens.
Diagram

graph TD
  TOK["optional/tokens.components.css"] --> CMP["optional/components.css"] --> BTN[".sf-btn variants"] --> CQ["@container full-width"]
  CMP --> CARD[".sf-card subparts"] --> SUBS["__media/__avatar/__title"]
  TOK --> BTN
  TOK --> CARD
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Derive hover colors via `color-mix()` for all semantic variants
  • ➕ Reduces the number of explicit --*-strong/--*--hover tokens needed
  • ➕ Can produce smoother, more uniform hover steps across themes
  • ➖ Harder to guarantee contrast/accessibility across light/dark palettes
  • ➖ Less aligned with the framework’s existing direction-aware strong triplet approach
2. Use responsive `@media` breakpoints for full-width behavior
  • ➕ More widely understood than container queries
  • ➕ Can be driven by global design tokens more easily
  • ➖ Doesn’t adapt to component embedding context (container width)
  • ➖ Contradicts the PR’s intent to avoid breakpoint-driven behavior

Recommendation: Keep the PR’s approach: semantic variants map directly to existing palette tokens (including the direction-aware -strong triplet for status colors), which is more predictable across themes than computed mixes. Using container queries for the full-width rule is consistent with context-driven layout and matches the stated framework direction; the explicit .sf-btn--auto opt-out keeps the behavior controllable.

Files changed (2) +115 / -4

Enhancement (1) +100 / -4
components.cssAdd semantic button variants, container-query full-width, and card subcomponents +100/-4

Add semantic button variants, container-query full-width, and card subcomponents

• Extends staged '.sf-btn' with a tokenized base font-size, adds semantic variants (primary/neutral/success/warning/info) and fixes danger hover to use the '-strong' token instead of brightness filtering. Adds container-query logic to make buttons full-width under 20rem with a '.sf-btn--auto' opt-out. Expands staged '.sf-card' with '__media', '__avatar', '__title', plus a scoped nested-button font-size override via '--sf-btn-font-size'.

optional/components.css

Other (1) +15 / -0
tokens.components.cssDocument staged tokens for smart-card subcomponents and button sizing/full-width behavior +15/-0

Document staged tokens for smart-card subcomponents and button sizing/full-width behavior

• Adds documentation for new staged card subcomponent tokens (media ratio/radius, heading size, nested button font-size) and introduces a documented '--sf-btn-font-size' token. Documents the container-query-driven full-width button behavior and notes why the 20rem threshold is a literal (container query conditions cannot read 'var()').

optional/tokens.components.css

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@optional/tokens.components.css`:
- Around line 55-58: The button token contract documents --sf-btn-min-height,
but .sf-btn does not actually use it because the staged button rule still
hardcodes var(--sf-touch-target). Update the .sf-btn styling in
optional/components.css so it consumes --sf-btn-min-height instead of the fixed
token, and keep the token definition in optional/tokens.components.css aligned
with the class implementation.
🪄 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: 1d37a616-7c30-46e2-828f-b50182acd5aa

📥 Commits

Reviewing files that changed from the base of the PR and between 2be3d13 and ac79bca.

📒 Files selected for processing (2)
  • optional/components.css
  • optional/tokens.components.css

Comment thread optional/tokens.components.css
@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

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

Context used
✅ Compliance rules (platform): 6 rules

Grey Divider


Remediation recommended

1. Warning hover on-color mismatch 🐞 Bug ≡ Correctness
Description
.sf-btn--warning:hover switches the background to --sf-color-warning-strong but keeps text color
--sf-color-text--on-warning, even though the framework’s own auto-contrast formula (threshold 0.6)
would choose the opposite text color for the strong variant’s lower lightness. With the default
palette, warning is L=0.75 (dark text) while warning-strong is derived as L=0.50 (light text), so
hover can render with the wrong on-color.
Code

optional/components.css[R103-111]

+  .sf-btn--warning {
+    background:   var(--sf-color-warning);
+    color:        var(--sf-color-text--on-warning);
+    border-color: var(--sf-color-warning);
+  }
+  .sf-btn--warning:hover:not(:disabled, .is-disabled, .is-loading) {
+    background:   var(--sf-color-warning-strong);
+    border-color: var(--sf-color-warning-strong);
+  }
Relevance

⭐⭐⭐ High

Repo accepts on-color/contrast fixes for semantic variants (e.g., restoring correct foreground
tokens in PR #386).

PR-#386

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The PR changes .sf-btn--warning:hover to use --sf-color-warning-strong but does not change
color, so it continues using --sf-color-text--on-warning. In core tokens, warning’s default
lightness is 0.75 (dark text selected by the 0.6 threshold), while warning-strong is explicitly
derived by subtracting 0.25 lightness (≈0.50), which falls below the threshold where the same
formula would select light text—demonstrating the mismatch.

optional/components.css[103-111]
core/tokens.css[28-46]
core/tokens.css[488-495]
core/tokens.css[583-623]
core/tokens.css[1381-1399]

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

### Issue description
`.sf-btn--warning:hover` changes `background`/`border-color` to `--sf-color-warning-strong` but leaves the text color as `--sf-color-text--on-warning`. In the core token system, `--sf-color-text--on-warning` is computed from the *base* warning color (L=0.75 by default) and therefore selects dark text, while `--sf-color-warning-strong` in light theme is derived as `l - 0.25` (≈ 0.50) which would flip the auto-contrast decision to light text.

### Issue Context
- Default warning source token is `oklch(0.75 0.17 80)` (L=0.75).
- `--sf-color-warning-strong` (light mode) is computed as `oklch(from var(--sf-color-warning-source-light) calc(l - 0.25) ...)` → L≈0.50.
- Auto-contrast threshold is `--sf-contrast-threshold: 0.6`, and the on-color tokens are computed with `clamp(0.1, sign(threshold - l) * 999, 0.95)`.

### Fix Focus Areas
- optional/components.css[103-111]
- core/tokens.css[28-46]
- core/tokens.css[488-495]
- core/tokens.css[583-623]
- core/tokens.css[1381-1399]

### Suggested fix
1. Introduce a dedicated token (e.g. `--sf-color-text--on-warning-strong`) in `core/tokens.css`, computed with the same auto-contrast formula but based on `--sf-color-warning-strong`.
2. Update `.sf-btn--warning:hover` to set `color: var(--sf-color-text--on-warning-strong, var(--sf-color-text--on-warning));` so the hover state uses the correct on-color.
3. If the token is public, update `tests/token-api.snapshot.json` accordingly to keep token API tests in sync.

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


Grey Divider

Qodo Logo

Staged (commented until v0.8) additions to the optional component module,
using the framework's own mechanisms — design tokens, the OKLCH palette,
container queries instead of breakpoints, and explicit BEM classes instead
of fragile attribute selectors.

Smart-card subcomponents:
- .sf-card__media (16/9 cover image, inner radius), .sf-card__avatar
  (round), .sf-card__title (unified heading size)
- nested .sf-btn shrinks via the --sf-card-btn-font-size token

Button semantic variants:
- tokenize base font-size as --sf-btn-font-size
- add --primary/--neutral (hover via the --hover token) and
  --success/--warning/--info (hover via the direction-aware -strong
  triplet)
- fix --danger hover to use --sf-color-danger-strong instead of
  filter: brightness() for correct light/dark behaviour

Button width (predictable 2-state model, verified in Chromium across
wide/narrow/no-container contexts):
- .sf-btn: intrinsic width by default (like Bootstrap/Bulma/Tailwind)
- .sf-btn--block: unconditional full width, works everywhere
- .sf-btn--block-cq: opt-in full width only inside a query container
  narrower than 20rem (needs a container ancestor: layout primitives
  or .sf-cq). No @media breakpoints.

New staged tokens: --sf-card-media-ratio/-radius, --sf-card-heading-size,
--sf-card-btn-font-size, --sf-btn-font-size.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Crh1FKPkmEzMbUQbi2Fi8H
@jackgranatowski
jackgranatowski force-pushed the claude/acss-slashed-components-analysis-sj77ci branch from 2aa447e to ed5c28c Compare June 28, 2026 10:04

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
optional/components.css (1)

212-218: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Make .sf-card__media block-level.

<img>/<video> are inline by default, so this selector will leave baseline whitespace under common card media usage. That creates a visible gap beneath the supposed edge-to-edge cover asset.

Suggested fix
  .sf-card__media {
+    display:       block;
     inline-size:   100%;
     aspect-ratio:  var(--sf-card-media-ratio, var(--sf-ratio-video));
     object-fit:    cover;
     border-radius: var(--sf-card-media-radius, var(--sf-card-radius, var(--sf-radius-m)));
     overflow:      clip;
  }
🤖 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 `@optional/components.css` around lines 212 - 218, The .sf-card__media rule
leaves inline replaced elements like img and video at inline layout, which can
create unwanted baseline whitespace under card media. Update the .sf-card__media
selector in components.css to make the media block-level while preserving the
existing sizing, aspect-ratio, object-fit, border-radius, and overflow behavior
so edge-to-edge card media renders flush.
🤖 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.

Outside diff comments:
In `@optional/components.css`:
- Around line 212-218: The .sf-card__media rule leaves inline replaced elements
like img and video at inline layout, which can create unwanted baseline
whitespace under card media. Update the .sf-card__media selector in
components.css to make the media block-level while preserving the existing
sizing, aspect-ratio, object-fit, border-radius, and overflow behavior so
edge-to-edge card media renders flush.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: dd357d41-111e-4b03-b7c3-9a367415fc45

📥 Commits

Reviewing files that changed from the base of the PR and between ac79bca and 2aa447e.

📒 Files selected for processing (2)
  • optional/components.css
  • optional/tokens.components.css
✅ Files skipped from review due to trivial changes (1)
  • optional/tokens.components.css

- .sf-btn consumes --sf-btn-min-height (fallback --sf-touch-target) so
  the documented token contract matches the class implementation
- .sf-card__media is display:block to avoid inline baseline whitespace
  under cover images/video

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Crh1FKPkmEzMbUQbi2Fi8H
@jackgranatowski
jackgranatowski merged commit 6a4f53e into main Jun 28, 2026
13 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