-
Notifications
You must be signed in to change notification settings - Fork 1
feat(calendar): add day mode #337
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
Conversation
📝 WalkthroughWalkthroughThis PR introduces a new day calendar view feature that displays an hourly timeline with DST handling, date-picker sidebar, and occurrence/note management. It includes the new DayCalendar component, supporting page and routing, a shared useCurrentTime hook, and navigation buttons in existing month/week calendar views to link to the day view. Changes
Sequence DiagramsequenceDiagram
actor User
participant Calendar as Day Calendar UI
participant DatePicker as Date Picker<br/>(Sidebar)
participant Timeline as Hourly Timeline
participant Drawer as Note/Occurrence<br/>Drawer
User->>Calendar: Navigate to /calendar/day/:year/:month/:day
Calendar->>DatePicker: Sync focusedDate from URL (or use today)
Calendar->>Timeline: Fetch occurrences & notes for focused date
Calendar->>Timeline: Compute DST type & transition hour
Calendar->>Timeline: Group occurrences by hour & habit
Timeline->>Timeline: Mark skipped/duplicated hours (if DST)
Calendar->>Timeline: Render hourly slots (0-23) with occurrences
User->>DatePicker: Select different date
DatePicker->>Calendar: Update focusedDate & URL params
Calendar->>Timeline: Refresh occurrences for new date
User->>Timeline: Click "Add note" or "Log occurrence"
Timeline->>Drawer: Open note/occurrence drawer
User->>Drawer: Create/edit entry
Drawer->>Calendar: Save & refresh display
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
#190 Bundle Size — 1.84MiB (+0.53%).5d9e68e(current) vs ef0520a main#189(baseline) Warning Bundle contains 2 duplicate packages – View duplicate packages Bundle metrics
Bundle size by type
Bundle analysis report Branch feat/day-calendar Project dashboard Generated by RelativeCI Documentation Report issue |
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
🤖 Fix all issues with AI agents
In `@src/components/calendar/DayCalendar.tsx`:
- Around line 67-71: Validate the URL params before constructing CalendarDate:
ensure year, month, day (the route params used to create paramsDate) are numeric
and within valid ranges (e.g., month 1–12, day 1–31 or use a month/day check) or
wrap the new CalendarDate(Number(year), Number(month), Number(day)) call in a
try-catch; on invalid input or exception, fall back to a safe value (e.g.,
today's date) or redirect to a canonical calendar route and log/handle the error
so CalendarDate construction in DayCalendar.tsx cannot throw for malformed
params.
🧹 Nitpick comments (2)
src/components/calendar/DayCalendar.tsx (1)
58-76: Potential infinite loop risk in URL parameter sync effect.Including
focusedDatein the dependency array while also callingsetFocusedDateinside the effect can cause unnecessary re-renders. WhenfocusedDateupdates, the effect runs again, even though the params haven't changed.Consider removing
focusedDatefrom the dependency array and using a ref or comparing only param values:♻️ Suggested refactor
React.useEffect(() => { const currentDay = today(timeZone); const { day = currentDay.day, month = currentDay.month, year = currentDay.year, } = params; const paramsDate = new CalendarDate( Number(year), Number(month), Number(day) ); - if (focusedDate.toString() !== paramsDate.toString()) { - setFocusedDate(toCalendarDate(paramsDate)); - } - }, [params, timeZone, focusedDate]); + setFocusedDate((prev) => { + if (prev.toString() !== paramsDate.toString()) { + return toCalendarDate(paramsDate); + } + return prev; + }); + }, [params, timeZone]);src/hooks/use-current-time.ts (1)
8-16: Consider synchronizing the interval with clock minutes.The current implementation starts the 60-second interval immediately on mount, which means the time could be up to 59 seconds out of sync if the component mounts mid-minute. For a current-time indicator, this might cause the indicator to appear misaligned briefly.
♻️ Optional: Sync to minute boundary
React.useEffect(() => { + // Update immediately at next minute boundary, then every 60s + const msUntilNextMinute = (60 - new Date().getSeconds()) * 1000; + const timeout = setTimeout(() => { + setNow(new Date()); + // Then set up the regular interval + }, msUntilNextMinute); + const interval = setInterval(() => { setNow(new Date()); }, 60_000); return () => { + clearTimeout(timeout); clearInterval(interval); }; }, []);This is a minor UX enhancement and can be deferred if not needed.
| const paramsDate = new CalendarDate( | ||
| Number(year), | ||
| Number(month), | ||
| Number(day) | ||
| ); |
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.
Consider validating URL parameters before constructing CalendarDate.
If a user manually enters invalid URL parameters (e.g., /calendar/day/2026/13/45), the CalendarDate constructor may throw an error. Adding basic validation or a try-catch would improve robustness.
🛡️ Suggested defensive handling
+ try {
const paramsDate = new CalendarDate(
Number(year),
Number(month),
Number(day)
);
+ setFocusedDate((prev) => {
+ if (prev.toString() !== paramsDate.toString()) {
+ return toCalendarDate(paramsDate);
+ }
+ return prev;
+ });
+ } catch {
+ // Invalid date params, keep current date
+ }🤖 Prompt for AI Agents
In `@src/components/calendar/DayCalendar.tsx` around lines 67 - 71, Validate the
URL params before constructing CalendarDate: ensure year, month, day (the route
params used to create paramsDate) are numeric and within valid ranges (e.g.,
month 1–12, day 1–31 or use a month/day check) or wrap the new
CalendarDate(Number(year), Number(month), Number(day)) call in a try-catch; on
invalid input or exception, fall back to a safe value (e.g., today's date) or
redirect to a canonical calendar route and log/handle the error so CalendarDate
construction in DayCalendar.tsx cannot throw for malformed params.
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.