Conversation
|
@shinokada is attempting to deploy a commit to the Bergside Team on Vercel. A member of the Team first needs to authorize it. |
|
Important Review skippedToo many files! 100 files out of 250 files are above the max files limit of 150. You can disable this status message by setting the 📝 WalkthroughWalkthroughThis PR introduces a comprehensive theme-selector component system supporting multiple selectable themes, refactors theme management into a centralized module with public API exports, implements accessibility-focused reduced-motion support across animation-heavy components, updates UI styling (border-radius and sizing tokens), removes legacy static theme CSS files, and adds extensive documentation and test coverage. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant ThemeUI as ThemeSelector UI
participant Store as themeStore
participant DOM as Document
participant CSS as CSS Link
User->>ThemeUI: Click theme selector
ThemeUI->>ThemeUI: Toggle dropdown visibility
ThemeUI->>Store: Iterate themeConfigs
Store-->>ThemeUI: Return themes with metadata
ThemeUI->>ThemeUI: Render theme list + color swatches
User->>ThemeUI: Select a theme
ThemeUI->>Store: loadTheme(themeId)
rect rgb(200, 220, 255)
Note over Store,CSS: Dynamic Theme Loading
Store->>Store: Lookup cssPath & fontUrl in themeConfigs
Store->>DOM: Remove old theme link (if exists)
DOM->>DOM: querySelectorAll & removeChild
Store->>CSS: Create new <link> for CSS
CSS-->>DOM: Inject into head
Store->>CSS: Create new <link> for Google Font
CSS-->>DOM: Inject into head
end
Store->>Store: localStorage.setItem(themeId)
Store->>Store: Update getCurrentTheme store
ThemeUI->>Store: getSelectedTheme()
Store-->>ThemeUI: Return FlowbiteTheme object
ThemeUI->>ThemeUI: Update button label & active styling
ThemeUI-->>User: Display selected theme
sequenceDiagram
participant Browser as Browser/User
participant Svelte as Svelte Component
participant Motion as prefersReducedMotion
participant Transition as Transition Handler
Browser->>Browser: Set OS reduced-motion preference
rect rgb(255, 230, 200)
Note over Svelte,Motion: Component Mount
Svelte->>Svelte: isBrowser = typeof window !== "undefined"
Svelte->>Motion: Subscribe to prefersReducedMotion.current
end
Browser->>Svelte: Trigger animation-triggering action (click/open)
rect rgb(230, 255, 230)
Note over Svelte,Transition: Effective Params Calculation
Svelte->>Svelte: Check isBrowser AND prefersReducedMotion.current
alt Reduced Motion Active
Svelte->>Svelte: effectiveParams = { duration: 0, ...params }
else Normal Mode
Svelte->>Svelte: effectiveParams = { duration: 100+, ...params }
end
end
Svelte->>Transition: Apply effectiveParams to transition directive
Transition->>Browser: Execute animation with computed duration
Browser-->>Svelte: Animation complete (instant or normal timing)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related issues
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
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: 1
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/lib/darkmode/theme.tssrc/lib/toolbar/theme.tssrc/routes/layouts/FlowbiteSvelteLayout.sveltesrc/routes/utils/ThemeSelector.sveltesrc/routes/utils/icons/Discord.svelte
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{svelte,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{svelte,ts,tsx,js,jsx}: Use the Flowbite-Svelte MCP server to discover components by name or category before implementing UI components
Always use findComponent tool FIRST to locate the correct Flowbite-Svelte component before fetching documentation
After calling findComponent, use getComponentDoc tool to fetch complete documentation including usage examples, props, and best practices for Flowbite-Svelte components
Files:
src/routes/utils/ThemeSelector.sveltesrc/lib/darkmode/theme.tssrc/routes/utils/icons/Discord.sveltesrc/lib/toolbar/theme.tssrc/routes/layouts/FlowbiteSvelteLayout.svelte
🧠 Learnings (4)
📚 Learning: 2025-12-16T13:21:19.575Z
Learnt from: shinokada
Repo: themesberg/flowbite-svelte PR: 1882
File: src/lib/bottom-navigation/BottomNavHeaderItem.svelte:9-9
Timestamp: 2025-12-16T13:21:19.575Z
Learning: When using ThemeProvider with reactive themes in Flowbite-Svelte, ensure that getTheme("componentName") calls are placed inside a $derived() expression (not at module scope) so components react to dynamic theme changes. This applies broadly to Svelte components in this repo that rely on theme-derived values. For example:
// bad: at module scope
const theme = getTheme("button");
// good: inside a derived store
import { derived } from 'svelte/store';
const themeDerived = derived(themeProviderStore, ($themeProvider) => getTheme("button"));
// use $themeDerived in template/component
Applied to files:
src/routes/utils/ThemeSelector.sveltesrc/routes/utils/icons/Discord.sveltesrc/routes/layouts/FlowbiteSvelteLayout.svelte
📚 Learning: 2025-12-28T05:16:44.735Z
Learnt from: shinokada
Repo: themesberg/flowbite-svelte PR: 0
File: :0-0
Timestamp: 2025-12-28T05:16:44.735Z
Learning: In flowbite-svelte with Svelte 5, props are not reactive by default. To make prop values reactive (so they update when the prop changes), they must be wrapped with `$derived()`. For example, `const styling = $derived(classes)` is necessary—not redundant—to ensure `styling` reacts to changes in the `classes` prop.
Applied to files:
src/routes/utils/ThemeSelector.svelte
📚 Learning: 2024-10-18T09:38:03.879Z
Learnt from: Chizaruu
Repo: themesberg/flowbite-svelte PR: 1465
File: src/lib/forms/Timepicker.svelte:105-116
Timestamp: 2024-10-18T09:38:03.879Z
Learning: In `Timepicker.svelte`, the `Dropdown` opens as intended and is used exclusively with the 'dropdown' and 'timerange-dropdown' Timepicker types.
Applied to files:
src/routes/utils/ThemeSelector.svelte
📚 Learning: 2025-11-25T07:26:46.504Z
Learnt from: CR
Repo: themesberg/flowbite-svelte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T07:26:46.504Z
Learning: Applies to **/*.{svelte,ts,tsx,js,jsx} : Use the Flowbite-Svelte MCP server to discover components by name or category before implementing UI components
Applied to files:
src/routes/layouts/FlowbiteSvelteLayout.svelte
🔇 Additional comments (4)
src/routes/utils/icons/Discord.svelte (1)
1-1: LGTM! Icon size reduction is consistent with the PR's styling theme.The Discord icon size has been appropriately reduced from
h-6 w-6toh-5 w-5, maintaining proper accessibility attributes and aligning with the broader UI refinements across toolbar components.src/routes/layouts/FlowbiteSvelteLayout.svelte (2)
75-89: Styling updates are consistent across toolbar components.The padding, border-radius, and spacing adjustments align well with the overall PR objective of refining the top navigation styling. The use of
p-1.5,rounded-sm, and reduced spacing (ms-1) creates a more compact and cohesive toolbar appearance.
75-81: Fix incorrect Discord sizing class and address invalidsize="md"string in GitHub icon.The GitHub icon component does support the
sizeprop, but passingsize="md"(a string) will not render correctly—SVG width/height attributes expect numeric values. The Discord icon does not support thesizeprop and uses hardcodedclass="h-6 w-6"(24×24px) rather thanh-5 w-5as mentioned in the code comment. Either use numeric sizing for GitHub (e.g.,size="20") or remove the size prop, and ensure Discord's hardcoded sizing is intentional.⛔ Skipped due to learnings
Learnt from: CR Repo: themesberg/flowbite-svelte PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-11-25T07:26:39.377Z Learning: Use searchDocs tool for full-text search across Flowbite-Svelte documentation when looking for specific information that might span multiple components or when users ask about features or patternssrc/lib/darkmode/theme.ts (1)
4-4: No action needed. Therounded-baseTailwind utility is properly defined through the Flowbite theme system. It maps to the--radius-baseCSS variable defined in the@themeblock ofsrc/routes/layout.css(default value: 12px), and is extended as a custom utility by the@plugin "flowbite/plugin"directive. The utility is actively used throughout the codebase in 30+ component theme files.
| <svg class="w-4.5 h-4.5" aria-hidden="true" xmlns="http://www.w3.org/2000/svg" width="24" height="24" fill="none" viewBox="0 0 24 24"> | ||
| <path stroke="currentColor" stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M12 7h.01m3.486 1.513h.01m-6.978 0h.01M6.99 12H7m9 4h2.706a1.957 1.957 0 0 0 1.883-1.325A9 9 0 1 0 3.043 12.89 9.1 9.1 0 0 0 8.2 20.1a8.62 8.62 0 0 0 3.769.9 2.013 2.013 0 0 0 2.03-2v-.857A2.036 2.036 0 0 1 16 16Z"></path> | ||
| </svg> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for custom size utilities in Tailwind config
find . -name "tailwind.config.*" -o -name "tailwind.config.*" | head -20Repository: themesberg/flowbite-svelte
Length of output: 52
🏁 Script executed:
#!/bin/bash
# Search for w-4.5 and h-4.5 in Tailwind config
rg "4\.5|spacing" --type-add 'config:*.{js,ts,cjs,mjs}' --type config -A 3 -B 3Repository: themesberg/flowbite-svelte
Length of output: 741
🏁 Script executed:
#!/bin/bash
# First, let's verify the file exists and read the relevant lines
if [ -f "src/routes/utils/ThemeSelector.svelte" ]; then
echo "=== File content around lines 31-33 ==="
sed -n '25,40p' "src/routes/utils/ThemeSelector.svelte"
else
echo "File not found"
fiRepository: themesberg/flowbite-svelte
Length of output: 1269
🏁 Script executed:
#!/bin/bash
# Search for all size utility definitions in the codebase
rg "w-4\.5|h-4\.5|spacing.*4\.5" -A 2 -B 2Repository: themesberg/flowbite-svelte
Length of output: 757
🏁 Script executed:
#!/bin/bash
# Find Tailwind config in common locations and parent directories
find . -maxdepth 3 -type f \( -name "tailwind.config.*" -o -name "postcss.config.*" \) 2>/dev/null | head -10Repository: themesberg/flowbite-svelte
Length of output: 52
🏁 Script executed:
#!/bin/bash
# Check for Tailwind config in root and src directories
ls -la | grep -i tailwind
ls -la src/ | grep -i tailwindRepository: themesberg/flowbite-svelte
Length of output: 52
🏁 Script executed:
#!/bin/bash
# Search for all occurrences of "4.5" in the codebase to see if it's defined elsewhere
rg "4\.5" -t ts -t js -t svelte --max-count 20Repository: themesberg/flowbite-svelte
Length of output: 100
🏁 Script executed:
#!/bin/bash
# Check the package.json for Tailwind version and any custom configuration
head -100 package.json | grep -A 20 -B 5 "tailwind\|extend"Repository: themesberg/flowbite-svelte
Length of output: 1712
🌐 Web query:
Tailwind CSS 3 default sizing utilities width height 4.5
💡 Result:
Short answer: Tailwind v3 does not include a built-in w-4.5 / h-4.5 utility by default. The default spacing scale includes 4 (1rem / 16px) and 5 (1.25rem / 20px) but not 4.5. [1]
If you want w-4.5 / h-4.5, add it to your theme spacing (4.5 = 4.5 * 0.25rem = 1.125rem = 18px). Example:
theme.extend.spacing: { '4.5': '1.125rem' }
That will generate w-4.5 and h-4.5. [1][2]
Sources:
[1] Tailwind CSS — Customizing spacing / Default spacing scale.
[2] Tailwind CSS — theme.extend spacing example.
🏁 Script executed:
#!/bin/bash
# Search for CSS files that might define w-4.5 and h-4.5
find . -type f \( -name "*.css" -o -name "*.scss" \) -exec grep -l "4\.5\|w-4\.5\|h-4\.5" {} \; 2>/dev/nullRepository: themesberg/flowbite-svelte
Length of output: 52
🏁 Script executed:
#!/bin/bash
# Check if Flowbite CSS files are in node_modules
find node_modules/flowbite* -name "*.css" 2>/dev/null | head -5Repository: themesberg/flowbite-svelte
Length of output: 52
🌐 Web query:
Tailwind CSS v4 default spacing scale sizing utilities width height
💡 Result:
Short answer — Tailwind v4 uses a single spacing scale (base unit = 0.25rem = 4px) that powers spacing and sizing utilities (padding, margin, gap, inset, translate, width/height/min/max, etc.). By default the numeric scale includes (name → value → px):
- 0 → 0rem → 0px
- px → 0.0625rem → 1px
- 0.5 → 0.125rem → 2px
- 1 → 0.25rem → 4px
- 1.5 → 0.375rem → 6px
- 2 → 0.5rem → 8px
- 2.5 → 0.625rem → 10px
- 3 → 0.75rem → 12px
- 3.5 → 0.875rem → 14px
- 4 → 1rem → 16px
- 5 → 1.25rem → 20px
- 6 → 1.5rem → 24px
- 7 → 1.75rem → 28px
- 8 → 2rem → 32px
- 9 → 2.25rem → 36px
- 10 → 2.5rem → 40px
- 11 → 2.75rem → 44px
- 12 → 3rem → 48px
- 14 → 3.5rem → 56px
- 16 → 4rem → 64px
- 20 → 5rem → 80px
- 24 → 6rem → 96px
- 28 → 7rem → 112px
- 32 → 8rem → 128px
- 36 → 9rem → 144px
- 40 → 10rem → 160px
- 44 → 11rem → 176px
- 48 → 12rem → 192px
- 52 → 13rem → 208px
- 56 → 14rem → 224px
- 60 → 15rem → 240px
- 64 → 16rem → 256px
- 72 → 18rem → 288px
- 80 → 20rem → 320px
- 96 → 24rem → 384px
Usage: classes like w-, h-, p-, m- map to those scale keys (e.g., h-16 = 4rem). In v4 the spacing system is more dynamic — a single --spacing token (0.25rem) lets every multiple be available (so numeric utilities beyond the fixed set can work) while keeping the option to lock an explicit scale in your theme.
🌐 Web query:
Flowbite Svelte w-4.5 h-4.5 sizing utilities custom theme
💡 Result:
Short answer — two easy options:
- Add a custom spacing token so Tailwind generates w-4.5 / h-4.5:
- tailwind.config.js
{
theme: {
extend: {
spacing: { '4.5': '1.125rem' } // 4.5 * 0.25rem = 1.125rem (18px)
}
}
} - Use class="w-4.5 h-4.5" in your Svelte/Flowbite markup and rebuild your app. (Flowbite components use Tailwind sizing classes.) [1][2]
- Use Tailwind arbitrary values (no config change):
- class="w-[1.125rem] h-[1.125rem]"
References:
- Tailwind spacing/customization docs. [1]
- Flowbite docs showing w-4.5 usage in components. [2]
- Tailwind guide on adding custom utilities / arbitrary values. [3]
🏁 Script executed:
#!/bin/bash
# Look for theme/CSS configuration in src/lib
find src/lib -type f \( -name "*.css" -o -name "*theme*" -o -name "*config*" \) 2>/dev/null | head -20Repository: themesberg/flowbite-svelte
Length of output: 611
🏁 Script executed:
#!/bin/bash
# Search for size/width/height utility definitions in src/lib
rg "w-4\.5|h-4\.5|spacing.*4\.5" src/lib 2>/dev/null | head -20Repository: themesberg/flowbite-svelte
Length of output: 52
🏁 Script executed:
#!/bin/bash
# Check the app.css or global CSS file
find . -maxdepth 2 -name "app.css" -o -name "global.css" -o -name "styles.css" 2>/dev/nullRepository: themesberg/flowbite-svelte
Length of output: 52
🏁 Script executed:
#!/bin/bash
# Look in src/ directory structure for CSS/config
ls -la src/ | head -20
cat src/app.css 2>/dev/null | head -50Repository: themesberg/flowbite-svelte
Length of output: 645
🏁 Script executed:
#!/bin/bash
# Find all CSS files in the repo root and src
find . -maxdepth 3 -name "*.css" -type f 2>/dev/null | grep -v node_modulesRepository: themesberg/flowbite-svelte
Length of output: 500
🏁 Script executed:
#!/bin/bash
# Check svelte.config.js for any CSS preprocessing or theme configuration
cat svelte.config.js 2>/dev/null | head -80Repository: themesberg/flowbite-svelte
Length of output: 1229
🏁 Script executed:
#!/bin/bash
# Check if there's a vite.config or similar that might define Tailwind utilities
cat vite.config.ts 2>/dev/null | head -100Repository: themesberg/flowbite-svelte
Length of output: 2275
🏁 Script executed:
#!/bin/bash
# Check the main layout.css file
cat src/routes/layout.cssRepository: themesberg/flowbite-svelte
Length of output: 22233
🏁 Script executed:
#!/bin/bash
# Search in theme CSS files for w-4.5 or h-4.5 definitions
grep -r "w-4\.5\|h-4\.5\|4\.5rem\|1\.125rem" static/themes/ 2>/dev/null | head -20Repository: themesberg/flowbite-svelte
Length of output: 297
🏁 Script executed:
#!/bin/bash
# Check if there are any @layer directives or custom utilities defined
rg "@layer|@apply" src/ --type css -A 2 -B 2 | head -50Repository: themesberg/flowbite-svelte
Length of output: 1535
🏁 Script executed:
#!/bin/bash
# Look for any Flowbite-Svelte Icon component that uses w-4.5 h-4.5
rg "w-4\.5|h-4\.5" --type ts --type tsx -B 5 -A 5 | head -50Repository: themesberg/flowbite-svelte
Length of output: 97
🏁 Script executed:
#!/bin/bash
# Look for Icon component in Flowbite-Svelte source
find src/lib -name "*icon*" -o -name "*Icon*" | head -20Repository: themesberg/flowbite-svelte
Length of output: 295
🏁 Script executed:
#!/bin/bash
# Check if there are any inline SVG examples using w-4.5
rg "class.*w-\d" src/ -A 2 -B 2 --max-count 30 | grep -E "(w-[0-9.]+|h-[0-9.]+)" | head -30Repository: themesberg/flowbite-svelte
Length of output: 2819
🌐 Web query:
Flowbite SVG icon sizing w-4 w-5 w-6 standard sizes
💡 Result:
Flowbite icons use Tailwind sizing classes. Standard mappings:
- w-4 / h-4 = 1rem = 16px. [1]
- w-5 / h-5 = 1.25rem = 20px. [1]
- w-6 / h-6 = 1.5rem = 24px. [1]
(Flowbite icon components accept those Tailwind classes or size presets that map to them.) [2]
Sources: Tailwind CSS sizing reference. [1] Flowbite icons/docs (size presets). [2]
Use standard Tailwind sizing utilities for the SVG.
The inline SVG uses w-4.5 h-4.5 classes, which are not part of Tailwind's default spacing scale. Tailwind v4 includes sizes 0-4 and 5-96 in its scale, but not 4.5. Replace with standard utilities like w-5 h-5 (20px) or use arbitrary values w-[1.125rem] h-[1.125rem] if 18px is specifically required.
🤖 Prompt for AI Agents
In src/routes/utils/ThemeSelector.svelte around lines 31 to 33, the SVG uses
non-standard Tailwind classes `w-4.5 h-4.5`; replace them with Tailwind-safe
sizing — either use a nearest standard size like `w-5 h-5` or, if 18px is
required, use arbitrary value classes `w-[1.125rem] h-[1.125rem]` so the project
stays compatible with Tailwind defaults.
There was a problem hiding this comment.
It is calculated by width: calc(var(--spacing) * 4.5);.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for the --spacing CSS variable definition
rg "--spacing" --type css -B 2 -A 2
# Also check if there's custom Tailwind spacing configuration
rg "spacing.*4\.5|w-4\.5" -B 3 -A 3
Length of output: 978
🏁 Script executed:
#!/bin/bash
# Search for the --spacing CSS variable definition (fixed pattern)
rg "\-\-spacing" --type css -B 2 -A 2
# Check for Tailwind config files
cat tailwind.config.ts 2>/dev/null || cat tailwind.config.js 2>/dev/null
# Look for where w-4.5 utilities might be defined
rg "calc\(var\(\-\-spacing\)" -B 2 -A 2
Length of output: 1106
🏁 Script executed:
#!/bin/bash
# Look for base --spacing variable (not -8xl suffix)
rg "^\s*--spacing:" --type css -B 2 -A 2
# Check theme CSS files more thoroughly
head -100 static/themes/default.css
# Look for Tailwind v4 config or CSS @theme
rg "@theme|@config" --type css -B 2 -A 5
Length of output: 6538
🏁 Script executed:
#!/bin/bash
# Check if there's a --spacing base variable in Tailwind v4 theme
rg "^\s*--spacing[^-]" --type css -B 2 -A 2
# Look for how Tailwind v4 generates sizing utilities
cat src/routes/layout.css | head -200
# Check if w-4.5 is explicitly defined anywhere
rg "\.w-4\.5|w-4\.5:" -B 2 -A 2
Length of output: 5586
@shinokada, thank you for the clarification! You're absolutely right. I apologize for the incorrect flag.
In Tailwind v4, the spacing system works dynamically with the --spacing CSS variable (which defaults to 0.25rem). The framework automatically generates utilities for fractional values like w-4.5 and h-4.5, which compile to:
width: calc(var(--spacing) * 4.5); /* = 1.125rem = 18px */
height: calc(var(--spacing) * 4.5);This is a valid and intentional part of Tailwind v4's new CSS-first configuration architecture. Unlike v3, which required explicit spacing scale configuration for non-standard values, v4 supports arbitrary numeric spacing values out of the box.
Thank you for pointing this out!
🧠 Learnings used
Learnt from: Chizaruu
Repo: themesberg/flowbite-svelte PR: 1465
File: src/lib/forms/Timepicker.svelte:105-116
Timestamp: 2024-10-18T09:38:03.879Z
Learning: In `Timepicker.svelte`, the `Dropdown` opens as intended and is used exclusively with the 'dropdown' and 'timerange-dropdown' Timepicker types.
Learnt from: shinokada
Repo: themesberg/flowbite-svelte PR: 1882
File: src/lib/bottom-navigation/BottomNavHeaderItem.svelte:9-9
Timestamp: 2025-12-16T13:21:28.743Z
Learning: In flowbite-svelte, when using ThemeProvider with reactive themes, `getTheme("componentName")` calls must be placed inside `$derived()` expressions (not at module scope) to ensure components react to dynamic theme changes. This pattern is intentional for reactive theming support.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (8)
src/tests/theme-selector/basic-theme-selector.test.svelte (1)
1-5: Test file lacks assertions and test logic.This test file only renders the ThemeSelector component without any assertions or test validation. Consider adding test cases with assertions to verify the component's behavior, or rename it to indicate it's a visual demo rather than a unit test.
Example: Add basic test assertions
<script lang="ts"> import { ThemeSelector } from "$lib"; + import { render, screen } from "@testing-library/svelte"; + import { describe, it, expect } from "vitest"; + + describe("ThemeSelector", () => { + it("renders theme selector button", () => { + render(ThemeSelector); + expect(screen.getByRole("button")).toBeInTheDocument(); + }); + }); </script> - -<ThemeSelector />src/tests/theme-selector/theme-selector-interaction.test.svelte (1)
1-23: Test component lacks test assertions.While this interaction test harness correctly uses reactive patterns (getCurrentTheme in $derived, per learnings), it's missing actual test assertions to verify the behavior. Consider adding test cases that exercise the onThemeChange callback and validate theme switching.
Example: Add interaction test assertions
import { render, screen, fireEvent } from "@testing-library/svelte"; import { describe, it, expect, vi } from "vitest"; import ThemeSelectorInteractionTest from "./theme-selector-interaction.test.svelte"; describe("ThemeSelector Interaction", () => { it("triggers onThemeChange when theme is selected", async () => { const onThemeChange = vi.fn(); render(ThemeSelectorInteractionTest, { props: { onThemeChange } }); // Simulate theme selection const button = screen.getByRole("button"); await fireEvent.click(button); // Verify callback was triggered expect(onThemeChange).toHaveBeenCalled(); }); });Based on learnings: The use of getCurrentTheme() inside $derived() (line 11) is correct for ensuring reactive theme changes.
src/tests/theme-selector/ThemeSelector.test.ts (2)
178-183: Fragile wait pattern for localStorage verification.The fixed 100ms
setTimeoutis a flaky test pattern. IfloadThemepersists synchronously (which it does based onthemeStore.svelte.ts), this wait is unnecessary. If async, usewaitForfrom testing-library instead.🔎 Suggested improvement
- // Wait for localStorage to be updated - await new Promise((resolve) => setTimeout(resolve, 100)); - // Check localStorage expect(localStorageMock.getItem("flowbite-svelte-theme")).toBe("playful");Or if async behavior is needed:
import { waitFor } from "@testing-library/svelte"; await waitFor(() => { expect(localStorageMock.getItem("flowbite-svelte-theme")).toBe("playful"); });
94-97: CSS class selectors couple tests to implementation details.Querying by escaped CSS class names like
.h-3\\.5is fragile—these will break if styling changes. Consider addingdata-testidattributes to color swatches for more resilient test selectors.src/lib/theme-selector/themes.ts (1)
11-47: Consider addingas const satisfiesfor stronger type inference.Adding
as const satisfies FlowbiteTheme[]would make the array readonly and provide better literal type inference for theme IDs while still validating against the interface.🔎 Suggested improvement
-export const themeConfigs: FlowbiteTheme[] = [ +export const themeConfigs = [ { id: "default", // ... }, // ... -]; +] as const satisfies readonly FlowbiteTheme[];src/lib/theme-selector/ThemeSelector.svelte (2)
47-49: Potential key collision if colors array contains duplicates.Using
(color)as the iteration key could cause issues if the same Tailwind class string appears multiple times in a theme's colors array. Consider using the index as part of the key.🔎 Suggested fix
- {#each theme.colors as color, index (color)} + {#each theme.colors as color, index (`${theme.id}-${index}`)}
26-48: Consider using thethemeSelectorTV function for styling.The component defines inline classes while
src/lib/theme-selector/theme.tsexports athemeSelectorTV function with matching slot definitions. Using the TV function would centralize styling and align with the tested variant configuration.src/tests/theme-selector/themeStore.test.ts (1)
5-20: Consider extracting shared localStorage mock to a test utility.The localStorage mock implementation is duplicated between this file and
ThemeSelector.test.ts. Extracting to a shared test utility would improve maintainability.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
src/lib/index.tssrc/lib/theme-selector/ThemeSelector.sveltesrc/lib/theme-selector/index.tssrc/lib/theme-selector/theme.tssrc/lib/theme-selector/themeStore.svelte.tssrc/lib/theme-selector/themes.tssrc/routes/+layout.sveltesrc/routes/layouts/FlowbiteSvelteLayout.sveltesrc/routes/theme-test/+page.sveltesrc/routes/theme-test/color/+page.sveltesrc/routes/theme-test/components/+page.sveltesrc/routes/theme-test/text-variables/+page.sveltesrc/tests/darkmode/darkmode.test.tssrc/tests/speed-dial/interactions.test.tssrc/tests/theme-selector/TEST_COVERAGE.mdsrc/tests/theme-selector/ThemeSelector.test.tssrc/tests/theme-selector/basic-theme-selector.test.sveltesrc/tests/theme-selector/theme-selector-interaction.test.sveltesrc/tests/theme-selector/theme.test.tssrc/tests/theme-selector/themeStore.test.ts
💤 Files with no reviewable changes (1)
- src/tests/speed-dial/interactions.test.ts
✅ Files skipped from review due to trivial changes (1)
- src/tests/theme-selector/TEST_COVERAGE.md
🚧 Files skipped from review as they are similar to previous changes (1)
- src/routes/layouts/FlowbiteSvelteLayout.svelte
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{svelte,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{svelte,ts,tsx,js,jsx}: Use the Flowbite-Svelte MCP server to discover components by name or category before implementing UI components
Always use findComponent tool FIRST to locate the correct Flowbite-Svelte component before fetching documentation
After calling findComponent, use getComponentDoc tool to fetch complete documentation including usage examples, props, and best practices for Flowbite-Svelte components
Files:
src/lib/index.tssrc/lib/theme-selector/index.tssrc/lib/theme-selector/theme.tssrc/tests/theme-selector/basic-theme-selector.test.sveltesrc/lib/theme-selector/themeStore.svelte.tssrc/tests/darkmode/darkmode.test.tssrc/routes/+layout.sveltesrc/tests/theme-selector/theme.test.tssrc/lib/theme-selector/themes.tssrc/tests/theme-selector/ThemeSelector.test.tssrc/tests/theme-selector/themeStore.test.tssrc/routes/theme-test/text-variables/+page.sveltesrc/tests/theme-selector/theme-selector-interaction.test.sveltesrc/routes/theme-test/+page.sveltesrc/routes/theme-test/color/+page.sveltesrc/routes/theme-test/components/+page.sveltesrc/lib/theme-selector/ThemeSelector.svelte
🧠 Learnings (5)
📚 Learning: 2025-12-16T13:21:28.743Z
Learnt from: shinokada
Repo: themesberg/flowbite-svelte PR: 1882
File: src/lib/bottom-navigation/BottomNavHeaderItem.svelte:9-9
Timestamp: 2025-12-16T13:21:28.743Z
Learning: In flowbite-svelte, when using ThemeProvider with reactive themes, `getTheme("componentName")` calls must be placed inside `$derived()` expressions (not at module scope) to ensure components react to dynamic theme changes. This pattern is intentional for reactive theming support.
Applied to files:
src/lib/index.tssrc/lib/theme-selector/index.tssrc/lib/theme-selector/themeStore.svelte.tssrc/lib/theme-selector/themes.tssrc/tests/theme-selector/ThemeSelector.test.tssrc/tests/theme-selector/themeStore.test.ts
📚 Learning: 2025-11-25T07:26:46.504Z
Learnt from: CR
Repo: themesberg/flowbite-svelte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T07:26:46.504Z
Learning: Applies to **/*.{svelte,ts,tsx,js,jsx} : Use the Flowbite-Svelte MCP server to discover components by name or category before implementing UI components
Applied to files:
src/lib/theme-selector/index.tssrc/lib/theme-selector/themeStore.svelte.tssrc/lib/theme-selector/themes.ts
📚 Learning: 2025-11-25T07:26:46.504Z
Learnt from: CR
Repo: themesberg/flowbite-svelte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T07:26:46.504Z
Learning: Applies to **/*.{svelte,ts,tsx,js,jsx} : Always use findComponent tool FIRST to locate the correct Flowbite-Svelte component before fetching documentation
Applied to files:
src/lib/theme-selector/index.ts
📚 Learning: 2025-12-16T13:21:19.575Z
Learnt from: shinokada
Repo: themesberg/flowbite-svelte PR: 1882
File: src/lib/bottom-navigation/BottomNavHeaderItem.svelte:9-9
Timestamp: 2025-12-16T13:21:19.575Z
Learning: When using ThemeProvider with reactive themes in Flowbite-Svelte, ensure that getTheme("componentName") calls are placed inside a $derived() expression (not at module scope) so components react to dynamic theme changes. This applies broadly to Svelte components in this repo that rely on theme-derived values. For example:
// bad: at module scope
const theme = getTheme("button");
// good: inside a derived store
import { derived } from 'svelte/store';
const themeDerived = derived(themeProviderStore, ($themeProvider) => getTheme("button"));
// use $themeDerived in template/component
Applied to files:
src/tests/theme-selector/basic-theme-selector.test.sveltesrc/routes/+layout.sveltesrc/routes/theme-test/text-variables/+page.sveltesrc/tests/theme-selector/theme-selector-interaction.test.sveltesrc/routes/theme-test/+page.sveltesrc/routes/theme-test/color/+page.sveltesrc/routes/theme-test/components/+page.sveltesrc/lib/theme-selector/ThemeSelector.svelte
📚 Learning: 2024-10-18T09:38:03.879Z
Learnt from: Chizaruu
Repo: themesberg/flowbite-svelte PR: 1465
File: src/lib/forms/Timepicker.svelte:105-116
Timestamp: 2024-10-18T09:38:03.879Z
Learning: In `Timepicker.svelte`, the `Dropdown` opens as intended and is used exclusively with the 'dropdown' and 'timerange-dropdown' Timepicker types.
Applied to files:
src/routes/theme-test/+page.sveltesrc/lib/theme-selector/ThemeSelector.svelte
🧬 Code graph analysis (5)
src/lib/theme-selector/theme.ts (1)
src/lib/theme/themeUtils.ts (1)
Classes(41-47)
src/tests/theme-selector/theme.test.ts (1)
src/lib/theme-selector/theme.ts (1)
themeSelector(7-40)
src/lib/theme-selector/themes.ts (2)
src/lib/theme-selector/index.ts (3)
ThemeId(3-3)FlowbiteTheme(3-3)themeConfigs(2-2)src/lib/theme-selector/themeStore.svelte.ts (3)
ThemeId(95-95)FlowbiteTheme(95-95)themeConfigs(94-94)
src/tests/theme-selector/ThemeSelector.test.ts (2)
src/lib/theme-selector/themeStore.svelte.ts (2)
loadTheme(23-83)themeConfigs(94-94)src/lib/theme-selector/themes.ts (1)
themeConfigs(11-47)
src/tests/theme-selector/themeStore.test.ts (1)
src/lib/theme-selector/themeStore.svelte.ts (4)
loadTheme(23-83)getCurrentTheme(86-88)getSelectedTheme(90-92)themeConfigs(94-94)
🔇 Additional comments (17)
src/tests/darkmode/darkmode.test.ts (2)
31-38: LGTM! Test expectation updated to match implementation changes.The class string correctly reflects the styling changes from
w-10 h-10withrounded-xltow-8 h-8withrounded-base, aligning with the darkmode button styling updates in the implementation.
127-139: The test correctly uses the same localStorage key as the implementation.The test uses the literal string
"THEME_PREFERENCE_KEY"on lines 134 and 138, which exactly matches the key used insrc/lib/darkmode/DarkMode.svelteon lines 23, 29, and 30. There is no mismatch between the test and implementation—both consistently use the same key value.Note: The commented-out constant on line 7 of DarkMode.svelte (
const THEME_PREFERENCE_KEY = 'color-theme') suggests this was originally intended to use'color-theme'as the key. Using a magic string instead of a constant is a code quality concern in the implementation, but the test correctly validates the actual behavior as currently implemented.Likely an incorrect or invalid review comment.
src/lib/index.ts (1)
8-8: LGTM - Clean barrel export addition.The theme-selector module is now properly exposed through the public API barrel, maintaining consistency with the existing export pattern.
src/routes/+layout.svelte (1)
6-6: LGTM - Import refactored to use public API.The change correctly updates the import to use the library barrel, aligning with the centralized theme utilities exposed through the public API.
src/routes/theme-test/text-variables/+page.svelte (1)
2-2: LGTM - Import refactored to use public barrel.Consistent with the PR's goal of exposing ThemeSelector through the library's public API surface.
src/lib/theme-selector/themeStore.svelte.ts (3)
3-3: LGTM - Theme configuration centralized.The refactor to import theme definitions from a dedicated module improves maintainability and separation of concerns.
21-21: Correct use of $derived for reactive theme selection.The selectedTheme correctly uses $derived to reactively compute the selected theme based on currentTheme changes, which aligns with Svelte 5's reactivity patterns.
94-95: Clean public API exports.Re-exporting themeConfigs and types creates a clear public interface for theme configuration consumers.
src/lib/theme-selector/theme.ts (1)
1-40: LGTM - Well-structured theme variants with Tailwind v4 syntax.The ThemeSelector variants are correctly defined using tailwind-variants. The Tailwind CSS v4 syntax is properly used, including:
- Opacity syntax with slash notation (e.g.,
bg-brand-900/20)- Logical property utilities (
rounded-s-xs,rounded-e-xs)- New utility scales like
rounded-xsThe slot-based approach provides good flexibility for styling customization.
src/routes/theme-test/+page.svelte (1)
31-31: LGTM - ThemeSelector imported from public barrel.Consistent with the PR-wide refactor to use the library's public API surface for ThemeSelector.
src/routes/theme-test/components/+page.svelte (1)
30-32: LGTM!The import path change aligns with the new public API surface exposed via the library barrel. ThemeSelector is now properly exported from
$libalongside other components, which provides a cleaner and more consistent import pattern for consumers.src/lib/theme-selector/index.ts (1)
1-4: Clean barrel export pattern for the theme-selector module.The public API surface is well-organized with proper type exports. The wildcard re-export from
./theme(line 4) will exposethemeSelectorandThemeSelectorVariants- verify this is the intended public surface.src/routes/theme-test/color/+page.svelte (1)
2-2: LGTM!Import path updated to use the library barrel export, consistent with the public API surface changes across the codebase.
src/tests/theme-selector/ThemeSelector.test.ts (1)
1-7: LGTM - Well-structured test imports and setup.The test file imports from appropriate sources. Note the mixed import pattern:
themeConfigsfrom the direct module path ($lib/theme-selector/themes) andloadThemefrom the barrel ($lib/theme-selector). Consider using consistent imports from the barrel for both if all needed exports are available there.src/lib/theme-selector/themes.ts (1)
1-9: LGTM - Clean type definitions for theme configuration.The
ThemeIdunion type andFlowbiteThemeinterface provide a well-structured contract for theme definitions. The interface captures all necessary properties for runtime theme switching.src/tests/theme-selector/theme.test.ts (1)
1-181: LGTM - Comprehensive test coverage for themeSelector variant configuration.The test suite thoroughly validates:
- All slot definitions and base classes
- Active variant behavior (true/false/default)
- SwatchPosition variant (first/middle/last)
- Combined variant compatibility
- Type safety guarantees
Well-structured tests that will catch regressions in the styling configuration.
src/lib/theme-selector/ThemeSelector.svelte (1)
7-8: Correct reactive theme pattern.Using
$derived()forgetCurrentTheme()andgetSelectedTheme()calls ensures the component reacts to dynamic theme changes, which aligns with the repository's reactive theming guidelines. Based on learnings, this is the correct approach.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/routes/docs/pages/compiler-speed.mdsrc/routes/docs/pages/theme-selector.md
💤 Files with no reviewable changes (1)
- src/routes/docs/pages/compiler-speed.md
🧰 Additional context used
🧠 Learnings (9)
📚 Learning: 2025-11-25T07:26:46.504Z
Learnt from: CR
Repo: themesberg/flowbite-svelte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T07:26:46.504Z
Learning: Applies to **/*.{svelte,ts,tsx,js,jsx} : After calling findComponent, use getComponentDoc tool to fetch complete documentation including usage examples, props, and best practices for Flowbite-Svelte components
Applied to files:
src/routes/docs/pages/theme-selector.md
📚 Learning: 2025-12-16T13:21:28.743Z
Learnt from: shinokada
Repo: themesberg/flowbite-svelte PR: 1882
File: src/lib/bottom-navigation/BottomNavHeaderItem.svelte:9-9
Timestamp: 2025-12-16T13:21:28.743Z
Learning: In flowbite-svelte, when using ThemeProvider with reactive themes, `getTheme("componentName")` calls must be placed inside `$derived()` expressions (not at module scope) to ensure components react to dynamic theme changes. This pattern is intentional for reactive theming support.
Applied to files:
src/routes/docs/pages/theme-selector.md
📚 Learning: 2025-11-25T07:26:46.504Z
Learnt from: CR
Repo: themesberg/flowbite-svelte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T07:26:46.504Z
Learning: Applies to **/*.{svelte,ts,tsx,js,jsx} : Always use findComponent tool FIRST to locate the correct Flowbite-Svelte component before fetching documentation
Applied to files:
src/routes/docs/pages/theme-selector.md
📚 Learning: 2025-11-25T07:26:39.377Z
Learnt from: CR
Repo: themesberg/flowbite-svelte PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T07:26:39.377Z
Learning: After calling findComponent, use getComponentDoc tool to fetch complete documentation including usage examples, props, and best practices for Flowbite-Svelte components
Applied to files:
src/routes/docs/pages/theme-selector.md
📚 Learning: 2025-11-25T07:26:39.377Z
Learnt from: CR
Repo: themesberg/flowbite-svelte PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T07:26:39.377Z
Learning: Use searchDocs tool for full-text search across Flowbite-Svelte documentation when looking for specific information that might span multiple components or when users ask about features or patterns
Applied to files:
src/routes/docs/pages/theme-selector.md
📚 Learning: 2025-11-25T07:26:46.504Z
Learnt from: CR
Repo: themesberg/flowbite-svelte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T07:26:46.504Z
Learning: Use searchDocs tool when finding specific information that might span multiple Flowbite-Svelte components or when exploring features and patterns across the documentation
Applied to files:
src/routes/docs/pages/theme-selector.md
📚 Learning: 2025-11-25T07:26:46.504Z
Learnt from: CR
Repo: themesberg/flowbite-svelte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T07:26:46.504Z
Learning: Applies to **/*.{svelte,ts,tsx,js,jsx} : Use the Flowbite-Svelte MCP server to discover components by name or category before implementing UI components
Applied to files:
src/routes/docs/pages/theme-selector.md
📚 Learning: 2025-11-25T07:26:46.504Z
Learnt from: CR
Repo: themesberg/flowbite-svelte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T07:26:46.504Z
Learning: Use getComponentList tool to discover available Flowbite-Svelte components or help users explore component options
Applied to files:
src/routes/docs/pages/theme-selector.md
📚 Learning: 2025-11-25T07:26:39.377Z
Learnt from: CR
Repo: themesberg/flowbite-svelte PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T07:26:39.377Z
Learning: Use getComponentList tool to discover available Flowbite-Svelte components with their categories or to help users explore component options
Applied to files:
src/routes/docs/pages/theme-selector.md
🪛 markdownlint-cli2 (0.18.1)
src/routes/docs/pages/theme-selector.md
48-48: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (3)
src/routes/docs/pages/theme-selector.md (3)
351-351: Verify Svelte version requirement for$staterune syntax.Line 351 uses the
$staterune, a Svelte 5 feature. Confirm this documentation is compatible with the project's minimum Svelte version.
397-399: Verify internal documentation links.The related components section references other documentation pages. Ensure these links (ThemeProvider at
/docs/pages/theme-providerand Customization at/docs/pages/customization) exist and are correctly routed.
1-403: Comprehensive and well-structured documentation.The documentation thoroughly covers the ThemeSelector component with clear sections on setup, API reference, patterns, and troubleshooting. Examples are well-formatted and usage patterns are clearly demonstrated. The accessibility and best practices sections add valuable guidance.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
src/tests/theme-selector/ThemeSelector.test.ts (4)
95-96: Consider using test IDs or more semantic queries instead of CSS class selectors.The test relies on the specific CSS class
.h-3\.5to find color swatches. This creates tight coupling to implementation details and may break if styling changes.🔎 Suggested refactoring approach
Consider adding data-testid attributes to color swatches in the component:
- <div class="h-3.5 w-[1.125rem] {color} ..."></div> + <div class="h-3.5 w-[1.125rem] {color} ..." data-testid="theme-color-swatch"></div>Then query using a more stable selector:
- const colorSwatches = document.querySelectorAll(".h-3\\.5"); + const colorSwatches = document.querySelectorAll("[data-testid='theme-color-swatch']");
107-111: Test relies on implementation-specific CSS classes for active theme detection.Using
.bg-brand-50and filtering by text content makes this test brittle. If the active theme styling changes, this test will break even though the functionality remains correct.🔎 More robust testing approach
Consider adding a data-testid or aria-current attribute to the active theme:
In ThemeSelector.svelte:
<DropdownItem onclick={handleThemeChange(theme.id)} + data-active={theme.id === currentTheme} + aria-current={theme.id === currentTheme ? "true" : undefined} class="...{theme.id === currentTheme ? 'bg-brand-50 dark:bg-brand-900/20' : ''}">Then test using semantic attributes:
- const dropdownButtons = screen.getAllByRole("button").filter((btn) => - btn.textContent?.includes("Default") && btn.classList.contains("bg-brand-50")); + const activeButton = screen.getByRole("button", { current: "true" }); + expect(activeButton).toHaveTextContent("Default");
125-130: Button selection strategy is fragile and may select incorrect elements.Filtering by
.text-leftclass and text content to find the correct button is implementation-dependent. This pattern is repeated throughout the interaction tests (lines 125, 153, 172, 266).🔎 More reliable button selection
Consider using more specific role queries or adding test IDs to theme option buttons:
<DropdownItem onclick={handleThemeChange(theme.id)} + data-testid={`theme-option-${theme.id}`} class="...">Then in tests:
- const allButtons = screen.getAllByRole("button"); - const minimalButton = allButtons.find((btn) => - btn.textContent?.includes("Minimal") && btn.classList.contains("text-left")); + const minimalButton = screen.getByTestId("theme-option-minimal");This pattern should be applied to all theme selection tests (lines 125, 153, 172, 266).
269-271: Test uses brittle querySelector for color swatches with compound classes.Lines 269, 282, and 293 all use compound CSS class selectors like
.h-3\.5.rounded-s-xswhich are tightly coupled to the component's internal styling implementation.Consider the same data-testid approach suggested earlier, or alternatively, test the visual outcome through more semantic queries (e.g., checking that each theme button contains the expected number of child elements representing colors).
src/lib/theme-selector/ThemeSelector.svelte (1)
31-39: Consider extracting the inline SVG icon into a reusable component.While the inline SVG works fine for this single use case, extracting it into a separate icon component would improve maintainability if this icon is used elsewhere or needs to be updated.
🔎 Example icon component extraction
Create a new component
ThemeIcon.svelte:<svg class="h-4.5 w-4.5" aria-hidden="true" xmlns="http://www.w3.org/2000/svg" width="24" height="24" fill="none" viewBox="0 0 24 24" > <path stroke="currentColor" stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M12 7h.01m3.486 1.513h.01m-6.978 0h.01M6.99 12H7m9 4h2.706a1.957 1.957 0 0 0 1.883-1.325A9 9 0 1 0 3.043 12.89 9.1 9.1 0 0 0 8.2 20.1a8.62 8.62 0 0 0 3.769.9 2.013 2.013 0 0 0 2.03-2v-.857A2.036 2.036 0 0 1 16 16Z" /> </svg>Then import and use:
+ import ThemeIcon from "./ThemeIcon.svelte"; ... - <svg class="h-4.5 w-4.5" ...></svg> + <ThemeIcon />src/tests/theme-selector/themeStore.test.ts (1)
64-68: Consider adding a comment to clarify the mock clearing strategy.The pattern of loading a theme (line 65) and then clearing mockLinks (line 68) might be confusing to future maintainers. While this gives tests a clean slate after initial setup, a brief comment would improve readability.
🔎 Suggested clarification
// Reset theme to default to prevent state leaking (after mocks are set up) loadTheme("default"); - // Clear mockLinks after initial theme load so tests start fresh + // Clear mockLinks after initial theme load so each test starts with a clean + // slate and doesn't count the default theme's link elements in assertions mockLinks.length = 0;
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/lib/theme-selector/ThemeSelector.sveltesrc/lib/theme-selector/themes.tssrc/routes/docs/pages/theme-selector.mdsrc/tests/theme-selector/ThemeSelector.test.tssrc/tests/theme-selector/themeStore.test.ts
✅ Files skipped from review due to trivial changes (1)
- src/routes/docs/pages/theme-selector.md
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/theme-selector/themes.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{svelte,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{svelte,ts,tsx,js,jsx}: Use the Flowbite-Svelte MCP server to discover components by name or category before implementing UI components
Always use findComponent tool FIRST to locate the correct Flowbite-Svelte component before fetching documentation
After calling findComponent, use getComponentDoc tool to fetch complete documentation including usage examples, props, and best practices for Flowbite-Svelte components
Files:
src/tests/theme-selector/themeStore.test.tssrc/tests/theme-selector/ThemeSelector.test.tssrc/lib/theme-selector/ThemeSelector.svelte
🧠 Learnings (4)
📚 Learning: 2024-11-22T08:37:27.996Z
Learnt from: mrh1997
Repo: themesberg/flowbite-svelte PR: 1442
File: src/lib/utils/Popper.svelte:144-145
Timestamp: 2024-11-22T08:37:27.996Z
Learning: In `src/lib/utils/Popper.svelte`, inconsistencies in event handling (e.g., using both `mouseenter`/`mouseleave` and `pointerenter`/`pointerleave`) may be intentional to minimize modifications and reduce the risk of new problems.
Applied to files:
src/tests/theme-selector/themeStore.test.ts
📚 Learning: 2025-12-16T13:21:28.743Z
Learnt from: shinokada
Repo: themesberg/flowbite-svelte PR: 1882
File: src/lib/bottom-navigation/BottomNavHeaderItem.svelte:9-9
Timestamp: 2025-12-16T13:21:28.743Z
Learning: In flowbite-svelte, when using ThemeProvider with reactive themes, `getTheme("componentName")` calls must be placed inside `$derived()` expressions (not at module scope) to ensure components react to dynamic theme changes. This pattern is intentional for reactive theming support.
Applied to files:
src/tests/theme-selector/ThemeSelector.test.ts
📚 Learning: 2025-12-16T13:21:19.575Z
Learnt from: shinokada
Repo: themesberg/flowbite-svelte PR: 1882
File: src/lib/bottom-navigation/BottomNavHeaderItem.svelte:9-9
Timestamp: 2025-12-16T13:21:19.575Z
Learning: When using ThemeProvider with reactive themes in Flowbite-Svelte, ensure that getTheme("componentName") calls are placed inside a $derived() expression (not at module scope) so components react to dynamic theme changes. This applies broadly to Svelte components in this repo that rely on theme-derived values. For example:
// bad: at module scope
const theme = getTheme("button");
// good: inside a derived store
import { derived } from 'svelte/store';
const themeDerived = derived(themeProviderStore, ($themeProvider) => getTheme("button"));
// use $themeDerived in template/component
Applied to files:
src/lib/theme-selector/ThemeSelector.svelte
📚 Learning: 2024-10-18T09:38:03.879Z
Learnt from: Chizaruu
Repo: themesberg/flowbite-svelte PR: 1465
File: src/lib/forms/Timepicker.svelte:105-116
Timestamp: 2024-10-18T09:38:03.879Z
Learning: In `Timepicker.svelte`, the `Dropdown` opens as intended and is used exclusively with the 'dropdown' and 'timerange-dropdown' Timepicker types.
Applied to files:
src/lib/theme-selector/ThemeSelector.svelte
🧬 Code graph analysis (2)
src/tests/theme-selector/themeStore.test.ts (3)
src/lib/theme-selector/index.ts (4)
loadTheme(2-2)getCurrentTheme(2-2)getSelectedTheme(2-2)themeConfigs(2-2)src/lib/theme-selector/themeStore.svelte.ts (4)
loadTheme(23-83)getCurrentTheme(86-88)getSelectedTheme(90-92)themeConfigs(94-94)src/lib/theme-selector/themes.ts (1)
themeConfigs(11-47)
src/tests/theme-selector/ThemeSelector.test.ts (3)
src/lib/theme-selector/index.ts (2)
loadTheme(2-2)themeConfigs(2-2)src/lib/theme-selector/themeStore.svelte.ts (2)
loadTheme(23-83)themeConfigs(94-94)src/lib/theme-selector/themes.ts (1)
themeConfigs(11-47)
🔇 Additional comments (7)
src/tests/theme-selector/ThemeSelector.test.ts (1)
1-297: Overall test quality is good with comprehensive coverage.The test suite covers rendering, interactions, configuration, accessibility, and visual elements. The structure is clear and well-organized. The main area for improvement is reducing coupling to CSS implementation details (as noted in previous comments).
src/lib/theme-selector/ThemeSelector.svelte (2)
43-43: Good use of explicit keys in Svelte each blocks.Using
(theme.id)and(${theme.id}-${index})as explicit keys ensures proper component identity tracking during updates. This prevents potential rendering issues when themes are reordered or updated.Also applies to: 47-47
2-2: themeConfigs is correctly exported from themeStore.svelte.ts.The export chain is properly configured:
themes.tsexportsthemeConfigs,themeStore.svelte.tsimports and re-exports it, andThemeSelector.sveltecan import it via"./themeStore.svelte"which resolves to the.svelte.tsfile due to the TypeScript configuration (allowImportingTsExtensions: true). No action needed.src/tests/theme-selector/themeStore.test.ts (4)
42-47: Mock implementation correctly delegates to original method for non-theme selectors.The selective mocking of
querySelectorAllensures that only theme-related queries are intercepted while other DOM queries work normally. This is a good pattern for avoiding over-mocking.
119-209: Comprehensive loadTheme test coverage.This test suite thoroughly validates:
- Theme loading success
- DOM element creation (CSS and font links)
- Correct path/URL assignment
- localStorage persistence
- Invalid theme handling
- Link removal before loading
- State updates
The tests are well-structured and cover both happy paths and edge cases.
275-321: Error handling tests properly validate resilience.The error handling suite covers:
- localStorage failures (graceful degradation)
- Font loading failures (warning logged)
- CSS loading failures (error logged)
The use of spies and proper restoration ensures these tests don't pollute the console output.
1-322: Excellent test coverage for theme store functionality.The test suite is comprehensive, well-organized, and follows testing best practices:
- Proper setup/teardown with beforeEach/afterEach
- Isolated tests with mocked dependencies
- Clear test descriptions
- Coverage of both success and failure scenarios
The complex mocking strategy is justified given the DOM manipulation requirements.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
src/lib/drawer/theme.tssrc/lib/theme-selector/themes.tssrc/lib/theme-selector/themes/default-runtime.csssrc/lib/theme-selector/themes/enterprise-runtime.csssrc/lib/theme-selector/themes/minimal-runtime.csssrc/lib/theme-selector/themes/mono-runtime.csssrc/lib/theme-selector/themes/playful-runtime.csssrc/lib/theme-selector/themes/themes-runtime.csssrc/routes/builder/drawer/+page.sveltesrc/routes/docs/pages/theme-selector.mdsrc/routes/utils/Newsletter.sveltestatic/themes/default.cssstatic/themes/enterprise.cssstatic/themes/minimal.cssstatic/themes/mono.cssstatic/themes/playful.cssstatic/themes/themes.css
💤 Files with no reviewable changes (6)
- static/themes/mono.css
- static/themes/enterprise.css
- static/themes/minimal.css
- static/themes/default.css
- static/themes/playful.css
- static/themes/themes.css
✅ Files skipped from review due to trivial changes (3)
- src/lib/drawer/theme.ts
- src/routes/docs/pages/theme-selector.md
- src/routes/utils/Newsletter.svelte
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/theme-selector/themes.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{svelte,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{svelte,ts,tsx,js,jsx}: Use the Flowbite-Svelte MCP server to discover components by name or category before implementing UI components
Always use findComponent tool FIRST to locate the correct Flowbite-Svelte component before fetching documentation
After calling findComponent, use getComponentDoc tool to fetch complete documentation including usage examples, props, and best practices for Flowbite-Svelte components
Files:
src/routes/builder/drawer/+page.svelte
🧠 Learnings (1)
📚 Learning: 2025-12-16T13:21:19.575Z
Learnt from: shinokada
Repo: themesberg/flowbite-svelte PR: 1882
File: src/lib/bottom-navigation/BottomNavHeaderItem.svelte:9-9
Timestamp: 2025-12-16T13:21:19.575Z
Learning: When using ThemeProvider with reactive themes in Flowbite-Svelte, ensure that getTheme("componentName") calls are placed inside a $derived() expression (not at module scope) so components react to dynamic theme changes. This applies broadly to Svelte components in this repo that rely on theme-derived values. For example:
// bad: at module scope
const theme = getTheme("button");
// good: inside a derived store
import { derived } from 'svelte/store';
const themeDerived = derived(themeProviderStore, ($themeProvider) => getTheme("button"));
// use $themeDerived in template/component
Applied to files:
src/routes/builder/drawer/+page.svelte
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (6)
src/tests/theme-selector/ThemeSelector.test.ts (2)
94-97: Consider using more resilient selectors instead of CSS class queries.Querying elements by CSS class names like
.h-3\\.5couples tests to implementation details. If styling classes change, these tests will break even though functionality remains correct. Consider usingdata-testidattributes or semantic queries for more maintainable tests.
107-111: Class-based assertions are fragile.Filtering buttons by
classList.contains("bg-brand-50")ties tests to specific Tailwind classes. If theme styling changes, tests break unnecessarily. Consider addingdata-activeoraria-selectedattributes to theme items for more semantic testing.src/lib/theme-selector/themeStore.svelte.ts (1)
24-31: Consider usingThemeIdtype for stricter type safety.The
themeIdparameter is typed asstring, but you have aThemeIdunion type available. UsingThemeIdwould provide compile-time validation and better autocomplete.🔎 Proposed fix
-export function loadTheme(themeId: string): void { +export function loadTheme(themeId: ThemeId): void {Note: If you need to accept arbitrary strings (e.g., from localStorage), you could keep
stringbut add runtime validation, or use a type guard.src/tests/theme-selector/themeStore.test.ts (2)
5-25: Consider extracting shared localStorage mock to a test utility.This localStorage mock is duplicated in
ThemeSelector.test.ts. Extracting it to a shared test utility (e.g.,src/tests/utils/localStorageMock.ts) would reduce duplication and simplify maintenance.
42-47: Type cast for mock NodeList is acceptable but slightly fragile.The cast
mockLinks as unknown as NodeListOf<Element>works but doesn't fully implement the NodeList interface. For more robust mocking, consider using a proper NodeList-like object withforEachandlengthproperties. However, this is acceptable for the current test scope.src/lib/utils/Popper.svelte (1)
82-82: Consider usingeffectiveParamsdirectly.The
paramsOptionsvariable simply assignseffectiveParamswithout any transformation. SinceeffectiveParamsis already reactive via$derived, you could use it directly on line 286 instead of creating an additional derived layer.However, if this indirection serves a specific purpose in Svelte 5's reactivity model, it can remain as-is.
🔎 Proposed refactor
- const paramsOptions = $derived(effectiveParams);Then update line 286:
- transition:transition={paramsOptions as ParamsType} + transition:transition={effectiveParams as ParamsType}
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
e2e/page.spec.tssrc/lib/accordion/Accordion.sveltesrc/lib/accordion/AccordionItem.sveltesrc/lib/alert/Alert.sveltesrc/lib/badge/Badge.sveltesrc/lib/banner/Banner.sveltesrc/lib/carousel/Slide.sveltesrc/lib/datepicker/Datepicker.sveltesrc/lib/dialog/Dialog.sveltesrc/lib/drawer/Drawer.sveltesrc/lib/modal/Modal.sveltesrc/lib/sidebar/Sidebar.sveltesrc/lib/sidebar/SidebarDropdownWrapper.sveltesrc/lib/theme-selector/ThemeIcon.sveltesrc/lib/theme-selector/ThemeSelector.sveltesrc/lib/theme-selector/themeStore.svelte.tssrc/lib/theme-selector/themes.tssrc/lib/toast/Toast.sveltesrc/lib/types.tssrc/lib/utils/Popper.sveltesrc/routes/builder/drawer/+page.sveltesrc/routes/docs/pages/theme-selector.mdsrc/routes/layout.csssrc/routes/utils/Newsletter.sveltesrc/tests/theme-selector/ThemeSelector.test.tssrc/tests/theme-selector/themeStore.test.ts
💤 Files with no reviewable changes (2)
- e2e/page.spec.ts
- src/routes/layout.css
✅ Files skipped from review due to trivial changes (1)
- src/routes/utils/Newsletter.svelte
🚧 Files skipped from review as they are similar to previous changes (1)
- src/routes/docs/pages/theme-selector.md
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{svelte,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{svelte,ts,tsx,js,jsx}: Use the Flowbite-Svelte MCP server to discover components by name or category before implementing UI components
Always use findComponent tool FIRST to locate the correct Flowbite-Svelte component before fetching documentation
After calling findComponent, use getComponentDoc tool to fetch complete documentation including usage examples, props, and best practices for Flowbite-Svelte components
Files:
src/lib/utils/Popper.sveltesrc/lib/theme-selector/ThemeIcon.sveltesrc/lib/dialog/Dialog.sveltesrc/lib/theme-selector/themeStore.svelte.tssrc/routes/builder/drawer/+page.sveltesrc/lib/types.tssrc/lib/banner/Banner.sveltesrc/tests/theme-selector/themeStore.test.tssrc/lib/carousel/Slide.sveltesrc/lib/datepicker/Datepicker.sveltesrc/lib/sidebar/SidebarDropdownWrapper.sveltesrc/lib/sidebar/Sidebar.sveltesrc/lib/accordion/AccordionItem.sveltesrc/lib/theme-selector/themes.tssrc/lib/accordion/Accordion.sveltesrc/lib/modal/Modal.sveltesrc/lib/theme-selector/ThemeSelector.sveltesrc/tests/theme-selector/ThemeSelector.test.tssrc/lib/badge/Badge.sveltesrc/lib/alert/Alert.sveltesrc/lib/toast/Toast.sveltesrc/lib/drawer/Drawer.svelte
🧠 Learnings (7)
📚 Learning: 2024-11-22T08:37:27.996Z
Learnt from: mrh1997
Repo: themesberg/flowbite-svelte PR: 1442
File: src/lib/utils/Popper.svelte:144-145
Timestamp: 2024-11-22T08:37:27.996Z
Learning: In `src/lib/utils/Popper.svelte`, inconsistencies in event handling (e.g., using both `mouseenter`/`mouseleave` and `pointerenter`/`pointerleave`) may be intentional to minimize modifications and reduce the risk of new problems.
Applied to files:
src/lib/utils/Popper.sveltesrc/lib/dialog/Dialog.sveltesrc/routes/builder/drawer/+page.sveltesrc/tests/theme-selector/themeStore.test.tssrc/lib/carousel/Slide.sveltesrc/lib/datepicker/Datepicker.sveltesrc/lib/sidebar/SidebarDropdownWrapper.sveltesrc/lib/sidebar/Sidebar.sveltesrc/lib/accordion/AccordionItem.sveltesrc/lib/modal/Modal.sveltesrc/lib/drawer/Drawer.svelte
📚 Learning: 2025-12-28T05:16:44.735Z
Learnt from: shinokada
Repo: themesberg/flowbite-svelte PR: 0
File: :0-0
Timestamp: 2025-12-28T05:16:44.735Z
Learning: In flowbite-svelte with Svelte 5, props are not reactive by default. To make prop values reactive (so they update when the prop changes), they must be wrapped with `$derived()`. For example, `const styling = $derived(classes)` is necessary—not redundant—to ensure `styling` reacts to changes in the `classes` prop.
Applied to files:
src/lib/utils/Popper.svelte
📚 Learning: 2025-12-16T13:21:19.575Z
Learnt from: shinokada
Repo: themesberg/flowbite-svelte PR: 1882
File: src/lib/bottom-navigation/BottomNavHeaderItem.svelte:9-9
Timestamp: 2025-12-16T13:21:19.575Z
Learning: When using ThemeProvider with reactive themes in Flowbite-Svelte, ensure that getTheme("componentName") calls are placed inside a $derived() expression (not at module scope) so components react to dynamic theme changes. This applies broadly to Svelte components in this repo that rely on theme-derived values. For example:
// bad: at module scope
const theme = getTheme("button");
// good: inside a derived store
import { derived } from 'svelte/store';
const themeDerived = derived(themeProviderStore, ($themeProvider) => getTheme("button"));
// use $themeDerived in template/component
Applied to files:
src/lib/utils/Popper.sveltesrc/lib/theme-selector/ThemeIcon.sveltesrc/lib/dialog/Dialog.sveltesrc/routes/builder/drawer/+page.sveltesrc/lib/banner/Banner.sveltesrc/lib/carousel/Slide.sveltesrc/lib/datepicker/Datepicker.sveltesrc/lib/sidebar/SidebarDropdownWrapper.sveltesrc/lib/sidebar/Sidebar.sveltesrc/lib/accordion/AccordionItem.sveltesrc/lib/accordion/Accordion.sveltesrc/lib/modal/Modal.sveltesrc/lib/theme-selector/ThemeSelector.sveltesrc/lib/badge/Badge.sveltesrc/lib/alert/Alert.sveltesrc/lib/toast/Toast.sveltesrc/lib/drawer/Drawer.svelte
📚 Learning: 2025-12-16T13:21:28.743Z
Learnt from: shinokada
Repo: themesberg/flowbite-svelte PR: 1882
File: src/lib/bottom-navigation/BottomNavHeaderItem.svelte:9-9
Timestamp: 2025-12-16T13:21:28.743Z
Learning: In flowbite-svelte, when using ThemeProvider with reactive themes, `getTheme("componentName")` calls must be placed inside `$derived()` expressions (not at module scope) to ensure components react to dynamic theme changes. This pattern is intentional for reactive theming support.
Applied to files:
src/lib/theme-selector/themeStore.svelte.tssrc/lib/theme-selector/themes.tssrc/tests/theme-selector/ThemeSelector.test.ts
📚 Learning: 2025-11-25T07:26:46.504Z
Learnt from: CR
Repo: themesberg/flowbite-svelte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T07:26:46.504Z
Learning: Applies to **/*.{svelte,ts,tsx,js,jsx} : Use the Flowbite-Svelte MCP server to discover components by name or category before implementing UI components
Applied to files:
src/lib/theme-selector/themeStore.svelte.tssrc/lib/theme-selector/themes.ts
📚 Learning: 2024-10-18T09:38:03.879Z
Learnt from: Chizaruu
Repo: themesberg/flowbite-svelte PR: 1465
File: src/lib/forms/Timepicker.svelte:105-116
Timestamp: 2024-10-18T09:38:03.879Z
Learning: In `Timepicker.svelte`, the `Dropdown` opens as intended and is used exclusively with the 'dropdown' and 'timerange-dropdown' Timepicker types.
Applied to files:
src/lib/datepicker/Datepicker.sveltesrc/lib/sidebar/SidebarDropdownWrapper.sveltesrc/lib/theme-selector/ThemeSelector.svelte
📚 Learning: 2024-10-15T22:41:47.429Z
Learnt from: Chizaruu
Repo: themesberg/flowbite-svelte PR: 1464
File: src/routes/component-data/Datepicker.json:0-0
Timestamp: 2024-10-15T22:41:47.429Z
Learning: The component should be named 'Datepicker' (with lowercase 'p') throughout the codebase to maintain consistency.
Applied to files:
src/lib/datepicker/Datepicker.svelte
🧬 Code graph analysis (2)
src/lib/theme-selector/themes.ts (2)
src/lib/theme-selector/themeStore.svelte.ts (3)
ThemeId(96-96)FlowbiteTheme(96-96)themeConfigs(95-95)src/lib/theme-selector/index.ts (3)
ThemeId(3-3)FlowbiteTheme(3-3)themeConfigs(2-2)
src/tests/theme-selector/ThemeSelector.test.ts (2)
src/lib/theme-selector/themeStore.svelte.ts (2)
loadTheme(24-84)themeConfigs(95-95)src/lib/theme-selector/themes.ts (1)
themeConfigs(18-54)
🔇 Additional comments (27)
src/lib/theme-selector/ThemeIcon.svelte (1)
1-9: LGTM!Clean implementation of a decorative SVG icon component. The use of
aria-hidden="true"correctly marks it as decorative, andstroke="currentColor"ensures proper color inheritance from parent context.src/tests/theme-selector/ThemeSelector.test.ts (2)
1-44: Good test infrastructure setup.The localStorage mock, DOM cleanup in beforeEach, and state reset via
loadTheme("default")provide proper test isolation. The structure follows testing best practices.
216-253: Good accessibility test coverage.Tests verify ARIA attributes (
aria-haspopup,aria-expanded,aria-label) and keyboard accessibility. This ensures the component meets accessibility requirements.src/lib/theme-selector/themes.ts (2)
1-16: Clean type definitions and Vite URL imports.Good use of Vite's
?urlquery for runtime CSS loading. TheThemeIdunion type andFlowbiteThemeinterface provide clear contracts for theme data.
18-54: Well-structured theme configuration.The
as const satisfies readonly FlowbiteTheme[]pattern preserves literal types while ensuring type safety. This allows TypeScript to infer exactThemeIdvalues from the array.src/lib/theme-selector/ThemeSelector.svelte (3)
8-9: Correct use of$derived()for reactive theme state.This follows the established pattern for ThemeProvider reactivity -
getCurrentTheme()andgetSelectedTheme()are wrapped in$derived()expressions to ensure components react to dynamic theme changes. Based on learnings.
24-34: Good accessibility implementation.The button includes proper ARIA attributes (
aria-haspopup,aria-expanded,aria-label) for screen reader support.
36-42: Proper keyed iteration for theme items and color swatches.Using
theme.idas the key for themes and composite keys for colors ensures efficient DOM updates during theme switching.src/lib/theme-selector/themeStore.svelte.ts (3)
55-66: Robust style recalculation on theme load.The
onloadhandler forces layout recalculation usingoffsetHeightreads and temporary transition reset. This ensures CSS custom properties are applied immediately.
74-79: Good defensive error handling for localStorage.The try/catch around localStorage operations prevents crashes in restricted environments (private browsing, storage quota exceeded).
95-96: Clean public API re-exports.Re-exporting
themeConfigs,FlowbiteTheme, andThemeIdfrom the store maintains a unified public API surface for the theme-selector module.src/tests/theme-selector/themeStore.test.ts (3)
291-305: Correctonerrorhandler invocation.The error handler is now correctly invoked with a single
Eventobject, matching the DOM API signature. This was previously flagged and has been fixed.
307-321: Correctonerrorhandler invocation for theme CSS.Same fix applied correctly here for the theme CSS link error handler.
76-210: Comprehensive loadTheme test coverage.Tests cover successful loading, link element creation, CSS/font URL validation, localStorage persistence, invalid theme handling, and state synchronization. Good coverage of the public API.
src/lib/banner/Banner.svelte (2)
51-51: Correct usage of effectiveParams in transition.The transition correctly uses
effectiveParamsto apply reduced-motion awareness. Once the spread operator order issue on line 38 is fixed, this will properly respect user preferences.
3-3: TheprefersReducedMotionAPI usage is correct. This is a MediaQuery object available insvelte/motion(since v5.7.0), and accessing its boolean value via the.currentproperty is the proper documented pattern.src/lib/modal/Modal.svelte (1)
36-40: LGTM - Correct spread operator order.The implementation correctly ensures
duration: 0takes precedence by placing it last in the spread. The nullish coalescing operator properly handles undefined transitionParams.src/lib/datepicker/Datepicker.svelte (1)
456-463: LGTM - Inline reduced-motion check is appropriate here.The inline ternary
typeof window !== "undefined" && prefersReducedMotion.current ? 0 : 100correctly applies reduced motion by directly setting the duration. This simpler pattern is suitable for this component's needs.src/lib/sidebar/Sidebar.svelte (1)
80-84: LGTM - Correct spread operator order.The implementation correctly places
duration: 0last in the spread, ensuring it always takes precedence when reduced motion is preferred. This properly implements the accessibility feature.src/lib/sidebar/SidebarDropdownWrapper.svelte (1)
6-6: TheprefersReducedMotion.currentusage is correct.Svelte 5's
prefersReducedMotionfromsvelte/motion(v5.7.0+) is a MediaQuery store accessed via the.currentproperty, as confirmed in your implementation at line 21. The official documentation uses this exact pattern:prefersReducedMotion.currentto check the reduced motion preference.src/lib/drawer/Drawer.svelte (1)
7-7: LGTM! Reduced-motion handling implemented correctly.The implementation properly respects user motion preferences with an SSR-safe browser guard and ensures
duration: 0takes precedence when reduced motion is preferred (by spreadingtransitionParamsbefore settingduration: 0).Also applies to: 49-53
src/routes/builder/drawer/+page.svelte (1)
101-101: Partial fix for past review comment.Adding
dismissable={false}to both the generated code and the live demo resolves the behavior mismatch flagged in the previous review. However,dismissableremains hardcoded rather than configurable in the builder UI (unlikeoutsideclose). Consider making it a toggleable option if users should control keyboard dismissal behavior.Also applies to: 133-133
src/lib/accordion/AccordionItem.svelte (1)
8-8: Good addition of reduced-motion support via context.The pattern of reading
respectReducedMotionfrom context and applyingeffectiveTransitionParamsis well-implemented and consistent with the broader PR changes. See separate comment about parameter order.Also applies to: 85-85
src/lib/accordion/Accordion.svelte (1)
10-10: LGTM! Public API for reduced motion properly added.The new
respectReducedMotionprop (defaulting totrue) is correctly integrated into the component's props, context, and documentation. The default value appropriately prioritizes accessibility.Also applies to: 22-24, 57-57
src/lib/types.ts (1)
205-205: LGTM! Type definitions correctly added.The
respectReducedMotionproperty is properly typed in bothAccordionPropsandAccordionContextType, maintaining type safety for the new accessibility feature.Also applies to: 349-349
src/lib/carousel/Slide.svelte (1)
7-7: Excellent accessibility improvement with SSR safety.The addition of reduced-motion support enhances accessibility for users with motion sensitivities. The browser detection ensures SSR compatibility.
Also applies to: 16-17
src/lib/utils/Popper.svelte (1)
76-77: LGTM!The SSR safety check is correctly implemented using the standard
typeof window !== "undefined"pattern.
1 participant
🔗 Related issue (optional)
Closes #
📑 Description
🔍 PR Type
🚦 PR Status
✅ Checklist
pnpm lint && pnpm check && pnpm test:e2eand all tests passmainbranch (not the published npm version)🧪 Screenshots / Demos (optional)
ℹ️ Additional Information
Summary by CodeRabbit
Release Notes
New Features
Enhancements
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.