refactor(picker): 删除3D环绕效果 #2815
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Walkthrough此拉取请求主要涉及对 Changes
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
src/packages/picker/pickerpanel.taro.tsx (2)
274-287: 建议优化渲染性能当前实现正确但可以通过以下方式进行优化：
- 考虑使用虚拟列表来优化长列表的渲染性能
- 可以将固定的样式抽离到CSS中，减少内联样式的使用
建议参考以下实现：
+const ITEM_STYLE = { + height: '36px', + lineHeight: '36px', +} {options.map((item, index) => { return ( <View className="nut-picker-roller-item-title" key={item.value ? item.value : index} - style={{ - height: `${lineSpacing.current}px`, - lineHeight: `${lineSpacing.current}px`, - }} + style={{ + ...ITEM_STYLE, + height: `${lineSpacing.current}px`, + lineHeight: `${lineSpacing.current}px`, + }} >
Line range hint
1-294: 建议加强错误处理机制为提高组件的健壮性，建议在以下几个关键位置添加错误处理：
- 触摸事件处理中添加try-catch
- 在ref获取失败时提供合理的降级处理
- 在options为空时提供友好的提示
建议在关键函数中添加错误处理，例如：
const getReference = async () => { + try { const refe = await getRectByTaro(listRef?.current) lineSpacing.current = refe.height ? refe.height : 36 + } catch (error) { + console.error('获取元素尺寸失败:', error) + lineSpacing.current = 36 + } }src/packages/picker/pickerpanel.tsx (1)
263-272: 建议增加无障碍访问支持当前实现可以通过以下方式优化无障碍访问体验：
- 为选项添加
role="option"属性- 为当前选中项添加
aria-selected="true"- 添加适当的
aria-label描述建议按如下方式修改代码：
{options.map((item, index) => { return ( <div className="nut-picker-roller-item-title" key={item.value ? item.value : index} + role="option" + aria-selected={currIndex === index + 1} + aria-label={`选项 ${item.text}`} > <>{item.text}</> </div> ) })}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (21)
src/packages/datepicker/__test__/datepicker.spec.tsx(0 hunks)src/packages/datepicker/datepicker.taro.tsx(0 hunks)src/packages/datepicker/datepicker.tsx(0 hunks)src/packages/datepicker/demos/h5/demo1.tsx(0 hunks)src/packages/datepicker/demos/taro/demo1.tsx(0 hunks)src/packages/datepicker/doc.en-US.md(0 hunks)src/packages/datepicker/doc.md(0 hunks)src/packages/datepicker/doc.taro.md(0 hunks)src/packages/datepicker/doc.zh-TW.md(0 hunks)src/packages/picker/demo.taro.tsx(0 hunks)src/packages/picker/demo.tsx(0 hunks)src/packages/picker/demos/h5/demo5.tsx(0 hunks)src/packages/picker/demos/taro/demo5.tsx(0 hunks)src/packages/picker/doc.en-US.md(0 hunks)src/packages/picker/doc.md(0 hunks)src/packages/picker/doc.taro.md(0 hunks)src/packages/picker/doc.zh-TW.md(0 hunks)src/packages/picker/picker.taro.tsx(0 hunks)src/packages/picker/picker.tsx(0 hunks)src/packages/picker/pickerpanel.taro.tsx(2 hunks)src/packages/picker/pickerpanel.tsx(2 hunks)
💤 Files with no reviewable changes (19)
- src/packages/datepicker/doc.en-US.md
- src/packages/picker/demos/taro/demo5.tsx
- src/packages/datepicker/doc.md
- src/packages/datepicker/demos/taro/demo1.tsx
- src/packages/datepicker/doc.zh-TW.md
- src/packages/datepicker/doc.taro.md
- src/packages/picker/demos/h5/demo5.tsx
- src/packages/datepicker/test/datepicker.spec.tsx
- src/packages/datepicker/demos/h5/demo1.tsx
- src/packages/picker/doc.md
- src/packages/picker/doc.taro.md
- src/packages/datepicker/datepicker.tsx
- src/packages/picker/picker.taro.tsx
- src/packages/datepicker/datepicker.taro.tsx
- src/packages/picker/picker.tsx
- src/packages/picker/doc.en-US.md
- src/packages/picker/doc.zh-TW.md
- src/packages/picker/demo.tsx
- src/packages/picker/demo.taro.tsx
🔇 Additional comments (2)
src/packages/picker/pickerpanel.taro.tsx (1)
270-270: 样式简化改动符合预期！
移除3D环绕效果后，统一使用 touchTileStyle() 来处理滚动变换，使代码更加清晰。
src/packages/picker/pickerpanel.tsx (1)
259-259: 样式更改符合预期！
移除3D效果后，统一使用 touchTileStyle() 来处理滚动动画，使代码更加简洁清晰。
Comment on lines
+259
to
+272
| style={touchTileStyle()} | ||
| onTransitionEnd={stopMomentum} | ||
| > | ||
| {/* 3D 效果 */} | ||
| {threeDimensional && | ||
| options.map((item, index) => { | ||
| return ( | ||
| <div | ||
| className={`nut-picker-roller-item ${ | ||
| isHidden(index + 1) && 'nut-picker-roller-item-hidden' | ||
| }`} | ||
| style={{ | ||
| transform: `rotate3d(1, 0, 0, ${ | ||
| -rotation * (index + 1) | ||
| }deg) translate3d(0px, 0px, 104px)`, | ||
| }} | ||
| key={item.value ? item.value : index} | ||
| > | ||
| <>{item.text}</> | ||
| </div> | ||
| ) | ||
| })} | ||
| {/* 平铺 */} | ||
| {!threeDimensional && | ||
| options.map((item, index) => { | ||
| return ( | ||
| <div | ||
| className="nut-picker-roller-item-title" | ||
| key={item.value ? item.value : index} | ||
| > | ||
| <>{item.text}</> | ||
| </div> | ||
| ) | ||
| })} | ||
| {options.map((item, index) => { | ||
| return ( | ||
| <div | ||
| className="nut-picker-roller-item-title" | ||
| key={item.value ? item.value : index} | ||
| > | ||
| <>{item.text}</> | ||
| </div> | ||
| ) | ||
| })} |
There was a problem hiding this comment.
💡 Codebase verification
需要更新测试用例和文档
基于验证结果：
- 在 properties.json 中仍存在 3D 效果相关的配置项
- 测试用例中没有针对 3D 效果的测试，但需要更新以反映新的交互方式
- 文档中没有明确提到 3D 效果相关的内容
建议：
- 从 properties.json 中移除 threeDimensional 相关配置
- 更新测试用例以验证新的滚动交互
- 确保视觉测试覆盖新的滚动效果
🔗 Analysis chain
请确认视觉回归测试的更新
由于移除了3D效果，建议：
- 更新相关的视觉回归测试用例
- 确保文档中已移除所有关于3D效果的描述
- 验证在不同机型上的滚动效果是否正常
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查是否还有其他文件包含 threeDimensional 相关代码
rg -l "threeDimensional" .
# 检查文档中是否还有 3D 相关描述
rg -l "3D|三维|环绕" "src/packages/picker/doc*.md"
Length of output: 169
Script:
#!/bin/bash
# 检查 picker 相关的文档文件
fd "doc.*\.md" src/packages/picker/
# 检查 properties.json 中的 threeDimensional 相关内容
cat scripts/properties.json | grep -A 5 -B 5 "threeDimensional"
# 检查测试文件中是否包含相关测试用例
fd ".*test.*\.(ts|tsx|js|jsx)" src/packages/picker/
# 检查是否有视觉测试相关的配置文件
fd ".*\.(test|spec)\.(ts|tsx|js|jsx)" -e storybook -e cypress -e screenshot
Length of output: 745
Script:
#!/bin/bash
# 检查文档中的3D效果相关描述
cat src/packages/picker/doc.md
cat src/packages/picker/doc.en-US.md
cat src/packages/picker/doc.zh-TW.md
cat src/packages/picker/doc.taro.md
# 检查是否存在视觉测试相关目录
fd -t d "(test|__test__|cypress|screenshot|storybook)"
Length of output: 12916
Script:
#!/bin/bash
# 检查 picker 组件的测试文件内容
cat src/packages/picker/__tests__/*
# 检查 properties.json 中是否还有其他与 3D 效果相关的配置
cat scripts/properties.json | jq '.[] | select(.第二列 | contains("3D"))'
Length of output: 5367
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## feat_v3.x #2815 +/- ##
=============================================
+ Coverage 83.99% 84.10% +0.10%
=============================================
Files 220 273 +53
Lines 17885 18065 +180
Branches 2679 2681 +2
=============================================
+ Hits 15023 15194 +171
- Misses 2857 2866 +9
Partials 5 5 ☔ View full report in Codecov by Sentry. |
irisSong
requested changes
Dec 4, 2024
| return ( | ||
| <View | ||
| className={`nut-picker-roller-item ${ | ||
| isHidden(index + 1) && 'nut-picker-roller-item-hidden' |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/packages/picker/pickerpanel.tsx (1)
250-259: 建议添加无障碍访问支持为了提高组件的可访问性，建议添加适当的ARIA属性：
<div className="nut-picker-roller-item-title" key={item.value ? item.value : index} + role="option" + aria-selected={currIndex === index + 1} > <>{item.text}</> </div>这样可以：
- 让屏幕阅读器能够正确识别选项
- 提供更好的无障碍体验
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
migrate-from-v2.md(1 hunks)src/packages/picker/pickerpanel.taro.tsx(2 hunks)src/packages/picker/pickerpanel.tsx(2 hunks)
🔇 Additional comments (8)
migrate-from-v2.md (2)
444-444: 确认移除 Picker 组件的 3D 效果
这项更改与当前 PR 的目标一致，移除了 Picker 组件中的 3D 环绕效果。这种简化有助于提升组件的性能和可维护性。
Line range hint 1-1000: 文档结构清晰，迁移指南完整
该迁移指南详细记录了从 v2 到 v3 版本的所有重要变更，包括：
- 清晰的升级步骤说明
- 完整的 API 变更列表
- 按组件类别组织的变更说明
建议在合并此 PR 后，确保该文档与其他相关文档保持同步更新。
src/packages/picker/pickerpanel.tsx (2)
246-247: 样式应用逻辑简化，代码更清晰！
移除3D效果后，样式应用变得更加直接和统一，提高了代码的可维护性。
Line range hint 1-270: 请确认是否完全清理了所有3D效果相关的配置
基于之前的审查发现，建议进一步确认：
- properties.json 中是否还存在 3D 效果相关的配置项
- 文档是否已更新以反映移除 3D 效果的变化
- 相关的测试用例是否已更新
#!/bin/bash
# 检查是否还有其他文件包含 threeDimensional 相关代码
rg -l "threeDimensional" .
# 检查文档中是否还有 3D 相关描述
rg -l "3D|三维|环绕" "src/packages/picker/doc*.md"
# 检查 properties.json 中的配置
cat scripts/properties.json | grep -A 5 -B 5 "threeDimensional"src/packages/picker/pickerpanel.taro.tsx (4)
3-6: 导入语句重新排序符合最佳实践
React hooks的导入顺序调整更加合理，便于维护。
261-274: 确认扁平化渲染逻辑的完整性
移除3D效果后，选项渲染逻辑已简化为扁平化展示。建议：
- 确保所有选项的可见性和交互性
- 验证选项切换的过渡动画是否流畅
建议添加以下测试场景：
- 快速滑动时的性能表现
- 边界条件下的滚动行为
- 不同数量选项时的显示效果
Line range hint 281-281: 样式清理建议
响应之前的评审意见，建议进一步检查并清理以下内容：
- 相关的CSS/Less文件中的3D样式
- 主题文件中的相关变量
- 动画相关的过渡效果
257-257: 建议检查样式兼容性
在移除3D效果后，touchTileStyle函数仍保留了transform属性。建议验证在不同设备上的滚动表现是否一致。
✅ Verification successful
Let me check the touchTileStyle function implementation to understand what transform properties are being used.
transform 样式实现兼容
经过验证，touchTileStyle 函数仅使用了 translate3d 进行垂直方向的滚动变换，这是一个广泛支持且性能良好的变换方式。目前的实现：
- 仅包含必要的垂直位移变换
- 使用了标准的 cubic-bezier 缓动函数
- 没有使用可能导致兼容性问题的 3D 效果
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查其他组件中是否还存在3D相关样式
rg -g '*.less' -g '*.scss' -g '*.css' '3[dD]|transform.*rotate|perspective'
Length of output: 9521
Script:
#!/bin/bash
# Check touchTileStyle implementation
ast-grep --pattern 'touchTileStyle'
Length of output: 337
Script:
#!/bin/bash
# Check the implementation details of touchTileStyle function
ast-grep --pattern $'const touchTileStyle = () => {
$$$
}'
Length of output: 961
irisSong
approved these changes
Dec 5, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🤔 这个变动的性质是？
🔗 相关 Issue
💡 需求背景和解决方案
☑️ 请求合并前的自查清单
Summary by CodeRabbit
新功能
文档