-
Notifications
You must be signed in to change notification settings - Fork 330
feat(theme-mobile): optimize the theme-mobile project css variable names and add undefined css variables #2782
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…d command specifications
WalkthroughThe changes represent a comprehensive renaming and restructuring of CSS variables and styling methods across the mobile theme package. The primary modification involves changing the prefix of CSS custom properties from Changes
Suggested Labels
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 (
|
|
[e2e-test-warn] The title of the Pull request should look like "fix(vue-renderless): [action-menu, alert] fix xxx bug". Please make sure you've read our contributing guide |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 9
🧹 Nitpick comments (55)
packages/theme-mobile/src/index-bar-anchor/vars.less (1)
1-3: Clean and consistent variable naming convention.The updated naming follows a more maintainable pattern:
- Concise
--tvm-prefix instead of--ti-mobile-- PascalCase component name matches Vue component conventions
- Clear separation between component name and property purpose
However, consider documenting these naming conventions in a style guide to ensure consistency across the team.
packages/theme-mobile/src/progress/index.less (2)
80-81: Consider keeping border-radius configurable.The border-radius has been changed from a CSS variable to a fixed value of
999px. This reduces theming flexibility. Consider keeping it configurable:- border-radius: 999px; + border-radius: var(--tvm-Progress-bar-border-radius, 999px);Also applies to: 94-94
99-99: Use modern pseudo-element syntax.The single colon syntax (
:after) for pseudo-elements is outdated. Use the modern double colon syntax (::after) instead:- &:after { + &::after {Also applies to: 112-112
packages/theme-mobile/src/multi-select/vars.less (1)
2-37: Consider adding English translations for comments.While the Chinese comments are helpful, adding English translations would improve international collaboration. Consider using a bilingual comment format:
Example format:
- // 背景色 + // Background color | 背景色packages/theme-mobile/src/switch/vars.less (2)
1-1: Consider using consistent casing in the function name.The function name
.inject-Switch-vars()mixes PascalCase ("Switch") with kebab-case ("vars"). Consider using consistent casing throughout, either:
.inject-switch-vars()(all kebab-case), or.injectSwitchVars()(all camelCase)
1-21: Consider translating Chinese comments to English.The file contains Chinese comments. For better international collaboration, consider translating these comments to English.
Example translations:
- // 默认尺寸宽度 + // Default size width - // 默认尺寸高度 + // Default size height - // 小尺寸宽度 + // Mini size width - // 小尺寸高度 + // Mini size height - // 默认尺寸按钮大小 + // Default button size - // 小尺寸按钮大小 + // Mini button size - // 默认关状态背景色 + // Default (off state) background color - // 禁用状态下背景色 + // Disabled state background color - // 开状态禁用背景色 + // Checked disabled state background color - // 开状态背景色 + // Checked state background colorpackages/theme-mobile/src/mask/vars.less (1)
2-3: Consider translating Chinese comments to EnglishFor better international collaboration, consider translating the Chinese comment "遮罩层背景色" to English (e.g., "Mask layer background color").
packages/theme-mobile/src/index-bar/vars.less (1)
1-1: Document breaking changes and provide migration guideThis PR introduces breaking changes by renaming CSS variables from
--ti-mobile-*to--tvm-*. Please:
- Document these changes in the changelog
- Provide a migration guide for users
- Consider adding a codemod script to help users migrate
packages/theme-mobile/src/picker-column/vars.less (2)
1-1: Align function naming with other componentsFor consistency with other components, consider using PascalCase in the function name:
-.inject-picker-column-vars() { +.inject-PickerColumn-vars() {
2-6: Consider using base variable referencesFor consistency with other components, consider using base variable references for colors:
- --tvm-PickerColumn-item-color: #333; + --tvm-PickerColumn-item-color: var(--tvm-base-color-dark, #333); - --tvm-PickerColumn-item-select-color: #039be5; + --tvm-PickerColumn-item-select-color: var(--tvm-base-color-primary-normal, #039be5);packages/theme-mobile/src/tabbar-item/vars.less (1)
3-3: Consider using base color variableFor consistency and theme customization, consider using a base color variable:
- --tvm-TabbarItem-active-color: #f04b3d; + --tvm-TabbarItem-active-color: var(--tvm-base-color-primary-normal, #f04b3d);packages/theme-mobile/src/image-viewer/vars.less (1)
3-3: Consider using base color variable for close buttonFor consistency and theme customization, consider using a base color variable:
- --tvm-ImageViewer-close-bgcolor: #606266; + --tvm-ImageViewer-close-bgcolor: var(--tvm-base-color-secondary, #606266);packages/theme-mobile/src/badge/vars.less (1)
8-8: Use base danger color variableFor consistency with other status colors, use the base danger color variable:
- --tvm-Badge-danger-bgcolor: #f36f64; + --tvm-Badge-danger-bgcolor: var(--tvm-base-color-danger-normal, #f36f64);packages/theme-mobile/src/slider/vars.less (1)
1-14: LGTM! Variables follow the new naming convention.The variables are well-organized and cover all necessary aspects of the slider component (track, handle, tips, input).
Consider documenting color values.
For better maintainability, consider using semantic color variables (like
--tvm-base-color-*) for hardcoded colors such as#ccc,#00c696, etc.packages/theme-mobile/src/dialog-box/vars.less (1)
1-10: LGTM! Good use of semantic color variables.The implementation properly uses semantic color variables for theming.
Consider responsive width and relative font sizes.
- The fixed width of
270pxmight not be optimal for all screen sizes. Consider using relative units or a responsive approach.- Font sizes are using fixed pixel values. Consider using relative units (rem/em) for better accessibility and responsiveness.
packages/theme-mobile/src/mini-picker/vars.less (1)
1-12: LGTM! Variables follow the new naming convention.The variables are well-structured and cover all aspects of the mini picker component.
Consider using more semantic color variables.
Several colors are hardcoded (
#ddd,#666,#333). Consider using semantic color variables like--tvm-base-color-*for better theming support.packages/theme-mobile/src/pull-refresh/vars.less (1)
1-16: LGTM! Good use of semantic variables.The implementation properly uses semantic variables for colors and font sizes.
Translate Chinese comments to English.
For better international collaboration, consider translating the Chinese comments to English:
- // 文字颜色 + // Text color - // 文字尺寸 + // Font size - // 加载背景色 + // Loading background color - // 加载圆圈外径 + // Loading circle outer diameter - // 加载圆圈内径 + // Loading circle inner diameter - // 加载圆圈背景色 + // Loading circle background color - // 组件foot高度 + // Footer heightpackages/theme-mobile/src/file-upload/vars.less (1)
1-11: LGTM! Consider documenting variable relationships.The variable renaming follows the new convention consistently. The use of fallback values in
var()functions is well-structured.Consider adding a comment block at the top of the file documenting the relationships between these variables and the base theme variables they reference (e.g.,
--tvm-base-color-info-normal,--tvm-base-color-placeholder, etc.).packages/theme-mobile/src/exception/vars.less (1)
1-12: Consider standardizing dimension units.The variable renaming is consistent with the new convention. However, there's an opportunity to improve maintainability:
Consider using relative units (rem/em) for dimensions or referencing base spacing variables for better scalability. For example:
- --tvm-Exception-image-width: 200px; - --tvm-Exception-image-height: 100px; + --tvm-Exception-image-width: calc(var(--tvm-base-space-unit, 4px) * 50); + --tvm-Exception-image-height: calc(var(--tvm-base-space-unit, 4px) * 25);packages/theme-mobile/src/numeric/vars.less (1)
1-20: Translate comments to English for international collaboration.The variable renaming and token usage is consistent. However:
Consider translating the Chinese comments to English:
- // 宽度 + // Width - // 高度 + // Height - // icon高度 + // Icon height - // 按钮背景色 + // Button background color - // 文字尺寸 + // Font size - // 字重 + // Font weight - // 文字颜色 + // Text color - // icon颜色 + // Icon color - // icon颜色-不可用 + // Icon color - disabledpackages/theme-mobile/src/list/vars.less (1)
1-13: Use theme tokens consistently for colors.The variable renaming follows the convention, but there are hardcoded colors that should use theme tokens:
Replace hardcoded colors with theme tokens:
- --tvm-List-active-bgcolor: #ddd; - --tvm-List-border-color: #ddd; + --tvm-List-active-bgcolor: var(--tvm-color-bg-active, #ddd); + --tvm-List-border-color: var(--tvm-color-border-default, #ddd);packages/theme-mobile/src/radio/vars.less (1)
1-21: Excellent use of design tokens. Consider translating comments.The implementation demonstrates best practices in using design system tokens for colors, spacing, and typography.
Consider translating the Chinese comments to English:
- // 组件尺寸 + // Component size - // 外圈尺寸 + // Outer circle size - // 内圆尺寸 + // Inner circle size - // 外圈默认颜色 + // Default border color - // 不可用状态填充颜色 + // Disabled state fill color - // 不可用状态背景色 + // Disabled state background color - // 文字大小 + // Font size - // 文字颜色 + // Text color - // 文字左边距 + // Text left paddingpackages/theme-mobile/src/multi-select-item/vars.less (1)
1-25: LGTM! Comprehensive variable renaming with preserved documentation.The changes:
- Follow the new naming convention consistently
- Preserve the Chinese documentation comments
- Update references to common variables
Consider adding English translations for the Chinese comments to improve international developer experience.
packages/theme-mobile/src/label/vars.less (1)
1-27: LGTM! Comprehensive variable renaming with preserved documentation.The changes:
- Follow the new naming convention consistently
- Preserve the Chinese documentation comments
- Update references to common color variables
Consider adding English translations for the Chinese comments to improve international developer experience.
packages/theme-mobile/src/modal/vars.less (2)
13-26: Consider using common variables for consistency.While the variable renaming is correct, some values are hardcoded instead of using common variables:
- Line 14:
14pxcould usevar(--tvm-font-size-m)- Line 15:
16pxcould usevar(--tvm-font-size-l)- Line 17:
22pxcould use a common variableApply this diff to improve consistency:
.inject-Modal-vars() { - --tvm-Modal-text-font-size: 14px; - --tvm-Modal-header-font-size: 16px; + --tvm-Modal-text-font-size: var(--tvm-font-size-m, 14px); + --tvm-Modal-header-font-size: var(--tvm-font-size-l, 16px); --tvm-Modal-text-color: var(--tvm-common-color-text-primary, #191919); - --tvm-Modal-alert-font-size: 22px; + --tvm-Modal-alert-font-size: var(--tvm-font-size-xxl, 22px);
Line range hint
1-1: Consider adding documentation for this breaking change.This PR implements a systematic renaming of CSS variables and mixins, which is a breaking change for consumers. Consider:
- Adding a migration guide documenting the old → new variable mappings
- Adding a BREAKING_CHANGE notice in the commit message
- Bumping the major version number
- Considering a deprecation period where both old and new variables coexist
packages/theme-mobile/src/mask/index.less (1)
20-20: LGTM! Consider documenting the breaking changes.The variable renaming follows the new convention correctly. However, since this changes the public API, it would be helpful to document these changes in the changelog or migration guide.
Consider adding a note in the documentation about the variable name changes from
--ti-mobile-mask-*to--tvm-Mask-*to help users migrate their custom styles.Also applies to: 27-27
packages/theme-mobile/src/search/vars.less (1)
1-31: Well-structured variable renaming with consistent fallbacks.The variable renaming follows the new convention consistently while preserving the semantic meaning and fallback values.
Consider translating the Chinese comments to English for better international collaboration:
- // 搜索框高度 + // Search box height - // 输入框高度 + // Input box heightpackages/theme-mobile/src/loading/vars.less (1)
13-45: Well-organized variable structure with clear size variants.The variable renaming is consistent and maintains a clear hierarchy for different loading sizes.
Consider translating the Chinese comments to English for better international collaboration and adding JSDoc-style documentation for the function:
+/** + * Injects CSS variables for the Loading component + * @less + */ .inject-Loading-vars() { - // 字体色 + // Font color - // 图标色 + // Icon colorpackages/theme-mobile/src/checkbox/vars.less (2)
1-37: Well-structured variable organization with comprehensive state coverage.The variable renaming is consistent and maintains clear semantics for different checkbox states.
Consider translating the Chinese comments to English for better international collaboration:
- // 复选框图标宽度 + // Checkbox icon width - // 复选框图标高度 + // Checkbox icon heightConsider adding TypeScript definitions or documentation for these CSS custom properties to improve IDE support and developer experience.
1-1: Overall: Consistent and well-structured variable renaming.The changes across all files follow a clear pattern:
- Function names changed from
.component-css-vars-*()to.inject-*-vars()- Variable prefixes changed from
--ti-mobile-to--tvm-- Component names are now capitalized in variable names
Consider the following improvements for better maintainability:
- Add a migration guide documenting the variable name changes
- Translate Chinese comments to English
- Add TypeScript definitions for CSS custom properties
- Update documentation to reflect the new naming convention
packages/theme-mobile/src/popover/vars.less (1)
13-34: Add descriptive comments for CSS variables.While the variables are well-organized, adding descriptive comments would improve maintainability. Consider documenting each variable's purpose, similar to the button component's implementation.
Example format:
.inject-Popover-vars() { + // Popover background color --tvm-Popover-bg-color: var(--tvm-base-color-light, #ffffff); + // Popover text color --tvm-Popover-text-color: #606266; // ... (add comments for other variables) }packages/theme-mobile/src/button/vars.less (1)
1-51: Consider translating Chinese comments to English.The code is well-documented, but for better international collaboration, consider translating the Chinese comments to English.
Example translation:
.inject-Button-vars() { - // 按钮高度-大 + // Button height - large --tvm-Button-height-large: 40px; - // 按钮文字大小-大 + // Button font size - large --tvm-Button-font-size-large: var(--tvm-font-size-m, 14px); // ... (translate other comments) }packages/theme-mobile/src/avatar/index.less (1)
66-66: Add fallback value for brand color.The
--tvm-base-color-brand-1variable is used without a default value. Consider adding a fallback color to prevent undefined styles.- background-color: var(--tvm-base-color-brand-1); + background-color: var(--tvm-base-color-brand-1, #1476ff);packages/theme-mobile/src/nav-bar/index.less (1)
Line range hint
74-74: Use CSS variable for hardcoded color.The color
#f0f0f0is hardcoded in the.main-titleclass while a CSS variable is used elsewhere for the same color.- color: #f0f0f0; + color: var(--tvm-NavBar-title-color, #f0f0f0);packages/theme-mobile/src/steps/vars.less (1)
13-35: LGTM! Consistent variable renaming.The renaming from
--ti-mobile-to--tvm-and.component-css-vars-steps()to.inject-Steps-vars()follows the new convention consistently.Consider documenting this breaking change in the migration guide to help users update their custom themes and component styles that might be using the old variable names.
packages/theme-mobile/src/alert/vars.less (1)
1-59: LGTM! Thorough and consistent variable renaming.The changes maintain the semantic meaning of all variables while following the new naming convention.
Consider internationalizing the Chinese comments for better maintainability in a global context.
packages/theme-mobile/src/exception/index-global.less (1)
Line range hint
1-1: Consider comprehensive migration strategy.This PR introduces a breaking change in the CSS variable naming convention. To ensure a smooth transition:
- Update the major version number following semver
- Create a detailed migration guide documenting the changes from
--ti-mobile-to--tvm-and.component-css-vars-*()to.inject-*-vars()- Consider providing a codemod or migration script to help users update their custom themes
- Add deprecation warnings for the old variable names in the next minor version before the breaking change
packages/theme-mobile/src/badge/index.less (1)
73-96: Consider consolidating color variables.While the changes follow the new naming convention, consider using a single base color variable with modifiers for different states (primary, success, warning, etc.) to improve maintainability.
Example approach:
- background-color: var(--tvm-Badge-primary-bgcolor, #1890ff); + background-color: var(--tvm-Badge-bgcolor-primary, #1890ff);packages/theme-mobile/src/list/index.less (1)
65-93: Consider grouping text-related variables.While the changes follow the new naming convention, consider grouping text-related variables under a common prefix for better organization.
Example approach:
- color: var(--tvm-List-main-text-color, #333); - font-size: var(--tvm-List-main-text-font-size, 16px); + color: var(--tvm-List-text-color-main, #333); + font-size: var(--tvm-List-text-size-main, 16px);packages/theme-mobile/src/mini-picker/index.less (1)
51-75: Consider consolidating toolbar-related variables.The toolbar variables could be better organized under a common prefix structure.
Example approach:
- height: var(--tvm-MiniPicker-toolbar-height, 44px); - color: var(--tvm-MiniPicker-toolbar-action-color, #039be5); + height: var(--tvm-MiniPicker-toolbar-size-height, 44px); + color: var(--tvm-MiniPicker-toolbar-color-action, #039be5);packages/theme-mobile/src/tabbar-item/index.less (1)
Line range hint
73-88: Consider consolidating info-related variables.The info badge variables could be better organized under a common prefix structure.
Example approach:
- font-size: var(--tvm-TabbarItem-info-font-size, 12px); - background-color: var(--tvm-TabbarItem-info-bgcolor, #ee0a24); + font-size: var(--tvm-TabbarItem-info-size-font, 12px); + background-color: var(--tvm-TabbarItem-info-color-bg, #ee0a24);packages/theme-mobile/src/exception/index.less (1)
31-32: Consider using a shared variable for repeated values.These lines use the same fallback value
200px. Consider extracting it to a shared variable to follow DRY principles.+@exception-view-size: 200px; - width: var(--tvm-Exception-view-size, 200px); - height: var(--tvm-Exception-view-size, 200px); + width: var(--tvm-Exception-view-size, @exception-view-size); + height: var(--tvm-Exception-view-size, @exception-view-size);packages/theme-mobile/src/action-sheet/index.less (1)
46-50: Consider documenting the new visibility classes.The new classes
is-toggleandis-not-contentaffect the component's display behavior. Consider adding comments to document their purpose and usage.+ // Toggle class to control the slide-up animation &.is-toggle { transform: translate(0, 0); } + // Class to adjust z-index when content is not present &.is-not-content { z-index: 2000; }packages/theme-mobile/src/numeric/index.less (1)
22-22: Consider maintaining consistent naming conventions between class names and CSS variables.The component uses kebab-case for class names (
mobile-numeric) but camelCase for CSS variables (--tvm-Numeric-*). Consider aligning these naming conventions for better maintainability.Also applies to: 26-26, 36-36, 46-50, 75-78, 86-88, 95-95, 111-111, 118-118
packages/theme-mobile/src/image-viewer/index.less (1)
63-63: Extract hardcoded color values into CSS variables.Consider extracting hardcoded color values (#606266, #bfbfbf, etc.) into CSS variables for better theme customization and maintenance.
- background-color: var(--tvm-ImageViewer-close-bgcolor, #606266); + background-color: var(--tvm-ImageViewer-close-bgcolor, var(--tvm-color-gray-600, #606266));Also applies to: 72-72, 75-75, 78-78, 101-101
packages/theme-mobile/src/alert/index.less (1)
77-79: Fix formatting in alert-variant mixin calls.The alert-variant mixin calls have inconsistent spacing. Remove the spaces after semicolons for better readability.
- var(--tvm-Alert-success-icon-color) ; - var(--tvm-Alert-success-link-color) ; - var(--tvm-Alert-success-bg-color) + var(--tvm-Alert-success-icon-color); + var(--tvm-Alert-success-link-color); + var(--tvm-Alert-success-bg-color)Also applies to: 85-87, 93-95, 101-103
packages/theme-mobile/src/radio/index.less (1)
92-92: Remove hardcoded color from variable fallback.The fallback value should be a CSS variable instead of a hardcoded color.
- border-color: var(--tvm-Radio-checked-color #1476ff); + border-color: var(--tvm-Radio-checked-color, var(--tvm-color-primary, #1476ff));packages/theme-mobile/src/input/vars.less (1)
1-94: LGTM! Consider updating documentation.The CSS variable renaming is consistent with the new naming convention. All variables maintain their original values and fallbacks.
Consider updating any related documentation or style guides to reflect the new
--tvm-*prefix and naming conventions for future contributors.packages/theme-mobile/src/slider/index.less (1)
20-20: LGTM! Consider consolidating repeated values.The variable renaming is consistent and maintains all slider functionality. However, there are some repeated values that could be consolidated into shared variables.
Consider extracting repeated values like
2pxinto shared variables for better maintainability:+ --tvm-Slider-default-size: 2px; - --tvm-Slider-track-size: 2px; + --tvm-Slider-track-size: var(--tvm-Slider-default-size);Also applies to: 23-25, 39-39, 54-54, 74-75, 92-93, 98-98, 120-123, 130-130, 139-139, 149-150, 156-159
packages/theme-mobile/src/multi-select/index.less (1)
21-21: Consider standardizing spacing variables.While the variable renaming is consistent, there's a mix of component-specific spacing variables and global spacing tokens (e.g.,
--tvm-space-2x). Consider standardizing the approach to spacing variables across components.Also applies to: 24-24, 33-36, 42-42, 45-45, 121-121, 125-127, 133-133, 143-143, 151-151, 153-153, 168-168, 176-178
packages/theme-mobile/src/steps/index.less (1)
19-21: Consider reorganizing timeline-specific variables.While the variable renaming is consistent, consider grouping timeline-specific variables more clearly, perhaps under a separate namespace like
--tvm-Steps-Timeline-*for better organization.Also applies to: 24-24, 28-30, 36-37, 53-53, 59-59, 67-67, 71-76, 79-79, 81-81, 88-88, 94-94, 99-101, 107-107, 109-109, 116-116, 130-130, 142-142, 148-148, 164-165, 194-196, 200-200, 204-204, 208-208, 231-231, 253-253, 296-296, 300-300, 331-331
packages/theme-mobile/src/modal/index.less (1)
35-36: Consider adding fallback values for critical styling properties.The CSS variables for font size and color lack fallback values. This could cause styling issues if the variables are not defined in the theme.
- font-size: var(--tvm-Modal-text-font-size); - color: var(--tvm-Modal-text-color); + font-size: var(--tvm-Modal-text-font-size, 14px); + color: var(--tvm-Modal-text-color, #333);Also applies to: 85-86
packages/theme-mobile/src/input/index.less (2)
141-149: Consider extracting disabled state styles into a mixin.The disabled state styles are repeated across different selectors. Consider creating a mixin to reduce code duplication.
+ // Add to mixins/input.less + .input-disabled() { + border-color: var(--tvm-border-color-disabled); + background: var(--tvm-Input-bg-color-disabled); + color: var(--tvm-Input-text-color-disabled); + cursor: not-allowed; + } &.is-disabled { .@{input-prefix-cls}__wrapper { - border-color: var(--tvm-border-color-disabled); - background: var(--tvm-Input-bg-color-disabled); + .input-disabled(); } .@{input-prefix-cls}__inner { - cursor: not-allowed; - color: var(--tvm-Input-text-color-disabled); + .input-disabled(); .placeholder(@color: var(--tvm-Input-text-color-disabled)); } }
371-380: Consider using CSS custom properties for consistent spacing.The prepend and append padding values could be derived from a base spacing unit for better maintainability.
&__prepend { order: 1; - padding: 0 var(--tvm-Input-prepend-padding-horizontal); + padding: 0 var(--tvm-spacing-base, 8px); color: var(--tvm-Input-prepend-text-color); background-color: var(--tvm-Input-prepend-bg-color); } &__append { order: 5; - padding: 0 var(--tvm-Input-append-padding-horizontal); + padding: 0 var(--tvm-spacing-base, 8px); color: var(--tvm-Input-append-text-color); background-color: var(--tvm-Input-append-bg-color); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (82)
packages/theme-mobile/package.json(1 hunks)packages/theme-mobile/src/action-sheet/index.less(4 hunks)packages/theme-mobile/src/action-sheet/vars.less(1 hunks)packages/theme-mobile/src/alert/index.less(5 hunks)packages/theme-mobile/src/alert/vars.less(1 hunks)packages/theme-mobile/src/avatar/index.js(0 hunks)packages/theme-mobile/src/avatar/index.less(1 hunks)packages/theme-mobile/src/avatar/vars.less(1 hunks)packages/theme-mobile/src/badge/index.js(0 hunks)packages/theme-mobile/src/badge/index.less(2 hunks)packages/theme-mobile/src/badge/vars.less(1 hunks)packages/theme-mobile/src/base/basic-var.less(1 hunks)packages/theme-mobile/src/base/index.js(0 hunks)packages/theme-mobile/src/base/vars.less(1 hunks)packages/theme-mobile/src/button/index.less(3 hunks)packages/theme-mobile/src/button/vars.less(1 hunks)packages/theme-mobile/src/checkbox/index.less(6 hunks)packages/theme-mobile/src/checkbox/vars.less(1 hunks)packages/theme-mobile/src/container/index.less(1 hunks)packages/theme-mobile/src/container/vars.less(1 hunks)packages/theme-mobile/src/dialog-box/index.less(4 hunks)packages/theme-mobile/src/dialog-box/vars.less(1 hunks)packages/theme-mobile/src/error-page/index-global.less(0 hunks)packages/theme-mobile/src/error-page/index.less(0 hunks)packages/theme-mobile/src/error-page/vars.less(0 hunks)packages/theme-mobile/src/exception/index-global.less(3 hunks)packages/theme-mobile/src/exception/index.less(3 hunks)packages/theme-mobile/src/exception/vars.less(1 hunks)packages/theme-mobile/src/file-upload/index.less(5 hunks)packages/theme-mobile/src/file-upload/vars.less(1 hunks)packages/theme-mobile/src/form-item/index.less(6 hunks)packages/theme-mobile/src/form-item/vars.less(1 hunks)packages/theme-mobile/src/form/index.less(0 hunks)packages/theme-mobile/src/image-viewer/index.less(7 hunks)packages/theme-mobile/src/image-viewer/vars.less(1 hunks)packages/theme-mobile/src/index-bar-anchor/index.less(1 hunks)packages/theme-mobile/src/index-bar-anchor/vars.less(1 hunks)packages/theme-mobile/src/index-bar/index.less(2 hunks)packages/theme-mobile/src/index-bar/vars.less(1 hunks)packages/theme-mobile/src/input/index.less(15 hunks)packages/theme-mobile/src/input/vars.less(1 hunks)packages/theme-mobile/src/label/index.less(2 hunks)packages/theme-mobile/src/label/vars.less(1 hunks)packages/theme-mobile/src/list/index.less(3 hunks)packages/theme-mobile/src/list/vars.less(1 hunks)packages/theme-mobile/src/loading/index.less(3 hunks)packages/theme-mobile/src/loading/vars.less(1 hunks)packages/theme-mobile/src/mask/index.less(2 hunks)packages/theme-mobile/src/mask/vars.less(1 hunks)packages/theme-mobile/src/mini-picker/index.less(4 hunks)packages/theme-mobile/src/mini-picker/vars.less(1 hunks)packages/theme-mobile/src/modal/index.less(12 hunks)packages/theme-mobile/src/modal/vars.less(1 hunks)packages/theme-mobile/src/multi-select-item/index.less(2 hunks)packages/theme-mobile/src/multi-select-item/vars.less(1 hunks)packages/theme-mobile/src/multi-select/index.less(8 hunks)packages/theme-mobile/src/multi-select/vars.less(1 hunks)packages/theme-mobile/src/nav-bar/index.less(4 hunks)packages/theme-mobile/src/nav-bar/vars.less(1 hunks)packages/theme-mobile/src/numeric/index.less(5 hunks)packages/theme-mobile/src/numeric/vars.less(1 hunks)packages/theme-mobile/src/picker-column/index.less(4 hunks)packages/theme-mobile/src/picker-column/vars.less(1 hunks)packages/theme-mobile/src/popover/index.less(4 hunks)packages/theme-mobile/src/popover/vars.less(1 hunks)packages/theme-mobile/src/popup/index.less(0 hunks)packages/theme-mobile/src/progress/index.less(3 hunks)packages/theme-mobile/src/progress/vars.less(1 hunks)packages/theme-mobile/src/pull-refresh/index.less(3 hunks)packages/theme-mobile/src/pull-refresh/vars.less(1 hunks)packages/theme-mobile/src/radio/index.less(6 hunks)packages/theme-mobile/src/radio/vars.less(1 hunks)packages/theme-mobile/src/search/index.less(5 hunks)packages/theme-mobile/src/search/vars.less(1 hunks)packages/theme-mobile/src/slider/index.less(8 hunks)packages/theme-mobile/src/slider/vars.less(1 hunks)packages/theme-mobile/src/steps/index.less(10 hunks)packages/theme-mobile/src/steps/vars.less(1 hunks)packages/theme-mobile/src/switch/index.less(4 hunks)packages/theme-mobile/src/switch/vars.less(1 hunks)packages/theme-mobile/src/tabbar-item/index.less(4 hunks)packages/theme-mobile/src/tabbar-item/vars.less(1 hunks)
⛔ Files not processed due to max files limit (20)
- packages/theme-mobile/src/tabbar/index.less
- packages/theme-mobile/src/tabbar/vars.less
- packages/theme-mobile/src/table/index.less
- packages/theme-mobile/src/table/vars.less
- packages/theme-mobile/src/tabs/index.less
- packages/theme-mobile/src/tabs/vars.less
- packages/theme-mobile/src/tag/index.less
- packages/theme-mobile/src/tag/vars.less
- packages/theme-mobile/src/tall-storage/index.less
- packages/theme-mobile/src/tall-storage/vars.less
- packages/theme-mobile/src/time-line/index.less
- packages/theme-mobile/src/time-line/vars.less
- packages/theme-mobile/src/toast/index.less
- packages/theme-mobile/src/toast/vars.less
- packages/theme-mobile/src/upload-list/index.less
- packages/theme-mobile/src/upload-list/vars.less
- packages/theme-mobile/src/user-head/index.less
- packages/theme-mobile/src/user-head/vars.less
- packages/theme-mobile/src/wheel/index.less
- packages/theme-mobile/src/wheel/vars.less
💤 Files with no reviewable changes (8)
- packages/theme-mobile/src/badge/index.js
- packages/theme-mobile/src/form/index.less
- packages/theme-mobile/src/base/index.js
- packages/theme-mobile/src/popup/index.less
- packages/theme-mobile/src/error-page/vars.less
- packages/theme-mobile/src/avatar/index.js
- packages/theme-mobile/src/error-page/index-global.less
- packages/theme-mobile/src/error-page/index.less
✅ Files skipped from review due to trivial changes (3)
- packages/theme-mobile/src/form-item/vars.less
- packages/theme-mobile/src/base/vars.less
- packages/theme-mobile/src/base/basic-var.less
🔇 Additional comments (41)
packages/theme-mobile/src/index-bar-anchor/vars.less (1)
2-3: Verify the common theme variables are defined.The CSS variables use fallback colors appropriately, but we should verify that the referenced common theme variables exist:
--tvm-common-bg-color-dark-gray--tvm-common-color-text-weaken-dark✅ Verification successful
Common theme variables are properly defined ✅
Both
--tvm-common-bg-color-dark-grayand--tvm-common-color-text-weaken-darkare defined in the base theme file and used consistently across multiple components.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the common theme variable definitions echo "Searching for common theme variable definitions..." rg --type less "common-bg-color-dark-gray|common-color-text-weaken-dark" packages/theme-mobile/src/Length of output: 1640
packages/theme-mobile/src/progress/index.less (2)
26-26: LGTM! Consistent variable naming for status colors.The variable naming follows the new
--tvm-Progress-*convention while maintaining the fallback colors.Also applies to: 30-30, 34-34
120-121: LGTM! Consistent variable naming for text styles.The text styling variables follow the new naming convention while maintaining appropriate default values.
packages/theme-mobile/src/multi-select/vars.less (4)
1-1: LGTM! Function name change follows the new convention.The new function name
.inject-MultiSelect-vars()is more semantic and follows the standardized naming pattern being implemented across the theme-mobile package.
3-35: LGTM! Well-structured variable declarations with proper fallbacks.The CSS variables:
- Follow a consistent naming pattern
- Use appropriate design tokens as fallbacks
- Maintain a clear hierarchy of values
1-37: Verify the impact on dependent components.Since this is part of a larger refactoring effort changing from
--ti-mobile-to--tvm-prefix, we should verify that all dependent components have been updated accordingly.✅ Verification successful
Changes are properly contained within the multi-select component ✅
The variable prefix changes are correctly implemented and isolated to the multi-select component. All usages follow the new
--tvm-prefix pattern, and no other components in the theme-mobile package are affected by these changes.
- Note: There's an undefined variable
--tvm-MultiSelect-item-bg-colorbeing used in index.less that should be defined in vars.less.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for any remaining old prefix usage rg --type less "--ti-mobile-multi-select-" packages/theme-mobile/ # Check for components that might be using these variables rg --type less "var\(--t[iv]m-MultiSelect-" packages/theme-mobile/Length of output: 2817
27-27: Review the variable reference for potential circular dependency.The variable
--tvm-MultiSelect-search-icon-widthis set tovar(--tvm-MultiSelect-header-height). Consider using the base valuevar(--tvm-space-12x, 48px)directly to avoid potential circular dependencies.packages/theme-mobile/src/switch/vars.less (1)
3-9: Verify the existence of referenced base variables.The size variables reference several base variables that need verification:
--tvm-base-size-width-small--tvm-base-size-height-mini--tvm-base-size-width-small✅ Verification successful
Base variables verified and correctly used
All referenced base size variables are properly defined in
packages/theme-mobile/src/base/vars.lesswith matching fallback values.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for base size variable definitions echo "Searching for base size variable definitions..." rg --type less "(--tvm-base-size-(width|height)-(small|mini))" packages/theme-mobile/src/Length of output: 966
packages/theme-mobile/src/container/vars.less (1)
1-2: Verify the referenced variable--tvm-space-3xThe variable references a spacing variable that should be defined in a common/base style file.
✅ Verification successful
✓ Variable
--tvm-space-3xis correctly definedThe variable is properly defined in the base styles with the same value (12px) as the fallback, ensuring consistent spacing across components.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the definition of the space variable rg --type=less "(?:--tvm-space-3x|--ti-mobile-space-3x)" -B 2 -A 2Length of output: 1744
packages/theme-mobile/src/nav-bar/vars.less (1)
2-7: Consider centralizing color definitionsSeveral color values are hard-coded (#343745, #f0f0f0). Consider moving these to a central color palette for better maintainability and consistency.
Also, verify the referenced base color variable:
✅ Verification successful
Variable reference is properly defined and used correctly
The
--tvm-base-color-lightvariable is defined in base/vars.less and consistently used across components with matching fallback values.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the definition of the base color variable rg --type=less "(?:--tvm-base-color-light|--ti-mobile-base-color-light)" -B 2 -A 2Length of output: 5041
packages/theme-mobile/src/index-bar/vars.less (1)
1-5: Verify referenced common color variablesMultiple variables reference common color variables that should be defined in a base theme file.
✅ Verification successful
Common color variables verified successfully
All referenced common color variables are properly defined in the base theme file and used consistently across components.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the definition of common color variables rg --type=less "(?:--tvm-common-(?:bg-color-main|color-text-secondary|color-text-white))" -B 2 -A 2Length of output: 3916
packages/theme-mobile/src/avatar/vars.less (1)
1-10: Implementation looks good!The variable naming, base variable references, and values are consistent with the new conventions.
packages/theme-mobile/src/badge/vars.less (1)
1-1: Document breaking changesThe renaming of CSS variables from
--ti-mobile-*to--tvm-*and functions from.component-css-vars-*()to.inject-*-vars()are breaking changes. Please:
- Document these changes in the changelog
- Update the migration guide
- Consider adding a codemod to help users migrate
packages/theme-mobile/src/action-sheet/vars.less (1)
1-10: LGTM! Excellent use of semantic color variables.The implementation makes good use of semantic color variables (e.g.,
--tvm-base-color-light,--tvm-base-color-info-normal) which will help with theming and maintainability.packages/theme-mobile/src/index-bar-anchor/index.less (1)
19-19: LGTM! Variable renaming follows the new convention.The changes consistently follow the new naming pattern, maintaining the same styling functionality.
However, this change might affect consumers of these CSS variables.
Let's check for any external usage of the old variable names:
Also applies to: 21-21, 27-27
✅ Verification successful
✅ No external usage found for the old variable names
The variable renaming is safe to proceed as no other files in the codebase are using the old variable pattern.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining usage of old variable names rg --type=less --type=vue --type=css "var\(--ti-mobile-index-bar-anchor-"Length of output: 103
Script:
#!/bin/bash # Search for any usage of old variable names without file type restrictions rg "ti-mobile-index-bar-anchor-" # Also search with only less type as fallback rg --type=less "ti-mobile-index-bar-anchor-"Length of output: 77
packages/theme-mobile/src/container/index.less (1)
19-19: LGTM! Variable renaming follows the new convention.The changes consistently follow the new naming pattern, maintaining the same layout functionality.
However, this change might affect consumers of these CSS variables.
Let's check for any external usage of the old variable names:
Also applies to: 27-27, 35-35
packages/theme-mobile/src/index-bar/index.less (1)
20-20: LGTM! Consistent variable renaming.The changes follow the new naming convention while maintaining the component's styling structure.
Also applies to: 36-36, 44-44, 51-52
packages/theme-mobile/src/multi-select-item/index.less (1)
Line range hint
21-62: LGTM! Well-structured component styles.The implementation follows BEM methodology, uses flexbox appropriately, and maintains consistent variable naming.
packages/theme-mobile/src/label/index.less (1)
19-19: LGTM! Consistent variable usage across all states.The changes maintain consistency across all component states (sizes, colors, required, etc.) while following the new naming convention.
Also applies to: 27-28, 59-60, 64-65, 69-70, 74-75, 79-79, 83-83, 87-87, 91-91, 95-95, 99-99, 103-103
packages/theme-mobile/package.json (1)
68-68: LGTM! Necessary stylelint configuration update.The addition of
"custom-property-pattern": nullappropriately supports the new CSS variable naming convention.packages/theme-mobile/src/exception/index-global.less (1)
26-26: LGTM! Consistent variable renaming across the component.The changes follow the new naming convention while preserving all functionality and image references.
Also applies to: 31-32, 67-68, 74-75, 84-84, 86-86
packages/theme-mobile/src/badge/index.less (1)
19-19: LGTM! Consistent variable naming for badge properties.The changes follow the new naming convention while maintaining the same default values.
Also applies to: 26-31
packages/theme-mobile/src/list/index.less (1)
20-20: LGTM! Consistent variable naming for list properties.The changes follow the new naming convention while maintaining the same functionality.
Also applies to: 28-44
packages/theme-mobile/src/mini-picker/index.less (1)
20-20: LGTM! Consistent variable naming for picker mask and content.The changes follow the new naming convention while maintaining the same functionality.
Also applies to: 28-37
packages/theme-mobile/src/tabbar-item/index.less (1)
21-21: LGTM! Consistent variable naming for tabbar item properties.The changes follow the new naming convention while maintaining the same functionality.
Also applies to: 38-50
packages/theme-mobile/src/picker-column/index.less (1)
Line range hint
35-92: LGTM! Consistent variable naming for picker column properties.The changes follow the new naming convention while maintaining the same functionality.
packages/theme-mobile/src/pull-refresh/index.less (1)
19-19: LGTM! CSS variable renaming is consistent.The changes follow the new naming convention, properly using PascalCase for the component name "PullRefresh" and maintaining all fallback values.
Also applies to: 34-35, 59-60, 62-62, 70-71, 73-73, 81-82, 90-92
packages/theme-mobile/src/exception/index.less (1)
19-19: LGTM! CSS variable renaming is consistent.The changes follow the new naming convention, properly using PascalCase for the component name "Exception" and maintaining all fallback values.
Also applies to: 26-26, 31-32, 40-40, 44-44, 75-76, 81-82, 88-89, 98-98, 100-100
packages/theme-mobile/src/loading/index.less (1)
19-19: LGTM! CSS variable renaming is consistent.The changes follow the new naming convention, properly using PascalCase for the component name "Loading" and maintaining all fallback values. The animation keyframes remain unaffected, as they should.
Also applies to: 28-29, 48-49, 55-55, 57-57, 62-63, 71-72, 78-78, 84-85, 92-93, 99-100
packages/theme-mobile/src/file-upload/index.less (1)
23-23: LGTM! CSS variable renaming is consistent.The changes follow the new naming convention, properly using PascalCase for the component name "FileUpload" and maintaining all fallback values. The hairline mixin usage is correctly preserved.
Also applies to: 30-30, 34-35, 45-46, 57-58, 62-63, 75-79, 93-93
packages/theme-mobile/src/action-sheet/index.less (1)
20-20: LGTM! CSS variable renaming is consistent.The changes follow the new naming convention, properly using PascalCase for the component name "ActionSheet" and maintaining all fallback values.
Also applies to: 38-38, 46-46, 50-50, 57-57, 74-75, 81-81, 85-85, 99-99, 102-102, 109-109
packages/theme-mobile/src/checkbox/index.less (1)
22-22: LGTM! Variable renaming is consistent.The changes maintain all functionality while updating to the new naming convention. Complex calculations, transitions, and hover states are preserved correctly.
Also applies to: 27-27, 42-44, 50-52, 62-62, 70-70, 97-100, 107-107, 111-111, 122-122, 139-140, 145-148, 152-152
packages/theme-mobile/src/form-item/index.less (1)
25-25: LGTM! Error handling styles preserved.The variable renaming is consistent, and all error handling styles and complex selectors are maintained correctly.
Also applies to: 76-76, 121-121, 142-142, 152-152, 163-163, 169-169
packages/theme-mobile/src/switch/index.less (1)
17-17: LGTM! Complex calculations preserved.The variable renaming is consistent, and all complex calculations and animations are maintained correctly.
Also applies to: 24-24, 32-32, 34-34, 37-37, 46-47, 51-51, 59-59, 72-72, 76-76, 87-87, 92-92
packages/theme-mobile/src/search/index.less (1)
22-22: LGTM! The variable renaming improves readability and maintainability.The changes follow a consistent pattern:
- Shorter prefix (
tvmvsti-mobile)- PascalCase for component names
- Clear variable hierarchy
Also applies to: 27-27, 33-33, 35-35, 65-65, 69-69, 79-79, 81-81, 85-87, 90-90, 95-97, 100-100, 106-106, 108-108, 113-114, 116-116, 125-125
packages/theme-mobile/src/popover/index.less (1)
20-20: LGTM! Arrow positioning and styling preserved.The variable renaming maintains the complex positioning logic for the popover arrows while improving the naming convention.
Run this script to verify no regressions in arrow positioning:
Also applies to: 23-26, 30-31, 47-47, 54-54, 58-58, 64-64, 70-70, 77-77, 84-84, 90-90, 96-96, 102-102, 108-108, 115-115, 122-122, 129-129, 139-140, 159-159, 163-163, 167-169, 177-178, 183-184, 196-197, 199-199
✅ Verification successful
Arrow styling verified: Implementation consistent with base theme
The
.popper__arrowimplementation in the mobile theme maintains full compatibility with the base theme's positioning system, including all placement variants (top, bottom, left, right) and pseudo-element structure.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for any arrow-related style overrides in other files rg --type=less "popper__arrow"Length of output: 5623
packages/theme-mobile/src/button/index.less (1)
23-23: LGTM! Button states and loading animation preserved.The variable renaming maintains all button states and the loading animation functionality while improving naming consistency.
Run this script to verify no regressions in loading animation:
Also applies to: 29-29, 46-50, 54-55, 61-61, 66-67, 71-71, 75-75, 80-81, 88-90, 94-95, 100-100, 108-108, 112-113, 118-120, 125-126, 132-133, 139-140, 145-146, 151-152, 158-162, 165-166, 175-176, 182-183, 189-189, 195-195, 203-203, 208-209, 212-213
✅ Verification successful
Loading animation verified and working as intended
The loading animation is consistently implemented across all themes, with the mobile button intentionally using a slightly faster rotation speed (1.5s vs 2s) for better UX on mobile devices.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for any loading animation overrides rg --type=less "keyframes rotating"Length of output: 417
Script:
#!/bin/bash # Show the keyframe implementations and surrounding context rg --type=less -A 5 -B 2 "@keyframes rotating"Length of output: 2503
Script:
#!/bin/bash # Show complete keyframe definitions and loading class usage rg --type=less -A 8 "@keyframes rotating" && echo "=== Loading Usage ===" && rg --type=less "animation.*rotating"Length of output: 3550
packages/theme-mobile/src/modal/index.less (2)
24-24: LGTM! Method call updated for consistency.The update from
.component-css-vars-modal()to.inject-Modal-vars()aligns with the new naming convention.
366-371: Verify button dimensions across different device sizes.The button dimensions are now controlled by CSS variables. Ensure these values are responsive and work well across different device sizes.
packages/theme-mobile/src/input/index.less (2)
24-24: LGTM! Method call updated for consistency.The update from
.component-css-vars-input()to.inject-Input-vars()aligns with the new naming convention.
310-318: Verify input sizing across different breakpoints.The input sizes (medium, small, mini) use CSS variables. Ensure these values work well across different device sizes and breakpoints.
✅ Verification successful
Input sizes are appropriately implemented for mobile devices
The current implementation uses fixed sizes that are suitable for touch targets on mobile devices (42px, 36px, 24px). These sizes don't need responsive adjustments as they're already optimized for mobile interaction, with the medium size (42px) meeting the minimum touch target size recommendations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any media queries or responsive adjustments related to input sizes rg -A 3 "@media.*{.*--tvm-Input-(medium|small|mini)-height"Length of output: 202
Script:
#!/bin/bash # Search for media queries in input-related files fd "input.*\.(less|css|scss)$" | xargs rg "@media" # Search for the CSS variable names rg --type=less "--tvm-Input-(medium|small|mini)-height" # Search for breakpoint-related code in input files fd "input.*\.(less|css|scss)$" | xargs rg "breakpoint|screen|@media"Length of output: 620
Script:
#!/bin/bash # Search for the CSS variable definitions rg "tvm-Input-(medium|small|mini)-height" # Search for responsive styles in input files fd "input" | xargs rg -l "@media" # Look for theme configuration files that might define these variables fd "theme|config|variable" | grep -i "theme\|var" | xargs rg "tvm-Input.*height" # Search for responsive mixins used with inputs rg -B 2 -A 2 "\.input-size"Length of output: 6099
| @progress-prefix-cls: ~'@{css-prefix}mobile-progress'; | ||
|
|
||
| .@{progress-prefix-cls} { | ||
| .inject-Popover-vars(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Rename .inject-Popover-vars() to .inject-Progress-vars()
The mixin contains Progress-specific variables but is incorrectly named as "Popover". It should be renamed to better reflect its purpose.
packages/theme-mobile/src/progress/vars.less: Rename the mixinpackages/theme-mobile/src/progress/index.less: Update the mixin reference
🔗 Analysis chain
Verify the necessity of Popover styles.
The Progress component doesn't appear to have any Popover-related functionality. Is the .inject-Popover-vars() mixin necessary here?
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any Popover usage in the Progress component
rg -l "Popover" packages/theme-mobile/src/progress/
Length of output: 142
Script:
#!/bin/bash
# Check the content of vars.less for Popover-related code
cat packages/theme-mobile/src/progress/vars.less
# Also check for any Popover-related DOM elements or classes in the component
rg -i "popover" packages/theme-mobile/src/progress/ -A 3 -B 3
Length of output: 1197
| --tvm-Switch-width-normal: calc(var(--tvm-base-size-width-small) + 4px); | ||
| // 默认尺寸高度 | ||
| --ti-mobile-switch-height-mormal: var(--ti-mobile-base-size-height-mini, 24px); | ||
| --tvm-Switch-height-mormal: var(--tvm-base-size-height-mini, 24px); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix typo in variable name.
The variable name contains a typo: --tvm-Switch-height-mormal should be --tvm-Switch-height-normal.
| // 小尺寸按钮大小 | ||
| --ti-mobile-switch-btn-size-mini: calc(var(--ti-mobile-base-size-width-min) - 8px); | ||
| --tvm-Switch-btn-size-mini: calc(var(--tvm-base-size-width-min) - 8px); | ||
| // 默认关状态背景色 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Fix incorrect base variable reference
The switch component references a non-existent variable --tvm-base-size-width-min. This should be corrected to --tvm-base-size-width-mini which is defined in base/vars.less.
packages/theme-mobile/src/switch/vars.less: Update both size calculations to use--tvm-base-size-width-mini
🔗 Analysis chain
Verify the base variable name.
Both button size variables reference --tvm-base-size-width-min. Should this be --tvm-base-size-width-mini instead? This needs verification to ensure the correct base variable is being used.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for both possible base variable names
echo "Searching for potential base variable definitions..."
rg --type less "(--tvm-base-size-width-min[^i]|--tvm-base-size-width-mini)" packages/theme-mobile/src/
Length of output: 610
| // 禁用状态下背景色 | ||
| --ti-mobile-switch-bg-color-disabled: var(--ti-mobile-color-bg-control-disable-1, #dbdbdb); | ||
| --tvm-Switch-bg-color-disabled: var(--tvm-color-bg-control-disable-1, #dbdbdb); | ||
| // 开状态禁用背景色 | ||
| --ti-mobile-switch-bg-color-checked-disabled: var(--ti-mobile-color-bg-control-disable-2, #b3d6ff); | ||
| --tvm-Switch-bg-color-checked-disabled: var(--tvm-color-bg-control-disable-2, #b3d6ff); | ||
| // 开状态背景色 | ||
| --ti-mobile-switch-bg-color-checked: var(--ti-mobile-color-bg-control-active, #1476ff); | ||
| --tvm-Switch-bg-color-checked: var(--tvm-color-bg-control-active, #1476ff); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Fix incorrect variable names in Switch component
The Switch component uses incorrect variable names:
--tvm-color-bg-control-disable-1should be--tvm-color-bg-control-disabled-1--tvm-color-bg-control-disable-2should be--tvm-color-bg-control-disabled-2
The base variables are correctly defined with "disabled" in basic-var.less, and this naming pattern is also used correctly in other components like Checkbox.
🔗 Analysis chain
Verify the existence of referenced color variables.
The background color variables reference several base color variables that need verification:
--tvm-color-bg-control-default--tvm-color-bg-control-disable-1--tvm-color-bg-control-disable-2--tvm-color-bg-control-active
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for base color variable definitions
echo "Searching for base color variable definitions..."
rg --type less "(--tvm-color-bg-control-(default|disable-[12]|active))" packages/theme-mobile/src/
Length of output: 903
Script:
#!/bin/bash
# Search for disable-related color variables with broader pattern
rg --type less "color-bg-control-disable" packages/theme-mobile/src/
Length of output: 672
| .inject-Popover-vars() { | ||
| --tvm-Progress-success-color: #52c41a; | ||
| --tvm-Progress-warning-color: #faad14; | ||
| --tvm-Progress-exception-color: #f5222d; | ||
| --tvm-Progress-bar-outer-bgcolor: #f5f5f5; | ||
| --tvm-Progress-bar-inner-text-color: #fff; | ||
| --tvm-Progress-text-font-size: 12px; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix function name mismatch
The function name .inject-Popover-vars() doesn't match the component name (Progress) and its variables. This appears to be a copy-paste error.
-.inject-Popover-vars() {
+.inject-Progress-vars() {Additionally, consider centralizing the status colors (#52c41a, #faad14, #f5222d) in a common theme file for better maintainability.
📝 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.
| .inject-Popover-vars() { | |
| --tvm-Progress-success-color: #52c41a; | |
| --tvm-Progress-warning-color: #faad14; | |
| --tvm-Progress-exception-color: #f5222d; | |
| --tvm-Progress-bar-outer-bgcolor: #f5f5f5; | |
| --tvm-Progress-bar-inner-text-color: #fff; | |
| --tvm-Progress-text-font-size: 12px; | |
| } | |
| .inject-Progress-vars() { | |
| --tvm-Progress-success-color: #52c41a; | |
| --tvm-Progress-warning-color: #faad14; | |
| --tvm-Progress-exception-color: #f5222d; | |
| --tvm-Progress-bar-outer-bgcolor: #f5f5f5; | |
| --tvm-Progress-bar-inner-text-color: #fff; | |
| --tvm-Progress-text-font-size: 12px; | |
| } |
|
|
||
| .@{picker-column-prefix-cls} { | ||
| .component-css-vars-picker-column(); | ||
| .inject-picker-column-vars(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Maintain consistent PascalCase naming.
The mixin name uses kebab-case (picker-column) while other components use PascalCase. Consider updating to .inject-PickerColumn-vars() for consistency.
- .inject-picker-column-vars();
+ .inject-PickerColumn-vars();Committable suggestion skipped: line range outside the PR's diff.
|
|
||
| .@{dialog-box-prefix-cls} { | ||
| .component-css-vars-dialog-box(); | ||
| .inject-DialogBox-vars(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Replace hardcoded colors with semantic theme variables
The review comment is valid. The dialog-box component uses hardcoded colors instead of the available semantic theme variables. Here are the specific replacements needed:
#ccc→var(--tvm-color-border-default)#999→var(--tvm-color-text-placeholder)#333→var(--tvm-color-text-primary)
🔗 Analysis chain
Extract hardcoded color values into theme variables.
Consider replacing hardcoded color values with theme variables for better maintainability and consistency.
Example refactor:
- color: var(--tvm-DialogBox-cancel-button-color, #999);
+ color: var(--tvm-DialogBox-cancel-button-color, var(--tvm-color-gray-500, #999));Also applies to: 26-27, 50-51, 56-57, 71-71, 87-88, 92-92
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for other components using similar color values
rg -l '#999|#333|#ccc' 'packages/theme-mobile/src/**/*.less'
Length of output: 138
Script:
#!/bin/bash
# Search for color values in all .less files
rg -l '#[0-9a-fA-F]{3,6}|rgb\(|rgba\(' --type less
# Search for existing color variables in theme files
rg '(?i)--tvm-color-gray-|--tvm-color-neutral-' --type less
Length of output: 9788
Script:
#!/bin/bash
# Check base variables in theme-mobile
cat packages/theme-mobile/src/base/basic-var.less
# Check dialog-box variables
cat packages/theme-mobile/src/dialog-box/vars.less
Length of output: 10146
💡 Codebase verification
Breaking change confirmed: CSS variable renaming needs documentation
The review comment is valid. The codebase shows extensive use of the new --tvm-* variable naming pattern across all components, replacing the old --ti-mobile-* pattern. However, there's no documentation about this change in the README files or elsewhere. This will impact users who have created custom themes using the old variable names.
- The change affects all theme components in
packages/theme-mobile/src/* - No migration guide exists in the documentation
- README files need updating to reflect the new naming convention
🔗 Analysis chain
Consider documenting breaking changes and providing a migration guide.
The renaming of CSS variables from --ti-mobile-* to --tvm-* is a breaking change that affects theme customization. Consider:
- Adding a migration guide for users to update their custom themes.
- Documenting the new naming convention in the project's style guide.
- Providing a codemod or script to help users migrate their custom themes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Find potential custom theme files that might need updates
fd -e less -e css custom-theme
Length of output: 32
Script:
#!/bin/bash
# Search for old and new variable patterns
echo "=== Searching for CSS variable patterns ==="
rg --type=less --type=css "(--ti-mobile-|--tvm-)" -l
echo -e "\n=== Looking for documentation files ==="
fd -e md . packages/theme-mobile
echo -e "\n=== Finding theme-related files ==="
fd -e less -e css . packages/theme-mobile/src
Length of output: 10410
Script:
#!/bin/bash
# Check documentation and base variable files
echo "=== Checking README files ==="
cat packages/theme-mobile/README.md packages/theme-mobile/README.zh-CN.md
echo -e "\n=== Checking base variable definitions ==="
cat packages/theme-mobile/src/base/basic-var.less packages/theme-mobile/src/base/vars.less
echo -e "\n=== Checking dialog-box variables ==="
cat packages/theme-mobile/src/dialog-box/vars.less
Length of output: 12903
| width: var(--ti-mobile-switch-width-normal, 40px); | ||
| height: var(--ti-mobile-switch-height-mormal, 24px); | ||
| width: var(--tvm-Switch-width-normal, 40px); | ||
| height: var(--tvm-Switch-height-mormal, 24px); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix typo in height variable name.
There's a typo in the height variable name: height-mormal should be height-normal.
- height: var(--tvm-Switch-height-mormal, 24px);
+ height: var(--tvm-Switch-height-normal, 24px);📝 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.
| height: var(--tvm-Switch-height-mormal, 24px); | |
| height: var(--tvm-Switch-height-normal, 24px); |
| left: 0; | ||
| width: 100%; | ||
| background-color: var(--ti-mobile-action-sheet-content-bg-color, #fff); | ||
| background-color: var(--tvm-ActionSheet-content-bg-color, #fff); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zzcr 建议把这个前缀定义成一个变量,放到一个公共的地方,后续要改就改这个变量就行了。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个貌似在运行态无法实现,我看看能不能在编译态定制
优化theme-mobile工程css变量名称,补充未定义的css变量
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
Based on the comprehensive summary of changes, here are the release notes:
Release Notes: Theme Mobile Styling Update
Styling Refactoring
ti-mobile-*prefix totvm-*prefixAffected Components
No Functional Changes