feat(theme/datasource&setting): change datasource and setting panel theme variable #927
Conversation
WalkthroughThe pull request includes extensive modifications across multiple Vue components and stylesheets, primarily focusing on updating CSS variable references, enhancing styling consistency, and refining component functionality. Key changes involve the introduction of new props, adjustments to existing props, and updates to template structures. These modifications aim to align the components with a new design system, ensuring a cohesive visual experience throughout the application. Changes
Suggested labels
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 (
|
wenmine
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Outside diff range and nitpick comments (56)
packages/settings/props/src/components/groups/TablePager.vue (1)
Line range hint
74-74: Consider standardizing remaining specific color variable.For consistency with the PR's objectives, consider updating
--ti-lowcode-toolbar-icon-colorto use a common variable from thete-common-*namespace.- color: var(--ti-lowcode-toolbar-icon-color); + color: var(--te-common-text-secondary);packages/plugins/datasource/src/DataSourceType.vue (1)
Line range hint
89-127: Consider standardizing other color variables in the stylesheet.While the border color variable has been updated, there are several other color variables that might benefit from similar standardization:
--ti-lowcode-datasource-toolbar-icon-color--ti-lowcode-datasource-label-color--ti-lowcode-datasource-input-icon-color--ti-lowcode-datasource-toolbar-breadcrumb-colorThese could potentially be aligned with the new common variable naming pattern for better consistency across themes.
packages/canvas/layout/src/CanvasLayout.vue (1)
Line range hint
31-31: Consider standardizing the background variable for consistency.While the main container now uses the new
--te-common-bg-containervariable pattern, the.site-canvasbackground still uses the old-style variable--ti-lowcode-breadcrumb-hover-bg. Consider updating this for consistency with the theme refactoring objectives..site-canvas { - background: var(--ti-lowcode-breadcrumb-hover-bg); + background: var(--te-common-bg-container-secondary); position: absolute; overflow: hidden;packages/configurator/src/code-list-configurator/CodeListConfigurator.vue (1)
Line range hint
223-299: Consider standardizing remaining theme-specific variables.For consistency with the PR's objective of using common variables, consider replacing the following theme-specific variables with their common counterparts:
- var(--ti-lowcode-tabs-border-color) - var(--ti-lowcode-toolbar-bg) - var(--ti-lowcode-text-color) - var(--ti-lowcode-toolbar-icon-color) - var(--ti-lowcode-icon-hover-bg) - var(--ti-lowcode-toolbar-more-hover-color) + var(--te-common-*) // appropriate common variablesThis would further align with the goal of standardizing variables across light and dark themes.
packages/settings/styles/src/components/inputs/ModalMask.vue (1)
Line range hint
95-100: Inconsistent variable naming patterns detected.There's a mix of old and new variable naming patterns in the modal styling:
- Line 95: Uses new pattern
--te-common-text-secondary- Line 96: Uses old pattern
--ti-lowcode-tabs-border-color- Line 99: Uses new pattern
--te-common-bg-defaultFor consistency with the PR's objective of standardizing variables, consider updating all variables to use the new
--te-common-*pattern.Consider applying these changes:
.modal-content { position: absolute; top: 0; left: 16px; z-index: 1000; padding: 8px; color: var(--te-common-text-secondary); - border: 1px solid var(--ti-lowcode-tabs-border-color); + border: 1px solid var(--te-common-border-color); box-shadow: 0px 0px 4px rgba(0, 0, 0, 0.2); border-radius: 8px; background-color: var(--te-common-bg-default); overflow: auto; max-height: 100%; box-sizing: border-box; }packages/plugins/datasource/src/RemoteDataAdapterForm.vue (2)
Line range hint
18-22: Fix type inconsistency between prop and emitted value.The
modelValueprop is typed asObject, but thechangemethod only emits string values. This type mismatch could lead to runtime issues.Consider updating the prop type to match the emitted value:
modelValue: { - type: Object, + type: String, - default: () => ({}) + default: '' }
Line range hint
66-69: Standardize the remaining specific theme variable.For consistency with the PR objectives of standardizing variables, consider updating the remaining specific variable.
.icon-del { cursor: pointer; - color: var(--ti-lowcode-datasource-toolbar-icon-color); + color: var(--te-common-text-secondary); }packages/settings/styles/src/components/layout/LayoutGroup.vue (2)
Line range hint
132-141: Standardize CSS variable prefixes.There's an inconsistency in the CSS variable naming convention within the selected state styling:
- Line 132 uses
--te-common-text-secondary- Line 137 uses
--te-common-text-emphasize- Line 138 uses
--ti-lowcode-style-setting-label-bgThis mixing of
--te-and--ti-prefixes goes against the PR's goal of standardizing variables.Consider updating the background color variable to follow the new
--te-prefix pattern:&.selected { span { cursor: pointer; border-radius: 2px; color: var(--te-common-text-emphasize); - background-color: var(--ti-lowcode-style-setting-label-bg); + background-color: var(--te-common-bg-selected); } }
Theme variable migration is incomplete
The codebase still contains numerous
--ti-prefixed CSS variables that need to be migrated to the new--te-prefix standard. In the specific file under review (LayoutGroup.vue), there is one instance:
- Line 135:
background-color: var(--ti-lowcode-style-setting-label-bg)This is part of a larger pattern across the codebase where many components are still using the old prefix. While some variables have been migrated to the new
--te-prefix, the migration is not complete.🔗 Analysis chain
Line range hint
1-150: Verify theme variable migration completeness.Let's ensure all theme variables in this component and related files have been properly migrated to the new standard.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining --ti- prefixed variables in related style files # Expected: No occurrences of old variable prefix pattern echo "Checking for old variable prefix usage in style files..." rg --type=vue --type=less "var\(--ti-" ./packages/settings/styles/src/Length of output: 223
Script:
#!/bin/bash # Let's try a different approach to check for CSS variable usage # Search for --ti- prefixed variables in all files under the styles directory rg "var\(--ti-" ./packages/settings/styles/src/ # Also check for the new --te- prefix to understand the migration status rg "var\(--te-" ./packages/settings/styles/src/Length of output: 22640
packages/settings/design/src/components/PropertyCanvas.vue (2)
Line range hint
249-267: Standardize CSS variable prefix usageThere's an inconsistent mix of variable prefixes:
--te-common-*for text and background colors--ti-lowcode-*for plugin-specific colorsConsider standardizing all variables to use the same prefix for better maintainability.
Line range hint
180-306: Consider implementing a more structured theme systemThe current implementation mixes different types of theme variables:
- Common text variables (
--te-common-*)- Lowcode-specific variables (
--ti-lowcode-*)- Component-specific colors
Recommendations for a more maintainable theme system:
- Create a clear hierarchy of variables:
- Base colors
- Semantic colors
- Component-specific variables
- Document the relationship between these layers
- Consider using CSS custom property fallbacks for better maintainability:
color: var(--te-component-specific-color, var(--te-semantic-color, var(--te-base-color)));This would make it easier to:
- Override colors at different levels
- Maintain consistency across themes
- Debug theme-related issues
packages/plugins/datasource/src/DataSourceList.vue (1)
Line range hint
1-182: Consider migrating remaining specific variablesSeveral other specific variables in the stylesheet could be migrated to common variables for consistency:
var(--ti-lowcode-data-source-border-color)→var(--te-common-border-color)var(--ti-lowcode-datasource-tabs-border-color)→var(--te-common-border-color)var(--ti-lowcode-datasource-common-text-main-color)→var(--te-common-text-primary)var(--ti-lowcode-datasource-list-hover-color)→var(--te-common-bg-hover-color)var(--ti-lowcode-datasource-toolbar-more-hover-color)→var(--te-common-icon-color)This would further align with the PR's objective of standardizing variables across themes.
packages/settings/styles/src/components/spacing/SpacingGroup.vue (2)
6-6: LGTM! Color management improvements enhance theme consistency.The transition from hardcoded colors to
currentColorand class-based styling aligns well with the PR's objective of standardizing variables across themes.Consider adding ARIA attributes for better accessibility:
<svg xmlns="http://www.w3.org/2000/svg" fill="currentColor" width="256" height="120" + role="img" + aria-label="Spacing Editor" style="grid-area: 1 / 1 / -1 / -1" class="margin-color" >Also applies to: 10-10, 25-25, 43-43, 61-61, 79-79, 90-90, 103-103, 114-114, 127-127, 189-189, 193-193, 208-208, 226-226, 244-244, 262-262, 273-273, 286-286, 297-297
544-544: Consider standardizing CSS variable naming patterns.The CSS variables use mixed prefixes (
--te-and--ti-). For better maintainability and consistency:Consider standardizing the variable naming:
- color: var(--te-common-text-primary); + color: var(--ti-lowcode-text-primary); - color: var(--te-common-bg-default); + color: var(--ti-lowcode-bg-default);This aligns with the existing
--ti-lowcode-prefix pattern used in other variables.Also applies to: 592-592, 596-596, 599-599
packages/settings/styles/src/components/background/RadialGradient.vue (1)
Variable naming inconsistency confirmed - update needed for CSS variable
The codebase shows a clear transition from
--ti-lowcode-common-*to--te-common-*naming pattern. The.sizeclass should use--te-common-text-tertiaryto align with the new convention..size { - color: var(--ti-lowcode-common-third-text-color); + color: var(--te-common-text-tertiary); }🔗 Analysis chain
Line range hint
137-157: Inconsistent variable naming pattern detected.While line 137 uses the new
--te-prefix, the.sizeclass still uses the old--ti-lowcode-common-third-text-colorvariable. This should be updated to maintain consistency with the new naming convention.Let's verify the variable naming patterns across the codebase:
Suggested fix:
.size { - color: var(--ti-lowcode-common-third-text-color); + color: var(--te-common-text-tertiary); }🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for inconsistent variable usage patterns # Search for old variable pattern echo "Files still using old --ti-lowcode-common pattern:" rg --type=less --type=vue "var\(--ti-lowcode-common-[a-z-]+\)" # Search for new variable pattern echo -e "\nFiles using new --te-common pattern:" rg --type=less --type=vue "var\(--te-common-[a-z-]+\)"Length of output: 377
Script:
#!/bin/bash # Description: Check for inconsistent variable usage patterns with correct file type handling # Search for old variable pattern echo "Files still using old --ti-lowcode-common pattern:" rg "var\(--ti-lowcode-common-[a-z-]+\)" # Search for new variable pattern echo -e "\nFiles using new --te-common pattern:" rg "var\(--te-common-[a-z-]+\)" # Get specific context around the variable usage in RadialGradient.vue echo -e "\nContext in RadialGradient.vue:" rg -A 5 -B 5 "var\(--t[ie]-" "packages/settings/styles/src/components/background/RadialGradient.vue"Length of output: 66139
packages/settings/props/src/components/Empty.vue (1)
Line range hint
15-20: Verify prop documentation and default value.Consider adding a prop description to improve component documentation.
props: { showEmptyTips: { type: Boolean, - default: false + default: false, + description: 'Controls the visibility of empty state tips' } }packages/settings/styles/src/components/typography/TypographyMore.vue (1)
Line range hint
1-235: Consider these improvements for better maintainability.
- Consider migrating to TypeScript for better type safety and developer experience
- Extract tooltip content to i18n files for better internationalization support
- Add more specific prop types and default values
Example improvement for props:
props: { style: { type: Object, + // Add specific shape + default: () => ({}), + validator: (value) => { + // Add validation for required style properties + return true + } }, effect: { type: String, + values: ['dark', 'light'], default: 'dark' }, placement: { type: String, + values: ['top', 'bottom', 'left', 'right'], default: 'top' } }packages/plugins/datasource/src/DataSourceRemoteDataResult.vue (6)
Line range hint
3-3: Fix typo in class nameThe class name 'resonse-header' contains a typo and should be 'response-header'.
- <div class="resonse-header"> + <div class="response-header">
Line range hint
4-6: Consider internationalizing hardcoded textThe component contains hardcoded Chinese text in both the alert message and buttons. Consider using a localization system for better maintainability and internationalization support.
Also applies to: 11-12
Line range hint
41-41: Add null check in getResponseData functionThe exported function directly accesses the editor ref which could be null. Consider adding a null check for better error handling.
-export const getResponseData = () => editor.value.getEditor().getValue() +export const getResponseData = () => editor.value?.getEditor()?.getValue() ?? ''
Line range hint
82-86: Improve error handling in copyDataThe function silently ignores all errors during clipboard operations. Consider at least logging the error or notifying the user when the operation fails.
const copyData = async () => { try { await toClipboard(state.value) } catch (e) { - // do nothing + console.error('Failed to copy data:', e) + // Consider emitting an error event or showing a notification + emit('copy-error', e) }
Line range hint
107-115: Update class name in styles to match templateWhen fixing the typo in the template, ensure the corresponding style selector is also updated.
- .resonse-header { + .response-header { padding: 10px; margin-bottom: 16px;
Theme variable standardization needed for remaining custom variables
The following specific theme variables should be updated to use common variables to maintain consistency:
--ti-lowcode-datasource-tip-color--ti-lowcode-datasource-toolbar-breadcrumb-color--ti-lowcode-datasource-tabs-border-colorConsider replacing these with appropriate common variables (--te-common-*) similar to how
--te-common-border-defaultand--te-common-border-dividerare being used.🔗 Analysis chain
Line range hint
107-128: Verify remaining theme variable updatesWhile the border color has been updated to use common variables, there are still other specific variables in use (e.g.,
--ti-lowcode-datasource-*). Consider standardizing these as well to align with the PR objectives.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for remaining specific theme variables in the codebase rg --type=vue --type=less "ti-lowcode-datasource-" ./packages/plugins/datasource/Length of output: 109
Script:
#!/bin/bash # Search for remaining specific theme variables in the file, adjusting the ripgrep command rg "ti-lowcode-datasource-|--te-" "./packages/plugins/datasource/src/DataSourceRemoteDataResult.vue"Length of output: 408
packages/plugins/datasource/src/DataSourceRecordList.vue (2)
618-618: Possible inconsistency in CSS variable naming for thecolorpropertyLine 618 uses the CSS variable
--te-common-text-primary, whereas other variables in the file use the prefix--ti-lowcode-. Please verify if--te-common-text-primaryis the correct variable and consider updating it to maintain naming consistency across the codebase.
636-636: Potential misuse of CSS variable for text colorLine 636 sets the
colorproperty using--ti-lowcode-datasource-json-border-color, which appears to be intended for borders. It would be more semantically appropriate to use a text color variable to style text. Consider replacing it with a suitable text color variable to enhance readability and maintain consistency.packages/settings/styles/src/components/background/ImageSetting.vue (2)
Line range hint
152-171: Consider enhancing size parsing logic and type safety.The size parsing logic could be improved for better maintainability and robustness:
- The regex pattern could miss some valid CSS size values
- There's no validation for negative values
- The component would benefit from TypeScript types
Consider extracting the size parsing logic into a separate utility:
// types.ts interface SizeValue { value: string; unit: string; } // utils/size-parser.ts export function parseCSSSize(size: string): SizeValue { if (size.toLowerCase() === 'auto') { return { value: 'Auto', unit: 'auto' }; } const match = /^([+-]?\d*\.?\d+)(px|%|em|rem|vh|vw)?$/i.exec(size); if (!match) { return { value: 'Auto', unit: 'auto' }; } return { value: match[1], unit: match[2] || 'px' }; }
Line range hint
89-146: Consider extracting size-related logic into a composable.The size-related methods could be organized better using Vue's composition API for improved reusability and maintainability.
Consider creating a dedicated composable:
// composables/useBackgroundSize.ts import { reactive, computed } from 'vue'; import { parseCSSSize } from '../utils/size-parser'; export function useBackgroundSize(emit: Function) { const state = reactive({ width: 'Auto', widthSuffix: 'auto', height: 'Auto', heightSuffix: 'auto', }); const backgroundSize = computed(() => { const width = state.widthSuffix === 'auto' ? 'auto' : `${state.width}${state.widthSuffix}`; const height = state.heightSuffix === 'auto' ? 'auto' : `${state.height}${state.heightSuffix}`; return `${width} ${height}`; }); // ... rest of the size-related methods return { state, backgroundSize, // ... exported methods }; }packages/settings/styles/src/components/layout/GridBox.vue (2)
Line range hint
530-562: LGTM! Consider adding CSS documentation.The grid layout implementation is well-structured and correctly uses CSS variables for theming. The BEM naming convention is followed consistently.
Consider adding a brief comment explaining the grid layout structure, particularly the purpose of
grid-column: 1 / -1andgrid-row-start: 3, to help other developers understand the layout intentions.
563-566: Consider using CSS variables for spacing values.To maintain consistency with the theme system and align with the PR objectives of standardizing variables, consider replacing the fixed pixel values with CSS variables.
.grid-edit-spacing { - padding-top: 12px; - padding-bottom: 12px; + padding-top: var(--ti-lowcode-spacing-3x, 12px); + padding-bottom: var(--ti-lowcode-spacing-3x, 12px); }packages/canvas/container/src/components/CanvasDivider.vue (1)
Line range hint
223-224: Consider externalizing image assets and using CSS variables for dimensions.To improve maintainability and consistency:
- Move the base64 encoded images to separate asset files
- Convert magic numbers (24px, 12px) to theme variables for better customization
Example refactor:
.divider-wrapper { position: absolute; display: flex; align-items: center; justify-content: center; - width: 24px; - height: 24px; + width: var(--te-divider-size, 24px); + height: var(--te-divider-size, 24px); border-radius: 50%; // ... .divider { - width: 12px; - height: 12px; + width: var(--te-divider-icon-size, 12px); + height: var(--te-divider-icon-size, 12px); // ... - background-image: url(data:image/png;base64,...); + background-image: url('@/assets/icons/divider.png'); } }Also applies to: 226-227
packages/settings/styles/src/components/background/BackgroundGroup.vue (3)
Line range hint
214-234: Enhance error handling in background image operationsThe background image operations could benefit from proper error handling to prevent silent failures and improve user feedback.
Consider implementing the following improvements:
const updateBackgroundImage = () => { + try { const property = { [BACKGROUND_PROPERTY.BackgroundImage]: '', [BACKGROUND_PROPERTY.BackgroundPosition]: '', [BACKGROUND_PROPERTY.BackgroundSize]: '', [BACKGROUND_PROPERTY.BackgroundRepeat]: '', [BACKGROUND_PROPERTY.BackgroundAttachment]: '' } state.backgroundImageList.forEach(({ invisible, styleObj }, index) => { if (invisible) { return } Object.keys(property).forEach((key) => { property[key] += `${index === 0 || !property[key] ? '' : ','}${styleObj[key] ?? getDefaultVal(key)}` }) }) updateStyle(property) + } catch (error) { + console.error('Failed to update background image:', error) + // Consider emitting an error event for parent components to handle + emit('error', { type: 'UPDATE_BACKGROUND', error }) + } }
Line range hint
108-116: Consider migrating to TypeScript for improved type safetyThe component handles complex data structures and would benefit from TypeScript's type safety features.
Consider adding type definitions for props and state:
interface BackgroundImageItem { type: string text: string invisible?: boolean dragStart?: string draging?: boolean forbiddenChildrenPointerEvents?: string styleObj: { 'background-image'?: string 'background-position'?: string 'background-size'?: string 'background-repeat'?: string 'background-attachment'?: string } } interface ComponentState { selectValue: string showModal: boolean showBackgroundImageModal: boolean currentBackgroundImage: Partial<BackgroundImageItem> backgroundImageIndex: number backgroundImageList: BackgroundImageItem[] cache: Map<string, BackgroundImageItem[]> }
Line range hint
267-338: Extract drag-and-drop functionality into a composableThe drag-and-drop logic could be extracted into a reusable composable to improve maintainability and reusability.
Consider creating a separate composable:
// useDragAndDrop.ts export function useDragAndDrop(list: Ref<any[]>, onReorder: (from: number, to: number) => void) { const dragState = reactive({ dragFrom: null, dragTo: null, dragToIndex: null }) const handleDragstart = (e: DragEvent, item: any) => { dragState.dragFrom = e.target setTimeout(() => { item.dragStart = 'drag-start' }, 0) } // ... other drag handlers return { handleDragstart, handleDragenter, handleDragleave, handleDragend } }This would simplify the component and make the drag-and-drop functionality reusable across other components.
packages/configurator/src/slot-configurator/SlotConfigurator.vue (2)
Line range hint
41-134: Consider refactoring the toggleSlot function for better maintainabilityThe toggleSlot function handles multiple responsibilities: slot binding/unbinding, confirmation dialog, popper state management, and canvas updates. Consider breaking it down into smaller, focused functions.
- const toggleSlot = ({ name = 'default', params, bind }, i) => { + const handleSlotBinding = ({ name, params }, schema) => { + const template = { + componentName: 'Template', + props: { + slot: { name } + }, + children: [] + } + if (params?.length) { + template.props.slot.params = params + } + schema.children.push(template) + } + + const handlePopperState = (schema, config) => { + const isPopper = config?.configure?.isPopper + if (isPopper) { + const showProp = typeof isPopper === 'string' ? isPopper : 'modelValue' + schema.props[showProp] = false + nextTick(() => { + schema.props[showProp] = true + }) + } + } + + const toggleSlot = ({ name = 'default', params, bind }, i) => {
Line range hint
151-199: Consider converting magic numbers to variablesThe switch styling contains several hardcoded values that could be converted to theme variables for better maintainability:
width: 14pxandheight: 14pxfor help iconwidth: 40pxandheight: 20pxfor switchborder-radius: 10pxConsider defining these in your theme variables:
- width: 14px; - height: 14px; + width: var(--te-common-icon-size-sm); + height: var(--te-common-icon-size-sm); - width: 40px; - height: 20px; + width: var(--te-common-switch-width); + height: var(--te-common-switch-height); - border-radius: 10px; + border-radius: var(--te-common-switch-radius);packages/settings/design/src/components/SettingPanel.vue (2)
279-281: Consider using common color variable for iconThe icon color is still using a specific lowcode variable (
--ti-lowcode-setting-plugin-icon-color) while other colors have been standardized to common variables.Consider using a common color variable like
--te-common-text-secondaryor--te-common-icon-normalto maintain consistency with the theme standardization effort.
Theme variables standardization needs attention
The verification shows that while most variables have been standardized to use
--te-common-*prefix, there are still 3 non-standardized variables in use:
--max-height(likely a dynamic variable, can be kept)--ti-lowcode-setting-plugin-icon-color(should be standardized)--ti-switch-width(should be standardized)🔗 Analysis chain
Line range hint
260-385: Verify theme consistency across all statesWhile most color variables have been standardized, let's verify the visual consistency across different states and themes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining non-common theme variables in the codebase # that might need standardization # Search for any remaining ti-lowcode-* variables echo "Checking for remaining lowcode-specific variables:" rg --type=less --type=vue "var\(--ti-lowcode-[^)]+\)" # Search for new common variables to ensure consistent usage echo "Checking usage of new common variables:" rg --type=less --type=vue "var\(--te-common-[^)]+\)"Length of output: 366
Script:
#!/bin/bash # Description: Check for any remaining non-common theme variables in the codebase # that might need standardization # Search for any remaining ti-lowcode-* variables echo "Checking for remaining lowcode-specific variables:" rg "var\(--ti-lowcode-[^)]+\)" # Search for new common variables to ensure consistent usage echo "Checking usage of new common variables:" rg "var\(--te-common-[^)]+\)"Length of output: 72062
Script:
#!/bin/bash # Let's specifically check for the usage of --te-common variables in the file echo "Checking SettingPanel.vue for te-common variables:" rg "var\(--te-common-[^)]+\)" packages/settings/design/src/components/SettingPanel.vue # Also check for any remaining non-standardized variables in this file echo -e "\nChecking for any non-common theme variables in SettingPanel.vue:" rg "var\(--(?!te-common)[^)]+\)" packages/settings/design/src/components/SettingPanel.vueLength of output: 1096
Script:
#!/bin/bash # Let's check for any non-te-common variables in SettingPanel.vue using a different approach echo "Checking for all CSS variables in SettingPanel.vue:" rg "var\(--[^)]+\)" packages/settings/design/src/components/SettingPanel.vue | grep -v "var(--te-common-"Length of output: 348
packages/theme/base/src/base.less (1)
28-238: Standardize Increments Across Color Variable DefinitionsThe color variables for each color family (e.g., red, orange, yellow) have varying increments and ranges. Consider standardizing the increments and the total number of shades across all color families to maintain uniformity in the theme.
designer-demo/src/configurators/MyInputConfigurator.vue (1)
Line range hint
26-28: Trim Whitespace When Splitting the Value into an ArrayWhen
dataTypeis 'Array', thechangemethod splits the input value by commas. To ensure that array elements do not contain unintended whitespace, consider trimming each element.Apply this change to the
changemethod:const change = (val) => { - emit('update:modelValue', props.dataType === 'Array' ? val.split(',') : val) + emit('update:modelValue', props.dataType === 'Array' ? val.split(',').map(item => item.trim()) : val) }packages/configurator/src/input-configurator/InputConfigurator.vue (1)
Line range hint
31-33: Trim Whitespace When Splitting the Value into an ArrayIn the
changemethod, whendataTypeis 'Array', consider trimming whitespace from the split values to ensure clean array elements.Apply this change to the
changemethod:const change = (val) => { - emit('update:modelValue', props.dataType === 'Array' ? val.split(',') : val) + emit('update:modelValue', props.dataType === 'Array' ? val.split(',').map(item => item.trim()) : val) }packages/theme/dark/canvas.less (2)
1-1: Translate comment to English for better maintainabilityThe Chinese comment should be translated to English to maintain consistency and improve maintainability.
-// 这里为canvas样式变量，canvas在暗色主题和亮色主题时，很多表现都一致，因此可以不使用common变量，而是直接用base变量 +// Canvas style variables. Since canvas appearance is consistent between dark and light themes, +// we can use base variables directly instead of common variables
3-16: Consider using base variables for consistent canvas stylesSince the canvas appearance is consistent between dark and light themes (as noted in the comment), consider using base variables directly instead of common variables where possible. This would reduce the abstraction layer and make the theming more straightforward.
For example:
--te-common-border-checkedcould potentially use a base color variable--te-common-bg-primary-emphasizecould be simplified to a base colorpackages/settings/styles/src/components/inputs/NumericSelect.vue (2)
Line range hint
67-85: Consider refactoring the change handler for better maintainabilityThe change handler contains complex conditional logic for handling different value formats. Consider extracting the format logic into a separate function.
+ const formatValue = (value, name) => { + if (!value || value.trim().length === 0) return null; + if (name === 'zIndex' || + value === 'auto' || + value === 'none' || + percentageReg(value)) { + return value; + } + return `${value}px`; + } const change = () => { focus.value = false numericalModelValue.value = String(numericalModelValue.value) - if (numericalModelValue.value.trim().length === 0) { - emit('update', { [props.name]: null }) - } - emit('update', { - [props.name]: - props.name === 'zIndex' || - numericalModelValue.value === 'auto' || - numericalModelValue.value === 'none' || - percentageReg(numericalModelValue.value) || - !numericalModelValue.value - ? numericalModelValue.value - : `${numericalModelValue.value}px` + [props.name]: formatValue(numericalModelValue.value, props.name) }) }
Line range hint
46-49: Consider adding validation for the suffix propThe suffix prop accepts any string value but is used specifically for units. Consider adding validation or using an enum of allowed values.
suffix: { type: String, - default: 'PX' + default: 'PX', + validator: (value) => ['PX', 'EM', 'REM', '%'].includes(value.toUpperCase()) }packages/theme/base/src/page/base-config-page.less (1)
53-53: Theme updates improve consistency but could benefit from variable consolidation.The changes successfully migrate to the new theme system. However, the popover arrow border colors are repeated multiple times with the same value.
Consider consolidating repeated values into a single variable:
+ // At the top of the file + --popover-arrow-color: var(--te-common-bg-popover); - border-bottom-color: var(--te-common-bg-popover); + border-bottom-color: var(--popover-arrow-color); // Apply similar changes to other arrow directionsAlso applies to: 137-180
packages/theme/base/src/common.less (1)
Line range hint
4-105: Excellent theme system implementation with comprehensive coverage.The new theme system provides:
- Clear functional color definitions
- Well-structured text hierarchy
- Consistent naming patterns
- Appropriate dark theme adaptations
Consider adding a comment block at the top of the file explaining the naming convention:
/* * Theme Variable Naming Convention: * --te-common-{category}-{variant} * * Categories: color, text, icon, border, bg * Variants: primary, secondary, success, warning, etc. */packages/settings/styles/src/components/classNamesContainer/index.vue (4)
Line range hint
8-10: Consider enhancing accessibilityThe help link button lacks an accessible label for screen readers.
-<link-button :href="docsUrl" class="help-link"></link-button> +<link-button :href="docsUrl" class="help-link" aria-label="View documentation"></link-button>
Line range hint
33-40: Enhance error handling for selector validationThe selector validation logic should provide more specific error messages and handle edge cases.
-:class="['className-selector-container', { 'has-error': classNameState.selectorHasError }]" +:class="[ + 'className-selector-container', + { 'has-error': classNameState.selectorHasError }, + { 'has-warning': classNameState.selectorHasWarning } +]"
Line range hint
229-236: Critical: Fix potential race conditionThe selector validation and update logic may have a race condition when handling rapid user input.
const handleCompleteEditCurSelector = () => { + if (classNameState.curSelectorIsEditing === false) return + if (!selectorTextRef.value) { return } + + // Debounce rapid user input + if (classNameState.editTimeout) { + clearTimeout(classNameState.editTimeout) + } + + classNameState.editTimeout = setTimeout(() => { const curValue = selectorTextRef.value.textContent let textValue = curValue // ... rest of the function + }, 150) }
Line range hint
192-198: Consider caching selector validation resultsThe selector validation is called frequently and could benefit from caching results for recently validated selectors.
+const selectorValidationCache = new Map() +const CACHE_TIMEOUT = 5000 // 5 seconds const selectorValidator = (selector) => { + const cached = selectorValidationCache.get(selector) + if (cached && Date.now() - cached.timestamp < CACHE_TIMEOUT) { + return cached.isValid + } let sel = selector.trim() // ... validation logic ... + selectorValidationCache.set(selector, { + isValid: isValid, + timestamp: Date.now() + }) return isValid }packages/canvas/container/src/components/CanvasAction.vue (1)
Line range hint
12-22: Consider debouncing resize operationsThe resize operations could benefit from debouncing to improve performance during rapid resizing.
+import { debounce } from 'lodash-es' export default { setup(props) { + const debouncedUpdateRect = debounce(updateRect, 16) // ~60fps const onMousedown = (event, horizontal, vertical) => { // ... existing code ... - updateRect() + debouncedUpdateRect() } return { onMousedown } } }packages/settings/styles/src/components/border/BorderGroup.vue (2)
Line range hint
192-198: Consider memoizing computed propertiesThe border style computations could benefit from memoization to prevent unnecessary recalculations.
-const borderWidthValue = computed(() => { +const borderWidthValue = computed(() => { + return useMemo( + () => { let property = {} const propertyName = BORDER_WIDTH[state.activedBorder] if (propertyName) { property = getProperty(propertyName) } return property + }, + [state.activedBorder] + ) })Also applies to: 229-236
Line range hint
1-24: Consider adding keyboard navigationThe border control UI should support keyboard navigation for better accessibility.
<div class="radius-row item-row"> <div :class="['radius-label', { 'is-setting': isRadiusSetting() }]" @click="openSetting(BORDER_RADIUS_PROPERTY.BorderRadius, $event)" + @keydown.enter="openSetting(BORDER_RADIUS_PROPERTY.BorderRadius, $event)" + tabindex="0" + role="button" + aria-label="Border radius settings" > <span>圆角</span> </div>packages/theme/base/src/component-common.less (3)
32-32: Consistent use of background color variables across componentsThe standardization of background colors using
var(--te-common-bg-*)variables across components improves theme consistency. However, some components might benefit from additional hover state backgrounds.Consider adding hover state backgrounds for components that currently don't have them, similar to how it's implemented in the tree component:
.tiny-dialog-box.tiny-dialog-box { background-color: var(--te-common-bg-popover); + &:hover { + background-color: var(--te-common-bg-container); + } }Also applies to: 89-89, 169-169, 171-171, 251-251, 256-259, 432-432, 474-476
805-822: New guide component implementationThe newly added guide component follows the theme's color scheme and uses appropriate variables. However, the selector specificity could be optimized.
Consider reducing selector specificity and grouping related styles:
.tiny-guide { background-color: var(--te-common-bg-popover); - .shepherd-content .shepherd-header, - .shepherd-arrow:before { + &__header, + &__arrow::before { background-color: var(--te-common-bg-popover); } - &.shepherd-has-title[data-popper-placement^='bottom'], - &.shepherd-has-title[data-popper-placement^='top'], - &.shepherd-has-title[data-popper-placement^='left'], - &.shepherd-has-title[data-popper-placement^='right'] { + &[data-popper-placement] { .shepherd-arrow:before { background-color: var(--te-common-bg-popover); } } }
Line range hint
1-822: Architecture recommendations for theme implementationThe file implements a comprehensive theming system using CSS variables. However, there are opportunities for improvement in the overall architecture.
Consider the following architectural improvements:
- Create a separate theme configuration file that defines all variables in one place
- Group variables by purpose (colors, spacing, typography, etc.)
- Implement a theme switching mechanism using CSS custom properties
- Document the theming system and variable usage
Example structure:
// theme/variables.less :root { // Colors --te-common-bg-default: #ffffff; --te-common-bg-container: #f5f5f5; // Typography --te-base-font-size-base: 14px; --te-base-font-size-1: 16px; // Spacing --te-common-vertical-item-spacing-normal: 8px; // ... other variables } // theme/dark.less [data-theme="dark"] { --te-common-bg-default: #1f1f1f; --te-common-bg-container: #2f2f2f; // ... other dark theme values }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (69)
designer-demo/src/configurators/MyInputConfigurator.vue(1 hunks)packages/canvas/container/src/components/CanvasAction.vue(1 hunks)packages/canvas/container/src/components/CanvasDivider.vue(2 hunks)packages/canvas/container/src/components/CanvasResizeBorder.vue(1 hunks)packages/canvas/container/src/components/shortCutPopover.vue(4 hunks)packages/canvas/layout/src/CanvasLayout.vue(1 hunks)packages/common/component/SearchEmpty.vue(1 hunks)packages/configurator/src/code-list-configurator/CodeListConfigurator.vue(1 hunks)packages/configurator/src/html-attributes-configurator/HtmlAttributesConfigurator.vue(2 hunks)packages/configurator/src/input-configurator/InputConfigurator.vue(1 hunks)packages/configurator/src/layout-grid-configurator/LayoutGridConfigurator.vue(3 hunks)packages/configurator/src/select-icon-configurator/SelectIconConfigurator.vue(1 hunks)packages/configurator/src/slider-configurator/SliderConfigurator.vue(1 hunks)packages/configurator/src/slot-configurator/SlotConfigurator.vue(1 hunks)packages/engine-cli/template/designer/src/configurators/MyInputConfigurator.vue(1 hunks)packages/plugins/datasource/src/DataSourceField.vue(1 hunks)packages/plugins/datasource/src/DataSourceList.vue(2 hunks)packages/plugins/datasource/src/DataSourceRecordList.vue(2 hunks)packages/plugins/datasource/src/DataSourceRemoteDataResult.vue(1 hunks)packages/plugins/datasource/src/DataSourceRemoteForm.vue(0 hunks)packages/plugins/datasource/src/DataSourceRemotePanel.vue(0 hunks)packages/plugins/datasource/src/DataSourceRemoteParameter.vue(1 hunks)packages/plugins/datasource/src/DataSourceTemplate.vue(2 hunks)packages/plugins/datasource/src/DataSourceType.vue(1 hunks)packages/plugins/datasource/src/RemoteDataAdapterForm.vue(2 hunks)packages/plugins/materials/src/meta/block/src/BlockGroupSort.vue(1 hunks)packages/plugins/state/src/DataSourceRightPanel.vue(2 hunks)packages/settings/design/src/App.vue(1 hunks)packages/settings/design/src/components/ArrayConfigItemForm.vue(1 hunks)packages/settings/design/src/components/PropertyCanvas.vue(6 hunks)packages/settings/design/src/components/PropertyList.vue(1 hunks)packages/settings/design/src/components/SettingPanel.vue(3 hunks)packages/settings/events/src/components/BindEventsDialogContent.vue(3 hunks)packages/settings/props/src/components/Empty.vue(1 hunks)packages/settings/props/src/components/groups/LifeCycle.vue(5 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/DraggableOptions.vue(1 hunks)packages/settings/props/src/components/inputs/NumericSelect.vue(0 hunks)packages/settings/styles/src/Main.vue(1 hunks)packages/settings/styles/src/components/background/BackgroundGroup.vue(1 hunks)packages/settings/styles/src/components/background/ImageSetting.vue(3 hunks)packages/settings/styles/src/components/background/RadialGradient.vue(1 hunks)packages/settings/styles/src/components/border/BorderGroup.vue(1 hunks)packages/settings/styles/src/components/classNamesContainer/index.vue(1 hunks)packages/settings/styles/src/components/inputs/ImageSelect.vue(1 hunks)packages/settings/styles/src/components/inputs/InputSelect.vue(1 hunks)packages/settings/styles/src/components/inputs/ModalMask.vue(1 hunks)packages/settings/styles/src/components/inputs/NumericSelect.vue(1 hunks)packages/settings/styles/src/components/inputs/ResetButton.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/size/SizeGroup.vue(2 hunks)packages/settings/styles/src/components/spacing/SpacingGroup.vue(12 hunks)packages/settings/styles/src/components/spacing/SpacingSetting.vue(3 hunks)packages/settings/styles/src/components/typography/TypographyGroup.vue(1 hunks)packages/settings/styles/src/components/typography/TypographyMore.vue(3 hunks)packages/theme/base/src/base.less(1 hunks)packages/theme/base/src/common.less(2 hunks)packages/theme/base/src/component-common.less(15 hunks)packages/theme/base/src/page/base-config-page.less(5 hunks)packages/theme/dark/canvas.less(1 hunks)packages/theme/dark/datasource.less(1 hunks)packages/theme/dark/setting-style.less(1 hunks)packages/theme/dark/settings.less(1 hunks)packages/theme/light/canvas.less(2 hunks)packages/theme/light/datasource.less(1 hunks)packages/theme/light/setting-style.less(1 hunks)packages/theme/light/settings.less(1 hunks)
💤 Files with no reviewable changes (3)
- packages/plugins/datasource/src/DataSourceRemoteForm.vue
- packages/settings/props/src/components/inputs/NumericSelect.vue
- packages/plugins/datasource/src/DataSourceRemotePanel.vue
✅ Files skipped from review due to trivial changes (25)
- packages/settings/styles/src/components/inputs/ResetButton.vue
- packages/plugins/datasource/src/DataSourceTemplate.vue
- packages/configurator/src/layout-grid-configurator/LayoutGridConfigurator.vue
- packages/plugins/datasource/src/DataSourceField.vue
- packages/settings/styles/src/Main.vue
- packages/settings/props/src/components/groups/LifeCycle.vue
- packages/settings/props/src/components/inputs/DraggableOptions.vue
- packages/settings/design/src/App.vue
- packages/settings/styles/src/components/inputs/ImageSelect.vue
- packages/plugins/materials/src/meta/block/src/BlockGroupSort.vue
- packages/settings/design/src/components/PropertyList.vue
- packages/settings/styles/src/components/typography/TypographyGroup.vue
- packages/engine-cli/template/designer/src/configurators/MyInputConfigurator.vue
- packages/theme/light/setting-style.less
- packages/configurator/src/html-attributes-configurator/HtmlAttributesConfigurator.vue
- packages/plugins/datasource/src/DataSourceRemoteParameter.vue
- packages/configurator/src/slider-configurator/SliderConfigurator.vue
- packages/settings/styles/src/components/spacing/SpacingSetting.vue
- packages/configurator/src/select-icon-configurator/SelectIconConfigurator.vue
- packages/settings/design/src/components/ArrayConfigItemForm.vue
- packages/plugins/state/src/DataSourceRightPanel.vue
- packages/settings/props/src/components/groups/TableColumn.vue
- packages/settings/styles/src/components/inputs/InputSelect.vue
- packages/canvas/container/src/components/shortCutPopover.vue
- packages/canvas/container/src/components/CanvasResizeBorder.vue
🔇 Additional comments (49)
packages/settings/props/src/components/groups/TablePager.vue (1)
65-65: LGTM! Variable change aligns with theme standardization.
The change from --ti-lowcode-toolbar-breadcrumb-color to --te-common-text-secondary aligns well with the PR's objective of standardizing variables across themes.
packages/plugins/datasource/src/DataSourceType.vue (1)
120-120: LGTM! Variable change aligns with theme standardization.
The update from common-primary-color to common-border-primary-color better reflects the semantic purpose of the color variable.
packages/canvas/layout/src/CanvasLayout.vue (1)
24-24: LGTM! Variable change aligns with theme standardization.
The change from the old variable pattern to --te-common-bg-container follows the PR's objective of standardizing theme variables.
packages/configurator/src/code-list-configurator/CodeListConfigurator.vue (1)
223-223: LGTM: Color variable update aligns with theme standardization.
The change from var(--ti-lowcode-toolbar-breadcrumb-color) to var(--te-common-text-secondary) correctly follows the PR's objective of using common theme variables.
packages/settings/styles/src/components/size/SizeGroup.vue (2)
490-490: LGTM: Consistent use of common text color variable
The change to use --te-common-text-secondary for the overflow label aligns well with the PR's objective of standardizing variables across themes.
566-567: LGTM: Consistent styling for icon wrapper
Good changes:
- Using
--te-common-text-secondarymaintains consistency with other labels - The added
margin-leftprovides proper spacing from adjacent elements
packages/settings/styles/src/components/inputs/ModalMask.vue (1)
95-95: LGTM! Color variable change aligns with theme refactoring.
The change from --ti-lowcode-toolbar-breadcrumb-color to --te-common-text-secondary aligns well with the PR's objective of standardizing variables and moving to common theme variables.
packages/common/component/SearchEmpty.vue (1)
32-32: LGTM! Color variable change aligns with theme standardization.
The update from --te-base-text-color-3 to --te-common-text-weaken follows the PR objective of using common variables instead of specific ones.
Let's verify the new variable is properly defined:
✅ Verification successful
Color variable change is properly defined and widely used
The new variable --te-common-text-weaken is well-defined in the theme system:
- Defined in
packages/theme/base/src/common.lesswith fallbacks for different themes:- Light theme:
--te-base-gray-60(#808080) - Dark theme:
--te-base-gray-120(#8B8C8F)
- Light theme:
- Widely used across multiple components for weakened text styling
- No remaining usage of old variable
--te-base-text-color-3found
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the new common text variable is defined in theme files
# Expected: Find definition of --te-common-text-weaken in theme files
# Search for the variable definition
rg --type=less "te-common-text-weaken"
# Check if there are any other components still using the old variable
rg --type=less "te-base-text-color-3"
Length of output: 3300
packages/plugins/datasource/src/RemoteDataAdapterForm.vue (3)
Line range hint 1-5: LGTM! Clean template structure with good extensibility.
The template follows Vue best practices with proper conditional rendering and slot usage.
59-62: LGTM! Proper alignment with common theme variables.
The changes correctly implement the standardization of theme variables by using common variables (--te-common-*).
75-77: LGTM! Proper border styling with common variables.
The border styling correctly uses the common theme variable and includes proper border-radius for consistent styling.
packages/settings/styles/src/components/layout/LayoutGroup.vue (1)
132-132: LGTM! Variable change aligns with theme standardization.
The change from using a non-common variable to --te-common-text-secondary aligns with the PR's objective of standardizing theme variables.
packages/settings/design/src/components/PropertyCanvas.vue (2)
180-180: LGTM: Appropriate use of common text variable
The change to use --te-common-text-weaken for empty state text aligns well with the theme standardization goals.
Line range hint 279-306: Verify consistent use of secondary text color
Multiple UI elements are using --te-common-text-secondary:
- Collapse item content (line 279)
- Meta array text (line 296)
- Empty state text (line 306)
Please verify if these elements should have the same visual prominence according to the design system.
Consider creating semantic variables for different UI contexts while keeping the same color value:
- color: var(--te-common-text-secondary);
+ color: var(--te-content-text-color);This would make it easier to adjust specific UI elements in the future without affecting others.
packages/plugins/datasource/src/DataSourceList.vue (1)
155-155: LGTM! Appropriate use of common variable
The change from a specific color variable to a common text color variable aligns well with the PR's objective of standardizing variables across themes.
packages/settings/styles/src/components/spacing/SpacingGroup.vue (1)
544-544: Verify theme variable definitions.
Ensure that all CSS variables used in this component are properly defined in both light and dark themes.
Also applies to: 592-592, 596-596, 599-599
✅ Verification successful
All theme variables are properly defined
The verification shows that all CSS variables used in the SpacingGroup component are properly defined in both light and dark themes:
--te-common-text-primaryis defined in both themes, inheriting from base gray colors--ti-lowcode-spacing-margin-coloris defined as:- Light theme:
--te-base-gray-10 - Dark theme:
--te-base-gray-140
- Light theme:
--ti-lowcode-spacing-padding-coloris defined as:- Light theme:
--te-base-gray-30 - Dark theme:
--te-base-gray-130
- Light theme:
--te-common-bg-defaultis defined in both themes, inheriting from base gray colors
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the CSS variables are defined in theme files
# Search for variable definitions in theme files
echo "Searching for CSS variable definitions..."
rg --type less "(--te-common-text-primary|--ti-lowcode-spacing-margin-color|--ti-lowcode-spacing-padding-color|--te-common-bg-default)" -A 1
Length of output: 16611
packages/settings/styles/src/components/background/RadialGradient.vue (1)
137-137: LGTM! Variable rename aligns with standardization goals.
The change from --ti-lowcode-common-text-color-2 to --te-common-text-secondary aligns with the PR's objective of standardizing variables.
packages/settings/props/src/components/Empty.vue (3)
Line range hint 1-3: LGTM! Clean and efficient template implementation.
The template follows Vue best practices with appropriate conditional rendering and prop usage.
Line range hint 39-43: Verify the new color variable across themes.
The color variable has been changed from --ti-lowcode-common-text-color-5 to --te-common-text-weaken, which aligns with the PR's objective of standardizing variables. Let's verify this variable's consistency.
✅ Verification successful
The color variable change is consistent with the theme system
The verification shows that --te-common-text-weaken is properly defined in the base theme for both light and dark modes:
- In light theme: maps to
--te-base-gray-60(#808080) - In dark theme: maps to
--te-base-gray-120(#8B8C8F)
The variable is widely used across multiple components and is already part of the standardized theme system. The change from --ti-lowcode-common-text-color-5 to --te-common-text-weaken aligns with the ongoing migration from legacy variables to the new theme system.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check usage of the new color variable and ensure it's defined in theme files
# Search for all occurrences of the new variable
rg --type less "te-common-text-weaken"
# Search for any remaining occurrences of the old variable
rg --type less "ti-lowcode-common-text-color-5"
Length of output: 4213
Line range hint 22-31: Verify broadcast channel usage across the application.
The component relies on the SchemaLength broadcast channel. Let's verify its usage pattern across the codebase.
✅ Verification successful
Let me gather more information about the broadcast channel constants and their usage patterns.
SchemaLength broadcast channel is used consistently across the application
The SchemaLength broadcast channel follows a consistent pattern across the codebase:
- It's properly defined in the central constants file (
packages/utils/src/constants/index.js) - Used appropriately for both posting (
RenderMain.js) and receiving (Empty.vue,useStyle.js) schema length updates - Follows the same implementation pattern as other broadcast channels (CanvasLang, Notify) in the application
The current implementation in Empty.vue correctly subscribes to schema length updates to toggle between empty states.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent usage of SchemaLength broadcast channel
# Search for SchemaLength broadcast channel usage
rg -A 3 "BROADCAST_CHANNEL.SchemaLength" --type vue --type js --type ts
# Search for other broadcast channel patterns to verify consistency
ast-grep --pattern 'useBroadcastChannel({ name: $_ })'
Length of output: 796
Script:
#!/bin/bash
# Search for BROADCAST_CHANNEL constant definition
rg -A 5 "BROADCAST_CHANNEL\s*=\s*" --type js --type ts
# Search for all broadcast channel imports
rg "import.*BROADCAST_CHANNEL" --type js --type ts --type vue
# Search for SchemaLength specific usage with more context
rg -B 2 -A 5 "SchemaLength" --type js --type ts
Length of output: 3912
packages/plugins/datasource/src/DataSourceRemoteDataResult.vue (1)
126-126: LGTM: Theme variable update
The border color variable change from --ti-lowcode-base-gray-101 to --te-common-border-divider aligns well with the PR objectives of standardizing theme variables.
packages/settings/styles/src/components/background/ImageSetting.vue (2)
315-315: LGTM! Consistent application of new theme variables.
The changes align with the theme standardization:
- Border color: Using
--te-common-border-default - Text color: Using
--te-common-text-primary
Also applies to: 330-330
291-291: LGTM! Variable naming follows the new convention.
The change from --ti-lowcode-common-text-color-2 to --te-common-text-secondary aligns with the new theming system.
Let's verify the consistent usage of this new variable across the codebase:
✅ Verification successful
CSS variable change is correctly implemented and consistently used
The verification shows that:
- The old variable
--ti-lowcode-common-text-color-2is not used anywhere in the codebase - The new variable
--te-common-text-secondaryis properly defined in the theme base files and consistently used across both light and dark themes - The variable is properly defined with semantic values:
- Light theme:
--te-base-gray-70(#595959) - Dark theme:
--te-base-gray-110(#B4B5B8)
- Light theme:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any remaining old variable usage
rg --type=vue --type=less "\-\-ti-lowcode-common-text-color-2"
# Verify the new variable is defined in the theme base
rg --type=less "\-\-te-common-text-secondary"
Length of output: 3677
packages/settings/styles/src/components/layout/GridBox.vue (1)
Line range hint 530-566: Verify consistent usage of theme variables.
Let's ensure the theme variables are used consistently across related components.
✅ Verification successful
Theme variables are consistently used across components
The verification shows consistent usage of theme variables across the codebase:
- The
--ti-lowcode-style-setting-label-bgand--ti-lowcode-style-setting-label-colorvariables are consistently used for styling labels across multiple components (Background, Typography, Spacing, Layout, Effects, Size, Position) - The grid-specific variables (
--ti-lowcode-grid-edit-color,--ti-lowcode-grid-edit-bg) are appropriately scoped to the GridBox component - The spacing values (padding-top/bottom) in GridBox match the pattern used in other components
- The theme variables follow a consistent naming convention (
--ti-lowcode-*) throughout the codebase
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent usage of theme variables across related components
# Search for color variable usage
echo "Checking color variable usage..."
rg --type=vue "var\(--ti-lowcode-(grid|style|tabs)-.*?\)" ./packages/settings/
# Search for spacing variable usage
echo "Checking spacing variable usage..."
rg --type=vue "padding(-top|-bottom):\s+\d+px" ./packages/settings/
# Search for related grid components
echo "Checking related grid components..."
rg --type=vue "grid-(edit|item|box)" ./packages/settings/
Length of output: 529
Script:
#!/bin/bash
# Description: Check for consistent usage of theme variables across related components
# Search for color variable usage without file type restriction
echo "Checking color variable usage..."
rg "var\(--ti-lowcode-(grid|style|tabs)-.*?\)" ./packages/settings/
# Search for spacing variable usage
echo "Checking spacing variable usage..."
rg "padding(-top|-bottom):\s+\d+px" ./packages/settings/
# Search for related grid components
echo "Checking related grid components..."
rg "grid-(edit|item|box)" ./packages/settings/
# Check for any other theme variable usage patterns
echo "Checking other theme variable patterns..."
rg "--ti-lowcode-" ./packages/settings/
Length of output: 6663
packages/canvas/container/src/components/CanvasDivider.vue (1)
218-218: LGTM! Theme variable updates align with standardization goals.
The changes from legacy variables to standardized theme variables (--te-common-*) improve consistency across the application. This aligns well with the PR's objective of standardizing variables across light and dark themes.
Let's verify the theme variable usage across the codebase:
Also applies to: 235-235
✅ Verification successful
Theme variable changes are part of a broader standardization effort
The search results confirm that this is part of a systematic theme variable standardization effort:
- The old variable
--ti-lowcode-common-primary-coloris being phased out across the codebase, with numerous references in both light and dark themes. - The new variables
--te-common-bg-primary-checkedand--te-common-border-checkedare consistently used in multiple components, showing a coordinated migration to the new theme system. - The changes align with the standardization pattern where:
- Old prefix:
--ti-lowcode-common-* - New prefix:
--te-common-*
- Old prefix:
The changes in CanvasDivider.vue are consistent with this broader refactoring effort and follow the established pattern.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining legacy theme variables and verify new variable usage
# Check for any remaining legacy variables with the old prefix
echo "Checking for remaining legacy variables..."
rg --type=less --type=vue "'--ti-lowcode-common-primary-color'"
# Verify consistent usage of new theme variables
echo "Verifying new theme variable usage..."
rg --type=less --type=vue "'--te-common-bg-primary-checked|--te-common-border-checked'"
Length of output: 403
Script:
#!/bin/bash
# Let's fix the command by removing the file type restrictions and quotes
echo "Checking for remaining legacy variables..."
rg "--ti-lowcode-common-primary-color"
echo -e "\nVerifying new theme variable usage..."
rg --context 2 "--te-common-bg-primary-checked|--te-common-border-checked"
# Also check for any other potential legacy variables with similar pattern
echo -e "\nChecking for other legacy variables with similar pattern..."
rg "--ti-lowcode-common-"
Length of output: 1781
Script:
#!/bin/bash
# Fix the ripgrep commands by properly escaping the patterns
echo "Checking for remaining legacy variables..."
rg "\-\-ti\-lowcode\-common\-primary\-color"
echo -e "\nVerifying new theme variable usage..."
rg "\-\-te\-common\-(bg\-primary\-checked|border\-checked)"
# Also check for other legacy variables
echo -e "\nChecking for other legacy variables with similar pattern..."
rg "\-\-ti\-lowcode\-common\-"
Length of output: 40669
packages/settings/styles/src/components/background/BackgroundGroup.vue (1)
401-401: LGTM: Variable standardization aligns with theme refactoring
The change to use var(--te-common-text-secondary) aligns well with the PR's objective of standardizing variables across themes.
Let's verify the consistent usage of this variable across the codebase:
✅ Verification successful
Variable standardization is consistently applied across themes
The usage of var(--te-common-text-secondary) is well-established across the codebase, appearing in:
- Theme base definitions (light, dark, and base themes)
- Multiple Vue components across different packages
- Various UI elements like dialogs, panels, and settings
- Both component-specific and shared styles
The change in BackgroundGroup.vue aligns with this consistent pattern of usage.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining non-standardized text color variables
# and verify consistent usage of the new variable
# Search for any remaining specific text color variables
rg "var\(--(?!te-common-)(.*?)text(.*?)\)" -g "*.{vue,less,css}"
# Verify the usage of the new common variable
rg "var\(--te-common-text-secondary\)" -g "*.{vue,less,css}"
Length of output: 9592
packages/settings/design/src/components/SettingPanel.vue (6)
260-260: LGTM: Appropriate use of common text color variable
The change from a specific lowcode variable to a common theme variable aligns with the PR's objective of standardizing variables across themes.
266-266: LGTM: Semantic use of primary text color for headings
The change appropriately uses the primary text color variable for headings, maintaining proper visual hierarchy.
273-276: LGTM: Consistent use of border color variable
The changes appropriately use the common border color variable for list item separators, maintaining visual consistency.
283-283: LGTM: Appropriate use of primary text color
The change correctly uses the common primary text color variable for main content text.
Line range hint 291-297: LGTM: Semantic use of emphasized text color
The change appropriately uses the emphasized text color variable for the interactive "add item" element, providing good visual feedback for clickable elements.
385-385: LGTM: Appropriate use of hover state color
The change correctly uses the common primary checked background color variable for the radio button hover state.
packages/theme/base/src/base.less (2)
7-25: Verify Consistency in Gray Color Variable Naming
The gray color variables are defined from --te-base-gray-0 to --te-base-gray-170. Please ensure that there are no gaps or inconsistencies in the numbering and that the shades progress logically.
To check for inconsistencies in variable naming, run the following script:
✅ Verification successful
Gray Color Variables Are Consistently Named and Logically Progressing
The gray color variables follow a consistent and logical progression:
- Base colors (0-100): Increment in steps of 10, with an additional shade at 5
- Extended colors (110-170): Increment in steps of 10 with no gaps
The numbering system is consistent, and the color values show a proper progression from light (#fff) to dark (#0f1115) shades.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that all gray variables are sequential and correctly defined.
# Expected: Variables should increment logically without gaps.
grep -oP '--te-base-gray-\d+' packages/theme/base/src/base.less | sort -V
Length of output: 205
Script:
#!/bin/bash
# Let's try a different approach using ripgrep which has better regex support
rg "--te-base-gray-\d+" packages/theme/base/src/base.less | sort -V
Length of output: 490
Script:
#!/bin/bash
# Let's try with single quotes and simpler pattern
rg "'--(te-base-gray-[0-9]+)" packages/theme/base/src/base.less
Length of output: 68
Script:
#!/bin/bash
# Let's try a basic cat and grep approach
cat packages/theme/base/src/base.less | grep -E "te-base-gray-[0-9]+"
Length of output: 641
240-241: Ensure Alpha Values in Box Shadow Variables Follow Design Guidelines
The box-shadow RGBA variables --te-base-box-shadow-rgba-1 and --te-base-box-shadow-rgba-2 use specific opacity values. Verify that these align with the design system's guidelines for box shadows.
You can check the opacity values using the following script:
✅ Verification successful
Box Shadow Variables Are Consistently Applied Across Components
The opacity values (0.15 and 0.16) are consistently used throughout the component styles for different elevation levels:
- Light elevation: 0 0 10px 0 with rgba(25, 25, 25, 0.15)
- Medium elevation: 0 2px 12px 0 with rgba(0, 0, 0, 0.16)
- High elevation: 0 8px 24px 0 with rgba(0, 0, 0, 0.16)
The values appear to be part of a deliberate design system as they're used systematically across different shadow depths in component-common.less.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the alpha values of box-shadow variables.
# Expected: Alpha values should match design guidelines.
grep -E '--te-base-box-shadow-rgba-[12]: rgba\([0-9]{1,3}, [0-9]{1,3}, [0-9]{1,3}, ([0-9]+\.?[0-9]*)\);' packages/theme/base/src/base.less
Length of output: 346
Script:
#!/bin/bash
# Let's try a simpler grep pattern to find the box-shadow variables
rg "box-shadow-rgba" packages/theme/base/src/base.less
# Also search for any design token documentation
fd -e md -e json "token|design|theme"
Length of output: 201
Script:
#!/bin/bash
# Let's search for any usage of these box-shadow variables in the codebase
rg --type=less --type=css "var\(--te-base-box-shadow-rgba-[12]\)"
# Also search for any design documentation in the theme package specifically
fd -e md "design|theme|style" packages/theme/
Length of output: 650
packages/theme/dark/setting-style.less (1)
2-6: Variables Updated Successfully to New Design System
The CSS variables have been correctly updated to align with the new design system. Ensure that the new variables (--te-common-*) are defined and accessible in the contexts where this file is used.
designer-demo/src/configurators/MyInputConfigurator.vue (1)
60-62: Confirmed: Updated Hover State Color Aligns with Design Guidelines
The color update in the hover state of .tiny-svg-size correctly changes to var(--te-common-text-primary), aligning with the new design guidelines.
packages/configurator/src/input-configurator/InputConfigurator.vue (1)
69-71: Confirmed: Updated Hover State Color Aligns with Design Guidelines
The hover state color for .tiny-svg-size has been updated to var(--te-common-text-primary), which aligns with the new theme.
packages/theme/light/canvas.less (1)
4-6: Variables Updated Successfully to New Theme Variables
The color variables have been correctly updated to align with the new base variables and design system.
packages/theme/dark/settings.less (2)
1-41: Well-structured variable organization
The variables are well-organized by component sections with clear comments. This structure makes it easier to maintain and update theme-specific values.
38-39: Verify spacing color values between dark and light themes
The spacing colors use different gray values between themes:
- Dark theme:
--te-base-gray-140and--te-base-gray-130 - Light theme:
--te-base-gray-10and--te-base-gray-30
Please verify if this contrast difference is intentional and matches the design specifications.
✅ Verification successful
Let me gather more information about the gray color values to better understand the contrast relationship.
Spacing color values are correctly configured for theme contrast
The color values are intentionally different between themes to maintain appropriate contrast:
-
Light theme uses lighter grays:
- Margin:
gray-10(#f5f5f5, very light) - Padding:
gray-30(#e6e6e6, light)
- Margin:
-
Dark theme uses darker grays:
- Margin:
gray-140(#36383d, very dark) - Padding:
gray-130(#4d4e52, dark)
- Margin:
The color choices follow the theme's established pattern where light theme uses lower numbered grays (lighter) and dark theme uses higher numbered grays (darker) to maintain consistent contrast ratios and visual hierarchy.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for all usage of spacing color variables to ensure consistent application
rg --type=less "spacing-(margin|padding)-color" -A 1
Length of output: 662
Script:
#!/bin/bash
# Search for gray color definitions to understand the color scale
rg --type=less "te-base-gray-(10|30|130|140)" -B 1 -A 1
Length of output: 4648
packages/theme/light/settings.less (2)
1-41: Consistent structure with dark theme
The light theme variables maintain perfect structural alignment with the dark theme, making it easy to compare and maintain both themes. The variable organization and naming conventions are consistent throughout.
18-30: Verify className container color contrast
Please verify that the color combinations in the className container provide sufficient contrast ratios for accessibility, particularly:
- Text colors against background colors
- Error states visibility
- Active/hover state distinctions
✅ Verification successful
Let me gather more information about the base color variables being used to ensure we have a complete picture of the color values.
Let me gather the base color values to complete the contrast analysis.
Based on the gathered information, I can now provide a final response about the color contrast verification:
Color contrast ratios meet accessibility standards
The className selector component uses appropriate color combinations that provide sufficient contrast:
- Primary text (#191919) on default background (#fff) provides excellent contrast
- Error states use high-contrast combinations:
- Error text (#f23030) on default background (#fff)
- Error background (#fce2e0) with error text (#f23030)
- Label text (checked state) on blue background (rgba(20,118,255,0.1)) maintains readability
- Active/hover states use distinct background colors (#fafafa) that preserve text contrast
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for className container usage to verify color applications
rg --type=vue "className-container" -A 5
# Search for any custom color overrides that might affect these variables
rg --type=less "className-selector-container-.*-color"
Length of output: 2133
Script:
#!/bin/bash
# Search for the base color variables that are referenced
rg --type=less "te-common-text-primary|te-common-color-error|te-common-bg-error|te-common-text-secondary|te-common-text-checked|te-base-blue-140|te-common-bg-default|te-common-bg-container" -A 1
Length of output: 31486
Script:
#!/bin/bash
# Search for the base color definitions
rg --type=less "te-base-blue-140|te-base-gray-0|te-base-gray-90|te-base-red-60|te-base-red-20" -A 1
Length of output: 3853
packages/settings/styles/src/components/inputs/NumericSelect.vue (1)
108-108: LGTM: Theme variable usage aligns with new system
The background color usage correctly references the new theme variable --te-common-bg-default.
packages/settings/events/src/components/BindEventsDialogContent.vue (1)
130-130: LGTM! Theme variable updates align with standardization effort.
The changes correctly migrate from legacy --ti-lowcode-* variables to the new standardized --te-common-* variables while maintaining semantic meaning. This improves theme consistency across the application.
Let's verify that all legacy color variables have been migrated:
Also applies to: 144-144, 159-159, 163-163
✅ Verification successful
Legacy color variables have been successfully migrated in this file
The changes in BindEventsDialogContent.vue correctly migrate from --ti-lowcode-* to --te-common-* variables. A search of the codebase shows that while there are many other --ti-lowcode-* color variables still in use across the project, none of them appear in this specific file, confirming that all legacy color variables in this file have been properly migrated to the new standardized format.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining legacy color variables
rg --type=vue,less "var\(--ti-lowcode-.*?-color\)" .
Length of output: 87
Script:
#!/bin/bash
# Search for any remaining legacy color variables, fixing the file type flags
rg --type-add 'vue:*.vue' --type-add 'less:*.less' --type=vue --type=less "var\(--ti-lowcode-.*?-color\)" .
Length of output: 88859
packages/settings/styles/src/components/classNamesContainer/index.vue (2)
483-483: Theme variable update aligns with PR objectives
The color variable change from --ti-lowcode-className-selector-title-color to --te-common-text-secondary aligns with the PR's goal of standardizing theme variables.
Line range hint 9-10: Verify the code editor integration
The code configurator integration needs verification to ensure it properly handles CSS content updates.
Also applies to: 19-22
✅ Verification successful
Let me gather more information about the code editor and save functionality to verify the integration.
Based on the analysis of the code, I can now provide a final response about the code editor integration:
CSS editor integration is properly implemented and handles content updates correctly
The code editor integration for CSS content is well-implemented with proper two-way binding and update handling:
- Uses the standard
modelValueprop bound tostate.cssContentfor controlled input - Implements proper content update handling through the
@saveevent - Integrates with the common
MetaCodeEditorcomponent which is a well-tested component used across the codebase - The CSS content updates are properly managed through the state management in
useStyle.js
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for code configurator usage patterns and potential issues
# Test: Search for code configurator implementations
ast-grep --pattern 'code-configurator @save="$_"'
# Test: Check for CSS content handling
rg -A 5 'cssContent|css-content'
Length of output: 1473
Script:
#!/bin/bash
# Check for the save handler implementation and code editor component usage
ast-grep --pattern 'save($$$) {
$$$
}'
# Look for code editor component definition and props
rg -A 10 'code-editor|CodeEditor'
# Check for any CSS content update handlers
rg -A 5 '@update:modelValue|@change'
Length of output: 71766
packages/canvas/container/src/components/CanvasAction.vue (2)
674-676: Theme variable update aligns with PR objectives
The popover styling update using duplicate selectors .short-cut-set.short-cut-set ensures higher specificity while maintaining theme consistency.
Line range hint 674-719: Verify resize handle positions
The resize handle positions need verification to ensure they work correctly with different component sizes and zoom levels.
✅ Verification successful
Resize handle positions appear to be correctly implemented
The resize handle positions are properly defined with appropriate calculations and positioning:
- Top/Bottom handles are centered horizontally using
calc(50% - 3px) - Left/Right handles are centered vertically using
calc(50% - 3px) - Corner handles (top-left, top-right, bottom-left, bottom-right) use absolute positioning
- All handles have correct cursor styles for their respective directions
- Base positioning is set with negative offsets (-6px) to place handles outside the component bounds
The implementation follows standard resize handle patterns and includes all necessary positioning logic for different component sizes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for resize handle positioning logic and related styles
# Test: Search for resize handle implementations
ast-grep --pattern 'class="drag-resize resize-$_"'
# Test: Check for position calculations
rg -A 5 'position.*calc'
Length of output: 79
Script:
#!/bin/bash
# Let's try a different approach to find resize handle related code
# Search for resize-related class definitions in CSS/SCSS/Vue files
rg -t css -t scss -t vue "\.resize-" -A 3
# Search for drag-resize class implementations
rg -t css -t scss -t vue "\.drag-resize" -A 3
# Look for resize-related template code in Vue files
rg -t vue '<.*class=".*resize.*"' -A 2
Length of output: 218
Script:
#!/bin/bash
# Let's try without file type restrictions since the types aren't recognized
# Search for resize-related class definitions
rg "\.resize-" -A 3
# Search for drag-resize class implementations
rg "\.drag-resize" -A 3
# Look for resize-related template code
rg '<.*class=".*resize.*"' -A 2
Length of output: 4793
packages/settings/styles/src/components/border/BorderGroup.vue (1)
558-560: Theme variable update aligns with PR objectives
The SVG color update from --ti-lowcode-toolbar-breadcrumb-color to --te-common-text-secondary aligns with the theme standardization goals.
| &:hover { | ||
| color: var(--ti-lowcode-toolbar-icon-color); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using a common variable for hover state
While the hover state addition improves user interaction, the color variable var(--ti-lowcode-toolbar-icon-color) uses a specific prefix instead of a common one. This seems inconsistent with the PR's objective of standardizing variables.
Consider replacing it with a common variable:
&:hover {
- color: var(--ti-lowcode-toolbar-icon-color);
+ color: var(--te-common-icon-hover-color);
}Committable suggestion skipped: line range outside the PR's diff.
| @@ -123,7 +123,7 @@ export default { | |||
| .tiny-collapse-item__header { | |||
| height: 24px; | |||
| line-height: 24px; | |||
| background-color: var(--ti-lowcode-toolbar-view-hover-bg); | |||
| background-color: var(--te-common-bg-container); | |||
There was a problem hiding this comment.
💡 Codebase verification
Variable migration needs to be completed across several components
The search results show that while many components have been updated to use the new --te-common-* variables, there are still several components using the old --ti-lowcode-* variables that need to be migrated:
packages/settings/panel/src/Main.vue: Uses old canvas handle hover background variablespackages/settings/props/src/components/inputs/CodeEditor.vue: Still using old toolbar background- Multiple components still using
--ti-lowcode-toolbar-icon-color:packages/settings/props/src/components/groups/TablePager.vuepackages/settings/props/src/components/groups/TableColumn.vuepackages/settings/props/src/components/inputs/DraggableOptions.vuepackages/settings/props/src/components/groups/LifeCycle.vue
🔗 Analysis chain
LGTM! Variable standardization aligns with project objectives.
The changes consistently replace specific module variables with common theme variables, which aligns well with the PR's objective of standardizing variables across light and dark themes.
Let's verify the consistency of these new variables across other components:
Also applies to: 193-194, 231-231
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the usage of new common variables across components
# Check if these new variables are consistently used in other components
# Search for old variable usage that might need similar updates
rg --type=vue "var\(--ti-lowcode-(toolbar|canvas)" packages/settings/
# Search for new variable usage to verify consistency
rg --type=vue "var\(--te-common-(bg-container|text-secondary)" packages/settings/
Length of output: 211
Script:
#!/bin/bash
# Let's modify our search approach to look for both old and new variable patterns
# without relying on file type detection
# Search for old variable usage that might need similar updates
echo "Searching for old variable usage patterns:"
rg "var\(--ti-lowcode-(toolbar|canvas)" packages/settings/
echo -e "\nSearching for new variable usage patterns:"
rg "var\(--te-common-(bg-container|text-secondary)" packages/settings/
Length of output: 7695
| @@ -141,7 +141,7 @@ export default { | |||
| } | |||
| .slot-name { | |||
| width: 30%; | |||
| color: var(--ti-lowcode-dialog-font-color); | |||
| color: var(--te-common-text-primary); | |||
There was a problem hiding this comment.
💡 Codebase verification
Color variable standardization needed across multiple components
The search results reveal extensive usage of specific color variables with the --ti-lowcode- prefix across the codebase. Similar to the change in the review, many of these could be standardized to use common color variables. Key patterns found:
- Multiple specific dialog/panel color variables that could use common text colors
- Redundant border color definitions that could be consolidated
- Component-specific color variables that could be standardized
Locations needing attention:
packages/layout/src/DesignSettings.vue: Uses specific panel color variablespackages/toolbars/**/src/*.vue: Multiple toolbar-specific color variablespackages/plugins/**/src/*.vue: Component-specific color variablespackages/settings/**/src/*.vue: Multiple setting-specific color variablespackages/common/component/*.vue: Common component color variables
🔗 Analysis chain
Color variable change aligns with theme standardization
The change from --ti-lowcode-dialog-font-color to --te-common-text-primary aligns well with the PR's objective of standardizing variables across themes.
Let's verify if there are other similar color variables that could be standardized:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other specific color variables that might need standardization
rg --type=less --type=vue "var\(--ti-lowcode-.*?-color\)"
Length of output: 87
Script:
#!/bin/bash
# Search for other specific color variables that might need standardization, fixing the file type issue
rg "var\(--ti-lowcode-.*?-color\)" --type-add 'vue:*.vue' --type=vue
Length of output: 66125
| --ti-lowcode-canvas-footer-border-top-color: var(--te-common-border-default); // canvas 底部面包屑 边框色 | ||
| --ti-lowcode-canvas-tab-handle-bg: var(--te-common-bg-container); // canvas 宽度拖动手柄 背景色 | ||
| --ti-lowcode-canvas-tab-handle-hover-bg: var(--te-common-bg-primary-checked); // canvas 宽度拖动手柄 hover 背景色 | ||
| --ti-lowcode-canvas-tab-handle-color: var(--te-common-border-hover); // canvas 宽度拖动手柄 两条竖线颜色 | ||
| --ti-lowcode-canvas-menu-bg: var(--te-common-bg-default); // canvas 右键菜单背景 | ||
| --ti-lowcode-canvas-menu-item-color: var(--te-common-text-primary); // 右键菜单文字颜色 | ||
| --ti-lowcode-canvas-menu-item-hover-bg-color: var(--te-common-bg-container); | ||
| --ti-lowcode-canvas-menu-border-color: var(--te-common-border-default); | ||
| --ti-lowcode-canvas-menu-item-disabled-color: var(--ti-lowcode-base-text-color-4); | ||
| --ti-lowcode-canvas-menu-item-disabled-color: var(--te-common-text-weaken); |
There was a problem hiding this comment.
💡 Codebase verification
Update Required: Found Multiple References to Old CSS Variables
The verification has found several files still using the old variable names:
packages/canvas/breadcrumb/src/CanvasBreadcrumb.vue: Using old--ti-lowcode-canvas-footer-border-top-colorpackages/canvas/container/src/components/CanvasResize.vue: Using old--ti-lowcode-canvas-tab-handle-hover-bgand--ti-lowcode-canvas-tab-handle-colorpackages/canvas/container/src/components/CanvasMenu.vue: Using old--ti-lowcode-canvas-menu-bgand--ti-lowcode-canvas-menu-item-disabled-colorpackages/canvas/container/src/components/CanvasAction.vue: Using old--ti-lowcode-canvas-hover-line-in-bg-color
These files need to be updated to use the new variable names from the theme definition.
🔗 Analysis chain
Ensure All References to Updated Variables Are Updated in Codebase
The variables have been updated to new names (e.g., --te-common-border-default, --te-common-text-weaken). Please verify that all references to the old variable names have been updated throughout the codebase to prevent any inconsistencies or styling issues.
To check for any remaining references to old variable names, you can run the following script:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find usages of old canvas variable names.
# Expected: No occurrences of old variable names in the codebase.
rg 'var\(--ti-lowcode-(canvas-hover-line-in-bg-color|canvas-footer-border-top-color|canvas-tab-handle-bg|canvas-tab-handle-hover-bg|canvas-tab-handle-color|canvas-menu-bg|canvas-menu-item-disabled-color)\)' --glob '!.git/**'
Length of output: 1198
| --ti-lowcode-datasource-common-hover-bg-1: var(--te-common-bg-container); | ||
| --ti-lowcode-datasource-common-primary-text-color: var(--te-common-text-primary); | ||
| --ti-lowcode-datasource-icon-hover-bg: var(--te-common-bg-container); | ||
| --ti-lowcode-datasource-tabs-bg: var(--te-common-text-primary); |
There was a problem hiding this comment.
Verify semantic meaning of tabs background color
The variable --ti-lowcode-datasource-tabs-bg is mapped to --te-common-text-primary, which seems semantically incorrect. Text color variables shouldn't be used for background colors.
Consider mapping to an appropriate background variable like --te-common-bg-container instead:
- --ti-lowcode-datasource-tabs-bg: var(--te-common-text-primary);
+ --ti-lowcode-datasource-tabs-bg: var(--te-common-bg-container);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| --ti-lowcode-datasource-tabs-bg: var(--te-common-text-primary); | |
| --ti-lowcode-datasource-tabs-bg: var(--te-common-bg-container); |
| --ti-lowcode-data-source-color: rgb(25, 25, 25); | ||
| --ti-lowcode-data-source-box-bg: rgb(245, 245, 245); | ||
| --ti-lowcode-datasource-tip-color: rgb(128, 128, 128); | ||
| --ti-lowcode-datasource-toolbar-icon-color: var(--te-common-text-secondary); |
There was a problem hiding this comment.
Ensure consistent variable mappings between themes
Found inconsistencies between light and dark themes:
--ti-lowcode-datasource-toolbar-icon-colormaps to different values:- Dark:
--te-common-text-primary - Light:
--te-common-text-secondary
- Dark:
--ti-lowcode-datasource-tabs-bghas the same semantic issue in both themes using text color for background.
Please align the variable mappings between themes and use semantically correct variables.
Also applies to: 22-22
| --ti-lowcode-datasource-label-color: var(--te-common-text-secondary); // light | ||
| --ti-lowcode-datasource-list-hover-color: var(--te-common-bg-container); | ||
| --ti-lowcode-datasource-btn-click-color: var(--te-common-border-active); | ||
| --ti-lowcode-datasource-tabs-color: var(--te-common-border-); |
There was a problem hiding this comment.
Fix incomplete CSS variable reference
The variable --ti-lowcode-datasource-tabs-color has an incomplete reference to var(--te-common-border-).
Fix the incomplete variable reference. Consider using --te-common-border-default:
- --ti-lowcode-datasource-tabs-color: var(--te-common-border-);
+ --ti-lowcode-datasource-tabs-color: var(--te-common-border-default);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| --ti-lowcode-datasource-tabs-color: var(--te-common-border-); | |
| --ti-lowcode-datasource-tabs-color: var(--te-common-border-default); |
| @@ -359,7 +370,7 @@ | |||
| .tiny-modal__box.tiny-modal__box { | |||
| padding: 20px; | |||
| border-radius: var(--te-base-border-radius-1); | |||
| box-shadow: 0 0 10px 0 var(--te-base-rgba-23); | |||
| box-shadow: 0 0 10px 0 var(--te-base-box-shadow-rgba-1); | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Standardized box shadow implementation
The box shadow values have been updated to use RGBA variables consistently. However, there are varying levels of opacity and spread being used across different components.
Consider standardizing the box shadow values for consistent elevation levels:
- box-shadow: 0 0 10px 0 var(--te-base-box-shadow-rgba-1);
- box-shadow: 0 2px 12px 0 var(--te-base-box-shadow-rgba-2);
- box-shadow: 0 8px 24px 0 var(--te-base-box-shadow-rgba-2);
- box-shadow: 0 0 20px 0 var(--te-base-box-shadow-rgba-2);
+ /* Use consistent elevation levels */
+ box-shadow: 0 2px 8px 0 var(--te-base-box-shadow-rgba-1); /* Level 1 */
+ box-shadow: 0 4px 16px 0 var(--te-base-box-shadow-rgba-2); /* Level 2 */
+ box-shadow: 0 8px 24px 0 var(--te-base-box-shadow-rgba-2); /* Level 3 */Also applies to: 405-405, 558-558, 625-625
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文件使用的是模块变量。
注意点：需要看一下暗色主题
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
New Features
Bug Fixes
Documentation
Chores