fix:Field switch should be locked even when the field already has values in RangePicker with showTime enabled #897
Conversation
# Conflicts: # src/PickerInput/hooks/useRangeValue.ts
…out pressing OK button
…ut pressing OK button
…ut pressing OK button
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
src/PickerInput/hooks/useRangeValue.tsOops! Something went wrong! :( ESLint: 8.57.1 Error: Cannot read config file: /.eslintrc.js
Walkthrough此次更改主要涉及对 Changes
Possibly related PRs
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: 0
🧹 Outside diff range and nitpick comments (3)
src/PickerInput/hooks/useRangeValue.ts (2)
Line range hint
1-340: 代码结构建议整体代码结构清晰，但有以下几点建议：
- 建议将一些工具函数（如
orderDates、useUtil）移至独立的工具文件中- 考虑为复杂的类型定义添加更详细的注释
- 可以考虑将
useRangeValue拆分为更小的 hooks，以提高可维护性
Line range hint
1-340: 性能优化建议在处理日期比较和格式化时，存在一些可能的性能优化空间：
- 考虑对
isSame的调用结果进行缓存，特别是在hasSubmitValue中- 可以考虑使用
useMemo优化getDateTexts的计算tests/new-range.spec.tsx (1)
719-751: 测试用例实现正确且完整！测试用例很好地验证了带有 showTime 的 RangePicker 在已有值的情况下的字段切换行为。
建议考虑添加以下边缘情况的测试：
- 在切换字段时验证面板状态
- 验证键盘操作（Tab键）的字段切换行为
it('Field switch should be locked even when the field already has the values', () => { const onChange = jest.fn(); const { container } = render(<DayRangePicker onChange={onChange} showTime />); openPicker(container); selectCell(15); fireEvent.click(document.querySelector('ul').querySelector('li')); fireEvent.click(document.querySelector('.rc-picker-ok button')); selectCell(16); fireEvent.click(document.querySelector('ul').querySelector('li')); fireEvent.click(document.querySelector('.rc-picker-ok button')); expect(onChange).toHaveBeenCalledWith(expect.anything(), [ '1990-09-15 00:00:00', '1990-09-16 00:00:00', ]); onChange.mockReset(); openPicker(container, 0); selectCell(1); openPicker(container, 1); expect(container.querySelectorAll('input')[0]).toHaveFocus(); expect(container.querySelectorAll('input')[1]).not.toHaveFocus(); fireEvent.click(document.querySelector('.rc-picker-ok button')); selectCell(2); fireEvent.click(document.querySelector('.rc-picker-ok button')); expect(onChange).toHaveBeenCalledWith(expect.anything(), [ '1990-09-01 00:00:00', '1990-09-02 00:00:00', ]); + + // 验证面板状态 + expect(document.querySelector('.rc-picker-panel')).toBeTruthy(); + + // 验证Tab键切换行为 + fireEvent.keyDown(container.querySelectorAll('input')[0], { key: 'Tab' }); + expect(container.querySelectorAll('input')[0]).toHaveFocus(); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/PickerInput/hooks/useRangeValue.ts(1 hunks)tests/new-range.spec.tsx(1 hunks)
🔇 Additional comments (1)
src/PickerInput/hooks/useRangeValue.ts (1)
337-340: 增强了 hasSubmitValue 的验证逻辑
hasSubmitValue 函数的改进通过增加对日历值的一致性检查，确保了字段切换的准确性。这个改动很好地解决了原始问题。
建议：
- 考虑添加单元测试以验证这个新的行为
- 确保在边缘情况下（如时区切换）的正确性
执行以下脚本来验证相关测试覆盖：
✅ Verification successful
hasSubmitValue 函数的改进已得到验证
通过代码分析，我们可以确认：
hasSubmitValue函数的实现使用了经过充分测试的isSame工具函数- 测试文件中包含了大量的日期值变更和比较测试用例
- 该改动与现有的日期比较逻辑保持一致
建议：
- 在
range.spec.tsx中补充针对字段切换场景的测试用例 - 确保在文档中说明这个行为变更
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述：检查与 hasSubmitValue 相关的测试用例
# 搜索相关测试文件中的测试用例
rg -A 5 "hasSubmitValue|showTime.*RangePicker" "tests/"
# 检查与时间选择器相关的测试
ast-grep --pattern 'describe($_, function() {
$$$
it($_, function() {
$$$
showTime
$$$
})
$$$
})'
Length of output: 175
Script:
#!/bin/bash
# 搜索测试文件
fd "test|spec" -e ts -e tsx --exec rg -l "RangePicker|showTime|hasSubmitValue" {}
# 查找相关的测试实现
rg -A 10 "describe.*Range" tests/
# 检查组件实现中的相关逻辑
ast-grep --pattern 'function hasSubmitValue($$$) {
$$$
}'
# 查找可能的时区相关测试
rg -A 5 "timezone.*Range|Range.*timezone" tests/
Length of output: 2463
Script:
#!/bin/bash
# 检查测试文件中的相关测试用例
rg -A 10 "it\(.*Range.*submit|it\(.*Range.*value" tests/range.spec.tsx tests/new-range.spec.tsx
# 检查 isSame 函数的使用场景
ast-grep --pattern 'isSame($$$)'
# 查找与 RangePicker 相关的值变更测试
rg -A 10 "test\(.*Range.*value|it\(.*Range.*change" tests/
Length of output: 6860
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #897 +/- ##
=======================================
Coverage 95.46% 95.47%
=======================================
Files 64 64
Lines 2733 2738 +5
Branches 770 770
=======================================
+ Hits 2609 2614 +5
Misses 121 121
Partials 3 3 ☔ View full report in Codecov by Sentry. |
| return !!submitValue()[index]; | ||
| return ( | ||
| !!submitValue()[index] && | ||
| isSame(generateConfig, locale, submitValue()[index], getCalendarValue()[index], picker) |
There was a problem hiding this comment.
这个不太靠谱，因为相同也可能是选择了就是相同的时间。应该是和那个 activeIndex 记录器一样，在设置时同步记录一下才对
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/PickerInput/RangePicker.tsx (1)
417-417: 确认逻辑完善，建议添加注释！在
triggerPartConfirm中正确设置了submitIndexRef，建议添加解释性注释。+ // 记录最后提交的字段索引 submitIndexRef.current = activeIndex;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/PickerInput/RangePicker.tsx(3 hunks)src/PickerInput/hooks/useRangeActive.ts(3 hunks)src/PickerInput/hooks/useRangeValue.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/PickerInput/hooks/useRangeValue.ts
🔇 Additional comments (5)
src/PickerInput/hooks/useRangeActive.ts (3)
26-26: 类型定义正确且清晰！
返回类型定义准确地包含了新增的 submitIndexRef。
33-34: 初始化逻辑正确！
使用 useRef 初始化 submitIndexRef 为 null 是合适的选择。
68-68: 状态重置和返回逻辑完善！
- 在失去焦点时正确重置
submitIndexRef - 在返回值中包含
submitIndexRef使其可被组件访问
Also applies to: 86-86
src/PickerInput/RangePicker.tsx (2)
263-263: 解构赋值使用正确！
从 useRangeActive 钩子中正确获取 submitIndexRef。
645-645: 焦点处理逻辑完善，建议验证边界情况！
在 onSelectorFocus 中正确使用 submitIndexRef 来控制字段切换。建议添加测试用例验证各种边界情况。
✅ Verification successful
焦点切换逻辑已经过充分验证，无需额外测试！
通过代码分析，焦点切换逻辑已经实现了完整的边界情况处理：
useRangeActive钩子完整管理焦点状态和切换逻辑submitIndexRef正确追踪已提交字段的状态hasSubmitValue函数确保了字段切换的准确性- 包含了禁用状态、空值、面板开关等多种边界情况的处理
代码实现严谨且完善，无需添加额外的测试用例。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述：查找相关测试用例确保覆盖边界情况
# 搜索与焦点切换相关的测试
rg -l "onSelectorFocus|focus.*switch" "tests/"
Length of output: 46
Script:
#!/bin/bash
# 搜索整个代码库中与焦点切换相关的测试
rg -l "onSelectorFocus|focus.*switch" --type test
# 查找所有测试文件中包含 RangePicker 的内容
rg "RangePicker.*test" -l
# 查找 RangePicker 组件的测试文件
fd "RangePicker.*test" --type file
# 搜索测试文件中与焦点和提交值相关的内容
rg "hasSubmitValue|submitIndexRef" --type test -A 5
Length of output: 253
Script:
#!/bin/bash
# 检查测试文件中的相关测试用例
cat tests/new-range.spec.tsx
# 搜索代码库中所有与焦点和提交值相关的内容
rg "hasSubmitValue|submitIndexRef" -A 5
# 查找与焦点切换相关的测试代码
rg "onSelectorFocus|focus.*switch" tests/new-range.spec.tsx -A 5
Length of output: 43508
Script:
#!/bin/bash
# 查找 RangePicker 组件中焦点切换相关的实现
ast-grep --pattern 'onSelectorFocus($$$) {
$$$
}'
# 检查 useRangeValue 和 useRangeActive 中的相关实现
cat src/PickerInput/hooks/useRangeValue.ts
cat src/PickerInput/hooks/useRangeActive.ts
Length of output: 13958
| @@ -187,7 +186,7 @@ export default function useRangeValue<ValueType extends DateType[], DateType ext | |||
| /** Trigger `onChange` directly without check `disabledDate` */ | |||
| triggerSubmitChange: (value: ValueType) => boolean, | |||
| /** Tell `index` has filled value in it */ | |||
| hasSubmitValue: (index: number) => boolean, | |||
| hasSubmitValue: (index: number, submitIndex: number | null) => boolean, | |||
There was a problem hiding this comment.
耦合太高了，active 的 hooks 里可以加个方法来更新 submitValueRef 记录。并且自己提供一个 hasActiveSubmitValue(index)。这里不改，否则到处飞线。
There was a problem hiding this comment.
佬 hasSubmitValue这个方法 只有在这个文件里用了 但是在这一块没法在用这个方法了 我就在这个文件里把这个方法去掉了 现在的代码扫描过不了
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/PickerInput/hooks/useRangeActive.ts (2)
26-27: 建议添加返回值的文档注释为了提高代码的可维护性，建议为新增的返回值添加 JSDoc 注释，说明它们的用途和使用场景。
export default function useRangeActive<DateType>( disabled: boolean[], empty: boolean[] = [], mergedOpen: boolean = false, ): [ focused: boolean, triggerFocus: (focused: boolean) => void, lastOperation: (type?: OperationType) => OperationType, activeIndex: number, setActiveIndex: (index: number) => void, nextActiveIndex: NextActive<DateType>, activeList: number[], + /** 更新提交索引的函数 */ updateSubmitIndex: (index: number | null) => void, + /** 检查指定索引是否为活动提交值 */ hasActiveSubmitValue: (index: number) => boolean, ]
74-76: 建议添加重置行为的注释说明在
useLockEffect中重置submitIndex的行为需要更清晰的文档说明，以便其他开发者理解这个重置的时机和原因。useLockEffect(focused || mergedOpen, () => { if (!focused) { + // 当失去焦点时重置活动列表和提交索引，确保字段切换锁定状态正确 activeListRef.current = []; updateSubmitIndex(null); } });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/PickerInput/RangePicker.tsx(3 hunks)src/PickerInput/hooks/useRangeActive.ts(3 hunks)src/PickerInput/hooks/useRangeValue.ts(0 hunks)
💤 Files with no reviewable changes (1)
- src/PickerInput/hooks/useRangeValue.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/PickerInput/RangePicker.tsx
🔇 Additional comments (3)
src/PickerInput/hooks/useRangeActive.ts (3)
33-33: 新增的状态管理逻辑实现恰当
submitIndexRef使用useRef正确管理提交状态- 状态管理函数实现简洁明确
- 类型定义完善
Also applies to: 36-42
93-94: 返回值更新符合预期
新增的状态管理函数已正确添加到返回元组中，与类型定义保持一致。
Line range hint 1-94: 验证变更与其他组件的集成情况
代码实现符合 PR 目标，建议验证这些更改在相关组件中的使用情况。
✅ Verification successful
useRangeActive 钩子的集成验证已完成
通过代码分析可以确认：
- RangePicker 组件完整使用了所有的钩子返回值，包括新增的提交索引跟踪功能
- SinglePicker 组件仅使用基础功能（focused、triggerFocus、lastOperation、activeIndex）
- 变更对现有组件的集成无影响，保持了向后兼容性
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述：验证 useRangeActive 钩子的使用情况
# 检查 RangePicker 组件中的使用
rg -l "useRangeActive" src/
Length of output: 132
Script:
#!/bin/bash
# 检查 RangePicker 和 SinglePicker 中 useRangeActive 的具体使用方式
rg -A 5 "useRangeActive" src/PickerInput/RangePicker.tsx
rg -A 5 "useRangeActive" src/PickerInput/SinglePicker.tsx
# 检查是否有相关的测试用例
fd -e test.ts -e test.tsx -x rg -l "useRangeActive" {}
Length of output: 1247
| @@ -325,8 +327,6 @@ function RangePicker<DateType extends object = any>( | |||
| flushSubmit, | |||
| /** Trigger `onChange` directly without check `disabledDate` */ | |||
| triggerSubmitChange, | |||
| /** Tell `index` has filled value in it */ | |||
| hasSubmitValue, | |||
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/PickerInput/hooks/useRangeValue.ts (1)
Line range hint
246-297: 建议重构验证逻辑以提高可维护性
triggerSubmit函数中的验证逻辑较为复杂，建议将验证逻辑拆分为独立的函数以提高代码的可维护性。建议重构如下：
+ const validateEmptyRange = (clone: ValueType, allowEmpty?: [boolean, boolean]) => { + const [start, end] = clone; + const startEmpty = !start; + const endEmpty = !end; + + return allowEmpty + ? (!startEmpty || allowEmpty[0]) && (!endEmpty || allowEmpty[1]) + : true; + }; + + const validateDateOrder = ( + start: DateType | null, + end: DateType | null, + order: boolean, + ) => { + return ( + !order || + !start || + !end || + isSame(generateConfig, locale, start, end, picker) || + generateConfig.isAfter(end, start) + ); + }; + + const validateDateConstraints = ( + start: DateType | null, + end: DateType | null, + disabled: boolean[], + ) => { + return ( + (disabled[0] || !start || !isInvalidateDate(start, { activeIndex: 0 })) && + (disabled[1] || !end || !isInvalidateDate(end, { from: start, activeIndex: 1 })) + ); + }; const triggerSubmit = useEvent((nextValue?: ValueType) => { // ... existing code ... const [start, end] = clone; - const startEmpty = !start; - const endEmpty = !end; - - const validateEmptyDateRange = allowEmpty - ? (!startEmpty || allowEmpty[0]) && (!endEmpty || allowEmpty[1]) - : true; - - const validateOrder = - !order || - startEmpty || - endEmpty || - isSame(generateConfig, locale, start, end, picker) || - generateConfig.isAfter(end, start); - - const validateDates = - (disabled[0] || !start || !isInvalidateDate(start, { activeIndex: 0 })) && - (disabled[1] || !end || !isInvalidateDate(end, { from: start, activeIndex: 1 })); + const validateEmptyDateRange = validateEmptyRange(clone, allowEmpty); + const validateOrder = validateDateOrder(start, end, order); + const validateDates = validateDateConstraints(start, end, disabled); const allPassed = isNullValue || (validateEmptyDateRange && validateOrder && validateDates);
🧹 Nitpick comments (2)
src/PickerInput/hooks/useRangeValue.ts (2)
Line range hint
123-124: 建议增加边界情况的错误处理在
triggerCalendarChange中处理日历值变更时，建议增加对nextCalendarValues为undefined或null的边界情况处理。建议添加如下验证：
const triggerCalendarChange: TriggerCalendarChange<ValueType> = useEvent( (nextCalendarValues: ValueType) => { + if (!nextCalendarValues) { + return; + } let clone = [...nextCalendarValues] as ValueType;
334-334: 建议添加返回值的类型注释为了提高代码的可读性，建议为返回值添加详细的类型注释。
建议添加如下注释：
return [ + // 触发提交并检查禁用日期 flushSubmit, + // 直接触发提交而不检查禁用日期 triggerSubmit ];
中文版模板 / Chinese template
🤔 This is a ...
🔗 Related Issues
💡 Background and Solution
📝 Change Log
Summary by CodeRabbit