feat(theme): change incorrect css variables and delete the ‘less’ file #1121
feat(theme): change incorrect css variables and delete the ‘less’ file #1121hexqi merged 8 commits intoopentiny:refactor/developfrom
Conversation
WalkthroughThis pull request refines the CSS styles and variable usages across multiple components. It updates CSS variable names in several Vue components, replaces variable references with fixed values in certain cases, and removes numerous legacy and redundant Less files for both dark and light themes. The changes focus on the styling layers, ensuring design consistency and eliminating outdated configurations. Changes
Sequence Diagram(s)Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (1)
✨ Finishing Touches
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 (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/settings/design/src/components/PropertyList.vue (1)
97-97:Details
❓ Verification inconclusive
Verify the new CSS variable definitions and update documentation.
The CSS variables have been updated to use the new
te-setting-designprefix:
--te-setting-design-secondary-text-color--te-setting-design-hover-bg-colorLet's verify these variables are properly defined:
Consider adding theme migration documentation if this change affects custom themes. This helps users understand:
- The rationale behind the variable name changes
- How to update their custom themes
- The complete list of renamed variables
Would you like me to help draft the theme migration documentation?
Also applies to: 119-119
🏁 Script executed:
#!/bin/bash # Search for the definition of new CSS variables rg --type css --type less "te-setting-design-(secondary-text|hover-bg)-color" # Search for any remaining old variable names to ensure complete migration rg --type css --type less "ti-lowcode-common-(secondary-text|hover-bg)-color"Length of output: 159
Action Required: Verify CSS Variable Definitions and Update Theme Documentation
The changes in lines 97 and 119 update the CSS variables to the new
te-setting-designprefix. However, our automated search did not return any definitions for these variables. Please manually verify that:
The new variables:
--te-setting-design-secondary-text-color(line 97)--te-setting-design-hover-bg-color(line 119)are defined in the codebase or imported via external theme/style files.
There are no remaining references to the old variable names (with the
ti-lowcode-commonprefix) that might cause inconsistencies.Additionally, if the migration to these new variables impacts custom themes, consider updating the theme migration documentation to clearly communicate the changes and provide guidance for users updating their custom themes.
packages/theme/base/src/base.less (1)
321-324: Enhance documentation for the new "基础配置" section.For consistency with other sections in the file, consider:
- Adding English translation alongside Chinese comments
- Including more detailed documentation about the purpose and usage of these base configurations
Apply this diff to improve documentation:
/** - * 基础配置 + * 基础配置 (Base Configuration) + * Contains fundamental layout settings used across components + * @property --te-base-min-width Minimum width for the application layout **/
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (50)
packages/layout/src/Main.vue(1 hunks)packages/plugins/tree/src/styles/vars.less(1 hunks)packages/settings/design/src/App.vue(3 hunks)packages/settings/design/src/components/ArrayConfigItemForm.vue(1 hunks)packages/settings/design/src/components/PropertyCanvas.vue(1 hunks)packages/settings/design/src/components/PropertyList.vue(2 hunks)packages/settings/design/src/components/SettingPanel.vue(0 hunks)packages/settings/styles/src/components/inputs/ModalMask.vue(1 hunks)packages/settings/styles/src/components/typography/TypographyGroup.vue(0 hunks)packages/theme/base/src/base.less(1 hunks)packages/theme/dark/block.less(0 hunks)packages/theme/dark/bridge.less(0 hunks)packages/theme/dark/canvas.less(0 hunks)packages/theme/dark/datasource.less(0 hunks)packages/theme/dark/events.less(0 hunks)packages/theme/dark/gpt-dialog.less(0 hunks)packages/theme/dark/help.less(0 hunks)packages/theme/dark/i18n.less(0 hunks)packages/theme/dark/index.less(0 hunks)packages/theme/dark/life-cycles.less(0 hunks)packages/theme/dark/materials.less(0 hunks)packages/theme/dark/metaComponent.less(0 hunks)packages/theme/dark/pageManage.less(0 hunks)packages/theme/dark/plugin-js.less(0 hunks)packages/theme/dark/setting-style.less(0 hunks)packages/theme/dark/settings.less(0 hunks)packages/theme/dark/toolbar.less(0 hunks)packages/theme/dark/tree.less(0 hunks)packages/theme/dark/tutorial.less(0 hunks)packages/theme/dark/variable.less(0 hunks)packages/theme/light/block.less(0 hunks)packages/theme/light/bridge.less(0 hunks)packages/theme/light/canvas.less(0 hunks)packages/theme/light/datasource.less(0 hunks)packages/theme/light/events.less(0 hunks)packages/theme/light/gpt-dialog.less(0 hunks)packages/theme/light/help.less(0 hunks)packages/theme/light/i18n.less(0 hunks)packages/theme/light/index.less(0 hunks)packages/theme/light/life-cycles.less(0 hunks)packages/theme/light/materials.less(0 hunks)packages/theme/light/metaComponent.less(0 hunks)packages/theme/light/pageManage.less(0 hunks)packages/theme/light/plugin-js.less(0 hunks)packages/theme/light/setting-style.less(0 hunks)packages/theme/light/settings.less(0 hunks)packages/theme/light/toolbar.less(0 hunks)packages/theme/light/tree.less(0 hunks)packages/theme/light/tutorial.less(0 hunks)packages/theme/light/variable.less(0 hunks)
💤 Files with no reviewable changes (42)
- packages/theme/light/plugin-js.less
- packages/theme/light/tree.less
- packages/theme/light/help.less
- packages/theme/light/bridge.less
- packages/theme/dark/settings.less
- packages/theme/dark/canvas.less
- packages/theme/dark/plugin-js.less
- packages/theme/dark/toolbar.less
- packages/theme/dark/block.less
- packages/settings/styles/src/components/typography/TypographyGroup.vue
- packages/theme/dark/tree.less
- packages/theme/dark/help.less
- packages/theme/light/settings.less
- packages/theme/dark/i18n.less
- packages/theme/light/block.less
- packages/theme/dark/materials.less
- packages/theme/light/metaComponent.less
- packages/theme/light/datasource.less
- packages/theme/dark/setting-style.less
- packages/theme/dark/bridge.less
- packages/theme/light/toolbar.less
- packages/theme/light/life-cycles.less
- packages/theme/dark/events.less
- packages/theme/dark/datasource.less
- packages/theme/light/tutorial.less
- packages/theme/dark/tutorial.less
- packages/theme/dark/gpt-dialog.less
- packages/theme/light/events.less
- packages/theme/dark/metaComponent.less
- packages/theme/light/i18n.less
- packages/theme/dark/life-cycles.less
- packages/theme/dark/pageManage.less
- packages/theme/dark/variable.less
- packages/theme/light/pageManage.less
- packages/theme/light/setting-style.less
- packages/theme/light/variable.less
- packages/settings/design/src/components/SettingPanel.vue
- packages/theme/light/index.less
- packages/theme/light/materials.less
- packages/theme/light/canvas.less
- packages/theme/dark/index.less
- packages/theme/light/gpt-dialog.less
✅ Files skipped from review due to trivial changes (6)
- packages/layout/src/Main.vue
- packages/plugins/tree/src/styles/vars.less
- packages/settings/design/src/components/ArrayConfigItemForm.vue
- packages/settings/design/src/App.vue
- packages/settings/styles/src/components/inputs/ModalMask.vue
- packages/settings/design/src/components/PropertyCanvas.vue
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (1)
packages/theme/base/src/base.less (1)
324-324:Details
❓ Verification inconclusive
Consider responsive design implications of fixed min-width.
While
1280pxis a common desktop breakpoint, a fixed minimum width might cause issues on smaller screens. Consider:
- Using a responsive approach with media queries
- Documenting the rationale for this specific value
Let's check if this value is causing any responsive design issues:
🏁 Script executed:
#!/bin/bash # Search for any existing responsive design patterns or media queries rg -i "media|breakpoint|responsive" --type cssLength of output: 8264
Responsive Breakpoint Evaluation for Fixed Min-Width
In
packages/theme/base/src/base.lessat line 324, the variable--te-base-min-width: 1280px;is set as a fixed value. Our search shows that the codebase employs several media queries (e.g., inmockServer/src/assets/css/0.1.20/index.css) using various breakpoints. Please verify that a fixed min-width of 1280px is fully aligned with your overall responsive design strategy. If this value is an intentional design decision for desktop layouts, consider adding a comment that documents the rationale. Otherwise, refactoring to incorporate a responsive approach—potentially by leveraging media queries—might help avoid issues on smaller screens.
d00f6b6 to
d0f7b36
Compare
936ce66 to
3e2c954
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/build/vite-config/src/vite-plugins/devAliasPlugin.js (1)
53-53: Consider documenting theme migration strategy.With the removal of dark and light themes, and only the base theme remaining, it would be helpful to document:
- How theme customization should now be handled
- Migration steps for projects using the removed themes
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
designer-demo/package.json(1 hunks)packages/build/vite-config/src/vite-plugins/devAliasPlugin.js(1 hunks)packages/engine-cli/src/commands/generateConfig.js(0 hunks)
💤 Files with no reviewable changes (1)
- packages/engine-cli/src/commands/generateConfig.js
🚧 Files skipped from review as they are similar to previous changes (1)
- designer-demo/package.json
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (1)
packages/build/vite-config/src/vite-plugins/devAliasPlugin.js (1)
83-83:Details
✅ Verification successful
Verify theme-related dependencies after alias removal.
The removal of theme-related aliases suggests a significant change in the theming approach. Please ensure that:
- All theme-related imports in the codebase have been updated
- No components are still trying to import the removed theme packages
🏁 Script executed:
#!/bin/bash # Description: Search for any remaining references to the removed theme packages echo "Searching for theme package imports..." rg -l '@opentiny/tiny-engine-theme-dark|@opentiny/tiny-engine-theme-light' echo "Checking package.json files for theme dependencies..." find . -name 'package.json' -exec grep -l '"@opentiny/tiny-engine-theme-\(dark\|light\)"' {} \;Length of output: 379
Theme removal verified and approved.
The alias refactor in
packages/build/vite-config/src/vite-plugins/devAliasPlugin.js(line 83) cleanly removes theme-related mappings. Our search found no lingering imports or dependency declarations for@opentiny/tiny-engine-theme-darkand@opentiny/tiny-engine-theme-lightin the codebase or in anypackage.jsonfiles. This confirms that the intended architectural changes to remove theme functionality have been successfully applied.
- Confirmed removal of theme aliases and related code.
- No residual dependencies or imports were detected.
English | 简体中文
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Background and solution
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