refactor(dashboard): adopt native Svelte class syntax and slim the icon bundle - #648
Conversation
…on bundle
Replace hand-built class strings (filter/join helpers, ternaries in
interpolations, || '' guards, string concatenation, heavy class:
directive stacks) with the clsx-style class={[...]} / class={{...}}
syntax built into Svelte 5.16+ (svelte.dev/docs/svelte/class). No
library needed. displayRowClass now returns a class array consumed
directly by the class attribute.
Icon.svelte now renders from a curated 56-icon registry instead of
lucide's full icons map (whose dynamic lookup defeated tree-shaking),
and uses a recursive svelte:element snippet instead of a hand-rolled
SVG serializer with @html. Minified bundle: 1,176 KB -> 800 KB.
Closes #646
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe dashboard replaces interpolated CSS class strings with array-based Svelte bindings, adds a curated Lucide icon registry, renders icon nodes recursively without raw HTML injection, and updates virtual-model class output tests. ChangesDashboard rendering changes
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/dashboard/src/pages/overview/ProviderStatusCardDetails.svelte (1)
6-9: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve the breaker-state CSS selectors through template class tokens.
providerBreakerStateClass(provider)produces one dynamic class, so there is no static.provider-status-health-state.is-healthy,.provider-status-health-state.is-degraded, or.provider-status-health-state.is-unhealthytoken in this template. Keep the scoped styles with fixed class tokens, or mark only the non-static state parts/global as needed, and change the header comment so it no longer says breaker-state palette classes are applied viaclass:directives.🤖 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 `@web/dashboard/src/pages/overview/ProviderStatusCardDetails.svelte` around lines 6 - 9, Update the breaker-state styling in ProviderStatusCardDetails so the healthy, degraded, and unhealthy selector tokens remain discoverable despite providerBreakerStateClass(provider) returning a dynamic class. Preserve the scoped CSS by adding fixed template class tokens or narrowly adjusting scope/global handling, and revise the header comment to accurately describe the implementation instead of claiming class: directives apply the palette classes.
🤖 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 `@web/dashboard/src/lib/components/atoms/icons.js`:
- Around line 86-89: Add the missing Inbox icon import and register it as the
"inbox" entry in iconRegistry alongside the existing icon mappings, so
EmptyState.svelte's default icon resolves correctly.
---
Outside diff comments:
In `@web/dashboard/src/pages/overview/ProviderStatusCardDetails.svelte`:
- Around line 6-9: Update the breaker-state styling in ProviderStatusCardDetails
so the healthy, degraded, and unhealthy selector tokens remain discoverable
despite providerBreakerStateClass(provider) returning a dynamic class. Preserve
the scoped CSS by adding fixed template class tokens or narrowly adjusting
scope/global handling, and revise the header comment to accurately describe the
implementation instead of claiming class: directives apply the palette classes.
🪄 Autofix
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: ASSERTIVE
Plan: Pro Plus
Run ID: 3f2aa89e-6e70-454c-906e-3b2235474cfa
⛔ Files ignored due to path filters (3)
internal/admin/dashboard/static/dist/assets/index-BTMyFhIF.jsis excluded by!**/dist/**internal/admin/dashboard/static/dist/assets/index-DwpOmyPo.jsis excluded by!**/dist/**internal/admin/dashboard/static/dist/index.htmlis excluded by!**/dist/**
📒 Files selected for processing (21)
web/dashboard/src/lib/components/atoms/CopyButton.svelteweb/dashboard/src/lib/components/atoms/DialogCloseButton.svelteweb/dashboard/src/lib/components/atoms/Icon.svelteweb/dashboard/src/lib/components/atoms/SegmentedControl.svelteweb/dashboard/src/lib/components/atoms/Spinner.svelteweb/dashboard/src/lib/components/atoms/TableActionButton.svelteweb/dashboard/src/lib/components/atoms/icons.jsweb/dashboard/src/lib/components/molecules/DatePickerCalendar.svelteweb/dashboard/src/lib/components/molecules/FilterInput.svelteweb/dashboard/src/lib/components/molecules/LoadingState.svelteweb/dashboard/src/lib/components/organisms/EditorDialog.svelteweb/dashboard/src/pages/audit-logs/AuditEntryMetadata.svelteweb/dashboard/src/pages/audit-logs/AuditPaneTabs.svelteweb/dashboard/src/pages/audit-logs/ChatMessage.svelteweb/dashboard/src/pages/mcp-servers/McpServerList.svelteweb/dashboard/src/pages/models/virtualModelsLogic.jsweb/dashboard/src/pages/overview/ContributionCalendar.svelteweb/dashboard/src/pages/overview/ProviderStatusCardDetails.svelteweb/dashboard/src/pages/settings/RuntimeRefresh.svelteweb/dashboard/src/pages/workflows/WorkflowChart.svelteweb/dashboard/tests/models-virtual-models.test.js
Confidence Score: 4/5Not ready to merge until the shared EmptyState default icon is restored. The dashboard's default empty-state rendering was exercised through compiled Svelte SSR and consistently produced an empty SVG, while a registered icon control produced drawable SVG elements. Files Needing Attention: web/dashboard/src/lib/components/atoms/EmptyState.svelte and web/dashboard/src/lib/components/atoms/icons.js need matching default-name and registry updates.
 T-Rex LogsWhat T-Rex did
Comments Outside Diff (2)
Reviews (1): Last reviewed commit: "refactor(dashboard): adopt native Svelte..." | Re-trigger Greptile |
Review follow-up: the curated registry missed "inbox" — used only as EmptyState's icon prop default, a source the usage sweep didn't cover — so default empty states rendered a blank SVG. Also rewrites a stale ProviderStatusCardDetails header comment that still described the class: directives replaced by the class array. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ngs (#649) * refactor(dashboard): pass lucide icons as values instead of name strings Follow-up to #648. Icon took a kebab-case name and resolved it through a curated registry, so an unregistered name rendered a blank SVG instead of failing — exactly the EmptyState "inbox" bug found in review there. Callers now import the icon and pass it (<Icon icon={Pencil} />), which deletes the registry: there is no list to keep in sync, and tree-shaking follows the imports structurally rather than from an enumerated map. Config-driven icons (sidebar nav, theme toggle, confirm dialogs, budget periods) hold the icon itself instead of a name. The bundler does not warn on missing named exports and jsconfig runs with checkJs:false, so a typo would still slip through silently. A guard test resolves every lucide import in the source tree against the package, failing with the offending file name. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(dashboard): convert the icon call sites the codemod missed Review follow-up. Three EditorDialog callers passed submitIcon as a name string, and budgetPeriodIcon() still returned names — both render a blank SVG now that Icon takes the icon itself. The codemod only rewrote <Icon> tags and icon: config literals, so neither shape was touched. Adds two guards: one asserts no icon binding is assigned a name string, one asserts budgetPeriodIcon returns a drawable icon. Both were confirmed to fail on the exact bugs above. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Summary
Closes #646.
Issue #646 asked to research a class-name library — the answer is that no library is needed: since Svelte 5.16 the
classattribute natively accepts objects and arrays with clsx semantics (docs), and the dashboard is on Svelte 5.35. This PR adopts that syntax everywhere hand-built class strings added noise, and folds in two Icon.svelte cleanups spotted along the way.Class syntax (17 components)
ChatMessage.svelte: thearticleClass()filter/join helper is gone; the<article>declaresclass={[base, msg.roleClass, { "is-anchor": …, "is-after-anchor": … }]}inline.{x || ''}guards, and string concatenation (WorkflowChart,AuditPaneTabs,AuditEntryMetadata,ContributionCalendar,McpServerList,EditorDialog,RuntimeRefresh) → array form; falsy values drop automatically.class:stacks → object form:DatePickerCalendar(6 directives → one object) andProviderStatusCardDetails(3 directives that comparedbreakerClassto literals just to re-apply it →class={["provider-status-health-state", breakerClass]}).classNameprop (Spinner,TableActionButton,CopyButton,DialogCloseButton,SegmentedControl,FilterInput,LoadingState) use array composition; no more trailing space when the prop is empty.displayRowClass()returns a class array consumed directly by the class attribute (tests updated).Deliberately untouched: single/double
class:directives (still doc-idiomatic),class="base {helper()}"badge sites where helpers return one token, and imperativedocument.body.classListcalls (body is outside any template).Icon bundle: −32%
Icon.svelteimported lucide's fulliconsmap and looked icons up dynamically, defeating tree-shaking — the dashboard uses 26 static + 30 dynamically-referenced icons out of ~1,600 shipped. A curated 56-entry registry (icons.js) now backs the lookup, and the hand-rolled SVG serializer +{@html}is replaced with a recursive<svelte:element>snippet. Minified bundle: 1,176 KB → 800 KB (gzip 234 KB); Vite's chunk-size warning is gone.One subtlety worth reviewer attention: snippet bodies don't inherit the SVG namespace from the surrounding
<svg>, so thesvelte:elementcarries an explicitxmlns(without it, paths compile as HTML elements and icons render blank — verified in compiled output).User-visible impact
None intended: identical rendered classes (minus cosmetic trailing spaces in class attributes), identical icons, smaller dashboard payload.
Testing
vite buildclean,svelte-check0 errors/0 warnings (315 files), 435/435 dashboard unit tests pass.🤖 Generated with Claude Code
Summary by CodeRabbit
UI Improvements
Tests