feat(theme): [button] submit the initial version of aurora-theme-vars #2596
Conversation
WalkthroughThe changes in this pull request introduce a new CSS file, Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (3)packages/theme/src/base/aurora-theme.less (3)
Ensure consistency between variable values and comments for information colors The values assigned to Apply this diff to correct the variable values: - --tv-color-act-info-bg-light: rgba(51,51,51,0.06); // #f0f7ff 信息背景色-1（信息主色衍生色）：朴素
+ --tv-color-act-info-bg-light: #f0f7ff; // #f0f7ff 信息背景色-1（信息主色衍生色）：朴素
- --tv-color-act-info-bg-light-hover: #333; // #f0f7ff 信息hover背景色-1
+ --tv-color-act-info-bg-light-hover: #f0f7ff; // #f0f7ff 信息hover背景色-1
- --tv-color-act-info-bg-light-active:#54657e; // #f0f7ff 信息active背景色-1
+ --tv-color-act-info-bg-light-active: #f0f7ff; // #f0f7ff 信息active背景色-1
Inconsistent variable value and comment for '--tv-color-text-important' The variable Apply this diff to correct the inconsistency: - --tv-color-text-important: #333; // #191919 重要文本色-文本_金额
+ --tv-color-text-important: #191919; // #191919 重要文本色-文本_金额
Inconsistent variable value and comment for '--tv-color-icon-hover' The variable Apply this diff to correct the inconsistency: - --tv-color-icon-hover: #1890ff; // #40a9ff 图标悬浮色
+ --tv-color-icon-hover: #40a9ff; // #40a9ff 图标悬浮色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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (2)
packages/theme/src/option/index.less (1)
96-103: LGTM! Consider documenting the disabled state styling.The changes improve the visual hierarchy of disabled options by properly handling different states (checked, unchecked, half-selected) with appropriate color variables. This enhancement provides better visual feedback to users.
Consider adding a comment block explaining the different disabled states and their corresponding color variables for better maintainability.
packages/theme/src/button/vars.less (1)
263-263: Consider standardizing the-hover-1suffix in border color variables.Some ghost active border colors use
-hover-1suffix while others use-hover. This inconsistency might cause confusion:
--tv-Button-border-color-ghost-active-success: var(--tv-color-act-success-border-hover-1)--tv-Button-border-color-ghost-active-danger: var(--tv-color-act-danger-border-hover-1)Consider standardizing these variable names for better maintainability.
Also applies to: 293-293
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (12)
packages/theme/src/base/aurora-theme.less(1 hunks)packages/theme/src/base/index.less(1 hunks)packages/theme/src/base/old-theme.less(0 hunks)packages/theme/src/base/vars.less(0 hunks)packages/theme/src/breadcrumb/index.less(0 hunks)packages/theme/src/button/vars.less(2 hunks)packages/theme/src/calendar/vars.less(1 hunks)packages/theme/src/option/index.less(1 hunks)packages/theme/src/option/vars.less(1 hunks)packages/theme/src/pager/vars.less(1 hunks)packages/theme/src/popeditor/vars.less(1 hunks)packages/theme/src/selected-box/vars.less(1 hunks)
💤 Files with no reviewable changes (3)
- packages/theme/src/breadcrumb/index.less
- packages/theme/src/base/old-theme.less
- packages/theme/src/base/vars.less
✅ Files skipped from review due to trivial changes (1)
- packages/theme/src/base/index.less
🔇 Additional comments (5)
packages/theme/src/popeditor/vars.less (1)
23-23: Use consistent icon sizing with '--tv-icon-size'
Updating --tv-Popeditor-icon-font-size to use --tv-icon-size enhances consistency across components by standardizing icon sizes.
packages/theme/src/option/vars.less (1)
33-36: Add missing variable for unchecked disabled state
The addition of --tv-Option-icon-color-unchecked-disabled improves clarity and control over the styling of unchecked disabled option icons. The updated comments clearly distinguish between the checked and unchecked disabled states.
packages/theme/src/calendar/vars.less (1)
27-27: Standardize hover background color with theme variable
Changing --tv-Calendar-day-hover-bg to use --tv-color-bg-hover promotes consistency across components by utilizing the common hover background color defined in the theme variables.
packages/theme/src/pager/vars.less (1)
55-57: LGTM! Improved state management for pagination items.
The changes enhance visual feedback by using dedicated background colors for active and hover states, improving the consistency with the aurora theme's color system.
packages/theme/src/button/vars.less (1)
236-238: LGTM! Standardized active state styling across button themes.
The changes improve consistency by standardizing active states to use hover variants across all button themes (primary, success, warning, danger, info).
Also applies to: 242-244, 251-253, 257-259, 281-283, 287-289
| --tv-color-text-placeholder: #999; // #999 占位文本色 | ||
| --tv-color-text-disabled: #999; // #999 禁用文本色 | ||
| --tv-color-text-active: #1890ff; // #1890ff 选中文本色 / 文本高亮色（搜索关键字高亮） | ||
| --tv-color-text-important: #333; // #191919 重要文本色-文本_金额 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Align variable value with comment for '--tv-color-text-important'
The variable --tv-color-text-important is assigned #333, but the comment mentions #191919. Please ensure that the variable value matches the color code described in the comment to prevent inconsistencies.
Apply this diff to update the variable value or comment:
Committable suggestion skipped: line range outside the PR's diff.
| --tv-color-act-info-bg-light: rgba(51,51,51,0.06); // #f0f7ff 信息背景色-1（信息主色衍生色）：朴素 | ||
| --tv-color-act-info-bg-light-hover: #333; // #f0f7ff 信息hover背景色-1 | ||
| --tv-color-act-info-bg-light-active:#54657e; // #f0f7ff 信息active背景色-1 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure consistency between variable values and comments
The values assigned to --tv-color-act-info-bg-light, --tv-color-act-info-bg-light-hover, and --tv-color-act-info-bg-light-active do not match the color codes mentioned in the comments. For example, --tv-color-act-info-bg-light is set to rgba(51,51,51,0.06), but the comment notes #f0f7ff. Please update the comments or variable values to maintain consistency and avoid confusion.
Apply this diff to correct the comments:
Committable suggestion skipped: line range outside the PR's diff.
| --tv-base-color-common-12: #2e3243; | ||
| --tv-base-color-common-12: #000000; | ||
| --tv-base-color-common-12: #d9d9d9; | ||
|
|
There was a problem hiding this comment.
Remove duplicate declarations of '--tv-base-color-common-12'
The variable --tv-base-color-common-12 is declared multiple times with different values at lines 30 to 33. This can cause confusion and only the last declaration will take effect. Please consolidate these into a single definition with the intended value.
Apply this diff to remove duplicate declarations:
Committable suggestion skipped: line range outside the PR's diff.
| @@ -25,7 +25,7 @@ | |||
| // 图标色 | |||
| --tv-SelectedBox-icon-color: var(--tv-color-icon, #808080); | |||
| // 图标悬浮色 | |||
| --tv-SelectedBox-icon-color-hover: var(--tv-color-icon-hover#191919); | |||
| --tv-SelectedBox-icon-color-hover: var(--tv-color-icon-hover, #191919); | |||
There was a problem hiding this comment.
Fix syntax error in CSS variable fallback
The correction in --tv-SelectedBox-icon-color-hover adds the missing comma between the variable and fallback value, ensuring proper CSS variable syntax and preventing potential parsing errors.
MomoPoppy
force-pushed
the
MomoPoppy/feat-aurora-theme
branch
from
f95ac1a to
ea30b4c
Compare
MomoPoppy
changed the title
|
|
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
packages/theme/src/base/aurora-theme.less (1)
142-142: Add comment for '--tv-color-act-primary-plain-text-hover'Consider adding a descriptive comment for
--tv-color-act-primary-plain-text-hoverto maintain consistency and improve code readability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (12)
packages/theme/src/base/aurora-theme.less(1 hunks)packages/theme/src/base/index.less(1 hunks)packages/theme/src/base/old-theme.less(0 hunks)packages/theme/src/base/vars.less(0 hunks)packages/theme/src/breadcrumb/index.less(0 hunks)packages/theme/src/button/vars.less(2 hunks)packages/theme/src/calendar/vars.less(1 hunks)packages/theme/src/option/index.less(1 hunks)packages/theme/src/option/vars.less(1 hunks)packages/theme/src/pager/vars.less(1 hunks)packages/theme/src/popeditor/vars.less(1 hunks)packages/theme/src/selected-box/vars.less(1 hunks)
💤 Files with no reviewable changes (3)
- packages/theme/src/breadcrumb/index.less
- packages/theme/src/base/old-theme.less
- packages/theme/src/base/vars.less
🚧 Files skipped from review as they are similar to previous changes (8)
- packages/theme/src/base/index.less
- packages/theme/src/calendar/vars.less
- packages/theme/src/popeditor/vars.less
- packages/theme/src/selected-box/vars.less
- packages/theme/src/option/vars.less
- packages/theme/src/option/index.less
- packages/theme/src/pager/vars.less
- packages/theme/src/button/vars.less
🔇 Additional comments (3)
packages/theme/src/base/aurora-theme.less (3)
29-32: Remove duplicate declarations of '--tv-base-color-common-12'
The variable --tv-base-color-common-12 is declared multiple times with different values from lines 29 to 32. This can cause confusion, and only the last declaration will take effect. Please consolidate these into a single definition with the intended value.
244-246: Ensure consistency between variable values and comments
The values assigned to --tv-color-act-info-bg-light, --tv-color-act-info-bg-light-hover, and --tv-color-act-info-bg-light-active do not match the color codes mentioned in the comments. For example, --tv-color-act-info-bg-light is set to rgba(51,51,51,0.06), but the comment notes #f0f7ff. Please update the comments or variable values to maintain consistency and avoid confusion.
267-267: Align variable value with comment for '--tv-color-text-important'
The variable --tv-color-text-important is assigned #333, but the comment mentions #191919. Please ensure that the variable value matches the color code described in the comment to prevent inconsistencies.
|
|
||
| /** 2.5 图标色 **/ | ||
| --tv-color-icon: #bfbfbf; // #bfbfbf 默认图标 | ||
| --tv-color-icon-hover: #1890ff; // #40a9ff 图标悬浮色 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Inconsistent variable value and comment for '--tv-color-icon-hover'
The variable --tv-color-icon-hover is assigned #1890ff, but the comment mentions #40a9ff. Please update the variable value or the comment to ensure consistency.
Apply this diff to correct the inconsistency:
- --tv-color-icon-hover: #1890ff; // #40a9ff 图标悬浮色
+ --tv-color-icon-hover: #40a9ff; // #40a9ff 图标悬浮色📝 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.
| --tv-color-icon-hover: #1890ff; // #40a9ff 图标悬浮色 | |
| --tv-color-icon-hover: #40a9ff; // #40a9ff 图标悬浮色 |
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
New Features
Bug Fixes
Documentation
Chores