feat(configurator): redesign visual editor panels + mobile ergonomics + control UX - #402
Merged
Merged
Conversation
…comments on PR #401 CI fixes: - CategoryHeader: add panel__title class to h2 so shell/undo-redo e2e tests that cycle domains via keyboard still find the heading (9 tests × 3 browsers) - DomainPanel: restore ScaleGenerator for visual-studio domains (typography, spacing) — removes the !usesVisualStudio guard so generator.spec.js and the undo-redo spec can locate .gen elements - ColorStudio: add panel__card + cfg-card classes and rename "Main colors" disclosure to "Core brand colors" to satisfy shell.spec.js panel__card check - LayoutStudio: replace custom .container-rails/.rail markup with ContainerBars component so domain-preview.spec.js .cbars__bar assertions pass; convert workflow nav to semantic <ol><li>; fix Polish description - a11y.spec.js: expand allvars disclosure before querying .row__info-btn — Layout Settings now use FriendlyControl, raw TokenRows live in allvars CodeRabbit review fixes: - Header: import onDestroy and clearTimeout(_shareTimer) on component teardown - EffectsStudio / SpacingStudio / ColorStudio / TypographyStudio: replace mixed-Polish descriptions with full English copy - TypographyStudio: drive specimen content from the active tab (Headings shows heading stack, Body shows paragraph, Code shows mono sample, Overview all) - controlSchema: add options array when heuristic selects gradient-direction as a select control; check font-weight before the generic /font/ branch to give weight tokens a number control instead of a font-family picker - MotionStudio: add prefers-reduced-motion media query to disable infinite animations for users who have requested reduced motion Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A2b6qg3hbi9QsBZmaFwjUd
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A2b6qg3hbi9QsBZmaFwjUd
|
Warning Review limit reached
More reviews will be available in 7 minutes and 15 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 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 configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughThe PR adds studio-based configurator controls, shared token-editing components, new studio screens for multiple domains, and updated DomainPanel and Header rendering. It also adjusts focus behavior in tablist editors and adds schema and component tests. ChangesConfigurator studio redesign
Sequence Diagram(s)No additional diagrams. Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
PR Summary by QodoRedesign configurator domains with Studio editors and Friendly controls Description
Diagram
High-Level Assessment
Files changed (27)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
5 rules 1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (3)
configurator/src/components/editors/StudioWorkflow.svelte (1)
5-8: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse ordered-list semantics for the step sequence.
Line 5–8 renders ordered steps with
<span>nodes; using<ol>/<li>gives better accessibility semantics for this numbered workflow.Suggested change
-<nav class="studio-workflow" aria-label={ariaLabel}> - {`#each` steps as step, index (step)} - <span><b>{index + 1}</b>{step}</span> - {/each} -</nav> +<nav aria-label={ariaLabel}> + <ol class="studio-workflow"> + {`#each` steps as step, index (step)} + <li><b>{index + 1}</b>{step}</li> + {/each} + </ol> +</nav>🤖 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/editors/StudioWorkflow.svelte` around lines 5 - 8, The step sequence in StudioWorkflow.svelte is rendered as generic spans inside the nav, but it should use ordered-list semantics for accessibility. Update the markup around the steps loop in the StudioWorkflow component so the numbered workflow is represented with an ol containing li items instead of span elements, while preserving the existing aria-label and step rendering logic.configurator/src/components/DomainPanel.svelte (1)
89-90: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffDuplicated domain→studio source of truth.
redesignedDomains(Line 89) and the studio{#if}/{:else if}chain (Lines 190-206) independently enumerate the same eight domain ids. They're in sync now, but adding/removing a studio requires editing both places, and a mismatch would silently render neither the studio nor the friendly-controls fallback. Consider driving both from one map (id → component).♻️ Sketch
- const redesignedDomains = new Set(['colors', 'typography', 'spacing', 'layout', 'borders', 'shadows', 'motion', 'effects']); - const usesVisualStudio = $derived(redesignedDomains.has(domain.id)); + const STUDIO_BY_DOMAIN = { + typography: TypographyStudio, colors: ColorStudio, spacing: SpacingStudio, + layout: LayoutStudio, borders: ShapeStudio, shadows: ShadowStudio, + motion: MotionStudio, effects: EffectsStudio, + }; + const Studio = $derived(STUDIO_BY_DOMAIN[domain.id] ?? null); + const usesVisualStudio = $derived(Studio != null);Then render
{#ifStudio}<svelte:component this={Studio} />{/if}in place of the if/else chain.Also applies to: 190-206
🤖 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/DomainPanel.svelte` around lines 89 - 90, The DomainPanel.svelte logic duplicates the same domain-to-studio mapping in both redesignedDomains/usesVisualStudio and the Studio if/else chain, so consolidate them into one source of truth. Update the DomainPanel component to derive the active studio from a single map or lookup keyed by domain.id, and then render that resolved component directly instead of maintaining separate enumerations for the same eight ids. Ensure the friendly-controls fallback still handles unknown domains when no studio is found.configurator/src/components/editors/StudioFrame.svelte (1)
13-15: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
aria-labelon a generic<div>is not reliably exposed.
aria-labelon an element without a semantic/landmark role is ignored by many AT combinations. If these workflow steps are decorative, drop the label; if they convey state, use a list (<ol>/<li>) or add an appropriate role so the label is announced.🤖 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/editors/StudioFrame.svelte` around lines 13 - 15, The workflow container in StudioFrame’s studio__steps block uses aria-label on a generic div, which may not be exposed to assistive tech. Either remove the label if the preview/tune/verify text is purely decorative, or change the structure to a semantic list or add an appropriate role so the label is actually announced; keep the existing step content but adjust the wrapper markup in StudioFrame.svelte accordingly.
🤖 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/editors/EffectsStudio.svelte`:
- Line 58: The `.pending` shimmer animation in `EffectsStudio.svelte` ignores
reduced-motion preferences. Add a `prefers-reduced-motion: reduce` override in
the same component’s styles to disable or minimize the `shimmer` animation for
`.pending`, and make sure the `@keyframes shimmer` usage is only active when
motion is allowed.
In `@configurator/src/components/editors/MotionStudio.svelte`:
- Line 13: The MotionStudio description copy is mixing locales, so update the
StudioFrame description to use one consistent language only. Edit the
MotionStudio.svelte string on the StudioFrame component so the title/description
text stays fully in a single locale, keeping the surrounding copy style
consistent.
In `@configurator/src/components/editors/ShadowStudio.svelte`:
- Line 12: The StudioFrame description in ShadowStudio.svelte mixes Polish and
English, so update the description text to use a single locale consistently.
Adjust the Shadow Studio copy in the StudioFrame title/description block so the
full sentence is entirely in English (or entirely in Polish), keeping the
message aligned with the surrounding UI language.
In `@configurator/src/components/editors/ShapeStudio.svelte`:
- Line 13: The ShapeStudio description string is mixing English and Polish, so
update the StudioFrame copy to use one consistent locale only. Locate the
ShapeStudio.svelte component and replace the mixed-language description with
fully English text, or move it into the existing i18n/localization flow if that
is how other studio copy is handled, so the displayed UX text is consistent.
In `@configurator/src/components/editors/TypographyStudio.svelte`:
- Line 68: The TypographyStudio.svelte styles include an unused strong selector
that triggers the CI warning; update the component’s scoped CSS to remove strong
from the combined strong, code rule and keep only the code selector, since the
.specimen markup renders code but not strong.
In `@configurator/src/components/FriendlyControl.svelte`:
- Around line 29-34: The select in FriendlyControl.svelte can show the wrong
option when activeValue is not included in meta.options, causing the UI to
misrepresent the stored override/token value. Update the select rendering so the
current activeValue is preserved visibly when it’s out of range, such as by
adding a fallback option or normalizing the value before binding; use the
existing meta.control, meta.options, activeValue, and onSelect logic to keep the
displayed selection aligned with the store.
In `@configurator/src/components/Header.svelte`:
- Line 9: The header’s override count is using raw object key count, which can
include stale or unknown entries and drift from the canonical token count.
Update Header.svelte’s modCount derived value to count only token-aware valid
overrides from the same source used for sharing/status, so the header reflects
the canonical override state and does not enable sharing on stale keys.
In `@configurator/tests-e2e/a11y.spec.js`:
- Around line 66-67: The accessibility test is toggling the allvars details
panel unconditionally before locating the row, which can close an already-open
`details.allvars` and cause the row lookup to fail. Update `a11y.spec.js` so the
`page.locator('details.allvars summary').click()` step only opens the panel when
it is closed, or otherwise avoid re-clicking an already open `details.allvars`
before querying the `.allvars__body .row` for “Reading width”. Use the existing
`details.allvars` and row locator logic to keep the panel state stable before
asserting on the row info button.
---
Nitpick comments:
In `@configurator/src/components/DomainPanel.svelte`:
- Around line 89-90: The DomainPanel.svelte logic duplicates the same
domain-to-studio mapping in both redesignedDomains/usesVisualStudio and the
Studio if/else chain, so consolidate them into one source of truth. Update the
DomainPanel component to derive the active studio from a single map or lookup
keyed by domain.id, and then render that resolved component directly instead of
maintaining separate enumerations for the same eight ids. Ensure the
friendly-controls fallback still handles unknown domains when no studio is
found.
In `@configurator/src/components/editors/StudioFrame.svelte`:
- Around line 13-15: The workflow container in StudioFrame’s studio__steps block
uses aria-label on a generic div, which may not be exposed to assistive tech.
Either remove the label if the preview/tune/verify text is purely decorative, or
change the structure to a semantic list or add an appropriate role so the label
is actually announced; keep the existing step content but adjust the wrapper
markup in StudioFrame.svelte accordingly.
In `@configurator/src/components/editors/StudioWorkflow.svelte`:
- Around line 5-8: The step sequence in StudioWorkflow.svelte is rendered as
generic spans inside the nav, but it should use ordered-list semantics for
accessibility. Update the markup around the steps loop in the StudioWorkflow
component so the numbered workflow is represented with an ol containing li items
instead of span elements, while preserving the existing aria-label and step
rendering logic.
🪄 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: 20c96cf5-6365-4e5b-8751-1e379209c1e7
⛔ Files ignored due to path filters (2)
dist/badge-essential.jsonis excluded by!**/dist/**dist/badge-optimal.jsonis excluded by!**/dist/**
📒 Files selected for processing (25)
configurator/src/components/CategoryHeader.svelteconfigurator/src/components/ControlPreview.svelteconfigurator/src/components/ControlSection.svelteconfigurator/src/components/DomainPanel.svelteconfigurator/src/components/FriendlyControl.svelteconfigurator/src/components/Header.svelteconfigurator/src/components/HeadingEditor.svelteconfigurator/src/components/RadiusEditor.svelteconfigurator/src/components/SmartSettings.svelteconfigurator/src/components/editors/ColorStudio.svelteconfigurator/src/components/editors/EffectsStudio.svelteconfigurator/src/components/editors/LayoutStudio.svelteconfigurator/src/components/editors/MotionStudio.svelteconfigurator/src/components/editors/ShadowStudio.svelteconfigurator/src/components/editors/ShapeStudio.svelteconfigurator/src/components/editors/SpacingStudio.svelteconfigurator/src/components/editors/StudioControls.svelteconfigurator/src/components/editors/StudioFrame.svelteconfigurator/src/components/editors/StudioWorkflow.svelteconfigurator/src/components/editors/TypographyStudio.svelteconfigurator/src/lib/controlSchema.jsconfigurator/src/lib/studioSchema.jsconfigurator/tests-components/studios.test.jsconfigurator/tests-e2e/a11y.spec.jsconfigurator/tests/studio-schema.test.js
…_info-btn The info button (showRawInfo) is only rendered in SmartSettings gradient sections, not in allvars raw rows. Update the aria-expanded test to click into the Gradients panel where Raw gradient rows expose .row__info-btn. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A2b6qg3hbi9QsBZmaFwjUd
- ColorStudio: replace navigating <a href="/"> with inert <span role="link"> - EffectsStudio: add prefers-reduced-motion override for .pending shimmer - MotionStudio, ShadowStudio, ShapeStudio: fix mixed Polish/English descriptions - TypographyStudio: remove unused `strong` CSS selector (clears CI warning) - FriendlyControl: add fallback <option> when activeValue not in meta.options - Header: use modifiedCountsByDomain() instead of raw Object.keys count - StudioWorkflow: convert <span> items to <ol>/<li> for proper list semantics - StudioFrame: convert studio__steps div to <ol> so aria-label is reliably exposed - DomainPanel: consolidate redesignedDomains + if/else chain into STUDIO_BY_DOMAIN map Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
jackgranatowski
changed the title
5 tasks
This was referenced Jun 25, 2026
Merged
This was referenced Jun 26, 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
Supersedes #401 (closed without merge). Carries the full studio redesign forward with all CI failures resolved, CodeRabbit review threads addressed, and three additional production-readiness items completed.
What's included
Studio redesign (from #401)
CategoryHeader,ControlSection,ControlPreview,FriendlyControl,StudioFrame,StudioWorkflowcontrolSchema.js— heuristic token→control-type mapper (color, font, length, radius, shadow, motion, wrap, …)DomainPanelrefactored to embed studio views;Headercompacted and hardenedCI fixes & review comments (vs #401)
usesVisualStudioguard hiding ScaleGenerators and brand-color cards.panel__titleclass toCategoryHeader; reusedContainerBarsinLayoutStudiofor.cbars__barassertiona11y.spec.js— expand allvars before querying row info buttonprefers-reduced-motionoverrides inMotionStudioandEffectsStudioHeader.sveltememory leak (onDestroyfor share timer)controlSchema.js: fixed gradient direction select options; added font-weight guard before generic font ruleDomainPanel: consolidatedredesignedDomainsset + if/else chain into singleSTUDIO_BY_DOMAINmap +<svelte:component>StudioWorkflow/StudioFrame: converted<span>/<div>workflow steps to<ol>/<li>for proper list semanticsFriendlyControl: added fallback<option>whenactiveValuenot inmeta.optionsHeader: usemodifiedCountsByDomain()instead of rawObject.keys(overrides).lengthColorStudio: replaced navigating<a href="/">with inert<span role="link">TypographyStudio: removed unusedstrongCSS selector (cleared CI warning)Item 6 — Screenshot QA
tests-e2e/screenshots.spec.js: 9 domains × 3 viewports (1280 / 768 / 390 px) — 27 PNGs captured via--project=screenshotsscreenshotsproject toplaywright.config.jsItem 7 — Mobile ergonomics
app.css: WCAG 2.5.5 touch targets — 44 px min-height for.cfg-btn/.cfg-input/.cfg-selectat ≤980 pxStudioFrame: description hidden + h3 shrinks at ≤820 px (left column no longer towers above controls on mobile)StudioWorkflow+LayoutStudio: horizontal scroll fallback at ≤480 px for the 4-step workflow navControlSection: starts collapsed on mobile (window.innerWidth < 768) to avoid 4000+ px page wallsItem 8 — "What does this control do"
FriendlyControl: "drives N" badge viadependentsCount(); accent colour when ≥10 dependents (master token)ControlPreview: short caption below every preview (colour / typeface / corner / elevation / timing / …)SmartSettings: aggregate drives count in section summary; token count in reset button labelTest plan
npm run build— clean, no Svelte warningsnpm run test:unit— 525 unit tests passnpx playwright test --project=chromium— all e2e tests passnpx playwright test --project=screenshots— 27 PNGs intest-results/screenshots/--sf-space-scale) — should show "drives 45" badge in FriendlyControl