feat(calendar-picker): 添加日期跳转边界处理#7048
Conversation
- 新增 jumpToPage 方法处理默认 min/max 边界 - 重构 jumpTo 和 jumpToToday 使用新方法 - 添加测试验证边界扩展逻辑
dosubot
Bot
added
size:S
|
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough新增内部 jumpToPage 以统一处理 jumpTo/jumpToToday 的跳转逻辑：把 Page 转为月份 dayjs、在无外部 min/max 时扩展 defaultMin/defaultMax、在有外部边界时钳制目标月份，随后更新 current 并触发滚动；增加三组测试覆盖扩展、保持选中项与钳制行为。 Changes日历跳转统一化
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #7048 +/- ##
==========================================
- Coverage 92.97% 92.97% -0.01%
==========================================
Files 337 337
Lines 7374 7388 +14
Branches 1868 1847 -21
==========================================
+ Hits 6856 6869 +13
- Misses 482 511 +29
+ Partials 36 8 -28 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/calendar-picker-view/tests/calendar-picker-view.test.tsx (1)
198-235: ⚡ Quick win建议补充“已设置
min”场景的回归测试。当前用例只验证了未设置
min/max时的自动扩展；但该 PR 关联问题是min存在时jumpTo/jumpToToday异常。建议新增一条用例覆盖min已设置时的跳转行为（边界内可跳转、越界按边界处理），避免问题回归。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/calendar-picker-view/tests/calendar-picker-view.test.tsx` around lines 198 - 235, Add a regression test that covers the case when a `min` (and optionally `max`) prop is provided so `jumpTo`/`jumpToToday` respect boundaries: create a test (e.g., "jumpTo respects provided min/max boundaries") that renders a component like the existing test but passes `min={{ year: 2022, month: 3 }}` to CalendarPickerView (using the same `CalendarPickerViewRef` and `jumpTo` buttons), then assert that a `jumpTo` to an in-range month (e.g., 2022-4) renders that month, while a `jumpTo` to an out-of-range past month (e.g., 2020-1) results in rendering clipped to the `min` (data-year-month="2022-3"); also add a check for `jumpToToday` behavior when today is before the `min` to ensure it lands on `min`.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/calendar-picker-view/calendar-picker-view.tsx`:
- Around line 165-175: The jumpToPage flow sets next = convertPageToDayjs(page)
but never clamps that date to external props.min/props.max, so setCurrent(next)
and scrollTo(next) can move outside the rendered range; update jumpToPage to
first clamp next to the inclusive [props.min, props.max] bounds at month
granularity (use startOf('month')/endOf('month') comparisons or isBefore/isAfter
with month-normalized values), then keep the existing behavior that expands
defaultMin/defaultMax only when props.min/props.max are not provided (i.e., call
setDefaultMin/setDefaultMax as before), and finally call setCurrent(clampedNext)
and scrollTo(clampedNext); apply the same clamping change to the analogous jump
handler later in the file (the other jump/scroll block referenced around lines
188-193).
---
Nitpick comments:
In `@src/components/calendar-picker-view/tests/calendar-picker-view.test.tsx`:
- Around line 198-235: Add a regression test that covers the case when a `min`
(and optionally `max`) prop is provided so `jumpTo`/`jumpToToday` respect
boundaries: create a test (e.g., "jumpTo respects provided min/max boundaries")
that renders a component like the existing test but passes `min={{ year: 2022,
month: 3 }}` to CalendarPickerView (using the same `CalendarPickerViewRef` and
`jumpTo` buttons), then assert that a `jumpTo` to an in-range month (e.g.,
2022-4) renders that month, while a `jumpTo` to an out-of-range past month
(e.g., 2020-1) results in rendering clipped to the `min`
(data-year-month="2022-3"); also add a check for `jumpToToday` behavior when
today is before the `min` to ensure it lands on `min`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 436ef4f2-3394-439a-9ee7-ae8cfaf1dbef
📒 Files selected for processing (2)
src/components/calendar-picker-view/calendar-picker-view.tsxsrc/components/calendar-picker-view/tests/calendar-picker-view.test.tsx
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors calendar navigation by introducing a jumpToPage function that automatically expands the rendering range when bounds are not explicitly defined. A review comment identifies critical issues regarding boundary clamping for fixed ranges and potential date rollover bugs when using Dayjs, providing a more robust implementation suggestion.
| const jumpToPage = (page: Page) => { | ||
| const next = convertPageToDayjs(page) | ||
| if (!props.min && next.isBefore(defaultMin)) { | ||
| setDefaultMin(next.date(1)) | ||
| } | ||
| if (!props.max && next.isAfter(defaultMax)) { | ||
| setDefaultMax(next.endOf('month')) | ||
| } | ||
| setCurrent(next) | ||
| scrollTo(next) | ||
| } |
There was a problem hiding this comment.
The jumpToPage function has two issues:
- Boundary Clamping: If
props.minorprops.maxare provided, the rendering range is fixed. If the targetpageis outside these bounds,setCurrent(next)will set an unreachable month, andscrollTo(next)will fail because the corresponding DOM element won't be rendered. The target date should be clamped to the allowed range defined byminDayandmaxDay. - Dayjs Rollover Bug: The
convertPageToDayjsutility (used here) has a potential bug where it can roll over to the wrong month if today is the 31st (e.g., calling it for February on January 31st might result in March). It's safer to create thedayjsobject by setting the date to the 1st before setting the year and month.
Additionally, next.date(1) is redundant if the date is already set to the 1st.
const jumpToPage = (page: Page) => {
// Use a safe way to create the dayjs object to avoid rollover issues when today is the 31st
let next = dayjs().date(1).year(page.year).month(page.month - 1)
if (next.isBefore(minDay, 'month')) {
if (props.min) {
next = minDay.date(1)
} else {
setDefaultMin(next)
}
} else if (next.isAfter(maxDay, 'month')) {
if (props.max) {
next = maxDay.date(1)
} else {
setDefaultMax(next.endOf('month'))
}
}
setCurrent(next)
scrollTo(next)
}
- 当跳转日期超出min/max范围时自动修正到边界值 - 添加测试用例验证边界条件处理
- 调整跳转逻辑保持选中日期在渲染范围内 - 优化默认最小/最大日期的更新条件
dosubot
Bot
added
size:M
| setDefaultMin(next) | ||
| } | ||
| if (!props.max) { | ||
| setDefaultMax(next.add(6, 'month').endOf('month')) |
There was a problem hiding this comment.
有点 hardcode，要不然抽一个 alignRange 的方法。然后初始化和jumpTo 都通过这个方法来重置好了？
- 提取 VISIBLE_MONTHS 为常量 - 新增 alignRange 方法统一处理边界对齐 - 默认 max 值调整为月份结束日期
- 简化默认max的计算逻辑 - 保持min和max计算方式一致
2 participants
close #6518
Summary by CodeRabbit
错误修复
测试