feat: begin fixing dark mode in the webcomponents #972
Conversation
WalkthroughThis pull request introduces modifications to various components and configuration files, focusing on enhancing the installation process for the Unraid API, updating color themes, and refining the visual presentation across the application. Key changes include a forced linking option in the API installation script, removal of dark theme styles from CSS, and adjustments to multiple Vue components to reflect new color schemes. The overall structure of components remains intact while improving error handling and styling consistency. Changes
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
elibosley
force-pushed
the
feat/dark-mode-fixes
branch
from
92c3ab9 to
4ea4890
Compare
elibosley
had a problem deploying
to
production
GitHub Actions
Failure
elibosley
had a problem deploying
to
production
GitHub Actions
Failure
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🔭 Outside diff range comments (2)
web/components/UserProfile/ServerState.vue (1)
Color tokens need attention for dark mode compatibility
Based on the codebase analysis:
text-unraid-redis consistently used across multiple components for error states and warningstext-orange-darkis used for interactive elements like purchase buttons and hover states- The tailwind config shows these are static color tokens without dark mode variants
- ChangelogModal.vue already implements dark mode color switching (
dark:prose-a:text-orange), suggesting a pattern for dark mode handlingAction items:
- Update
text-unraid-redto use semantic error tokens with dark mode variants- Replace
text-orange-darkwith appropriate dark mode compatible interactive state tokens- Follow the pattern used in ChangelogModal.vue for dark mode color switching
🔗 Analysis chain
Line range hint
31-31: Verify color token usage across themesThe component uses specific color tokens (
text-unraid-red,text-orange-dark) that might need to be reviewed for dark mode compatibility.Let's check how these color tokens are used across the codebase:
Also applies to: 35-39
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check usage of specific color tokens and potential theme-aware alternatives # Search for similar color token usage rg -l "text-(unraid-red|orange-dark)" --type vue # Look for any theme-specific color definitions rg -l "theme.*colors?.*dark" --type css --type scss rg -l "theme.*colors?.*light" --type css --type scssLength of output: 245
Script:
#!/bin/bash # Let's try without file type restrictions since they weren't recognized # Search for color token usage rg "text-(unraid-red|orange-dark)" # Look for theme-related configurations rg "theme.*colors?" # Search for any tailwind config files that might define these colors fd "tailwind.config"Length of output: 6215
Script:
#!/bin/bash # Let's check the color definitions in the tailwind config cat web/tailwind.config.ts | rg -A 5 "colors:" # Check if there are any dark mode specific styles for these colors rg "dark:(text-unraid-red|text-orange)"Length of output: 227
web/components/UserProfile.ce.vue (1)
Based on the gathered information, I can now provide the final response:
Dropdown visibility needs adjustment in dark mode
The current implementation has potential issues:
--shadow-popover-foregroundis derived from the theme's--popover-foregroundvariable, which correctly switches between light (3.9% black) and dark (98% white) modes- However, the border color uses
--color-headerTextPrimarywhich appears inverted in the theme store:
- Dark mode:
#1c1c1c(very dark)- Light mode:
#f2f2f2(very light)This will make the dropdown arrow border nearly invisible in dark mode since it uses a dark color against a dark background.
Recommendations:
- Use
--color-betafor the border color instead, which maintains proper contrast in both modes- Or adjust the
defaultColors.dark.headerTextPrimaryvalue inweb/store/theme.tsto use a light color🔗 Analysis chain
Line range hint
147-158: Review dropdown visibility in dark modeThe changes to shadow and border colors need careful testing:
--shadow-popover-foregroundshould provide sufficient depth in both modes--color-headerTextPrimaryas a border color might need adjustment for dark mode🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check shadow and color variable definitions rg "(--shadow-popover-foreground|--color-headerTextPrimary)" "web/**/theme*"Length of output: 132
Script:
#!/bin/bash # Let's try searching for CSS variable definitions in all web files rg "(--shadow-popover-foreground|--color-headerTextPrimary)" "web/" # Also search for any theme-related files that might contain these variables fd "theme|style" "web/"Length of output: 335
Script:
#!/bin/bash # Let's check the theme store to understand how these variables are defined cat web/store/theme.ts # Also search for any CSS files that might define these variables fd -e css -e scss "web/" --exec cat {}Length of output: 4928
Script:
#!/bin/bash # Let's search for CSS files in the web directory correctly fd -e css -e scss . web/ # And then check their contents for shadow and color definitions fd -e css -e scss . web/ -x cat {}Length of output: 1413
🧹 Nitpick comments (13)
web/store/updateOs.ts (1)
72-72: Consider making the error handling more robust.While the current implementation is good, here are some suggestions to make it even better:
- throw new Error("[localCheckForUpdate] Error checking for updates\n" + JSON.stringify(error)); + const errorDetails = error instanceof Error + ? { message: error.message, ...error } + : error; + throw new Error( + `[localCheckForUpdate] Error checking for updates\n${ + JSON.stringify(errorDetails, Object.getOwnPropertyNames(errorDetails)) + }` + );This improvement:
- Handles Error objects properly by extracting their properties
- Uses
Object.getOwnPropertyNamesto handle non-enumerable properties- Maintains consistent formatting with template literals
Additionally, consider applying similar error handling in the
cancelUpdatemethod for consistency.web/components/UserProfile/DropdownTrigger.vue (1)
52-52: Remove trailing space in class attributeThere's an unnecessary trailing space in the class attribute.
- <span v-if="text" class="relative leading-none "> + <span v-if="text" class="relative leading-none">web/components/HeaderOsVersion.ce.vue (1)
81-83: Consider documenting the new theming systemThe move from abstract color names (
text-gamma) to semantic tokens (text-header-text-secondary) is a good practice for maintainable theming. Consider:
- Documenting the new color token system
- Creating a theme migration guide
- Adding theme token validation to prevent accidental use of deprecated colors
plugin/source/dynamix.unraid.net/etc/rc.d/rc.unraid-api (1)
39-41: Enhance error handling for npm link operationThe script checks for package.json but doesn't specifically handle npm link failures. Consider adding explicit error handling for the npm link operation.
- cd "${api_base_directory}" && npm link --force - # bail if expected file does not exist - [[ ! -f "${api_base_directory}/package.json" ]] && echo "unraid-api install failed" && exit 1 + cd "${api_base_directory}" || { echo "Failed to change directory"; exit 1; } + # Check if package.json exists before attempting npm link + [[ ! -f "${api_base_directory}/package.json" ]] && echo "package.json not found" && exit 1 + # Attempt npm link with error handling + if ! npm link --force; then + echo "npm link failed" >&2 + exit 1 + fiweb/components/Modal.vue (2)
101-101: Consider aligning focus ring with brand colorsWhile the hover and focus states are well implemented, the
ring-indigo-500seems inconsistent with the application's brand colors (which use unraid-red for interactions).- class="rounded-md text-foreground bg-transparent p-2 hover:text-white focus:text-white hover:bg-unraid-red focus:bg-unraid-red focus:outline-none focus:ring-2 focus:ring-indigo-500 focus:ring-offset-2" + class="rounded-md text-foreground bg-transparent p-2 hover:text-white focus:text-white hover:bg-unraid-red focus:bg-unraid-red focus:outline-none focus:ring-2 focus:ring-unraid-red focus:ring-offset-2"
140-140: LGTM! Consider documenting semantic token usageThe footer background implementation using
bg-popovereffectively complements the header'sbg-card, creating clear visual hierarchy. Consider documenting this pattern of semantic token usage (card vs popover) for future maintainers.Add a comment above the modal component explaining the semantic token usage pattern:
+<!-- + Modal uses semantic background tokens for visual hierarchy: + - bg-muted/bg-background: Main container + - bg-card: Header background + - bg-popover: Footer background +--> <template>web/pages/index.vue (3)
47-48: Consider making the key URL configurable.The hardcoded key URL might make maintenance difficult and could vary across environments.
Consider moving this to a configuration file or environment variable:
- keyUrl: - 'https://keys.lime-technology.com/unraid/7f7c2ddff1c38f21ed174f5c5d9f97b7b4577344/Starter.key', + keyUrl: import.meta.env.VITE_UNRAID_KEY_URL,
104-104: Fix typo in component name.There's a typo in the heading: "DowngraadeOsCe" should be "DowngradeOsCe".
- <h3 class="text-lg font-semibold font-mono">DowngraadeOsCe</h3> + <h3 class="text-lg font-semibold font-mono">DowngradeOsCe</h3>
130-143: Good practice: Legacy components showcase.The addition of legacy badge and button components sections provides a good reference for the ongoing theme redesign. This helps visualize all variants and ensures consistency during the migration.
Consider adding comments or documentation about the migration strategy from these legacy components to their new counterparts.
web/components/UserProfile.ce.vue (1)
Line range hint
100-158: Consider implementing a comprehensive dark mode strategyThe current changes move in the right direction with semantic naming, but consider:
- Implementing a CSS custom properties structure specifically for dark mode
- Using CSS color-mix() for automatic dark mode variants
- Adding automated contrast checking in the CI pipeline
Would you like help setting up a more systematic approach to dark mode theming?
web/components/ColorSwitcher.ce.vue (1)
56-58: Remove console.log statementRemove debugging console.log statement from production code.
- console.log(newVal);web/store/theme.ts (2)
40-83: Consider typing CSS custom propertiesThe light and dark variables could benefit from TypeScript types to catch potential errors and improve maintainability.
+type CSSVariables = { + [key: `--${string}`]: string; +}; -const lightVariables = { +const lightVariables: CSSVariables = { '--background': '0 0% 100%', // ... rest of the variables }; -const darkVariables = { +const darkVariables: CSSVariables = { '--background': '0 0% 3.9%', // ... rest of the variables };
107-126: Consider extracting color manipulation logicThe color manipulation logic in setCssVars could be extracted into a separate function for better maintainability and testability.
+const getThemeColors = (isDarkMode: boolean, userTheme?: Theme): ColorMode => { + const baseColors = isDarkMode ? defaultColors.dark : defaultColors.light; + + return { + headerTextPrimary: userTheme?.textColor ?? baseColors.headerTextPrimary, + headerTextSecondary: userTheme?.metaColor ?? baseColors.headerTextSecondary, + headerBackgroundColor: userTheme?.bgColor ?? baseColors.headerBackgroundColor, + }; +}; const setCssVars = () => { const body = document.body; - let { headerTextPrimary, headerTextSecondary, headerBackgroundColor } = darkMode.value - ? defaultColors.dark - : defaultColors.light; - if (theme.value?.textColor) { - headerTextPrimary = theme.value?.textColor; - } - // ... rest of the color setting logic + const colors = getThemeColors(darkMode.value, theme.value); + + body.style.setProperty('--header-text-primary', colors.headerTextPrimary); + body.style.setProperty('--header-text-secondary', colors.headerTextSecondary); + body.style.setProperty('--header-background-color', colors.headerBackgroundColor);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (30)
plugin/source/dynamix.unraid.net/etc/rc.d/rc.unraid-api(1 hunks)web/assets/main.css(0 hunks)web/components/Brand/Button.vue(1 hunks)web/components/ColorSwitcher.ce.vue(1 hunks)web/components/HeaderOsVersion.ce.vue(1 hunks)web/components/Modal.vue(3 hunks)web/components/Notifications/Indicator.vue(1 hunks)web/components/Notifications/Sidebar.vue(2 hunks)web/components/Ui/Badge.vue(0 hunks)web/components/Ui/CardWrapper.vue(1 hunks)web/components/Ui/Switch.vue(1 hunks)web/components/UpdateOs/CheckUpdateResponseModal.vue(1 hunks)web/components/UserProfile.ce.vue(4 hunks)web/components/UserProfile/Dropdown.vue(1 hunks)web/components/UserProfile/DropdownContent.vue(1 hunks)web/components/UserProfile/DropdownItem.vue(1 hunks)web/components/UserProfile/DropdownLaunchpad.vue(2 hunks)web/components/UserProfile/DropdownTrigger.vue(2 hunks)web/components/UserProfile/DropdownWrapper.vue(1 hunks)web/components/UserProfile/ServerState.vue(1 hunks)web/components/shadcn/sheet/index.ts(1 hunks)web/nuxt.config.ts(1 hunks)web/package.json(1 hunks)web/pages/index.vue(8 hunks)web/pages/webComponents.vue(1 hunks)web/store/replaceRenew.ts(2 hunks)web/store/theme.ts(3 hunks)web/store/updateOs.ts(1 hunks)web/tailwind.config.ts(3 hunks)web/types/ui/badge.ts(1 hunks)
💤 Files with no reviewable changes (2)
- web/components/Ui/Badge.vue
- web/assets/main.css
✅ Files skipped from review due to trivial changes (8)
- web/components/Ui/Switch.vue
- web/components/UserProfile/DropdownWrapper.vue
- web/pages/webComponents.vue
- web/components/UserProfile/Dropdown.vue
- web/components/UpdateOs/CheckUpdateResponseModal.vue
- web/nuxt.config.ts
- web/components/UserProfile/DropdownLaunchpad.vue
- web/components/UserProfile/DropdownContent.vue
🔇 Additional comments (28)
web/store/updateOs.ts (1)
72-72: LGTM! Enhanced error handling with detailed context.
The addition of JSON stringified error details will help with debugging by providing more context about what went wrong during update checks.
web/components/Ui/CardWrapper.vue (2)
26-26: LGTM! Good move to semantic color tokens.
The change from specific color variables to semantic tokens (text-foreground, bg-background, border-muted) is a positive step towards better theme support, making the component more adaptable to different color schemes including dark mode.
26-26: Verify the new color token definitions
Let's ensure these new semantic color tokens are properly defined in the theme system.
✅ Verification successful
Color tokens are properly defined and used consistently
The color tokens text-foreground, bg-background, and border-muted are correctly defined in the Tailwind configuration and are being used consistently across the application. The tokens are part of the ShadCN design system integration, where:
foregroundandbackgroundare defined as HSL variables in the color thememutedis defined with its variants in the color configuration- The base layer properly applies these tokens globally
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for the definition and usage of the new color tokens
# Expected: Find definitions of text-foreground, bg-background, and border-muted in theme/CSS files
# Search for token definitions in tailwind config or CSS files
echo "Searching for color token definitions..."
rg -t css -t js -t ts "(?:text-foreground|bg-background|border-muted)" web/
# Check if these tokens are used consistently across other components
echo "Checking usage consistency across components..."
rg -t vue "(?:text-foreground|bg-background|border-muted)" web/components/
Length of output: 4487
Script:
#!/bin/bash
# Let's check the Tailwind configuration for these color tokens
echo "Checking Tailwind config files..."
fd -e js -e ts "tailwind.config" --exec cat {}
# Also check for any CSS files that might define these tokens
echo "\nChecking CSS files for token definitions..."
fd -e css . --exec cat {}
Length of output: 10763
web/components/UserProfile/DropdownItem.vue (1)
30-30: Verify color contrast ratios with the new text-foreground class
The change from text-beta to text-foreground aligns with the dark mode improvements. However, ensure that the new semantic color maintains sufficient contrast ratios in both light and dark modes for accessibility compliance.
Let's verify the consistent usage of these color classes across the codebase:
✅ Verification successful
Change from text-beta to text-foreground is consistent with design system
The verification shows that:
- No instances of
text-betaremain in the codebase text-foregroundis consistently used across multiple components as part of a cohesive design system- The color is properly defined in the Tailwind configuration using CSS custom properties (HSL values) that support both light and dark modes
- The typography configuration ensures proper contrast by using the same semantic tokens
The change aligns with the dark mode improvements and follows the established pattern across the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining instances of text-beta and verify text-foreground usage
# Check for any remaining text-beta instances that might need updating
echo "Checking for remaining text-beta instances:"
rg "text-beta" --type vue
# Check text-foreground usage in related components
echo -e "\nChecking text-foreground usage in related components:"
rg "text-foreground" --type vue
# Look for any potential color-related CSS custom properties
echo -e "\nChecking for color-related CSS custom properties:"
rg "(?:--foreground|--beta):" --type css
Length of output: 491
Script:
#!/bin/bash
# Let's try without file type restrictions since Vue files might not be recognized
echo "Checking for remaining text-beta instances:"
rg "text-beta"
echo -e "\nChecking text-foreground usage in related components:"
rg "text-foreground"
echo -e "\nChecking for color-related CSS custom properties in all files:"
rg "(?:--foreground|--beta):"
# Also check for any tailwind config files that might define these colors
echo -e "\nChecking tailwind configuration:"
fd "tailwind.config" --exec cat {}
Length of output: 12037
web/components/UserProfile/DropdownTrigger.vue (1)
43-43: Verify dark mode behavior for text-header-text-primary
The new class text-header-text-primary needs verification for proper dark mode adaptation. Ensure that this color token provides sufficient contrast in both light and dark themes.
Let's check the color token definition and usage:
✅ Verification successful
Dark mode implementation for text-header-text-primary is correctly configured
The color token text-header-text-primary is properly implemented for dark mode support:
- It's defined in the theme store (
web/store/theme.ts) with appropriate values for both dark and light modes:- Dark mode:
#1c1c1c - Light mode:
#f2f2f2
- Dark mode:
- The color is dynamically set via CSS variable
--header-text-primary - The theme store handles the mode switching and applies the correct color values
- The implementation follows the project's dark mode architecture using CSS variables
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for text-header-text-primary color token definition
rg -l "text-header-text-primary" --type css --type html --type vue
# Search for any theme-related configuration files
fd -e js -e ts -e json "theme|tailwind|color"
Length of output: 214
Script:
#!/bin/bash
# Let's check the theme and tailwind configuration files for color token definitions
cat web/store/theme.ts web/tailwind-shadcn.config.ts web/tailwind.config.ts
# Let's also search for any usage of header-text-primary in these files
rg "header-text-primary" web/store/theme.ts web/tailwind-shadcn.config.ts web/tailwind.config.ts
# Since .vue files weren't recognized, let's try a direct grep
rg "text-header-text-primary" --type-add 'vue:*.vue' --type vue
Length of output: 17201
web/components/HeaderOsVersion.ce.vue (1)
81-83: Verify the new color token implementation
The change from text-gamma to text-header-text-secondary aligns with the dark mode fixes. However, we should ensure this new color token is properly implemented.
✅ Verification successful
Let me verify the hover/focus color implementation for dark mode compatibility.
Color tokens are properly implemented for theme compatibility
The verification shows that:
text-header-text-secondaryis properly defined in the Tailwind config and dynamically set via CSS variables in the theme store, allowing for both light/dark mode supportorange-darkis a static color (#f15a2c) consistently used across components for hover/focus states, which is an intentional design choice for interactive elements regardless of theme
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the new color token is defined in CSS/Tailwind config
rg -A 5 "header-text-secondary"
Length of output: 2505
Script:
#!/bin/bash
# Check the definition and usage of orange-dark color token
rg "orange-dark" -A 3
Length of output: 2107
plugin/source/dynamix.unraid.net/etc/rc.d/rc.unraid-api (1)
39-39: Verify the necessity of --force flag in npm link
The addition of --force to npm link could potentially overwrite existing valid symlinks. While this might resolve installation issues, it could mask underlying problems.
Let's verify if there are any reported symlink issues:
✅ Verification successful
The --force flag is appropriate in this context
The use of --force with npm link is justified based on the following findings:
- The script has proper error handling after the npm link command, checking for package.json existence
- This is part of a system-level installation script where forcing symlink creation is acceptable
- The script is used in a controlled environment (Unraid system) where overwriting existing links during updates is desired
- There's proper error handling and exit conditions if the installation fails
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any related issues or discussions about npm link problems
# Look for related comments in package.json or documentation
# Check package.json for any relevant comments
rg -A 5 "npm link|symlink" "${api_base_directory}/package.json" || true
# Check for any documentation about installation issues
fd -t f "README|INSTALL|CONTRIBUTING" -X rg -l "npm link|symlink" || true
Length of output: 178
Script:
#!/bin/bash
# Let's try a different approach to find the package.json and any documentation
# First, locate the package.json files
echo "=== Finding package.json files ==="
fd "package.json$"
echo -e "\n=== Finding README and documentation files ==="
fd -i "readme|install|contributing"
echo -e "\n=== Checking git history for npm link related changes ==="
git log -n 10 --oneline --grep="npm link\|symlink"
echo -e "\n=== Checking for npm related error handling in the script ==="
rg "npm (link|install)" plugin/source/dynamix.unraid.net/etc/rc.d/rc.unraid-api -A 2 -B 2
Length of output: 1075
Script:
#!/bin/bash
# Let's check the content of the API's package.json and README for any relevant information
echo "=== API package.json content ==="
cat api/package.json
echo -e "\n=== API README content ==="
cat api/README.md
echo -e "\n=== Checking for error handling around npm commands in the script ==="
rg "npm|error|fail" plugin/source/dynamix.unraid.net/etc/rc.d/rc.unraid-api -C 5
echo -e "\n=== Checking if there are any npm-related issues in the web component ==="
rg "npm link" web/package.json web/README.md web/CONTRIBUTING.md 2>/dev/null || true
Length of output: 9347
web/components/Modal.vue (2)
118-118: LGTM! Semantic use of bg-card
The header background implementation using bg-card with opacity provides good visual hierarchy while maintaining theme consistency.
97-97: LGTM! Verify color contrast ratios
The transition to semantic color tokens (text-foreground, bg-muted, dark:bg-background) aligns with modern theming practices. However, ensure the color contrast ratios meet WCAG accessibility standards in both light and dark modes.
✅ Verification successful
The semantic color tokens implementation is correctly configured
Based on the Tailwind configuration, the color tokens (text-foreground, bg-muted, dark:bg-background) are properly defined using HSL variables with appropriate dark mode support. The semantic tokens are part of a well-structured theming system that ensures consistent color contrast across modes:
foreground: Base text color with dark/light mode variantsmuted: Subtle background color with appropriate contrastbackground: Base background color with dark/light mode variants
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any hardcoded color values that might override these semantic tokens
rg -g '*.{css,scss}' '(--)(background|foreground|muted):'
Length of output: 59
Script:
#!/bin/bash
# Let's try a broader search for theme-related files and color definitions
fd -e css -e scss | xargs cat
# Also search for tailwind config which typically defines these semantic tokens
fd -g '*tailwind*' | xargs cat
Length of output: 13183
web/pages/index.vue (3)
2-14: LGTM! Well-structured imports.
The imports are well-organized and follow good practices:
- Individual icon imports enable better tree-shaking
- Clear separation between external and internal imports
152-156: LGTM! Clean Tailwind implementation.
The styling changes follow Tailwind best practices and maintain consistency with the dark mode implementation.
67-68: Verify dark mode integration with ColorSwitcherCe.
The addition of ColorSwitcherCe aligns with the PR's dark mode objectives. Let's verify its implementation.
✅ Verification successful
Let me gather more information about the ColorSwitcherCe component specifically.
Dark mode integration with ColorSwitcherCe is properly implemented
The verification shows that ColorSwitcherCe is well-integrated with the dark mode functionality:
- It's connected to the theme store using
useThemeStore - The component handles theme switching through
setTheme - Dark mode styles are properly applied in the parent component (
dark:classes in index.vue) - The theme system supports both light and dark themes through the store
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check ColorSwitcherCe implementation
ast-grep --pattern 'class ColorSwitcherCe {
$$$
}'
# Look for dark mode related utilities
rg -A 3 "dark:"
Length of output: 7517
Script:
#!/bin/bash
# Search for ColorSwitcherCe component file and its implementation
fd ColorSwitcherCe
# Look for theme switching related code
rg -A 5 "ColorSwitcherCe"
rg -A 5 "toggleTheme|setTheme|useTheme"
Length of output: 5646
web/components/UserProfile.ce.vue (4)
104-104: Ensure header text remains visible in both modes
The new text-header-text-secondary class is more semantic, but header text visibility is crucial for accessibility.
✅ Verification successful
Header text visibility is properly handled in both modes
The text-header-text-secondary class is well-defined in the theme configuration with appropriate color values for both light and dark modes:
- Light mode: Uses
#999999(grey) - Dark mode: Uses
#999999(grey)
The color is consistently used across header components and maintains visibility in both modes. The theme store properly manages the color transitions and ensures accessibility through the setCssVars function.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for header text color definitions in theme configuration
rg "header-text-secondary" -A 3 "web/**/*.{css,ts,js}"
Length of output: 115
Script:
#!/bin/bash
# Check for header text color definitions in tailwind config
fd "tailwind.config" --exec cat {}
# Check for any color definitions in theme files
fd "theme" --extension ts --extension js --extension css --exec cat {}
# Check for any usage of the class in other components
rg "text-header-text-secondary"
Length of output: 14491
111-114: Review visual hierarchy in dark mode
The distinction between primary and secondary text is important for visual hierarchy. Ensure sufficient contrast between:
- Primary header text (
text-header-text-primary) - Secondary header text (
text-header-text-secondary) - Background in both modes
✅ Verification successful
Color contrast is properly defined and implemented
Based on the verification results, the color contrast implementation is well-defined:
- Light mode:
- Primary:
#1c1c1c(dark gray) - Secondary:
#999999(medium gray)
- Primary:
- Dark mode:
- Primary:
#f2f2f2(light gray/white) - Secondary:
#999999(medium gray)
- Primary:
These color combinations provide sufficient contrast for visual hierarchy in both modes, with:
- Clear distinction between primary and secondary text
- Consistent secondary color across modes
- High contrast between text and typical dark/light backgrounds
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check color contrast definitions
rg "color-headerText(Primary|Secondary)" -A 3 "web/**/theme*"
Length of output: 117
Script:
#!/bin/bash
# Let's try different patterns and locations to find theme-related color definitions
rg -i "headerText|header.*text" web/
Length of output: 2711
Script:
#!/bin/bash
# Search for tailwind color configurations
fd "tailwind" --type f
Length of output: 75
Script:
#!/bin/bash
# Look for any theme or color configuration files
fd -e js -e ts -e json "theme|color" --type f
Length of output: 66
Script:
#!/bin/bash
# Check the actual component for any imported theme files
cat "web/components/UserProfile.ce.vue"
Length of output: 6470
100-100: Verify dark mode contrast for text-foreground class
The change from text-alpha to text-foreground is more semantic, but we should ensure proper contrast ratios in both light and dark modes.
✅ Verification successful
The text-foreground class is properly configured for dark mode
The change from text-alpha to text-foreground is verified to be correct. The codebase shows a well-implemented dark mode system where:
- Light mode sets
--foreground: '0 0% 3.9%'(near black) - Dark mode sets
--foreground: '0 0% 98%'(near white) - The theme store automatically switches these values based on the dark mode state
- The contrast ratios are appropriate for both modes
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for text-foreground class definition and its dark mode variant
rg "text-foreground" -A 3 "tailwind.config*"
Length of output: 101
Script:
#!/bin/bash
# Let's try to find the tailwind configuration file with a broader search
fd "tailwind.config"
# Also search for text-foreground class in CSS files
rg "text-foreground" --type css -A 3
Length of output: 188
Script:
#!/bin/bash
# Let's check the tailwind config file for color definitions
cat web/tailwind.config.ts
# Also search for background and foreground color definitions in theme files
fd -e ts -e js "theme|colors" web/
Length of output: 9263
Script:
#!/bin/bash
# Let's check the theme store for CSS variable definitions
cat web/store/theme.ts
# Also search for any CSS files that might define these variables
rg "var\(--foreground\)" --type css -A 3
Length of output: 4608
128-128: Verify separator visibility in dark mode
The change from bg-gamma to bg-popover should maintain sufficient contrast with surrounding elements in both modes.
web/tailwind.config.ts (4)
81-83: CSS variables for header colors—looks good.
Defining header-text-primary, header-text-secondary, and header-background-color using CSS variables enhances theming flexibility.
85-116: Updated color definitions to use CSS variables—good approach.
Using CSS variables with HSL values for colors like border, input, ring, background, foreground, and others improves theming consistency and maintainability.
218-257: Typography settings updated to use theme colors—looks good.
The typography settings now utilize theme colors defined in the extended color palette, enhancing visual consistency across components.
7-7:
Verify 'darkMode' configuration value 'selector'.
The darkMode option in Tailwind CSS configuration typically accepts 'media', 'class', or an array like ['class', '.selector']. Using 'selector' alone may not be valid unless you have a custom implementation. Please verify that 'selector' is acceptable in your Tailwind configuration.
If 'selector' is not valid, consider changing it to a valid option like 'class' or customizing it as ['class', '.your-selector'].
web/types/ui/badge.ts (1)
4-4: Verify removal of 'alpha' from 'UiBadgePropsColor' does not affect other components.
The 'alpha' option has been removed from UiBadgePropsColor. Please verify that no other components or files rely on 'alpha' as a valid badge color to prevent potential runtime errors.
Run the following script to check for usages of 'alpha' in the codebase:
✅ Verification successful
Removal of 'alpha' from UiBadgePropsColor is safe
Based on the codebase analysis, there are no usages of 'alpha' as a badge color. The only component importing and using UiBadgePropsColor is HeaderOsVersion.ce.vue, which uses 'yellow' and 'orange' colors. The removal of the 'alpha' option is safe and won't cause any runtime errors.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for usages of 'alpha' in badge color assignments.
rg --json --type-add 'vue:*.vue' -tvue '"alpha"'
Length of output: 307
Script:
#!/bin/bash
# Search for 'alpha' in all files, including Vue components, with surrounding context
rg -C 2 "alpha" --type-add 'vue:*.vue' --type vue --type typescript --type javascript
# Search specifically for badge color assignments in Vue files
rg -C 2 "UiBadgePropsColor.*alpha" --type-add 'vue:*.vue' --type vue --type typescript --type javascript
# Search for any imports or usage of UiBadgePropsColor
rg -C 2 "UiBadgePropsColor" --type-add 'vue:*.vue' --type vue --type typescript --type javascript
Length of output: 395
Script:
#!/bin/bash
# Search for 'alpha' in all files, including Vue components, with surrounding context
rg -C 2 "alpha" --type-add 'vue:*.vue' --type vue -t ts -t js
# Search specifically for badge color assignments in Vue files
rg -C 2 "UiBadgePropsColor.*alpha" --type-add 'vue:*.vue' --type vue -t ts -t js
# Search for any imports or usage of UiBadgePropsColor
rg -C 2 "UiBadgePropsColor" --type-add 'vue:*.vue' --type vue -t ts -t js
Length of output: 1800
web/components/shadcn/sheet/index.ts (1)
13-13: Updated background classes in 'sheetVariants'—looks good.
The background classes have been updated to 'bg-muted dark:bg-background', which should correctly handle theming in both light and dark modes.
web/components/Notifications/Indicator.vue (1)
49-51: Consistent use of theme colors for indicator background—looks good.
The background colors for the notification indicators are dynamically set based on indicatorLevel, enhancing visual feedback for different notification states.
web/store/theme.ts (1)
27-38:
Review color contrast ratios for accessibility
The current color combinations might not meet WCAG accessibility standards for contrast ratios:
- Dark theme: Text #1c1c1c on background #f2f2f2
- Light theme: Secondary text #999999 on dark background
Consider using a color contrast checker to ensure all combinations meet WCAG AA standards (minimum 4.5:1 for normal text, 3:1 for large text).
web/components/Brand/Button.vue (1)
51-51: LGTM: Improved dark mode compatibility
The change from specific color tokens (alpha/beta) to semantic color tokens (primary/muted) improves dark mode support by making the button colors theme-aware.
web/components/Notifications/Sidebar.vue (2)
4-4: LGTM: Using theme-aware button component
Using the shadcn Button component ensures consistent styling and proper dark mode support.
54-54: LGTM: Removed hardcoded background color
Removing the hardcoded light theme background color (bg-[#f2f2f2]) improves dark mode compatibility. The component will now inherit the appropriate theme background.
web/store/replaceRenew.ts (1)
67-67: LGTM: Consistent semantic color usage
Replacing 'gamma' with 'gray' for status indicators aligns with semantic color naming conventions and improves dark mode compatibility.
Also applies to: 113-113
| stringifiedData, | ||
| import.meta.env.VITE_CALLBACK_KEY, | ||
| ).toString(); | ||
| const encryptedMessage = AES.encrypt(stringifiedData, import.meta.env.VITE_CALLBACK_KEY).toString(); |
There was a problem hiding this comment.
💡 Codebase verification
Encryption implementation needs to be updated for consistency
The verification revealed inconsistencies in the encryption key usage:
- The
index.vuefile directly usesimport.meta.env.VITE_CALLBACK_KEY - The callback store uses
callbackActions.encryptionKey - Both implementations use the same AES encryption/decryption pattern
Recommendations:
- Use the centralized
callbackActions.encryptionKeyfrom the callback store instead of directly accessing the environment variable - Update the encryption in
index.vueto use the store'ssendmethod which already handles the encryption consistently
🔗 Analysis chain
Verify the encryption implementation.
The encryption key's usage looks correct, but let's ensure it's properly handled across the application.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other uses of AES encryption to ensure consistent implementation
rg -A 3 "AES.encrypt"
# Check for the presence of the encryption key in environment files
fd -g ".env*" --exec grep -l "VITE_CALLBACK_KEY"
Length of output: 870
Script:
#!/bin/bash
# Search for decryption implementations to ensure consistency
rg -A 3 "AES.decrypt"
# Search for encryption key handling in the callback store
rg -A 5 "encryptionKey" web/store/callback.ts
# Search for environment files including hidden ones
fd -H -g ".env*" --exec grep -l "VITE_CALLBACK_KEY"
Length of output: 1253
Script:
#!/bin/bash
# Check the content of environment files to verify key format
cat web/.env.example
# Look for any other encryption-related environment variables
rg "VITE.*KEY" web/.env.example web/.env.staging
# Check the complete callback store implementation for encryption handling
cat web/store/callback.ts
Length of output: 7506
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
web/pages/index.vue (1)
30-30:⚠️ Potential issueUse centralized encryption key management
The direct use of
import.meta.env.VITE_CALLBACK_KEYcreates inconsistency in encryption key management.Use the centralized
callbackActions.encryptionKeyfrom the callback store instead:- const encryptedMessage = AES.encrypt(stringifiedData, import.meta.env.VITE_CALLBACK_KEY).toString(); + const encryptedMessage = AES.encrypt(stringifiedData, callbackActions.encryptionKey).toString();
🧹 Nitpick comments (2)
web/pages/index.vue (2)
100-100: Fix typo in heading"DowngraadeOsCe" contains a typo (extra 'a').
- <h3 class="text-lg font-semibold font-mono">DowngraadeOsCe</h3> + <h3 class="text-lg font-semibold font-mono">DowngradeOsCe</h3>
126-132: Consider consolidating dark mode color handlingThe legacy components sections introduce multiple color variations. Consider:
- Using CSS custom properties for consistent color management
- Implementing a theme provider to handle dark/light mode transitions
Example approach:
:root { --badge-bg-color: theme('colors.gray.200'); --badge-text-color: theme('colors.black'); } .dark { --badge-bg-color: theme('colors.gray.800'); --badge-text-color: theme('colors.white'); }Also applies to: 133-139
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (2)
web/components/Notifications/Indicator.vue(1 hunks)web/pages/index.vue(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- web/components/Notifications/Indicator.vue
🔇 Additional comments (1)
web/pages/index.vue (1)
148-148: LGTM: Style changes follow Tailwind best practices
The styling changes for code and pre elements are well-structured and use appropriate Tailwind classes.
Also applies to: 152-152
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
web/components/ColorSwitcher.ce.vue (3)
14-25: Consolidate toggle functions to reduce duplicationThe toggle functions follow the same pattern and can be consolidated into a single reusable function.
-const toggleSwitch = (value: boolean) => { - setDarkMode.value = value; -}; -const toggleGradient = (value: boolean) => { - setGradient.value = value; -}; -const toggleDescription = (value: boolean) => { - setDescription.value = value; -}; -const toggleBanner = (value: boolean) => { - setBanner.value = value; -}; +type ToggleType = 'darkMode' | 'gradient' | 'description' | 'banner'; +const toggleSetting = (type: ToggleType, value: boolean) => { + switch(type) { + case 'darkMode': + setDarkMode.value = value; + break; + case 'gradient': + setGradient.value = value; + break; + case 'description': + setDescription.value = value; + break; + case 'banner': + setBanner.value = value; + break; + } +};Update template usage:
-<Switch id="dark-mode" @update:checked="toggleSwitch" /> +<Switch id="dark-mode" @update:checked="(value) => toggleSetting('darkMode', value)" />
56-58: Remove console.log statementRemove debugging console.log statement before production deployment.
- console.log(newVal);
56-68: Add debounce to theme updatesConsider adding debounce to prevent rapid theme updates when values change quickly.
+import { debounce } from 'lodash-es'; + +const updateTheme = debounce((newVal) => { + const themeToSet: Theme = { + banner: setBanner.value, + bannerGradient: setGradient.value, + descriptionShow: setDescription.value, + textColor: textPrimaryToSet.value, + metaColor: textSecondaryToSet.value, + bgColor: bgColorToSet.value, + name: setDarkMode.value ? 'black' : 'light', + }; + themeStore.setTheme(themeToSet); +}, 300); + -watch([setDarkMode, bgColorToSet, textSecondaryToSet, textPrimaryToSet], (newVal) => { - console.log(newVal); - const themeToSet: Theme = { - banner: setBanner.value, - bannerGradient: setGradient.value, - descriptionShow: setDescription.value, - textColor: textPrimaryToSet.value, - metaColor: textSecondaryToSet.value, - bgColor: bgColorToSet.value, - name: setDarkMode.value ? 'black' : 'light', - }; - themeStore.setTheme(themeToSet); -}); +watch( + [setDarkMode, bgColorToSet, textSecondaryToSet, textPrimaryToSet], + updateTheme +);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (1)
web/components/ColorSwitcher.ce.vue(1 hunks)
🔇 Additional comments (3)
web/components/ColorSwitcher.ce.vue (3)
1-8: Add missing Switch component import
While Nuxt can handle auto-imports, explicit imports improve code readability and maintainability.
<script lang="ts" setup>
+import Switch from '~/components/shadcn/switch/Switch.vue';
import Input from '~/components/shadcn/input/Input.vue';27-54: Add color input validation and refactor computed properties
-
Color validation is still needed to ensure valid color values.
-
The computed properties follow similar patterns and can be refactored to reduce duplication.
+const getComputedColor = (
+ userValue: string,
+ darkDefault: string,
+ lightDefault: string
+): string => {
+ if (userValue) {
+ return userValue;
+ }
+ return darkMode.value ? darkDefault : lightDefault;
+};
+
-const textPrimaryToSet = computed(() => {
- if (textPrimary.value) {
- return textPrimary.value;
- }
- return darkMode.value ? defaultColors.dark.headerTextPrimary : defaultColors.light.headerTextPrimary;
-});
+const textPrimaryToSet = computed(() =>
+ getComputedColor(
+ textPrimary.value,
+ defaultColors.dark.headerTextPrimary,
+ defaultColors.light.headerTextPrimary
+ )
+);91-95: LGTM!
The Tailwind CSS setup follows best practices.
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
elibosley
force-pushed
the
feat/dark-mode-fixes
branch
from
8ed9df4 to
1a01a8c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
web/store/theme.ts (2)
21-75: Consider using consistent color format across theme definitions.The theme variables mix HSL values (e.g., '0 0% 3.9%') with hex colors (e.g., '#1c1c1c'). Consider converting all colors to the same format for better maintainability and consistency.
Example conversion to HSL for header colors:
- '--header-text-primary': '#1c1c1c', - '--header-text-secondary': '#999999', - '--header-background-color': '#f2f2f2', + '--header-text-primary': '0 0% 11%', + '--header-text-secondary': '0 0% 60%', + '--header-background-color': '0 0% 95%',
91-93: Simplify the banner gradient condition.The nested conditions can be simplified using a single return statement.
- if (!theme.value?.banner || !theme.value?.bannerGradient) { - return undefined; - } + return theme.value?.banner && theme.value?.bannerGradient + ? `background-image: linear-gradient(90deg, ${start} 0, ${end} 30%);` + : undefined;web/tailwind.config.ts (2)
81-83: Document header color variable requirementsThe new header-specific CSS variables need documentation about:
- Expected color formats (HSL, RGB, hex)
- Default values
- Usage context within header components
Add JSDoc comments above these variables explaining their purpose and expected values:
+/** + * Header-specific color variables + * @var --header-text-primary - Main text color for headers + * @var --header-text-secondary - Secondary text color for headers + * @var --header-background-color - Background color for header components + */ 'header-text-primary': 'var(--header-text-primary)', 'header-text-secondary': 'var(--header-text-secondary)', 'header-background-color': 'var(--header-background-color)',
218-257: Optimize typography color configurationThe typography configuration thoroughly implements the new color system, but there's repetition in color assignments that could be simplified.
Consider extracting common color assignments into reusable variables:
+const proseColors = (theme) => ({ + base: theme('colors.foreground'), + inverted: theme('colors.background'), + links: theme('colors.primary'), +}); typography: (theme: PluginAPI['theme']) => ({ DEFAULT: { css: { - '--tw-prose-body': theme('colors.foreground'), - '--tw-prose-headings': theme('colors.foreground'), + '--tw-prose-body': proseColors(theme).base, + '--tw-prose-headings': proseColors(theme).base, // ... apply similar pattern to other prose colors } } })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (29)
plugin/source/dynamix.unraid.net/etc/rc.d/rc.unraid-api(1 hunks)web/assets/main.css(0 hunks)web/components/Brand/Button.vue(1 hunks)web/components/ColorSwitcher.ce.vue(1 hunks)web/components/HeaderOsVersion.ce.vue(1 hunks)web/components/Modal.vue(3 hunks)web/components/Notifications/Indicator.vue(1 hunks)web/components/Notifications/Sidebar.vue(2 hunks)web/components/Ui/Badge.vue(0 hunks)web/components/Ui/CardWrapper.vue(1 hunks)web/components/Ui/Switch.vue(1 hunks)web/components/UpdateOs/CheckUpdateResponseModal.vue(1 hunks)web/components/UserProfile.ce.vue(4 hunks)web/components/UserProfile/Dropdown.vue(1 hunks)web/components/UserProfile/DropdownContent.vue(1 hunks)web/components/UserProfile/DropdownItem.vue(1 hunks)web/components/UserProfile/DropdownLaunchpad.vue(2 hunks)web/components/UserProfile/DropdownTrigger.vue(1 hunks)web/components/UserProfile/DropdownWrapper.vue(1 hunks)web/components/UserProfile/ServerState.vue(1 hunks)web/components/shadcn/sheet/index.ts(1 hunks)web/nuxt.config.ts(3 hunks)web/pages/index.vue(8 hunks)web/pages/webComponents.vue(1 hunks)web/store/replaceRenew.ts(2 hunks)web/store/theme.ts(4 hunks)web/store/updateOs.ts(1 hunks)web/tailwind.config.ts(3 hunks)web/types/ui/badge.ts(1 hunks)
💤 Files with no reviewable changes (2)
- web/components/Ui/Badge.vue
- web/assets/main.css
🚧 Files skipped from review as they are similar to previous changes (25)
- web/components/UserProfile/DropdownWrapper.vue
- web/components/Ui/CardWrapper.vue
- web/components/UserProfile/Dropdown.vue
- web/components/Brand/Button.vue
- web/store/updateOs.ts
- web/components/UpdateOs/CheckUpdateResponseModal.vue
- web/components/UserProfile/ServerState.vue
- web/components/UserProfile/DropdownLaunchpad.vue
- web/components/UserProfile/DropdownItem.vue
- plugin/source/dynamix.unraid.net/etc/rc.d/rc.unraid-api
- web/components/HeaderOsVersion.ce.vue
- web/components/Notifications/Indicator.vue
- web/components/UserProfile/DropdownContent.vue
- web/components/Notifications/Sidebar.vue
- web/components/ColorSwitcher.ce.vue
- web/pages/index.vue
- web/components/Ui/Switch.vue
- web/nuxt.config.ts
- web/pages/webComponents.vue
- web/components/UserProfile/DropdownTrigger.vue
- web/components/UserProfile.ce.vue
- web/types/ui/badge.ts
- web/components/shadcn/sheet/index.ts
- web/components/Modal.vue
- web/store/replaceRenew.ts
🔇 Additional comments (3)
web/store/theme.ts (2)
77-84: LGTM! Well-structured store setup.
The store implementation follows Pinia best practices with proper typing and state initialization.
102-131:
Add error handling and SSR considerations.
The theme application logic directly manipulates the DOM without proper error handling. Additionally, this could fail in SSR environments.
Consider these improvements:
const setCssVars = () => {
+ if (typeof window === 'undefined') return;
+
+ try {
const customColorVariables = structuredClone(defaultColors);
const body = document.body;
+ if (!body) {
+ throw new Error('Document body not available');
+ }
const selectedMode = darkMode.value ? 'dark' : 'light';
if (theme.value?.textColor) {
+ if (!/^#([A-Fa-f0-9]{6}|[A-Fa-f0-9]{3})$/.test(theme.value.textColor)) {
+ console.warn(`Invalid hex color: ${theme.value.textColor}`);
+ return;
+ }
customColorVariables[selectedMode]['--header-text-primary'] = theme.value.textColor;
}
// ... rest of the function
+ } catch (error) {
+ console.error('Failed to apply theme:', error);
+ }
};Likely invalid or redundant comment.
web/tailwind.config.ts (1)
7-7: Verify dark mode selector compatibility with web components
Changing from class to selector based dark mode is a significant change that could affect existing implementations. Ensure that:
- All web components can properly detect and respond to the new dark mode selector
- The transition doesn't break existing dark mode functionality
- Shadow DOM boundaries don't interfere with dark mode propagation
✅ Verification successful
Dark mode implementation is compatible with web components
The change from class to selector based dark mode is safe and properly implemented. The codebase shows:
- Dark mode is managed through a Pinia store that adds/removes the
darkclass on the body element - CSS variables for both light and dark themes are properly defined and applied
- The current implementation uses class-based toggling (
document.body.classList.add('dark')) which works across shadow DOM boundaries - Web components are already using the dark variant utilities (e.g.,
dark:bg-background,dark:text-white)
The change to selector mode in Tailwind config is compatible with this implementation as it will work with the existing class-based approach while also supporting potential future selector-based implementations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for existing dark mode class usage that might need updates
rg -l "dark:" web/
rg -l "\.dark\s" web/
Length of output: 214
Script:
#!/bin/bash
# Check the content of theme store and components for dark mode implementation
rg "dark:" -A 5 -B 5 web/store/theme.ts web/pages/index.vue web/pages/webComponents.vue web/components/Modal.vue
# Look for any data-theme attributes that might be used with the new selector approach
rg "data-theme" web/
# Check for any dark mode related CSS variables
rg "var\(--dark" web/
Length of output: 10534
Script:
#!/bin/bash
# Check for theme-related code in the store implementation
cat web/store/theme.ts
# Look for any HTML/body level theme attribute setting
rg -l "document\.documentElement\.setAttribute.*theme" web/
rg -l "html.*data-theme" web/
# Check for any theme-related utilities or composables
fd "theme" web/ --type f
Length of output: 4717
| // ShadCN | ||
| border: 'hsl(var(--border))', | ||
| input: 'hsl(var(--input))', | ||
| ring: 'hsl(var(--ring))', | ||
| background: 'hsl(var(--background))', | ||
| foreground: 'hsl(var(--foreground))', | ||
| primary: { | ||
| DEFAULT: 'hsl(0 0% 9%)', | ||
| foreground: 'hsl(0 0% 98%)', | ||
| DEFAULT: 'hsl(var(--primary))', | ||
| foreground: 'hsl(var(--primary-foreground))', | ||
| }, | ||
| secondary: { | ||
| DEFAULT: 'hsl(0 0% 96.1%)', | ||
| foreground: 'hsl(0 0% 9%)', | ||
| DEFAULT: 'hsl(var(--secondary))', | ||
| foreground: 'hsl(var(--secondary-foreground))', | ||
| }, | ||
| destructive: { | ||
| DEFAULT: 'hsl(0 84.2% 60.2%)', | ||
| foreground: 'hsl(0 0% 98%)', | ||
| DEFAULT: 'hsl(var(--destructive))', | ||
| foreground: 'hsl(var(--destructive-foreground))', | ||
| }, | ||
| muted: { | ||
| DEFAULT: 'hsl(0 0% 96.1%)', | ||
| foreground: 'hsl(0 0% 45.1%)', | ||
| DEFAULT: 'hsl(var(--muted))', | ||
| foreground: 'hsl(var(--muted-foreground))', | ||
| }, | ||
| accent: { | ||
| DEFAULT: 'hsl(0 0% 96.1%)', | ||
| foreground: 'hsl(0 0% 9%)', | ||
| DEFAULT: 'hsl(var(--accent))', | ||
| foreground: 'hsl(var(--accent-foreground))', | ||
| }, | ||
| popover: { | ||
| DEFAULT: 'hsl(0 0% 100%)', | ||
| foreground: 'hsl(0 0% 3.9%)', | ||
| DEFAULT: 'hsl(var(--popover))', | ||
| foreground: 'hsl(var(--popover-foreground))', | ||
| }, | ||
| card: { | ||
| DEFAULT: 'hsl(0 0% 100%)', | ||
| foreground: 'hsl(0 0% 3.9%)', | ||
| DEFAULT: 'hsl(var(--card))', | ||
| foreground: 'hsl(var(--card-foreground))', |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add fallback values for CSS color variables
The new color system using HSL variables is well-structured, but it lacks fallback values which could cause issues if the CSS variables aren't properly initialized.
Add fallback values to ensure graceful degradation:
- background: 'hsl(var(--background))',
+ background: 'hsl(var(--background, 0 0% 100%))',
- foreground: 'hsl(var(--foreground))',
+ foreground: 'hsl(var(--foreground, 240 10% 3.9%))',
primary: {
- DEFAULT: 'hsl(var(--primary))',
+ DEFAULT: 'hsl(var(--primary, 240 5.9% 10%))',
- foreground: 'hsl(var(--primary-foreground))',
+ foreground: 'hsl(var(--primary-foreground, 0 0% 98%))',
},Committable suggestion skipped: line range outside the PR's diff.
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
3 participants
Summary by CodeRabbit
Release Notes
New Features
ColorSwitchercomponent for user interface theme customization.Visual Enhancements
HeaderOsVersionandServerStatecomponents for improved visual consistency.Bug Fixes
Chores