fix: fix change theme error when open playground #2681
Conversation
WalkthroughThis pull request introduces comprehensive updates to theme management and versioning across multiple files in the project. The changes primarily focus on updating version constants, modifying theme-related configurations, and simplifying theme handling logic. The modifications affect routing, theme selection, and playground functionality, with a particular emphasis on updating theme mappings and removing deprecated references. Changes
Sequence DiagramsequenceDiagram
participant App as App.vue
participant Router as Router
participant ThemeTool as useTheme.js
participant Demo as Demo Component
App->>ThemeTool: Initialize Theme
ThemeTool-->>App: Set Current Theme
App->>Router: Configure Routes
Router-->>App: Routing Configured
Demo->>ThemeTool: Request Theme
ThemeTool-->>Demo: Provide Theme Details
Possibly related PRs
Suggested labels
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
|
[e2e-test-warn] The title of the Pull request should look like "fix(vue-renderless): [action-menu, alert] fix xxx bug". Please make sure you've read our contributing guide |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
examples/sites/src/tools/useTheme.js (1)
44-44: Consider adding theme validationThe code assumes theme values are always valid. Consider adding validation when:
- Setting the initial theme
- Changing themes
- Returning the default theme
+const isValidTheme = (theme) => theme in themeToolValuesMap + -const currentThemeKey = hooks.ref(DEFAULT_THEME) +const currentThemeKey = hooks.ref(isValidTheme(DEFAULT_THEME) ? DEFAULT_THEME : Object.keys(themeToolValuesMap)[0]) export default function useTheme() { !initWatchRoute && watchRoute() return { getThemeData, changeTheme, currentThemeKey, designConfig, - defaultTheme: DEFAULT_THEME + defaultTheme: isValidTheme(DEFAULT_THEME) ? DEFAULT_THEME : Object.keys(themeToolValuesMap)[0] } }Also applies to: 103-103
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
examples/sites/playground/App.vue(4 hunks)examples/sites/src/const.ts(2 hunks)examples/sites/src/router.js(2 hunks)examples/sites/src/tools/useTheme.js(4 hunks)examples/sites/src/views/components/demo.vue(1 hunks)
🔇 Additional comments (5)
examples/sites/src/router.js (1)
38-38: Verify default theme redirect behavior
The redirect now uses DEFAULT_THEME directly instead of going through THEME_ROUTE_MAP. While this simplifies the code, ensure that:
- The redirect works correctly with the new theme value format
- All components support the new theme path format
✅ Verification successful
Let me gather more information about the theme-related changes to verify the impact comprehensively.
Let me gather one final piece of information about the theme path handling in components.
Let me check the const.ts file specifically to understand the theme configuration.
Based on the gathered information, I can now provide the final response:
Theme redirect behavior is correctly implemented
The redirect using DEFAULT_THEME directly is safe because:
DEFAULT_THEMEis properly defined as'os-theme'inconst.ts- The theme system is well-structured with proper mappings in
themeToolValuesMap - Components correctly handle theme paths through
useThemehook which supports all defined themes including the default one
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for components that might expect the old theme format in routes
rg -A 5 "params\.theme" --glob '!{*.test.*,*.spec.*}'
Length of output: 868
Script:
#!/bin/bash
# Check for DEFAULT_THEME definition and usage
rg "DEFAULT_THEME" -A 2
# Check for any other theme-related route handling
rg "theme.*route" -i
# Look for theme-related component configurations
ast-grep --pattern 'const $_ = {
theme: $_,
$$$
}'
Length of output: 6965
Script:
#!/bin/bash
# Check for theme-related path construction in components
rg -A 3 "/${theme}/" --glob '*.{vue,js,ts}'
# Check the const.ts file for theme-related configurations
cat examples/sites/src/const.ts
Length of output: 67752
Script:
#!/bin/bash
# Check the theme-related constants and configurations
cat examples/sites/src/const.ts
# Check for any theme path usage in components
rg -A 2 "/${theme}/" --type-add 'vue:*.vue' --type vue
Length of output: 73262
examples/sites/playground/App.vue (3)
14-14: LGTM: Version bump to 3.20
The version update is consistent with the versions array and CDN import logic.
86-89: Verify SMB theme configuration
The addition of 'smb' theme support needs corresponding design configuration package.
✅ Verification successful
SMB theme configuration is properly set up
The SMB theme is well-configured in the codebase with:
- Package
@opentiny/vue-design-smbexists with proper dependencies - Theme is properly imported and used in multiple files including
examples/sites/src/tools/useTheme.js - Theme configuration is documented and referenced across the codebase
- Multiple components have been adapted to support the SMB theme as evidenced by the changelog entries
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the existence and configuration of SMB theme package
# Check if the SMB theme package exists in package.json
rg -A 5 "@opentiny/vue-design-smb"
# Check for SMB theme-related configurations
rg "smb.*theme"
Length of output: 12029
157-167: Verify theme import configurations
The theme import logic has been updated to support the new theme structure.
✅ Verification successful
Let me gather more information about the theme configuration to ensure completeness of verification.
Theme import configurations are correctly structured
The verification confirms that:
- Theme tool imports are consistently used across the codebase
- Both aurora and old themes are properly imported in theme-tool.js
- Design configuration is correctly provided using design.configKey
- The theme import logic in App.vue aligns with the theme structure used throughout the project
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify theme import configurations
# Check theme tool imports
rg "import.*theme-tool"
# Check old theme imports
rg "import.*old-theme-index"
# Verify theme configuration usage
ast-grep --pattern 'import TinyThemeTool from "@opentiny/vue-theme/theme-tool"'
Length of output: 1558
Script:
#!/bin/bash
# Check theme-related imports and configurations
# Check aurora theme imports
rg "import.*aurora-theme-index"
# Check theme configuration patterns
rg "designThemeMap\[.*\]"
# Check theme configuration object
ast-grep --pattern "const designThemeMap = {
$$$
}"
# Check other theme-related configurations
rg "provide\(design\.configKey"
Length of output: 592
examples/sites/src/views/components/demo.vue (1)
193-193:
Fix: Theme key extraction logic
The change in theme key extraction from split('-')[1] to split('-')[0] appears to be the root cause fix for the theme change error. This modification affects how themes are determined when opening the playground.
Please verify:
- The format of
currentThemeKeyvalues - The impact on existing theme configurations
- Test different theme combinations to ensure the fix works as expected
✅ Verification successful
Let me gather more context about the theme-related code to understand the change better.
Let me check the theme configuration and initialization code to fully understand the theme key structure.
Based on the code analysis, I can now generate the final response:
Theme key extraction change is correct
The modification from split('-')[1] to split('-')[0] is the right fix because:
- The theme keys in
const.tsare defined asos-theme,aurora-theme, andsmb-theme - The
tinyThemevariable is used to load design configurations and theme files with the base theme name (e.g., 'saas', 'os', 'aurora') - Taking the first part of the theme key (
split('-')[0]) correctly extracts the base theme name needed for imports
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify theme key format and usage
# Check theme key formats in the codebase
rg "currentThemeKey.*split.*\[.*\]"
# Check theme-related constants and their usage
ast-grep --pattern 'const currentThemeKey = $$$'
Length of output: 320
Script:
#!/bin/bash
# Check the DEFAULT_THEME constant and theme-related configurations
rg "DEFAULT_THEME.*=.*" -A 2
# Look for theme-related patterns and their usage
rg "theme.*split.*\[.*\]" -B 2 -A 2
# Check for any other theme key processing
ast-grep --pattern 'const $_ = $_.split("-")'
# Find theme-related imports and configurations
rg "import.*Theme" -A 2
Length of output: 8447
Script:
#!/bin/bash
# Check the content of useTheme.js to understand theme initialization
cat examples/sites/src/tools/useTheme.js
# Look for theme key usage in demo.vue
cat examples/sites/src/views/components/demo.vue
Length of output: 23969
| // 主题工具导出主题变量和主题的映射关系 | ||
| export const themeToolValuesMap = { | ||
| [DEFAULT_THEME]: '', | ||
| [AURORA_THEME]: tinyAuroraTheme, | ||
| [SMB_THEME]: '', | ||
| [OLD_THEME]: tinyOldTheme |
There was a problem hiding this comment.
Review empty theme implementations in themeToolValuesMap
The map contains empty string values for DEFAULT_THEME and SMB_THEME. This could lead to unexpected behavior when these themes are selected.
Please clarify:
- Why are some theme values empty strings?
- What is the expected behavior when a theme with an empty value is selected?
- Should we import and use the corresponding theme implementations for these themes?
| export const DEFAULT_THEME = 'os-theme' | ||
| export const AURORA_THEME = 'aurora-theme' | ||
| export const SMB_THEME = 'smb-theme' | ||
| export const OLD_THEME = 'old-theme' |
There was a problem hiding this comment.
💡 Codebase verification
Theme constant values need to be updated across multiple files
The verification reveals several files still using the old theme values with 'tiny-' prefix that need to be updated to match the new simplified theme constants:
examples/sites/src/views/components/float-settings.vue: Uses 'tiny-smb-theme'examples/docs/newsrc/uses/useTheme.js: References 'tiny-default-theme', 'tiny-aurora-theme', and 'tiny-smb-theme'examples/docs/newsrc/pc.vue: Multiple occurrences in dropdown items and config mappackages/theme/src/aurora-theme-index.js: Theme ID using 'tiny-aurora-theme'packages/theme/build/release.js: Theme ID using 'tiny-aurora-theme'
🔗 Analysis chain
Verify theme constant value changes across the codebase
The theme constant values have been simplified by removing the 'tiny-' prefix. This change could impact other parts of the codebase that might be using the old values.
Let's examine the actual usage of these theme values in the found files to determine if they need updates.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining references to old theme values
rg -l "tiny-(default|aurora|smb)-theme"
Length of output: 235
Script:
#!/bin/bash
# Check the context of theme value usage in the affected files
echo "=== float-settings.vue ==="
rg -A 2 -B 2 "tiny-(default|aurora|smb)-theme" examples/sites/src/views/components/float-settings.vue
echo -e "\n=== useTheme.js ==="
rg -A 2 -B 2 "tiny-(default|aurora|smb)-theme" examples/docs/newsrc/uses/useTheme.js
echo -e "\n=== pc.vue ==="
rg -A 2 -B 2 "tiny-(default|aurora|smb)-theme" examples/docs/newsrc/pc.vue
echo -e "\n=== aurora-theme-index.js ==="
rg -A 2 -B 2 "tiny-(default|aurora|smb)-theme" packages/theme/src/aurora-theme-index.js
echo -e "\n=== release.js ==="
rg -A 2 -B 2 "tiny-(default|aurora|smb)-theme" packages/theme/build/release.js
Length of output: 2210
| if (!loadedTheme && themeToolValuesMap[val]) { | ||
| currentThemeKey.value = val | ||
| const themeTool = new TinyThemeTool() | ||
| themeTool.changeTheme(tinyAuroraTheme) | ||
| themeTool.changeTheme(themeToolValuesMap[val]) |
There was a problem hiding this comment.
Review theme loading logic
The theme loading logic has potential issues:
- The
loadedThemeflag prevents subsequent theme changes - No error handling for invalid theme values
- No cleanup of theme tool instance
Consider these improvements:
watch(
() => router.currentRoute.value.params.theme,
(val) => {
- if (!loadedTheme && themeToolValuesMap[val]) {
+ if (themeToolValuesMap[val]) {
currentThemeKey.value = val
const themeTool = new TinyThemeTool()
themeTool.changeTheme(themeToolValuesMap[val])
- loadedTheme = true
+ } else {
+ console.warn(`Theme '${val}' not found in themeToolValuesMap`)
}
}
)Committable suggestion skipped: line range outside the PR's diff.
| { | ||
| value: [OLD_THEME], | ||
| label: isEn ? 'Default Theme' : '冰川主题', | ||
| tips: isEn ? 'Accurate, Efficient, Distinct' : '精准、高效、清晰', | ||
| icon: glaciersIcon, | ||
| bgImage: glaciers | ||
| }, |
There was a problem hiding this comment.
💡 Codebase verification
Based on the evidence gathered, I can now provide the final response:
Theme metadata for OLD_THEME is inconsistent with its identifier
The OLD_THEME (defined as 'old-theme' in const.ts) is being presented with misleading metadata:
- It's labeled as "Default Theme"/"冰川主题" (Glacier theme)
- It uses glacier-themed assets (glaciers.png, glaciers-icon.png)
- The description suggests "Accurate, Efficient, Distinct"/"精准、高效、清晰"
This creates a confusing user experience as the theme name 'old-theme' doesn't match its presentation as a glacier/default theme. The theme metadata should accurately reflect the theme's identity and visual characteristics.
🔗 Analysis chain
Verify theme metadata for OLD_THEME
The OLD_THEME entry reuses the "冰川主题" (Glacier theme) metadata. This might be confusing for users if the visual appearance doesn't match the theme name and description.
Let me check the content of these files to understand how the theme names and descriptions are used.
Let me check the OLD_THEME definition and usage to understand its relationship with the theme metadata.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other occurrences of theme names/descriptions
rg -l "冰川主题|Glacier theme|Default Theme"
Length of output: 254
Script:
#!/bin/bash
# Check content of files containing theme references
echo "=== useTheme.js ==="
rg -A 2 -B 2 "冰川主题|Glacier theme|Default Theme" examples/sites/src/tools/useTheme.js
echo -e "\n=== changelog.md ==="
rg -A 2 -B 2 "冰川主题|Glacier theme|Default Theme" examples/sites/demos/pc/webdoc/changelog.md
echo -e "\n=== theme-en.md ==="
rg -A 2 -B 2 "冰川主题|Glacier theme|Default Theme" examples/sites/demos/pc/webdoc/theme-en.md
echo -e "\n=== badge.js ==="
rg -A 2 -B 2 "冰川主题|Glacier theme|Default Theme" examples/sites/demos/mobile-first/app/badge/webdoc/badge.js
echo -e "\n=== pc.vue ==="
rg -A 2 -B 2 "冰川主题|Glacier theme|Default Theme" examples/docs/newsrc/pc.vue
Length of output: 2302
Script:
#!/bin/bash
# Search for OLD_THEME definition and usage
echo "=== OLD_THEME definition ==="
rg -A 2 -B 2 "OLD_THEME.*=|const.*OLD_THEME|let.*OLD_THEME|var.*OLD_THEME"
echo -e "\n=== OLD_THEME usage ==="
rg -A 2 -B 2 "OLD_THEME"
Length of output: 1823
| @@ -143,7 +143,7 @@ const state = reactive({ | |||
|
|
|||
| const designThemeMap = { | |||
| aurora: 'tinyAuroraTheme', | |||
| infinity: 'tinyInfinityTheme' | |||
| old: 'tinyOldTheme' | |||
There was a problem hiding this comment.
Breaking Change: Renamed 'infinity' theme to 'old'
This change removes support for the 'infinity' theme key, which could break existing implementations using this theme.
Consider:
- Adding a deprecation notice for 'infinity' theme
- Supporting both theme keys temporarily for backward compatibility
- Documenting the migration path in the changelog
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
tiny-vue-version-3.20, enhancing version management.old, alongside existing themes.Bug Fixes
Democomponent to ensure correct theme application.Chores