fix(theme): Fixed an issue where some global styles become invalid when a theme is switched using an alias. #2395
Conversation
WalkthroughThe changes in this pull request involve modifications to the Changes
Possibly related PRs
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
packages/theme/build/edit‐dir‐theme.js (1)
235-241: Add documentation explaining the theme structure and import order.The order of CSS imports is crucial for proper cascading. Please add comments explaining:
- Why these specific imports are required
- The significance of their order
- The relationship to theme switching functionality
Add a comment block above the replacement:
+ // These imports are required to maintain global styles during theme switching: + // 1. reset.less: Provides base CSS reset + // 2. transition.less: Ensures animations persist across theme changes + // 3. svg/index.less: Maintains SVG styles during theme switches newDataStr = baseContent.replace('}', newDataStr).replace(
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- packages/theme/build/edit‐dir‐theme.js (1 hunks)
- packages/theme/package.json (2 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/theme/package.json
🔇 Additional comments (1)
packages/theme/build/edit‐dir‐theme.js (1)
235-241: 🛠️ Refactor suggestionConsider making the import paths configurable and add error handling.
While the changes address the theme switching issue by ensuring global styles are properly imported, there are a few areas for improvement:
- The import paths are hardcoded, which could make maintenance difficult.
- There's no validation that the imported files exist.
Consider refactoring to:
+ const THEME_IMPORTS = [ + '../../base/reset.less', + '../../base/transition.less', + '../../svg/index.less' + ]; newDataStr = baseContent.replace('}', newDataStr).replace( ':root', - `@import '../../base/reset.less'; -@import '../../base/transition.less'; -@import '../../svg/index.less'; -:root` + THEME_IMPORTS.map(path => `@import '${path}';`).join('\n') + '\n:root' )Let's verify the existence of these imported files:
✅ Verification successful
Based on the output, I can now provide the final response since I have verified that:
All the required files exist in the repository:
packages/theme/src/base/reset.lesspackages/theme/src/base/transition.lesspackages/theme/src/svg/index.lessThese files are properly referenced in multiple theme files, including:
packages/theme/src/index.lesspackages/theme/src/base/index.lesspackages/theme-saas/src/index.lessThe import paths in the code are correct and valid.
The relative paths used in the code (
../../base/reset.less,../../base/transition.less,../../svg/index.less) are correctly pointing to the existing files in the theme package, following the repository's structure. The imports are also consistently used across different theme implementations, indicating this is the established pattern in the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all required theme files exist echo "Checking for required theme files..." for file in "base/reset.less" "base/transition.less" "svg/index.less"; do fd -t f "^${file}$" src/ doneLength of output: 683
Script:
#!/bin/bash # Let's first understand the repository structure fd -t d --max-depth 2 # Then search for the less files in the entire repository fd -t f -e less # Also check if there are any references to these files in the codebase rg -l "reset\.less|transition\.less|svg/index\.less"Length of output: 31846
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
reset.less,transition.less, andsvg/index.less.Chores
@opentiny/vue-themepackage from3.18.3to3.18.4.