Skip to content

Add forceEditable prop to TokenRow for Smart Settings - #395

Merged
jackgranatowski merged 2 commits into
mainfrom
claude/dreamy-einstein-2ihv92
Jun 24, 2026
Merged

Add forceEditable prop to TokenRow for Smart Settings#395
jackgranatowski merged 2 commits into
mainfrom
claude/dreamy-einstein-2ihv92

Conversation

@jackgranatowski

Copy link
Copy Markdown
Contributor

Summary

Adds a new forceEditable prop to the TokenRow component to allow consumption-role tokens to be rendered as editable in Smart Settings panels, where they are explicitly surfaced as configurable entry points.

Changes

  • TokenRow.svelte

    • Added forceEditable prop (default: false) to component signature with JSDoc documentation
    • Updated row styling logic: row--modified class now applies when modified && (!isConsume || forceEditable)
    • Updated row styling logic: row--consume class now applies when isConsume && !forceEditable
    • Updated control rendering: TokenEditor now displays when !isConsume || forceEditable (previously only !isConsume)
    • Fixed JSDoc alignment for consistency
  • SmartSettings.svelte

    • Applied forceEditable prop to all TokenRow instances in Smart Settings panels:
      • Raw gradient token row
      • Duration and easing token rows
      • Control token rows
    • This enables editing of consumption-role tokens within the Smart Settings UI while maintaining their read-only status in Advanced mode

Implementation Details

The forceEditable prop overrides the normal consumption-role behavior, allowing tokens that would normally be read-only (because they are derived from configure tokens) to be edited when explicitly surfaced in Smart Settings. The styling and control visibility logic respects this override, ensuring the UI correctly reflects the editable state.

https://claude.ai/code/session_01SrEDodTGN4MpmWJfP1p5Vb

…consumption

18 tokens surfaced in Smart Settings panels (typography, spacing, borders,
shadows, layout) carry role="consumption" in the API index because their
CSS defaults reference other variables via var()/calc(). TokenRow was
treating this as read-only, greying them out with a dashed box.

Added a `forceEditable` prop to TokenRow that bypasses the consumption
guard when a token is explicitly curated as a configurable entry point.
SmartSettings now passes forceEditable to every TokenRow it renders so
all controls — including Heading font, radius ramp, shadow ramp, gutter,
touch-target, etc. — are fully editable with live preview and reset.

The full All Tokens catalogue view is unchanged: consumption tokens remain
read-only there to preserve the "edit the upstream configure token" guidance.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SrEDodTGN4MpmWJfP1p5Vb
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@coderabbitai

coderabbitai Bot commented Jun 24, 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 32 minutes and 6 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: dadf1827-8e2f-4969-b7c0-95933fc0d020

📥 Commits

Reviewing files that changed from the base of the PR and between 129d78a and 4894e93.

📒 Files selected for processing (3)
  • configurator/src/components/SmartSettings.svelte
  • configurator/src/components/TokenEditor.svelte
  • configurator/src/components/TokenRow.svelte
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/dreamy-einstein-2ihv92

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

Allow Smart Settings to edit consumption-role tokens via TokenRow forceEditable
🐞 Bug fix ✨ Enhancement 🕐 10-20 Minutes

Grey Divider

Description

• Add forceEditable override to make curated consumption tokens editable in Smart Settings.
• Update TokenRow styling and control gating to reflect overridden editability.
• Pass forceEditable for all Smart Settings TokenRow instances (gradient, motion, controls).
Diagram

