-
Notifications
You must be signed in to change notification settings - Fork 269
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: calendar #2983
base: next
Are you sure you want to change the base?
refactor: calendar #2983
Conversation
Walkthrough此次变更对日历组件及相关工具函数进行了重构和样式调整。具体修改包括:
Changes
Sequence Diagram(s)sequenceDiagram
participant 用户 as 用户
participant 日历 as CalendarItem
participant 工具 as 工具函数
用户->>日历: 点击日期
日历->>工具: 调用 getDay / compareDate / getNumTwoBit 等
工具-->>日历: 返回日期数据
日历->>日历: 更新状态并重新渲染
Possibly related PRs
Suggested reviewers
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 (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## next #2983 +/- ##
==========================================
+ Coverage 84.19% 84.46% +0.27%
==========================================
Files 267 267
Lines 18099 18021 -78
Branches 2665 2668 +3
==========================================
- Hits 15238 15221 -17
+ Misses 2856 2795 -61
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
🧹 Nitpick comments (8)
src/packages/calendaritem/calendaritem.taro.tsx (5)
194-205
: 优化 getMonthData 的月份遍历逻辑
该方法在循环生成月份时调用了getPreMonthDates
,getDaysStatus
并动态计算cssHeight
,请确认极端场景(例如跨年、多年跨度)能正确生成完整列表,以及考虑性能开销。Also applies to: 210-211, 218-228
370-402
: 在 renderCurrentDate 中处理多类型逻辑时注意可读性
switch 分支较多,各 case 均会调用handleDayClick
。若后续需求增加,建议进一步拆分或封装,便于维护与测试。
421-423
: scrollTop 赋值后立刻 setScrollTop
目前逻辑简单明了,如需在不同生命周期插入插桩逻辑,可酌情封装滚动管理函数。
496-497
: 平滑滚动后立即强制对齐
这两段逻辑在平滑滚动后将滚动条位置校正到准确位置,如对滚动体验有新需求,应考虑进一步的滚动缓冲方案。Also applies to: 502-503
547-572
: getClasses 内部判断分支较多
分支逻辑包含多种状态合并判断,可考虑在分支体内拆分出更多辅助函数,便于今后维护。src/utils/date.ts (1)
49-70
: 建议加强类型安全性
getMonthDays
函数的实现可以更加类型安全:
- 参数类型使用
string
过于宽松- 数组访问使用
as any
不够安全建议如下改进:
-export const getMonthDays = (year: string, month: string): number => { +export const getMonthDays = (year: string, month: string): number => { + const monthNum = parseInt(month.replace(/^0/, '')) + if (isNaN(monthNum) || monthNum < 1 || monthNum > 12) { + throw new Error('Invalid month') + } if (/^0/.test(month)) { month = month.split('')[1] } - return ( - [ - 0, - 31, - isLeapYear(Number(year)) ? 29 : 28, - 31, - 30, - 31, - 30, - 31, - 31, - 30, - 31, - 30, - 31, - ] as number[] - )[month as any] + const monthDays = [ + 0, + 31, + isLeapYear(Number(year)) ? 29 : 28, + 31, + 30, + 31, + 30, + 31, + 31, + 30, + 31, + 30, + 31, + ] + return monthDays[monthNum]🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 51-52: src/utils/date.ts#L51-L52
Added lines #L51 - L52 were not covered by testssrc/packages/calendaritem/calendaritem.tsx (2)
201-242
: 建议拆分复杂的日期处理逻辑
getMonthData
函数较为复杂,建议拆分为更小的函数以提高可维护性:
- 日期计算逻辑
- 滚动位置计算逻辑
- 月份数据组装逻辑
需要我帮你重构这部分代码吗?
607-683
: 建议优化日期选择逻辑
chooseDay
函数包含了多个日期选择模式的处理逻辑,建议:
- 将不同模式的处理逻辑拆分为独立函数
- 使用策略模式处理不同的选择类型
- 添加详细的单元测试
需要我帮你重构这部分代码吗?
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 649-649: src/packages/calendaritem/calendaritem.tsx#L649
Added line #L649 was not covered by tests
[warning] 657-658: src/packages/calendaritem/calendaritem.tsx#L657-L658
Added lines #L657 - L658 were not covered by tests
[warning] 661-661: src/packages/calendaritem/calendaritem.tsx#L661
Added line #L661 was not covered by tests
[warning] 668-669: src/packages/calendaritem/calendaritem.tsx#L668-L669
Added lines #L668 - L669 were not covered by tests
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/packages/calendar/calendar.scss
(1 hunks)src/packages/calendar/calendar.taro.tsx
(2 hunks)src/packages/calendar/calendar.tsx
(2 hunks)src/packages/calendar/demos/taro/demo1.tsx
(0 hunks)src/packages/calendar/utils.tsx
(2 hunks)src/packages/calendarcard/utils.ts
(3 hunks)src/packages/calendaritem/calendaritem.taro.tsx
(15 hunks)src/packages/calendaritem/calendaritem.tsx
(16 hunks)src/utils/date.ts
(3 hunks)
💤 Files with no reviewable changes (1)
- src/packages/calendar/demos/taro/demo1.tsx
🧰 Additional context used
🪛 GitHub Check: codecov/patch
src/packages/calendaritem/calendaritem.tsx
[warning] 294-294: src/packages/calendaritem/calendaritem.tsx#L294
Added line #L294 was not covered by tests
[warning] 297-297: src/packages/calendaritem/calendaritem.tsx#L297
Added line #L297 was not covered by tests
[warning] 312-312: src/packages/calendaritem/calendaritem.tsx#L312
Added line #L312 was not covered by tests
[warning] 314-314: src/packages/calendaritem/calendaritem.tsx#L314
Added line #L314 was not covered by tests
[warning] 328-328: src/packages/calendaritem/calendaritem.tsx#L328
Added line #L328 was not covered by tests
[warning] 330-330: src/packages/calendaritem/calendaritem.tsx#L330
Added line #L330 was not covered by tests
[warning] 333-333: src/packages/calendaritem/calendaritem.tsx#L333
Added line #L333 was not covered by tests
[warning] 491-491: src/packages/calendaritem/calendaritem.tsx#L491
Added line #L491 was not covered by tests
[warning] 493-493: src/packages/calendaritem/calendaritem.tsx#L493
Added line #L493 was not covered by tests
[warning] 585-586: src/packages/calendaritem/calendaritem.tsx#L585-L586
Added lines #L585 - L586 were not covered by tests
[warning] 649-649: src/packages/calendaritem/calendaritem.tsx#L649
Added line #L649 was not covered by tests
[warning] 657-658: src/packages/calendaritem/calendaritem.tsx#L657-L658
Added lines #L657 - L658 were not covered by tests
[warning] 661-661: src/packages/calendaritem/calendaritem.tsx#L661
Added line #L661 was not covered by tests
[warning] 668-669: src/packages/calendaritem/calendaritem.tsx#L668-L669
Added lines #L668 - L669 were not covered by tests
src/utils/date.ts
[warning] 51-52: src/utils/date.ts#L51-L52
Added lines #L51 - L52 were not covered by tests
[warning] 132-146: src/utils/date.ts#L132-L146
Added lines #L132 - L146 were not covered by tests
[warning] 148-159: src/utils/date.ts#L148-L159
Added lines #L148 - L159 were not covered by tests
[warning] 161-169: src/utils/date.ts#L161-L169
Added lines #L161 - L169 were not covered by tests
[warning] 172-172: src/utils/date.ts#L172
Added line #L172 was not covered by tests
[warning] 174-174: src/utils/date.ts#L174
Added line #L174 was not covered by tests
[warning] 176-181: src/utils/date.ts#L176-L181
Added lines #L176 - L181 were not covered by tests
[warning] 183-183: src/utils/date.ts#L183
Added line #L183 was not covered by tests
[warning] 185-185: src/utils/date.ts#L185
Added line #L185 was not covered by tests
[warning] 187-190: src/utils/date.ts#L187-L190
Added lines #L187 - L190 were not covered by tests
[warning] 193-197: src/utils/date.ts#L193-L197
Added lines #L193 - L197 were not covered by tests
src/packages/calendar/utils.tsx
[warning] 12-12: src/packages/calendar/utils.tsx#L12
Added line #L12 was not covered by tests
[warning] 28-28: src/packages/calendar/utils.tsx#L28
Added line #L28 was not covered by tests
[warning] 32-32: src/packages/calendar/utils.tsx#L32
Added line #L32 was not covered by tests
[warning] 37-37: src/packages/calendar/utils.tsx#L37
Added line #L37 was not covered by tests
🪛 Biome (1.9.4)
src/packages/calendaritem/calendaritem.taro.tsx
[error] 299-299: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 301-301: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 317-318: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 319-320: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 582-583: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 632-633: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 674-675: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 675-676: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (24)
src/packages/calendaritem/calendaritem.taro.tsx (14)
8-18
: 直接导入各日期工具函数,提升可读性与可维护性
这些改动通过精细化引入getDay
,compareDate
等工具函数,减少对通用Utils
对象的依赖,有利于 tree-shaking 并让职责更清晰。
81-82
: 注意默认日期范围是否符合业务需求
当前将startDate
默认设为getDay(0)
,endDate
默认设为getDay(365)
,可能在特定业务场景下并不合理;需要确认是否涵盖了所需的日期跨度。
116-116
: 解构 value 属性
从 props 中解构value
并无逻辑问题,但需要注意其与其它默认值逻辑 (defaultValue
) 的兼容性。
140-141
: 统一前缀常量便于后续样式维护
使用classPrefix
与dayPrefix
提升可维护性,这种形式在日后统一维护样式类名时更灵活。
158-159
: 确认回退逻辑是否适用
回退到getDay(0)
与getDay(365)
的逻辑可能在极端场景下不满足需求,建议验证是否需要增加更灵活的处理方式。
404-406
: getter 函数命名清晰,此处实现简单
getMonthsPanel
直接返回monthsPanel.current
,代码可读性尚可。
408-410
: 保持函数式访问 monthsRef,便于后续扩展
getMonthsRef
同理,返回 ref 变量,设计合理。
429-436
: getMonthNum 边界处理
当计算结果为非正数时,以 1 作为默认值能避免异常,但可能导致逻辑与上层预期不符,建议进一步确认需求或加异常提示。
438-452
: initData 逻辑集中
在initData
中一次性调用获取月份数据、默认数据、索引与渲染,流程清晰;但逻辑量大,如后期扩展可能需考虑拆分为更细函数。
473-475
: compareDate 判断区间逻辑需仔细验证
if (compareDate(date, propStartDate)) date = propStartDate
以及else if (!compareDate(date, propEndDate)) date = propEndDate
的对比条件较易混淆,请确保与业务预期一致。
509-509
: monthsViewScroll 函数的计算逻辑较复杂
需仔细测试当月数较多、首次/末次滚动时的定位是否准确;也可考虑通过观察者的方式实时更新current
。Also applies to: 519-519, 521-521, 522-522
539-545
: isDisable 函数根据 day.type 与 compareDate 判断
如果存在更多禁用逻辑(如法定节假日、特殊日期等),需在此统一扩展;也要注意性能影响。
667-667
: classPrefix 赋值保持一致风格
此处与上文命名风格相同,易于维护,推荐继续保持。
674-674
: headerClasses 使用 classNames
结合popup
、showTitle
等条件,简洁表达多重 class 状态,写法可读性较好。🧰 Tools
🪛 Biome (1.9.4)
[error] 674-675: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/packages/calendar/utils.tsx (2)
1-1
: 从 Utils 转为直接导入函数
此变更提升了模块化与可维护性,减少冗余依赖。
12-12
: 缺少单测覆盖
新增/修改的行 (12, 28, 32, 37) 未被测试覆盖,建议添加相关测试用例,确保边界情况(空数组、日期字符串格式不一致等)得到验证。Also applies to: 28-28, 32-32, 37-37
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 12-12: src/packages/calendar/utils.tsx#L12
Added line #L12 was not covered by testssrc/packages/calendarcard/utils.ts (2)
1-1
: 直接导入 getMonthPreDay 与 getMonthDays
避免从Utils
拖入不必要的函数,能更好地控制包体积与维护责任。
32-32
: 确保 off-by-one 情况得到正确处理
在getPrevMonthDays
与getCurrentMonthDays
等函数中,需防范首日或最后一天越界的情况,建议在测试中验证当月天数不满 30 天、跨月份边缘等情形。Also applies to: 38-38, 54-54
src/packages/calendar/calendar.tsx (1)
4-4
: 代码重构改进了模块化!将
Utils
对象的方法改为直接导入具体函数是一个很好的改进:
- 提高了代码的可维护性
- 支持了更好的 tree-shaking
- 使依赖关系更加明确
Also applies to: 47-48
src/packages/calendar/calendar.taro.tsx (1)
4-4
: 保持了与 React 版本的一致性!Taro 版本的改动与 React 版本保持一致,这很好:
- 统一了代码风格
- 保持了两个版本的同步性
- 便于后续维护
Also applies to: 47-48
src/utils/date.ts (2)
1-7
: 代码模块化改进值得肯定!将工具函数从
Utils
对象拆分为独立导出的函数是很好的改进:
- 支持更好的 tree-shaking
- 提高了代码的可维护性
- 使依赖关系更加清晰
294-294
: 需要补充测试用例代码覆盖率报告显示多个新增的日期处理逻辑缺少测试覆盖:
- 日期比较逻辑
- 周处理逻辑
- 格式化逻辑
建议添加相应的单元测试用例。需要我帮你生成测试代码吗?
Also applies to: 297-297, 312-312, 314-314, 328-328, 330-330, 333-333, 491-491, 493-493, 585-586, 649-649, 657-658, 661-661, 668-669
src/packages/calendaritem/calendaritem.tsx (1)
7-18
: 导入优化提高了代码清晰度!直接导入所需的工具函数而不是使用
Utils
对象:
- 使依赖关系更加明确
- 便于代码分析和优化
- 支持更好的 tree-shaking
src/packages/calendar/calendar.scss (1)
197-204
: 代码结构优化得当!将禁用状态的样式移至
choose
类下作为子类是个很好的改进,这样的组织结构更加清晰和符合逻辑。建议验证在所有状态下样式表现是否一致,特别是:
- 日期选择时的禁用状态
- 范围选择时的禁用状态
- RTL模式下的禁用状态
✅ Verification successful
🏁 Script executed:
#!/bin/bash # 检查样式变更是否影响其他组件 rg -l "nut-calendar.*disabled"Length of output: 243
代码修改结构和样式一致性均符合预期!
通过搜索结果我们发现:
- 测试文件(如
src/packages/calendar/__tests__/__snapshots__/calendar.spec.tsx.snap
和src/packages/calendarcard/__tests__/calendarcard.spec.tsx
)以及 demo 文件中均检测到相关的disabled
样式引用,说明该样式被各处使用,并且经过调整后命名结构更清晰;- 虽然目前没有发现明显问题,但依然建议开发者在不同场景下(如日期选择、范围选择和 RTL 模式)进行手动验证,确保视觉表现与预期一致。
|
||
switch (type) { | ||
case 'multiple': | ||
if (Array.isArray(currentDate)) { | ||
if (currentDate.length > 0) { | ||
const hasIndex = currentDate.findIndex((item) => item === newDate) | ||
if (isFirst) { | ||
state.currDateArray.push([...days]) | ||
} else if (hasIndex > -1) { | ||
currentDate.splice(hasIndex, 1) | ||
state.currDateArray.splice(hasIndex, 1) | ||
} else { | ||
currentDate.push(newDate) | ||
state.currDateArray.push([...days]) | ||
} | ||
} else { | ||
currentDate.push(newDate) | ||
state.currDateArray = [[...days]] | ||
} | ||
}) | ||
if (isFirst) { | ||
state.currDateArray.push([...days]) | ||
} else if (hasIndex !== '') { | ||
;(currentDate as string[]).splice(hasIndex, 1) | ||
state.currDateArray.splice(hasIndex, 1) | ||
} else { | ||
;(currentDate as string[]).push(days[3]) | ||
state.currDateArray.push([...days]) | ||
} | ||
} else { | ||
;(currentDate as string[]).push(days[3]) | ||
state.currDateArray = [[...days]] | ||
} | ||
} else if (type === 'range') { | ||
const curDataLength = Object.values(currentDate).length | ||
if (curDataLength === 2 || curDataLength === 0) { | ||
Array.isArray(currentDate) && | ||
currentDate.splice(0) && | ||
currentDate.push(days[3]) | ||
state.currDateArray = [[...days]] | ||
} else if (Utils.compareDate(currentDate[0], days[3])) { | ||
Array.isArray(currentDate) && currentDate.push(days[3]) | ||
state.currDateArray = [...state.currDateArray, [...days]] | ||
} else { | ||
Array.isArray(currentDate) && currentDate.unshift(days[3]) | ||
state.currDateArray = [[...days], ...state.currDateArray] | ||
} | ||
} else if (type === 'week') { | ||
const weekArr = Utils.getWeekDate(y, m, `${day.day}`, firstDayOfWeek) | ||
if (propStartDate && Utils.compareDate(weekArr[0], propStartDate)) { | ||
weekArr.splice(0, 1, propStartDate) | ||
} | ||
if (propEndDate && Utils.compareDate(propEndDate, weekArr[1])) { | ||
weekArr.splice(1, 1, propEndDate) | ||
} | ||
Array.isArray(currentDate) && | ||
currentDate.splice(0) && | ||
currentDate.push(...weekArr) | ||
state.currDateArray = [ | ||
Utils.formatResultDate(weekArr[0]), | ||
Utils.formatResultDate(weekArr[1]), | ||
] | ||
} else { | ||
setCurrentDate(days[3]) | ||
state.currDateArray = [...days] | ||
break | ||
case 'range': | ||
if (Array.isArray(currentDate)) { | ||
if (currentDate.length === 2 || currentDate.length === 0) { | ||
currentDate.splice(0, currentDate.length, newDate) | ||
state.currDateArray = [[...days]] | ||
} else if (compareDate(currentDate[0], newDate)) { | ||
currentDate.push(newDate) | ||
state.currDateArray = [...state.currDateArray, [...days]] | ||
} else { | ||
currentDate.unshift(newDate) | ||
state.currDateArray = [[...days], ...state.currDateArray] | ||
} | ||
} | ||
break | ||
case 'week': | ||
// eslint-disable-next-line no-case-declarations | ||
const weekArr = getWeekDate(y, m, `${day.day}`, firstDayOfWeek) | ||
if (compareDate(weekArr[0], propStartDate)) { | ||
weekArr[0] = propStartDate | ||
} | ||
if (compareDate(propEndDate, weekArr[1])) { | ||
weekArr[1] = propEndDate | ||
} | ||
;(currentDate as string[]).splice(0, currentDate.length, ...weekArr) | ||
state.currDateArray = [ | ||
formatResultDate(weekArr[0]), | ||
formatResultDate(weekArr[1]), | ||
] | ||
break | ||
default: | ||
setCurrentDate(newDate) | ||
state.currDateArray = [...days] | ||
break | ||
} |
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
handleDayClick 中声明变量的 switch 子句易造成变量交叉访问
静态分析工具提示需要将 case 'multiple'
、case 'range'
、case 'week'
等分支内声明的临时变量用花括号包裹,避免跨分支访问风险。可参考以下做法:
switch (type) {
case 'multiple': {
- // eslint-disable-next-line no-case-declarations
- const defaultArr = [] as string[]
+ const defaultArr = [] as string[]
// 同理对 obj 等声明也进行作用域封闭
break
}
...
}
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (1.9.4)
[error] 582-583: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 632-633: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
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
♻️ Duplicate comments (1)
src/packages/calendaritem/calendaritem.taro.tsx (1)
589-642
: 🛠️ Refactor suggestion优化 handleDayClick 中的 switch 语句结构
switch 语句中的变量声明需要使用代码块来避免作用域泄漏问题。同时建议对重复的日期处理逻辑进行抽象。
建议按如下方式修改:
switch (type) { - case 'week': - // eslint-disable-next-line no-case-declarations - const weekArr = getWeekDate(y, m, `${day.day}`, firstDayOfWeek) + case 'week': { + const weekArr = getWeekDate(y, m, `${day.day}`, firstDayOfWeek) if (compareDate(weekArr[0], propStartDate)) { weekArr[0] = propStartDate } if (compareDate(propEndDate, weekArr[1])) { weekArr[1] = propEndDate } ;(currentDate as string[]).splice(0, currentDate.length, ...weekArr) state.currDateArray = [ formatResultDate(weekArr[0]), formatResultDate(weekArr[1]), ] break + }🧰 Tools
🪛 Biome (1.9.4)
[error] 632-633: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🧹 Nitpick comments (3)
src/packages/calendaritem/calendaritem.tsx (3)
414-425
: 建议优化月份计算逻辑当前的月份计算方法可以更简洁。
建议重构为:
- const getMonthNum = () => { - let monthNum = Number(endDates[1]) - Number(startDates[1]) - const yearNum = Number(endDates[0]) - Number(startDates[0]) - if (yearNum > 0) { - monthNum += 12 * yearNum - } - if (monthNum <= 0) { - monthNum = 1 - } - setMonthsNum(monthNum) - return monthNum - } + const getMonthNum = () => { + const monthNum = Math.max( + 1, + (Number(endDates[0]) - Number(startDates[0])) * 12 + + Number(endDates[1]) - Number(startDates[1]) + ) + setMonthsNum(monthNum) + return monthNum + }🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 418-419: src/packages/calendaritem/calendaritem.tsx#L418-L419
Added lines #L418 - L419 were not covered by tests
[warning] 421-422: src/packages/calendaritem/calendaritem.tsx#L421-L422
Added lines #L421 - L422 were not covered by tests
528-534
: 建议优化禁用日期判断逻辑isDisable 函数的实现可以更简洁。
建议重构为:
- const isDisable = (day: CalendarDay, month: CalendarMonthInfo) => { - if (day.type !== 'active') return true - const dateStr = getCurrDate(day, month) - if (compareDate(dateStr, propStartDate)) return true - if (compareDate(propEndDate, dateStr)) return true - return false - } + const isDisable = (day: CalendarDay, month: CalendarMonthInfo) => { + if (day.type !== 'active') return true + const dateStr = getCurrDate(day, month) + return compareDate(dateStr, propStartDate) || compareDate(propEndDate, dateStr) + }
711-774
: 建议将渲染逻辑拆分为更小的组件renderItem 和 renderPanel 函数包含了大量的渲染逻辑,建议将它们拆分为独立的组件以提高可维护性。
建议:
- 创建 CalendarDay 组件处理日期项渲染
- 创建 CalendarMonth 组件处理月份面板渲染
- 将相关的样式和逻辑移至这些组件中
需要我提供具体的组件拆分示例吗?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/packages/calendaritem/calendaritem.taro.tsx
(15 hunks)src/packages/calendaritem/calendaritem.tsx
(15 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/packages/calendaritem/calendaritem.taro.tsx
[error] 299-299: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 301-301: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 317-318: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 319-320: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 632-633: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 674-675: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 675-676: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🪛 GitHub Check: codecov/patch
src/packages/calendaritem/calendaritem.tsx
[warning] 268-269: src/packages/calendaritem/calendaritem.tsx#L268-L269
Added lines #L268 - L269 were not covered by tests
[warning] 271-271: src/packages/calendaritem/calendaritem.tsx#L271
Added line #L271 was not covered by tests
[warning] 273-273: src/packages/calendaritem/calendaritem.tsx#L273
Added line #L273 was not covered by tests
[warning] 282-284: src/packages/calendaritem/calendaritem.tsx#L282-L284
Added lines #L282 - L284 were not covered by tests
[warning] 286-287: src/packages/calendaritem/calendaritem.tsx#L286-L287
Added lines #L286 - L287 were not covered by tests
[warning] 293-293: src/packages/calendaritem/calendaritem.tsx#L293
Added line #L293 was not covered by tests
[warning] 298-299: src/packages/calendaritem/calendaritem.tsx#L298-L299
Added lines #L298 - L299 were not covered by tests
[warning] 307-307: src/packages/calendaritem/calendaritem.tsx#L307
Added line #L307 was not covered by tests
[warning] 309-309: src/packages/calendaritem/calendaritem.tsx#L309
Added line #L309 was not covered by tests
[warning] 311-313: src/packages/calendaritem/calendaritem.tsx#L311-L313
Added lines #L311 - L313 were not covered by tests
[warning] 316-316: src/packages/calendaritem/calendaritem.tsx#L316
Added line #L316 was not covered by tests
[warning] 367-371: src/packages/calendaritem/calendaritem.tsx#L367-L371
Added lines #L367 - L371 were not covered by tests
[warning] 373-373: src/packages/calendaritem/calendaritem.tsx#L373
Added line #L373 was not covered by tests
[warning] 375-379: src/packages/calendaritem/calendaritem.tsx#L375-L379
Added lines #L375 - L379 were not covered by tests
[warning] 381-383: src/packages/calendaritem/calendaritem.tsx#L381-L383
Added lines #L381 - L383 were not covered by tests
[warning] 385-385: src/packages/calendaritem/calendaritem.tsx#L385
Added line #L385 was not covered by tests
[warning] 392-393: src/packages/calendaritem/calendaritem.tsx#L392-L393
Added lines #L392 - L393 were not covered by tests
[warning] 418-419: src/packages/calendaritem/calendaritem.tsx#L418-L419
Added lines #L418 - L419 were not covered by tests
[warning] 421-422: src/packages/calendaritem/calendaritem.tsx#L421-L422
Added lines #L421 - L422 were not covered by tests
[warning] 463-463: src/packages/calendaritem/calendaritem.tsx#L463
Added line #L463 was not covered by tests
[warning] 465-465: src/packages/calendaritem/calendaritem.tsx#L465
Added line #L465 was not covered by tests
[warning] 508-508: src/packages/calendaritem/calendaritem.tsx#L508
Added line #L508 was not covered by tests
[warning] 510-511: src/packages/calendaritem/calendaritem.tsx#L510-L511
Added lines #L510 - L511 were not covered by tests
[warning] 541-544: src/packages/calendaritem/calendaritem.tsx#L541-L544
Added lines #L541 - L544 were not covered by tests
[warning] 546-549: src/packages/calendaritem/calendaritem.tsx#L546-L549
Added lines #L546 - L549 were not covered by tests
[warning] 551-553: src/packages/calendaritem/calendaritem.tsx#L551-L553
Added lines #L551 - L553 were not covered by tests
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: lint
🔇 Additional comments (5)
src/packages/calendaritem/calendaritem.tsx (3)
7-18
: 代码重构改进了模块化!将工具函数从 Utils 对象改为直接导入的方式提高了代码的可维护性和tree-shaking能力。
267-278
: 需要增加单元测试覆盖率这段处理单选模式的逻辑缺少测试覆盖。建议添加以下场景的测试用例:
- 空日期的处理
- 日期范围验证
- 边界条件处理
我可以帮您生成相关的单元测试代码。需要我为您创建测试用例吗?
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 268-269: src/packages/calendaritem/calendaritem.tsx#L268-L269
Added lines #L268 - L269 were not covered by tests
[warning] 271-271: src/packages/calendaritem/calendaritem.tsx#L271
Added line #L271 was not covered by tests
[warning] 273-273: src/packages/calendaritem/calendaritem.tsx#L273
Added line #L273 was not covered by tests
282-289
: 多选模式的日期验证逻辑需要测试覆盖range、multiple 和 week 模式的日期验证逻辑都缺少测试覆盖。这些是关键的业务逻辑,需要完整的测试用例确保正确性。
我可以帮您编写这些模式的测试用例,包括:
- 范围选择的边界测试
- 多选的重复选择测试
- 周选择的跨月份测试
需要我提供具体实现吗?Also applies to: 293-324
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 282-284: src/packages/calendaritem/calendaritem.tsx#L282-L284
Added lines #L282 - L284 were not covered by tests
[warning] 286-287: src/packages/calendaritem/calendaritem.tsx#L286-L287
Added lines #L286 - L287 were not covered by testssrc/packages/calendaritem/calendaritem.taro.tsx (2)
1-19
: 优化了工具函数的导入方式!将 Utils 对象导入改为直接导入具体函数的方式有助于实现更好的 tree-shaking,可以减小最终的包体积。
725-788
: 代码结构优化得当!将 renderItem 和 renderPanel 提取为独立函数提高了代码的可读性和可维护性,使组件结构更加清晰。
switch (type) { | ||
case 'range': | ||
if (compareDate(currentDate[0], propStartDate)) { | ||
currentDate[0] = propStartDate | ||
} | ||
if (compareDate(propEndDate, currentDate[1])) { | ||
currentDate[1] = propEndDate | ||
} | ||
defaultData = [ | ||
...splitDate(currentDate[0]), | ||
...splitDate(currentDate[1]), | ||
] | ||
break | ||
case 'multiple': | ||
// eslint-disable-next-line no-case-declarations | ||
const defaultArr = [] as string[] | ||
// eslint-disable-next-line no-case-declarations | ||
const obj: Record<string, unknown> = {} | ||
currentDate.forEach((item: string) => { | ||
if ( | ||
!compareDate(item, propStartDate) && | ||
!compareDate(propEndDate, item) | ||
) { | ||
if (!Object.hasOwnProperty.call(obj, item)) { | ||
defaultArr.push(item) | ||
obj[item] = item | ||
} | ||
} | ||
}) | ||
currentDate.splice(0, currentDate.length, ...defaultArr) | ||
defaultData = [...splitDate(defaultArr[0])] | ||
break | ||
case 'week': | ||
// eslint-disable-next-line no-case-declarations | ||
const [y, m, d] = splitDate(currentDate[0]) | ||
// eslint-disable-next-line no-case-declarations | ||
const weekArr = getWeekDate(y, m, d, firstDayOfWeek) | ||
currentDate.splice(0, currentDate.length, ...weekArr) | ||
if (compareDate(currentDate[0], propStartDate)) { | ||
currentDate.splice(0, 1, propStartDate) | ||
} | ||
}) | ||
currentDate.splice(0) && currentDate.push(...defaultArr) | ||
defaultData = [...splitDate(defaultArr[0])] | ||
} | ||
} else if (type === 'week' && Array.isArray(currentDate)) { | ||
if (currentDate.length > 0) { | ||
const [y, m, d] = splitDate(currentDate[0]) | ||
const weekArr = Utils.getWeekDate(y, m, d, firstDayOfWeek) | ||
currentDate.splice(0) && currentDate.push(...weekArr) | ||
if (propStartDate && Utils.compareDate(currentDate[0], propStartDate)) { | ||
currentDate.splice(0, 1, propStartDate) | ||
} | ||
if (propEndDate && Utils.compareDate(propEndDate, currentDate[1])) { | ||
currentDate.splice(1, 1, propEndDate) | ||
} | ||
defaultData = [ | ||
...splitDate(currentDate[0]), | ||
...splitDate(currentDate[1]), | ||
] | ||
} | ||
} else if (currentDate) { | ||
if (currentDate.length > 0) { | ||
if ( | ||
propStartDate && | ||
Utils.compareDate(currentDate as string, propStartDate) | ||
) { | ||
defaultData = [...splitDate(propStartDate as string)] | ||
} else if ( | ||
propEndDate && | ||
!Utils.compareDate(currentDate as string, propEndDate) | ||
) { | ||
defaultData = [...splitDate(propEndDate as string)] | ||
} else { | ||
defaultData = [...splitDate(currentDate as string)] | ||
} | ||
} else { | ||
defaultData = [] | ||
if (compareDate(propEndDate, currentDate[1])) { | ||
currentDate.splice(1, 1, propEndDate) | ||
} | ||
defaultData = [ | ||
...splitDate(currentDate[0]), | ||
...splitDate(currentDate[1]), | ||
] | ||
break | ||
default: | ||
break | ||
} |
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
优化 switch 语句中的变量作用域
switch 语句中的变量声明可能会泄漏到其他 case 分支。建议使用代码块来限制变量作用域。
建议按如下方式修改:
switch (type) {
case 'multiple': {
- // eslint-disable-next-line no-case-declarations
- const defaultArr = [] as string[]
- // eslint-disable-next-line no-case-declarations
- const obj: Record<string, unknown> = {}
+ const defaultArr = [] as string[]
+ const obj: Record<string, unknown> = {}
currentDate.forEach((item: string) => {
if (
!compareDate(item, propStartDate) &&
!compareDate(propEndDate, item)
) {
if (!Object.hasOwnProperty.call(obj, item)) {
defaultArr.push(item)
obj[item] = item
}
}
})
currentDate.splice(0, currentDate.length, ...defaultArr)
defaultData = [...splitDate(defaultArr[0])]
break
}
case 'week': {
- // eslint-disable-next-line no-case-declarations
- const [y, m, d] = splitDate(currentDate[0])
- // eslint-disable-next-line no-case-declarations
- const weekArr = getWeekDate(y, m, d, firstDayOfWeek)
+ const [y, m, d] = splitDate(currentDate[0])
+ const weekArr = getWeekDate(y, m, d, firstDayOfWeek)
currentDate.splice(0, currentDate.length, ...weekArr)
if (compareDate(currentDate[0], propStartDate)) {
currentDate.splice(0, 1, propStartDate)
}
if (compareDate(propEndDate, currentDate[1])) {
currentDate.splice(1, 1, propEndDate)
}
defaultData = [
...splitDate(currentDate[0]),
...splitDate(currentDate[1]),
]
break
}
📝 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.
switch (type) { | |
case 'range': | |
if (compareDate(currentDate[0], propStartDate)) { | |
currentDate[0] = propStartDate | |
} | |
if (compareDate(propEndDate, currentDate[1])) { | |
currentDate[1] = propEndDate | |
} | |
defaultData = [ | |
...splitDate(currentDate[0]), | |
...splitDate(currentDate[1]), | |
] | |
break | |
case 'multiple': | |
// eslint-disable-next-line no-case-declarations | |
const defaultArr = [] as string[] | |
// eslint-disable-next-line no-case-declarations | |
const obj: Record<string, unknown> = {} | |
currentDate.forEach((item: string) => { | |
if ( | |
!compareDate(item, propStartDate) && | |
!compareDate(propEndDate, item) | |
) { | |
if (!Object.hasOwnProperty.call(obj, item)) { | |
defaultArr.push(item) | |
obj[item] = item | |
} | |
} | |
}) | |
currentDate.splice(0, currentDate.length, ...defaultArr) | |
defaultData = [...splitDate(defaultArr[0])] | |
break | |
case 'week': | |
// eslint-disable-next-line no-case-declarations | |
const [y, m, d] = splitDate(currentDate[0]) | |
// eslint-disable-next-line no-case-declarations | |
const weekArr = getWeekDate(y, m, d, firstDayOfWeek) | |
currentDate.splice(0, currentDate.length, ...weekArr) | |
if (compareDate(currentDate[0], propStartDate)) { | |
currentDate.splice(0, 1, propStartDate) | |
} | |
}) | |
currentDate.splice(0) && currentDate.push(...defaultArr) | |
defaultData = [...splitDate(defaultArr[0])] | |
} | |
} else if (type === 'week' && Array.isArray(currentDate)) { | |
if (currentDate.length > 0) { | |
const [y, m, d] = splitDate(currentDate[0]) | |
const weekArr = Utils.getWeekDate(y, m, d, firstDayOfWeek) | |
currentDate.splice(0) && currentDate.push(...weekArr) | |
if (propStartDate && Utils.compareDate(currentDate[0], propStartDate)) { | |
currentDate.splice(0, 1, propStartDate) | |
} | |
if (propEndDate && Utils.compareDate(propEndDate, currentDate[1])) { | |
currentDate.splice(1, 1, propEndDate) | |
} | |
defaultData = [ | |
...splitDate(currentDate[0]), | |
...splitDate(currentDate[1]), | |
] | |
} | |
} else if (currentDate) { | |
if (currentDate.length > 0) { | |
if ( | |
propStartDate && | |
Utils.compareDate(currentDate as string, propStartDate) | |
) { | |
defaultData = [...splitDate(propStartDate as string)] | |
} else if ( | |
propEndDate && | |
!Utils.compareDate(currentDate as string, propEndDate) | |
) { | |
defaultData = [...splitDate(propEndDate as string)] | |
} else { | |
defaultData = [...splitDate(currentDate as string)] | |
} | |
} else { | |
defaultData = [] | |
if (compareDate(propEndDate, currentDate[1])) { | |
currentDate.splice(1, 1, propEndDate) | |
} | |
defaultData = [ | |
...splitDate(currentDate[0]), | |
...splitDate(currentDate[1]), | |
] | |
break | |
default: | |
break | |
} | |
switch (type) { | |
case 'range': | |
if (compareDate(currentDate[0], propStartDate)) { | |
currentDate[0] = propStartDate | |
} | |
if (compareDate(propEndDate, currentDate[1])) { | |
currentDate[1] = propEndDate | |
} | |
defaultData = [ | |
...splitDate(currentDate[0]), | |
...splitDate(currentDate[1]), | |
] | |
break | |
case 'multiple': { | |
const defaultArr = [] as string[] | |
const obj: Record<string, unknown> = {} | |
currentDate.forEach((item: string) => { | |
if ( | |
!compareDate(item, propStartDate) && | |
!compareDate(propEndDate, item) | |
) { | |
if (!Object.hasOwnProperty.call(obj, item)) { | |
defaultArr.push(item) | |
obj[item] = item | |
} | |
} | |
}) | |
currentDate.splice(0, currentDate.length, ...defaultArr) | |
defaultData = [...splitDate(defaultArr[0])] | |
break | |
} | |
case 'week': { | |
const [y, m, d] = splitDate(currentDate[0]) | |
const weekArr = getWeekDate(y, m, d, firstDayOfWeek) | |
currentDate.splice(0, currentDate.length, ...weekArr) | |
if (compareDate(currentDate[0], propStartDate)) { | |
currentDate.splice(0, 1, propStartDate) | |
} | |
if (compareDate(propEndDate, currentDate[1])) { | |
currentDate.splice(1, 1, propEndDate) | |
} | |
defaultData = [ | |
...splitDate(currentDate[0]), | |
...splitDate(currentDate[1]), | |
] | |
break | |
} | |
default: | |
break | |
} |
🧰 Tools
🪛 Biome (1.9.4)
[error] 299-299: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 301-301: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 317-318: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 319-320: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
export const isEqual = (date1: string, date2: string): boolean => { | ||
const startTime = new Date((date1 || '').replace(/-/g, '/')).getTime() | ||
const endTime = new Date(date2.replace(/-/g, '/')).getTime() | ||
if (startTime === endTime) { |
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.
直接 return startTime===endTIme
let date = new Date() | ||
const diff = i * (1000 * 60 * 60 * 24) | ||
date = new Date(date.getTime() + diff) | ||
return date2Str(date) |
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.
在 getDay 方法中理解 date2Str 方法,容易让人认为是把时间转为: Wed Feb 12 2025 10:05:58 GMT+0800 (中国标准时间) 。实际上 date2Str 方法是将年月日进行 join 操作。
🤔 这个变动的性质是?
🔗 相关 Issue
💡 需求背景和解决方案
☑️ 请求合并前的自查清单
Summary by CodeRabbit