fix(configurator): preview reachability on narrow viewports + QA-sweep fixes - #312
Merged
Conversation
Previously the preview pane was display:none below 1100px — the header toggle silently did nothing on smaller desktop windows — and below 600px the toggle button itself was hidden, so mobile had no path to the preview at all. - Below 1100px the preview is now a slide-over overlay (fixed, right edge, min(440px, 94vw)) above a click-to-dismiss scrim; it starts closed so the scrim never buries the panel on first paint - The header preview toggle stays visible at every width; only the sidebar toggle (genuinely ineffective on phones) is hidden <=600px Verified with Playwright at 1280px (pane toggles as before), 1000px (overlay opens, scrim closes) and 480px (button present, overlay opens and live-updates after a brand color edit). https://claude.ai/code/session_01DCCWK2EPSRdhBDZ7f25NxT
|
Warning Review limit reached
More reviews will be available in 41 minutes and 56 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe preview pane becomes a slide-over overlay on narrow viewports (≤1100px): it starts closed, opens via the header toggle, and closes via a scrim. Header/Home/Sidebar were wired to shared helpers for export and modified-counts; domain docs base URL and a radius-presets test were added. ChangesPreview slide-over overlay on narrow viewports
Shared helpers, domains, model, store, and tests
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 `@configurator/src/App.svelte`:
- Around line 58-62: The current $effect only checks
window.matchMedia('(max-width: 1100px)') once and never updates when the
viewport crosses that threshold; wrap the media query in a persistent listener:
create const mq = window.matchMedia('(max-width: 1100px)'), set the initial
state (if mq.matches then ui.previewOpen = false), add a change listener
(mq.addEventListener('change' or mq.addListener for older browsers) that sets
ui.previewOpen = false when e.matches is true), and remove that listener in the
$effect cleanup so the listener is torn down when the component unmounts;
reference the existing $effect, ui.previewOpen and window.matchMedia symbols
when applying this change.
🪄 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: 0049f36d-e021-4775-a3ab-b1b8d42bdc1c
📒 Files selected for processing (2)
configurator/src/App.svelteconfigurator/src/components/Header.svelte
Findings from a 7-angle static review of the IA-restructure diff plus a behavioral Playwright sweep (console-clean across every route in both modes at 1600/1000/480px; hostile-input sanitisation; corrupt-storage recovery; import round-trip; generator edge inputs; theme gallery): - App: the narrow-viewport check is now a matchMedia change listener — shrinking the window dismisses the preview overlay instead of dropping a scrim over the app, and widening it back restores the desktop pane (previously the preview stayed closed forever after one resize) - model.js: new modifiedCountsByDomain() shared by Sidebar badges and the Home checklist, so the two counts can never drift - store: openOutputDrawer() helper deduplicates the header pill and Home shortcut logic - Home: 'start here' pointer hoisted to a derived (was an O(n^2) findIndex re-run inside the each template) - domains.js: docs links base URL extracted to DOCS_BASE_URL - tests: border presets now pin the framework default radii (4/8/12px) so a framework retune redesigns the presets instead of silently skewing their semantics https://claude.ai/code/session_01DCCWK2EPSRdhBDZ7f25NxT
There was a problem hiding this comment.
🧹 Nitpick comments (1)
configurator/tests/style-presets.test.js (1)
73-81: 💤 Low valueConsider adding token existence checks for clearer test failures.
The test directly accesses
.valueon the result oftokenByName.get(). If a radius token is missing from the catalogue, this will throwTypeError: Cannot read property 'value' of undefinedrather than a clear assertion failure. Other tests in this file (lines 30-31) follow the pattern of checkingtokenByName.has(name)first.♻️ Proposed improvement for clearer test diagnostics
test('border presets stay anchored to the framework default radii', () => { // The preset semantics encode knowledge of the defaults: Subtle is half // of 4/8/12px, Rounded IS 4/8/12px, Pill scales beyond them. If the // framework retunes its radius steps this must fail so the presets get // redesigned alongside (same parity idea as the fluid-engine defaults). + assert.ok(tokenByName.has('--sf-radius-s'), '--sf-radius-s missing from catalogue'); + assert.ok(tokenByName.has('--sf-radius-m'), '--sf-radius-m missing from catalogue'); + assert.ok(tokenByName.has('--sf-radius-l'), '--sf-radius-l missing from catalogue'); assert.equal(tokenByName.get('--sf-radius-s').value, 'calc(4px * var(--sf-radius-scale))'); assert.equal(tokenByName.get('--sf-radius-m').value, 'calc(8px * var(--sf-radius-scale))'); assert.equal(tokenByName.get('--sf-radius-l').value, 'calc(12px * var(--sf-radius-scale))'); });🤖 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/tests/style-presets.test.js` around lines 73 - 81, The test "border presets stay anchored to the framework default radii" directly calls tokenByName.get(...).value which will throw if the token is missing; update the test to first assert tokenByName.has('--sf-radius-s'), tokenByName.has('--sf-radius-m'), and tokenByName.has('--sf-radius-l') before accessing .value so failures report missing tokens instead of TypeError, keeping the existing value assertions for tokenByName.get('--sf-radius-s').value, tokenByName.get('--sf-radius-m').value and tokenByName.get('--sf-radius-l').value unchanged.
🤖 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/tests/style-presets.test.js`:
- Around line 73-81: The test "border presets stay anchored to the framework
default radii" directly calls tokenByName.get(...).value which will throw if the
token is missing; update the test to first assert
tokenByName.has('--sf-radius-s'), tokenByName.has('--sf-radius-m'), and
tokenByName.has('--sf-radius-l') before accessing .value so failures report
missing tokens instead of TypeError, keeping the existing value assertions for
tokenByName.get('--sf-radius-s').value, tokenByName.get('--sf-radius-m').value
and tokenByName.get('--sf-radius-l').value unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8afb0088-a9d8-47f8-b47c-fc8f1ad8a903
📒 Files selected for processing (9)
configurator/src/App.svelteconfigurator/src/components/DomainPanel.svelteconfigurator/src/components/Header.svelteconfigurator/src/components/Home.svelteconfigurator/src/components/Sidebar.svelteconfigurator/src/lib/domains.jsconfigurator/src/lib/model.jsconfigurator/src/lib/store.svelte.jsconfigurator/tests/style-presets.test.js
🚧 Files skipped from review as they are similar to previous changes (1)
- configurator/src/App.svelte
Found by the exploratory QA sweep: pasting unparseable CSS (or CSS with only unknown tokens) into the import box reported 'Imported 0 tokens' yet still ran the destructive replaceOverrides(), deleting every active override. The import now bails out before replacing unless the paste contains at least one known token, with an explicit 'your overrides are untouched' message. Verified in-browser: garbage paste and unknown-token paste are no-ops (localStorage byte-identical), a valid paste still imports. https://claude.ai/code/session_01DCCWK2EPSRdhBDZ7f25NxT
QA sweep part 2 findings: - On narrow viewports the slide-over preview covered the header's theme / reduced-motion / preview toggles — the only place they existed — so the overlay could not be inspected in dark mode or reduced motion without closing it and toggling blind. The preview bar now carries its own light/dark + reduced-motion toggles (same ui state as the header) plus an explicit close button that renders only below 1100px - Output drawer CSS/Diff and @layer/:root segment buttons now expose aria-pressed, matching every other segmented control in the app - Dropped a dead pre-existing CSS rule in Preview.svelte Verified in-browser at 480/1000/1600px: overlay-bar toggles flip the stage for real, close button works, no horizontal overflow, exactly one aria-pressed=true per output segment group. https://claude.ai/code/session_01DCCWK2EPSRdhBDZ7f25NxT
This was referenced Jun 24, 2026
This was referenced Jul 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #311 (merged). Fixes the live preview being unreachable on narrower viewports, plus the findings of a full QA sweep of the restructured configurator (static review of the whole IA-restructure diff + exploratory Playwright pass — fixes incoming on this branch).
Preview reachability (was: broken on desktop windows < 1100px, no button at all on phones)
display: none— the header ◨ toggle silently did nothing; below 600px the toggle button itself was hidden, so mobile had no path to the preview at all.min(440px, 94vw), click-to-dismiss scrim), starting closed so the scrim never buries the panel on first paint. The ◨ toggle stays visible at every width; only the sidebar toggle (genuinely ineffective on phones) remains hidden ≤600px.QA-sweep fixes (pushed to this PR)
matchMediachange listener (previously a one-shot check: shrinking a desktop window with the preview open dropped an overlay+scrim onto the app, and widening back left the preview permanently closed).Testing
cd configurator && npm test(node --test)npm run buildhttps://claude.ai/code/session_01DCCWK2EPSRdhBDZ7f25NxT
Generated by Claude Code
Summary by CodeRabbit
New Features
Improvements
Tests