Skip to content

fix: fix change theme error when open playground #2681

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 25, 2024
Merged

fix: fix change theme error when open playground #2681

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions examples/sites/playground/App.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import logoUrl from './assets/opentiny-logo.svg?url'
import GitHub from './icons/Github.vue'
import Share from './icons/Share.vue'

const VERSION = 'tiny-vue-version-3.19'
const VERSION = 'tiny-vue-version-3.20'
const NOTIFY_KEY = 'tiny-vue-playground-notify'
const LAYOUT = 'playground-layout'
const LAYOUT_REVERSE = 'playground-layout-reverse'
Expand Down Expand Up @@ -83,7 +83,7 @@ const createImportMap = (version) => {
'@opentiny/vue-renderless/': `${cdnHost}/@opentiny/vue-renderless${versionDelimiter}${version}/${fileDelimiter}`,
'sortablejs': `${cdnHost}/sortablejs${versionDelimiter}1.15.0/${fileDelimiter}modular/sortable.esm.js`
}
if (['aurora', 'saas'].includes(tinyTheme)) {
if (['aurora', 'saas', 'smb'].includes(tinyTheme)) {
imports[`@opentiny/vue-design-${tinyTheme}`] =
`${cdnHost}/@opentiny/vue-design-${tinyTheme}${versionDelimiter}${version}/${fileDelimiter}index.js`
}
Expand Down Expand Up @@ -143,7 +143,7 @@ const state = reactive({

const designThemeMap = {
aurora: 'tinyAuroraTheme',
infinity: 'tinyInfinityTheme'
old: 'tinyOldTheme'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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:

  1. Adding a deprecation notice for 'infinity' theme
  2. Supporting both theme keys temporarily for backward compatibility
  3. Documenting the migration path in the changelog

}

function setTinyDesign() {
Expand All @@ -154,16 +154,16 @@ function setTinyDesign() {
useCode += 'app.provide("TinyMode", "mobile-first");\n'
}

if (['aurora', 'saas'].includes(tinyTheme)) {
if (['aurora', 'saas', 'smb'].includes(tinyTheme)) {
importCode += `import designConfig from '@opentiny/vue-design-${tinyTheme}';
import { design } from '@opentiny/vue-common';\n`
useCode += 'app.provide(design.configKey, designConfig);\n'
}

if (['aurora', 'infinity'].includes(tinyTheme)) {
if (['aurora', 'old'].includes(tinyTheme)) {
const designTheme = designThemeMap[tinyTheme]
importCode += `import TinyThemeTool from '@opentiny/vue-theme/theme-tool';
import { ${designTheme} } from '@opentiny/vue-theme/theme';\n`
import ${designTheme} from '@opentiny/vue-theme/${tinyTheme}-theme-index.js';\n`
useCode += `const theme = new TinyThemeTool(${designTheme});\n`
}

Expand Down
22 changes: 12 additions & 10 deletions examples/sites/src/const.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { tinyOldTheme, tinyAuroraTheme } from '@opentiny/vue-theme/theme-tool'

// localStorage中保存语言的key
export const LANG_KEY = '_lang'
// localStorage中保存语言的value
Expand All @@ -12,17 +14,17 @@ export const LANG_PATH_MAP = {
export const CURRENT_THEME_KEY = 'tiny-current-theme'

// 主题
export const DEFAULT_THEME = 'tiny-default-theme'
export const AURORA_THEME = 'tiny-aurora-theme'
export const SMB_THEME = 'tiny-smb-theme'
export const INFINITY_THEME = 'tiny-infinity-theme'
export const DEFAULT_THEME = 'os-theme'
export const AURORA_THEME = 'aurora-theme'
export const SMB_THEME = 'smb-theme'
export const OLD_THEME = 'old-theme'
Comment on lines +17 to +20
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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 map
  • packages/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


// 主题对应路由参数映射表
export const THEME_ROUTE_MAP = {
[DEFAULT_THEME]: 'os-theme',
[AURORA_THEME]: 'aurora-theme',
[SMB_THEME]: 'smb-theme',
[INFINITY_THEME]: 'infinity-theme'
// 主题工具导出主题变量和主题的映射关系
export const themeToolValuesMap = {
[DEFAULT_THEME]: '',
[AURORA_THEME]: tinyAuroraTheme,
[SMB_THEME]: '',
[OLD_THEME]: tinyOldTheme
Comment on lines +22 to +27
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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:

  1. Why are some theme values empty strings?
  2. What is the expected behavior when a theme with an empty value is selected?
  3. Should we import and use the corresponding theme implementations for these themes?

}

/**
Expand Down
5 changes: 2 additions & 3 deletions examples/sites/src/router.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { createRouter, createWebHistory } from 'vue-router'
import Layout from '@/views/layout/layout.vue'
import { LANG_PATH_MAP, ZH_CN_LANG, THEME_ROUTE_MAP, DEFAULT_THEME } from './const'
import { LANG_PATH_MAP, ZH_CN_LANG, DEFAULT_THEME } from './const'

const Components = () => import('@/views/components/components.vue')
const Docs = () => import('@/views/docs/docs.vue')
Expand Down Expand Up @@ -35,8 +35,7 @@ let routes = [
path: '/:pathMatch(.*)*',
redirect: () => {
const langPath = LANG_PATH_MAP[ZH_CN_LANG]
const theme = THEME_ROUTE_MAP[DEFAULT_THEME]
return { path: `${context}${langPath}/${theme}/overview` }
return { path: `${context}${langPath}/${DEFAULT_THEME}/overview` }
}
}
]
Expand Down
52 changes: 15 additions & 37 deletions examples/sites/src/tools/useTheme.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,62 +4,44 @@ import designSaasConfig from '@opentiny/vue-design-saas'
import designSMBConfig from '@opentiny/vue-design-smb'
import { router } from '@/router'
import { appData } from './appData'
import { THEME_ROUTE_MAP, CURRENT_THEME_KEY, DEFAULT_THEME, AURORA_THEME, SMB_THEME, INFINITY_THEME } from '../const'
import { CURRENT_THEME_KEY, DEFAULT_THEME, AURORA_THEME, OLD_THEME, themeToolValuesMap } from '../const'
import glaciers from '@/assets/images/glaciers.png'
import glaciersIcon from '@/assets/images/glaciers-icon.png'

import infinitely from '@/assets/images/Infinitely.png'
import infinitelyIcon from '@/assets/images/Infinitely-icon.png'

import oceanic from '@/assets/images/oceanic.png'
import oceanicIcon from '@/assets/images/oceanic-icon.png'

import starrySky from '@/assets/images/starry-sky.png'
import starrySkyIcon from '@/assets/images/starry-sky-icon.png'
import TinyThemeTool, { tinyOldTheme, tinyAuroraTheme } from '@opentiny/vue-theme/theme-tool'
import TinyThemeTool from '@opentiny/vue-theme/theme-tool'

const isEn = appData.lang === 'enUS'

const themeData = [
{
value: [DEFAULT_THEME],
label: isEn ? 'Default Theme' : '冰川主题',
tips: isEn ? 'Accurate, Efficient, Distinct' : '精准、高效、清晰',
icon: glaciersIcon,
bgImage: glaciers
},
{
value: [SMB_THEME],
label: isEn ? 'Star Theme' : '星空主题',
tips: isEn ? 'Leading, Innovative, Reliable' : '领先、创新、信赖',
icon: starrySkyIcon,
bgImage: starrySky
},
{
value: [OLD_THEME],
label: isEn ? 'Default Theme' : '冰川主题',
tips: isEn ? 'Accurate, Efficient, Distinct' : '精准、高效、清晰',
icon: glaciersIcon,
bgImage: glaciers
},
Comment on lines +28 to +34
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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

{
value: [AURORA_THEME],
label: isEn ? 'Ocean Theme' : '海洋主题',
tips: isEn ? 'Simple, Agile, Delightful' : '简约、敏捷、愉悦',
icon: oceanicIcon,
bgImage: oceanic
},
{
value: [INFINITY_THEME],
label: isEn ? 'Infinity Theme' : '无限主题',
tips: isEn ? 'Creative, Scientific, Efficient' : '创造、科学、高效',
icon: infinitelyIcon,
bgImage: infinitely
}
]

const designConfigMap = {
[DEFAULT_THEME]: {},
[INFINITY_THEME]: {},
[AURORA_THEME]: {},
[SMB_THEME]: {}
}

const defaultThemeKey = DEFAULT_THEME
const currentThemeKey = hooks.ref(defaultThemeKey)
const currentThemeKey = hooks.ref(DEFAULT_THEME)

watch(
() => currentThemeKey.value,
Expand All @@ -82,7 +64,7 @@ const designConfig = computed(() => {
const changeTheme = (themeKey) => {
router.push({
params: {
theme: THEME_ROUTE_MAP[themeKey]
theme: themeKey
},
hash: router?.currentRoute.value.hash
})
Expand All @@ -101,14 +83,10 @@ const watchRoute = () => {
watch(
() => router.currentRoute.value.params.theme,
(val) => {
if (!loadedTheme && val === 'old-theme') {
const themeTool = new TinyThemeTool()
themeTool.changeTheme(tinyOldTheme)
loadedTheme = true
}
if (!loadedTheme && val === 'aurora-theme') {
if (!loadedTheme && themeToolValuesMap[val]) {
currentThemeKey.value = val
const themeTool = new TinyThemeTool()
themeTool.changeTheme(tinyAuroraTheme)
themeTool.changeTheme(themeToolValuesMap[val])
Comment on lines +86 to +89
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Review theme loading logic

The theme loading logic has potential issues:

  1. The loadedTheme flag prevents subsequent theme changes
  2. No error handling for invalid theme values
  3. 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.

loadedTheme = true
}
}
Expand All @@ -122,6 +100,6 @@ export default function useTheme() {
changeTheme,
currentThemeKey,
designConfig,
defaultTheme: THEME_ROUTE_MAP[defaultThemeKey]
defaultTheme: DEFAULT_THEME
}
}
2 changes: 1 addition & 1 deletion examples/sites/src/views/components/demo.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ export default defineComponent({
},
openPlayground(demo, open = true) {
const cmpId = router.currentRoute.value.params.cmpId
const tinyTheme = templateModeState.isSaas ? 'saas' : currentThemeKey.value.split('-')[1]
const tinyTheme = templateModeState.isSaas ? 'saas' : currentThemeKey.value.split('-')[0]
const openModeQuery = open ? '' : '&openMode=preview'
// TODO: 目前mf只有Options写法，后续再放开compositon
const url = `${import.meta.env.VITE_PLAYGROUND_URL}?cmpId=${cmpId}&fileName=${demo.codeFiles[0]}&apiMode=${
Expand Down
Loading