-
Notifications
You must be signed in to change notification settings - Fork 260
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(progress): fix style & support icon demo #2729
base: V3.0
Are you sure you want to change the base?
Conversation
Walkthrough本次更改涉及多个组件的修改,主要集中在 Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 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 #2729 +/- ##
=======================================
Coverage 83.98% 83.99%
=======================================
Files 221 221
Lines 17867 17878 +11
Branches 2676 2676
=======================================
+ Hits 15005 15016 +11
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 (3)
src/packages/progress/demos/taro/demo4.tsx (1)
10-11
: 优化图片尺寸的响应式处理将固定像素值改为使用 pxTransform 是一个很好的改进,这样可以:
- 确保在不同设备上保持适当的显示比例
- 提高组件的跨平台兼容性
- 符合 Taro 的最佳实践
建议考虑将这些转换后的值定义为常量,以提高代码的可维护性。
const Demo4 = () => { + const ICON_SIZE = pxTransform(20) return ( <Cell> <Progress percent={60} showText> <Image - width={pxTransform(20)} - height={pxTransform(20)} + width={ICON_SIZE} + height={ICON_SIZE} src="https://img12.360buyimg.com/imagetools/jfs/t1/229362/18/22746/11607/669f8bfeF5fcbeaab/d4162bbf29bb1b00.png" /> </Progress> </Cell> ) }src/packages/progress/demos/taro/demo6.tsx (1)
8-12
: 建议优化 iconStyle 对象的声明方式虽然当前实现可以正常工作,但建议考虑以下优化:
- 将 iconStyle 提升到组件外部以避免重复创建
- 考虑使用 useMemo 来优化性能
建议修改如下:
-const Demo6 = () => { - const iconStyle = { - marginTop: 0, - marginLeft: pxTransform(5), - marginRight: pxTransform(5), - } +const iconStyle = { + marginTop: 0, + marginLeft: pxTransform(5), + marginRight: pxTransform(5), +} + +const Demo6 = () => {src/packages/progress/progress.taro.tsx (1)
163-163
: 文本定位逻辑优化将动态百分比值改为固定值 '100%' 是一个更好的实现方式,因为:
- 配合 CSS transform 可以更准确地实现文本居中
- 简化了 RTL 和 LTR 布局的处理逻辑
- 提高了跨平台兼容性
建议添加注释说明这个固定值配合 CSS transform 使用的原因:
getTextStyle = () => { + // 使用固定的 100% 配合 CSS transform 实现文本居中,确保在 RTL/LTR 布局下都能正确定位 return rtl ? { right: '100%' } : { left: '100%' } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
packages/nutui-taro-demo/src/app.config.ts
(5 hunks)src/packages/progress/demo.taro.tsx
(1 hunks)src/packages/progress/demos/taro/demo4.tsx
(1 hunks)src/packages/progress/demos/taro/demo6.tsx
(2 hunks)src/packages/progress/progress.scss
(1 hunks)src/packages/progress/progress.taro.tsx
(1 hunks)
🔇 Additional comments (7)
src/packages/progress/demos/taro/demo4.tsx (1)
3-3
: 导入 pxTransform 工具函数用于响应式单位转换
引入 pxTransform 工具函数是一个很好的改进,这有助于确保组件在不同设备上具有一致的显示效果。
src/packages/progress/demos/taro/demo6.tsx (2)
5-5
: 导入 pxTransform 工具函数以统一单位转换
引入 pxTransform 工具函数有助于确保在不同平台上保持一致的单位转换逻辑。
28-28
: 图标样式统一化处理得当
使用统一的 iconStyle 对象来处理 Checked 和 Tips 图标的样式是个很好的改进,这样可以:
- 确保样式的一致性
- 便于后续的维护和修改
Also applies to: 32-32
src/packages/progress/progress.scss (1)
42-42
: 文本居中定位属性修复已确认!
启用 transform: translate(-50%)
属性是正确的做法,这确保了进度条文本能够在水平方向上精确居中,与绝对定位配合使用时能够避免文本偏移问题。
让我们验证在不同屏幕尺寸下的显示效果:
✅ Verification successful
文本居中定位属性验证完成,RTL支持正确!
经过代码分析,transform: translate(-50%)
属性的使用是正确的,并且已经正确处理了RTL布局的支持:
- LTR布局下使用
transform: translate(-50%)
实现水平居中 - RTL布局下使用
transform: translate(50%)
实现水平居中 - 相关样式已经在
.nut-rtl
和[dir=rtl]
选择器中正确配置
这种实现方式确保了进度条文本在不同语言方向下都能保持正确的居中定位。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述:检查是否存在与进度条文本定位相关的响应式样式
# 预期:确认文本居中样式在各种屏幕尺寸下的一致性
# 搜索所有与进度条相关的媒体查询
rg -l "@media.*nut-progress"
# 检查是否有其他地方覆盖了文本定位样式
rg "transform:.*translate.*nut-progress-text"
Length of output: 78
Script:
#!/bin/bash
# 检查进度条组件的所有样式文件
fd -e scss -e css progress
# 检查所有包含 transform 或 translate 的样式规则
rg -A 2 -B 2 "transform|translate" --type css
# 检查所有媒体查询相关样式
rg "@media" --type css -A 3 -B 3
# 检查RTL相关样式
rg "rtl|direction" --type css -A 3 -B 3
Length of output: 373544
src/packages/progress/demo.taro.tsx (1)
59-60
: 代码逻辑简化,但需要验证平台兼容性
移除条件渲染使代码结构更加清晰,但建议确认 Demo4 在所有目标平台上都能正常工作。
运行以下脚本以验证 Demo4 的平台兼容性:
✅ Verification successful
移除条件渲染是安全的,代码验证通过
Demo4 组件使用了跨平台的 @nutui/nutui-react-taro
组件库和通用的 pxTransform
工具函数,没有使用任何平台特定的 API 或组件。移除条件渲染不会影响其在不同平台上的兼容性。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述:检查 Demo4 中是否存在特定平台的 API 或组件使用
# 测试:搜索 Demo4 实现中的平台特定代码
ast-grep --pattern 'import { $_ } from "@tarojs/components"' src/packages/progress/demos/taro/demo4.tsx
# 测试:检查是否有平台特定的条件判断
rg -A 3 'process\.env\.TARO_ENV' src/packages/progress/demos/taro/demo4.tsx
Length of output: 183
Script:
#!/bin/bash
# 首先检查 Demo4 文件是否存在
fd "demo4\.tsx" src/packages/progress
# 如果文件不在预期位置,尝试其他可能的位置
fd "demo4\.(tsx|jsx)" src/packages
# 检查 demo.taro.tsx 中 Demo4 的导入
rg "import.*Demo4" src/packages/progress/demo.taro.tsx -A 2
# 查看 demo.taro.tsx 的完整内容以了解上下文
cat src/packages/progress/demo.taro.tsx
Length of output: 8578
Script:
#!/bin/bash
# 检查 Demo4 的具体实现
cat src/packages/progress/demos/taro/demo4.tsx
# 检查 Demo4 是否使用了平台特定的 API
rg "process\.env\.TARO_ENV|@tarojs/(components|api)" src/packages/progress/demos/taro/demo4.tsx
# 检查是否有其他平台特定的导入
rg "import.*from.*@tarojs" src/packages/progress/demos/taro/demo4.tsx
Length of output: 714
packages/nutui-taro-demo/src/app.config.ts (2)
124-133
: 配置结构优化合理
应用配置的改动保持了良好的一致性,window 配置的属性值符合规范。
51-52
: 请确认路由命名变更的影响范围
将子包的 root 从 "dataentry" 更改为 "dentry1" 可能会影响现有的路由跳转逻辑。建议:
- 确认是否需要更新相关文档
- 检查是否有依赖原路径的代码需要同步修改
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.
关注下 js 中截的图
}, | ||
{ | ||
"root": "dataentry", | ||
"pages": [ | ||
root: "dentry1", |
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.
这个不大对~
}, | ||
components: ['pages/index/index', ...(subPackages.map(subPackage => { |
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.
这个看一下~
🤔 这个变动的性质是?
🔗 相关 Issue
💡 需求背景和解决方案
☑️ 请求合并前的自查清单
Summary by CodeRabbit
Demo4
组件现在无条件渲染,提升了可见性。Demo6
组件简化了Checked
和Tips
组件的渲染逻辑。.nut-progress-text
类的 CSS 属性,以确保文本在进度条上居中显示。Progress
组件的文本位置样式,使其在不同平台上保持一致。