refactor(configurator): flatten IA to one list with two depths - #367
Conversation
Replace the Basic/Advanced complexity mode with a single, always-the-same navigation axis (category) and progressive disclosure inside each panel. Each category now leads with an inputs-first "Settings" surface (presets, brand colors, font stacks, scale generators, global "Scaling" multipliers) and a collapsed "All variables" disclosure exposes the full domain catalogue, so every token stays one click away. - Remove ui.mode and the header Basic/Advanced toggle; drop the mode guard, mode persistence and the b/a keyboard shortcuts (legacy persisted mode is ignored on restore). - Sidebar always lists Overview + the full taxonomy; depth lives in the panel, not the nav. - DomainPanel: two zones (Settings + All variables); scaling knobs move out of the fenced "Power knobs" details into Settings; search collapses the panel to the filtered catalogue. Domains with no curated Settings (e.g. Misc) render the catalogue inline. - Rename BASIC_DOMAIN_IDS -> OVERVIEW_DOMAIN_IDS and domain powerIntro -> scaleIntro to match the new model. - Update unit, component and e2e specs for the flat IA. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fg7U9rTiqBgbsPuz9kdWea
📝 WalkthroughWalkthroughRemoves the Basic/Advanced ( ChangesRemove Basic/Advanced mode; introduce Overview + All Variables
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 docstrings
🧪 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 |
The flat-IA panel now always renders the full catalogue (collapsed in the 'All variables' disclosure), so the primary brand-color token has two inputs with the same aria-label — the Settings brand row and the catalogue row. Scope the two affected e2e locators to .bcr__input to resolve the strict-mode violation. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fg7U9rTiqBgbsPuz9kdWea
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
configurator/src/lib/store.svelte.js (1)
109-112: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueJSDoc return type is incomplete.
The
sanitiseUiStatefunction can also returnuiThemeandbundlefields (see lines 44-50 inuiState.js), but the documented return type only mentionsdomainandoutputMode.📝 Suggested fix
- * `@returns` {{ domain?: string, outputMode?: string }} + * `@returns` {{ domain?: string, outputMode?: 'layer'|'root', uiTheme?: 'light'|'dark', bundle?: string }}🤖 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/lib/store.svelte.js` around lines 109 - 112, The JSDoc `@returns` type annotation for the `sanitiseUiState` function is incomplete. Update the `@returns` line to include all four fields that the function returns: `domain`, `outputMode`, `uiTheme`, and `bundle`, not just `domain` and `outputMode`. Ensure the type annotations match the actual return types for each field.configurator/src/lib/uiState.js (1)
21-22: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueJSDoc return type is incomplete.
The return type documents only
domainandoutputMode, but the function also returnsuiThemeandbundlefields (lines 44-50).📝 Suggested fix
- * `@returns` {{ domain?: string, outputMode?: 'layer'|'root' }} + * `@returns` {{ domain?: string, outputMode?: 'layer'|'root', uiTheme?: 'light'|'dark', bundle?: string }}🤖 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/lib/uiState.js` around lines 21 - 22, The JSDoc return type annotation for the function starting at line 21 is incomplete and only documents domain and outputMode properties. Update the `@returns` JSDoc type annotation to include all properties that are actually returned by the function, specifically adding uiTheme and bundle fields to the type definition alongside the existing domain and outputMode properties to match what the function actually returns on lines 44-50.
🤖 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/DomainPanel.svelte`:
- Around line 104-105: The showAll variable in DomainPanel maintains its open
state across domain navigation, breaking the expected collapsed-by-default
behavior. Reset the showAll variable to false whenever the active domain
changes. This should be done in a reactive context that detects when the domain
selection changes (around the code referenced in lines 279-286). Add logic to
set showAll back to its initial false state whenever a new domain becomes active
to ensure the disclosure state is reset for each domain.
In `@configurator/tests-e2e/a11y.spec.js`:
- Around line 81-82: The assertion on line 82 using page.locator('.row').first()
is too generic and can match rows outside the filtered catalogue, allowing the
test to pass even if catalogue rendering regresses. Instead of selecting the
first row anywhere on the page, scope the row assertion to specifically target
the first row within the filtered catalogue container. Use a more specific
selector that combines the catalogue context (such as a parent container that
wraps the filtered results) with the row selector to ensure you are validating
that rows are visible within the filtered catalogue results.
---
Nitpick comments:
In `@configurator/src/lib/store.svelte.js`:
- Around line 109-112: The JSDoc `@returns` type annotation for the
`sanitiseUiState` function is incomplete. Update the `@returns` line to include
all four fields that the function returns: `domain`, `outputMode`, `uiTheme`,
and `bundle`, not just `domain` and `outputMode`. Ensure the type annotations
match the actual return types for each field.
In `@configurator/src/lib/uiState.js`:
- Around line 21-22: The JSDoc return type annotation for the function starting
at line 21 is incomplete and only documents domain and outputMode properties.
Update the `@returns` JSDoc type annotation to include all properties that are
actually returned by the function, specifically adding uiTheme and bundle fields
to the type definition alongside the existing domain and outputMode properties
to match what the function actually returns on lines 44-50.
🪄 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: 51748001-f5f7-46d2-ad82-6c4b5917290a
📒 Files selected for processing (16)
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/store.svelte.jsconfigurator/src/lib/uiState.jsconfigurator/tests-e2e/a11y.spec.jsconfigurator/tests-e2e/axe.spec.jsconfigurator/tests-e2e/presets.spec.jsconfigurator/tests-e2e/shell.spec.jsconfigurator/tests-e2e/undo-redo.spec.jsconfigurator/tests/basics.test.jsconfigurator/tests/domains.test.jsconfigurator/tests/ui-state.test.js
There was a problem hiding this comment.
1 issue found and verified against the latest diff
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
…gue assertion - Expand the sanitiseUiState / loadUiState @returns JSDoc to include the uiTheme and bundle fields the validator actually returns (flagged by CodeRabbit and cubic). - Scope the a11y search assertion to catalogue rows (.group .row) so it can't pass on stray rows. The catalogue renders outside the collapsed disclosure during search, so .group (TokenGroup) is the correct selector. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fg7U9rTiqBgbsPuz9kdWea
Why
The configurator forced users to juggle five overlapping vocabularies: a global Basic/Advanced mode toggle, an Essentials subset, framework tier, token role (knob/consumption), and a fenced Power knobs bucket. ACSS v4 and Core Framework avoid all of this — one navigation axis (category), you set a few system inputs, the rest is derived, and depth is progressive disclosure inside each category. This PR adopts that model.
What changed
One list, two depths
ui.mode, persistence, theb/ashortcuts, and the mode-guard effect).Inputs-first panels — each category leads with a Settings zone (one-click presets, brand colors, font stacks, scale generators, and the global multipliers as a plain "Scaling" group — no more amber "Power" fence). The full catalogue moves into a collapsed "Show all variables (N)" disclosure with the tier/modified/usage filters in its header. Domains with no curated Settings (e.g. Misc) render the catalogue inline so they're never a dead end. An active search collapses the panel to the filtered catalogue ("find any token in this category").
Naming —
BASIC_DOMAIN_IDS→OVERVIEW_DOMAIN_IDS; domainpowerIntro→scaleIntro. Legacy persistedmodefields are ignored on restore.Tests
node --test): 378 pass — updateddomains.test.js,basics.test.js,ui-state.test.js.vitest): 6 pass.svelte-check: 0 errors. Build +check:curation+check:version: green.shell,a11y,presets,undo-redo,axe). Playwright browsers couldn't be downloaded in the build environment, so the e2e run is left to CI.Robustness
Removing modes hides nothing — the "All variables" disclosure guarantees every token is one click away in every category, and the
check-curationtripwire still ensures new framework knobs get a home.🤖 Generated with Claude Code
Generated by Claude Code
Summary by cubic
Flattened the configurator to a single category list with two depths: Settings first, and a collapsed All variables catalogue in every panel. Removed Basic/Advanced, moved scaling knobs into Settings, and made search filter the catalogue.
Refactors
ui.mode, the header toggle, mode guards, and B/A shortcuts; persistence no longer stores mode (legacy values are ignored).[and]cycle that list.BASIC_DOMAIN_IDS→OVERVIEW_DOMAIN_IDSand domainpowerIntro→scaleIntro..bcr__inputand the a11y search assertion to.group .row.Migration
ui.mode, mode-specific guards, and Basic/Advanced UI.BASIC_DOMAIN_IDSwithOVERVIEW_DOMAIN_IDS.powerIntrotoscaleIntro.Written for commit e53f3b3. Summary will update on new commits.
Summary by CodeRabbit
Release Notes
New Features
Changes