feat(refactor/theme): change incorrect css variables to the common va… #1027
Conversation
WalkthroughThis pull request encompasses a comprehensive update to the styling across multiple Vue components within the project. The changes primarily focus on transitioning from the Changes
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (5)
packages/toolbars/collaboration/src/Main.vue (1)
Line range hint
134-134: Continue variable migration for consistencyThere are still some
--ti-lowcode-*variables that should be migrated to maintain consistency with the PR objectives:&:hover { background: var(--ti-lowcode-toolbar-hover-color); // Should use --te-common-* variable } ... border: 1px solid var(--ti-lowcode-toolbar-user-img-border-color); // Should use --te-common-* variableAlso applies to: 171-171
packages/layout/src/DesignToolbars.vue (1)
Line range hint
85-180: Continue variable migration for consistencyThere are still several
--ti-lowcode-*variables that should be migrated to maintain consistency with the PR objectives:border-bottom: 1px solid var(--ti-lowcode-toolbar-border-bottom-color); ... color: var(--ti-lowcode-toolbar-title-color); ... background: var(--ti-lowcode-toolbar-view-active-bg); ... background: var(--ti-lowcode-toolbar-left-icon-bg-hover); ... color: var(--ti-lowcode-toolbar-right-line); ... color: var(--ti-lowcode-toolbar-icon-color);packages/plugins/state/src/CreateRemoteAPI.vue (1)
Line range hint
300-314: Standardize remaining icon hover variablesWhile the base text color has been standardized, there are still two non-standard variables in use:
--ti-lowcode-toolbar-icon-color--ti-lowcode-icon-hover-bgConsider replacing these with appropriate
--te-common-*variables for consistency.packages/plugins/state/src/CreateVariable.vue (2)
Line range hint
486-486: Replace remaining--ti-lowcode-*variables with common theme variablesThere are still several instances of the old
--ti-lowcode-toolbar-icon-colorvariable in use. According to the PR objectives, these should be replaced with the corresponding common theme variables.Consider replacing with appropriate
--te-common-*variables, for example:- color: var(--ti-lowcode-toolbar-icon-color); + color: var(--te-common-text-secondary);Also applies to: 492-492, 500-500
Line range hint
527-533: Replace advanced settings specific variables with common theme variablesThe advanced settings section is still using module-specific variables which should be replaced with common theme variables according to the PR objectives.
Consider replacing with appropriate common variables:
.show-advanced { font-size: 12px; - color: var(--ti-lowcode-data-advanced-text-color); + color: var(--te-common-text-secondary); &:hover { - color: var(--ti-lowcode-data-advanced-text-hover-color); + color: var(--te-common-link-hover); cursor: pointer; } }
🧹 Nitpick comments (5)
packages/plugins/i18n/src/Main.vue (1)
Line range hint
447-447: Consider updating remaining--ti-lowcode-*variables.For complete consistency with the PR objectives, consider updating these remaining non-standard variables:
- --ti-lowcode-i18n-loading-svg-color - --ti-lowcode-i18n-loading-text-color - --ti-lowcode-i18n-button-color - --ti-lowcode-i18n-border-color + --te-common-text-secondary + --te-common-text-secondary + --te-common-text-secondary + --te-common-border-defaultAlso applies to: 471-471, 484-484, 489-489
packages/toolbars/generate-code/src/FileSelector.vue (3)
123-137: Consider removing!importantflags from fill properties.The use of
!importantflags on lines 133 and 136 might indicate CSS specificity issues. Consider investigating why these are needed and refactor the CSS to avoid them if possible.- fill: var(--te-common-text-secondary) !important; + fill: var(--te-common-text-secondary); - fill: var(--te-common-text-primary) !important; + fill: var(--te-common-text-primary);
175-179: Review semantic naming of checkbox icon color variable.Using
--te-common-border-checkedfor the checkbox icon color (line 178) seems semantically incorrect. Consider using a more appropriate variable like--te-common-icon-checkedor--te-common-text-primaryif available in the design system.
Line range hint
123-221: Overall theme migration looks good with minor improvements needed.The migration from
--ti-lowcode-*to--te-common-*variables is consistent and aligns well with the PR objectives. The changes maintain visual hierarchy and component styling while standardizing the theme variables. Consider the suggested improvements regarding:
- CSS specificity (removal of
!important)- Semantic variable naming for checkboxes
- Primary button styling verification
packages/plugins/state/src/DataSourceList.vue (1)
Line range hint
1-205: Consider consolidating hover/active state stylesThe component's styling structure could be improved by:
- Grouping interactive state styles (hover, selected) together
- Using CSS custom properties for shared values
- Following a consistent pattern for spacing and layout values
This would make the styling more maintainable and aligned with the new theming system.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (52)
packages/canvas/common/src/utils.js(1 hunks)packages/common/component/BlockLinkEvent.vue(1 hunks)packages/common/component/ConfigCollapse.vue(1 hunks)packages/common/component/ConfigItem.vue(1 hunks)packages/common/component/EmptyTip.vue(2 hunks)packages/common/component/MetaCodeEditor.vue(1 hunks)packages/common/component/MetaDescription.vue(1 hunks)packages/common/component/MetaListActions.vue(1 hunks)packages/common/component/MetaListItem.vue(2 hunks)packages/common/component/MetaModal.vue(1 hunks)packages/common/component/MetaModalItem.vue(1 hunks)packages/common/component/MonacoEditor.vue(1 hunks)packages/common/component/PluginBlockList.vue(3 hunks)packages/common/component/PluginSetting.vue(1 hunks)packages/common/component/ToolbarBase.vue(1 hunks)packages/common/component/VideoGuide.vue(1 hunks)packages/configurator/src/tabs-group-configurator/TabsGroupConfigurator.vue(1 hunks)packages/design-core/src/preview/src/Toolbar.vue(1 hunks)packages/layout/src/DesignPlugins.vue(3 hunks)packages/layout/src/DesignToolbars.vue(2 hunks)packages/layout/src/ToolbarCollapse.vue(1 hunks)packages/plugins/block/src/BlockConfig.vue(1 hunks)packages/plugins/block/src/BlockEventList.vue(1 hunks)packages/plugins/block/src/BlockPropertyForm.vue(2 hunks)packages/plugins/block/src/BlockPropertyList.vue(1 hunks)packages/plugins/block/src/Main.vue(3 hunks)packages/plugins/block/src/SaveNewBlock.vue(1 hunks)packages/plugins/bridge/src/BridgeSetting.vue(2 hunks)packages/plugins/i18n/src/Main.vue(3 hunks)packages/plugins/materials/src/meta/block/src/BlockDetail.vue(4 hunks)packages/plugins/materials/src/meta/block/src/BlockGroup.vue(1 hunks)packages/plugins/materials/src/meta/block/src/BlockGroupArrange.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/BlockList.vue(0 hunks)packages/plugins/materials/src/meta/component/src/Main.vue(2 hunks)packages/plugins/page/src/PageInputOutput.vue(1 hunks)packages/plugins/page/src/PageSetting.vue(0 hunks)packages/plugins/schema/src/Main.vue(2 hunks)packages/plugins/script/src/Main.vue(1 hunks)packages/plugins/state/src/CreateRemoteAPI.vue(3 hunks)packages/plugins/state/src/CreateStore.vue(1 hunks)packages/plugins/state/src/CreateVariable.vue(1 hunks)packages/plugins/state/src/DataSourceList.vue(2 hunks)packages/plugins/state/src/Main.vue(2 hunks)packages/plugins/tree/src/Main.vue(1 hunks)packages/plugins/tutorial/src/TutorialVideoPanel.vue(1 hunks)packages/toolbars/collaboration/src/Main.vue(2 hunks)packages/toolbars/generate-code/src/FileSelector.vue(4 hunks)packages/toolbars/logo/src/Main.vue(1 hunks)packages/toolbars/logout/src/Main.vue(1 hunks)packages/toolbars/media/src/Main.vue(2 hunks)
💤 Files with no reviewable changes (2)
- packages/plugins/page/src/PageSetting.vue
- packages/plugins/materials/src/meta/block/src/BlockList.vue
✅ Files skipped from review due to trivial changes (41)
- packages/common/component/MetaListActions.vue
- packages/design-core/src/preview/src/Toolbar.vue
- packages/common/component/MetaModal.vue
- packages/common/component/VideoGuide.vue
- packages/layout/src/ToolbarCollapse.vue
- packages/plugins/schema/src/Main.vue
- packages/plugins/block/src/BlockPropertyList.vue
- packages/common/component/MetaModalItem.vue
- packages/plugins/block/src/BlockConfig.vue
- packages/plugins/materials/src/meta/block/src/BlockGroup.vue
- packages/common/component/MetaCodeEditor.vue
- packages/plugins/materials/src/meta/block/src/BlockGroupTransferPanel.vue
- packages/plugins/state/src/CreateStore.vue
- packages/common/component/MonacoEditor.vue
- packages/plugins/script/src/Main.vue
- packages/common/component/PluginSetting.vue
- packages/plugins/materials/src/meta/block/src/BlockGroupArrange.vue
- packages/common/component/ToolbarBase.vue
- packages/common/component/ConfigItem.vue
- packages/plugins/materials/src/meta/block/src/BlockGroupTransfer.vue
- packages/toolbars/media/src/Main.vue
- packages/plugins/tutorial/src/TutorialVideoPanel.vue
- packages/plugins/materials/src/meta/component/src/Main.vue
- packages/plugins/bridge/src/BridgeSetting.vue
- packages/plugins/block/src/BlockEventList.vue
- packages/plugins/block/src/BlockPropertyForm.vue
- packages/toolbars/logo/src/Main.vue
- packages/plugins/block/src/Main.vue
- packages/common/component/BlockLinkEvent.vue
- packages/plugins/page/src/PageInputOutput.vue
- packages/plugins/block/src/SaveNewBlock.vue
- packages/common/component/MetaListItem.vue
- packages/configurator/src/tabs-group-configurator/TabsGroupConfigurator.vue
- packages/canvas/common/src/utils.js
- packages/common/component/MetaDescription.vue
- packages/plugins/tree/src/Main.vue
- packages/plugins/state/src/Main.vue
- packages/plugins/materials/src/meta/block/src/BlockDetail.vue
- packages/layout/src/DesignPlugins.vue
- packages/common/component/EmptyTip.vue
- packages/common/component/PluginBlockList.vue
🔇 Additional comments (15)
packages/toolbars/collaboration/src/Main.vue (1)
123-123: LGTM! Good progress on variable migration.The changes from
--ti-lowcode-*to--te-common-*variables align well with the PR objectives:
--ti-lowcode-toolbar-breadcrumb-color→--te-common-text-secondary--ti-lowcode-dialog-font-color→--te-common-text-primaryAlso applies to: 145-145
packages/layout/src/DesignToolbars.vue (1)
82-82: LGTM! Good progress on variable migration.The changes from
--ti-lowcode-*to--te-common-*variables align well with the PR objectives:
--ti-lowcode-common-layout-bg→--te-common-bg-default--ti-lowcode-common-primary-color→--te-common-text-checkedAlso applies to: 140-140
packages/common/component/ConfigCollapse.vue (1)
43-46: LGTM! Consistent use of color variables.The hover state colors for both text and SVG icon now use the standardized
--te-common-icon-primaryvariable, ensuring visual consistency.packages/plugins/i18n/src/Main.vue (3)
510-510: LGTM! Appropriate use of secondary text color.The stroke color now uses the standardized
--te-common-text-secondaryvariable, which is semantically correct for this UI element.
520-525: LGTM! Consistent text color usage.Both the download button and text button now use
--te-common-text-primary, maintaining consistency in primary text styling.
561-561: LGTM! Semantic color variable for warning state.The warning icon now uses the standardized
--te-common-color-warningvariable, maintaining the semantic meaning while adhering to the new variable naming convention.packages/toolbars/generate-code/src/FileSelector.vue (2)
142-145: Verify primary button design specification.The removal of the border on the primary button (line 144) might affect its visibility on certain backgrounds. Please verify if this aligns with the design system's button specifications.
220-221: LGTM! Consistent use of secondary text color.The empty state text color properly uses the secondary text variable, maintaining consistency with other text elements in the component.
packages/toolbars/logout/src/Main.vue (2)
25-25: LGTM! Variable change aligns with new design system.The change from
--ti-lowcode-toolbar-breadcrumb-colorto--te-common-text-secondaryfollows the PR objectives of standardizing theme variables.
25-25: Verify theme variable compatibility and accessibility.Please ensure that:
--te-common-text-secondaryis defined in both light and dark themes- The color contrast meets WCAG accessibility standards for text readability
Let's verify the variable definition across theme files:
✅ Verification successful
Theme variable implementation verified and approved.
The
--te-common-text-secondaryvariable is properly defined in both themes with appropriate color values (#595959 for light theme, #b4b5b8 for dark theme) that follow standard accessibility patterns for secondary text.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the variable definition in theme files rg --type=less "te-common-text-secondary" # Check if the variable is defined in both light and dark themes fd -e less -e css "base.less|common.less" theme/Length of output: 3572
Script:
#!/bin/bash # Search for the base gray color definitions rg --type=less "te-base-gray-(70|110)" -B 1 -A 1Length of output: 3105
packages/plugins/state/src/CreateRemoteAPI.vue (4)
193-195: LGTM: Good variable standardizationThe change from
--ti-lowcode-error-tip-colorto--te-common-color-erroraligns with the standardization effort and uses a more semantic variable name.
249-255: LGTM: Consistent variable usageGood use of standardized variables for both secondary text and error colors, maintaining consistency throughout the component.
260-260: LGTM: Consistent border stylingGood use of the standardized border color variable, maintaining consistency with other borders in the component.
Line range hint
192-320: Complete the variable standardization effortWhile most CSS variables have been successfully standardized to the
--te-common-*prefix, there are still three non-standard variables that need to be addressed:
--ti-lowcode-toolbar-bg--ti-lowcode-toolbar-icon-color--ti-lowcode-icon-hover-bgTo fully align with the PR objectives of eliminating non-standard variables, these should be replaced with appropriate
--te-common-*alternatives.Let's verify if these variables are defined or used elsewhere in the codebase:
packages/plugins/state/src/CreateVariable.vue (1)
Line range hint
476-533: Ensure consistent usage of theme variables throughout the fileWhile some variables have been updated to use the new
--te-common-*prefix (like--te-common-bg-containerand--te-common-text-emphasize), others still use the old--ti-lowcode-*prefix. This inconsistency should be addressed to fully align with the PR objectives of standardizing theme variables.Let's verify if these variables are used consistently across other files:
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
此pr做的事情：
整体目的
剩余要做的事情：
举例：
1.直接改成common变量，等待之后调整
2.一步到位，看是哪一个模块的，然后新增模块变量，模块变量的值是common变量，然后vue文件使用的是模块变量。
注意点：需要看一下暗色主题
注意：对于一些分割线，使用的变量都是--te-common-border-divider，不要混用 --te-common-border-default了。可能需要全量排查一遍，可以排查分别设置 border-top，border-left, border-right，border-bottom的地方，看看有没有类似的分割线场景
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
Based on the comprehensive review of the provided changes, here's a concise summary of the release notes:
Style Updates
Design System Migration
--ti-lowcode-*to--te-common-*color variablesUser Interface Refinements
No functional changes were introduced, and the core logic of components remains unchanged.