UI: Add Heading/Radius editors, ContainerBars, expanded Preview sections, shade strip and fold-state persistence - #398
Conversation
|
Warning Review limit reached
More reviews will be available in 24 minutes and 18 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 (5)
📝 WalkthroughWalkthroughAdds ChangesConfigurator domain editors and Preview Hub expansion
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 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 QodoUI: Add Heading/Radius editors, ContainerBars, shade strip, and fold-state persistence Description
Diagram
High-Level Assessment
Files changed (11)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
5 rules 1.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
configurator/tests-e2e/domain-preview.spec.js (1)
49-55: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid hard-coding the number of container bars.
Line 50 locks this test to exactly 5 bars, so token-list growth will fail the test even when proportional rendering still works.
♻️ Proposed refactor
const bars = page.locator('.cbars__bar'); - await expect(bars).toHaveCount(5); + const barCount = await bars.count(); + expect(barCount, 'ContainerBars should render at least one row').toBeGreaterThan(0); const widths = await bars.evaluateAll((els) => els.map((el) => el.getBoundingClientRect().width) ); - expect(new Set(widths.map((w) => Math.round(w))).size, 'container bars use distinct proportional widths').toBeGreaterThanOrEqual(3); + expect( + new Set(widths.map((w) => Math.round(w))).size, + 'container bars use distinct proportional widths' + ).toBeGreaterThanOrEqual(Math.min(3, barCount));🤖 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-e2e/domain-preview.spec.js` around lines 49 - 55, The e2e assertion in the domain preview test is hard-coded to expect exactly five container bars, which makes the test brittle as the token list grows. Update the check around the bars locator in domain-preview.spec.js to avoid asserting a fixed count; instead validate that the rendered bars exist and keep the proportional width behavior using the existing bars locator and width comparison logic. Keep the distinct-width assertion, but make the count expectation dynamic or relative so the test continues to pass when the number of container bars changes.configurator/src/components/RadiusEditor.svelte (1)
47-57: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSame tablist ARIA/keyboard gap as HeadingEditor.
The radius tab strip mirrors the HeadingEditor tablist: no
role="tabpanel"on.rad__body, noaria-controls/idlinkage, and no rovingtabindex/arrow-key navigation. The mini level-switcher buttons (Lines 99-109) are correctly labeled, so consider applying the same care to the primary tab strip.🤖 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/RadiusEditor.svelte` around lines 47 - 57, The primary radius tab strip in RadiusEditor has the same ARIA/keyboard accessibility gap as HeadingEditor. Update the tablist so each tab button is linked to its panel via matching id/aria-controls and the body uses role="tabpanel", and add roving tabindex plus left/right arrow-key navigation for the tab buttons. Keep the mini level-switcher buttons unchanged, and apply the fix in the RadiusEditor tab strip markup and its associated interaction handlers.configurator/src/components/HeadingEditor.svelte (1)
75-89: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTablist is missing ARIA wiring and keyboard semantics.
The
role="tablist"/role="tab"markup has no associatedrole="tabpanel"(the.hed__bodypanel), noaria-controls/idlinkage, and no rovingtabindex/ arrow-key navigation. Screen-reader and keyboard users get a degraded experience versus the WAI-ARIA tabs pattern. Tabs are still Tab-focusable since they're<button>s, so this is not a hard blocker, but given the PR's accessibility goals it's worth wiring up.🤖 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/HeadingEditor.svelte` around lines 75 - 89, The HeadingEditor tab UI is missing the full WAI-ARIA tabs pattern, so update the tab markup and state wiring around TABS, activeTab, and tabHasOverride to add proper tab/panel relationships. Give each tab an id and aria-controls, make the .hed__body act as the matching role="tabpanel" with a linked aria-labelledby, and add roving tabindex plus arrow-key navigation so only the active tab is in the tab order while still allowing keyboard switching.
🤖 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/HeadingEditor.svelte`:
- Around line 75-89: The HeadingEditor tab UI is missing the full WAI-ARIA tabs
pattern, so update the tab markup and state wiring around TABS, activeTab, and
tabHasOverride to add proper tab/panel relationships. Give each tab an id and
aria-controls, make the .hed__body act as the matching role="tabpanel" with a
linked aria-labelledby, and add roving tabindex plus arrow-key navigation so
only the active tab is in the tab order while still allowing keyboard switching.
In `@configurator/src/components/RadiusEditor.svelte`:
- Around line 47-57: The primary radius tab strip in RadiusEditor has the same
ARIA/keyboard accessibility gap as HeadingEditor. Update the tablist so each tab
button is linked to its panel via matching id/aria-controls and the body uses
role="tabpanel", and add roving tabindex plus left/right arrow-key navigation
for the tab buttons. Keep the mini level-switcher buttons unchanged, and apply
the fix in the RadiusEditor tab strip markup and its associated interaction
handlers.
In `@configurator/tests-e2e/domain-preview.spec.js`:
- Around line 49-55: The e2e assertion in the domain preview test is hard-coded
to expect exactly five container bars, which makes the test brittle as the token
list grows. Update the check around the bars locator in domain-preview.spec.js
to avoid asserting a fixed count; instead validate that the rendered bars exist
and keep the proportional width behavior using the existing bars locator and
width comparison logic. Keep the distinct-width assertion, but make the count
expectation dynamic or relative so the test continues to pass when the number of
container bars changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 24b6bf2d-3cd6-4321-9b9e-de5213ace982
⛔ 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 (10)
configurator/src/components/BrandColorRow.svelteconfigurator/src/components/ContainerBars.svelteconfigurator/src/components/DomainPanel.svelteconfigurator/src/components/HeadingEditor.svelteconfigurator/src/components/Preview.svelteconfigurator/src/components/RadiusEditor.svelteconfigurator/src/components/ScaleGenerator.svelteconfigurator/src/components/SmartSettings.svelteconfigurator/src/lib/foldState.jsconfigurator/tests-e2e/domain-preview.spec.js
…unt, shade-strip perf - HeadingEditor/RadiusEditor: add full WAI-ARIA tabs pattern (id, aria-controls, role="tabpanel", aria-labelledby, roving tabindex, arrow-key navigation) - domain-preview.spec.js: replace hard-coded toHaveCount(5) with dynamic count check so the test stays valid as the container token list grows - BrandColorRow: narrow shade-strip $effect to row-specific overrides only, avoiding redundant measureBackground reflows on unrelated token edits Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DfCd6Mb2ocTxPFXYjeKg4F
…fix failing e2e test The domain-sync effect navigates the preview to the 'colors' section when the Colors sidebar item is clicked, hiding the pv__btn--primary that previously only lived in Overview. Adding a compact buttons row to the Colors section restores the live-repaint test and improves the UX (color changes are now immediately visible on real components within the Colors section). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DfCd6Mb2ocTxPFXYjeKg4F
2 participants
Motivation
Description
HeadingEditor.svelte,RadiusEditor.svelte, andContainerBars.svelteto provide dedicated UIs for typography headings, radius specimens, and container-width comparison bars respectively.BrandColorRow.svelteto render a 7-step inline shade strip; it now importsmeasureBackground/setProbeContextand tracksui.previewThemeto measure effective shade colors.ScaleGenerator.svelteoptionally collapsible and persistent by introducingfoldState.jsand wiringgetFold/setFoldto store open/closed state; added a toggle UI and conditional rendering when collapsed.foldStateintoSmartSettings.svelteso section<details>state is persisted across reloads.DomainPanel.svelteflow to replace the old inline preview with the new editors and curated cards (and to place generators and quick knobs differently), and added imports for the new components.Preview.sveltewith new sections and aDOMAIN_TO_SECTIONmapping so the preview tab follows the active domain; added many UI blocks for gradients, spacing, borders, shadows, motion, and effects.src/lib/foldState.jsto store UI open/closed state inlocalStorage.dist/badge-essential.json,dist/badge-optimal.json) with new gzip size values.Testing
npm run build) which completed successfully.npm test) and the tests passed.Codex Task
Summary by CodeRabbit
New Features
Bug Fixes