-
Notifications
You must be signed in to change notification settings - Fork 291
feat(price): dynamic和harmony结构保持一致 #3401
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
base: feat_v3.x
Are you sure you want to change the base?
Conversation
概览在条件渲染逻辑中添加环境变量检查:修改了 变更
预估代码审查工作量🎯 2 (简单) | ⏱️ ~8 分钟
建议的审查者
诗句
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## feat_v3.x #3401 +/- ##
==========================================
Coverage 88.15% 88.15%
==========================================
Files 291 291
Lines 19212 19212
Branches 2988 2988
==========================================
Hits 16937 16937
Misses 2269 2269
Partials 6 6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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
🧹 Nitpick comments (1)
src/packages/price/price.taro.tsx (1)
169-183: 动态环境与 Harmony 同步使用 Text 包裹逻辑合理,整体实现 LGTM这行条件从单纯
harmony()扩展为harmony() || process.env.TARO_ENV === 'dynamic',让 dynamic 环境也走 Text 包裹分支,与 Harmony 保持一致,符合“结构保持一致”的目标,目前看不会引入明显副作用。可以考虑的小优化(可选):
- 为了统一平台判断逻辑,后续可以在
@/utils/taro/platform里抽一个类似isHarmonyLike()/isHarmonyOrDynamic()的工具方法,在各组件里只调用这个工具,避免到处手写'dynamic'字面量,减轻后续再扩展环境时的维护成本。请你再确认一处:当前 Taro 配置里 dynamic 环境的标识是否就是严格等于字符串
'dynamic',以及是否还有其他组件也需要同步从harmony()扩展为 “Harmony + dynamic” 的判断,避免不同组件在相同平台下行为不一致。Based on learnings, ...
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/packages/price/price.taro.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-11-06T02:16:07.069Z
Learnt from: oasis-cloud
Repo: jdf2e/nutui-react PR: 2540
File: src/packages/trendarrow/trendarrow.taro.tsx:29-30
Timestamp: 2024-11-06T02:16:07.069Z
Learning: 在`TrendArrow`组件(`src/packages/trendarrow/trendarrow.taro.tsx`)中,CSS变量在鸿蒙系统(Harmony OS)中可能未被完全支持,导致兼容性问题。因此,使用硬编码的十六进制颜色值可以确保在鸿蒙系统上的正确显示。
Applied to files:
src/packages/price/price.taro.tsx
🧬 Code graph analysis (1)
src/packages/price/price.taro.tsx (1)
src/utils/taro/platform.ts (1)
harmony(3-7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: test
🤔 这个变动的性质是?
🔗 相关 Issue
💡 需求背景和解决方案
☑️ 请求合并前的自查清单
Summary by CodeRabbit
发布说明
✏️ Tip: You can customize this high-level summary in your review settings.