Skip to content

Expand Preview Hub with gradients, spacing, borders, shadows, motion, effects sections - #397

Closed
jackgranatowski wants to merge 5 commits into
mainfrom
claude/friendly-goodall-9o6laj
Closed

Expand Preview Hub with gradients, spacing, borders, shadows, motion, effects sections#397
jackgranatowski wants to merge 5 commits into
mainfrom
claude/friendly-goodall-9o6laj

Conversation

@jackgranatowski

@jackgranatowski jackgranatowski commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Summary

Significantly expands the Preview Hub (right panel) with six new design token showcase sections, plus three new specialized editor components for typography, border radius, and container widths. The preview now covers the full breadth of the design system, and the left panel's domain editors are streamlined to focus on controls rather than redundant previews.

Key Changes

Preview Hub Expansion

  • Gradients section: Brand gradients, directional fades, and gradient application on real components (hero + card)
  • Spacing section: Full space scale with fluid value visualization, component gap demos, and section padding examples
  • Borders section: Radius scale, border color variants, focus ring states, and border width ramp
  • Shadows section: Elevation ramp, shadows on real UI cards, specialty shadows (text, drop, glow, inner)
  • Motion section: Live animations (pulse, spin, bounce, slide, shimmer, ping), duration scale, easing functions, and transition demos
  • Effects section: Blur scale, opacity scale, scrim/overlay, and frosted glass examples

New Editor Components

  • HeadingEditor.svelte: Tabbed typography editor with live specimen for each heading level (H1–H6), body, and monospace. Shows all per-level tokens and global heading settings.
  • RadiusEditor.svelte: Tabbed radius editor with live shape specimen for each radius step (2xs–full). Includes mini shape grid for quick navigation.
  • ContainerBars.svelte: Visual ruler for container-width tokens, showing proportional bars and parsed pixel equivalents for each container size.

Domain Panel Refactor

  • Removed DomainPreview component and DOMAIN_PREVIEWS mapping
  • Left panel now focuses on editing controls (color roles, generators, smart settings) rather than redundant preview cards
  • Preview navigation now driven by DOMAIN_TO_SECTION mapping in Preview.svelte, which auto-routes users to the relevant section when switching domains

Navigation & State

  • Added DOMAIN_TO_SECTION mapping to auto-navigate Preview Hub when domain changes
  • Added $effect to sync active section with current domain
  • Integrated getFold/setFold from new foldState.js for persistent collapse state on ScaleGenerator

BrandColorRow Enhancement

  • Added inline shade strip showing all shade variants (superlight → superdark) for each brand color
  • Shade colors are measured live via measureBackground and parseRgb utilities
  • Perceived brightness calculation to support future contrast checks

Minor Updates

  • Renamed "Typography" tab to "Type" for brevity
  • ScaleGenerator now supports collapsible prop with fold-state persistence
  • SmartSettings integrated with fold-state for section collapse persistence

Implementation Notes

  • All new preview sections use CSS custom properties (--sf-*) to render live token values
  • Shade strip in BrandColorRow uses setProbeContext to measure computed colors in the current theme
  • Container bars parse CSS values (rem, ch, px, %) to compute relative widths
  • Motion demos respect the ui.previewMotion setting (reduced motion support)
  • All new components follow existing configurator patterns: reactive $derived, token lookups via tokenByName, override tracking

https://claude.ai/code/session_01Epa7UcqpdP8E7mcG9nHLWH

Summary by CodeRabbit

  • New Features
    • Added new token editors for headings/typography, border radius, and container sizing.
    • Expanded the preview with a new Gradients section plus token demos for spacing, borders, shadows, motion, and effects.
    • Added inline brand color shade ramps and a clearer container-width ruler.
  • Bug Fixes
    • Settings sections now persist expanded/collapsed state between visits.
  • UI Improvements
    • Refined the domain panel to show controls-focused editors instead of the in-panel live preview.
    • Improved scale controls with optional collapsible panels.

@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

Review Change Stack

📝 Walkthrough

Walkthrough

Adds persisted fold state, new typography/border/layout editor components, a brand color shade strip, and new preview sections. DomainPanel now routes selected domains to those editors and removes the previous inline live preview.

Changes

Configurator domain editor expansion

