Skip to content

fix(configurator): mobile overlay, scale overflow, color-scheme picker - #319

Merged
jackgranatowski merged 3 commits into
mainfrom
claude/loving-galileo-0197q7
Jun 13, 2026
Merged

fix(configurator): mobile overlay, scale overflow, color-scheme picker#319
jackgranatowski merged 3 commits into
mainfrom
claude/loving-galileo-0197q7

Conversation

@jackgranatowski

@jackgranatowski jackgranatowski commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

$(cat <<'EOF'

Summary

Three bugs found and fixed — two root-cause discoveries after Playwright investigation:

1. ScaleGenerator <div class="preview"> rendered as fixed overlay on mobile (root cause of "transparent container" bug)

App.svelte used :global(.preview) to apply position: fixed; z-index: 50; box-shadow: -16px 0 48px rgba(0,0,0,0.5) to the live preview slide-over on narrow viewports. This selector matched every element with class preview in the document — including ScaleGenerator's internal <div class="preview"> (its scale visualization section).

On mobile, navigating to the Spacing or Typography tab (which renders a ScaleGenerator with basicGenerators) caused that div to be positioned as a 338×800 px fixed overlay covering the entire screen.

Fix: narrowed the selector to section.preview — the Preview component's root is a <section>, ScaleGenerator's internal div is a <div>, so the fix is surgical.

2. --sf-color-scheme got a color picker instead of a text input

isColorToken() in model.js checked token.namespace === 'color' before the hasTopLevelSpace(v) guard. --sf-color-scheme has namespace: "color" but value: "light dark" (a CSS keyword list, not a color). It was classified as a colour and rendered with the OKLCH picker.

Moved the hasTopLevelSpace check before the namespace check — tokens whose default value contains a top-level space (composite values, keyword lists) are never a single colour. Confirmed: only --sf-color-scheme changes behaviour across all 800+ tokens.

3. ScaleGenerator preview rows overflowing on mobile

Typography samples used font-size: clamp(...) at full token size — display-l rows were 50 px+ tall, making the scale preview dominate the screen. Spacing bars had no max-inline-size cap.

  • max-block-size: 3rem on .preview__sample caps row height on all viewports
  • max-inline-size: 100% on .preview__bar prevents bar overflow
  • At ≤600 px: 2-column grid (token + clamp value), visual sample hidden

Also fixed the same overflow in Preview.svelte's spacing scale section (pv__space-bar): added overflow: hidden on the row, flex-shrink: 0 on the label, max-inline-size: 100% + flex-shrink: 1 on the bar.

Test plan

  • Mobile (360 px): navigate to Spacing tab — no fixed overlay, no shadow, content fully interactive
  • Mobile (360 px): navigate to Typography tab — same
  • Spacing/Typography ScaleGenerator: scale preview rows don't dominate the screen; bars don't overflow
  • Live preview open on mobile: spacing scale bars shrink to fit the pane width
  • --sf-color-scheme token: shows text input, not color picker

https://claude.ai/code/session_01MAgtQ7JY16X2TqZZyGfkuu
EOF
)


Generated by Claude Code

Summary by CodeRabbit

  • Style
    • Improved preview pane layout responsiveness and CSS selectors for better viewport handling
    • Enhanced spacing scale display with improved overflow handling and text wrapping behavior
    • Added mobile-optimized layout adjustments for devices under 600px width
    • Fixed inline code and spacing bar sizing to prevent unexpected shrinking or expansion

claude added 3 commits June 13, 2026 08:24
… mobile

Typography samples (font-size: clamp(...)) were expanding each scale row
to the full rendered font size — display-l rows could be 50px+ tall,
making the scale preview dominate the screen. Spacing bars also had no
max-inline-size cap.

- Cap .preview__sample to max-block-size: 3rem so large text steps don't
  blow up the grid row height on any viewport
