enhance: Lock switch field if needConfirm #892
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Walkthrough此拉取请求对多个文件进行了修改，主要集中在 Changes
Possibly related PRs
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (6)
src/PickerInput/hooks/useRangeActiveLock.ts (2)
12-14: 建议添加状态初始化的注释说明为了提高代码可维护性，建议为这些状态变量添加用途说明的注释。
建议添加如下注释：
+ // 当前激活的索引 const [activeIndex, setActiveIndex] = React.useState<number | null>(null); + // 组件是否处于焦点状态 const [focused, setFocused] = React.useState<boolean>(false); + // 激活状态的历史记录 const [activeList, setActiveList] = React.useState<number[]>([]);
1-23: 缺少 JSDoc 文档作为一个公共 Hook，建议添加完整的 JSDoc 文档说明其用途、参数和返回值。
建议在文件开头添加如下文档：
+/** + * 用于管理日期范围选择器的激活状态的 Hook + * @returns {[boolean, (focused: boolean) => void, (index: number) => void]} + * - focused: 当前是否处于焦点状态 + * - triggerFocus: 设置焦点状态的函数 + * - setActiveIndex: 设置当前激活索引的函数 + */ export default function useRangeActiveLock(): [docs/examples/debug.tsx (1)
86-86: 建议完善示例文档这个新增的 RangePicker 实例虽然展示了 allowEmpty 的用法，但建议：
- 添加注释说明 allowEmpty 属性的作用和使用场景
- 根据 PR 的目标，展示 needConfirm 相关的功能
- 为不同的使用场景提供完整的示例代码
建议添加如下注释：
+ // 允许清空的日期范围选择器示例 + // allowEmpty: 允许用户清空已选择的日期范围 <RangePicker {...sharedLocale} style={{ width: 400 }} allowEmpty />src/PickerInput/hooks/useRangeValue.ts (1)
337-339: 建议为hasSubmitValue函数添加 JSDoc 文档为了提高代码的可维护性，建议添加 JSDoc 文档说明以下内容：
- 函数的用途
- 参数
index的含义和取值范围- 返回值的含义
建议添加如下文档：
+/** + * 检查指定索引位置是否存在已提交的值 + * @param index - 要检查的索引位置（0 表示开始日期，1 表示结束日期） + * @returns 如果指定位置存在值则返回 true，否则返回 false + */ function hasSubmitValue(index: number) { return !!submitValue()[index]; }src/PickerInput/RangePicker.tsx (1)
635-649: 增强的焦点管理逻辑新增的焦点管理逻辑通过以下方式提升了用户体验：
- 在需要确认时防止不必要的焦点切换
- 确保用户完成当前字段的输入后才能切换到下一个字段
- 与 allowEmpty 和 needConfirm 属性完美配合
建议添加注释说明这段复杂逻辑的具体用途。
onSelectorFocus: SelectorProps['onFocus'] = (event, index) => { + // 当需要确认且上一个值未提交时，保持焦点在上一个输入框 const activeListLen = activeIndexList.length; const lastActiveIndex = activeIndexList[activeListLen - 1]; if (tests/range.spec.tsx (1)
2059-2073: 测试用例实现正确且有价值！该测试用例很好地验证了在需要确认的情况下，点击第二个输入框不应该切换焦点的行为。建议考虑增加以下测试场景：
- 确认后再点击第二个输入框的情况
- 在第一个输入框选择日期后按 ESC 取消的情况
建议添加如下测试用例:
it('should not click to focus on next field if first field is not confirm', () => { const onCalendarChange = jest.fn(); const { container } = render( <DayRangePicker onCalendarChange={onCalendarChange} showTime needConfirm />, ); // Select first field openPicker(container, 0); selectCell(11); expect(onCalendarChange).toHaveBeenCalled(); // Not click confirm and click next field openPicker(container, 1); expect(container.querySelectorAll('.rc-picker-input')[0]).toHaveClass('rc-picker-input-active'); + + // Should allow focus second field after confirm + fireEvent.click(document.querySelector('.rc-picker-ok button')); + openPicker(container, 1); + expect(container.querySelectorAll('.rc-picker-input')[1]).toHaveClass('rc-picker-input-active'); + + // Should keep focus on first field when press ESC + openPicker(container, 0); + selectCell(12); + fireEvent.keyDown(container, { keyCode: KeyCode.ESC }); + openPicker(container, 1); + expect(container.querySelectorAll('.rc-picker-input')[0]).toHaveClass('rc-picker-input-active'); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
docs/examples/debug.tsx(1 hunks)src/PickerInput/RangePicker.tsx(3 hunks)src/PickerInput/hooks/useRangeActiveLock.ts(1 hunks)src/PickerInput/hooks/useRangeValue.ts(3 hunks)tests/range.spec.tsx(2 hunks)
🔇 Additional comments (5)
src/PickerInput/hooks/useRangeValue.ts (3)
79-79: 移除 useCalendarValue 的导出是个好的改进！
将 useCalendarValue 改为模块内部函数有助于更好的封装，这样的改动符合信息隐藏原则。
189-190: 返回类型的更新清晰且类型安全！
返回类型的文档注释和实际返回值保持一致，维护了良好的类型安全性。
Also applies to: 342-342
337-342: 验证 hasSubmitValue 的使用情况
为确保新增的 hasSubmitValue 函数被正确使用，建议验证其调用位置。
✅ Verification successful
hasSubmitValue 函数使用验证完成
hasSubmitValue 函数在代码中的使用是合理且安全的：
- 在
RangePicker.tsx中作为useRangeValuehook 的返回值被正确解构 - 用于判断指定索引位置是否已填充值的逻辑检查
- 与
allowEmpty和calendarValue配合使用，确保日期选择器的正确交互行为
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 搜索 hasSubmitValue 的使用位置
rg "hasSubmitValue" --type ts
Length of output: 416
Script:
#!/bin/bash
# 查看 RangePicker.tsx 中 hasSubmitValue 的具体使用上下文
rg "hasSubmitValue" src/PickerInput/RangePicker.tsx -B 5 -A 5
Length of output: 755
src/PickerInput/RangePicker.tsx (2)
36-36: 类型导入优化！
使用 TypeScript 的 type 关键字导入类型是一个很好的实践，它可以帮助优化编译性能。
328-329: 新增 hasSubmitValue 函数用于验证提交状态
新增的 hasSubmitValue 函数用于检查特定索引是否有已提交的值，这对于后续的焦点管理逻辑至关重要。
| export default function useRangeActiveLock(): [ | ||
| focused: boolean, | ||
| triggerFocus: (focused: boolean) => void, | ||
| // lastOperation: (type?: OperationType) => OperationType, | ||
| // activeIndex: number, | ||
| setActiveIndex: (index: number) => void, | ||
| // nextActiveIndex: NextActive<DateType>, | ||
| // activeList: number[], | ||
| ] { |
There was a problem hiding this comment.
类型定义与实现不匹配
返回类型签名中包含了被注释掉的类型定义，这与实际实现的返回值不一致。建议移除未使用的类型定义或实现完整功能。
建议应用以下更改：
export default function useRangeActiveLock(): [
focused: boolean,
triggerFocus: (focused: boolean) => void,
- // lastOperation: (type?: OperationType) => void,
- // activeIndex: number,
setActiveIndex: (index: number) => void,
- // nextActiveIndex: NextActive<DateType>,
- // activeList: number[],
] {📝 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.
| export default function useRangeActiveLock(): [ | |
| focused: boolean, | |
| triggerFocus: (focused: boolean) => void, | |
| // lastOperation: (type?: OperationType) => OperationType, | |
| // activeIndex: number, | |
| setActiveIndex: (index: number) => void, | |
| // nextActiveIndex: NextActive<DateType>, | |
| // activeList: number[], | |
| ] { | |
| export default function useRangeActiveLock(): [ | |
| focused: boolean, | |
| triggerFocus: (focused: boolean) => void, | |
| setActiveIndex: (index: number) => void, | |
| ] { |
| setActiveList([...activeList, index]); | ||
| }; | ||
|
|
||
| return [focused, setFocused, onActive]; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
返回值命名不一致
返回值中 setFocused 与类型定义中的 triggerFocus 命名不一致，onActive 与 setActiveIndex 命名不一致。
建议修改为：
- return [focused, setFocused, onActive];
+ return [focused, triggerFocus: setFocused, setActiveIndex: onActive];Committable suggestion skipped: line range outside the PR's diff.
| const onActive = (index: number) => { | ||
| setActiveIndex(index); | ||
| setActiveList([...activeList, index]); | ||
| }; |
There was a problem hiding this comment.
潜在的内存泄漏问题
activeList 数组只会不断增长而没有清理机制，可能导致内存泄漏。建议限制数组大小或在适当时机清理。
建议修改实现方式：
const onActive = (index: number) => {
setActiveIndex(index);
- setActiveList([...activeList, index]);
+ setActiveList(prevList => {
+ const newList = [...prevList, index];
+ // 只保留最近的 N 个记录
+ return newList.slice(-5);
+ });
};📝 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.
| const onActive = (index: number) => { | |
| setActiveIndex(index); | |
| setActiveList([...activeList, index]); | |
| }; | |
| const onActive = (index: number) => { | |
| setActiveIndex(index); | |
| setActiveList(prevList => { | |
| const newList = [...prevList, index]; | |
| // 只保留最近的 N 个记录 | |
| return newList.slice(-5); | |
| }); | |
| }; |
| // disabledTime={(date, range, info) => { | ||
| // // console.log(`Time-${range}`, range, info); | ||
| // const { from } = info; | ||
|
|
||
| if (from) { | ||
| console.log( | ||
| `Time-${range}`, | ||
| from.format('YYYY-MM-DD HH:mm:ss'), | ||
| date.format('YYYY-MM-DD HH:mm:ss'), | ||
| ); | ||
| } | ||
| // if (from) { | ||
| // console.log( | ||
| // `Time-${range}`, | ||
| // from.format('YYYY-MM-DD HH:mm:ss'), | ||
| // date.format('YYYY-MM-DD HH:mm:ss'), | ||
| // ); | ||
| // } | ||
|
|
||
| if (from && from.isSame(date, 'day')) { | ||
| return { | ||
| disabledHours: () => [from.hour()], | ||
| disabledMinutes: () => [0, 1, 2, 3], | ||
| disabledSeconds: () => [0, 1, 2, 3], | ||
| }; | ||
| } | ||
| return {}; | ||
| }} | ||
| // if (from && from.isSame(date, 'day')) { | ||
| // return { | ||
| // disabledHours: () => [from.hour()], | ||
| // disabledMinutes: () => [0, 1, 2, 3], | ||
| // disabledSeconds: () => [0, 1, 2, 3], | ||
| // }; | ||
| // } | ||
| // return {}; | ||
| // }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
建议删除注释代码而不是保留
建议完全删除这段被注释的代码，而不是将其保留在文件中。如果这些代码对于示例或测试很重要，应该：
- 将其移动到单独的示例文件中
- 或者在文档中说明这种用法
保留注释掉的代码会：
- 增加维护负担
- 造成代码混乱
- 可能导致过时的实现被重新使用
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #892 +/- ##
==========================================
+ Coverage 95.43% 95.46% +0.02%
==========================================
Files 64 64
Lines 2719 2732 +13
Branches 762 769 +7
==========================================
+ Hits 2595 2608 +13
Misses 121 121
Partials 3 3 ☔ View full report in Codecov by Sentry. |
ref ant-design/ant-design#50979
Summary by CodeRabbit
RangePicker组件中新增allowEmpty属性，允许选择空日期范围。useRangeActiveLock，管理组件的活动状态。