graph TD
  Smart["SmartSettings"] --> Row["TokenRow (forceEditable)"] --> Editor["TokenEditor"]
  Row --> Readonly["Readonly display"]
  Row --> Store[("overrides store")]
  Row --> Model["model: isConsumptionToken"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Create a SmartSettings-specific row wrapper/component
  • ➕ Avoids expanding TokenRow’s API surface area
  • ➕ Lets Smart Settings own any future special-casing (styling, help text, affordances)
  • ➖ Duplicate logic already centralized in TokenRow (modified state, readonly vs editor rendering)
  • ➖ More components to maintain and keep behavior consistent with Advanced/catalogue views
2. Drive editability from token metadata / section configuration (no prop)
  • ➕ Keeps TokenRow stateless with respect to context-specific policy
  • ➕ Centralizes “what is editable where” in domain settings data
  • ➖ Requires plumbing additional metadata through SmartSettings and potentially other callers
  • ➖ Harder to reason about at call sites versus an explicit forceEditable flag
3. Change the API index role classification for curated tokens
  • ➕ Eliminates the need for UI overrides; role would match intended edit surface
  • ➖ Conceptually incorrect if the token is still derived/consumption in the broader system
  • ➖ Risk of confusing other consumers of the index that rely on role semantics

Recommendation: Keep the current forceEditable prop approach: it’s explicit at the SmartSettings call site, preserves correct consumption semantics elsewhere (catalogue/Advanced), and minimally changes behavior to only the curated surfaces that require it.

Files changed (2) +20 / -16

Enhancement (1) +17 / -13
TokenRow.svelteAdd forceEditable prop to override consumption read-only behavior +17/-13

Add forceEditable prop to override consumption read-only behavior

• Introduces a 'forceEditable' prop (default 'false') with documentation. Updates modified/consume styling classes and readonly-vs-editor rendering so consumption tokens can be edited when explicitly overridden by the parent.

configurator/src/components/TokenRow.svelte

Bug fix (1) +3 / -3
SmartSettings.svelteForce TokenRow editing for all Smart Settings rows +3/-3

Force TokenRow editing for all Smart Settings rows

• Adds 'forceEditable' to each TokenRow rendered by Smart Settings (gradient raw token, motion duration/easing tokens, and general control tokens). This ensures curated Smart Settings controls remain editable even when the underlying token is marked as consumption-role.

configurator/src/components/SmartSettings.svelte

@qodo-code-review

qodo-code-review Bot commented Jun 24, 2026

Copy link
Copy Markdown

Code Review by Qodo

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

Context used
✅ Compliance rules (platform): 5 rules

Grey Divider


Remediation recommended

1. Picker init mismatch ✓ Resolved 🐞 Bug ≡ Correctness
Description
forceEditable can expose consume-role color tokens to OklchPicker, but the picker parser does
not handle derived expressions like oklch(from var(...)), so it fails to initialize from the
token’s current value. Opening the picker can therefore start from arbitrary defaults and emit
unrelated colors, causing unexpected override jumps.
Code

configurator/src/components/TokenRow.svelte[R169-172]

+    {#if isConsume && !forceEditable}
      <div
        class="row__readonly"
        title="Consumption token — derived from configure tokens. Edit the upstream source instead."
Evidence
TokenEditor passes current || token.value into OklchPicker. For consume-role tokens like
--sf-shadow-color, the catalogue default is a derived expression (oklch(from var(...))), but
OklchPicker.parse() only matches literal oklch(...)/oklab(...)/hex/hsl forms and therefore
will not initialize its internal channels from that value.

configurator/src/components/TokenEditor.svelte[138-158]
configurator/src/components/OklchPicker.svelte[145-184]
configurator/src/lib/domainSettings.js[128-142]
configurator/src/data/api-index.generated.json[13227-13239]

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

### Issue description
With `forceEditable`, consume-role tokens can now render `TokenEditor`, which for color-like tokens renders an `OklchPicker`. `OklchPicker.parse()` only supports a limited set of literal formats (`oklch(<L> <C> <H>)`, `oklab(...)`, hex, and some `hsl(...)`). It does **not** parse derived color expressions like `oklch(from var(...))`.

As a result, editing derived color tokens (e.g. `--sf-shadow-color`) will open a picker whose sliders are not initialized to the current color, and interacting with the picker can replace the derived expression with an unrelated literal.

### Issue Context
Smart Settings includes consume-role color tokens like `--sf-shadow-color` whose catalogue default is `oklch(from var(--sf-color-neutral) var(--sf-shadow-lightness) c h)`.

### Fix Focus Areas
- configurator/src/components/TokenEditor.svelte[138-168]
- configurator/src/components/OklchPicker.svelte[145-184]
- configurator/src/lib/domainSettings.js[128-141]

### Suggested fix
Implement one of:
1) **Disable/fallback**: if `(current || token.value)` is not parseable by `OklchPicker.parse`, do not show the swatch/picker (or disable it) and fall back to the plain text input for color tokens.
2) **Parse computed color**: when the value is unparseable, resolve the computed color (e.g. via a hidden probe element + `getComputedStyle`) and convert that into OKLCH sliders so the picker opens at the correct color.

