feat(pickerview): 小程序支持自定义列表行高 #3102
Conversation
…ui-react into feat_add_pickerview
Walkthrough该 PR 对多个 picker 组件进行了配置和样式调整。在 demo 中移除了 ConfigProvider 主题配置中的 Changes
Sequence Diagram(s)sequenceDiagram
participant C as InternalPickerRoller
participant UE as useEffect Hook
participant A as getReactHeight
participant UI as 渲染逻辑
C->>UE: 组件挂载
UE->>A: 调用 getReactHeight 获取占位高度
A-->>UE: 返回高度或错误处理
UE->>C: 更新 isGetLineSpacing 状态
C->>UI: 根据状态条件渲染 picker 项
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🪧 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 (
|
github-actions
bot
added
action:review
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## feat_v3.x #3102 +/- ##
=============================================
+ Coverage 86.49% 86.95% +0.46%
=============================================
Files 280 280
Lines 18492 18456 -36
Branches 2774 2785 +11
=============================================
+ Hits 15994 16049 +55
+ Misses 2493 2402 -91
Partials 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/packages/pickerview/pickerroller.taro.tsx (2)
159-168: 实现异步获取元素高度的功能
getReactHeight函数通过getRectByTaro异步获取占位符元素的高度，并提供了适当的错误处理。这是实现自定义行高的核心功能，可以在运行时动态获取实际渲染的元素高度。建议考虑在错误处理中添加一个合理的默认高度值，而不仅仅是返回0，以确保在获取高度失败时组件仍能正常显示。
- return 0 + return 36 // 使用默认高度作为回退值
264-277: 条件渲染优化根据
isGetLineSpacing状态条件渲染列表项，确保在获取到行高之前不会渲染内容，避免了潜在的布局问题。同时，类名的引用也统一使用了classPrefix变量。建议考虑在未获取到行高时显示一个加载指示器或占位内容，而不是简单地返回
null，以提供更好的用户体验。- ) : null} + ) : ( + <View className={`${classPrefix}-item-loading`}> + 加载中... + </View> + )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/packages/datepickerview/__test__/__snapshots__/datepickerview.spec.tsx.snapis excluded by!**/*.snapsrc/packages/pickerview/__test__/__snapshots__/pickerview.spec.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (4)
src/packages/picker/demos/taro/demo8.tsx(0 hunks)src/packages/pickerview/pickerroller.taro.tsx(5 hunks)src/packages/pickerview/pickerroller.tsx(4 hunks)src/packages/pickerview/pickerview.scss(1 hunks)
💤 Files with no reviewable changes (1)
- src/packages/picker/demos/taro/demo8.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (12)
src/packages/pickerview/pickerview.scss (1)
52-54: 新增的占位符样式有利于自定义行高添加的占位符样式类
.nut-pickerview-roller-placeholder与其设置的高度$picker-item-height很好地支持了小程序列表行高自定义的需求。通过这个占位符元素，组件可以在运行时获取实际渲染高度。src/packages/pickerview/pickerroller.tsx (4)
28-28: 类名前缀变量化，提高代码可维护性将类名前缀提取为常量
classPrefix，有助于统一管理类名，提高代码的可维护性和一致性。
213-213: 类名引用统一使用变量将硬编码的类名替换为
classPrefix变量，保持了类名引用的一致性。
222-224: 组件项类名引用统一使用变量使用模板字符串和
classPrefix变量替换硬编码的类名，提高了代码的可维护性。这种模式确保了类名的一致性，并便于后续的样式调整。
236-238: 平铺视图类名引用统一使用变量平铺视图中的类名同样使用
classPrefix变量进行统一管理，保持了代码风格的一致性。src/packages/pickerview/pickerroller.taro.tsx (7)
16-17: 引入了必要的工具函数支持新功能引入
useUuid和getRectByTaro为实现自定义行高的功能提供了必要的工具支持。useUuid生成唯一标识符用于元素引用，而getRectByTaro则用于获取元素的尺寸信息。
32-34: 类名前缀变量化与唯一ID生成引入
classPrefix常量和uuid变量，有助于统一管理类名和生成唯一的元素ID，增强了组件的可维护性和在复杂页面中的稳定性。
47-47: 添加必要的状态管理新增的
placeholderRef和isGetLineSpacing状态变量对于实现自定义行高非常重要。placeholderRef用于引用占位符元素，而isGetLineSpacing则用于控制组件的渲染逻辑，确保在获取到行高后再渲染内容。Also applies to: 52-52
172-191: 改进的行高获取逻辑更新后的行高获取逻辑能够适应不同的平台环境。在Web环境中，它尝试使用CSS变量获取高度；在小程序环境中，它通过
getReactHeight函数异步获取占位符元素的高度。这种方法很好地支持了跨平台的自定义行高需求。代码的逻辑分支清晰，并且在成功获取高度后更新了
isGetLineSpacing状态，确保后续渲染能够使用正确的行高值。
251-254: 添加占位符元素新增的占位符元素与前面在SCSS中定义的样式配合使用，为获取实际行高提供了基础。通过设置唯一ID和引用，确保了在复杂页面中能够准确地获取到对应元素的高度。
256-256: 统一使用类名前缀变量将硬编码的类名替换为
classPrefix变量，保持了类名引用的一致性，提高了代码的可维护性。
284-286: 统一平铺视图的类名引用平铺视图中的类名也使用
classPrefix变量进行统一管理，保持了代码风格的一致性，提高了可维护性。
| /> | ||
| <ConfigProvider | ||
| theme={{ | ||
| nutuiPickerItemHeight: '48px', |
There was a problem hiding this comment.
建议保留在 theme 中增加 变量，把 style 中的去掉~~
There was a problem hiding this comment.
建议保留在 theme 中增加 变量，把 style 中的去掉~~
已修改
| {renderLabel(item)} | ||
| </View> | ||
| <> | ||
| {isGetLineSpacing ? ( |
| <View | ||
| className="nut-pickerview-roller-item-tiled" | ||
| className={classNames(`${classPrefix}-item-tiled`, { | ||
| [`${classPrefix}-item-active`]: index + 1 === currentIndex, |
| <View className="nut-pickerview-list" ref={pickerRollerRef}> | ||
| <View | ||
| className="nut-pickerview-roller" | ||
| className={`${classPrefix}-placeholder`} |
There was a problem hiding this comment.
这里 placeholder 不需要~~ 直接 classprefix 就可以~~
There was a problem hiding this comment.
这里 placeholder 不需要~~ 直接 classprefix 就可以~~
已修改
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/packages/picker/demos/h5/demo8.tsx(1 hunks)src/packages/picker/demos/taro/demo8.tsx(1 hunks)src/packages/pickerview/pickerroller.taro.tsx(5 hunks)src/packages/pickerview/pickerview.scss(0 hunks)
💤 Files with no reviewable changes (1)
- src/packages/pickerview/pickerview.scss
🚧 Files skipped from review as they are similar to previous changes (1)
- src/packages/picker/demos/taro/demo8.tsx
🧰 Additional context used
🧬 Code Definitions (1)
src/packages/pickerview/pickerroller.taro.tsx (2)
src/packages/pickerview/index.taro.ts (1) (1)
PickerOption(5-5)src/packages/pickerview/index.ts (1) (1)
PickerOption(5-5)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (9)
src/packages/picker/demos/h5/demo8.tsx (1)
48-48: 通过ConfigProvider修改了Picker项目行高在Demo8示例中，通过配置
nutuiPickerItemHeight属性将Picker的行高设置为28px，这种方式更加规范和统一。src/packages/pickerview/pickerroller.taro.tsx (8)
16-17: 引入了新的工具函数以支持自定义行高功能引入
useUuid和getRectByTaro两个工具函数，用于生成唯一标识符和获取元素尺寸信息，这对于实现自定义行高的功能是必要的。
32-34: 统一类名前缀提高了代码可维护性定义
classPrefix常量统一管理类名前缀，并使用useUuid生成唯一ID，这符合最佳实践，提高了代码的可维护性和组件的可靠性。
51-51: 添加状态追踪行高获取状态通过
isGetLineSpacing状态变量追踪行高信息是否已获取，并使用web()函数初始化，这有助于在不同平台上正确渲染组件。
158-167: 添加了异步获取元素高度的函数
getReactHeight函数通过getRectByTaro异步获取元素高度，并包含适当的错误处理。当获取失败时返回0作为默认值，确保了代码的健壮性。
172-190: 改进了行高获取的逻辑更新了useEffect钩子函数，根据平台类型选择不同的行高获取方式：Web平台使用CSS变量，非Web平台通过
getReactHeight函数异步获取元素高度。这种方式能更好地适应不同的运行环境。
250-252: 更新了类名和ID的使用方式使用统一的
classPrefix变量和唯一ID设置元素属性，提高了代码的一致性和可维护性。
260-272: 条件渲染优化展示逻辑基于
isGetLineSpacing状态条件渲染列表项，确保在获取到行高信息后才渲染内容，避免了可能的布局问题。但是这里使用的Fragment标签（<>...</>）可能是不必要的。建议将Fragment移除，简化代码：
- <> - {isGetLineSpacing ? ( - <View - className={classNames(`${classPrefix}-item`, { - [`${classPrefix}-item-hidden`]: isItemHidden(index + 1), - [`${classPrefix}-item-active`]: index + 1 === currentIndex, - })} - style={rollerStyle(index)} - key={item.value ?? index} - > - {renderLabel(item)} - </View> - ) : null} - </> + {isGetLineSpacing && ( + <View + className={classNames(`${classPrefix}-item`, { + [`${classPrefix}-item-hidden`]: isItemHidden(index + 1), + [`${classPrefix}-item-active`]: index + 1 === currentIndex, + })} + style={rollerStyle(index)} + key={item.value ?? index} + > + {renderLabel(item)} + </View> + )}
279-281: 统一更新了平铺模式的类名使用
classPrefix变量更新了平铺模式下的类名，保持了命名的一致性。这一行的条件判断逻辑可以考虑提取为一个单独的方法，以提高代码可读性。建议提取一个判断是否为激活项的辅助函数：
+ const isActiveItem = (index: number) => index + 1 === currentIndex; <View className={classNames(`${classPrefix}-item-tiled`, { - [`${classPrefix}-item-active`]: index + 1 === currentIndex, + [`${classPrefix}-item-active`]: isActiveItem(index), })} key={item.value ?? index} >
| getReactHeight().then((height) => { | ||
| currentLineSpacing = height | ||
| if (currentLineSpacing) { | ||
| lineSpacing.current = currentLineSpacing | ||
| setGetStatus(true) | ||
| } | ||
| }) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
异步获取行高逻辑需要考虑组件卸载情况
在异步获取行高的逻辑中，没有处理组件可能在异步操作完成前已经卸载的情况，这可能导致内存泄漏或者状态更新到已卸载组件的警告。
建议添加一个引用标志来跟踪组件是否已卸载：
+ const isMounted = useRef(true);
+
+ useEffect(() => {
+ return () => {
+ isMounted.current = false;
+ };
+ }, []);
getReactHeight().then((height) => {
currentLineSpacing = height
if (currentLineSpacing) {
lineSpacing.current = currentLineSpacing
+ if (isMounted.current) {
setGetStatus(true)
+ }
}
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| getReactHeight().then((height) => { | |
| currentLineSpacing = height | |
| if (currentLineSpacing) { | |
| lineSpacing.current = currentLineSpacing | |
| setGetStatus(true) | |
| } | |
| }) | |
| } | |
| const isMounted = useRef(true); | |
| useEffect(() => { | |
| return () => { | |
| isMounted.current = false; | |
| }; | |
| }, []); | |
| getReactHeight().then((height) => { | |
| currentLineSpacing = height; | |
| if (currentLineSpacing) { | |
| lineSpacing.current = currentLineSpacing; | |
| if (isMounted.current) { | |
| setGetStatus(true); | |
| } | |
| } | |
| }); | |
| } |
3 participants
🤔 这个变动的性质是？
🔗 相关 Issue
💡 需求背景和解决方案
☑️ 请求合并前的自查清单
Summary by CodeRabbit
Style
New Features