refactor(dropdown/action-menu): [dropdown,action-menu] refactor styles for drodpwn #2343
Conversation
WalkthroughThe changes in this pull request focus on enhancing the styling of the action menu and dropdown components within the theme package. Modifications include adjustments to padding, background colors, text decoration on hover for action menu items, and the introduction of border-radius properties for dropdown buttons. Additionally, new CSS variables for button border radii are defined to ensure consistency in styling across button groups. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
[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 |
MomoPoppy
changed the title
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
packages/theme/src/dropdown/vars.less (1)
38-43: LGTM! Consider minor formatting adjustment for readability.The new CSS variables for button border radii in button groups are well-implemented and consistent with the existing styling patterns. They provide a nice rounded effect for button groups and allow for easy theming.
Consider adjusting the formatting slightly for better readability:
- --tv-Dropdown-caret-button-border-radius: 0 var(--tv-border-radius-round, 999px) var(--tv-border-radius-round, 999px) - 0; + --tv-Dropdown-caret-button-border-radius: + 0 + var(--tv-border-radius-round, 999px) + var(--tv-border-radius-round, 999px) + 0; - --tv-Dropdown-title-button-border-radius: var(--tv-border-radius-round, 999px) 0 0 - var(--tv-border-radius-round, 999px); + --tv-Dropdown-title-button-border-radius: + var(--tv-border-radius-round, 999px) + 0 + 0 + var(--tv-border-radius-round, 999px);This formatting makes it easier to read the four values of the
border-radiusproperty at a glance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- packages/theme/src/action-menu/index.less (2 hunks)
- packages/theme/src/dropdown/index.less (1 hunks)
- packages/theme/src/dropdown/vars.less (1 hunks)
🧰 Additional context used
🔇 Additional comments (6)
packages/theme/src/action-menu/index.less (3)
40-41: LGTM: Padding adjustment for visible dropdown items.The removal of padding for visible dropdown items creates a more compact layout. This change appears intentional and aligns with the refactoring objectives.
58-60: LGTM: Enhanced hover feedback with underline.Adding an underline to the dropdown item wrap on hover improves the user experience by providing clear visual feedback. This change enhances accessibility and is consistent with common web design patterns.
Line range hint
40-60: Summary: Improved styling and interaction for action menu items.The changes in this file enhance the action menu component by:
- Adjusting padding for a more compact layout.
- Removing the background color on hover for a cleaner look.
- Adding an underline on hover for better visual feedback.
These modifications align well with the PR's refactoring objectives and should improve the overall user experience. However, please ensure that the hover state remains sufficiently visible after removing the background color change.
packages/theme/src/dropdown/index.less (3)
77-77: LGTM: Improved styling for the dropdown title buttonThe addition of the
border-radiusproperty using the--tv-Dropdown-title-button-border-radiusvariable enhances the visual consistency of the dropdown component and allows for easy customization.
85-86: LGTM: Enhanced styling for the dropdown caret buttonThe changes improve the visual consistency of the dropdown component:
- The addition of the
border-radiusproperty using the--tv-Dropdown-caret-button-border-radiusvariable allows for easy customization.- Setting
margin-left: 0ensures proper alignment with the title button.These modifications contribute to a more polished and cohesive appearance of the dropdown component.
77-86: Summary: Improved styling and customization for dropdown buttonsThe changes in this file enhance the dropdown component's appearance and flexibility:
- Added border-radius properties for both the title and caret buttons.
- Utilized CSS variables for easy customization.
- Ensured proper alignment between the title and caret buttons.
These modifications align well with the PR's refactoring objectives and contribute to a more polished and consistent user interface.
| .@{dropdown-item-prefix-cls} { | ||
| background-color: unset; | ||
|
|
There was a problem hiding this comment.
💡 Codebase verification
Removing background color on hover eliminates visual feedback for menu items.
The background color for non-disabled dropdown items is unset on hover without any alternative visual indicators. This can reduce the clarity of interactions for users. Consider adding another form of visual feedback to ensure a clear and intuitive user experience.
🔗 Analysis chain
Verify hover state visibility after background color removal.
Setting the background color to 'unset' on hover for non-disabled dropdown items removes the visual feedback users might expect when interacting with the menu. While this change aligns with the refactoring goals, please ensure that there's still sufficient visual indication when users hover over menu items.
To verify the hover state visibility, you can run the following command to check if there are any other hover effects applied to these elements:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for other hover effects on dropdown items
rg -i '(hover|:hover)' packages/theme/src/action-menu/
Length of output: 134
Script:
#!/bin/bash
# Description: Extract styles applied to &:not(.is-disabled):hover
rg -A 5 '^&:not\(.is-disabled\):hover\s*{' packages/theme/src/action-menu/index.less
Length of output: 214
Script:
#!/bin/bash
# Description: Extract styles applied to &:not(.is-disabled):hover with corrected regex
rg -A 5 '^&:not\(.is-disabled\):hover\s*\{' packages/theme/src/action-menu/index.less
Length of output: 87
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
Chores