Option (1) is the safest minimal change; option (2) gives the best UX but is more work.

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


2. Hidden consume overrides ✓ Resolved 🐞 Bug ≡ Correctness
Description
Smart Settings now allows overriding consumption-role tokens, but non-forceEditable views still
render those tokens via the readonly consume branch which displays token.value (not the active
override) and provides no per-token reset. This can misrepresent the currently-applied value and
makes the override hard to discover/clear outside Smart Settings.
Code

configurator/src/components/SmartSettings.svelte[148]

+              <TokenRow token={token(c.token)} label={c.label} help={c.help} showRawInfo forceEditable />
Evidence
SmartSettings now passes forceEditable to TokenRow, which enables editing (and thus creating
overrides) for consume-role tokens. However, when forceEditable is not provided (e.g.
DomainPanel/TokenGroup catalogue/advanced usage), consume-role tokens still render through the
readonly branch and that branch shows token.value even though the active value is
overrides[token.name] ?? token.value, and it omits the reset control entirely.

configurator/src/components/SmartSettings.svelte[139-151]
configurator/src/components/DomainPanel.svelte[352-372]
configurator/src/components/TokenGroup.svelte[31-39]
configurator/src/components/TokenRow.svelte[48-54]
configurator/src/components/TokenRow.svelte[168-185]

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

### Issue description
Smart Settings can now create overrides for consume-role tokens, but `TokenRow`’s consume readonly rendering path (used everywhere `forceEditable` is not set) still shows the framework default (`token.value`) and omits any reset affordance.

This causes two problems:
1) The UI can display a value that does not match what the system is actually applying (because overrides are applied from `overrides[token.name]`).
2) The override is not individually clearable in non-SmartSettings views.

### Issue Context
`TokenRow` already computes `modified` and `effectiveValue`, but the readonly consume branch ignores them.

### Fix Focus Areas
- configurator/src/components/TokenRow.svelte[48-54]
- configurator/src/components/TokenRow.svelte[85-86]
- configurator/src/components/TokenRow.svelte[168-185]

### Suggested fix
- In the `isConsume && !forceEditable` branch, render `effectiveValue` instead of `token.value` so the row reflects the active value.
- If `modified` is true, include a reset button (calling `clearOverride(token.name)`) even in the readonly consume branch, so overrides can be cleared from Advanced/catalogue views.
- Consider applying `row--modified` when `modified` is true regardless of consume status (or at least when `modified` is true for consume tokens) so overrides are visually discoverable.

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


Grey Divider

Qodo Logo

…ows; guard picker on parseability

Fixes two bugs surfaced by code review:

1. The consume readonly branch in TokenRow was displaying token.value
   (framework default) even when an active override existed, making the
   override invisible and impossible to clear from the catalogue view.
   Now shows effectiveValue (override ?? default), surfaces a reset
   button when modified, and applies row--modified for all modified
   tokens regardless of consume status.

2. TokenEditor's color picker (OklchPicker) cannot parse derived
   expressions like oklch(from var(...)). Opened against such a value
   it would start from arbitrary defaults and overwrite the expression
   with an unrelated literal. Now gated behind isPickerParseable() —
   tokens with unparseable default values fall back to the text input
   only until the user types a literal color that the picker can handle.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SrEDodTGN4MpmWJfP1p5Vb
@jackgranatowski
jackgranatowski merged commit 39d3944 into main Jun 24, 2026
21 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