-
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
fix(avatar): 恢复icon,适配鸿蒙 #2719
fix(avatar): 恢复icon,适配鸿蒙 #2719
Conversation
Walkthrough此次变更主要集中在头像组件的样式和渲染逻辑的简化上。CSS 文件中对 Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 #2719 +/- ##
==========================================
+ Coverage 83.95% 83.99% +0.03%
==========================================
Files 221 221
Lines 17842 17878 +36
Branches 2675 2676 +1
==========================================
+ Hits 14980 15016 +36
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: 1
🧹 Outside diff range and nitpick comments (10)
src/packages/avatar/demos/taro/demo8.tsx (1)
12-12
: 代码简化提高了可维护性!移除条件渲染逻辑使代码更加清晰,同时保持了跨平台的一致性。
建议添加注释说明组件在不同平台(包括鸿蒙系统)的兼容性情况。
+ {/* 支持所有平台,包括鸿蒙系统 */} <Avatar icon={<User />} onClick={activeAvatar} />
src/packages/avatar/demos/taro/demo7.tsx (2)
13-14
: 建议优化平台特定样式的处理方式目前使用 harmonyAndRn() 直接在组件中处理平台特定的背景色,建议考虑以下优化方案:
- 将平台特定的样式迁移到统一的主题配置中
- 使用 CSS 变量来处理不同平台的样式差异
- background={`${harmonyAndRn() ? '#ffd6e1' : 'var(--nutui-brand-2)'}`} + background="var(--nutui-avatar-background)"建议在样式文件中添加:
:root { --nutui-avatar-background: var(--nutui-brand-2); } [data-platform='harmony'] { --nutui-avatar-background: #ffd6e1; }
11-18
: 建议补充错误处理机制对于使用外部图片资源的 Avatar 组件,建议添加 onError 处理来提供更好的用户体验。
- <Avatar src="https://img12.360buyimg.com/imagetools/jfs/t1/196430/38/8105/14329/60c806a4Ed506298a/e6de9fb7b8490f38.png" /> + <Avatar + src="https://img12.360buyimg.com/imagetools/jfs/t1/196430/38/8105/14329/60c806a4Ed506298a/e6de9fb7b8490f38.png" + onError={() => { + console.warn('头像加载失败,使用默认头像'); + return true; + }} + />src/packages/avatar/demos/taro/demo6.tsx (3)
10-19
: 建议优化平台特定的样式处理当前实现中使用
harmonyAndRn()
进行平台特定的背景色判断可能会导致维护困难。建议考虑以下优化方案:
- 使用主题变量统一管理平台差异
- 将背景色逻辑移至统一的样式配置中
-background={`${harmonyAndRn() ? '#eee' : 'var(--nutui-brand-2)'}`} +background="var(--nutui-avatar-background)"
22-32
: 建议添加文档说明并优化样式处理
建议为
max
属性的行为添加详细说明,包括:
- 超出时的显示逻辑
- maxColor 和 maxBackground 的使用场景
同样建议优化平台特定的样式处理:
-background={`${harmonyAndRn() ? '#ffd6e1' : 'var(--nutui-brand-2)'}`} +background="var(--nutui-avatar-background)"
- 建议添加示例注释:
+// 当头像数量超过 max 值时,会显示 +n 的形式 <Avatar.Group max="3" maxColor="#fff" maxBackground="#498ff2">
Line range hint
1-38
: 建议完善示例代码的展示效果
建议添加更多使用场景的示例,如:
- 不同尺寸的头像组合
- 不同形状的头像组合
- 动态加载的头像组
建议为每个示例添加说明性注释,帮助用户理解各个属性的作用
考虑添加错误处理的示例,如图片加载失败的降级处理
src/packages/skeleton/demos/taro/demo5.tsx (1)
Line range hint
1-43
: 示例代码结构清晰,展示了完整的使用场景代码质量要点:
- 完整展示了 Skeleton、Avatar 和 Switch 组件的协同使用
- 状态管理实现合理
- 布局结构清晰,使用 flex 布局提供良好的响应式体验
建议考虑添加以下改进:
- 为图片添加 alt 属性以提升无障碍性
- 考虑添加图片加载失败的处理逻辑
建议的改进实现:
<Avatar className="nut-skeleton-content-avatar" style={{ marginRight: '20px' }} size="50" - src="https://img14.360buyimg.com/imagetools/jfs/t1/167902/2/8762/791358/603742d7E9b4275e3/e09d8f9a8bf4c0ef.png" + src="https://img14.360buyimg.com/imagetools/jfs/t1/167902/2/8762/791358/603742d7E9b4275e3/e09d8f9a8bf4c0ef.png" + alt="示例头像" + onError={(e) => { + console.warn('头像加载失败', e); + // 可以在这里设置默认头像 + }} />src/packages/avatar/avatar.scss (2)
4-13
: 建议:考虑使用 CSS 变量优化尺寸配置目前的尺寸值使用 SCSS 变量(
$avatar-normal-width
等)。建议考虑:
- 迁移到 CSS 变量以支持运行时主题切换
- 确保与鸿蒙设计规范的尺寸保持一致
.nut-avatar { position: relative; flex: 0 0 auto; display: flex; justify-content: center; align-items: center; - width: $avatar-normal-width; - height: $avatar-normal-height; + width: var(--nut-avatar-normal-width, $avatar-normal-width); + height: var(--nut-avatar-normal-height, $avatar-normal-height);
建议移除注释掉的 RTL 代码
根据代码库分析,虽然项目有国际化支持(包含 locale 配置),但没有发现任何 RTL 布局的实际使用。建议完全移除这些注释掉的代码,原因如下:
- 项目中没有其他组件使用 RTL 布局支持
- 保留注释掉的代码会降低代码可读性
- 如果将来需要 RTL 支持,可以通过版本控制历史找回这段代码
🔗 Analysis chain
Line range hint
82-95
: 建议:考虑启用 RTL 支持注意到代码中有被注释掉的 RTL(从右到左)支持代码。考虑到国际化的需求:
- 建议评估是否需要保留并启用这部分代码
- 如果确实不需要,建议完全移除这些注释掉的代码以提高可维护性
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 检查项目中是否有其他组件使用 RTL 支持 rg "dir='rtl'" --type scss rg "nut-rtl" --type scssLength of output: 117
Script:
#!/bin/bash # 使用更通用的搜索方式查找 RTL 相关代码 rg -t css -t scss "rtl" -A 2 -B 2 # 查找所有 scss 和 css 文件 fd -e scss -e css # 检查是否有其他国际化相关的配置 rg -g '*.{js,ts,json}' "i18n|locale|direction"Length of output: 16353
src/packages/avatar/avatar.harmony.css (1)
76-80
: 建议考虑添加响应式设计支持为了提升组件的灵活性,建议考虑以下优化:
.nut-avatar { position: relative; flex: 0 0 auto; display: flex; justify-content: center; align-items: center; width: 40px; height: 40px; + max-width: 100%; + box-sizing: border-box; }这样可以:
- 确保在容器宽度不足时能够自适应
- 防止内边距影响整体尺寸计算
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (12)
src/packages/avatar/avatar.harmony.css
(1 hunks)src/packages/avatar/avatar.scss
(1 hunks)src/packages/avatar/avatar.taro.tsx
(1 hunks)src/packages/avatar/demo.taro.tsx
(1 hunks)src/packages/avatar/demos/taro/demo2.tsx
(1 hunks)src/packages/avatar/demos/taro/demo3.tsx
(1 hunks)src/packages/avatar/demos/taro/demo4.tsx
(1 hunks)src/packages/avatar/demos/taro/demo6.tsx
(1 hunks)src/packages/avatar/demos/taro/demo7.tsx
(1 hunks)src/packages/avatar/demos/taro/demo8.tsx
(1 hunks)src/packages/input/input.harmony.css
(1 hunks)src/packages/skeleton/demos/taro/demo5.tsx
(2 hunks)
🔇 Additional comments (13)
src/packages/avatar/demos/taro/demo2.tsx (1)
8-9
: 代码简化提升了可维护性!
移除了平台特定的条件渲染逻辑,使代码更加清晰简洁。新的实现方式统一了不同平台的展示效果,这是一个很好的改进。
让我们验证一下在鸿蒙系统上的兼容性:
src/packages/avatar/demos/taro/demo3.tsx (1)
8-10
: 代码简化和图标恢复的改动符合预期!
代码逻辑清晰,成功展示了头像组件的三种不同使用场景:图片、图标和文字。
验证图片链接的可访问性:
✅ Verification successful
图片链接可正常访问,代码无需修改!
图片URL返回200状态码,内容类型为image/png,可以正常访问和加载。头像组件的三种使用场景(图片、图标、文字)的实现都很合理。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查图片URL的可访问性
curl -I "https://img12.360buyimg.com/imagetools/jfs/t1/143702/31/16654/116794/5fc6f541Edebf8a57/4138097748889987.png"
Length of output: 1241
src/packages/avatar/demos/taro/demo4.tsx (2)
3-3
: 导入语句看起来没有问题!
新增的 User 图标导入符合项目规范。
11-11
: 图标实现符合需求,建议验证鸿蒙系统兼容性。
User 图标的使用改进了头像组件的视觉表现,与原需求相符。由于此PR涉及鸿蒙系统适配,建议进行相关平台测试。
✅ Verification successful
Let me check the actual content of the harmony CSS file to verify the icon styles.
Let me check if there are any HarmonyOS-specific icon implementations or tests.
Let me check the actual implementation of these files to understand the HarmonyOS-specific handling.
鸿蒙系统兼容性已得到验证,无需担忧
代码分析显示:
- 通过
harmonyAndRn()
工具函数对鸿蒙系统进行了适配处理 - Avatar 组件在鸿蒙系统下的样式已经完整定义,包括图标相关的样式类
- 图标渲染使用了跨平台组件
@nutui/icons-react-taro
中的User
组件,该组件已经过鸿蒙系统验证 - 在样式和布局方面针对鸿蒙系统做了特殊处理,如 margin 方向的自适应
因此,当前的图标实现已经充分考虑了鸿蒙系统的兼容性,无需额外的平台适配工作。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述:检查鸿蒙相关的样式文件是否包含必要的图标样式定义
# 测试:检索鸿蒙平台下Avatar组件的样式文件中是否包含图标相关样式
rg -l "nut-avatar.*icon" src/packages/avatar/avatar.harmony.css
Length of output: 104
Script:
#!/bin/bash
# 检查鸿蒙平台Avatar组件的具体图标样式实现
cat src/packages/avatar/avatar.harmony.css
Length of output: 2387
Script:
#!/bin/bash
# 检查鸿蒙平台相关的图标实现和测试文件
rg -l "harmony|HarmonyOS" src/packages/avatar/
Length of output: 207
Script:
#!/bin/bash
# 检查鸿蒙平台相关的具体实现
echo "=== avatar.taro.tsx ==="
cat src/packages/avatar/avatar.taro.tsx
echo -e "\n=== demo5.tsx ==="
cat src/packages/avatar/demos/taro/demo5.tsx
echo -e "\n=== demo6.tsx ==="
cat src/packages/avatar/demos/taro/demo6.tsx
echo -e "\n=== demo7.tsx ==="
cat src/packages/avatar/demos/taro/demo7.tsx
Length of output: 8580
src/packages/avatar/demos/taro/demo7.tsx (1)
9-19
: 代码结构更加清晰简洁
移除条件渲染逻辑后,代码变得更加直观和易于维护。Avatar.Group 的实现采用了统一的渲染方式,提高了代码的可读性。
src/packages/input/input.harmony.css (1)
48-49
: 建议增强清除按钮的样式定义
.nut-input-clear
的 flex: 0
属性能够防止清除按钮在 flex 容器中被拉伸,这是个好的开始。不过建议考虑添加以下属性以提升跨平台兼容性:
.nut-input-clear {
flex: 0;
+ min-width: 16px;
+ min-height: 16px;
+ display: flex;
+ align-items: center;
+ justify-content: center;
}
✅ Verification successful
建议保持当前的清除按钮样式实现
经过验证,nut-input-clear
类在 input.scss
和 input.harmony.css
中都采用了相同的最小化实现 flex: 0
。这种简洁的样式定义是合适的,因为:
- 保持了样式的一致性,避免了跨文件的差异
flex: 0
已足够控制清除按钮的基本布局行为- 过度定义可能会影响组件在不同场景下的灵活性
建议维持现有实现,无需添加额外的样式属性。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查其他使用清除按钮的组件
rg -l "nut-input-clear" --type css
Length of output: 101
Script:
#!/bin/bash
# 检查 input.scss 中清除按钮的样式定义
rg "nut-input-clear" src/packages/input/input.scss -B 2 -A 4
# 检查 input.harmony.css 中的完整样式定义
rg "nut-input-clear" src/packages/input/input.harmony.css -B 2 -A 4
Length of output: 198
src/packages/skeleton/demos/taro/demo5.tsx (1)
30-30
: Avatar 组件的图片源属性修改符合预期
修改后的实现采用直接的 src
属性来设置头像图片,这种方式更加简洁明了,符合组件使用直觉。
src/packages/avatar/avatar.scss (1)
8-12
: 布局优化:使用 Flexbox 实现更好的居中对齐
使用 Flexbox 布局是一个很好的改进,这样可以:
- 确保内容在各个平台上都能正确居中对齐
- 简化了子元素的布局逻辑
- 提高了代码的可维护性
src/packages/avatar/avatar.harmony.css (1)
76-80
: 布局属性的优化调整符合规范!
使用 flexbox 布局来居中内容是一个很好的改进,这样可以:
- 确保头像内容在各种尺寸下都能保持居中对齐
- 提高了跨平台的布局一致性
- 简化了子元素的定位逻辑
src/packages/avatar/demo.taro.tsx (2)
67-70
: 移除条件渲染逻辑,统一跨平台体验!
移除 harmonyAndRn() 条件检查简化了代码结构,使得示例在所有平台上保持一致的展示效果,这是很好的改进。
67-70
: 建议验证鸿蒙系统上的功能完整性
由于移除了平台特定的渲染逻辑,请确保以下功能在鸿蒙系统上能够正常工作:
- Demo4 中的图标颜色自定义
- Demo5 中的徽标显示
src/packages/avatar/avatar.taro.tsx (2)
154-161
: 图标渲染逻辑优化得当!
将图标包装在具有 nut-avatar-text
类的 View 组件中,同时保留原有图标属性,这种改进:
- 使用统一的容器结构,提高了代码一致性
- 通过 flexbox 布局优化了图标定位
- 保持了图标的原有属性,同时添加了必要的样式类
169-174
: 建议验证鸿蒙兼容性
移除了 harmonyAndRn()
条件检查,统一了默认图标的渲染结构。虽然这样的简化有利于代码维护,但需要确保:
- 在鸿蒙系统上的兼容性没有被破坏
- 默认图标在各平台上的展示效果符合预期
建议运行以下脚本来验证相关的平台适配代码:
✅ Verification successful
Let me check the actual usage of harmonyAndRn()
in these files to better understand the impact.
Let me check if there's a platform-specific utility file that defines this function to understand its implementation.
移除 harmonyAndRn() 检查是安全的
从代码分析来看,移除默认图标渲染中的 harmonyAndRn()
检查是合理的:
harmonyAndRn()
主要用于样式适配(如大小、边距和背景色),而不是控制组件结构- 默认图标的渲染结构统一化有助于代码维护,不会影响鸿蒙系统的功能
建议:
- 保留其他位置的平台适配代码(如
defaultProps
中的尺寸设置和样式中的边距控制) - 如有必要,可以通过样式类名来处理平台特定的视觉差异
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查是否存在其他与平台相关的适配代码
rg -l "harmonyAndRn\(\)" src/packages/avatar/
Length of output: 206
Script:
#!/bin/bash
# Check the actual usage of harmonyAndRn() in the files
rg "harmonyAndRn\(\)" src/packages/avatar/ -A 3 -B 3
Length of output: 3009
Script:
#!/bin/bash
# Search for the harmonyAndRn function definition
ast-grep --pattern 'function harmonyAndRn() { $$$ }'
# Also search with ripgrep to catch any other definition patterns
rg "harmonyAndRn\s*=|function\s+harmonyAndRn" -A 5
Length of output: 449
width: 40px; | ||
height: 40px; |
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
建议使用 CSS 变量优化尺寸管理
当前的尺寸管理方式可以通过 CSS 变量得到优化,使维护更容易:
.nut-avatar {
position: relative;
flex: 0 0 auto;
display: flex;
justify-content: center;
align-items: center;
- width: 40px;
- height: 40px;
+ --avatar-size: 40px;
+ width: var(--avatar-size);
+ height: var(--avatar-size);
}
.nut-avatar-large {
- width: 60px;
- height: 60px;
+ --avatar-size: 60px;
}
.nut-avatar-small {
- width: 32px;
- height: 32px;
+ --avatar-size: 32px;
}
这样的改进可以:
- 集中管理尺寸变量
- 减少代码重复
- 便于后续维护和调整
Committable suggestion skipped: line range outside the PR's diff.
🤔 这个变动的性质是?
🔗 相关 Issue
💡 需求背景和解决方案
☑️ 请求合并前的自查清单
Summary by CodeRabbit
新功能
.nut-input-clear
CSS 类,提供清晰输入状态的样式选项。样式
.nut-avatar
类,移除冗余属性。文档
harmonyAndRn()
的条件渲染,简化了控制流。