fix(button): [button,cascader] fix button's radius and cascader bgcolor #2572
Conversation
WalkthroughThe pull request introduces new button categories in the Vue components, specifically adding "圆角按钮" (rounded buttons) and "圆形按钮" (circular buttons) sections. Each section includes multiple Changes
Possibly related PRs
Suggested labels
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: 0
🧹 Outside diff range and nitpick comments (5)
examples/sites/demos/pc/app/button/basic-usage-composition-api.vue (2)
21-29: Consider improving accessibility and internationalizationWhile the implementation is consistent with existing patterns, consider these improvements:
- Add ARIA labels for better accessibility
- Use i18n translations instead of hardcoded Chinese text
Example improvement:
- <p>圆角按钮</p> + <p role="heading" aria-level="2">{{ t('button.rounded') }}</p> <tiny-row> - <tiny-button type="primary" round> 主要按钮 </tiny-button> + <tiny-button + type="primary" + round + :aria-label="t('button.primary')" + > + {{ t('button.primary') }} + </tiny-button>
Line range hint
30-38: Add accessibility attributes for icon-only buttonsIcon-only buttons should include proper ARIA labels to ensure accessibility for screen readers.
Example improvement:
- <tiny-button :icon="IconSearch" circle></tiny-button> + <tiny-button + :icon="IconSearch" + circle + :aria-label="t('button.search')" + ></tiny-button>examples/sites/demos/pc/app/button/basic-usage.vue (2)
Line range hint
30-38: Consider adding ARIA labels for icon-only buttons.While the circular buttons are implemented correctly, icon-only buttons should include aria-labels for better accessibility.
Apply this enhancement to improve accessibility:
- <tiny-button :icon="IconSearch" circle></tiny-button> + <tiny-button :icon="IconSearch" circle aria-label="搜索"></tiny-button> - <tiny-button type="primary" :icon="IconEditor" circle></tiny-button> + <tiny-button type="primary" :icon="IconEditor" circle aria-label="编辑"></tiny-button> - <tiny-button type="success" :icon="IconYes" circle></tiny-button> + <tiny-button type="success" :icon="IconYes" circle aria-label="确认"></tiny-button> - <tiny-button type="info" :icon="IconMail" circle></tiny-button> + <tiny-button type="info" :icon="IconMail" circle aria-label="邮件"></tiny-button> - <tiny-button type="warning" :icon="IconStarO" circle></tiny-button> + <tiny-button type="warning" :icon="IconStarO" circle aria-label="收藏"></tiny-button> - <tiny-button type="danger" :icon="IconDeleteL" circle></tiny-button> + <tiny-button type="danger" :icon="IconDeleteL" circle aria-label="删除"></tiny-button>
Line range hint
69-73: Consider removing redundant margin-left style.The
margin-left: 0declaration appears to be redundant since it's not overriding any inherited styles.Apply this simplification:
.tiny-button { margin-bottom: 10px; - margin-left: 0; margin-right: 24px; }packages/theme/src/button/vars.less (1)
Line range hint
31-31: Consider implementing a systematic spacing calculation.The comment suggests replacing hardcoded values (24px) with calculated values (space-base * 6). This would improve maintainability and consistency across the design system.
Consider implementing a spacing calculation system:
- --tv-Button-padding-x: 24px; + --tv-Button-padding-x: calc(var(--tv-space-base) * 6);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
examples/sites/demos/pc/app/button/basic-usage-composition-api.vue(1 hunks)examples/sites/demos/pc/app/button/basic-usage.vue(1 hunks)examples/sites/demos/pc/app/button/webdoc/button.js(1 hunks)packages/theme/src/base/old-theme.less(0 hunks)packages/theme/src/base/vars.less(0 hunks)packages/theme/src/button/vars.less(1 hunks)packages/theme/src/cascader-node/vars.less(1 hunks)packages/theme/src/guide/vars.less(1 hunks)packages/theme/src/pager/vars.less(1 hunks)
💤 Files with no reviewable changes (2)
- packages/theme/src/base/old-theme.less
- packages/theme/src/base/vars.less
🔇 Additional comments (8)
packages/theme/src/cascader-node/vars.less (1)
15-15: LGTM! Verify visual consistency across themes.
The change to use var(--tv-color-bg, #f5f5f5) improves consistency by using the primary background color variable. This creates a more unified appearance with other components.
Let's verify this change doesn't impact other themes:
✅ Verification successful
Based on the search results, I can now generate the final response since I have enough information to verify the change:
Change to hover background color is consistent with design system
The change from var(--tv-color-bg-secondary, #f0f0f0) to var(--tv-color-bg, #f5f5f5) aligns with the design system's color tokens:
--tv-color-bgis defined as the standard background color for hover states across components (e.g., DropdownItem, Option, Transfer panel items)- The fallback value
#f5f5f5matches the design system's base token value inpackages/theme/src/base/vars.less - Other similar components like Tree and Transfer also use
var(--tv-color-bg, #f5f5f5)for their hover states
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any custom theme definitions that might override these colors
# and ensure the change is consistent across all themes
# Search for any custom theme definitions of the background colors
rg --type=less "tv-color-bg-secondary|tv-color-bg" packages/theme/src
# Check for any other components using these variables
rg --type=less "CascaderNode-hover-bg-color" packages/theme/src
Length of output: 39878
examples/sites/demos/pc/app/button/basic-usage-composition-api.vue (1)
Line range hint 21-38: Implementation aligns with PR objectives
The addition of rounded and circular button variants properly implements the button radius fixes mentioned in the PR title.
Let's verify the button radius changes across the codebase:
✅ Verification successful
Implementation aligns with button radius requirements
The verification confirms proper implementation of button radius styles:
- Button component has dedicated CSS variables for round and circle variants (
--tv-Button-border-radius-roundand--tv-Button-border-radius-circle) - Round buttons use
var(--tv-border-radius-round)which is set to 999px to ensure consistent rounded corners - Circle buttons also use the same radius variable to maintain perfect circular shape
- The changes are consistently used across multiple demo files showing proper implementation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent button radius implementation
# Search for button radius related styles
rg -t less "border-radius" "packages/theme/"
# Search for round/circle button usage
rg -t vue "(?:round|circle).*tiny-button"
Length of output: 41644
Script:
#!/bin/bash
# Search for button radius styles in theme files
rg -t less "Button.*border-radius" "packages/theme/src/button/"
# Search for round/circle button usage in Vue files (fixing file type)
rg "(?:round|circle).*tiny-button" --type-add 'vue:*.vue' -t vue
Length of output: 9319
examples/sites/demos/pc/app/button/basic-usage.vue (1)
21-29: LGTM! Consistent implementation of rounded buttons section.
The new rounded buttons section maintains consistency with existing patterns and properly implements the round attribute across all button variants.
packages/theme/src/guide/vars.less (2)
Line range hint 1-63: Missing cascader bgcolor changes mentioned in PR title.
The PR title mentions fixes for "cascader bgcolor" but no related changes are visible in this file. Please ensure these changes are included in the PR or update the PR title accordingly.
47-47: Verify the visual impact of the border radius change.
The change from --tv-border-radius-brand to --tv-border-radius-round for guide buttons should be tested to ensure it maintains visual consistency with other rounded buttons in the system.
✅ Verification successful
Border radius change aligns with design system patterns
The change from --tv-border-radius-brand to --tv-border-radius-round for guide buttons is consistent with the design system's usage patterns. The verification shows:
- Other components using
--tv-border-radius-roundinclude badges, image viewer actions, pager items, and various circular UI elements - Regular buttons use
--tv-border-radius-mdby default, with specific variants likeroundandcircleusing--tv-border-radius-round - The guide button's rounded style aligns with other action-oriented UI elements in the system
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent usage of border radius variables across button-related components
# Search for other components using border radius variables
rg --type=less "border-radius: var\(--tv-(border-radius-brand|border-radius-round)\)"
# Check for any potential conflicts or inconsistencies in button styling
rg --type=less "Button.*border-radius"
Length of output: 3513
packages/theme/src/pager/vars.less (1)
65-65: LGTM! Verify visual consistency with other components.
The change from --tv-border-radius-brand to --tv-border-radius-round aligns with the PR's objective of standardizing border radius values across components. However, please ensure this rounder style maintains visual harmony with adjacent components in various layouts.
Let's verify the usage of these border radius variables across components:
✅ Verification successful
Border radius change aligns with design system standards
The change from --tv-border-radius-brand to --tv-border-radius-round in the pager component is consistent with the design system's approach. The verification shows that:
--tv-border-radius-roundis widely used across multiple components (button, badge, calendar, dropdown, etc.)- It's defined in base theme files with a specific value of
999pxto avoid elliptical shapes that occur with50% - The pager's border radius now matches other interactive elements like buttons and dropdown items
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check the usage of border radius variables across components
# to ensure consistent styling approach
# Search for border radius variable usage patterns
rg --type less "border-radius-brand|border-radius-round" packages/theme/src/
Length of output: 3641
examples/sites/demos/pc/app/button/webdoc/button.js (1)
13-13: LGTM! Documentation updates accurately reflect the new button attributes.
The changes properly document the new round and circle button attributes in both Chinese and English descriptions, maintaining consistency across languages and aligning with the PR's objective to fix button radius functionality.
Let's verify that these documented attributes are actually implemented in the referenced component:
Also applies to: 15-15
✅ Verification successful
The documented round and circle button attributes are properly implemented in the basic usage demos
The verification shows that both round and circle attributes are correctly implemented in the button component's basic usage demos:
- Found in
examples/sites/demos/pc/app/button/basic-usage.vue:- Round buttons for all button types (primary, default, success, info, warning, danger)
- Circle buttons with icons for all button types
- Same implementation exists in the composition API version at
examples/sites/demos/pc/app/button/basic-usage-composition-api.vue
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the round and circle attributes are implemented in the basic-usage demo
# Expected: Find implementations of round and circle attributes in the basic-usage.vue file
# Search for round and circle attributes in the basic-usage demo file
fd -e vue 'basic-usage' | xargs rg -A 2 'round|circle'
Length of output: 17032
packages/theme/src/button/vars.less (1)
23-23: LGTM! Border radius standardization looks good.
The change from var(--tv-border-radius-brand) to var(--tv-border-radius-md) aligns with the PR objective to fix button radius. The comment indicates this restores the default to 6px.
Let's verify other components aren't negatively impacted by this change:
✅ Verification successful
Border radius change is consistent across components
The verification shows that var(--tv-border-radius-md) is consistently used as the default border radius across multiple components in the design system. The change from var(--tv-border-radius-brand) to var(--tv-border-radius-md) in the button component aligns with this pattern. The base theme defines this value as 6px in packages/theme/src/base/vars.less, which matches the comment "默认还原为6px" (default restored to 6px).
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for other components using the border radius variables
# to ensure consistency across the design system
# Search for border radius variable usage
rg --type=less "tv-border-radius-(brand|md)" -B 2 -A 2
Length of output: 20806
PR
PR Checklist
Please check if your PR fulfills the following requirements:
Other information
还原button的圆角
修复cascader的菜单的hover背景色
Summary by CodeRabbit