chore: migrate color system to CSS custom properties and tokens #43
Conversation
📝 WalkthroughWalkthroughThis pull request introduces a comprehensive design token migration and styling refactor across the frontend, establishing new CSS custom properties for UI surfaces, interactive states, and tooltips, then systematically replacing gray-based color utilities with these tokenized design colors throughout components and admin pages, alongside CSS layer reorganization and test updates. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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: 0
🧹 Nitpick comments (2)
frontend/src/components/Footer.svelte (1)
7-8: Footer spacing adjustments look good.The spacing changes reduce overall padding while adding more granular control with separate margin and padding on the footer element. The layered padding structure (outer
pt-8/pb-6+ innerpy-6) provides clear separation between the border and content.Optional: Consider consolidating padding
If the layered padding isn't required for specific design reasons, you could simplify by consolidating all spacing on a single element:
Option 1 - Keep padding on footer only:
-<footer class="relative z-30 mt-8 pt-8 pb-6 bg-bg-alt dark:bg-dark-bg-alt border-t border-border-default dark:border-dark-border-default"> - <div class="app-container py-6"> +<footer class="relative z-30 mt-8 py-8 bg-bg-alt dark:bg-dark-bg-alt border-t border-border-default dark:border-dark-border-default"> + <div class="app-container">Option 2 - Keep padding on inner container only:
-<footer class="relative z-30 mt-8 pt-8 pb-6 bg-bg-alt dark:bg-dark-bg-alt border-t border-border-default dark:border-dark-border-default"> - <div class="app-container py-6"> +<footer class="relative z-30 mt-8 bg-bg-alt dark:bg-dark-bg-alt border-t border-border-default dark:border-dark-border-default"> + <div class="app-container py-8">However, the current approach is valid if you need the flexibility of separate padding layers.
frontend/src/routes/Editor.svelte (1)
810-849: Consider retaining semantic<header>element for accessibility.The change from
<header>to<div>removes semantic meaning that assists screen readers in understanding the page structure. The<header>element was appropriate here as it contains the section title and related controls.If this change was intentional for styling reasons, consider adding
role="banner"or keeping it as<header>for better accessibility.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
frontend/src/App.sveltefrontend/src/app.cssfrontend/src/components/ErrorDisplay.sveltefrontend/src/components/Footer.sveltefrontend/src/components/NotificationCenter.sveltefrontend/src/components/Spinner.sveltefrontend/src/components/ToastContainer.sveltefrontend/src/components/__tests__/NotificationCenter.test.tsfrontend/src/components/__tests__/Spinner.test.tsfrontend/src/components/__tests__/ToastContainer.test.tsfrontend/src/routes/Editor.sveltefrontend/src/routes/Home.sveltefrontend/src/routes/Notifications.sveltefrontend/src/routes/admin/AdminEvents.sveltefrontend/src/routes/admin/AdminSagas.sveltefrontend/src/routes/admin/AdminUsers.sveltefrontend/src/styles/components.cssfrontend/src/styles/pages.css
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: E2E Tests
- GitHub Check: Build Frontend
🔇 Additional comments (38)
frontend/src/components/__tests__/Spinner.test.ts (1)
45-45: LGTM!The test expectation correctly updated to
text-fg-subtleto match the design token migration in the Spinner component. The test verifies the light-mode class, which is sufficient since the component applies both light and dark variants together.frontend/src/components/Spinner.svelte (1)
19-19: LGTM!The muted color correctly updated to use design tokens (
text-fg-subtle/text-dark-fg-subtle), aligning with the broader design system migration across the codebase.frontend/src/components/ErrorDisplay.svelte (1)
62-62: LGTM!The button hover states correctly updated to use the tokenized
interactive-hovervariants, ensuring consistent hover behavior across the application's design system.frontend/src/components/__tests__/NotificationCenter.test.ts (1)
214-214: LGTM!The test selector correctly updated to
.text-fg-mutedto match the design token migration in the Notifications component's low severity color mapping.frontend/src/routes/Notifications.svelte (1)
111-111: LGTM!The low severity color appropriately updated to use muted foreground tokens, which semantically aligns with low-priority notifications having a more subtle appearance compared to higher severities that use attention-grabbing colors.
frontend/src/components/__tests__/ToastContainer.test.ts (3)
23-23: LGTM!Container selector correctly updated from
.toasts-containerto.toast-containerto match the component's renamed class.
86-90: LGTM!Timer element selectors correctly updated from
.timerto.toast-timerto match the component's renamed class.
197-198: LGTM!Timer element selectors correctly updated for the multiple toasts test case.
frontend/src/components/ToastContainer.svelte (3)
67-67: LGTM!Base class renamed from
toasttotoast-item, providing clearer semantics and better namespacing to avoid potential CSS conflicts.
104-104: LGTM!Timer class renamed from
timertotoast-timer, improving specificity and maintaining consistent naming convention with the parent component.
115-152: LGTM!Container class renamed from
toasts-containertotoast-container, using the more conventional singular form that aligns with common CSS naming patterns.frontend/src/routes/Home.svelte (1)
25-28: LGTM! Clean structural simplification.The removal of the outer
overflow-x-hiddenwrapper and application ofoverflow-x-clipdirectly to the hero section is a good refactoring that reduces DOM nesting while maintaining the same visual effect.frontend/src/App.svelte (1)
93-102: LGTM! Layout simplification improves structure.Moving
ToastContainerto be a direct child of the layout alongsideHeaderandFootersimplifies the component hierarchy and aligns with the updated toast styling incomponents.css. The auth initialization spinner logic remains unchanged.frontend/src/routes/admin/AdminSagas.svelte (2)
35-41: LGTM! Consistent design token migration.The saga state color mappings have been updated to use neutral-based tokens, aligning with the broader design system migration. The
createdstate now usesbg-neutral-50 dark:bg-neutral-900/20which is consistent with the updated design tokens.
362-404: LGTM! Execution timeline styling updated correctly.The execution saga steps visualization has been updated to use neutral color tokens for:
- Background container (
bg-neutral-50 dark:bg-neutral-700/50)- Step connectors (
bg-neutral-300 dark:bg-neutral-600)- Incomplete step bubbles (
bg-neutral-100 dark:bg-neutral-800)This maintains visual consistency with the new design system.
frontend/src/routes/admin/AdminEvents.svelte (6)
221-222: LGTM! Default event type color updated correctly.The fallback color for unrecognized event types has been updated from gray to neutral tokens (
text-neutral-600 dark:text-neutral-400), maintaining consistency with the design system migration.
325-341: LGTM! Export dropdown uses new surface tokens.The export menu dropdown has been updated to use the new design tokens:
bg-surface-overlay dark:bg-dark-surface-overlayfor the containerborder-neutral-200 dark:border-neutral-700for bordershover:bg-interactive-hover dark:hover:bg-dark-interactive-hoverfor hover statesThis aligns with the tokenized design system.
686-693: LGTM! Tooltip styling migrated to dedicated tokens.The event type tooltip has been updated to use the dedicated
bg-tooltip-bgtoken instead of hard-coded colors. The tooltip arrow also uses the same token, ensuring visual consistency. This is a good example of the token-based theming approach.
720-738: LGTM! Action buttons use new interactive hover tokens.The action buttons (preview, replay, delete) have been updated to use
hover:bg-interactive-hover dark:hover:bg-dark-interactive-hoverfor their hover states, providing consistent interactive feedback across the UI.
1021-1053: LGTM! Replay preview modal updated to neutral tokens.The replay preview modal has been comprehensively updated to use neutral color tokens for:
- Helper text (
text-neutral-500 dark:text-neutral-400)- Event cards (
bg-neutral-50 dark:bg-neutral-800)- Event metadata (
text-neutral-500 dark:text-neutral-400)This maintains visual consistency throughout the modal.
1136-1139: LGTM! User overview stats updated to neutral tokens.The "Cancelled" stat card in the user overview has been updated from gray to neutral tokens (
bg-neutral-100 dark:bg-neutral-800,text-neutral-700 dark:text-neutral-300,text-neutral-900 dark:text-neutral-100), consistent with the design system migration.frontend/src/routes/admin/AdminUsers.svelte (1)
139-139: LGTM! API endpoint group color migrated to neutral tokens.The
apiendpoint group color has been updated from gray-based to neutral-based tokens, consistent with the design system migration. This affects the badge styling for API endpoint rate limits in the rate limit management modal.frontend/src/styles/components.css (7)
2-42: LGTM! Toast container restructured with proper CSS classes.The toast container styling has been moved from inline styles to dedicated CSS classes (
.toast-container,.toast-item,.toast-timer). This provides:
- Better separation of concerns
- Easier maintenance and customization
- Proper responsive design for mobile (
@media (max-width: 640px))- Consistent positioning and interaction behavior
The structure aligns with the updates in
ToastContainer.svelte.
66-72: LGTM! Form input updated for Tailwind v4 and new tokens.The bare input styling has been updated to:
- Use
focus:outline-hiddeninstead offocus:outline-none(Tailwind v4 compliance)- Apply new foreground tokens (
text-fg-default,placeholder-fg-muted)These changes align with Tailwind v4's breaking changes documented in the library documentation.
147-174: LGTM! Modal components migrated to new surface tokens.Modal styling has been comprehensively updated to use the new design tokens:
bg-surface-overlay dark:bg-dark-surface-overlayfor modal containerborder-border-default dark:border-dark-border-defaultfor borderstext-fg-default dark:text-dark-fg-defaultfor texttext-fg-muted dark:text-dark-fg-mutedfor muted elementsThis ensures consistent modal appearance across the application.
209-211: LGTM! Badge neutral uses skeleton token.The neutral badge now uses
bg-skeleton dark:bg-dark-skeletonwhich provides a consistent neutral background across the design system.
255-270: LGTM! Dropdown menu uses new surface and interactive tokens.The dropdown menu styling has been updated to:
- Use
bg-surface-overlayfor the container background- Use
hover:bg-interactive-hoverfor item hover states- Use new border tokens for consistency
This aligns with the design system's surface and interaction patterns.
273-277: LGTM! Tooltip uses dedicated tokens.The tooltip styling now uses dedicated
bg-tooltip-bgandtext-tooltip-fgtokens, providing a centralized way to manage tooltip appearance across the application. This is a good example of the token-based theming approach.
280-294: LGTM! Skeleton loading states use skeleton token.All skeleton loading classes now use the
bg-skeleton dark:bg-dark-skeletontoken, ensuring consistent loading state appearance throughout the application.frontend/src/components/NotificationCenter.svelte (3)
34-38: LGTM! Priority colors updated to use foreground tokens.The low priority color has been updated to use
text-fg-muted dark:text-dark-fg-muted, aligning with the design system's foreground token hierarchy. Other priorities continue to use semantic colors appropriately.
215-232: LGTM! Notification dropdown uses new surface tokens and explicit z-index.The notification center has been updated to:
- Use
z-40explicitly on the container for proper stacking context- Apply
bg-surface-overlay dark:bg-dark-surface-overlayto the dropdown- Use
border-border-default dark:border-dark-border-defaultfor borders- Set dropdown to
z-50for proper layering above the buttonThis ensures the notification dropdown renders correctly in the stacking order.
254-313: LGTM! Notification items updated with new color tokens.The notification items have been comprehensively updated to use:
text-fg-muted dark:text-dark-fg-mutedfor the empty state and notification bodytext-fg-subtle dark:text-dark-fg-subtlefor timestampshover:bg-interactive-hover dark:hover:bg-dark-interactive-hoverfor hover statesborder-border-default/50 dark:border-dark-border-defaultfor item bordersThis maintains visual consistency throughout the notification center.
frontend/src/styles/pages.css (3)
6-25: LGTM! CSS layering and responsive container improvements.The page styles have been wrapped in
@layer componentsfor proper cascade layering, and the editor grid container has been enhanced with:
max-width: 80remto constrain width on large screensmargin-left: auto; margin-right: auto;for centering- Base padding of
1remfor consistent spacingThese changes improve the editor's responsive behavior and visual consistency.
27-123: LGTM! Progressive responsive padding strategy.The editor container now has a well-designed responsive padding strategy:
- Base:
1rem(mobile)- 640px+:
1.5rem(tablet)- 768px+:
1.5rem(with layout changes)- 1024px+:
2rem(desktop)- 1280px+:
2rem 3rem(large desktop)This provides comfortable spacing across all viewport sizes while maintaining the existing grid layout structure.
169-296: LGTM! Home page animations properly layered.All home page styles including keyframe animations (
shape-float-1,shape-float-2,flyInHome), hero section pseudo-elements, and feature card styles have been properly nested within the@layer componentsblock. This ensures correct CSS cascade behavior while preserving all existing animations and visual effects.frontend/src/app.css (3)
452-459: Excellent token-based scrollbar styling.Replacing hardcoded scrollbar colors with design tokens improves maintainability and ensures consistent theming. The use of
var(--color-fg-subtle)for the thumb andvar(--color-skeleton)for the track is semantically appropriate, and the dark mode variants are properly scoped.
519-520: LGTM: Consistent tokenization for admin inputs.The migration from hardcoded colors to semantic tokens (
border-inputandsurface-overlay) improves maintainability and ensures the admin input styling remains consistent with the broader design system.
55-69: Remove unused token definitions or clarify their intended use.The CSS custom properties for surface-overlay, interactive-hover, and tooltip tokens are defined but have no usage in the codebase. Only the skeleton tokens are actively used (for scrollbar styling at lines 452 and 458). Either remove the unused tokens or add comments indicating they are reserved for future use. If they are intentionally introduced ahead of implementation, this should be documented.
Likely an incorrect or invalid review comment.
HardMax71
changed the title
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.