-
Notifications
You must be signed in to change notification settings - Fork 263
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
feat(button): update icon #2709
Conversation
Walkthrough该拉取请求对 Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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 #2709 +/- ##
=======================================
Coverage 83.95% 83.95%
=======================================
Files 221 221
Lines 17842 17842
Branches 2675 2675
=======================================
Hits 14980 14980
Misses 2857 2857
Partials 5 5 ☔ View full report in Codecov by Sentry. |
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: 0
🧹 Outside diff range and nitpick comments (5)
src/packages/button/demos/taro/demo4.tsx (4)
3-3
: 导入语句优化建议建议将图标导入语句与其他组件导入分开,使代码结构更清晰。
import React from 'react' import { Button, Cell } from '@nutui/nutui-react-taro' + import { Plus, Star } from '@nutui/icons-react-taro' import pxTransform from '@/utils/px-transform'
15-89
: 建议优化按钮样式的复用性当前代码中存在多处重复的样式定义。建议将共同的样式提取为统一的样式对象或使用统一的样式类。
建议重构为:
const buttonStyles = { base: { margin: pxTransform(8), }, primary: { backgroundColor: 'var(--nutui-color-primary-light-pressed)', borderColor: 'var(--nutui-color-primary)', color: 'var(--nutui-color-primary)', }, gray: { backgroundColor: 'var(--nutui-gray-3)', color: 'var(--nutui-gray-7)', } }然后在按钮中使用:
<Button type="primary" icon={<Star />} rightIcon={<Star />} style={{ ...buttonStyles.base, ...buttonStyles.primary }} > Button </Button>
90-108
: 图标按钮实现规范图标按钮的实现符合设计规范,使用了合适的属性组合。建议为图标按钮添加
aria-label
属性以提升可访问性。<Button shape="square" fill="outline" type="primary" icon={<Plus />} + aria-label="添加" style={marginStyle} />
109-128
: 大尺寸按钮样式建议大尺寸按钮的实现合理。不过建议考虑在移动端场景下,对于
xlarge
尺寸的按钮添加响应式处理,以确保在小屏幕设备上的展示效果。建议添加媒体查询样式:
const responsiveStyle = { ...marginStyle, '@media screen and (max-width: 375px)': { width: '100%', marginLeft: 0, marginRight: 0, }, }src/packages/button/demos/taro/demo5.tsx (1)
15-135
: 建议重构按钮展示逻辑,提高代码可维护性当前代码虽然功能正确,但存在大量重复的按钮配置代码。建议将按钮配置抽象为数据配置,通过映射渲染来减少代码重复。
建议按照以下方式重构:
const Demo5 = () => { const marginStyle = { marginRight: pxTransform(8), marginTop: pxTransform(8), marginLeft: pxTransform(8), marginBottom: pxTransform(8), } + const buttonConfigs = [ + { + type: 'primary', + icon: <Star />, + rightIcon: <Star />, + children: 'Disabled', + }, + { + type: 'primary', + fill: 'outline', + icon: <Star />, + rightIcon: <Star />, + children: 'Disabled', + }, + // ... 其他按钮配置 + ] return ( <> <Cell style={{ flexWrap: 'wrap' }}> + {buttonConfigs.map((config, index) => ( + <Button + key={index} + disabled + style={marginStyle} + {...config} + /> + ))} </Cell> </> ) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
src/packages/button/button.taro.tsx
(1 hunks)src/packages/button/demos/taro/demo4.tsx
(2 hunks)src/packages/button/demos/taro/demo5.tsx
(2 hunks)
🔇 Additional comments (3)
src/packages/button/demos/taro/demo5.tsx (2)
Line range hint 1-14
: 代码结构清晰,导入语句组织合理!
代码结构和样式定义符合最佳实践。
15-135
: 验证平台相关代码的移除情况
需要确认移除 harmonyAndRn() 条件渲染后的跨平台兼容性。
✅ Verification successful
Let me generate additional verification to check the button component's implementation:
平台相关代码检查结果
经过验证,harmonyAndRn()
是一个全局平台检测工具函数,用于判断当前运行环境是否为 RN/Harmony。在按钮组件中的使用是合理的,主要用于:
- 在 RN/Harmony 环境下应用特定样式
- 处理不同平台的样式兼容性
该函数的实现保留在 src/utils/platform-taro.ts
中是必要的,因为它仍被多个组件使用,用于跨平台适配。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 搜索是否还存在其他使用 harmonyAndRn 的地方
rg "harmonyAndRn" --type ts
Length of output: 13664
Script:
#!/bin/bash
# Check button component implementation
ast-grep --pattern 'const harmonyAndRn = () => {
$$$
}'
# Check button.taro.tsx implementation
rg -A 10 "harmonyAndRn" src/packages/button/button.taro.tsx
Length of output: 1131
src/packages/button/button.taro.tsx (1)
Line range hint 161-174
: 简化了加载状态的渲染逻辑,提高了代码可维护性
代码变更移除了基于平台的条件渲染,使得加载状态的处理更加直观和统一。这种改进:
- 提高了代码的可维护性
- 确保了跨平台的一致性
- 简化了组件的实现逻辑
让我们验证这个改动在不同平台上的影响:
✅ Verification successful
加载状态渲染逻辑的简化是安全且合理的
基于代码分析结果,我们可以确认:
harmonyAndRn()
在多个组件中使用,但主要用于样式控制而非加载状态逻辑- 加载图标
nut-icon-loading
的实现在其他组件(如 Uploader、Toast、Image)中保持一致 - Button 组件在不同平台(Taro 和 Web)的实现中都采用了相同的加载状态渲染逻辑
这次改动:
- 统一了跨平台的实现方式
- 保持了与其他组件的一致性
- 不会对依赖
harmonyAndRn()
的其他功能造成影响
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述:检查是否存在其他依赖于 harmonyAndRn() 的按钮加载逻辑
# 测试:搜索可能受影响的相关代码
rg -l "harmonyAndRn.*loading"
# 测试:检查按钮组件的测试用例
fd -g "*button*.test.*"
Length of output: 57
Script:
#!/bin/bash
# 让我们使用不同的方式来验证这些更改
# 1. 检查所有包含 harmonyAndRn 的文件
rg -l "harmonyAndRn"
# 2. 检查按钮组件在其他平台的实现
fd -g "*button*.(tsx|jsx)"
# 3. 检查是否有其他组件使用了类似的加载状态逻辑
rg "loading.*className.*nut-icon-loading" -A 2
# 4. 检查按钮组件的样式文件,确保样式支持
fd -g "*button*.scss"
Length of output: 4192
🤔 这个变动的性质是?
🔗 相关 Issue
💡 需求背景和解决方案
☑️ 请求合并前的自查清单
Summary by CodeRabbit
新功能
样式