-
Notifications
You must be signed in to change notification settings - Fork 1
feat(calendar): mark dst hours in week days #333
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 pull request introduces Daylight Saving Time (DST) detection capabilities to the calendar system. Two utility functions ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
#181 Bundle Size — 1.83MiB (+0.09%).c694049(current) vs 6d7ea64 main#180(baseline) Warning Bundle contains 2 duplicate packages – View duplicate packages Bundle metrics
Bundle size by type
Bundle analysis report Branch feat/week-calendar-dst-days 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: 3
🤖 Fix all issues with AI agents
In `@src/components/calendar/WeekCalendar.tsx`:
- Around line 162-166: Hoist the calendar time zone into a local variable (e.g.
const timeZone = state.timeZone) and use that variable instead of calling
getLocalTimeZone() for both DST checks; update the calls to
isDstTransitionDay(day, timeZone) and findDstTransitionHour(day, timeZone) so
DST markers use the configured calendar timezone rather than the local system
timezone, leaving hasNote(day) unchanged.
In `@src/utils/date.ts`:
- Around line 195-209: findDstTransitionHour fails to detect a midnight DST
shift because when hour === 0 it compares 00:00 of the same date to itself;
update the "before" calculation in findDstTransitionHour to use the previous
day's 23:00 instead of hour-1 when hour is 0 (i.e., compute a CalendarDate one
day earlier and call toCalendarDateTime with new Time(23,0,0)), keeping use of
toCalendarDateTime, Time, and toZoned for consistency.
- Around line 173-193: The current isDstTransitionDay function uses strict
equality (hoursInDay === 23/25) which misses non-1h DST shifts; change the logic
that computes hoursInDay (between startOfDay and endOfDay) to use range checks
with a small epsilon: if hoursInDay < 24 - epsilon return 'spring', if
hoursInDay > 24 + epsilon return 'fall', otherwise return null; keep the
existing startOfDay, endOfDay and hoursInDay calculations but replace the ===
checks with these tolerant comparisons to catch 30/45/120-minute transitions.
| const isNoteAdded = hasNote(day); | ||
| const dstType = isDstTransitionDay(day, getLocalTimeZone()); | ||
| const dstHour = dstType | ||
| ? findDstTransitionHour(day, getLocalTimeZone()) | ||
| : null; |
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.
🧩 Analysis chain
🏁 Script executed:
cat -n src/components/calendar/WeekCalendar.tsx | head -180 | tail -50Repository: domhhv/habitrack
Length of output: 2001
🏁 Script executed:
cat -n src/components/calendar/WeekCalendar.tsx | head -100Repository: domhhv/habitrack
Length of output: 3590
🏁 Script executed:
rg "state\.timeZone" src/components/calendar/WeekCalendar.tsxRepository: domhhv/habitrack
Length of output: 163
🏁 Script executed:
rg "isDstTransitionDay|findDstTransitionHour" src/components/calendar/WeekCalendar.tsx -A 2 -B 2Repository: domhhv/habitrack
Length of output: 470
🏁 Script executed:
rg "useCalendarState" src/ -A 5 | head -40Repository: domhhv/habitrack
Length of output: 1611
🏁 Script executed:
rg "state\.timeZone" src/ --type ts --type tsx -B 3 -A 1Repository: domhhv/habitrack
Length of output: 87
🏁 Script executed:
rg "state\.timeZone" src/ -B 3 -A 1 | head -60Repository: domhhv/habitrack
Length of output: 3675
🏁 Script executed:
cat -n src/components/calendar/MonthCalendarGrid.tsx | grep -A 10 "const startDate = toZoned"Repository: domhhv/habitrack
Length of output: 543
🏁 Script executed:
web_search
react-stately useCalendarState timeZone propertyRepository: domhhv/habitrack
Length of output: 162
🌐 Web query:
react-stately useCalendarState timeZone property documentation
💡 Result:
In React Stately, useCalendarState returns a CalendarState object that includes a:
timeZone: string— “The time zone of the dates currently being displayed.” [1]
(For range calendars, useRangeCalendarState similarly exposes timeZone: string on its returned state.) [2]
Sources: [1] React Stately useCalendarState docs, CalendarState interface. [2] React Stately useRangeCalendarState docs.
🏁 Script executed:
rg "isDstTransitionDay|findDstTransitionHour" src/Repository: domhhv/habitrack
Length of output: 497
🏁 Script executed:
cat -n src/utils/date.tsRepository: domhhv/habitrack
Length of output: 6727
🏁 Script executed:
cat -n src/components/calendar/WeekCalendar.tsx | sed -n '160,200p'Repository: domhhv/habitrack
Length of output: 2157
Use the calendar time zone for DST calculations.
getLocalTimeZone() on lines 163 and 165 may differ from state.timeZone, causing DST markers to be incorrect when the calendar is configured for a different timezone. Hoist a timeZone variable and reuse it for both DST calculations.
Suggested change
+ const timeZone = state.timeZone ?? getLocalTimeZone();- const dstType = isDstTransitionDay(day, getLocalTimeZone());
- const dstHour = dstType
- ? findDstTransitionHour(day, getLocalTimeZone())
- : null;
+ const dstType = isDstTransitionDay(day, timeZone);
+ const dstHour = dstType ? findDstTransitionHour(day, timeZone) : null;🤖 Prompt for AI Agents
In `@src/components/calendar/WeekCalendar.tsx` around lines 162 - 166, Hoist the
calendar time zone into a local variable (e.g. const timeZone = state.timeZone)
and use that variable instead of calling getLocalTimeZone() for both DST checks;
update the calls to isDstTransitionDay(day, timeZone) and
findDstTransitionHour(day, timeZone) so DST markers use the configured calendar
timezone rather than the local system timezone, leaving hasNote(day) unchanged.
| export const isDstTransitionDay = ( | ||
| date: CalendarDate, | ||
| timeZone: string | ||
| ): 'spring' | 'fall' | null => { | ||
| const startOfDay = toZoned(date, timeZone); | ||
| const endOfDay = toZoned(date.add({ days: 1 }), timeZone); | ||
|
|
||
| const hoursInDay = | ||
| (endOfDay.toDate().getTime() - startOfDay.toDate().getTime()) / | ||
| (1000 * 60 * 60); | ||
|
|
||
| if (hoursInDay === 23) { | ||
| return 'spring'; | ||
| } | ||
|
|
||
| if (hoursInDay === 25) { | ||
| return 'fall'; | ||
| } | ||
|
|
||
| return null; | ||
| }; |
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.
🧩 Analysis chain
🏁 Script executed:
cd /dev/null && find . -type f -name "date.ts" | head -5Repository: domhhv/habitrack
Length of output: 109
🏁 Script executed:
fd -t f "date\.ts" --exec wc -l {} \;Repository: domhhv/habitrack
Length of output: 83
🏁 Script executed:
cat -n src/utils/date.ts | sed -n '173,193p'Repository: domhhv/habitrack
Length of output: 658
🏁 Script executed:
# Check barrel exports for utils
cat -n src/utils/index.ts | grep -E "(isDstTransitionDay|findDstTransitionHour)"Repository: domhhv/habitrack
Length of output: 42
🏁 Script executed:
# Check if the new functions are present and how they're exported
rg "isDstTransitionDay" src/utils/Repository: domhhv/habitrack
Length of output: 113
🏁 Script executed:
# Check if barrel file exists
ls -la src/utils/ | grep -E "index\.(ts|js)"Repository: domhhv/habitrack
Length of output: 117
🏁 Script executed:
# Check complete utils barrel file if it exists
cat src/utils/index.tsRepository: domhhv/habitrack
Length of output: 313
🏁 Script executed:
# Check the complete date.ts around the function to understand imports
cat -n src/utils/date.ts | head -20Repository: domhhv/habitrack
Length of output: 578
🏁 Script executed:
# Check if there are any test files for isDstTransitionDay to understand expected behavior
fd -t f "date" src/ | grep -i testRepository: domhhv/habitrack
Length of output: 42
🏁 Script executed:
# Check if there are usages of isDstTransitionDay to understand how it's called
rg "isDstTransitionDay" --type ts --type tsx -B 2 -A 2Repository: domhhv/habitrack
Length of output: 87
🏁 Script executed:
# Look at the complete isDstTransitionDay and findDstTransitionHour functions
cat -n src/utils/date.ts | sed -n '173,212p'Repository: domhhv/habitrack
Length of output: 1257
Handle non-1h DST shifts (edge-case zones).
The strict hoursInDay === 23/25 check will miss zones with 30/45/120-minute DST changes (Australia, Nepal, etc.), so transition days won't be marked. Use range comparisons instead to catch all non-24-hour days.
Suggested adjustment
- if (hoursInDay === 23) {
+ if (hoursInDay < 24) {
return 'spring';
}
- if (hoursInDay === 25) {
+ if (hoursInDay > 24) {
return 'fall';
}📝 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 const isDstTransitionDay = ( | |
| date: CalendarDate, | |
| timeZone: string | |
| ): 'spring' | 'fall' | null => { | |
| const startOfDay = toZoned(date, timeZone); | |
| const endOfDay = toZoned(date.add({ days: 1 }), timeZone); | |
| const hoursInDay = | |
| (endOfDay.toDate().getTime() - startOfDay.toDate().getTime()) / | |
| (1000 * 60 * 60); | |
| if (hoursInDay === 23) { | |
| return 'spring'; | |
| } | |
| if (hoursInDay === 25) { | |
| return 'fall'; | |
| } | |
| return null; | |
| }; | |
| export const isDstTransitionDay = ( | |
| date: CalendarDate, | |
| timeZone: string | |
| ): 'spring' | 'fall' | null => { | |
| const startOfDay = toZoned(date, timeZone); | |
| const endOfDay = toZoned(date.add({ days: 1 }), timeZone); | |
| const hoursInDay = | |
| (endOfDay.toDate().getTime() - startOfDay.toDate().getTime()) / | |
| (1000 * 60 * 60); | |
| if (hoursInDay < 24) { | |
| return 'spring'; | |
| } | |
| if (hoursInDay > 24) { | |
| return 'fall'; | |
| } | |
| return null; | |
| }; |
🤖 Prompt for AI Agents
In `@src/utils/date.ts` around lines 173 - 193, The current isDstTransitionDay
function uses strict equality (hoursInDay === 23/25) which misses non-1h DST
shifts; change the logic that computes hoursInDay (between startOfDay and
endOfDay) to use range checks with a small epsilon: if hoursInDay < 24 - epsilon
return 'spring', if hoursInDay > 24 + epsilon return 'fall', otherwise return
null; keep the existing startOfDay, endOfDay and hoursInDay calculations but
replace the === checks with these tolerant comparisons to catch 30/45/120-minute
transitions.
| export const findDstTransitionHour = ( | ||
| date: CalendarDate, | ||
| timeZone: string | ||
| ): number | null => { | ||
| for (let hour = 0; hour < 24; hour++) { | ||
| const dt = toCalendarDateTime(date, new Time(hour, 0, 0)); | ||
| const before = toZoned( | ||
| toCalendarDateTime(date, new Time(hour === 0 ? 0 : hour - 1, 0, 0)), | ||
| timeZone | ||
| ); | ||
| const current = toZoned(dt, timeZone); | ||
|
|
||
| if (before.offset !== current.offset) { | ||
| return hour; | ||
| } |
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.
🧩 Analysis chain
🏁 Script executed:
# Check if the file exists and get its structure
fd "date.ts" src/ --type fRepository: domhhv/habitrack
Length of output: 77
🏁 Script executed:
# Read the specific lines mentioned (195-209) and surrounding context
if [ -f "src/utils/date.ts" ]; then
wc -l src/utils/date.ts
echo "=== Lines 190-215 ==="
sed -n '190,215p' src/utils/date.ts
fiRepository: domhhv/habitrack
Length of output: 595
🏁 Script executed:
# Search for the function and understand the full context
if [ -f "src/utils/date.ts" ]; then
rg "findDstTransitionHour" -A 20 src/utils/date.ts
fiRepository: domhhv/habitrack
Length of output: 527
🏁 Script executed:
# Check for the imported functions and types
if [ -f "src/utils/date.ts" ]; then
head -30 src/utils/date.ts
fiRepository: domhhv/habitrack
Length of output: 752
🏁 Script executed:
# Search for how CalendarDate is manipulated in the codebase
rg "date\.(subtract|add|toDate)" src/ -A 2 -B 2Repository: domhhv/habitrack
Length of output: 965
🏁 Script executed:
# Check for any examples of CalendarDate manipulation with subtract or similar methods
rg "CalendarDate" src/ -A 3 -B 1 | head -50Repository: domhhv/habitrack
Length of output: 2220
🏁 Script executed:
# Search for any date arithmetic operations in the utils
rg "subtract|add" src/utils/date.ts -B 2 -A 2Repository: domhhv/habitrack
Length of output: 757
Fix midnight DST transition detection.
When hour === 0, before and current both use 00:00 on the same date, so their offsets will be identical—DST transitions at midnight are never detected. Compare against the previous day's 23:00 instead.
Proposed fix
- const before = toZoned(
- toCalendarDateTime(date, new Time(hour === 0 ? 0 : hour - 1, 0, 0)),
- timeZone
- );
+ const beforeDateTime =
+ hour === 0
+ ? toCalendarDateTime(date.subtract({ days: 1 }), new Time(23, 0, 0))
+ : toCalendarDateTime(date, new Time(hour - 1, 0, 0));
+ const before = toZoned(beforeDateTime, timeZone);📝 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 const findDstTransitionHour = ( | |
| date: CalendarDate, | |
| timeZone: string | |
| ): number | null => { | |
| for (let hour = 0; hour < 24; hour++) { | |
| const dt = toCalendarDateTime(date, new Time(hour, 0, 0)); | |
| const before = toZoned( | |
| toCalendarDateTime(date, new Time(hour === 0 ? 0 : hour - 1, 0, 0)), | |
| timeZone | |
| ); | |
| const current = toZoned(dt, timeZone); | |
| if (before.offset !== current.offset) { | |
| return hour; | |
| } | |
| export const findDstTransitionHour = ( | |
| date: CalendarDate, | |
| timeZone: string | |
| ): number | null => { | |
| for (let hour = 0; hour < 24; hour++) { | |
| const dt = toCalendarDateTime(date, new Time(hour, 0, 0)); | |
| const beforeDateTime = | |
| hour === 0 | |
| ? toCalendarDateTime(date.subtract({ days: 1 }), new Time(23, 0, 0)) | |
| : toCalendarDateTime(date, new Time(hour - 1, 0, 0)); | |
| const before = toZoned(beforeDateTime, timeZone); | |
| const current = toZoned(dt, timeZone); | |
| if (before.offset !== current.offset) { | |
| return hour; | |
| } |
🤖 Prompt for AI Agents
In `@src/utils/date.ts` around lines 195 - 209, findDstTransitionHour fails to
detect a midnight DST shift because when hour === 0 it compares 00:00 of the
same date to itself; update the "before" calculation in findDstTransitionHour to
use the previous day's 23:00 instead of hour-1 when hour is 0 (i.e., compute a
CalendarDate one day earlier and call toCalendarDateTime with new Time(23,0,0)),
keeping use of toCalendarDateTime, Time, and toZoned for consistency.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.