feat: variable convergence in setting #1096
Conversation
WalkthroughThis pull request updates the styling of various UI components across the events, props, and styles packages. New LESS stylesheet imports are added, and existing CSS variable references are updated with new naming conventions. Additionally, new CSS custom properties have been introduced in dedicated variables files to standardize theming across events, props, and the overall application, including an update to the theme base. Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (39)
🚧 Files skipped from review as they are similar to previous changes (39)
⏰ 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 (
|
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: 2
🧹 Nitpick comments (11)
packages/settings/props/src/styles/vars.less (1)
1-11: Solid Integration of CSS Custom Properties
The new CSS variable definitions in the:rootblock accurately align with the PR objectives. The variables are clearly mapped to the common base variables (e.g.,--te-common-text-primary), ensuring a consistent theming approach across modules.Suggestion: Consider adding a brief comment above the variable declarations to explain their role (e.g., "Module variable definitions for the properties panel styling") for enhanced clarity to future contributors.
packages/settings/styles/src/components/inputs/ModalMask.vue (1)
110-110: Consider updating remaining--ti-prefixed variables for consistency.For better consistency with the new naming convention, consider updating the following variables:
--ti-modal-padding-y--ti-modal-padding-x--modal-right-offset--modal-spaceing(Note: this also has a typo in "spaceing")- padding: var(--ti-modal-padding-y, 14px) var(--ti-modal-padding-x, 20px); + padding: var(--te-styles-modal-padding-y, 14px) var(--te-styles-modal-padding-x, 20px); - right: var(--modal-right-offset, 280px); + right: var(--te-styles-modal-right-offset, 280px); - 100% - var(--modal-right-offset, 287px) - var(--modal-right-offset, 280px) - var(--modal-spaceing, 16px) + 100% - var(--te-styles-modal-right-offset, 287px) - var(--te-styles-modal-right-offset, 280px) - var(--te-styles-modal-spacing, 16px)Also applies to: 113-116
packages/settings/styles/src/components/typography/TypographyGroup.vue (1)
530-694: Consider adding CSS variable documentation.While the variable updates look good, it would be helpful to add documentation about these new CSS variables, their expected values, and their relationships.
Consider adding a comment block at the top of the style section:
<style lang="less" scoped> +/* CSS Variables used: + * --te-styles-common-text-color: Primary text color + * --te-styles-common-text-weaken-color: Secondary, weaker text color + * --te-styles-common-setting-color: Text color for setting state + * --te-styles-common-setting-bg-color: Background color for setting state + */ .typography-wrap {packages/settings/styles/src/components/border/BorderGroup.vue (1)
655-655: LGTM! Well-structured variable organization with semantic naming.The color and background variables have been thoughtfully organized with clear semantic naming that reflects their purpose. The module-specific prefix
--te-styles-is consistently applied.Consider documenting these variables in a central location to help other developers understand the theming system.
Consider adding a comment block at the top of the style section documenting the purpose and expected values of each custom property, or create a separate documentation file for the theming system.
Also applies to: 665-665, 695-695, 721-722
packages/settings/styles/src/components/position/PositionGroup.vue (1)
384-393: Add type checking and error handling in selectInsetValue.The function assumes
item.valueis always defined and can be split. Consider adding validation:const selectInsetValue = (item, index) => { let directionArr = [] + if (!item?.value) { + console.warn('Invalid item value in selectInsetValue') + return + } + directionArr = item.value === 'Full' ? ['0%', '0%', '0%', '0%'] : item.value.split(' ') Object.keys(state.directionText).forEach((key, index) => { state.directionText[key] = directionArr[index] }) state.selectedIndex = index updateStyle({ [POSITION_PROPERTY.Inset]: item.value }) }packages/settings/styles/src/styles/vars.less (1)
1-67: Consider adding documentation for variable usage.To improve maintainability, consider adding:
- A comment block at the top describing the purpose of these variables
- Documentation for each category explaining when to use which variables
packages/settings/styles/src/components/spacing/SpacingGroup.vue (1)
374-374: Consider using CSS variables for text colors.The hardcoded color
#808080for text elements could be replaced with a CSS variable for better theme consistency.- <text x="12" y="4" fill="#808080" font-weight="normal" font-size="10" dominant-baseline="hanging">Padding</text> + <text x="12" y="4" fill="var(--te-styles-common-text-weaken-color)" font-weight="normal" font-size="10" dominant-baseline="hanging">Padding</text>- <text x="12" y="4" fill="#808080" font-weight="normal" font-size="10" dominant-baseline="hanging">Margin</text> + <text x="12" y="4" fill="var(--te-styles-common-text-weaken-color)" font-weight="normal" font-size="10" dominant-baseline="hanging">Margin</text>Also applies to: 382-382
packages/settings/styles/src/components/layout/GridBox.vue (1)
556-558: Settings highlight variables updated.The
.is-settingclass now uses the standardized module variables for both color and background-color properties.However, consider extracting these commonly used combinations of color and background-color into a dedicated CSS class to promote reusability.
.is-setting { color: var(--te-styles-common-setting-color); background-color: var(--te-styles-common-setting-bg-color); } +.settings-highlight { + color: var(--te-styles-common-setting-color); + background-color: var(--te-styles-common-setting-bg-color); +}packages/settings/styles/src/components/spacing/SpacingSetting.vue (1)
115-117: Consider improvements to enhance maintainability.
- Consider extracting magic numbers into CSS variables for better maintainability:
- Component width (236px)
- Button dimensions (60px, 26px)
- The transition property should specify which properties to animate for better performance.
.content-wrap { - width: 236px; + width: var(--te-styles-spacing-setting-width); margin-bottom: 10px; .auto { - width: 60px; - height: 60px; - line-height: 60px; + width: var(--te-styles-spacing-setting-auto-size); + height: var(--te-styles-spacing-setting-auto-size); + line-height: var(--te-styles-spacing-setting-auto-size); } .option { - height: 26px; - line-height: 26px; + height: var(--te-styles-spacing-setting-option-height); + line-height: var(--te-styles-spacing-setting-option-height); - transition: 0.3s; + transition: color 0.3s, background-color 0.3s; } }Also applies to: 140-144, 175-180, 184-184
packages/settings/styles/src/components/typography/TypographyMore.vue (1)
200-201: Consider consolidating hover state variables.Multiple hover states are using different variables. Consider consolidating them for consistency:
--te-styles-common-colorfor text color--te-styles-typography-hover-bg-colorfor backgroundAlso applies to: 207-208
packages/settings/events/src/styles/vars.less (1)
9-19: Consider grouping dialog-related variables.The event binding section contains multiple dialog-related variables. Consider moving them to a separate section for better organization:
// 事件绑定 - --te-bind-event-dialog-color: var(--te-common-text-secondary); - --te-bind-event-dialog-content-left-border-color: var(--te-common-border-divider); - ... + // 事件绑定对话框 + --te-bind-event-dialog-color: var(--te-common-text-secondary); + --te-bind-event-dialog-content-left-border-color: var(--te-common-border-divider); + ...
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
packages/design-core/assets/border-radius-bottomleft.svgis excluded by!**/*.svgpackages/design-core/assets/border-radius-bottomright.svgis excluded by!**/*.svgpackages/design-core/assets/border-radius-topleft.svgis excluded by!**/*.svgpackages/design-core/assets/border-radius-topright.svgis excluded by!**/*.svg
📒 Files selected for processing (41)
packages/settings/events/index.js(1 hunks)packages/settings/events/src/components/AdvanceConfig.vue(2 hunks)packages/settings/events/src/components/BindEvents.vue(3 hunks)packages/settings/events/src/components/BindEventsDialog.vue(1 hunks)packages/settings/events/src/components/BindEventsDialogContent.vue(3 hunks)packages/settings/events/src/components/BindEventsDialogSidebar.vue(2 hunks)packages/settings/events/src/styles/vars.less(1 hunks)packages/settings/props/index.js(1 hunks)packages/settings/props/src/components/Empty.vue(1 hunks)packages/settings/props/src/components/groups/LifeCycle.vue(7 hunks)packages/settings/props/src/components/groups/TableColumn.vue(1 hunks)packages/settings/props/src/components/groups/TablePager.vue(1 hunks)packages/settings/props/src/components/inputs/CodeEditor.vue(1 hunks)packages/settings/props/src/components/inputs/DraggableOptions.vue(2 hunks)packages/settings/props/src/components/inputs/NumericSelect.vue(1 hunks)packages/settings/props/src/styles/vars.less(1 hunks)packages/settings/styles/index.js(1 hunks)packages/settings/styles/src/components/background/BackgroundGroup.vue(10 hunks)packages/settings/styles/src/components/background/BackgroundImageSetting.vue(3 hunks)packages/settings/styles/src/components/background/ImageSetting.vue(4 hunks)packages/settings/styles/src/components/background/PositionOrigin.vue(1 hunks)packages/settings/styles/src/components/border/BorderGroup.vue(7 hunks)packages/settings/styles/src/components/buttons/ButtonGroup.vue(2 hunks)packages/settings/styles/src/components/classNamesContainer/index.vue(10 hunks)packages/settings/styles/src/components/effects/EffectGroup.vue(4 hunks)packages/settings/styles/src/components/inputs/ImageSelect.vue(1 hunks)packages/settings/styles/src/components/inputs/InputSelect.vue(4 hunks)packages/settings/styles/src/components/inputs/ModalMask.vue(1 hunks)packages/settings/styles/src/components/inputs/NumericSelect.vue(2 hunks)packages/settings/styles/src/components/inputs/ResetButton.vue(1 hunks)packages/settings/styles/src/components/layout/FlexBox.vue(1 hunks)packages/settings/styles/src/components/layout/GridBox.vue(2 hunks)packages/settings/styles/src/components/layout/LayoutGroup.vue(1 hunks)packages/settings/styles/src/components/position/PositionGroup.vue(6 hunks)packages/settings/styles/src/components/size/SizeGroup.vue(5 hunks)packages/settings/styles/src/components/spacing/SpacingGroup.vue(3 hunks)packages/settings/styles/src/components/spacing/SpacingSetting.vue(3 hunks)packages/settings/styles/src/components/typography/TypographyGroup.vue(4 hunks)packages/settings/styles/src/components/typography/TypographyMore.vue(5 hunks)packages/settings/styles/src/styles/vars.less(1 hunks)packages/theme/base/src/common.less(2 hunks)
✅ Files skipped from review due to trivial changes (25)
- packages/settings/events/index.js
- packages/settings/styles/src/components/inputs/ImageSelect.vue
- packages/settings/props/src/components/inputs/CodeEditor.vue
- packages/settings/props/src/components/inputs/NumericSelect.vue
- packages/settings/styles/src/components/layout/FlexBox.vue
- packages/settings/props/src/components/groups/TablePager.vue
- packages/settings/styles/index.js
- packages/settings/props/src/components/Empty.vue
- packages/settings/styles/src/components/buttons/ButtonGroup.vue
- packages/settings/props/src/components/groups/TableColumn.vue
- packages/settings/events/src/components/BindEventsDialog.vue
- packages/settings/styles/src/components/size/SizeGroup.vue
- packages/settings/styles/src/components/classNamesContainer/index.vue
- packages/settings/styles/src/components/background/PositionOrigin.vue
- packages/settings/styles/src/components/background/ImageSetting.vue
- packages/settings/styles/src/components/inputs/ResetButton.vue
- packages/settings/styles/src/components/inputs/NumericSelect.vue
- packages/settings/events/src/components/AdvanceConfig.vue
- packages/settings/props/index.js
- packages/settings/events/src/components/BindEventsDialogSidebar.vue
- packages/settings/events/src/components/BindEventsDialogContent.vue
- packages/settings/styles/src/components/effects/EffectGroup.vue
- packages/settings/props/src/components/groups/LifeCycle.vue
- packages/settings/styles/src/components/background/BackgroundGroup.vue
- packages/settings/events/src/components/BindEvents.vue
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (32)
packages/theme/base/src/common.less (2)
48-48: New Light Theme Hover Variable Defined
The addition of--te-common-bg-gray-hover: var(--te-base-gray-40);in the light theme section clearly defines the hover state for gray backgrounds. The chosen color variable (var(--te-base-gray-40)) and accompanying comment are consistent with the overall theming approach.
108-108: New Dark Theme Hover Variable Defined
The new variable--te-common-bg-gray-hover: var(--te-base-gray-90);in the dark theme block successfully mirrors the light theme change by adjusting the hover color to a darker variant. The comment accurately explains its purpose, ensuring that the hover state is visually optimized for dark mode.packages/settings/styles/src/components/inputs/ModalMask.vue (1)
100-101: LGTM! CSS variable updates align with module convergence.The updated CSS variables follow the new naming convention and properly separate styling concerns.
Also applies to: 104-104
packages/settings/styles/src/components/typography/TypographyGroup.vue (4)
533-533: LGTM! Variable update aligns with PR objectives.The color variable has been updated from
--te-common-text-secondaryto--te-styles-common-text-color, which aligns with the PR's goal of reintegrating module variables.
620-620: LGTM! Consistent variable naming.The color variable has been updated from
--te-common-text-weakento--te-styles-common-text-weaken-color, maintaining consistency with the new naming convention.
655-655: LGTM! Consistent variable usage.Both instances of color variables have been updated to use
--te-styles-common-text-color, ensuring consistent styling across the component.Also applies to: 679-679
685-686: LGTM! Setting-specific variables.The color and background variables for the setting state have been appropriately updated to use dedicated variables:
--te-styles-common-setting-colorfor text color--te-styles-common-setting-bg-colorfor background colorThis change improves the modularity of the styling system.
packages/settings/styles/src/components/border/BorderGroup.vue (2)
550-550: LGTM! Color variables updated to use module-specific naming.The color variables have been correctly updated to use the new module-specific naming convention with the
--te-styles-prefix.Also applies to: 558-558
571-571: LGTM! Consistent use of module variables for interactive elements.The color variables for icons and interactive states have been updated consistently:
- Common text color for labels
- Icon-specific colors for SVG elements
- Active state colors for hover/selected states
Also applies to: 578-578, 594-596
packages/settings/styles/src/components/background/BackgroundImageSetting.vue (4)
168-169: LGTM! Variable naming improvementsThe changes to CSS variable references follow a more consistent and descriptive naming pattern, aligning with the module variable convergence effort.
173-174: LGTM! Improved context-specific variable namingThe new variable names better reflect their specific usage context, particularly the background color variable which now clearly indicates its use in dialog row items.
193-194: LGTM! Consistent state stylingThe hover and selected states now use appropriately scoped variables that maintain consistency with the new naming scheme.
Also applies to: 198-199
223-223: LGTM! Standardized prefix usageThe background color variable now uses the standardized
te-stylesprefix, replacing the inconsistentti-lowcodeprefix.packages/settings/styles/src/components/position/PositionGroup.vue (3)
466-466: LGTM! Text color variables updated consistently.The text color variables have been updated to use the new
--te-styles-prefix, maintaining consistency across the component.Also applies to: 512-512
525-525: LGTM! Background color variables standardized.Background color variables for various states (setting, hover, selected) have been updated to use the new standardized naming convention.
Also applies to: 529-529, 553-553, 556-556, 568-568
604-604: LGTM! Position-specific styling variables updated.Position-specific color variables for settings, relative positioning, and borders have been updated to follow the new module-specific variable pattern.
Also applies to: 605-605, 609-609, 612-612, 617-617, 620-620, 625-625
packages/settings/styles/src/styles/vars.less (1)
1-67: Well-organized variable definitions with clear categorization.The variables are well-structured and follow consistent naming patterns. The organization by feature (common, background, border, etc.) improves maintainability.
packages/settings/styles/src/components/spacing/SpacingGroup.vue (2)
544-544: LGTM! Variable updates align with new theming system.The changes correctly update the CSS variables to use the new
--te-styles-prefix, maintaining consistency with the new theming system.Also applies to: 558-558, 562-562
592-592: LGTM! Color variables properly updated.The color variables for margins, padding, and inner elements have been correctly updated to use the new module-specific variables.
Also applies to: 596-596, 599-599
packages/settings/styles/src/components/layout/GridBox.vue (3)
514-518: Variable updates align with module standardization.The changes to CSS variables in
.grid-edit-btnfollow the new naming convention, transitioning from--ti-lowcode-*to--te-styles-*prefix.
548-548: Consistent variable naming update.The color property now uses the standardized
--te-styles-common-text-colorvariable instead of the old--te-common-text-secondary.
492-568: Verify variable definitions exist.The changes look good, but we should verify that all the new
--te-styles-*variables are properly defined in the module's variables file.✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for the definition of the new CSS variables rg --type=less "(?:^|\s)(?:--|@)te-styles-common-(?:default-border-color|color|bg-color|text-color|setting-color|setting-bg-color)\s*:" -A 1Length of output: 1173
CSS Variables Definitions Verified
- Confirmed that all new
--te-styles-common-*variables (color,text-color,default-border-color,bg-color,setting-color, andsetting-bg-color) are defined in the filepackages/settings/styles/src/styles/vars.less.packages/settings/props/src/components/inputs/DraggableOptions.vue (3)
179-179: LGTM! Good variable naming updates.The changes properly migrate from global variables to module-specific ones, following the new naming convention with
--te-props-common-prefix. This improves modularity and maintainability.Also applies to: 184-184
200-200: LGTM! Good replacement of hardcoded colors.The changes properly replace hardcoded color values with module variables, improving theme consistency and maintainability.
Also applies to: 208-208
175-219: Verify module variable scope.The style changes correctly use module-specific variables with the
--te-props-common-prefix. Let's verify that no other components are directly using these variables outside the props module.✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check for usage of props module variables outside the props module # Search for any usage of --te-props-common- variables outside the props module directory rg --type=vue --type=less "var\(--te-props-common-" -l | grep -v "packages/settings/props/"Length of output: 119
🏁 Script executed:
#!/bin/bash # Re-run the search using explicit file globs for .vue and .less files rg --glob '*.vue' --glob '*.less' "var(--te-props-common-" -l | grep -v "packages/settings/props/"Length of output: 175
🏁 Script executed:
#!/bin/bash # Re-run the search using fixed-string mode (-F) for .vue and .less files to correctly match the variable usage. rg -F --glob '*.vue' --glob '*.less' "var(--te-props-common-" -l | grep -v "packages/settings/props/"Length of output: 101
Module variable usage is correctly scoped.
All searches for
var(--te-props-common-outside of thepackages/settings/props/directory returned no results. The changes consistently adhere to the module-specific naming convention.packages/settings/styles/src/components/inputs/InputSelect.vue (1)
121-121: LGTM! CSS variable updates align with standardization effort.The changes consistently update CSS variable references to use the new
--te-styles-common-*prefix, which aligns with the PR objective of standardizing module variables.Let's verify that these new variable names are defined and consistently used across the codebase:
Also applies to: 125-125, 145-145, 167-167, 176-176
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check for the existence and usage of the new CSS variable names # Search for variable definitions echo "Checking for variable definitions..." rg --type=less --type=css "^\s*--te-styles-common-(bg-color|canvas-handle-hover-bg|text-color|hover-bg-color)\s*:" # Search for any remaining old variable usage echo "Checking for any remaining old variable usage..." rg --type=vue --type=less --type=css "(--te-common-bg-default|--ti-lowcode-canvas-handle-hover-bg|--ti-lowcode-text-color|--ti-lowcode-icon-hover-bg)"Length of output: 947
🏁 Script executed:
#!/bin/bash # Description: Re-run search for any remaining old variable usage with vue file type defined echo "Checking for any remaining old variable usage (including .vue files)..." rg --type-add "vue:*.vue" --type=vue --type=less --type=css "(--te-common-bg-default|--ti-lowcode-canvas-handle-hover-bg|--ti-lowcode-text-color|--ti-lowcode-icon-hover-bg)"Length of output: 11963
CSS Variable Update in InputSelect.vue Verified
The new CSS variable (
--te-styles-common-bg-color) used inpackages/settings/styles/src/components/inputs/InputSelect.vueis correctly defined (as seen inpackages/settings/styles/src/styles/vars.less) and applied in the component. Although our search indicates that other files in the repository still reference the old variable names, the changes in InputSelect.vue are fully consistent with the PR’s objective of updating variable references.packages/settings/styles/src/components/spacing/SpacingSetting.vue (1)
124-124: CSS variable updates align with module standardization.The changes to CSS variables follow the new naming convention and maintain visual consistency across the application.
Let's verify that these new CSS variables are properly defined:
Also applies to: 145-146, 152-153, 181-182, 189-190
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check for the definition and usage of the new CSS variables # Search for variable definitions echo "Searching for variable definitions..." rg --type less "(?:--te-styles-common-icon-color|--te-styles-common-default-border-color|--te-styles-spacing-setting-bg-color|--te-styles-common-setting-color|--te-styles-common-setting-bg-color)" # Search for any remaining old variable usage echo "Checking for any remaining old variable usage..." rg --type vue "(?:--te-common-text-secondary|--ti-lowcode-style-setting-label-color|--te-common-bg-text-emphasize)"Length of output: 1123
CSS Variable Verification Confirmed
- The new CSS variable definitions (
--te-styles-common-icon-color,--te-styles-common-default-border-color,--te-styles-spacing-setting-bg-color,--te-styles-common-setting-color, and--te-styles-common-setting-bg-color) are defined inpackages/settings/styles/src/styles/vars.lesswith the expected mapped values.- A search for any remaining usage of old variables (e.g.
--te-common-text-secondary,--ti-lowcode-style-setting-label-color,--te-common-bg-text-emphasize) now needs to be verified. To ensure no accidental legacy usage, I executed another repository-wide search:If this search returns no matches, it confirms that all instances have been updated to the new standard.
Based on the available evidence, the changes align with the PR objectives and the new CSS variables are in place.
🏁 Script executed:
#!/bin/bash echo "Searching repository for remaining old variable usage..." rg "(--te-common-text-secondary|--ti-lowcode-style-setting-label-color|--te-common-bg-text-emphasize)"Length of output: 15509
CSS Variable Updates in SpacingSetting.vue Verified
- The new CSS variables in question are correctly defined in
packages/settings/styles/src/styles/vars.lessand are used appropriately in the SpacingSetting component (e.g. line 124 now usesvar(--te-styles-common-icon-color)).- Although the repository-wide search shows usage of older variables (e.g.
--te-common-text-secondary) in other files, these remain in legacy portions of the codebase and are not part of the current spacing settings update.packages/settings/styles/src/components/typography/TypographyMore.vue (3)
126-126: LGTM! Consistent variable naming pattern.The changes correctly replace common variables with module-specific ones using the
--te-styles-prefix, which aligns with the PR's objective of module variable convergence.Also applies to: 143-143, 146-146
231-231: LGTM! Consistent text color usage.The change correctly uses the module-specific text color variable.
174-174: Verify color variable definitions.The changes introduce new module-specific variables. Please ensure these variables are properly defined in the corresponding module's variables file.
Also applies to: 187-187, 193-194
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for the definition of the new style variables rg --type=less "te-styles-(typography|common)-(bg-color|default-border-color|text-color)"Length of output: 543
Variable definitions are correctly set.
The RG output confirms that the new module-specific variables (e.g.,--te-styles-common-default-border-color) are defined inpackages/settings/styles/src/styles/vars.less. No further action is needed.packages/settings/events/src/styles/vars.less (2)
2-8: LGTM! Well-organized advanced configuration variables.The variables are properly organized and follow a consistent naming pattern with clear mappings to common variables.
20-32: LGTM! Comprehensive event binding variables.The binding events section:
- Uses consistent naming patterns
- Covers all necessary UI states (hover, disabled, etc.)
- Properly references common variables
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/settings/styles/src/styles/vars.less (2)
20-27: Review: Background Styling Variables
The background variables are well grouped and correctly map to their corresponding common tokens. Consider adding fallback values tovar()calls if there is any risk a common variable might be undefined.
71-72: Review: File Termination and Formatting
The closing of the:rootblock is correctly applied. Consider removing any extraneous trailing spaces or blank lines to maintain a clean file format.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/settings/styles/src/Main.vue(3 hunks)packages/settings/styles/src/components/background/BackgroundGroup.vue(10 hunks)packages/settings/styles/src/styles/vars.less(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/settings/styles/src/Main.vue
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/settings/styles/src/components/background/BackgroundGroup.vue
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (9)
packages/settings/styles/src/styles/vars.less (9)
1-15: Review: Consolidated CSS Custom Properties Definition
This block defines a set of common styling variables under the:rootselector, mapping module-specific names to common design tokens (e.g.,--te-common-text-primary). This approach supports a consistent design language across the application.
16-19: Review: Editor Styling Variables
The editor-related variables are clearly defined and maintain the naming convention. Ensure that the source variables (e.g.,--te-common-bg-primary-emphasize) are maintained consistently across the codebase.
28-33: Review: Border Styling Variables
Border-related variables are accurately defined, with naming that clearly reflects their purpose. The references to common tokens appear intentional and aligned with the design system.
34-38: Review: Button Styling Variables
The button styling block defines variables for background, active text, and active background colors. The naming and usage of common tokens are consistent with the other sections.
39-48: Review: Global Styling Variables
This segment outlines global styling variables for various UI components, including selectors and dropdowns. The structured naming promotes clarity, though please ensure that these color choices meet accessibility guidelines such as sufficient contrast.
49-51: Review: Opacity/Effect Group Variable
A single variable is defined for the effect group's active color. As additional opacity or related styles are introduced, consider expanding this section.
52-60: Review: Positioning Variables
The positioning variables (for direction, relative positions, and hover states) are consistent and follow the naming conventions. Their references to common background and border tokens appear deliberate.
61-67: Review: Spacing Variables
Spacing-related variables (for margin, padding, inner color, etc.) are clearly defined and organized. Similar to earlier sections, you might consider adding fallback values to thevar()functions for robustness.
68-70: Review: Typography Styling Variables
Typography variables for background and hover states are succinctly defined. Confirm that these settings achieve the intended visual hierarchy and meet contrast requirements.
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
主要修改：高级，属性，样式面板
settings-design中使用外部变量的地方全部改成色值
a.检查每个模块插件包，用到的色值全都改成模块变量
b.检查模块变量有没有被非该模块的vue文件使用，修改非该模块的vue文件使用的变量
c.将模块变量回归到各个模块里面
d.模块变量回归模块后，再排查一遍该模块有没有直接使用--te-common的，有的话，需要转成模块变量。去保证整个链路是这样的： base/common less -> module less -> vue文件
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
Summary by CodeRabbit
Style
Chores