Open feat(theme/datasource&setting): change datasource and setting panel theme variable #928
Conversation
|
Warning Rate limit exceeded@wenmine has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 0 minutes and 45 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe pull request introduces a series of cosmetic changes across multiple Vue components, primarily focusing on updating CSS styles to align with a new design system. Changes include modifications to color properties for various classes, ensuring a consistent visual appearance. The underlying functionality and logic of the components remain unchanged, with no alterations to props, methods, or event handling. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Outside diff range and nitpick comments (34)
packages/settings/props/src/components/groups/TablePager.vue (1)
Line range hint
74-74: Update remaining legacy theme variableThere's still a legacy theme variable (
--ti-lowcode-toolbar-icon-color) that should be updated to match the new theme system.- color: var(--ti-lowcode-toolbar-icon-color); + color: var(--te-common-text-secondary);packages/settings/styles/src/components/layout/LayoutGroup.vue (1)
Line range hint
139-139: Replace legacy theme variableThe background color still uses the legacy
--ti-lowcode-style-setting-label-bgvariable, which doesn't align with the PR's objective of eliminating "ti-lowcode" references and standardizing theme variables.Consider replacing with an appropriate
--te-common-*variable:- background-color: var(--ti-lowcode-style-setting-label-bg); + background-color: var(--te-common-bg-emphasize);packages/settings/styles/src/Main.vue (2)
Line range hint
239-267: Standardize remaining theme variablesThere are several other
--ti-lowcodeprefixed variables that should be updated to use common theme variables for consistency:
--ti-lowcode-setting-style-btn-font-color--ti-lowcode-setting-style-btn-border-color--ti-lowcode-setting-style-input-bg--ti-lowcode-setting-style-input-font-colorThis aligns with the PR objective to "eliminate references to ti-lowcode" and standardize variables between light and dark themes.
Consider updating these variables to their corresponding common theme variables. For example:
.tiny-button { - color: var(--ti-lowcode-setting-style-btn-font-color); + color: var(--te-common-text-primary); } .tiny-button:hover { background: none; - border-color: var(--ti-lowcode-setting-style-btn-border-color); + border-color: var(--te-common-border-light); } .inline-bind-style { :deep(.tiny-input__inner) { - background: var(--ti-lowcode-setting-style-input-bg); - color: var(--ti-lowcode-setting-style-input-font-color); - border-color: var(--ti-lowcode-setting-style-input-bg); + background: var(--te-common-bg-dark); + color: var(--te-common-text-primary); + border-color: var(--te-common-border-light); } }
Line range hint
223-235: Consider consolidating color definitionsThe style block for
.line-stylehas a color definition that appears redundant with the.line-textcolor:.line-style { color: var(--ti-lowcode-setting-style-font-color); // This might be unnecessary .line-text { color: var(--te-common-text-secondary); } }Consider removing the parent color if it's not being used by other elements.
packages/settings/styles/src/components/background/BackgroundGroup.vue (1)
Line range hint
424-424: Migrate remaining ti-lowcode variables to common variablesSeveral CSS variables still use the "ti-lowcode" prefix, which should be migrated to common variables according to the PR objectives:
--ti-lowcode-optionitem-background-color--ti-lowcode-setting-style-drag-bar-bg--ti-lowcode-style-setting-label-bgConsider replacing these with appropriate common variables from the design system. For example:
- background-color: var(--ti-lowcode-optionitem-background-color); + background-color: var(--te-common-bg-container); - background-color: var(--ti-lowcode-setting-style-drag-bar-bg); + background-color: var(--te-common-bg-emphasize); - background-color: var(--ti-lowcode-style-setting-label-bg); + background-color: var(--te-common-bg-emphasize);Please verify these suggested replacements with the design specifications to ensure they match the intended visual appearance.
Also applies to: 476-476, 489-489, 583-583
packages/settings/styles/src/components/background/ImageSetting.vue (1)
Line range hint
315-330: Review button interaction consistencyWhile the theme variable changes from
ti-lowcodetote-commonprefixes are good, there's a potential UX inconsistency:
- The button has clickable styling (border, centered text)
- But it's set to
cursor: defaultwhich suggests non-interactivityConsider one of these approaches:
- If the button should be interactive:
- cursor: default; + cursor: pointer;
- If the button should be non-interactive:
- border-width: 1px; - border-style: solid; + background-color: var(--te-common-bg-disabled); + color: var(--te-common-text-disabled);packages/settings/styles/src/components/layout/GridBox.vue (2)
Line range hint
530-562: Update theme variables to align with new standardsThe code is still using the old
--ti-lowcode-prefixed variables which should be updated according to the PR objectives. Consider replacing them with the new common theme variables.Apply this diff to update the theme variables:
.is-setting { - color: var(--ti-lowcode-style-setting-label-color); - background-color: var(--ti-lowcode-style-setting-label-bg); + color: var(--te-common-text-primary); + background-color: var(--te-common-bg-selected); }
Line range hint
479-489: Additional theme variables need updatingThere are more instances of old theme variables that should be updated for consistency:
Apply this diff to update the remaining theme variables:
.grid-edit-btn { /* ... other properties ... */ - border: 1px solid var(--ti-lowcode-tabs-border-color); - color: var(--ti-lowcode-grid-edit-color); - background: var(--ti-lowcode-grid-edit-bg); + border: 1px solid var(--te-common-border-color); + color: var(--te-common-text-secondary); + background: var(--te-common-bg-container); /* ... other properties ... */ }packages/plugins/datasource/src/RemoteDataAdapterForm.vue (1)
Line range hint
68-68: Standardize the icon color variableThe icon color still uses the old
ti-lowcodeprefix (var(--ti-lowcode-datasource-toolbar-icon-color)), while other variables have been updated to use the newte-commonprefix.Consider updating to a common theme variable to maintain consistency:
- color: var(--ti-lowcode-datasource-toolbar-icon-color); + color: var(--te-common-text-secondary);packages/engine-cli/template/designer/src/configurators/MyInputConfigurator.vue (2)
Line range hint
1-4: Implement suffix icons renderingThe component accepts
suffixIconsas a prop but doesn't render them in the template. This creates a disconnect between the component's API and its implementation.Consider implementing the suffix icons like this:
<tiny-input v-model="value" :type="type" :placeholder="placeholder" :rows="rows" @update:modelValue="change"> + <template #suffix> + <span v-for="(icon, index) in suffixIcons" :key="index" class="tiny-svg-size"> + <component :is="icon" /> + </span> + </template> </tiny-input>
Line range hint
1-1: Consider internationalizing the hardcoded Chinese textThe text "我是自定义的 input configurator" should be internationalized to support multiple languages.
Consider using a translation key:
-<span>我是自定义的 input configurator</span> +<span>{{ t('configurator.customInput.title') }}</span>packages/plugins/state/src/DataSourceRightPanel.vue (1)
Theme variable changes need to be updated consistently
The search results show that the new
--te-common-*theme variables are only used in the file under review, while the rest of the codebase is still using the old--ti-lowcode-*theme variables. This inconsistency should be addressed to maintain a uniform theming system.
packages/plugins/state/src/DataSourceRightPanel.vueis using new--te-common-*variables- Multiple other files are still using the old
--ti-lowcode-*variables- The theme definitions in
packages/theme/light/andpackages/theme/dark/are using the old variable system🔗 Analysis chain
Line range hint
116-130: Verify theme variable changes across the codebaseLet's ensure these theme variable changes are consistent with other components and that we haven't missed any occurrences.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining old theme variables that should be updated echo "Checking for remaining ti-lowcode theme variables..." rg --type=vue "var\(--ti-lowcode-[^)]+\)" packages/ # Search for usage of new theme variables to ensure consistency echo "Checking usage of new theme variables..." rg --type=vue "var\(--te-common-[^)]+\)" packages/Length of output: 368
Script:
#!/bin/bash # Search for any remaining old theme variables that should be updated echo "Checking for remaining ti-lowcode theme variables..." rg "var\(--ti-lowcode-[^)]+\)" packages/ # Search for usage of new theme variables to ensure consistency echo "Checking usage of new theme variables..." rg "var\(--te-common-[^)]+\)" packages/Length of output: 81953
packages/configurator/src/slot-configurator/SlotConfigurator.vue (2)
Line range hint
89-93: Consider internationalizing modal messagesThe confirmation modal messages are hardcoded in Chinese. Consider extracting these strings to a localization system for better maintainability and internationalization support.
Example approach:
- title: '提示', - message: '关闭后插槽内的内容将被清空，是否继续？', + title: t('common.hint'), + message: t('slot.confirmUnbind'),
Line range hint
151-207: Consider using TinyVue Switch componentThe current implementation uses a custom switch styling. Consider using the
@opentiny/vueSwitch component for consistency with the design system and to reduce maintenance overhead.Example approach:
- <div :class="['e__switch', { 'e_is-checked': slot.bind }]"> - <span class="e__switch-core" @click="toggleSlot(slot, index)"></span> - </div> + <tiny-switch + v-model="slot.bind" + @change="(val) => toggleSlot(slot, index)" + />This would also allow removing the custom switch styles (lines 151-207).
packages/plugins/datasource/src/DataSourceList.vue (1)
Line range hint
142-180: Consider updating remaining legacy theme variables in future PRs.Several other
--ti-lowcode-*prefixed variables are still present in the styles:
--ti-lowcode-data-source-border-color--ti-lowcode-datasource-tabs-border-color--ti-lowcode-datasource-common-text-main-color--ti-lowcode-datasource-list-hover-color--ti-lowcode-datasource-toolbar-more-hover-colorThese could be standardized in future PRs to maintain consistency with the new theme system.
packages/settings/styles/src/components/spacing/SpacingGroup.vue (2)
6-6: Improved theme consistency with semantic class namesThe transition from hardcoded colors to CSS variables and more semantic class names (
padding-color/margin-color) improves maintainability and theme consistency.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
84-118: Consider optimizing SVG styles and valuesThe clipPath implementation is correct, but there's room for improvement in maintainability and DRY principles.
Consider these optimizations:
- Move repeated style attributes to CSS classes:
<style lang="less" scoped> + .clip-path-base { + pointer-events: none; + fill: currentColor; + } </style> <clipPath id="margin-outer" class="margin-color"> <rect x="0" y="0" width="256" height="120" - fill="currentColor" - rx="4" - ry="4" - style="pointer-events: none" + class="clip-path-base" ></rect> </clipPath>
- Consider using CSS variables for repeated values:
<style lang="less" scoped> .spacing-wrap { + --spacing-border-radius: 4px; + --spacing-rect-width: 256px; + --spacing-rect-height: 120px; } </style>Also applies to: 267-301
packages/configurator/src/slider-configurator/SliderConfigurator.vue (1)
Line range hint
116-126: Update remaining tooltip styles to use standardized variablesThe tooltip styles still use the old
--ti-lowcode-*prefix variables, which should be updated to maintain consistency with the PR's objective of removing allti-lowcodereferences.Consider updating these styles:
.tiny-slider__tips { - color: var(--ti-lowcode-breadcrumb-color); - background: var(--ti-lowcode-breadcrumb-bg); + color: var(--te-common-text-primary); + background: var(--te-common-bg-popup); padding: 4px 6px; border-radius: 2px; box-shadow: rgb(0 0 0 / 15%) 0px 5px 10px; margin-top: -8px; &::before, &::after { - border-color: var(--ti-lowcode-breadcrumb-bg) transparent; + border-color: var(--te-common-bg-popup) transparent; } }packages/settings/styles/src/components/background/RadialGradient.vue (2)
Line range hint
149-149: Standardize remaining theme variableThere's still one instance of the old variable naming pattern that should be updated for consistency.
- color: var(--ti-lowcode-common-third-text-color); + color: var(--te-common-text-tertiary);
Line range hint
73-86: Consider documenting complex regex patternsThe gradient parsing logic uses sophisticated regex patterns that would benefit from documentation explaining their purpose and matching patterns. Consider adding JSDoc comments to explain:
- What patterns each regex matches
- Example inputs and outputs
- Edge cases handled
Example documentation:
/** * Regex patterns for parsing radial gradient syntax: * - rExtentKeyword: Matches size keywords (e.g., 'closest-side') * - rPosition: Matches position percentages (e.g., '50% 50%') * - rGradientCapture: Combines above patterns to match full gradient syntax * * Example input: "circle closest-side at 50% 50%, #fff 0%, #000 100%" */packages/plugins/datasource/src/DataSourceRecordList.vue (3)
Line range hint
359-365: Remove hardcoded color variable in confirmation dialogThe confirmation dialog is using a hardcoded color variable
--ti-lowcode-datasource-modal-text-colorwhich contradicts the PR's objective of standardizing theme variables. This should be updated to use the new common theme variables.const messageSaved = { render: () => ( <span> - <span style="color:var(--ti-lowcode-datasource-modal-text-color)">{'您确定要删除该条数据吗?'}</span> + <span style="color:var(--te-common-text-primary)">{'您确定要删除该条数据吗?'}</span> </span> ) }
Line range hint
466-484: Enhance error handling in saveRecordFormDataThe error handling in the saveRecordFormData function could be improved. Currently, it silently returns on error without notifying the user.
const saveRecordFormData = (data) => { const { name, id, data: { columns, type, ...other } } = props.data const requestData = { name, id, data: { columns, type, ...other, data } } requestUpdateDataSource(props.data.id, requestData).then((res) => { if (!res) { + useNotify({ + type: 'error', + message: '数据源修改失败' + }) return } const key = `datasource${capitalize(camelize(name))}` const pageSchema = useCanvas().canvasApi.value.getSchema() if (pageSchema.state[key]) { pageSchema.state[key] = data.map(({ _id, ...other }) => other) } useNotify({ type: 'success', message: '数据源修改成功' }) fetchData(true) + }).catch(error => { + useNotify({ + type: 'error', + message: `数据源修改失败: ${error.message}` + }) }) }
Line range hint
644-696: Standardize remaining theme variablesThere are several remaining instances of
ti-lowcodeprefixed variables that should be updated to use the new standardized theme variables:
--ti-lowcode-datasource-dialog-demo-border-color--ti-lowcode-datasource-common-hover-bg-1--ti-lowcode-datasource-common-primary-text-color--ti-lowcode-datasource-toolbar-breadcrumb-color--ti-lowcode-datasource-icon-hover-bg--ti-lowcode-datasource-toolbar-icon-colorThis will ensure consistency with the PR's objective of standardizing theme variables across all modules.
packages/settings/design/src/components/SettingPanel.vue (2)
273-276: Consider optimizing border stylingWhile the border color variable change is good, having both top and bottom borders might create double borders between adjacent items. Consider using only one border direction consistently.
.option-item { - border-top: 1px solid var(--te-common-border-default); + border-bottom: 1px solid var(--te-common-border-default); } -.option-item:last-child { - border-bottom: 1px solid var(--te-common-border-default); -}
Replace remaining
--ti-lowcode-setting-plugin-icon-colorvariable with the standardized equivalentThe verification found one remaining
ti-lowcodeprefixed CSS variable that needs to be updated:
- Line 384: Replace
--ti-lowcode-setting-plugin-icon-colorwith the appropriate--te-prefixed variable🔗 Analysis chain
Line range hint
1-391: Verify complete removal of ti-lowcode variablesLet's verify that we've caught all ti-lowcode variables that need to be replaced.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining ti-lowcode variables in the file rg --type less "ti-lowcode" packages/settings/design/src/components/SettingPanel.vueLength of output: 144
packages/settings/styles/src/components/typography/TypographyMore.vue (1)
Line range hint
1-74: Consider enhancing accessibility with ARIA labelsWhile the template structure is well-organized, consider improving accessibility by:
- Adding
aria-labelattributes to interactive elements- Using more descriptive labels instead of empty
forattributes- Adding proper ARIA roles where appropriate
Example improvements:
- <label for="">Letter spacing</label> + <label for="letter-spacing" aria-label="Letter spacing">Letter spacing</label> + <numeric-select id="letter-spacing" aria-label="Adjust letter spacing"></numeric-select>packages/settings/design/src/components/PropertyCanvas.vue (2)
Line range hint
275-279: Standardize remaining theme variablesWhile text colors have been updated to use common variables, several background and border color variables still use the old
ti-lowcodeprefix. This should be updated for consistency.Consider updating these variables:
- background: var(--ti-lowcode-tabs-active-bg); - border-bottom: 1px solid var(--ti-lowcode-tabs-border-color); + background: var(--te-common-bg-container); + border-bottom: 1px solid var(--te-common-border-color); &.active { - background-color: var(--ti-lowcode-common-hover-bg-color); + background-color: var(--te-common-bg-hover); }Also applies to: 282-286
Line range hint
1-316: Consider a systematic approach to theme variable migrationWhile this file shows good progress in standardizing theme variables, a systematic approach would ensure consistency across all components. Consider:
- Creating a mapping document between old and new variable names
- Using a CSS preprocessor or PostCSS plugin to automate the migration
- Adding lint rules to prevent the use of deprecated variable prefixes
This would help maintain consistency and prevent the reintroduction of old variable patterns.
packages/settings/styles/src/components/inputs/ModalMask.vue (2)
96-96: Consider standardizing remaining theme variablesFor consistency with the PR's objective of removing "ti-lowcode" references, consider updating these variables to use the new common variable system:
var(--ti-lowcode-tabs-border-color)var(--ti-modal-padding-y)var(--ti-modal-padding-x)- border: 1px solid var(--ti-lowcode-tabs-border-color); + border: 1px solid var(--te-common-border-color); ... - padding: var(--ti-modal-padding-y, 14px) var(--ti-modal-padding-x, 20px); + padding: var(--te-common-spacing-m, 14px) var(--te-common-spacing-l, 20px);Also applies to: 108-109
Line range hint
112-114: Fix typo in CSS variable nameThere's a typo in the CSS variable name:
--modal-spaceingshould be--modal-spacingleft: calc( - 100% - var(--modal-right-offset, 287px) - var(--modal-right-offset, 280px) - var(--modal-spaceing, 16px) + 100% - var(--modal-right-offset, 287px) - var(--modal-right-offset, 280px) - var(--modal-spacing, 16px) );packages/settings/design/src/App.vue (1)
Line range hint
152-157: Consider migrating remaining ti-lowcode variables.For consistency with the new design system, consider migrating these variables to their
--te-*equivalents:- --ti-lowcode-common-secondary-text-color: #adb0b8; - --ti-lowcode-common-border-color: #dfe1e6; - --ti-lowcode-common-hover-bg-color: #f2f5fc; - --ti-lowcode-mask-bg: #fafafa; + --te-common-text-secondary: #adb0b8; + --te-common-border: #dfe1e6; + --te-common-bg-hover: #f2f5fc; + --te-common-bg-mask: #fafafa;packages/theme/dark/datasource.less (1)
28-28: Remove light theme comment from dark theme fileThe comment
// lightappears in the dark theme file, which is confusing and incorrect.Remove the misplaced comment:
- --ti-lowcode-datasource-label-color: var(--te-common-text-secondary); // light + --ti-lowcode-datasource-label-color: var(--te-common-text-secondary);packages/theme/light/datasource.less (1)
2-39: Consider consolidating redundant color variablesSeveral variables appear to serve similar purposes and could potentially be consolidated:
- Error colors:
--ti-lowcode-datasource-description-error-color--ti-lowcode-datasource-common-error-color--ti-lowcode-datasource-error-tip-color- Text colors:
--ti-lowcode-datasource-common-text-main-color--ti-lowcode-datasource-common-primary-text-colorConsider consolidating these variables to reduce maintenance overhead and improve consistency. For example:
- --ti-lowcode-datasource-description-error-color - --ti-lowcode-datasource-common-error-color - --ti-lowcode-datasource-error-tip-color + --ti-lowcode-datasource-error-color - --ti-lowcode-datasource-common-text-main-color - --ti-lowcode-datasource-common-primary-text-color + --ti-lowcode-datasource-text-colorpackages/theme/dark/settings.less (1)
18-30: LGTM! Consider consolidating similar variablesThe color variables are well-organized and follow a consistent pattern. However, some variables could potentially be consolidated to reduce duplication, such as error-related colors that share the same values.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (53)
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/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/dark/datasource.less(1 hunks)packages/theme/dark/settings.less(1 hunks)packages/theme/light/datasource.less(1 hunks)packages/theme/light/settings.less(1 hunks)
💤 Files with no reviewable changes (3)
- packages/plugins/datasource/src/DataSourceRemoteForm.vue
- packages/plugins/datasource/src/DataSourceRemotePanel.vue
- packages/settings/props/src/components/inputs/NumericSelect.vue
✅ Files skipped from review due to trivial changes (22)
- packages/settings/styles/src/components/typography/TypographyGroup.vue
- packages/plugins/datasource/src/DataSourceType.vue
- packages/settings/styles/src/components/inputs/InputSelect.vue
- packages/settings/styles/src/components/size/SizeGroup.vue
- packages/plugins/materials/src/meta/block/src/BlockGroupSort.vue
- packages/plugins/datasource/src/DataSourceRemoteParameter.vue
- packages/settings/styles/src/components/border/BorderGroup.vue
- packages/settings/props/src/components/groups/TableColumn.vue
- packages/settings/styles/src/components/classNamesContainer/index.vue
- packages/plugins/datasource/src/DataSourceTemplate.vue
- packages/plugins/datasource/src/DataSourceField.vue
- packages/configurator/src/html-attributes-configurator/HtmlAttributesConfigurator.vue
- packages/configurator/src/code-list-configurator/CodeListConfigurator.vue
- packages/settings/design/src/components/PropertyList.vue
- packages/settings/design/src/components/ArrayConfigItemForm.vue
- packages/configurator/src/layout-grid-configurator/LayoutGridConfigurator.vue
- packages/plugins/datasource/src/DataSourceRemoteDataResult.vue
- packages/settings/styles/src/components/inputs/ImageSelect.vue
- packages/settings/props/src/components/inputs/DraggableOptions.vue
- packages/settings/props/src/components/groups/LifeCycle.vue
- packages/settings/styles/src/components/spacing/SpacingSetting.vue
- packages/settings/events/src/components/BindEventsDialogContent.vue
🔇 Additional comments (39)
packages/settings/props/src/components/groups/TablePager.vue (1)
65-65: LGTM: Theme variable update aligns with standardization effort
The change from --ti-lowcode-toolbar-breadcrumb-color to --te-common-text-secondary correctly implements the new theme system as outlined in the PR objectives.
packages/settings/styles/src/components/layout/LayoutGroup.vue (1)
132-132: LGTM: Appropriate use of common theme variable
The change correctly implements the standardized theme variable --te-common-text-secondary for text color, aligning with the PR's objective of using common variables.
packages/configurator/src/select-icon-configurator/SelectIconConfigurator.vue (2)
110-111: LGTM: Theme variable updates align with standardization effort
The changes correctly implement the new common theme variables:
--te-common-bg-promptfor background--te-common-text-secondaryfor text color
This aligns with the PR objective of standardizing theme variables across modules.
118-118: LGTM: Icon color standardization
The icon color now uses the standardized --te-common-text-secondary variable, maintaining consistency with the parent text color.
packages/settings/styles/src/Main.vue (1)
232-232: LGTM! Variable change aligns with standardization goals.
The change from --ti-lowcode-setting-style-title-color to --te-common-text-secondary aligns with the PR objective to standardize theme variables.
packages/settings/props/src/components/Empty.vue (2)
39-39: LGTM! Theme variable change aligns with standardization effort.
The change from --ti-lowcode-common-text-color-5 to --te-common-text-weaken follows the new semantic naming convention and removes the deprecated "ti-lowcode" prefix.
39-39: Verify the new theme variable is properly defined.
Let's ensure the new --te-common-text-weaken variable is properly defined in the theme files.
✅ Verification successful
Theme variable --te-common-text-weaken is properly defined and used
The variable is well-defined in the base theme with appropriate color values:
- Light theme:
--te-base-gray-60(#808080) - Dark theme:
--te-base-gray-120(#8B8C8F)
It's also being used consistently across multiple theme files as a replacement for older variables, including the specific use case in datasource theme files where it replaces --ti-lowcode-common-text-color-5.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the new theme variable is defined in theme files
# Expected: Find definition of --te-common-text-weaken in theme files
# Search for the variable definition in less/css files
rg --type=less --type=css "te-common-text-weaken"
# Search for any remaining old variable references that might have been missed
rg --type=less --type=css "ti-lowcode-common-text-color-5"
Length of output: 4235
packages/settings/styles/src/components/background/BackgroundGroup.vue (1)
401-401: LGTM! Color variable change aligns with design system
The change to use --te-common-text-secondary aligns well with the standardization of theme variables.
packages/configurator/src/input-configurator/InputConfigurator.vue (2)
69-69: LGTM! Variable change aligns with theme standardization
The change from --ti-lowcode-dialog-font-color to --te-common-text-primary aligns well with the PR objectives of standardizing theme variables and removing "ti-lowcode" references.
69-69: Verify the new theme variable implementation
Let's ensure the new --te-common-text-primary variable is properly defined and consistently used across themes.
✅ Verification successful
The new theme variable implementation is properly defined and consistent
The variable --te-common-text-primary is well-defined in the theme system:
- Defined in base theme for both light and dark modes with appropriate color values
- Consistently used across multiple components
- Part of a comprehensive text color system including other variables like secondary, weaken, disabled, etc.
- Properly replacing the old variable
--ti-lowcode-dialog-font-color
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the new theme variable implementation and usage
# Check for the variable definition in theme files
echo "Checking for variable definition in theme files..."
rg --type=less "te-common-text-primary"
# Check for any remaining old variable usage
echo "Checking for any remaining old variable usage..."
rg --type=less "ti-lowcode-dialog-font-color"
# Look for similar color variables to ensure consistency
echo "Checking for similar color variable patterns..."
rg --type=less "te-common-text-[a-z]+"
Length of output: 24656
packages/settings/styles/src/components/background/ImageSetting.vue (1)
291-291: LGTM: Theme variable standardization
The color variable change from --ti-lowcode-common-text-color-2 to --te-common-text-secondary aligns with the PR's objective of standardizing theme variables across modules.
packages/settings/styles/src/components/layout/GridBox.vue (1)
563-566: LGTM!
The padding values are consistent and provide appropriate spacing for the grid edit section.
packages/plugins/datasource/src/RemoteDataAdapterForm.vue (3)
59-59: LGTM: Font size standardization
The font size change aligns with the PR's objective to standardize theme variables.
75-75: LGTM: Border color standardization
The border color change aligns with the standardization effort.
62-62: LGTM: Theme variable standardization
The change from a specific datasource variable to a common text variable aligns with the PR's goal of standardizing theme variables.
✅ Verification successful
Theme variable is properly defined and widely used
The variable --te-common-text-secondary is well-defined in the base theme (packages/theme/base/src/common.less) and is extensively used across both light and dark themes, including in datasource-related styles. The variable is properly mapped to base color values:
- Light theme:
--te-base-gray-70(#595959) - Dark theme:
--te-base-gray-110(#B4B5B8)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify the new theme variable is defined in theme files
rg "te-common-text-secondary" --type less
Length of output: 3578
packages/plugins/datasource/src/DataSourceList.vue (1)
155-155: LGTM! Theme variable update aligns with standardization effort.
The change from specific color variable to semantic theme variable --te-common-text-primary improves consistency.
Let's verify consistent usage of this new variable across related components:
✅ Verification successful
Theme variable update is consistent with the design system
The change from --ti-lowcode-base-gray-90 to --te-common-text-primary is part of a broader theme standardization effort. The search results confirm:
- The new variable
--te-common-text-primaryis properly defined in the theme system (packages/theme/base/src/common.less) for both light and dark themes - It's widely adopted across multiple components for primary text styling
- The old variable
--ti-lowcode-base-gray-90is only used in theme definition files, not directly in components
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining old variable usage
rg "ti-lowcode-base-gray-90" --type=vue
# Verify adoption of new variable
rg "te-common-text-primary" --type=vue
Length of output: 134
Script:
#!/bin/bash
# Search for any remaining old variable usage without file type restriction
rg "ti-lowcode-base-gray-90"
# Verify adoption of new variable
rg "te-common-text-primary"
Length of output: 10521
packages/configurator/src/slider-configurator/SliderConfigurator.vue (4)
98-98: LGTM: Theme variable update aligns with standardization effort
The background-color property now uses the standardized --te-common-bg-container variable, which is consistent with the PR's objective of unifying theme variables.
106-106: LGTM: Handle color now uses standardized text variable
The handle's background-color now uses --te-common-text-secondary, which is appropriate for this interactive element.
113-113: LGTM: Range background uses standardized variable
The range background-color properly uses the new --te-common-bg-container variable.
98-113: Verify consistent usage of new theme variables
Let's ensure these new theme variables are consistently used across other components and are properly defined in the theme system.
✅ Verification successful
Theme variables are properly defined and consistently used
The verification shows that:
- The new
te-commonvariables (bg-containerandtext-secondary) are properly defined in the theme system underpackages/theme/base/src/common.lessfor both light and dark themes - These variables are widely and consistently used across multiple components
- The variables are part of a systematic theme migration, as evidenced by their usage in mapping legacy
ti-lowcodevariables
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining ti-lowcode variables that should be migrated
rg --type=less --type=vue "var\(--ti-lowcode-"
# Verify the new te-common variables are defined in theme files
rg --type=less "te-common-(bg-container|text-secondary|bg-popup)"
Length of output: 7687
packages/settings/styles/src/components/background/RadialGradient.vue (2)
137-137: LGTM: Theme variable update aligns with standardization effort
The change from --ti-lowcode-common-text-color-2 to --te-common-text-secondary aligns with the PR objective of standardizing theme variables.
137-137: Verify theme variable usage across the codebase
Let's ensure all related theme variables have been updated consistently across the codebase.
packages/plugins/datasource/src/DataSourceRecordList.vue (2)
618-618: LGTM: Theme variable standardization
The change from a specific variable to the common text primary color variable aligns with the PR objectives of standardizing theme variables.
636-636: LGTM: Theme variable standardization
The change to use the standardized datasource JSON border color variable is consistent with the theme standardization effort.
packages/settings/design/src/components/SettingPanel.vue (3)
260-260: LGTM: Appropriate variable transition for label text color
The change from ti-lowcode to te-common variable aligns with the PR objectives of standardizing theme variables.
266-266: LGTM: Semantic color usage for headers
Using --te-common-text-primary for headers establishes proper visual hierarchy.
385-385: LGTM: Consistent hover state styling
The use of --te-common-bg-primary-checked for hover states aligns with the theme system.
packages/settings/styles/src/components/typography/TypographyMore.vue (3)
Line range hint 75-124: LGTM! Script implementation follows Vue 3 best practices
The component setup is well-structured with proper prop definitions and reactive state management.
126-126: Theme variable updates look good but need completion
The changes to use --te-common-* variables align with the PR objectives for theme standardization.
Also applies to: 193-194, 231-231
Line range hint 147-147: Complete the theme variable migration
There are still several instances of ti-lowcode variables that should be migrated to the new te-common variables for consistency:
--ti-lowcode-toolbar-icon-color--ti-lowcode-tabs-border-color
Consider updating these remaining instances to use the corresponding te-common variables:
- color: var(--ti-lowcode-toolbar-icon-color);
+ color: var(--te-common-text-primary);
- border: 1px solid var(--ti-lowcode-tabs-border-color);
+ border: 1px solid var(--te-common-border-color);Let's verify the new variable usage across the codebase:
Also applies to: 176-176, 189-189, 211-211
packages/settings/design/src/components/PropertyCanvas.vue (2)
180-180: LGTM: Appropriate use of common text color variable
The change to --te-common-text-weaken aligns with the standardization effort and correctly represents the semantic meaning for empty state text.
262-267: LGTM: Consistent use of common theme variables
The color variables for the collapse header section have been properly standardized using semantic common variables for both text and background colors.
packages/settings/styles/src/components/inputs/ModalMask.vue (2)
95-95: LGTM! Color variable change aligns with standardization goals.
The change from var(--ti-lowcode-toolbar-breadcrumb-color) to var(--te-common-text-secondary) correctly follows the PR's objective of standardizing theme variables.
Line range hint 19-34: Consider handling left edge case in modal positioning
The setPosition function handles cases where the modal would overflow on the right side but doesn't handle cases where x < modalHalfWidth, which could cause the modal to overflow on the left side of the screen.
Consider updating the positioning logic:
const setPosition = (event) => {
const panelWidth = window.getComputedStyle(document.body).getPropertyValue('--base-right-panel-width')
const innnerWidth = window.innerWidth
const modalHalfWidth = (parseInt(panelWidth) - 24) / 2
const x = event.x
const y = event.y
modal.top = y
+ // Ensure modal stays within viewport on both left and right edges
+ if (x < modalHalfWidth) {
+ modal.left = 0
if (x + modalHalfWidth > innnerWidth) {
modal.left = innnerWidth - (parseInt(panelWidth) - 24)
} else if (x - modalHalfWidth < innnerWidth - parseInt(panelWidth)) {
modal.left = innnerWidth - parseInt(panelWidth)
} else {
modal.left = x - modalHalfWidth
}
}packages/settings/design/src/App.vue (2)
187-187: LGTM! Variable change aligns with new design system.
The change from --ti-button-info-normal-bg-color to --te-common-color-info correctly follows the PR objective of standardizing theme variables.
Line range hint 152-157: Verify theme variable usage across components.
Let's verify that these theme variables are consistently used across other components in the codebase.
Also applies to: 187-187
✅ Verification successful
Based on the search results, I can see that there is a transition happening in the codebase from ti-lowcode-common-* variables to te-common-* variables. The search shows:
- Extensive usage of
ti-lowcode-common-*variables across multiple theme files and components - Growing adoption of
te-common-*variables, particularly in newer or updated components - Both sets of variables coexist in the codebase, suggesting an ongoing migration
Let me verify if there are any migration guides or documentation about this theme transition.
Based on the comprehensive search results, I can provide a definitive response about the theme variable usage:
Theme variable migration is in progress, with both old and new systems coexisting.
The codebase shows a clear transition from ti-lowcode-common-* to te-common-* variables:
- The old
ti-lowcode-common-*variables are still widely used across multiple components and theme files - The new
te-common-*variables are being adopted in newer components and theme files - Both systems are properly defined in theme files:
- Old:
packages/theme/light/variable.lessandpackages/theme/dark/variable.less - New:
packages/theme/base/src/common.less
- Old:
This coexistence is intentional during the migration period, and the variables are being used consistently within their respective scopes. No issues were found with the current theme variable usage.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining ti-lowcode variable usage
echo "Checking for remaining ti-lowcode variables:"
rg --type=vue --type=less "ti-lowcode-common-"
echo "\nChecking for new te-common variable adoption:"
rg --type=vue --type=less "te-common-"
Length of output: 345
Script:
#!/bin/bash
# Search for any remaining ti-lowcode variable usage with correct file extensions
echo "Checking for remaining ti-lowcode variables:"
rg "ti-lowcode-common-" --type-add 'vue:*.vue' --type-add 'less:*.less' --type=vue --type=less
echo -e "\nChecking for new te-common variable adoption:"
rg "te-common-" --type-add 'vue:*.vue' --type-add 'less:*.less' --type=vue --type=less
Length of output: 65872
Script:
#!/bin/bash
# Search for any documentation about theme migration
echo "Checking for theme migration documentation:"
rg -i "theme.*(migration|transition|upgrade)" --type=md
echo -e "\nChecking README files for theme information:"
fd -e md -e mdx "README|CHANGELOG" -x cat {}
Length of output: 23854
packages/theme/dark/datasource.less (1)
22-22: Review tabs background color assignment
Using --te-common-text-primary for background color (--ti-lowcode-datasource-tabs-bg) seems incorrect. Text colors typically have different opacity and color values than background colors.
Please verify if this should be using a background-specific variable instead, such as --te-common-bg-container.
packages/theme/dark/settings.less (1)
35-40: LGTM! Good use of semantic variables
The root variables are well-structured with appropriate use of semantic color variables. The spacing colors are correctly differentiated between light and dark themes.
packages/theme/light/settings.less (1)
1-40: Verify consistent variable usage across themes
The light theme variables mirror the dark theme structure well, with appropriate color value differences. However, to ensure complete consistency:
- Verify that all variables defined in one theme exist in the other
- Ensure semantic meaning is preserved across themes
✅ Verification successful
Theme variables are consistently defined with appropriate color mappings
The verification shows perfect symmetry between light and dark themes. The only differences are the expected color value mappings for spacing variables:
- Dark theme uses
gray-130/140for padding/margin - Light theme uses
gray-10/30for padding/margin
These differences are intentional and semantically correct for maintaining proper contrast in each theme mode.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Compare variable definitions between light and dark themes
# Extract variable names from both files
dark_vars=$(grep -o -- '--[^:]*' packages/theme/dark/settings.less | sort)
light_vars=$(grep -o -- '--[^:]*' packages/theme/light/settings.less | sort)
# Compare variables
echo "Variables only in dark theme:"
comm -23 <(echo "$dark_vars") <(echo "$light_vars")
echo -e "\nVariables only in light theme:"
comm -13 <(echo "$dark_vars") <(echo "$light_vars")
Length of output: 13229
Script:
#!/bin/bash
# Let's try a different approach using diff to compare the variables
# First, extract clean variable names without trailing characters
# Extract variables from dark theme
grep -o -- '--[^:]*:' packages/theme/dark/settings.less | sed 's/://g' | sort > dark_vars.txt
# Extract variables from light theme
grep -o -- '--[^:]*:' packages/theme/light/settings.less | sed 's/://g' | sort > light_vars.txt
# Compare the files
echo "=== Variables only in dark theme ==="
diff dark_vars.txt light_vars.txt | grep "^<" | cut -c 3-
echo -e "\n=== Variables only in light theme ==="
diff dark_vars.txt light_vars.txt | grep "^>" | cut -c 3-
# Cleanup
rm dark_vars.txt light_vars.txt
Length of output: 484
Script:
#!/bin/bash
# Let's try one more approach using direct string comparison of variable definitions
# Get both files content and compare
echo "=== Comparing variable definitions ==="
echo "Dark theme unique variables:"
grep -h "^[[:space:]]*--" packages/theme/dark/settings.less | sort > dark_vars.txt
grep -h "^[[:space:]]*--" packages/theme/light/settings.less | sort > light_vars.txt
# Show unique lines in each file
echo "=== Only in dark theme ==="
grep -Fxvf light_vars.txt dark_vars.txt
echo -e "\n=== Only in light theme ==="
grep -Fxvf dark_vars.txt light_vars.txt
# Cleanup
rm dark_vars.txt light_vars.txt
Length of output: 792
packages/configurator/src/select-icon-configurator/SelectIconConfigurator.vue
Show resolved
Hide resolved
packages/engine-cli/template/designer/src/configurators/MyInputConfigurator.vue
Show resolved
Hide resolved
…heme variable review
English | 简体中文
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Background and solution
此pr做的事情：
整体目的
剩余要做的事情：
举例：
1.直接改成common变量，等待之后调整
2.一步到位，看是哪一个模块的，然后新增模块变量，模块变量的值是common变量，然后vue文件使用的是模块变量。
注意点：需要看一下暗色主题
注意：对于一些分割线，使用的变量都是--te-common-border-divider，不要混用 --te-common-border-default了。可能需要全量排查一遍，可以排查分别设置 border-top，border-left, border-right，border-bottom的地方，看看有没有类似的分割线场景
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Chores