- Add max-inline-size: 100% to .preview__bar to prevent bar overflow
- At ≤600px: collapse to 2-column grid (token + clamp value), hide the
  visual sample column entirely — the layout was too cramped to be useful

https://claude.ai/code/session_01MAgtQ7JY16X2TqZZyGfkuu
…w pane

pv__space-bar uses inline-size: var(--sf-space-{s}) with no overflow
guard — large spacing tokens caused horizontal scroll (or bar bleedout)
inside the live preview overlay.

- overflow: hidden on pv__space-row clips the row
- flex-shrink: 0 on the code label keeps it readable
- max-inline-size: 100% + flex-shrink: 1 on the bar itself so it
  compresses rather than overflows when the pane is narrow (mobile)

https://claude.ai/code/session_01MAgtQ7JY16X2TqZZyGfkuu
1. ScaleGenerator overlay (the real "transparent container" bug)
   App.svelte used :global(.preview) which matched ALL elements with
   class preview — including ScaleGenerator's internal <div class="preview">.
   On mobile (≤1100px) that div was getting position:fixed, z-index:50 and
   the 48px drop-shadow, covering the entire screen on Spacing/Typography tabs.
   Fix: narrow the selector to section.preview (the Preview component root)
   so ScaleGenerator's div is unaffected.

2. --sf-color-scheme gets color picker instead of text input
   isColorToken() checked token.namespace === 'color' before the
   hasTopLevelSpace() guard, so --sf-color-scheme (value: "light dark")
   was classified as a colour despite being a CSS keyword list.
   Fix: move the value/space check before the namespace check so keyword
   lists like "light dark" fall through to the text control.

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

coderabbitai Bot commented Jun 13, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ffc330f0-8eef-469b-93fa-c9c5318d15d4

📥 Commits

Reviewing files that changed from the base of the PR and between 78f4591 and c09fc51.

📒 Files selected for processing (4)
  • configurator/src/App.svelte
  • configurator/src/components/Preview.svelte
  • configurator/src/components/ScaleGenerator.svelte
  • configurator/src/lib/model.js

📝 Walkthrough

Walkthrough

This PR refines component layout and spacing constraints across the configurator preview system and improves color token detection logic. CSS updates target preview responsiveness and flex behavior, while model.js clarifies the internal logic for distinguishing color tokens from composite values.

Changes

Layout and color token refinements

Layer / File(s) Summary
Narrow viewport preview selector
configurator/src/App.svelte
Responsive layout CSS selector updated from .preview to section.preview to more precisely target the preview pane element in narrow viewports.
Spacing scale UI layout constraints
configurator/src/components/Preview.svelte
Spacing row CSS improved by hiding overflow, preventing code label flex shrinking with flex-shrink: 0, and constraining spacing bar width with max-inline-size: 100% and flex-shrink: 1.
Scale preview responsive styling
configurator/src/components/ScaleGenerator.svelte
Preview sample baseline styling now includes max-block-size: 3rem; new mobile media query at max-width: 600px tightens column layout, hides sample element, and switches clamp text wrapping behavior for small screens.
Color token detection logic refactoring
configurator/src/lib/model.js
isColorToken function refactored to assign trimmed token.value before checking composite-value presence, with updated comments clarifying why shorthand/composite values are not treated as single colors.

Possibly related PRs

  • codeslash-dev/SLASHED#312: Both PRs update configurator/src/App.svelte preview layout styling within the narrow viewport media query, with #312 adjusting overlay/scrim behavior and this PR refining the CSS selector.
  • codeslash-dev/SLASHED#302: Color token detection refactoring in configurator/src/lib/model.js directly aligns with #302's changes to treat composite token values as non-color tokens.

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 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 three main fixes: mobile overlay issue in App.svelte (narrowing the selector to section.preview), scale overflow issues in ScaleGenerator and Preview components, and the color-scheme picker bug fix in model.js.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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/loving-galileo-0197q7

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.

@jackgranatowski
jackgranatowski merged commit ebb2641 into main Jun 13, 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