-
Notifications
You must be signed in to change notification settings - Fork 276
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.
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
fix(site): the aurora-theme switch function is added to the official website #2604
Conversation
WalkthroughThe pull request introduces updates to theme customization for the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
packages/theme/build/gulp-dist.js (1)
36-36
: Rename concatOldTheme function to better reflect its purposeThe function now handles both old and aurora themes, so its name should be updated accordingly.
Consider renaming to
concatThemes
or similar:- function concatOldTheme(cb) { + function concatThemes(cb) {Remember to update the task reference in
gulp.task('build', ...)
as well.examples/sites/src/tools/useTheme.js (1)
109-113
: Consider improving theme loading implementationWhile the implementation works, there are a few potential improvements:
- The shared
loadedTheme
flag could prevent switching between themes- Error handling for theme loading failures is missing
- The implementation is repetitive with the old-theme block
Consider this refactoring to address these issues:
-let loadedTheme = false +const loadedThemes = new Set() -if (!loadedTheme && val === 'old-theme') { - const themeTool = new TinyThemeTool() - themeTool.changeTheme(tinyOldTheme) - loadedTheme = true -} -if (!loadedTheme && val === 'aurora-theme') { - const themeTool = new TinyThemeTool() - themeTool.changeTheme(tinyAuroraTheme) - loadedTheme = true -} +const themeMap = { + 'old-theme': tinyOldTheme, + 'aurora-theme': tinyAuroraTheme +} + +if (!loadedThemes.has(val) && val in themeMap) { + try { + const themeTool = new TinyThemeTool() + themeTool.changeTheme(themeMap[val]) + loadedThemes.add(val) + } catch (error) { + console.error(`Failed to load theme: ${val}`, error) + } +}examples/sites/demos/pc/webdoc/theme.md (1)
119-121
: Consider enhancing Aurora theme documentationWhile the warning block correctly mentions the Aurora theme availability, consider adding:
- A dedicated section for Aurora theme configuration
- Example usage with specific variables
- Migration guide from other themes
Add a new section after the warning block:
<p> 如果需要切换 <code>Aurora</code> 主题,可以从上面导出 <code>tinyAuroraTheme</code> 的主题。 </p> </div> +### Aurora 主题配置 + +Aurora 主题提供了现代化的设计风格,使用方式如下: + +```ts +import TinyThemeTool, { tinyAuroraTheme } from '@opentiny/vue-theme/theme-tool' + +const themeTool = new TinyThemeTool() +themeTool.changeTheme(tinyAuroraTheme) +``` + +#### 主要特性 + +- 现代化的色彩系统 +- 优化的组件间距 +- 改进的交互反馈 + +#### 从其他主题迁移 + +如果您正在从其他主题迁移到 Aurora 主题,请注意以下变化: +...
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (7)
examples/sites/demos/pc/webdoc/theme.md
(2 hunks)examples/sites/src/tools/useTheme.js
(2 hunks)packages/theme/build/gulp-dist.js
(2 hunks)packages/theme/build/release.js
(1 hunks)packages/theme/src/aurora-theme-index.js
(1 hunks)packages/theme/src/aurora-theme-index.less
(1 hunks)packages/theme/src/theme-tool.js
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/theme/src/aurora-theme-index.less
- packages/theme/src/aurora-theme-index.js
🔇 Additional comments (3)
packages/theme/src/theme-tool.js (1)
2-2
: LGTM: Clean addition of aurora theme support
The changes cleanly integrate the new aurora theme while maintaining the existing theme tool functionality.
Let's verify the aurora theme file exists and has the expected exports:
Also applies to: 4-4
✅ Verification successful
Verified: Aurora theme file exists with correct exports
The aurora theme file exists at packages/theme/src/aurora-theme-index.js
and contains the expected theme exports with all required properties:
- id: 'tiny-aurora-theme'
- name: 'AuroraTheme'
- cnName: '欧若拉主题'
- css: Contains complete theme CSS variables
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify aurora theme file exists and exports
fd -t f "aurora-theme-index.js$"
# Check the theme exports
ast-grep --pattern 'export default { id: $_, name: $_, cnName: $_, css: $_ }'
Length of output: 31491
packages/theme/build/gulp-dist.js (1)
45-46
: LGTM: Clean addition of aurora theme compilation
The aurora theme compilation is correctly integrated into the build process.
examples/sites/src/tools/useTheme.js (1)
19-19
: LGTM: Import statement correctly adds tinyAuroraTheme
The import statement properly includes the new aurora theme alongside existing themes.
// 2.1、读取 aurora-theme-index.js , dist/aurora-theme-index.less, 合并后写入 dist/ aurora-theme-index.js | ||
jsStr = ` | ||
export default { | ||
id: 'tiny-aurora-theme', | ||
name: 'AuroraTheme', | ||
cnName: '欧若拉主题', | ||
css: \`#CSS#\` | ||
} | ||
` | ||
cssStr = fs.readFileSync(path.resolve(root, 'dist/aurora-theme-index.css'), 'utf8') | ||
|
||
jsStr = jsStr.replace('#CSS#', cssStr) | ||
fs.writeFileSync(path.resolve(root, 'src/aurora-theme-index.js'), jsStr) // 供开发时(pnpm site), 可以访问到最新的定制主题变量 | ||
fs.writeFileSync(path.resolve(root, 'dist/aurora-theme-index.js'), jsStr) // 打包发布用 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider refactoring theme processing to reduce duplication
The aurora theme processing block duplicates the logic from the old theme processing. Consider extracting this into a reusable function to improve maintainability.
Here's a suggested refactor:
- // 2、读取 old-theme-index.js , dist/old-theme-index.less, 合并后写入 dist/ old-theme-index.js
- let jsStr = `
- export default {
- id: 'tiny-old-theme',
- name: 'OldTheme',
- cnName: '旧的主题',
- css: \`#CSS#\`
- }
- `
- let cssStr = fs.readFileSync(path.resolve(root, 'dist/old-theme-index.css'), 'utf8')
-
- jsStr = jsStr.replace('#CSS#', cssStr)
- fs.writeFileSync(path.resolve(root, 'src/old-theme-index.js'), jsStr)
- fs.writeFileSync(path.resolve(root, 'dist/old-theme-index.js'), jsStr)
-
- // 2.1、读取 aurora-theme-index.js , dist/aurora-theme-index.less, 合并后写入 dist/ aurora-theme-index.js
- jsStr = `
- export default {
- id: 'tiny-aurora-theme',
- name: 'AuroraTheme',
- cnName: '欧若拉主题',
- css: \`#CSS#\`
- }
- `
- cssStr = fs.readFileSync(path.resolve(root, 'dist/aurora-theme-index.css'), 'utf8')
-
- jsStr = jsStr.replace('#CSS#', cssStr)
- fs.writeFileSync(path.resolve(root, 'src/aurora-theme-index.js'), jsStr)
- fs.writeFileSync(path.resolve(root, 'dist/aurora-theme-index.js'), jsStr)
+ function processTheme(themeConfig) {
+ const { id, name, cnName, filename } = themeConfig;
+ const jsStr = `
+ export default {
+ id: '${id}',
+ name: '${name}',
+ cnName: '${cnName}',
+ css: \`#CSS#\`
+ }
+ `;
+
+ const cssStr = fs.readFileSync(path.resolve(root, `dist/${filename}.css`), 'utf8');
+ const finalJsStr = jsStr.replace('#CSS#', cssStr);
+
+ fs.writeFileSync(path.resolve(root, `src/${filename}.js`), finalJsStr);
+ fs.writeFileSync(path.resolve(root, `dist/${filename}.js`), finalJsStr);
+ }
+
+ // Process themes
+ processTheme({
+ id: 'tiny-old-theme',
+ name: 'OldTheme',
+ cnName: '旧的主题',
+ filename: 'old-theme-index'
+ });
+
+ processTheme({
+ id: 'tiny-aurora-theme',
+ name: 'AuroraTheme',
+ cnName: '欧若拉主题',
+ filename: 'aurora-theme-index'
+ });
Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
packages/theme/src/tooltip/vars.less (1)
73-75
: Consider documenting the spacing token values.To help developers understand the actual spacing values, consider adding comments with the default pixel values of
--tv-space-lg
and--tv-space-xl
.Example:
// 弹框垂直内边距 (default: 12px) --tv-Tooltip-padding-y: var(--tv-space-lg); // 弹框水平内边距 (default: 16px) --tv-Tooltip-padding-x: var(--tv-space-xl);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
packages/theme/src/tooltip/vars.less
(1 hunks)
🔇 Additional comments (1)
packages/theme/src/tooltip/vars.less (1)
73-73
: LGTM! Verify spacing token values.
Good improvement replacing hardcoded pixel values with design system spacing tokens. This enhances theme customization and maintainability.
Let's verify the spacing token definitions:
Also applies to: 75-75
✅ Verification successful
Let me check the value of --tv-space-base
to complete the verification.
Spacing tokens verified and properly defined
The spacing tokens are well-defined across theme files:
--tv-space-lg
=12px
(3 × 4px base)--tv-space-xl
=16px
(4 × 4px base)
These values match the previous hardcoded values, ensuring layout consistency while improving maintainability.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the spacing tokens are defined and their values
# Expected: Find definitions of --tv-space-lg and --tv-space-xl variables
# Search for spacing token definitions
rg --type less "(--tv-space-lg|--tv-space-xl)\s*:" packages/theme/
Length of output: 592
Script:
#!/bin/bash
# Search for --tv-space-base definition
rg --type less "^[^/]*--tv-space-base\s*:" packages/theme/
Length of output: 247
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
Release Notes
New Features
Documentation
Bug Fixes
These updates enhance user experience by providing more customization options and clearer instructions for implementing themes.