Layer / File(s) Summary
Fold-state persistence utility
configurator/src/lib/foldState.js
New module reads and writes per-section open state in localStorage using a versioned key and in-memory cache, and exports getFold and setFold helpers.
Fold state wired into SmartSettings and ScaleGenerator
configurator/src/components/SmartSettings.svelte, configurator/src/components/ScaleGenerator.svelte
SmartSettings uses fold state for each section’s details element, and ScaleGenerator adds a collapsible header toggle that persists its open state and conditionally renders the generator body.
HeadingEditor domain editor
configurator/src/components/HeadingEditor.svelte
New HeadingEditor.svelte derives preview styles from overrides and theme, groups heading tokens by tab, renders a tabbed specimen view, and styles the heading preview layout.
RadiusEditor domain editor
configurator/src/components/RadiusEditor.svelte
New RadiusEditor.svelte defines radius levels and tabs, renders all-level and per-level specimens with override indicators, and includes the editor styling for tabs and shapes.
ContainerBars domain editor
configurator/src/components/ContainerBars.svelte
New ContainerBars.svelte resolves container token values from overrides and defaults, converts common CSS units to pixels, renders proportional bars, and adds the component styling for rows and annotations.
DomainPanel wired to new editors
configurator/src/components/DomainPanel.svelte
DomainPanel removes the in-panel preview plumbing, changes Zone 1 to controls-only content, and adds domain-specific branches for typography, borders, and layout editors.
BrandColorRow shade-ramp strip
configurator/src/components/BrandColorRow.svelte
BrandColorRow measures seven shade variables per brand color, stores the results in a shade array, renders an inline swatch strip, and updates the row layout and strip styling.
Preview new sections
configurator/src/components/Preview.svelte
Preview.svelte adds gradients to the section list, renames typography to Type, maps domains to sections, and adds new preview content and styles for gradients, spacing, borders, shadows, motion, and effects.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • codeslash-dev/SLASHED#328: Both PRs change configurator/src/components/Preview.svelte to expand section-based preview routing and add new preview content.
  • codeslash-dev/SLASHED#337: Both PRs modify configurator/src/components/ScaleGenerator.svelte to add collapsible open/close behavior.
  • codeslash-dev/SLASHED#382: Both PRs affect the domain settings flow by changing DomainPanel.svelte and SmartSettings.svelte.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: expanding the Preview Hub with the new showcase sections.
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 docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/friendly-goodall-9o6laj

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

CI Feedback 🧐

A test triggered by this PR failed. Here is an AI-generated analysis of the failure:

Action: Lint commit messages

Failed stage: Validate PR commits against Conventional Commits [❌]

Failed test name: ""

Failure summary:

