feat: variable convergence #1087
Conversation
|
Warning Rate limit exceeded@xuanlid has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 35 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
WalkthroughThis pull request introduces a comprehensive styling update across multiple plugins, focusing on standardizing color variables and improving visual consistency. The changes involve importing new LESS stylesheets ( Changes
Possibly related PRs
Suggested Reviewers
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 (
|
github-actions
bot
added
enhancement
![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
🧹 Nitpick comments (6)
packages/plugins/materials/src/styles/vars.less (1)
1-44: Well-organized variable definitions with clear component grouping!The variables are well-structured and properly reference common theme variables. The organization by component type (list, detail, group, etc.) makes it easy to maintain.
A few suggestions to improve consistency:
- Consider adding a comment header describing the purpose of this file.
- Consider grouping related variables using CSS custom property naming patterns, e.g.:
// 区块详情 - --te-materials-block-detail-bg-color: var(--te-common-bg-default); - --te-materials-block-detail-text-color: var(--te-common-text-primary); + --te-materials-block-detail-color-bg: var(--te-common-bg-default); + --te-materials-block-detail-color-text: var(--te-common-text-primary);packages/plugins/page/src/styles/vars.less (3)
7-9: Consider revising these variable mappings for better semantics.
- The
tip-text-dim-colormaps to--te-common-text-primary, which seems inconsistent with its "dim" designation. Consider mapping it to--te-common-text-weakeninstead.- Both tree-related background colors map to the same value. Consider consolidating them into a single variable to reduce redundancy.
- --te-page-manage-tip-text-dim-color: var(--te-common-text-primary); - --te-page-manage-tree-text-background-color: var(--te-common-bg-default); - --te-page-manage-tree-node-background-color: var(--te-common-bg-default); + --te-page-manage-tip-text-dim-color: var(--te-common-text-weaken); + --te-page-manage-tree-background-color: var(--te-common-bg-default);
1-23: Improve organization with semantic grouping and documentation.Consider reorganizing the variables into logical groups with comments for better maintainability. Here's a suggested structure:
:root { + /* General */ --te-page-manage-text-color: var(--te-common-text-secondary); --te-page-manage-icon-color: var(--te-common-icon-secondary); + + /* Input Fields */ --te-page-manage-input-background-color: var(--te-common-bg-default); --te-page-manage-input-border-color: var(--te-common-border-default); --te-page-manage-input-head-text-color: var(--te-common-text-primary); --te-page-manage-input-or-output-text-color: var(--te-common-text-secondary); + + /* Tree Component */ --te-page-manage-tree-node-background-color: var(--te-common-bg-default); --te-page-manage-tree-node-hover-background-color: var(--te-common-bg-container); + /* ... continue for other components */ }
1-23: Great job implementing a robust color system!The implementation successfully:
- Converts all colors to module variables
- Maintains proper variable flow from common to module-specific variables
- Provides good abstraction for theme customization
This architecture will make it easier to:
- Implement dark mode or other theme variations
- Maintain consistency across the application
- Make global color scheme updates
packages/plugins/script/src/Main.vue (1)
150-150: Consider renaming the dot indicator variable.The variable name
--te-plugin-js-dot-coloris somewhat generic. Consider a more semantic name that reflects its purpose as an unsaved changes indicator, such as--te-plugin-js-unsaved-indicator-color.packages/plugins/script/src/styles/vars.less (1)
1-9: Consider grouping and documenting variables.While the variables follow the correct naming pattern, consider:
- Grouping related variables (e.g., panel-related, border-related)
- Adding comments for all variables, not just the panel title
- Documenting the purpose of each variable and when to use them
Example structure:
:root { // Panel appearance --te-plugin-js-panel-bg-color: var(--te-common-bg-default); --te-plugin-js-panel-shadow-color: var(--te-common-shadow-panel); --te-plugin-js-panel-title-color: var(--te-common-text-primary); // Borders and dividers --te-plugin-js-common-border-color: var(--te-common-border-divider); // Status indicators --te-plugin-js-dot-color: var(--te-common-color-error); // Indicates unsaved changes }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
packages/plugins/materials/index.js(1 hunks)packages/plugins/materials/src/meta/block/src/BlockDetail.vue(3 hunks)packages/plugins/materials/src/meta/block/src/BlockGroup.vue(3 hunks)packages/plugins/materials/src/meta/block/src/BlockGroupArrange.vue(2 hunks)packages/plugins/materials/src/meta/block/src/BlockGroupFilters.vue(3 hunks)packages/plugins/materials/src/meta/block/src/BlockGroupSort.vue(1 hunks)packages/plugins/materials/src/meta/block/src/BlockGroupTransfer.vue(1 hunks)packages/plugins/materials/src/meta/block/src/BlockGroupTransferPanel.vue(1 hunks)packages/plugins/materials/src/meta/block/src/BlockPanel.vue(1 hunks)packages/plugins/materials/src/meta/block/src/BlockVersionSelect.vue(1 hunks)packages/plugins/materials/src/meta/component/src/Main.vue(2 hunks)packages/plugins/materials/src/styles/vars.less(1 hunks)packages/plugins/page/index.js(1 hunks)packages/plugins/page/src/PageGeneral.vue(2 hunks)packages/plugins/page/src/PageHome.vue(1 hunks)packages/plugins/page/src/PageInputOutput.vue(3 hunks)packages/plugins/page/src/PageTree.vue(3 hunks)packages/plugins/page/src/Tree.vue(3 hunks)packages/plugins/page/src/styles/vars.less(1 hunks)packages/plugins/schema/index.js(1 hunks)packages/plugins/schema/src/Main.vue(4 hunks)packages/plugins/schema/src/styles/vars.less(1 hunks)packages/plugins/script/index.js(1 hunks)packages/plugins/script/src/Main.vue(4 hunks)packages/plugins/script/src/styles/vars.less(1 hunks)
✅ Files skipped from review due to trivial changes (19)
- packages/plugins/page/index.js
- packages/plugins/materials/index.js
- packages/plugins/schema/index.js
- packages/plugins/materials/src/meta/block/src/BlockGroupSort.vue
- packages/plugins/materials/src/meta/block/src/BlockGroupTransfer.vue
- packages/plugins/page/src/PageHome.vue
- packages/plugins/materials/src/meta/block/src/BlockPanel.vue
- packages/plugins/materials/src/meta/block/src/BlockVersionSelect.vue
- packages/plugins/materials/src/meta/component/src/Main.vue
- packages/plugins/page/src/PageTree.vue
- packages/plugins/page/src/PageInputOutput.vue
- packages/plugins/page/src/Tree.vue
- packages/plugins/materials/src/meta/block/src/BlockGroupTransferPanel.vue
- packages/plugins/materials/src/meta/block/src/BlockGroupFilters.vue
- packages/plugins/script/index.js
- packages/plugins/schema/src/styles/vars.less
- packages/plugins/materials/src/meta/block/src/BlockDetail.vue
- packages/plugins/page/src/PageGeneral.vue
- packages/plugins/schema/src/Main.vue
🔇 Additional comments (9)
packages/plugins/materials/src/meta/block/src/BlockGroupArrange.vue (1)
77-77: Color variables properly converted to module-specific variables!The icon color states now use module-specific variables, improving maintainability and theme consistency:
- Default:
--te-materials-block-group-arrange-icon-color- Hover:
--te-materials-block-group-arrange-icon-hover-color- Active:
--te-materials-block-group-arrange-icon-active-colorAlso applies to: 86-86, 89-89
packages/plugins/materials/src/meta/block/src/BlockGroup.vue (3)
Line range hint
411-422: Block editing styles properly use module variables!The border and SVG colors now use module-specific variables:
- Border:
--te-materials-block-group-item-border-color- SVG:
--te-materials-block-group-svg-color
429-429: Consistent use of border color variable!The underline style uses the same module-specific border color variable, maintaining consistency.
469-469: Popper text colors properly use module variables!The popper text colors now use module-specific variables:
- Title:
--te-materials-block-group-popper-title-color- Content:
--te-materials-block-group-popper-content-colorAlso applies to: 475-475
packages/plugins/page/src/styles/vars.less (1)
1-23: LGTM! Well-structured variable naming convention.The variables follow a consistent and clear naming pattern that effectively prevents naming conflicts and clearly indicates their scope and purpose.
packages/plugins/script/src/Main.vue (3)
112-113: LGTM: Panel styling follows the module variable pattern.The background and shadow variables now correctly use the module-specific prefix
--te-plugin-js-*instead of generic variables.
124-124: Consistent border color usage across component.Good consistency in using
--te-plugin-js-common-border-colorfor both the header border and editor container border.Also applies to: 165-165
132-133: Mixed variable usage in title styling.While the color uses the module-specific
--te-plugin-js-panel-title-color, the font weight still uses the base variable--te-base-font-weight-bold. Consider if this is intentional or if font weight should also be module-specific.Let's verify if other plugins follow the same pattern for font weights:
✅ Verification successful
Font weight usage follows codebase convention
The mixed variable usage is intentional and consistent with the rest of the codebase. All plugins use base font-weight variables (
--te-base-font-weight-*) while having module-specific color variables, suggesting this is a deliberate architectural decision to maintain consistent typography while allowing for module-specific colors.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for font weight variable usage across plugins rg --type=vue --type=less "font-weight: var\(--te-(base|plugin)" packages/plugins/Length of output: 112
Script:
#!/bin/bash # Search for font weight variable usage without file type restrictions rg "font-weight: var\(--te-(base|plugin)" packages/plugins/ -A 2 # Search for font weight variable definitions rg "(?:--|@)te-.*font-weight" packages/plugins/Length of output: 1922
packages/plugins/script/src/styles/vars.less (1)
2-4: Verify variable inheritance across themes.These variables inherit from common variables. Let's verify that all referenced common variables are defined across all supported themes to prevent undefined values.
Also applies to: 7-8
✅ Verification successful
Common variables are properly defined across themes ✅
The referenced common variables (
--te-common-shadow-panel,--te-common-bg-default,--te-common-border-divider) are properly defined in both light and dark themes with appropriate values, ensuring consistent theming support.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the referenced common variables in theme files echo "Checking common variables used in vars.less:" common_vars=( "--te-common-shadow-panel" "--te-common-bg-default" "--te-common-border-divider" "--te-common-text-primary" "--te-common-color-error" ) for var in "${common_vars[@]}"; do echo "Checking $var:" rg "$var" packages/theme/ doneLength of output: 3169
Script:
#!/bin/bash # Search for the referenced common variables in theme files echo "Checking common variables used in vars.less:" echo "----------------------------------------" # Use fd to find all CSS and Less files in the theme package fd "\.(css|less)$" packages/theme/ -x rg -A 1 -B 1 \ -e "\\-\\-te-common-shadow-panel" \ -e "\\-\\-te-common-bg-default" \ -e "\\-\\-te-common-border-divider" \ {} || true # Also search in any potential theme configuration files fd "theme|style" packages/theme/ -x rg -A 1 -B 1 \ -e "\\-\\-te-common-shadow-panel" \ -e "\\-\\-te-common-bg-default" \ -e "\\-\\-te-common-border-divider" \ {} || trueLength of output: 13856
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/plugins/state/src/styles/vars.less (1)
1-21: Well-organized CSS variables with consistent naming!The variables are logically grouped and follow a consistent naming convention with the
--te-state-prefix. This promotes maintainability and reusability across the state management components.Consider adding comments to document the purpose and usage of each variable group. For example:
:root { + /* Button styles */ --te-state-add-btn-border-color: var(--te-common-border-default); --te-state-add-btn--hover-border-color: var(--te-common-border-hover); --te-state-add-btn--icon-color: var(--te-common-icon-secondary); + /* Common element styles */ --te-state-common-text-color: var(--te-common-text-primary); /* ... */ }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
packages/plugins/state/index.js(1 hunks)packages/plugins/state/src/CreateStore.vue(2 hunks)packages/plugins/state/src/CreateVariable.vue(3 hunks)packages/plugins/state/src/DataSourceList.vue(5 hunks)packages/plugins/state/src/EditorI18nTool.vue(1 hunks)packages/plugins/state/src/Main.vue(4 hunks)packages/plugins/state/src/StateFullscreenHead.vue(1 hunks)packages/plugins/state/src/StateTips.vue(1 hunks)packages/plugins/state/src/styles/vars.less(1 hunks)
✅ Files skipped from review due to trivial changes (6)
- packages/plugins/state/index.js
- packages/plugins/state/src/StateTips.vue
- packages/plugins/state/src/EditorI18nTool.vue
- packages/plugins/state/src/CreateStore.vue
- packages/plugins/state/src/StateFullscreenHead.vue
- packages/plugins/state/src/DataSourceList.vue
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (5)
packages/plugins/state/src/Main.vue (3)
407-409: LGTM! Button styling uses new state-specific variables.The button border and icon colors now use the state-specific variables, maintaining consistency with the new styling system.
Also applies to: 415-415
427-429: LGTM! Title styling uses new state-specific variables.The title's text color, font weight, and border color now use the state-specific variables.
458-460: LGTM! Panel styling uses new state-specific variables.The panel's shadow, border, background, and text colors now use the state-specific variables.
Also applies to: 473-475
packages/plugins/state/src/CreateVariable.vue (2)
442-443: LGTM! Tips styling uses new state-specific variables.The tips section's background and text colors now use the state-specific variables.
464-464: LGTM! Form elements use new state-specific variables.The form labels and description text colors now use the state-specific variables, maintaining consistency with the new styling system.
Also applies to: 468-468, 484-484
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
主要修改：页面JS，schema，物料（组件/区块），页面管理，状态管理，大纲树
a.检查每个模块插件包，用到的色值全都改成模块变量
b.检查模块变量有没有被非该模块的vue文件使用，修改非该模块的vue文件使用的变量
c.将模块变量回归到各个模块里面
d.模块变量回归模块后，再排查一遍该模块有没有直接使用--te-common的，有的话，需要转成模块变量。去保证整个链路是这样的： base/common less -> module less -> vue文件
common中的变量改为common中的变量，不使用模块变量
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
Styling Updates
--ti-to--te-prefix.New Features
Performance
Compatibility