-
Notifications
You must be signed in to change notification settings - Fork 284
refactor: datepicker #3013
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: datepicker #3013
Conversation
Walkthrough此次 PR 主要对 DatePicker 组件进行了重构和扩展。组件从函数式组件转变为 forwardRef 模式,内部状态管理得到增强,props 接口和事件处理逻辑也做了调整。同时,新增了自定义 hook(useDatePicker)用于默认日期描述和数值的管理,并在多个示例(h5、taro 和表单)中更新了日期逻辑。除此之外,新增了类型定义文件和工具函数文件以支持日期操作与格式化。 Changes
Sequence Diagram(s)sequenceDiagram
participant 用户
participant DatePicker 组件
participant useDatePicker Hook
participant Picker 组件
用户->>DatePicker: 点击打开日期选择器
DatePicker->>useDatePicker: 获取默认日期与描述
DatePicker->>Picker: 传递参数并显示 Picker
用户->>Picker: 选择日期
Picker->>DatePicker: 返回选择结果
DatePicker->>DatePicker: 调用 handleConfirm 处理确认逻辑
DatePicker->>用户: 返回确认后的日期数据
Possibly related PRs
Poem
✨ Finishing Touches
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (10)
src/packages/datepicker/demos/h5/demo8.tsx (1)
10-10
: 建议优化日期时间格式化逻辑建议将日期时间格式化逻辑抽取为一个独立的函数,以提高代码的可维护性和复用性。
+const formatDateTime = (date: Date, withTime = true) => { + const year = date.getFullYear() + const month = date.getMonth() + 1 + const day = date.getDate() + return withTime + ? `${year}年${month}月${day}日 06时` + : `${year}-${month}-${day} 06:00` +} const defaultDescription = formatDateTime(defaultValue, false) const [desc, setDesc] = useState(formatDateTime(defaultValue))Also applies to: 12-15
src/packages/form/demos/h5/demo7.tsx (1)
133-152
: 建议添加日期选择的表单验证当前实现缺少对日期选择的必填验证。建议添加表单验证规则。
<Form.Item label="DatePicker" name="DatePicker" + rules={[{ required: true, message: '请选择日期' }]} trigger="onConfirm" // ... 其他属性 >
src/packages/form/demos/taro/demo7.tsx (1)
121-154
: DatePicker 表单集成实现完整,但存在调试代码表单集成实现了完整的功能,包括初始值设置、值转换和触发器配置。但是 getValueFromEvent 中存在调试用的 console.log。
建议移除调试代码:
getValueFromEvent={(...args) => { - console.log('sssss', args[0]) return new Date(args[1].join('/')) }}
src/packages/datepicker/demos/h5/demo1.tsx (1)
1-85
: 建议减少 H5 和 Taro 示例代码的重复H5 和 Taro 示例代码存在大量重复,建议将共同逻辑抽取到共享文件中。
建议创建共享文件:
+ // src/packages/datepicker/demos/shared/useDatePickerDemo.ts + import { useState } from 'react' + import type { PickerOption } from '@nutui/nutui-react' + + export const useDatePickerDemo = (defaultDate: Date) => { + const { defaultDesc: defaultDesc1, defaultValue: defaultValue1 } = useDatePicker(defaultDate) + const { defaultDesc: defaultDesc2, defaultValue: defaultValue2 } = useDatePicker(defaultDate) + + const [show1, setShow1] = useState(false) + const [desc1, setDesc1] = useState(defaultDesc1) + const [value, setValue] = useState(defaultValue2) + const [show2, setShow2] = useState(false) + const [desc2, setDesc2] = useState(defaultDesc2) + + const handleConfirm = // ... 现有的 handleConfirm 实现 + + return { + show1, setShow1, desc1, value, show2, setShow2, desc2, + handleConfirm1: handleConfirm(setDesc1), + handleConfirm2: handleConfirm(setDesc2, setValue), + defaultValue1, defaultValue2 + } + }然后在 H5 和 Taro 示例中复用这个 Hook:
const Demo1 = () => { const { show1, setShow1, desc1, value, show2, setShow2, desc2, handleConfirm1, handleConfirm2, defaultValue1, defaultValue2 } = useDatePickerDemo(new Date()) return ( // ... 现有的 JSX ) }src/packages/datepicker/types.ts (2)
55-55
: 建议优化 children 属性的类型定义。当前
children
属性使用any
类型过于宽松,建议根据组件的实际使用场景限制类型:- children?: any + children?: ((value: number) => React.ReactNode) | React.ReactNode
11-56
: 建议为接口属性添加 JSDoc 文档注释。为了提高代码的可维护性和可读性,建议为
DatePickerProps
接口的关键属性添加详细的文档注释,包括:
- 属性的用途
- 可选值说明
- 默认值
- 使用示例
示例:
/** * DatePicker 组件的属性接口 */ export interface DatePickerProps extends BasicComponent { /** * 当前选中的日期值 * @default undefined */ value?: Date /** * 默认选中的日期值 * @default undefined */ defaultValue?: Date // ... 其他属性 }src/packages/datepicker/datepicker.tsx (2)
77-79
: 建议使用 useMemo 优化性能。
pickerValue
和pickerOptions
的状态可以使用useMemo
进行优化,避免不必要的重新计算:- const [pickerValue, setPickerValue] = useState<(string | number)[]>([]) - const [pickerOptions, setPickerOptions] = useState<PickerOption[][]>([]) + const pickerValue = useMemo(() => [], []) + const pickerOptions = useMemo(() => generatePickerColumns(), [innerDate, startDate, endDate]) - useEffect(() => { - setPickerOptions(generatePickerColumns()) - }, [innerDate, startDate, endDate])Also applies to: 214-220
137-145
: 建议优化事件处理函数。
handleCancel
和handleClose
函数可以使用useCallback
进行优化,避免在每次渲染时重新创建:- const handleCancel = () => { + const handleCancel = useCallback(() => { setInnerDate(selectedDate) onCancel?.() - } + }, [selectedDate, onCancel]) - const handleClose = () => { + const handleClose = useCallback(() => { setInnerVisible(false) onClose?.() - } + }, [setInnerVisible, onClose])src/packages/datepicker/utils.ts (2)
273-364
: 建议拆分 handlePickerValueChange 函数以提高可维护性。当前函数较长且复杂,建议拆分为更小的子函数:
// 处理日期类型的值变化 function handleDateTypeChange( selectedValue: (string | number)[], defaultDate: Date, rangeType: string ): Date | null { // ... 日期类型的处理逻辑 } // 处理时间类型的值变化 function handleTimeTypeChange( selectedValue: (string | number)[], defaultDate: Date, rangeType: string ): Date { // ... 时间类型的处理逻辑 } // 主函数 export const handlePickerValueChange = ( selectedOptions: PickerOption[], selectedValue: (string | number)[], index: number, type: string, defaultDate: Date, handleDateComparison: (newDate: Date | null, selectedOptions: PickerOption[], index: number) => void ) => { const rangeType = type.toLocaleLowerCase() const date = ['date', 'datetime', 'datehour', 'month-day', 'year-month'].includes(rangeType) ? handleDateTypeChange(selectedValue, defaultDate, rangeType) : handleTimeTypeChange(selectedValue, defaultDate, rangeType) handleDateComparison(date, selectedOptions, index) }
258-260
: 建议增强错误处理。当日期值无效时,直接使用 startDate 可能不是最佳选择。建议添加更详细的错误处理:
- if (!value || (value && !isDate(value))) { - value = startDate - } + if (!value) { + console.warn('DatePicker: value is null or undefined, using startDate as fallback') + value = startDate + } else if (!isDate(value)) { + console.warn('DatePicker: invalid date value provided, using startDate as fallback') + value = startDate + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/packages/datepicker/datepicker.taro.tsx
(5 hunks)src/packages/datepicker/datepicker.tsx
(5 hunks)src/packages/datepicker/demos/h5/demo1.tsx
(3 hunks)src/packages/datepicker/demos/h5/demo2.tsx
(1 hunks)src/packages/datepicker/demos/h5/demo8.tsx
(2 hunks)src/packages/datepicker/demos/taro/demo1.tsx
(3 hunks)src/packages/datepicker/demos/taro/demo2.tsx
(2 hunks)src/packages/datepicker/index.taro.ts
(1 hunks)src/packages/datepicker/index.ts
(1 hunks)src/packages/datepicker/types.ts
(1 hunks)src/packages/datepicker/utils.ts
(1 hunks)src/packages/form/demos/h5/demo7.tsx
(2 hunks)src/packages/form/demos/taro/demo7.tsx
(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/packages/datepicker/index.taro.ts
🧰 Additional context used
🪛 GitHub Check: build
src/packages/datepicker/datepicker.taro.tsx
[failure] 228-228:
Type '{ title: string; visible: true; value: (string | number)[]; options: PickerOption[][]; onClose: () => void; onCancel: () => void; onConfirm: (options: PickerOption[], value: (string | number)[]) => void; ... 9 more ...; popupProps?: Partial<...> | undefined; }' is not assignable to type 'Partial'.
🪛 GitHub Actions: CI
src/packages/datepicker/datepicker.taro.tsx
[error] 228-228: Type '{ title: string; visible: true; value: (string | number)[]; options: PickerOption[][]; onClose: () => void; onCancel: () => void; onConfirm: (options: PickerOption[], value: (string | number)[]) => void; ... 9 more ...; popupProps?: Partial<...> | undefined; }' is not assignable to type 'Partial'.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (5)
src/packages/datepicker/demos/h5/demo8.tsx (1)
49-49
: 更新标题以反映新的选择范围标题已更新为"选择年月日时",这与组件的实际功能相符。
src/packages/datepicker/demos/h5/demo2.tsx (1)
23-25
: 存在相同的日期处理问题此组件与 Taro 版本存在相同的日期范围处理问题。
请参考 Taro demo2 中的优化建议。
src/packages/datepicker/index.ts (1)
1-3
: 导入和导出语句的重构优化了代码组织结构将类型定义移至单独的 types 文件,同时使用默认导入导出模式,这样的改动提高了代码的模块化程度。
src/packages/datepicker/demos/taro/demo1.tsx (1)
5-16
: 优秀的自定义 Hook 封装useDatePicker Hook 很好地封装了日期相关的计算逻辑,提高了代码复用性。
src/packages/datepicker/datepicker.taro.tsx (1)
227-243
: 建议应用与 datepicker.tsx 相同的优化。为保持代码一致性,建议应用以下优化:
- 修复 Picker 组件的类型错误
- 使用 useMemo 优化 pickerValue 和 pickerOptions
- 使用 useCallback 优化事件处理函数
🧰 Tools
🪛 GitHub Check: build
[failure] 228-228:
Type '{ title: string; visible: true; value: (string | number)[]; options: PickerOption[][]; onClose: () => void; onCancel: () => void; onConfirm: (options: PickerOption[], value: (string | number)[]) => void; ... 9 more ...; popupProps?: Partial<...> | undefined; }' is not assignable to type 'Partial'.🪛 GitHub Actions: CI
[error] 228-228: Type '{ title: string; visible: true; value: (string | number)[]; options: PickerOption[][]; onClose: () => void; onCancel: () => void; onConfirm: (options: PickerOption[], value: (string | number)[]) => void; ... 9 more ...; popupProps?: Partial<...> | undefined; }' is not assignable to type 'Partial'.
startDate={new Date()} | ||
endDate={new Date(`${defaultValue.getFullYear()}-07-01`)} | ||
defaultValue={defaultValue} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
建议优化日期范围的处理方式
当前的日期范围设置可能存在以下问题:
- 每次组件重新渲染都会创建新的 Date 对象
- 结束日期的字符串拼接方式可能导致无效日期
建议如下优化:
+const getEndDate = (year: number) => {
+ return new Date(year, 6, 1) // 使用月份索引 6 表示七月
+}
-startDate={new Date()}
-endDate={new Date(`${defaultValue.getFullYear()}-07-01`)}
+startDate={defaultValue}
+endDate={getEndDate(defaultValue.getFullYear())}
📝 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.
startDate={new Date()} | |
endDate={new Date(`${defaultValue.getFullYear()}-07-01`)} | |
defaultValue={defaultValue} | |
+const getEndDate = (year: number) => { | |
+ return new Date(year, 6, 1) // 使用月份索引 6 表示七月 | |
+} | |
startDate={defaultValue} | |
endDate={getEndDate(defaultValue.getFullYear())} | |
defaultValue={defaultValue} |
getValueFromEvent={(...args) => { | ||
console.log('sssss', args[0]) | ||
return new Date(args[1].join('/')) | ||
}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
需要移除调试代码并优化日期处理
- 请移除生产环境中的 console.log
- 日期构造可能不安全,建议添加错误处理
getValueFromEvent={(...args) => {
- console.log('sssss', args[0])
- return new Date(args[1].join('/'))
+ try {
+ const dateStr = args[1].join('/')
+ const date = new Date(dateStr)
+ if (isNaN(date.getTime())) {
+ throw new Error('Invalid date')
+ }
+ return date
+ } catch (error) {
+ console.error('Date parsing error:', error)
+ return null
+ }
}}
📝 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.
getValueFromEvent={(...args) => { | |
console.log('sssss', args[0]) | |
return new Date(args[1].join('/')) | |
}} | |
getValueFromEvent={(...args) => { | |
try { | |
const dateStr = args[1].join('/') | |
const date = new Date(dateStr) | |
if (isNaN(date.getTime())) { | |
throw new Error('Invalid date') | |
} | |
return date | |
} catch (error) { | |
console.error('Date parsing error:', error) | |
return null | |
} | |
}} |
const handleConfirm = | ||
(setDesc: (desc: string) => void, setValue?: (value: string) => void) => | ||
(options: PickerOption[], values: (string | number)[]) => { | ||
if (setValue) { | ||
if (isEqual(values, ['2026', '02', '21'])) { | ||
setValue('2026/03/22') | ||
setDesc('2026年03月22日') | ||
} else { | ||
setValue(values.join('/')) | ||
setDesc(options.map((option) => option.text).join('')) | ||
} | ||
} else { | ||
setDesc(options.map((option) => option.text).join('')) | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
handleConfirm 中的硬编码日期比较需要重构
handleConfirm 函数中存在硬编码的日期比较 ['2026', '02', '21'],这可能导致维护问题。
建议将特殊日期处理逻辑抽取为配置:
+ const SPECIAL_DATES = {
+ '2026-02-21': {
+ value: '2026/03/22',
+ desc: '2026年03月22日'
+ }
+ }
const handleConfirm =
(setDesc: (desc: string) => void, setValue?: (value: string) => void) =>
(options: PickerOption[], values: (string | number)[]) => {
if (setValue) {
- if (isEqual(values, ['2026', '02', '21'])) {
+ const dateKey = values.join('-')
+ if (SPECIAL_DATES[dateKey]) {
+ setValue(SPECIAL_DATES[dateKey].value)
+ setDesc(SPECIAL_DATES[dateKey].desc)
} else {
setValue(values.join('/'))
setDesc(options.map((option) => option.text).join(''))
}
} else {
setDesc(options.map((option) => option.text).join(''))
}
}
📝 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 handleConfirm = | |
(setDesc: (desc: string) => void, setValue?: (value: string) => void) => | |
(options: PickerOption[], values: (string | number)[]) => { | |
if (setValue) { | |
if (isEqual(values, ['2026', '02', '21'])) { | |
setValue('2026/03/22') | |
setDesc('2026年03月22日') | |
} else { | |
setValue(values.join('/')) | |
setDesc(options.map((option) => option.text).join('')) | |
} | |
} else { | |
setDesc(options.map((option) => option.text).join('')) | |
} | |
} | |
const SPECIAL_DATES = { | |
'2026-02-21': { | |
value: '2026/03/22', | |
desc: '2026年03月22日' | |
} | |
} | |
const handleConfirm = | |
(setDesc: (desc: string) => void, setValue?: (value: string) => void) => | |
(options: PickerOption[], values: (string | number)[]) => { | |
if (setValue) { | |
const dateKey = values.join('-') | |
if (SPECIAL_DATES[dateKey]) { | |
setValue(SPECIAL_DATES[dateKey].value) | |
setDesc(SPECIAL_DATES[dateKey].desc) | |
} else { | |
setValue(values.join('/')) | |
setDesc(options.map((option) => option.text).join('')) | |
} | |
} else { | |
setDesc(options.map((option) => option.text).join('')) | |
} | |
} |
<Picker | ||
{...pickerProps} | ||
title={title} | ||
visible={innerVisible} | ||
value={pickerValue} | ||
options={pickerOptions} | ||
onClose={handleClose} | ||
onCancel={handleCancel} | ||
onConfirm={handleConfirm} | ||
onChange={( | ||
options: PickerOption[], | ||
value: (string | number)[], | ||
index: number | ||
) => handleChange(options, value, index)} | ||
threeDimensional={threeDimensional} | ||
/> | ||
)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
修复 Picker 组件的类型错误。
根据静态分析工具的提示,Picker 组件的属性类型存在不匹配问题。建议修复类型定义:
- <Picker
+ <Picker<PickerOption>
📝 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.
<Picker | |
{...pickerProps} | |
title={title} | |
visible={innerVisible} | |
value={pickerValue} | |
options={pickerOptions} | |
onClose={handleClose} | |
onCancel={handleCancel} | |
onConfirm={handleConfirm} | |
onChange={( | |
options: PickerOption[], | |
value: (string | number)[], | |
index: number | |
) => handleChange(options, value, index)} | |
threeDimensional={threeDimensional} | |
/> | |
)} | |
<Picker<PickerOption> | |
{...pickerProps} | |
title={title} | |
visible={innerVisible} | |
value={pickerValue} | |
options={pickerOptions} | |
onClose={handleClose} | |
onCancel={handleCancel} | |
onConfirm={handleConfirm} | |
onChange={( | |
options: PickerOption[], | |
value: (string | number)[], | |
index: number | |
) => handleChange(options, value, index)} | |
threeDimensional={threeDimensional} | |
/> | |
)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/packages/datepicker/utils.ts (3)
115-145
: 可考虑对无效时间戳进一步完善判断。
当前只在第 127 行对!selectedDate
做简单判断,若传入一个无效时间戳依旧会得到Invalid Date
。可结合isNaN(date.getTime())
进行更严格校验,提升健壮性。
197-229
: 为避免意外类型映射失败,可考虑在zhCNType
查找不到对应键时增加兜底逻辑。
当前若传入类型未包含在zhCNType
中,最终中文后缀可能为空,使用者可能无法分辨。可在此处添加默认文案或日志提示。
247-333
: 方法体较长,建议在将来考虑拆分以增强可读性。
handlePickerValueChange
里包含多层判断与分支,逻辑庞大。可提取部分通用或分支逻辑到独立函数,以便后续维护、测试和扩展。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/packages/datepicker/utils.ts
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (5)
src/packages/datepicker/utils.ts (5)
1-4
: 导入语句看起来很规范,暂不发现问题。
5-13
: 函数逻辑和实现都较为简洁,满足获取某月最后一天的需求。
70-78
: 建议更严格地校验selectedDate
是否有效。
这里仅通过if (!selected) return []
判断,new Date()
即使参数无效也会返回对象但可能是Invalid Date
状态。建议通过isNaN(new Date(selectedDate).getTime())
或类似方式校验,以免出现潜在的无效日期问题。
157-185
: 需要留意minuteStep
的取值范围以防止死循环。
若minuteStep
不大于 0,while
循环不会前进,可能导致死循环场景。建议在调用或生成列数据前,对minuteStep
进行限制或抛出异常。
230-245
: 日期校验逻辑清晰,边界数值处理到位。
若value
不合法则回退到startDate
,并按照startDate
和endDate
进行夹取,满足常见的日期范围限制需求。
// 返回边界值的对象 | ||
return { | ||
[`${type}Year`]: year, | ||
[`${type}Month`]: month, | ||
[`${type}Date`]: date, | ||
[`${type}Hour`]: hour, | ||
[`${type}Minute`]: minute, | ||
[`${type}Seconds`]: minute, // 返回秒数(与分钟相同) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
疑似获取秒数逻辑存在问题。
函数第 62 行直接将秒数赋值为 minute
,可能导致秒数计算不准确。如需同时限制秒数,可考虑使用 boundary.getSeconds()
或根据需求精确计算,避免秒数固定等于分钟。
下面的示例 diff 展示了如何将秒数同步为边界时间的秒数:
- [`${type}Seconds`]: minute, // 返回秒数(与分钟相同)
+ [`${type}Seconds`]: boundary.getSeconds(), // 应与边界秒数保持一致
📝 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.
// 返回边界值的对象 | |
return { | |
[`${type}Year`]: year, | |
[`${type}Month`]: month, | |
[`${type}Date`]: date, | |
[`${type}Hour`]: hour, | |
[`${type}Minute`]: minute, | |
[`${type}Seconds`]: minute, // 返回秒数(与分钟相同) | |
} | |
// 返回边界值的对象 | |
return { | |
[`${type}Year`]: year, | |
[`${type}Month`]: month, | |
[`${type}Date`]: date, | |
[`${type}Hour`]: hour, | |
[`${type}Minute`]: minute, | |
[`${type}Seconds`]: boundary.getSeconds(), // 应与边界秒数保持一致 | |
} |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feat_v3.x #3013 +/- ##
=============================================
+ Coverage 85.82% 85.87% +0.04%
=============================================
Files 281 282 +1
Lines 18494 18592 +98
Branches 2803 2815 +12
=============================================
+ Hits 15873 15966 +93
- Misses 2616 2621 +5
Partials 5 5 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/packages/picker/picker.taro.tsx (1)
340-353
:⚠️ Potential issue修复 Popup 组件类型错误
CI 管道显示 Popup 组件的属性类型存在不匹配问题。需要确保传递给 Popup 组件的属性符合其类型定义。
建议修改如下:
<Popup - {...popupProps} + {...(popupProps as PopupProps)} visible={innerVisible} position="bottom" onOverlayClick={() => { if (closeOnOverlayClick) { props.onCancel?.() setInnerVisible(false) } }} afterClose={() => { afterClose?.(setSelectedOptions(), innerValue, pickerRef) }} >🧰 Tools
🪛 GitHub Check: build
[failure] 340-340:
Type '{ children: Element; visible: boolean; position: "bottom"; onOverlayClick: () => void; afterClose: () => void; onClick?: ((event: MouseEvent<Element, MouseEvent>) => void) | undefined; ... 20 more ...; onOpen?: (() => void) | undefined; }' is not assignable to type 'Partial'.🪛 GitHub Actions: CI
[error] 340-340: Type '{ children: Element; visible: boolean; position: "bottom"; onOverlayClick: () => void; afterClose: () => void; onClick?: ((event: MouseEvent<Element, MouseEvent>) => void) | undefined; ... 20 more ...; onOpen?: (() => void) | undefined; }' is not assignable to type 'Partial'.
🧹 Nitpick comments (7)
src/packages/datepicker/__test__/datepicker.spec.tsx (4)
7-27
: 建议增强中文本地化测试用例当前测试用例仅验证了基本的中文显示功能。建议添加以下场景的测试:
- 不同日期类型的中文显示
- 自定义中文格式
- 边界值的中文显示
+test('Show Chinese with different date types', async () => { + const confirm = vi.fn() + const { container } = render( + <DatePicker + title="时间选择" + visible + type="datetime" + defaultValue={new Date(currentYear, 11, 31, 23, 59)} + showChinese + threeDimensional={false} + onConfirm={(options) => confirm(options)} + /> + ) + + const confirmBtn = container.querySelectorAll('.nut-picker-confirm-btn')[0] + fireEvent.click(confirmBtn) + await waitFor(() => { + expect( + confirm.mock.calls[0][0].map((option: any) => option.text).join('') + ).toEqual(`${currentYear}年12月31日23时59分`) + }) +})
134-154
: 建议增加错误处理测试用例当前的默认值测试仅覆盖了正常情况。建议添加以下场景的测试:
- 无效日期值处理
- 超出范围的默认值处理
- null/undefined 默认值处理
+test('should handle invalid defaultValue', async () => { + const confirm = vi.fn() + const { container } = render( + <DatePicker + title="日期选择" + visible + defaultValue={new Date('invalid')} + startDate={new Date(2020, 0, 1)} + endDate={new Date(2022, 0, 1)} + onConfirm={(options, values) => confirm(options)} + /> + ) + + const confirmBtn = container.querySelectorAll('.nut-picker-confirm-btn')[0] + fireEvent.click(confirmBtn) + await waitFor(() => { + expect(confirm).toHaveBeenCalled() + const selectedDate = new Date(confirm.mock.calls[0][1]) + expect(selectedDate >= new Date(2020, 0, 1)).toBeTruthy() + expect(selectedDate <= new Date(2022, 0, 1)).toBeTruthy() + }) +})
156-172
: 建议完善时间步进测试当前测试仅验证了分钟步进的选项数量。建议增加以下测试:
- 验证步进值的正确性
- 边界值测试
- 自定义步进值的有效性验证
+test('should validate minute step values', async () => { + const { container } = render( + <DatePicker + title="时间选择" + visible + type="time" + minuteStep={5} + defaultValue={new Date(2022, 0, 1, 12, 0)} + /> + ) + + const minuteColumn = container.querySelectorAll('.nut-picker-list')[1] + const minuteItems = minuteColumn.querySelectorAll('.nut-picker-roller-item-title') + minuteItems.forEach((item, index) => { + expect(Number(item.textContent)).toBe(index * 5) + }) +})
174-195
: 建议增加过滤器边界测试当前的过滤器测试较为基础。建议添加以下测试场景:
- 多个时间单位的组合过滤
- 过滤后的默认值处理
- 过滤条件变更时的重新渲染
+test('should handle multiple filters', async () => { + const filter = (type: string, options: any[]) => { + switch (type) { + case 'hour': + return options.filter((option) => Number(option.value) % 6 === 0) + case 'minute': + return options.filter((option) => Number(option.value) % 15 === 0) + default: + return options + } + } + + const { container } = render( + <DatePicker + title="时间选择" + visible + type="time" + filter={filter} + /> + ) + + const hourColumn = container.querySelectorAll('.nut-picker-list')[0] + const minuteColumn = container.querySelectorAll('.nut-picker-list')[1] + + expect(hourColumn.querySelectorAll('.nut-picker-roller-item').length).toBe(4) + expect(minuteColumn.querySelectorAll('.nut-picker-roller-item').length).toBe(4) +})src/packages/picker/picker.taro.tsx (1)
162-164
: 优化条件判断逻辑这里的条件判断存在冗余。
建议简化为:
- if (!innerValue.length && innerValue.length === 0) { + if (!innerValue.length) { setInnerValue([...data]) }src/packages/picker/picker.tsx (2)
159-161
: 优化条件判断逻辑与 Taro 版本类似,这里的条件判断也存在冗余。
建议简化为:
- if (!innerValue.length && innerValue.length === 0) { + if (!innerValue.length) { setInnerValue([...data]) }
282-295
: 建议使用 React.memo 优化性能标题栏组件可以使用 React.memo 进行优化,因为它的渲染只依赖于少量 props。
建议将 renderTitleBar 改为独立组件:
const PickerTitle = React.memo(({ title, onCancel, onConfirm, locale }: PickerTitleProps) => { return ( <div className="nut-picker-control"> <span className="nut-picker-cancel-btn" onClick={onCancel}> {locale?.cancel} </span> <div className="nut-picker-title">{title || ''}</div> <span className="nut-picker-confirm-btn" onClick={onConfirm}> {locale.confirm} </span> </div> ) })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/packages/datepicker/__test__/datepicker.spec.tsx
(1 hunks)src/packages/datepicker/types.ts
(1 hunks)src/packages/picker/index.taro.ts
(1 hunks)src/packages/picker/index.ts
(1 hunks)src/packages/picker/picker.taro.tsx
(1 hunks)src/packages/picker/picker.tsx
(1 hunks)src/packages/picker/types.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/packages/picker/index.ts
- src/packages/picker/index.taro.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/packages/datepicker/types.ts
🧰 Additional context used
🪛 GitHub Actions: CI
src/packages/picker/picker.taro.tsx
[error] 340-340: Type '{ children: Element; visible: boolean; position: "bottom"; onOverlayClick: () => void; afterClose: () => void; onClick?: ((event: MouseEvent<Element, MouseEvent>) => void) | undefined; ... 20 more ...; onOpen?: (() => void) | undefined; }' is not assignable to type 'Partial'.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (1)
src/packages/datepicker/__test__/datepicker.spec.tsx (1)
4-4
: 导入语句的更改符合组件重构导入语句从命名导入更改为默认导入,这与 DatePicker 组件重构为 forwardRef 组件的变更保持一致。
export interface PickerProps extends Omit<BasicComponent, 'children'> { | ||
visible?: boolean | undefined | ||
title?: string | ||
options: (PickerOption | PickerOption[])[] | ||
value?: (number | string)[] | ||
defaultValue?: (number | string)[] | ||
threeDimensional?: boolean | ||
duration: number | string | ||
closeOnOverlayClick: boolean | ||
popupProps: Partial< | ||
Omit<PopupProps, 'title' | 'onClose' | 'closeOnOverlayClick'> | ||
> | ||
onConfirm?: ( | ||
selectedOptions: PickerOption[], | ||
selectedValue: (string | number)[] | ||
) => void | ||
onCancel?: () => void | ||
onClose?: ( | ||
selectedOptions: PickerOption[], | ||
selectedValue: (string | number)[] | ||
) => void | ||
afterClose?: ( | ||
selectedOptions: PickerOption[], | ||
selectedValue: (string | number)[], | ||
pickerRef: RefObject<HTMLDivElement> | ||
) => void | ||
onChange?: ( | ||
selectedOptions: PickerOption[], | ||
selectedValue: (string | number)[], | ||
columnIndex: number | ||
) => void | ||
children?: any | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
优化 PickerProps 接口定义
该接口存在以下需要改进的地方:
- 接口使用
Omit<BasicComponent, 'children'>
移除了children
,但又在最后添加了children?: any
,这样的设计显得矛盾。 - 一些关键属性应该设置为必需。
建议按照以下方式修改:
export interface PickerProps extends Omit<BasicComponent, 'children'> {
visible?: boolean | undefined
title?: string
- options: (PickerOption | PickerOption[])[]
+ options: (PickerOption | PickerOption[])[] // 保持必需
value?: (number | string)[]
defaultValue?: (number | string)[]
threeDimensional?: boolean
- duration: number | string
- closeOnOverlayClick: boolean
+ duration?: number | string
+ closeOnOverlayClick?: boolean
popupProps: Partial<
Omit<PopupProps, 'title' | 'onClose' | 'closeOnOverlayClick'>
>
// ... 其他回调函数保持不变 ...
- children?: any
+ children?: React.ReactNode
}
📝 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 interface PickerProps extends Omit<BasicComponent, 'children'> { | |
visible?: boolean | undefined | |
title?: string | |
options: (PickerOption | PickerOption[])[] | |
value?: (number | string)[] | |
defaultValue?: (number | string)[] | |
threeDimensional?: boolean | |
duration: number | string | |
closeOnOverlayClick: boolean | |
popupProps: Partial< | |
Omit<PopupProps, 'title' | 'onClose' | 'closeOnOverlayClick'> | |
> | |
onConfirm?: ( | |
selectedOptions: PickerOption[], | |
selectedValue: (string | number)[] | |
) => void | |
onCancel?: () => void | |
onClose?: ( | |
selectedOptions: PickerOption[], | |
selectedValue: (string | number)[] | |
) => void | |
afterClose?: ( | |
selectedOptions: PickerOption[], | |
selectedValue: (string | number)[], | |
pickerRef: RefObject<HTMLDivElement> | |
) => void | |
onChange?: ( | |
selectedOptions: PickerOption[], | |
selectedValue: (string | number)[], | |
columnIndex: number | |
) => void | |
children?: any | |
} | |
export interface PickerProps extends Omit<BasicComponent, 'children'> { | |
visible?: boolean | undefined | |
title?: string | |
options: (PickerOption | PickerOption[])[] | |
value?: (number | string)[] | |
defaultValue?: (number | string)[] | |
threeDimensional?: boolean | |
duration?: number | string | |
closeOnOverlayClick?: boolean | |
popupProps: Partial< | |
Omit<PopupProps, 'title' | 'onClose' | 'closeOnOverlayClick'> | |
> | |
onConfirm?: ( | |
selectedOptions: PickerOption[], | |
selectedValue: (string | number)[] | |
) => void | |
onCancel?: () => void | |
onClose?: ( | |
selectedOptions: PickerOption[], | |
selectedValue: (string | number)[] | |
) => void | |
afterClose?: ( | |
selectedOptions: PickerOption[], | |
selectedValue: (string | number)[], | |
pickerRef: RefObject<HTMLDivElement> | |
) => void | |
onChange?: ( | |
selectedOptions: PickerOption[], | |
selectedValue: (string | number)[], | |
columnIndex: number | |
) => void | |
children?: React.ReactNode | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (2)
src/packages/picker/picker.taro.tsx (2)
178-185
:⚠️ Potential issueuseEffect 依赖项不完整
useEffect 的依赖数组中缺少了
onChange
和columnIndex
,这可能导致回调函数不会在这些值变化时执行。建议修改为:
useEffect(() => { if (!innerVisible) { return } Taro.getEnv() !== 'WEB' && setCurrentValue(defaultValuesConvert()) onChange && onChange(setSelectedOptions(), innerValue, columnIndex) -}, [innerValue, columnsList, innerVisible]) +}, [innerValue, columnsList, innerVisible, onChange, columnIndex])
203-222
:⚠️ Potential issue避免在 defaultValuesConvert 中直接修改状态
defaultValuesConvert 函数中直接修改了 selectedValue 状态,这可能导致意外的副作用。
建议修改为:
const defaultValuesConvert = () => { const defaultIndexs: number[] = [] if (innerValue.length > 0) { innerValue.forEach((value, index) => { for (let i = 0; i < columnsList?.[index]?.length; i++) { if (columnsList[index][i].value === value) { defaultIndexs.push(i) break } } }) } else if (columnsList && columnsList.length > 0) { columnsList.forEach((item) => { defaultIndexs.push(0) - item.length > 0 && selectedValue.push(item[0].value) + if (item.length > 0) { + setSelectedValue(prev => [...prev, item[0].value]) + } }) } return defaultIndexs }
🧹 Nitpick comments (4)
src/packages/datepicker/types.taro.ts (1)
4-18
: 建议添加类型文档注释类型定义的结构很好,但建议添加 JSDoc 注释来说明每个属性的用途,这样可以提供更好的 IDE 支持和代码提示。
建议添加如下文档注释:
+/** + * DatePicker 组件的属性定义 + * @extends {Omit<DatePickerWebProps, 'pickerProps'>} + */ export type DatePickerProps = Omit<DatePickerWebProps, 'pickerProps'> & { + /** + * Picker 组件的可选配置属性 + */ pickerProps: Partial< Omit< PickerProps, | 'defaultValue' | 'threeDimensional' | 'title' | 'value' | 'onConfirm' | 'onClose' | 'onCancel' | 'onChange' > > }src/packages/picker/picker.taro.tsx (1)
225-263
: 建议拆分 chooseItem 函数chooseItem 函数的复杂度较高,建议将级联和非级联的逻辑拆分为独立的函数,以提高可维护性。
建议重构为:
const handleCascadeChange = (columnOptions: PickerOption, start: number) => { const values: any = [] let columnIndex = start values[columnIndex] = columnOptions.value || '' while (columnOptions?.children?.[0]) { values[columnIndex + 1] = columnOptions.children[0].value columnIndex++ columnOptions = columnOptions.children[0] } if (columnOptions?.children?.length) { values[columnIndex + 1] = '' } const combineResult = [...innerValue.slice(0, start), ...values.splice(start)] setInnerValue(combineResult) setColumnsList(normalListData(combineResult) as PickerOption[][]) } const handleNormalChange = (columnOptions: PickerOption, columnIndex: number) => { setInnerValue((data: (number | string)[]) => { const cdata: (number | string)[] = [...data] cdata[columnIndex] = Object.prototype.hasOwnProperty.call( columnOptions, 'value' ) ? columnOptions.value : '' return cdata }) } const chooseItem = (columnOptions: PickerOption, columnIndex: number) => { if (!columnOptions || !Object.keys(columnOptions).length) return if (columnsType() === 'cascade') { handleCascadeChange(columnOptions, columnIndex) } else { handleNormalChange(columnOptions, columnIndex) } setColumnIndex(columnIndex) }src/packages/datepicker/datepicker.taro.tsx (2)
108-128
: 建议简化 handleDateComparison 函数handleDateComparison 函数的逻辑可以简化,并添加错误处理。
建议重构为:
const handleDateComparison = ( newDate: Date | null, selectedOptions: PickerOption[], index: number ) => { if (!newDate || !isDate(newDate)) { console.warn('无效的日期值') return } const currentDate = new Date(innerDate) const isEqual = currentDate?.getTime() === newDate.getTime() if (!isEqual) { setInnerDate(formatValue(newDate, startDate, endDate)) } onChange?.( selectedOptions, [ String(newDate.getFullYear()), String(newDate.getMonth() + 1), String(newDate.getDate()), ], index ) }
179-214
: 建议拆分 generatePickerColumns 函数generatePickerColumns 函数的复杂度较高,建议将列生成逻辑拆分为独立的函数。
建议重构为:
const generateColumn = ( rangeConfig: { type: string; range: [number, number] }, columnIndex: number ): PickerOption[] => { const { type: columnType, range } = rangeConfig const selectedValue = getDatePartValue(columnType, innerDate) const pickerColumn = generatePickerColumnWithCallback( range[0], range[1], selectedValue, columnType, minuteStep, (selectedIndex, options) => { pickerValue[columnIndex] = options[selectedIndex]?.value setPickerValue([...pickerValue]) }, showChinese, zhCNType, formatter ) return filter?.(columnType, pickerColumn) || pickerColumn } const generatePickerColumns = (): PickerOption[][] => { const dateRanges = generateDatePickerRanges( type, innerDate, startDate, endDate ) return dateRanges.map((rangeConfig, columnIndex) => generateColumn(rangeConfig, columnIndex) ) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/packages/datepicker/datepicker.taro.tsx
(5 hunks)src/packages/datepicker/index.taro.ts
(1 hunks)src/packages/datepicker/types.taro.ts
(1 hunks)src/packages/picker/index.taro.ts
(1 hunks)src/packages/picker/picker.taro.tsx
(1 hunks)src/packages/picker/types.taro.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/packages/datepicker/index.taro.ts
- src/packages/picker/index.taro.ts
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: test
- GitHub Check: test
- GitHub Check: build
🔇 Additional comments (1)
src/packages/picker/types.taro.ts (1)
4-8
: 类型定义结构清晰!类型定义使用了 TypeScript 的 Omit 工具类型来正确处理属性继承,并对 popupProps 进行了适当的类型约束。
useEffect(() => { | ||
setInnerDate(selectedDate) | ||
}, [selectedDate]) | ||
|
||
useEffect(() => { | ||
setPickerOptions(columns()) | ||
}, [selectedDate, startDate, endDate]) | ||
setPickerOptions(generatePickerColumns()) | ||
}, [innerDate, startDate, endDate]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
useEffect 依赖项不完整
useEffect 的依赖数组中缺少了一些依赖项,这可能导致更新不及时。
建议修改为:
useEffect(() => {
setPickerOptions(generatePickerColumns())
-}, [innerDate, startDate, endDate])
+}, [
+ innerDate,
+ startDate,
+ endDate,
+ type,
+ minuteStep,
+ showChinese,
+ formatter,
+ filter
+])
📝 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.
useEffect(() => { | |
setInnerDate(selectedDate) | |
}, [selectedDate]) | |
useEffect(() => { | |
setPickerOptions(columns()) | |
}, [selectedDate, startDate, endDate]) | |
setPickerOptions(generatePickerColumns()) | |
}, [innerDate, startDate, endDate]) | |
useEffect(() => { | |
setInnerDate(selectedDate) | |
}, [selectedDate]) | |
useEffect(() => { | |
setPickerOptions(generatePickerColumns()) | |
}, [ | |
innerDate, | |
startDate, | |
endDate, | |
type, | |
minuteStep, | |
showChinese, | |
formatter, | |
filter | |
]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (7)
src/packages/datepicker/datepicker.taro.tsx (4)
169-177
: 条件判断逻辑可以优化在
handleChange
函数中使用条件判断innerVisible &&
嵌套了一个复杂函数调用,这种写法不够清晰。建议修改为:
const handleChange = ( selectedOptions: PickerOption[], selectedValue: (string | number)[], index: number ) => { - innerVisible && - handlePickerValueChange( - selectedOptions, - selectedValue, - index, - type, - defaultValue || startDate || endDate, - handleDateComparison - ) + if (innerVisible) { + handlePickerValueChange( + selectedOptions, + selectedValue, + index, + type, + defaultValue || startDate || endDate, + handleDateComparison + ) + } }
128-137
: 添加日期边界检查提示
handleConfirmDateComparison
函数中可以添加日期超出范围时的友好提示。建议在日期超出
startDate
或endDate
范围时添加适当的警告或提示,以提高用户体验。const handleConfirmDateComparison = (newDate: Date | null) => { if (newDate && isDate(newDate)) { const isEqual = new Date(selectedDate)?.getTime() === newDate?.getTime() if (!isEqual) { + // 检查是否超出范围 + const isOutOfRange = newDate < startDate || newDate > endDate + if (isOutOfRange) { + console.warn('所选日期超出允许范围') + } setSelectedDate(formatValue(newDate, startDate, endDate)) } } }
180-215
: 优化 generatePickerColumns 函数当前
generatePickerColumns
函数每次调用都会重新创建所有列,而这个函数在多个地方被调用。考虑使用 React.useCallback 或 React.useMemo 来缓存这个函数或其结果,减少不必要的重新计算:
- const generatePickerColumns = (): PickerOption[][] => { + const generatePickerColumns = React.useCallback((): PickerOption[][] => { const dateRanges = generateDatePickerRanges( type, innerDate, startDate, endDate ) const columns = dateRanges.map((rangeConfig, columnIndex) => { // 函数内容保持不变... }) return columns || [] - } + }, [type, innerDate, startDate, endDate, minuteStep, showChinese, zhCNType, formatter, filter, pickerValue])
227-247
: 条件渲染优化当
pickerOptions.length
为 0 时,仍然会渲染空的View
元素。考虑优化条件渲染逻辑,当没有选项时可以完全不渲染容器元素:
return ( <> {typeof children === 'function' && children(selectedDate)} - <View className={`nut-datepicker ${className}`} style={style} {...rest}> - {pickerOptions.length && ( + {pickerOptions.length > 0 && ( + <View className={`nut-datepicker ${className}`} style={style} {...rest}> <Picker {...pickerProps} title={title} visible={innerVisible} value={pickerValue} options={pickerOptions} onClose={handleClose} onCancel={handleCancel} onConfirm={handleConfirm} onChange={( options: PickerOption[], value: (string | number)[], index: number ) => handleChange(options, value, index)} threeDimensional={threeDimensional} /> - )} - </View> + </View> + )} </> )src/packages/datepicker/datepicker.tsx (3)
167-176
: 条件判断逻辑可以优化在
handleChange
函数中使用条件判断innerVisible &&
嵌套了一个复杂函数调用,这种写法不够清晰。建议修改为:
const handleChange = ( selectedOptions: PickerOption[], selectedValue: (string | number)[], index: number ) => { - innerVisible && - handlePickerValueChange( - selectedOptions, - selectedValue, - index, - type, - defaultValue || startDate || endDate, - handleDateComparison - ) + if (innerVisible) { + handlePickerValueChange( + selectedOptions, + selectedValue, + index, + type, + defaultValue || startDate || endDate, + handleDateComparison + ) + } }
178-213
: 优化 generatePickerColumns 函数性能当前
generatePickerColumns
函数每次调用都会重新创建所有列,而这个函数在多个地方被调用。考虑使用 React.useCallback 或 React.useMemo 来缓存这个函数或其结果,减少不必要的重新计算:
- const generatePickerColumns = (): PickerOption[][] => { + const generatePickerColumns = React.useCallback((): PickerOption[][] => { const dateRanges = generateDatePickerRanges( type, innerDate, startDate, endDate ) const columns = dateRanges.map((rangeConfig, columnIndex) => { // 函数内容保持不变... }) return columns || [] - } + }, [type, innerDate, startDate, endDate, minuteStep, showChinese, zhCNType, formatter, filter, pickerValue])
225-246
: 优化条件渲染逻辑当前在
pickerOptions.length
为假值时仍然会渲染外层容器元素。建议优化条件渲染,当没有选项时完全不渲染容器:
return ( <> {typeof children === 'function' && children(selectedDate)} - <div className={`nut-datepicker ${className}`} style={style} {...rest}> - {pickerOptions.length && ( + {pickerOptions.length > 0 && ( + <div className={`nut-datepicker ${className}`} style={style} {...rest}> <Picker {...pickerProps} title={title} visible={innerVisible} value={pickerValue} options={pickerOptions} onClose={handleClose} onCancel={handleCancel} onConfirm={handleConfirm} onChange={( options: PickerOption[], value: (string | number)[], index: number ) => handleChange(options, value, index)} threeDimensional={threeDimensional} /> - )} - </div> + </div> + )} </> )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/packages/datepicker/datepicker.taro.tsx
(5 hunks)src/packages/datepicker/datepicker.tsx
(5 hunks)
🧰 Additional context used
🪛 GitHub Check: codecov/patch
src/packages/datepicker/datepicker.tsx
[warning] 97-98: src/packages/datepicker/datepicker.tsx#L97-L98
Added lines #L97 - L98 were not covered by tests
[warning] 100-101: src/packages/datepicker/datepicker.tsx#L100-L101
Added lines #L100 - L101 were not covered by tests
[warning] 132-132: src/packages/datepicker/datepicker.tsx#L132
Added line #L132 was not covered by tests
[warning] 138-140: src/packages/datepicker/datepicker.tsx#L138-L140
Added lines #L138 - L140 were not covered by tests
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (7)
src/packages/datepicker/datepicker.taro.tsx (3)
221-223
: useEffect 依赖项不完整useEffect 的依赖数组中缺少了一些依赖项,这可能导致组件在依赖项变化时不会重新生成选择器列。
建议修改为:
useEffect(() => { setPickerOptions(generatePickerColumns()) -}, [innerDate, startDate, endDate]) +}, [ + innerDate, + startDate, + endDate, + type, + minuteStep, + showChinese, + formatter, + filter +])
89-95
: innerVisible 状态管理的改进
innerVisible
状态使用usePropsValue
处理,但其初始值处理可能存在潜在问题。当props.visible
为undefined
时,会使用defaultValue
作为初始值。请确认这是否符合预期行为。通常 visible 的初始状态应该遵循 visible prop 或默认为 false,而不受 defaultValue 的影响。
95-104
: 缺少测试覆盖静态分析工具显示
actions
对象的方法缺少测试覆盖。这些方法是通过 ref 暴露给外部的重要 API。请确保为这些方法添加适当的单元测试,以确保它们按预期工作。特别是测试
open
和close
方法对innerVisible
状态的影响以及相关回调的触发。src/packages/datepicker/datepicker.tsx (4)
219-221
: useEffect 依赖项不完整useEffect 的依赖数组中缺少了一些依赖项,这可能导致组件在依赖项变化时不会重新生成选择器列。
建议修改为:
useEffect(() => { setPickerOptions(generatePickerColumns()) -}, [innerDate, startDate, endDate]) +}, [ + innerDate, + startDate, + endDate, + type, + minuteStep, + showChinese, + formatter, + filter +])
228-244
: 修复 Picker 组件的类型错误根据静态分析工具的提示,Picker 组件的属性类型存在不匹配问题。
建议修复类型定义:
- <Picker + <Picker<PickerOption>
138-140
: 内部状态复位逻辑
handleCancel
函数中将innerDate
重置为selectedDate
,但该逻辑缺少测试覆盖。确保当用户取消操作时,日期选择器会正确地恢复到先前选择的日期,而不是保留用户在取消前临时选择的值。建议增加相关测试用例。
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 138-140: src/packages/datepicker/datepicker.tsx#L138-L140
Added lines #L138 - L140 were not covered by tests
89-93
: innerVisible 状态管理的初始化逻辑
innerVisible
状态使用usePropsValue
进行管理,需要注意defaultValue
和finalValue
的赋值逻辑。当组件初次渲染且
props.visible
为undefined
时,组件会使用defaultValue
(false)作为初始值。请确认这符合预期行为,并考虑是否需要添加注释说明此行为。
const actions: DatePickerActions = { | ||
open: () => { | ||
setInnerVisible(true) | ||
}, | ||
close: () => { | ||
setInnerVisible(false) | ||
}, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
❓ Verification inconclusive
缺少测试覆盖
静态分析工具显示 actions
对象的方法缺少测试覆盖。这些方法是通过 ref 暴露给外部的重要 API。
请确保为这些方法添加适当的单元测试,以验证它们按预期工作。特别是测试 open
和 close
方法对 innerVisible
状态的影响以及相关回调的触发。
🏁 Script executed:
#!/bin/bash
# 检查是否有针对 DatePicker ref 方法的测试
# 查找测试文件中是否有使用 ref.current.open() 或 ref.current.close() 的测试用例
rg -p "ref\.current\.(open|close)" src/packages/datepicker/__test__
Length of output: 69
请为 DatePicker 组件 ref 暴露的 open 与 close 方法添加单元测试
经过验证,在 src/packages/datepicker/__test__
目录中未发现针对 ref.current.open()
或 ref.current.close()
的测试代码。为了确保这些关键 API 能够正确控制 innerVisible
状态并触发相关回调,请添加以下测试用例:
- 测试
open
方法:调用时应将innerVisible
设置为true
。 - 测试
close
方法:调用时应将innerVisible
设置为false
,并触发相应的回调(如果有)。
请尽快补充这些测试用例以确保组件 API 的稳定性和正确性。
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 97-98: src/packages/datepicker/datepicker.tsx#L97-L98
Added lines #L97 - L98 were not covered by tests
[warning] 100-101: src/packages/datepicker/datepicker.tsx#L100-L101
Added lines #L100 - L101 were not covered by tests
🤔 这个变动的性质是?
🔗 相关 Issue
💡 需求背景和解决方案
☑️ 请求合并前的自查清单
Summary by CodeRabbit
新功能
重构