The action failed because commitlint rejected the commit message for not following Conventional
Commits rules:
- subject-case: the commit subject is in a disallowed case (it "must not be
sentence-case, start-case, pascal-case, upper-case").
- type-enum: the commit type is not one of the
allowed types [build, chore, ci, docs, feat, fix, perf, refactor, revert, style, test].
commitlint
reported 2 problems and exited with code 1, causing the workflow to fail.

Relevant error logs:
1:  ##[group]Runner Image Provisioner
2:  Hosted Compute Agent
...

489:  - Shadows: elevation ramp (xs–2xl), shadow on real cards, text/drop/glow/
490:  inner shadow examples.
491:  - Motion: animation demos (all 6 keyframes), duration sweep bars, easing
492:  sweep bars, hover-transition demo cards.
493:  - Effects: blur scale (backdrop-filter), opacity scale, scrim/overlay demo,
494:  frosted-glass panel.
495:  DX win: inline DomainPreview cards removed from the left panel — previews
496:  live exclusively in the right Preview Hub, which auto-syncs. Left panel is
497:  now purely editing controls, with more vertical space for token rows.
498:  Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
499:  Claude-Session: https://claude.ai/code/session_01Epa7UcqpdP8E7mcG9nHLWH�[22m
500:  �[31m✖�[39m   subject must not be sentence-case, start-case, pascal-case, upper-case �[90m[subject-case]�[39m
501:  �[31m✖�[39m   type must be one of [build, chore, ci, docs, feat, fix, perf, refactor, revert, style, test] �[90m[type-enum]�[39m
502:  �[1m�[31m✖�[39m   found 2 problems, 0 warnings�[22m
503:  ⓘ   Get help: https://github.com/conventional-changelog/commitlint/#what-is-commitlint
504:  ##[error]Process completed with exit code 1.
505:  Post job cleanup.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Expand Preview Hub with new token sections and specialized editors
✨ Enhancement ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

Description

• Add Preview Hub sections for gradients, spacing, borders, shadows, motion, and effects.
• Refocus Domain panel on editing controls with collapsible generators and new editors.
• Persist fold/collapse UI state and enhance brand color rows with inline shade ramps.
Diagram

graph TD
  U["User"] --> DP["DomainPanel.svelte"] --> UI["ui domain state"] --> PV["Preview.svelte"]
  DP --> SG["ScaleGenerator.svelte"] --> FS["foldState.js"] --> LS[("localStorage")]
  DP --> SS["SmartSettings.svelte"] --> FS
  DP --> HE["HeadingEditor.svelte"]
  DP --> RE["RadiusEditor.svelte"]
  DP --> CB["ContainerBars.svelte"]
  DP --> BCR["BrandColorRow.svelte"] --> PH["probeHost.js"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Split Preview.svelte into per-section components
  • ➕ Reduces a single very large component and improves navigability/ownership
  • ➕ Enables lazy-loading or conditional imports for heavy demo sections
  • ➖ More files/boilerplate and some shared CSS extraction work
  • ➖ May complicate shared preview styling/state without a clear section API
2. Use a Svelte store for fold state instead of a standalone module
  • ➕ Reactive updates if multiple components need to respond to the same fold key
  • ➕ Centralizes persistence + defaults and can support migrations/versioning
  • ➖ Requires store wiring and may increase coupling to Svelte runtime
  • ➖ Standalone module is simpler and works from both .js and .svelte as-is
3. Drive shade-strip values directly from token values when available
  • ➕ Avoids computed-style probing complexity and microtask scheduling
  • ➕ Clearer dependency tracking than measuring CSS output
  • ➖ Not all shades may exist as explicit tokens; computed vars may still be needed
  • ➖ Would require additional model exposure for derived shades

Recommendation: Keep the PR’s overall approach (domain-synced Preview Hub + control-focused Domain panel); it’s a clear UX improvement and the fold persistence is appropriately lightweight. Consider a follow-up to componentize Preview.svelte by section to keep growth manageable, but it doesn’t need to block this change.

Files changed (9) +1077 / -41

Enhancement (7) +1004 / -5
BrandColorRow.svelteAdd inline shade ramp strip for each brand color +58/-2

Add inline shade ramp strip for each brand color

• Adds a 7-swatch shade strip (superlight → superdark) under each brand color row. Uses probeHost measurement (with theme/override context) to sample computed CSS variable colors and includes a perceived brightness helper for future contrast work.

configurator/src/components/BrandColorRow.svelte

ContainerBars.svelteIntroduce container width ruler visualization component +119/-0

Introduce container width ruler visualization component

• Adds a new component that renders proportional bars for --sf-container-* tokens with live values and approximate pixel equivalents. Reads overrides/defaults and computes relative widths for quick comparison.

configurator/src/components/ContainerBars.svelte

HeadingEditor.svelteAdd tabbed typography editor with live specimens (H1–H6/body/mono) +203/-0

Add tabbed typography editor with live specimens (H1–H6/body/mono)

• Introduces a tabbed editor showing live rendered specimens and the relevant tokens per tab. Highlights modified tabs via override detection and applies preview theme declarations to the specimen area.

configurator/src/components/HeadingEditor.svelte

Preview.svelteExpand Preview Hub tabs and add six new token showcase sections +403/-1

Expand Preview Hub tabs and add six new token showcase sections

• Adds new Preview Hub sections for Gradients, Spacing, Borders, Shadows, Motion, and Effects with live component demos. Renames the Typography tab label to 'Type' and auto-syncs the active preview section to the selected domain via DOMAIN_TO_SECTION + $effect.

configurator/src/components/Preview.svelte

RadiusEditor.svelteAdd tabbed radius editor with shape specimens per radius level +193/-0

Add tabbed radius editor with shape specimens per radius level

• Introduces a radius-focused editor with an 'All' overview and per-level tabs, including a large specimen and mini navigation grid. Highlights modified levels via override detection and edits tokens via TokenRow.

configurator/src/components/RadiusEditor.svelte

ScaleGenerator.svelteMake ScaleGenerator optionally collapsible with persisted open state +22/-1

Make ScaleGenerator optionally collapsible with persisted open state

• Adds a collapsible mode with a header toggle and conditional rendering of the generator body. Persists open/closed state per generator kind using foldState.js (getFold/setFold).

configurator/src/components/ScaleGenerator.svelte

SmartSettings.sveltePersist SmartSettings section collapse state +6/-1

Persist SmartSettings section collapse state

• Switches SmartSettings sections from always-open to using foldState persistence keyed by domainId + section id. Writes updated open state on <details> toggle.

configurator/src/components/SmartSettings.svelte

Refactor (1) +28 / -36
DomainPanel.svelteRefactor Domain panel to remove redundant previews and add new editors +28/-36

Refactor Domain panel to remove redundant previews and add new editors

• Removes DomainPreview usage and the domain preview mapping-driven lead card behavior. Makes ScaleGenerator instances collapsible, and introduces domain-specific editors for typography (HeadingEditor), borders (RadiusEditor), and layout (ContainerBars).

configurator/src/components/DomainPanel.svelte

Other (1) +45 / -0
foldState.jsAdd localStorage-backed fold/collapse persistence utility +45/-0

Add localStorage-backed fold/collapse persistence utility

• Introduces a small module to read/write fold state keyed by string identifiers, persisted in localStorage. Designed to be usable from both .svelte and .js without relying on Svelte reactivity.

configurator/src/lib/foldState.js

@coderabbitai coderabbitai Bot added the codex label Jun 24, 2026
claude added 4 commits June 24, 2026 10:16
…oldable generators

Phase 1 — Foldable ScaleGenerator:
- Add `collapsible` prop to ScaleGenerator; when true, starts collapsed and
  renders a chevron toggle button in the header.
- DomainPanel passes `collapsible` to every ScaleGenerator instance.

Phase 2 — Preview Hub with domain auto-sync:
- Preview pane SECTIONS expanded from 7 → 13 tabs, one per framework domain:
  Overview · Colors · Gradients · Palette · Type · Spacing · Layout ·
  Borders · Shadows · Motion · Effects · Macros · Tokens
- $effect in Preview.svelte syncs activeSection to ui.domain on every
  domain-tab switch; users can still manually override the preview tab.

Phase 3 — New preview sections with live token feedback:
- Gradients: brand + directional fades + gradient-on-content hero card.
- Spacing: full 2xs–3xl ruler, gap demos, section-pad demo.
- Borders: radius ramp on real cards, border-color variants, focus-ring trio.
- Shadows: elevation ramp, shadow on real cards, text/drop/glow/inner examples.
- Motion: animation demos, duration sweep bars, easing sweep bars.
- Effects: blur scale, opacity scale, scrim/overlay demo, frosted-glass panel.

DX: inline DomainPreview cards removed — previews live exclusively in the
right Preview Hub, which auto-syncs. Left panel is now purely editing controls.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Epa7UcqpdP8E7mcG9nHLWH
…rt section defaults

- Add HeadingEditor.svelte: ACSS-style tabbed editor (All/H1-H6/Body/Mono) with
  live specimen preview and per-level token rows; replaces flat basicGroups for
  the Typography domain in DomainPanel
- Add foldState.js: lightweight localStorage persistence for open/closed state
  keyed by section ID; survives page reloads without touching the Svelte store
- Wire foldState into SmartSettings: sections default to closed and remember
  their state per domain (key: `domainId:sectionId`)
- Wire foldState into ScaleGenerator: collapsed state persists across page loads
  (key: `generator:type/display/space`)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Epa7UcqpdP8E7mcG9nHLWH
… width bars

- BrandColorRow: add 7-step inline shade strip below each brand color row,
  resolved live against the active preview theme using the probe host
- RadiusEditor.svelte: ACSS-style tabbed radius editor (All + 2xs→full tabs)
  with a proportional shape specimen per level and a mini-map row for quick
  navigation; wired into the Borders domain in DomainPanel
- ContainerBars.svelte: proportional bar chart for all --sf-container-* tokens
  showing relative widths with live px estimates; wired into the Layout domain
  in DomainPanel

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Epa7UcqpdP8E7mcG9nHLWH
@jackgranatowski
jackgranatowski force-pushed the claude/friendly-goodall-9o6laj branch from c27db03 to f952d5a Compare June 24, 2026 10:16
@qodo-code-review

qodo-code-review Bot commented Jun 24, 2026

Copy link
Copy Markdown

Code Review by Qodo

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

Context used
✅ Compliance rules (platform): 5 rules

Grey Divider


Informational

1. Unused ContainerBars imports ✓ Resolved 🐞 Bug ⚙ Maintainability
Description
ContainerBars imports ui, measureBackground, and setProbeContext, and defines token(), but
none are used. This leaves dead code that increases maintenance burden and can confuse future edits.
Code

configurator/src/components/ContainerBars.svelte[R7-21]

+  import { overrides, ui } from '../lib/store.svelte.js';
+  import { tokenByName, defaultsByName } from '../lib/model.js';
+  import { measureBackground, setProbeContext } from '../lib/probeHost.js';
+
+  const CONTAINERS = [
+    { name: '--sf-container-narrow', label: 'Narrow',  help: 'Asides and narrow columns' },
+    { name: '--sf-container-prose',  label: 'Prose',   help: 'Long-form readable line length' },
+    { name: '--sf-container-default',label: 'Default', help: 'Main content container' },
+    { name: '--sf-container-wide',   label: 'Wide',    help: 'Marketing & docs sections' },
+    { name: '--sf-container-full',   label: 'Full',    help: 'Edge-to-edge / fluid' },
+  ];
+
+  const exists = (name) => tokenByName.has(name);
+  const token = (name) => tokenByName.get(name);
+
Relevance

⭐⭐⭐ High

Repo often accepts cleanup removing unused imports/dead code to satisfy lint (e.g., css.js unused
import fix).

PR-#315

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The file imports ui, measureBackground, and setProbeContext, and declares token(), but the
rest of the component never references them.

configurator/src/components/ContainerBars.svelte[7-21]

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

## Issue description
`ContainerBars.svelte` contains unused imports and an unused helper.

## Issue Context
The component only uses `overrides`, `tokenByName.has`, and `defaultsByName.get`.

## Fix Focus Areas
- configurator/src/components/ContainerBars.svelte[7-21]

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


2. Full radius shown twice ✓ Resolved 🐞 Bug ≡ Correctness
Description
In the new Borders preview section, the radius scale renders full twice: once via the radii
array (which already includes full) and again via an extra hardcoded full item. This duplicates
UI and can mislead users about the token set.
Code

configurator/src/components/Preview.svelte[R1161-1171]

+        <div class="pv__radii">
+          {#each radii as r (r)}
+            <div class="pv__radii-item">
+              <span class="pv__radii-box" style="border-radius:var(--sf-radius-{r})"></span>
+              <code>{r}</code>
+            </div>
+          {/each}
+          <div class="pv__radii-item">
+            <span class="pv__radii-box" style="border-radius:var(--sf-radius-full,999px)"></span>
+            <code>full</code>
+          </div>
Relevance

⭐⭐ Medium

No clear historical evidence on fixing duplicated preview UI items; seems plausible but unproven.

PR-#328

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
radii is defined to include 'full', and the Borders section both iterates radii and then
appends an additional hardcoded full block.

configurator/src/components/Preview.svelte[100-105]
configurator/src/components/Preview.svelte[1161-1171]

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

## Issue description
The Borders preview radius ramp displays the `full` step twice.

## Issue Context
`radii` already contains `'full'`, and the Borders section appends another `full` item with a fallback value.

## Fix Focus Areas
- configurator/src/components/Preview.svelte[100-105]
- configurator/src/components/Preview.svelte[1161-1171]

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


3. Collapsible defaults inverted 🐞 Bug ≡ Correctness
Description
ScaleGenerator and SmartSettings default to getFold(..., false), so newly-collapsible sections
start closed on first use, hiding key controls until expanded. This is a behavior regression from
the prior always-open sections and likely unintentional given these are primary editing surfaces.
Code

configurator/src/components/ScaleGenerator.svelte[R37-43]

+  /** @type {{ kinds?: Array<'type'|'display'|'space'>, collapsible?: boolean }} */
+  let { kinds = ['type', 'display', 'space'], collapsible = false } = $props();
+
+  // svelte-ignore state_referenced_locally
+  const foldKey = `generator:${kinds[0]}`;
+  // svelte-ignore state_referenced_locally
+  let open = $state(collapsible ? getFold(foldKey, false) : true);
Relevance

⭐ Low

Historical change made generators intentionally start collapsed to save space; default-closed
behavior appears desired.

PR-#337

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
ScaleGenerator initializes open from getFold(foldKey, false) and hides the body unless open is
true, while DomainPanel enables collapsible for generators; SmartSettings similarly uses
getFold(..., false) for <details open=...> so sections start closed absent prior persisted
state.

configurator/src/components/ScaleGenerator.svelte[37-44]
configurator/src/components/ScaleGenerator.svelte[188-214]
configurator/src/components/DomainPanel.svelte[245-251]
configurator/src/components/SmartSettings.svelte[69-73]

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

## Issue description
New collapsible sections default to closed (`getFold(..., false)`), which hides generator controls and SmartSettings sections on first use.

## Issue Context
Before this PR, ScaleGenerator content and SmartSettings `<details>` sections were effectively open by default. Now `DomainPanel` renders ScaleGenerator with `collapsible`, and `SmartSettings` computes `open` from fold state with a default of `false`, causing first-time users to see collapsed UI.

## Fix Focus Areas
- configurator/src/components/ScaleGenerator.svelte[37-44]
- configurator/src/components/SmartSettings.svelte[69-73]

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


View more (1)
4. Redundant probe updates 🐞 Bug ➹ Performance
Description
BrandColorRow’s shade-strip effect calls setProbeContext for every row on any override/theme
change, even though App.svelte already keeps the singleton probe host in sync. This adds repeated
signature-building work across all rows and is avoidable overhead during edits.
Code

configurator/src/components/BrandColorRow.svelte[R82-91]

+  $effect(() => {
+    for (const k in overrides) void overrides[k];
+    void ui.previewTheme;
+    queueMicrotask(() => {
+      setProbeContext({ overrides, theme: ui.previewTheme });
+      shadeColors = SHADE_SUFFIXES.map((s) => {
+        const rgb = measureBackground(`var(--sf-color-${colorKey}${s})`);
+        return rgb && rgb !== 'rgba(0, 0, 0, 0)' ? rgb : null;
+      });
+    });
Relevance

⭐ Low

Team recently added per-component setProbeContext before measuring swatches (ShadeRamp) despite
App-level sync.

PR-#388
PR-#312

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new shade-strip effect sets probe context inside each BrandColorRow. App.svelte already runs a
global effect to set probe context on overrides/theme changes, and probeHost’s setProbeContext
does non-trivial signature construction (sort/join) even when it early-returns.

configurator/src/components/BrandColorRow.svelte[82-91]
configurator/src/App.svelte[104-113]
configurator/src/lib/probeHost.js[58-67]

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

## Issue description
`BrandColorRow` calls `setProbeContext()` in a per-row `$effect`, duplicating work already done globally in `App.svelte`.

## Issue Context
`setProbeContext` computes a signature by sorting/stringifying override keys/values to decide whether to rebuild the host style. When many `BrandColorRow` instances are mounted, each override change can trigger N redundant signature computations.

## Fix Focus Areas
- configurator/src/components/BrandColorRow.svelte[82-91]
- configurator/src/App.svelte[104-113]
- configurator/src/lib/probeHost.js[58-67]

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


Grey Divider

Qodo Logo

@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: 4

🧹 Nitpick comments (3)
configurator/src/components/ScaleGenerator.svelte (1)

188-195: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Redundant collapsible guard inside the {#if collapsible} block.

The toggle button only renders when collapsible is true, so the inline if (collapsible) on Line 191 is always true. Harmless, but you can simplify.

♻️ Simplify
-        onclick={() => { open = !open; if (collapsible) setFold(foldKey, open); }}
+        onclick={() => { open = !open; setFold(foldKey, open); }}
🤖 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 `@configurator/src/components/ScaleGenerator.svelte` around lines 188 - 195,
The toggle handler in ScaleGenerator.svelte has a redundant collapsible check
inside the existing {`#if` collapsible} block. Simplify the button’s onclick logic
by removing the inner if and keeping the open toggle plus setFold(foldKey, open)
call tied to the button that only renders when collapsible is true, using the
gen__toggle control as the location to update.
configurator/src/components/BrandColorRow.svelte (2)

82-92: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoff

Per-row probe measurement may cause layout thrashing.

Each BrandColorRow instance runs this effect and, in the microtask, calls measureBackground 7 times — each invocation appends a probe element and reads getComputedStyle().backgroundColor, forcing a style/layout flush. With core + extended + status rows rendered together, this is dozens of synchronous reflows on every overrides/previewTheme change. The probe host's contain:strict bounds it somewhat and setProbeContext is signature-cached, but the repeated getComputedStyle reads are the hot path.

Consider batching the measurement across rows (single shared effect that measures all visible color keys once) or debouncing/coalescing per change, rather than one effect-per-row.

🤖 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 `@configurator/src/components/BrandColorRow.svelte` around lines 82 - 92, The
per-row measurement logic in BrandColorRow’s $effect is triggering too many
synchronous style flushes by calling measureBackground repeatedly for every row
and every shade. Refactor this so color probing is batched or coalesced across
rows—ideally via a shared measurement path keyed by colorKey and ui.previewTheme
rather than one queueMicrotask per component instance—while still updating
setProbeContext and shadeColors from the shared result.

94-98: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

perceived() is unused dead code.

This helper (and its parseRgb import) is defined but never referenced in the component. Either remove it, or wire it up if it was meant to drive something (e.g. picking a readable text/label color over each shade swatch, or ordering the ramp).

Want me to remove it, or implement the intended usage (e.g. an accessible label overlay per swatch)?

🤖 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 `@configurator/src/components/BrandColorRow.svelte` around lines 94 - 98, The
perceived() helper in BrandColorRow.svelte is currently unused dead code, along
with its parseRgb import. Either remove perceived() and the unused import if it
is not needed, or wire perceived() into the component’s rendering logic (for
example in the swatch markup or ramp ordering) so it actually influences
behavior, and ensure any related readable-label/text-color logic uses it
consistently.
🤖 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 `@configurator/src/components/ContainerBars.svelte`:
- Line 52: The width calculation in ContainerBars.svelte treats unknown
container sizes as full width, which makes unmeasurable values look like the
largest bar. Update the pct logic in the ContainerBars rendering block so that
when row.px is null/undefined it uses a neutral fallback (for example, zero or a
distinct placeholder width) instead of 100, and keep the existing maxPx-based
scaling for measurable rows.

In `@configurator/src/components/HeadingEditor.svelte`:
- Around line 57-61: The modified indicator logic in tabHasOverride is
incomplete for the all tab, because it only checks GLOBAL_HEADING_TOKENS and
ignores overrides in BODY_TOKENS, MONO_TOKENS, and PER_LEVEL_TOKENS. Update the
all branch in HeadingEditor.svelte so it aggregates hasOverride across every
token group used by the other tabs, ensuring any Body, Mono, or heading-level
edit also marks “All” as modified. Use the existing tabHasOverride and
hasOverride helpers to keep the behavior consistent.

In `@configurator/src/components/RadiusEditor.svelte`:
- Around line 33-36: The tabHasOverride helper in RadiusEditor.svelte is only
checking GLOBAL_TOKENS for the all tab, so the “All” badge misses overrides that
exist on individual levels. Update tabHasOverride so the all branch also
considers the LEVELS entries (using hasOverride on each level.token) while
preserving the existing per-level lookup for specific tab ids.

In `@configurator/src/lib/foldState.js`:
- Around line 16-21: The fold state hydration in foldState.js can assign a
primitive value to _state when JSON.parse returns null, true, a number, or
another non-object, which later breaks getFold and setFold when they use the in
operator. Update the localStorage load path so that after parsing, _state is
only accepted if it is a plain object/map-like value; otherwise fall back to the
default state shape. Keep the fix localized to the fold state initialization and
preserve the existing getFold/setFold behavior by ensuring _state is always safe
for key checks.

---

Nitpick comments:
In `@configurator/src/components/BrandColorRow.svelte`:
- Around line 82-92: The per-row measurement logic in BrandColorRow’s $effect is
triggering too many synchronous style flushes by calling measureBackground
repeatedly for every row and every shade. Refactor this so color probing is
batched or coalesced across rows—ideally via a shared measurement path keyed by
colorKey and ui.previewTheme rather than one queueMicrotask per component
instance—while still updating setProbeContext and shadeColors from the shared
result.
- Around line 94-98: The perceived() helper in BrandColorRow.svelte is currently
unused dead code, along with its parseRgb import. Either remove perceived() and
the unused import if it is not needed, or wire perceived() into the component’s
rendering logic (for example in the swatch markup or ramp ordering) so it
actually influences behavior, and ensure any related readable-label/text-color
logic uses it consistently.

In `@configurator/src/components/ScaleGenerator.svelte`:
- Around line 188-195: The toggle handler in ScaleGenerator.svelte has a
redundant collapsible check inside the existing {`#if` collapsible} block.
Simplify the button’s onclick logic by removing the inner if and keeping the
open toggle plus setFold(foldKey, open) call tied to the button that only
renders when collapsible is true, using the gen__toggle control as the location
to update.
🪄 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: c939b53e-769c-4995-bc1e-8155f63eeb5b

📥 Commits

Reviewing files that changed from the base of the PR and between 1a3197a and c27db03.

📒 Files selected for processing (9)
  • configurator/src/components/BrandColorRow.svelte
  • configurator/src/components/ContainerBars.svelte
  • configurator/src/components/DomainPanel.svelte
  • configurator/src/components/HeadingEditor.svelte
  • configurator/src/components/Preview.svelte
  • configurator/src/components/RadiusEditor.svelte
  • configurator/src/components/ScaleGenerator.svelte
  • configurator/src/components/SmartSettings.svelte
  • configurator/src/lib/foldState.js

Comment thread configurator/src/components/ContainerBars.svelte Outdated
Comment thread configurator/src/components/HeadingEditor.svelte
Comment thread configurator/src/components/RadiusEditor.svelte
Comment thread configurator/src/lib/foldState.js
- foldState: guard localStorage parse result against non-object types
- ScaleGenerator: remove redundant collapsible guard in toggle handler
- HeadingEditor: include all token groups in 'all' tab override detection
- RadiusEditor: include per-level tokens in 'all' tab override detection
- ContainerBars: use 0 fallback pct for unmeasurable widths (not 100)
- ContainerBars: remove unused imports from earlier Qodo cleanup
- Preview: remove hardcoded duplicate full-radius item
- BrandColorRow: remove unused perceived() helper and parseRgb import

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Epa7UcqpdP8E7mcG9nHLWH
@jackgranatowski

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 24, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

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

🧹 Nitpick comments (1)
configurator/src/components/BrandColorRow.svelte (1)

81-91: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Every row re-measures all 7 shades on any override change.

This effect depends on the entire overrides map, so editing any single token re-runs the effect for every BrandColorRow. Each run does 7 measureBackground calls, and each call appends a probe and reads getComputedStyle().backgroundColor, forcing a style/layout recalc. With several brand-color rows this is dozens of synchronous reflows per keystroke and can cause input jank.

Consider debouncing the remeasure (e.g. coalesce across rapid edits) or scoping re-measurement to changes that can actually affect this row's variables.

🤖 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 `@configurator/src/components/BrandColorRow.svelte` around lines 81 - 91, The
BrandColorRow $effect is re-running shade measurement for every override change,
causing repeated synchronous reflows across all rows. Update the effect in
BrandColorRow.svelte so it no longer reacts to the entire overrides map on every
keystroke; instead, coalesce rapid updates with a debounce/microtask gate or
narrow the dependency so only changes that affect this row’s colorKey actually
trigger remeasurement. Keep the existing probe setup in setProbeContext and the
shadeColors recomputation, but ensure measureBackground is not invoked
redundantly for unrelated token edits.
🤖 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 `@configurator/src/components/BrandColorRow.svelte`:
- Around line 81-91: The BrandColorRow $effect is re-running shade measurement
for every override change, causing repeated synchronous reflows across all rows.
Update the effect in BrandColorRow.svelte so it no longer reacts to the entire
overrides map on every keystroke; instead, coalesce rapid updates with a
debounce/microtask gate or narrow the dependency so only changes that affect
this row’s colorKey actually trigger remeasurement. Keep the existing probe
setup in setProbeContext and the shadeColors recomputation, but ensure
measureBackground is not invoked redundantly for unrelated token edits.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 38b5033b-fb2f-47d4-81fc-cdb5c2b18c5d

📥 Commits

Reviewing files that changed from the base of the PR and between c27db03 and b8c2c67.

📒 Files selected for processing (9)
  • configurator/src/components/BrandColorRow.svelte
  • configurator/src/components/ContainerBars.svelte
  • configurator/src/components/DomainPanel.svelte
  • configurator/src/components/HeadingEditor.svelte
  • configurator/src/components/Preview.svelte
  • configurator/src/components/RadiusEditor.svelte
  • configurator/src/components/ScaleGenerator.svelte
  • configurator/src/components/SmartSettings.svelte
  • configurator/src/lib/foldState.js
🚧 Files skipped from review as they are similar to previous changes (8)
  • configurator/src/components/SmartSettings.svelte
  • configurator/src/components/HeadingEditor.svelte
  • configurator/src/components/RadiusEditor.svelte
  • configurator/src/lib/foldState.js
  • configurator/src/components/Preview.svelte
  • configurator/src/components/ScaleGenerator.svelte
  • configurator/src/components/ContainerBars.svelte
  • configurator/src/components/DomainPanel.svelte

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants