修改picker组件issue #2360
修改picker组件issue #2360
Conversation
Walkthrough这些更改涉及InternalPicker组件内逻辑和控制流的修改。具体来说，调整了基于所选值的onChange事件处理，以解决特定问题并在确认选择时确保正确行为。此外，还重构了useEffect钩内的setInnerValue逻辑以处理选择的值并防止不必要的修改。 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Picker as InternalPicker
participant ConfirmHandler
User ->> Picker: Selects Value
Picker ->> Picker: setInnerValue(newValue)
alt if val is present
Picker ->> ConfirmHandler: props.onConfirm(val)
end
Picker ->> User: onChange Callback
Poem
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Additional context usedBiome
Additional comments not posted (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 as PR comments)
Additionally, you can add CodeRabbit Configration 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: 3
Outside diff range and nitpick comments (3)
src/packages/picker/picker.tsx (3)
Line range hint
280-282: 建议使用可选链操作符来简化代码。- columnOptions && Object.keys(columnOptions).length + columnOptions?.length
Line range hint
305-317: 鼠标事件应与键盘事件一起提供，以支持键盘导航。建议添加键盘事件处理函数，如
onKeyPress，以改善无障碍性。
Line range hint
322-327: 鼠标事件应与键盘事件一起提供，以支持键盘导航。建议添加键盘事件处理函数，如
onKeyPress，以改善无障碍性。
|
|
||
| onChange && onChange(setSelectedOptions(), combineResult, columnIndex) |
There was a problem hiding this comment.
建议使用可选链操作符来简化代码。
- onChange && onChange(setSelectedOptions(), combineResult, columnIndex)
+ onChange?.(setSelectedOptions(), combineResult, columnIndex)Also applies to: 262-264
| @@ -98,7 +98,7 @@ const InternalPicker: ForwardRefRenderFunction< | |||
| defaultValue: [...defaultValue], | |||
| finalValue: [...defaultValue], | |||
| onChange: (val: (string | number)[]) => { | |||
| props.onConfirm?.(setSelectedOptions(), val) | |||
| !val && props.onConfirm?.(setSelectedOptions(), val) | |||
There was a problem hiding this comment.
代码逻辑存在潜在问题。当 val 为 undefined 或 null 时，onConfirm 事件仍会被调用，这可能不是预期行为。
- !val && props.onConfirm?.(setSelectedOptions(), val)
+ val && props.onConfirm?.(setSelectedOptions(), val)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.
| !val && props.onConfirm?.(setSelectedOptions(), val) | |
| val && props.onConfirm?.(setSelectedOptions(), val) |
| // 此hook的作用是‘如果内部选中值与用户选中值不同则把内部值置用户选中值’保证用户打开选项时选中的是选择的值。 | ||
| // 但是当用户并没有进行确认选择，则不需要进行修改innerValue，否则会出现 issue#2290的问题 | ||
| if (innerValue !== selectedValue && selectedValue.length > 0) { | ||
| setInnerValue(selectedValue) | ||
| } |
There was a problem hiding this comment.
此处的逻辑处理可能会导致不必要的状态更新，建议仅在 selectedValue 有实质性变化时更新 innerValue。
- if (innerValue !== selectedValue && selectedValue.length > 0) {
+ if (!isEqual(innerValue, selectedValue) && selectedValue.length > 0) {
setInnerValue(selectedValue)
}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.
| // 此hook的作用是‘如果内部选中值与用户选中值不同则把内部值置用户选中值’保证用户打开选项时选中的是选择的值。 | |
| // 但是当用户并没有进行确认选择，则不需要进行修改innerValue，否则会出现 issue#2290的问题 | |
| if (innerValue !== selectedValue && selectedValue.length > 0) { | |
| setInnerValue(selectedValue) | |
| } | |
| // 此hook的作用是‘如果内部选中值与用户选中值不同则把内部值置用户选中值’保证用户打开选项时选中的是选择的值。 | |
| // 但是当用户并没有进行确认选择，则不需要进行修改innerValue，否则会出现 issue#2290的问题 | |
| if (!isEqual(innerValue, selectedValue) && selectedValue.length > 0) { | |
| setInnerValue(selectedValue) | |
| } |
|
请问这个问题啥时候解决呢 今天就遇到了 |
🤔 这个变动的性质是？
🔗 相关 Issue
Picker 和 FormItem 搭配使用时，ref.open() 会触发 Picker 的 onClose #2312
💡 需求背景和解决方案
背景：线上issue
解决方案：通过修改源代码的方式对issue进行修改，不破坏原逻辑，对性能没有影响
☑️ 请求合并前的自查清单
Summary by CodeRabbit
onChange事件处理逻辑，以确保在确认选择时行为正确。