-
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(numberkeyboard): harmony cpp adaption #2799
base: feat_v3.x
Are you sure you want to change the base?
Conversation
Walkthrough本次更改主要涉及多个组件的版本更新,所有相关组件的版本均已从 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant NumberKeyboard
participant View
User->>NumberKeyboard: 触发键盘显示
NumberKeyboard->>View: 渲染键盘视图
View->>User: 显示键盘
User->>NumberKeyboard: 输入数字
NumberKeyboard->>User: 返回输入结果
Possibly related PRs
Suggested labels
Suggested reviewers
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 @@
## feat_v3.x #2799 +/- ##
=============================================
+ Coverage 83.98% 83.99% +0.01%
=============================================
Files 221 220 -1
Lines 17879 17885 +6
Branches 2676 2679 +3
=============================================
+ Hits 15016 15023 +7
+ Misses 2858 2857 -1
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: 2
🧹 Outside diff range and nitpick comments (3)
src/packages/numberkeyboard/numberkeyboard.scss (2)
29-30
: 关闭按钮垂直居中定位的实现方式可以优化当前实现使用
top: 50%
和transform: translateY(-50%)
实现垂直居中是可行的,但考虑到性能优化,建议使用 Flexbox 方案。建议修改如下:
.nut-numberkeyboard-header { - position: relative; + position: relative; display: flex; align-items: center; &-close { position: absolute; right: 0; - top: 50%; - transform: translateY(-50%); + height: 100%; + display: flex; + align-items: center; } }
53-58
: 建议为条件编译添加说明注释当前代码使用条件编译来处理不同平台的样式差异,建议添加注释说明 harmony 适配的具体原因和场景。
建议添加如下注释:
+ /* Harmony 平台下使用固定宽度以确保更好的布局稳定性 */ /* #ifdef harmony */ width: 33%; /* #endif */ + /* 非 Harmony 平台使用 flex-basis 实现相同效果 */ /* #ifndef harmony */ flex-basis: 33%; /* #endif */src/packages/numberkeyboard/numberkeyboard.taro.tsx (1)
9-10
: 优化了属性处理和工具函数的使用引入了
pxTransform
和mergeProps
工具函数,并使用mergeProps
替代了之前的展开运算符方式来合并默认属性,这样的改动更加规范和安全。建议添加对
mergeProps
的类型检查,确保类型安全:- } = mergeProps(defaultProps, props) + } = mergeProps<NumberKeyboardProps>(defaultProps, props)Also applies to: 54-54
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
src/config.json
(1 hunks)src/packages/numberkeyboard/demos/taro/demo4.tsx
(2 hunks)src/packages/numberkeyboard/numberkeyboard.scss
(3 hunks)src/packages/numberkeyboard/numberkeyboard.taro.tsx
(5 hunks)
🔇 Additional comments (7)
src/packages/numberkeyboard/demos/taro/demo4.tsx (1)
3-3
: 导入语句添加正确!
从 @tarojs/components
导入 View
组件符合 Taro 的最佳实践。
src/packages/numberkeyboard/numberkeyboard.scss (3)
22-22
: 字体大小继承设置正确!
从 popup 组件继承字体大小设置,保持了组件间的视觉一致性。
87-89
: 侧边栏布局适配设置合理
为 harmony 平台添加的 flex-basis 设置与主体部分保持一致,确保了布局的统一性。
92-92
: 侧边栏容器宽度设置正确
设置 width: 100%
确保了侧边栏容器能够正确填充其父容器空间,对于维持布局稳定性很重要。
src/packages/numberkeyboard/numberkeyboard.taro.tsx (2)
91-92
: 使用 pxTransform 优化了样式单位转换
将固定像素值改为使用 pxTransform
进行转换,这样能更好地适配不同设备的屏幕尺寸。
127-127
: 简化了数字键盘按键的渲染逻辑
使用 Fragment 替代了 View 组件,减少了不必要的 DOM 层级。
src/config.json (1)
Line range hint 569-577
: 版本更新看起来符合预期!
NumberKeyboard 组件从 2.0.0 升级到 3.0.0 的版本更新与其他组件的升级模式一致。
运行以下脚本来验证相关文件中的版本一致性:
@@ -15,7 +16,7 @@ const Demo4 = () => { | |||
<NumberKeyboard | |||
visible={visible} | |||
title="标题" | |||
rightActions={<span>Done</span>} | |||
rightActions={<View>Done</View>} |
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.
💡 Codebase verification
需要更新 H5 版本的 demo4 示例代码
经过验证,发现 H5 版本的 demo4 中仍在使用 <span>Done</span>
,而 Taro 版本已正确使用 <View>Done</View>
。建议:
- 更新 src/packages/numberkeyboard/demos/h5/demo4.tsx 中的代码,将
<span>Done</span>
改为<div>Done</div>
- 由于这只是示例代码的改动,不涉及组件 API 变更,无需更新升级指南文档
🔗 Analysis chain
请确认版本升级文档已更新
将 span
替换为 View
是正确的改动,符合 Taro 组件的使用规范。建议确认以下几点:
- 版本升级指南中是否已说明此处 API 的变更
- 相关的示例代码是否都已更新
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查版本升级指南和其他示例代码
# 查找版本升级指南
echo "检查版本升级指南..."
rg -l "2.0.0.*3.0.0|NumberKeyboard.*升级|NumberKeyboard.*migration" --type md
# 检查其他示例代码中是否还有使用 span 的地方
echo "检查其他示例代码..."
rg -l "rightActions.*span" --type tsx src/packages/numberkeyboard/demos/
Length of output: 424
Script:
#!/bin/bash
# 检查版本升级指南和其他示例代码
# 查找版本升级指南
echo "检查版本升级指南..."
rg -l "2.0.0.*3.0.0|NumberKeyboard.*升级|NumberKeyboard.*migration" .
# 检查其他示例代码中是否还有使用 span 的地方
echo "检查其他示例代码..."
fd . src/packages/numberkeyboard/demos/ -e ts -e tsx -e jsx -x rg -l "rightActions.*span" {}
Length of output: 861
Script:
#!/bin/bash
# 检查 h5 demo4 中的具体代码
echo "检查 h5 demo4 的代码内容..."
cat src/packages/numberkeyboard/demos/h5/demo4.tsx
# 检查是否存在升级指南文档
echo "检查可能的升级指南文档..."
fd . -e md -x rg -l "NumberKeyboard|数字键盘|组件升级" {}
# 检查其他可能的 demo 文件
echo "检查其他 demo 文件..."
fd . src/packages/numberkeyboard/demos/ -e ts -e tsx -e jsx
Length of output: 1877
Script:
#!/bin/bash
# 检查升级指南文档内容
echo "检查升级指南文档内容..."
cat migrate-from-v2.md
# 检查文档中的相关说明
echo "检查组件文档..."
cat src/packages/numberkeyboard/doc.md
# 检查其他 demo4 的实现
echo "检查 demo4 的其他实现..."
cat src/packages/numberkeyboard/demos/taro/demo4.tsx
Length of output: 29203
<View | ||
className={`${classPrefix}-header-close`} | ||
onClick={onConfirm} | ||
> | ||
{rightActions || locale.done} | ||
</span> | ||
</View> |
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.
💡 Codebase verification
需要统一更新其他文件中的 span 元素
为保持组件的一致性,建议同步更新以下位置的代码:
src/packages/numberkeyboard/numberkeyboard.tsx
: 将 header-close 中的span
元素替换为View
组件src/packages/numberkeyboard/demos/h5/demo4.tsx
: 将 rightActions 中的span
元素替换为View
组件
🔗 Analysis chain
改进了右侧操作区域的结构
将 span 元素替换为 View 组件,保持了组件的一致性,更符合 Taro 的开发规范。
让我们验证一下这个改动是否在其他地方也保持一致:
Let's examine the specific usage of span
in these files to understand the context better.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查是否还有其他使用 span 的地方需要更新
rg -l 'span' src/packages/numberkeyboard/
Length of output: 135
Script:
#!/bin/bash
# Check the context of span usage in these files
echo "=== numberkeyboard.tsx ==="
rg -C 3 'span' src/packages/numberkeyboard/numberkeyboard.tsx
echo -e "\n=== demo4.tsx ==="
rg -C 3 'span' src/packages/numberkeyboard/demos/h5/demo4.tsx
Length of output: 827
🤔 这个变动的性质是?
🔗 相关 Issue
💡 需求背景和解决方案
☑️ 请求合并前的自查清单
Summary by CodeRabbit