-
Notifications
You must be signed in to change notification settings - Fork 266
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
fix(radio): color of icon #2768
Conversation
Walkthrough该拉取请求对 Changes
Possibly related PRs
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 (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## V3.0 #2768 +/- ##
=======================================
Coverage 83.98% 83.98%
=======================================
Files 221 221
Lines 17879 17879
Branches 2676 2676
=======================================
Hits 15016 15016
Misses 2858 2858
Partials 5 5 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
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: 1
🧹 Outside diff range and nitpick comments (2)
src/packages/radiogroup/radiogroup.scss (1)
33-34
: 建议在组件文档中说明布局行为为了帮助其他开发者理解组件的行为,建议在组件文档中说明水平布局使用了 flex 布局。
src/packages/radio/radio.taro.tsx (1)
93-94
: 建议将颜色值提取为常量为了提高可维护性和一致性,建议将颜色值
#c2c4cc
提取为组件级或主题级的常量。+const DISABLED_COLOR = '#c2c4cc' + const renderIcon = () => { if (disabled && !checkedStatement) { - const colors = harmony() ? { color: '#c2c4cc' } : {} + const colors = harmony() ? { color: DISABLED_COLOR } : {} return <CheckDisabled className={classNames(color())} {...colors} /> }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/packages/radio/radio.taro.tsx
(2 hunks)src/packages/radiogroup/radiogroup.scss
(1 hunks)
🔇 Additional comments (3)
src/packages/radiogroup/radiogroup.scss (1)
33-34
: 布局更改看起来合理!
添加 display: flex
有助于改善水平布局的对齐方式。建议在不同屏幕尺寸下测试此更改,以确保布局行为符合预期。
✅ Verification successful
布局更改已验证安全
经过验证,添加 display: flex
的更改是安全的:
- 该更改仅影响
.nut-radiogroup-horizontal
的直接布局 - 现有的子元素已经使用
display: inline-flex
,与新增的 flex 布局完全兼容 - RTL(从右到左)支持也已经正确处理
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查是否有其他组件依赖于 radio 组件的布局行为
rg -l "nut-radiogroup-horizontal"
Length of output: 80
Script:
#!/bin/bash
# 检查 radiogroup.harmony.css 中的相关样式
cat src/packages/radiogroup/radiogroup.harmony.css
# 搜索其他可能使用 radiogroup 的组件
rg -l "nut-radiogroup"
# 检查是否有测试用例涉及水平布局
rg "horizontal" src/packages/radiogroup/
Length of output: 1802
src/packages/radio/radio.taro.tsx (2)
13-13
: 导入语句正确添加!
从平台工具中导入 harmony 函数用于处理不同平台的颜色样式,符合代码组织规范。
110-118
: 请验证颜色值的一致性
注意到正常状态和禁用状态使用了相同的灰色值 (#c2c4cc),请确认这是否符合设计规范。同时建议检查这些颜色值是否与设计系统中的其他组件保持一致。
✅ Verification successful
颜色值使用符合设计规范
根据搜索结果,#c2c4cc
是设计系统中定义的灰色变量 $color-text-disabled
和 $color-border-disabled
,用于禁用状态的文本和边框颜色。这个颜色值在多个组件中都有一致的使用。
理由:
- 在
src/styles/variables.scss
中明确定义了这个颜色值用于禁用状态 - Radio 组件中使用该颜色值来表示禁用状态是符合设计规范的
- 该颜色值在整个组件库中被一致使用,包括 Button、Input、Checkbox 等组件的禁用状态
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 搜索代码库中其他组件使用的颜色值,以确保一致性
rg -i '#c2c4cc|#ffadbe|#ff0f23' --type tsx --type scss
Length of output: 84
Script:
#!/bin/bash
# 搜索所有文件中的颜色值
rg -i '#c2c4cc|#ffadbe|#ff0f23'
# 搜索颜色相关的常量或变量定义
ast-grep --pattern 'const $_ = { color: $_ }'
# 搜索颜色主题或变量定义文件
fd -e ts -e tsx -e scss -e css -e less | grep -i "color\|theme\|style"
Length of output: 166663
const colors = harmony() | ||
? { color: disabled ? '#ffadbe' : '#ff0f23' } | ||
: {} | ||
return React.isValidElement(activeIcon) ? ( | ||
React.cloneElement<any>(activeIcon, { | ||
...activeIcon.props, | ||
className: classNames(color()), | ||
...colors, | ||
}) | ||
) : ( | ||
<CheckChecked className={classNames(color())} /> | ||
<CheckChecked className={classNames(color())} {...colors} /> |
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
建议优化选中状态的颜色处理逻辑
当前代码在多处重复处理颜色逻辑,建议将颜色计算逻辑提取到单独的函数中,以提高代码的可维护性和复用性。
+const getCheckedStateColors = (disabled: boolean) => {
+ if (!harmony()) return {}
+ return {
+ color: disabled ? '#ffadbe' : '#ff0f23'
+ }
+}
if (checkedStatement) {
- const colors = harmony()
- ? { color: disabled ? '#ffadbe' : '#ff0f23' }
- : {}
+ const colors = getCheckedStateColors(disabled)
return React.isValidElement(activeIcon) ? (
React.cloneElement<any>(activeIcon, {
...activeIcon.props,
className: classNames(color()),
...colors,
})
) : (
<CheckChecked className={classNames(color())} {...colors} />
)
}
Committable suggestion skipped: line range outside the PR's diff.
已支持了,无需再做。关了~ |
🤔 这个变动的性质是?
🔗 相关 Issue
💡 需求背景和解决方案
☑️ 请求合并前的自查清单
Summary by CodeRabbit
新特性
样式
display: flex;
属性,优化了布局行为。