fix(button,button-group): adapting the button component to small screens#4080
fix(button,button-group): adapting the button component to small screens#4080James-9696 wants to merge 4 commits intodevfrom
Conversation
WalkthroughIntroduces a new adaptation theming system for the button and button-group components, including responsive styles optimized for narrow viewports (max-width 820px), a central index file to aggregate adaptation assets, and new CSS variables to support mobile-specific styling configurations. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/theme/src/button/adaptation.less`:
- Line 6: Replace the hardcoded selector ".tiny-svg" with the configured
variable so the CSS-prefix is respected: use the Less class interpolation
.@{svg-prefix-cls} (since `@svg-prefix-cls` is defined as ~'@{css-prefix}svg')
wherever ".tiny-svg" appears (e.g., in the selector on line with .tiny-svg) so
the selector changes with the css-prefix setting.
🧹 Nitpick comments (2)
packages/theme/src/button-group/adaptation.less (2)
25-32: Hardcodedfill:#c25700`` breaks the design-token convention.Every other color in the adaptation files is driven by a CSS variable, but this icon fill is hardcoded. If the theme is customized, this color won't adapt. Consider introducing a CSS variable (e.g.,
--tv-ButtonGroup-item-adapt-icon-fill-color) invars.lessand referencing it here.The same applies to the hardcoded
border-radius: 0 6pxon line 27—consider using a token for consistency.♻️ Proposed refactor
In
vars.less, add:--tv-ButtonGroup-item-adapt-icon-bg-color: var(--tv-color-warn-bg-light, `#ffebd1`); + --tv-ButtonGroup-item-adapt-icon-fill-color: `#c25700`;Then in this file:
&__sup-icon { background-color: var(--tv-ButtonGroup-item-adapt-icon-bg-color); - border-radius: 0 6px; - fill: `#c25700`; + border-radius: 0 var(--tv-ButtonGroup-border-radius, 6px); + fill: var(--tv-ButtonGroup-item-adapt-icon-fill-color); display: flex; align-items: center; justify-content: center; }
9-9: Unused variable@button-group-popover-prefix-cls.This LESS variable is defined but never referenced in the file. Remove it to keep the file clean.
🧹 Proposed fix
-@button-group-popover-prefix-cls: ~'@{css-prefix}group-item__more-popover';
| @import './vars.less'; | ||
|
|
||
| @button-prefix-cls: ~'@{css-prefix}button'; | ||
| @svg-prefix-cls: ~'@{css-prefix}svg'; |
There was a problem hiding this comment.
@svg-prefix-cls is defined but unused; hardcoded .tiny-svg on line 12 should use it.
The variable is declared to respect a configurable css-prefix, but line 12 bypasses it with a literal class name. If the prefix changes, this selector will silently break.
🐛 Proposed fix
&.is-icon {
- .tiny-svg {
+ .@{svg-prefix-cls} {
fill: var(--tv-Button-icon-adapt-inverse-tint);
}
}Also applies to: 12-12
🤖 Prompt for AI Agents
In `@packages/theme/src/button/adaptation.less` at line 6, Replace the hardcoded
selector ".tiny-svg" with the configured variable so the CSS-prefix is
respected: use the Less class interpolation .@{svg-prefix-cls} (since
`@svg-prefix-cls` is defined as ~'@{css-prefix}svg') wherever ".tiny-svg" appears
(e.g., in the selector on line with .tiny-svg) so the selector changes with the
css-prefix setting.
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