Conversation
- Split PresetSelector into PresetList, PresetEditDialog, and PresetManageDialog for better modularity. - Introduced usePasswordProtection hook to handle password verification and management. - Updated PresetSelector to utilize new components and hook for creating, editing, and deleting presets with password protection. - Removed redundant state management and form handling from PresetSelector, delegating to PresetEditDialog. - Enhanced user experience by managing dialogs and preset actions more efficiently.
…k for form handling
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughReplaces inline fetching/state with hook-driven data management (useCalendars, useShifts, usePresets, useNotes, useSSEConnection); extracts header/calendar/shift UIs into AppHeader, CalendarGrid, ShiftsList and preset dialogs/components; centralizes password protection via usePasswordProtection; adds password checks to notes APIs and moves DELETE password payloads to JSON bodies. Changes
Sequence DiagramsequenceDiagram
participant User
participant Client as App Client
participant Hooks as Data Hooks
participant API as API Server
participant SSE as SSE Stream
participant DB as Database
User->>Client: Open app / select calendar
Client->>Hooks: useCalendars(), useShifts(), usePresets(), useNotes()
Hooks->>API: GET /api/calendars, /api/shifts, /api/presets, /api/notes
API->>DB: Query data
DB-->>API: Data
API-->>Hooks: Responses
Hooks-->>Client: Provide state & actions to components (AppHeader, CalendarGrid, ShiftsList)
Client->>SSE: Open /api/events/stream?calendarId=...
SSE-->>Client: events (shift/preset/note)
Client->>Hooks: useSSEConnection triggers onShiftUpdate/onPresetUpdate/onNoteUpdate
Hooks->>Client: refetch or patch local state
User->>Client: Create/modify/delete shift/preset/note (may require password)
Client->>Hooks: invoke create/update/delete actions
Hooks->>API: POST/PATCH/PUT/DELETE /api/...
API->>DB: Persist change (verify password if protected)
DB-->>API: Confirm
API->>SSE: Broadcast update
SSE-->>Client: event delivered -> Hooks update state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–90 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ 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 |
There was a problem hiding this comment.
Pull request overview
This PR refactors the main application page by extracting logic into custom hooks and breaking down large components into smaller, reusable pieces. The refactoring improves code organization, maintainability, and separation of concerns.
Key changes:
- Extracted data fetching and state management into custom hooks (
useShifts,usePresets,useNotes,useCalendars,useSSEConnection,usePasswordProtection,useShiftForm) - Split large components into smaller, focused components (
AppHeader,CalendarGrid,ShiftsList,PresetList,PresetEditDialog, etc.) - Reduced the main page from ~1500 lines to ~700 lines by delegating functionality to hooks and components
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
| hooks/useShifts.ts | Custom hook for shift CRUD operations with optimistic updates |
| hooks/useShiftForm.ts | Form state management hook for shift dialog |
| hooks/useSSEConnection.ts | Server-Sent Events connection management for real-time updates |
| hooks/usePresets.ts | Preset data fetching hook |
| hooks/usePasswordProtection.ts | Password verification utilities |
| hooks/useNotes.ts | Calendar notes CRUD operations |
| hooks/useCalendars.ts | Calendar management and selection logic |
| components/shifts-list.tsx | Collapsible shifts list component with grouping by date |
| components/shift-form-fields.tsx | Extracted shift form fields from dialog |
| components/shift-dialog.tsx | Refactored to use custom hooks |
| components/preset-selector.tsx | Simplified by extracting sub-components |
| components/preset-select.tsx | Preset dropdown selector component |
| components/preset-manage-dialog.tsx | Preset management dialog |
| components/preset-list.tsx | Preset display and selection component |
| components/preset-edit-dialog.tsx | Preset creation/editing dialog |
| components/calendar-grid.tsx | Calendar grid display component |
| components/app-header.tsx | Application header with navigation |
| app/page.tsx | Main page refactored to use custom hooks and extracted components |
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
| return () => { | ||
| eventSource.close(); | ||
| }; | ||
| }, [calendarId]); |
There was a problem hiding this comment.
Missing dependencies in useEffect. The callback functions onShiftUpdate, onPresetUpdate, onNoteUpdate, and onStatsRefresh should be included in the dependency array. This will cause the SSE connection to reconnect unnecessarily on every render if these callbacks change.
| }, [calendarId]); | |
| }, [calendarId, onShiftUpdate, onPresetUpdate, onNoteUpdate, onStatsRefresh]); |
| useEffect(() => { | ||
| if (calendarId) { | ||
| fetchShifts(); | ||
| } else { | ||
| setShifts([]); | ||
| } | ||
| }, [calendarId]); |
There was a problem hiding this comment.
Missing dependency in useEffect. The fetchShifts function should be included in the dependency array, or alternatively, wrap it in a useCallback to stabilize its reference.
| useEffect(() => { | ||
| if (calendarId) { | ||
| fetchPresets(); | ||
| } else { | ||
| setPresets([]); | ||
| } | ||
| }, [calendarId]); |
There was a problem hiding this comment.
Missing dependency in useEffect. The fetchPresets function should be included in the dependency array, or alternatively, wrap it in a useCallback to stabilize its reference.
| onManageClick, | ||
| }: PresetListProps) { | ||
| const t = useTranslations(); | ||
| const [showSecondary, setShowSecondary] = React.useState(false); |
There was a problem hiding this comment.
Missing React import. The React.useState is used on line 23 but React is not imported. Add import React from "react"; at the top of the file.
| if (!calendarId) return; | ||
|
|
||
| try { | ||
| const response = await fetch(`/api/shifts?calendarId=${calendarId}`); |
There was a problem hiding this comment.
Missing error handling for API response status. The code should check response.ok before parsing JSON to handle HTTP errors properly. Currently, failed requests will throw errors when trying to parse non-JSON error responses.
| const response = await fetch(`/api/shifts?calendarId=${calendarId}`); | |
| const response = await fetch(`/api/shifts?calendarId=${calendarId}`); | |
| if (!response.ok) { | |
| console.error(`Failed to fetch shifts: ${response.status} ${response.statusText}`); | |
| toast.error(t("shift.fetchError")); | |
| return; | |
| } |
| ); | ||
| } | ||
|
|
||
| import React from "react"; |
There was a problem hiding this comment.
React import is at the wrong location. The import React from "react"; statement should be moved to the top of the file with other imports, not at the bottom (line 217).
| try { | ||
| const response = await fetch(`/api/notes?calendarId=${calendarId}`); | ||
| const data = await response.json(); | ||
| setNotes(data); |
There was a problem hiding this comment.
Missing error handling for API response status. The code should check response.ok before parsing JSON to handle HTTP errors properly. Currently, failed requests will throw errors when trying to parse non-JSON error responses.
| } from "date-fns"; | ||
| import { de, enUS } from "date-fns/locale"; | ||
| import { ShiftPreset, CalendarNote } from "@/lib/db/schema"; | ||
| import { CalendarNote, ShiftPreset } from "@/lib/db/schema"; |
There was a problem hiding this comment.
Unused import ShiftPreset.
| import { CalendarNote, ShiftPreset } from "@/lib/db/schema"; | |
| import { CalendarNote } from "@/lib/db/schema"; |
| const handleUpdateShift = async (id: string, formData: ShiftFormData) => { | ||
| const success = await updateShiftHook(id, formData, () => { | ||
| setPendingAction({ type: "edit", shiftId: id, formData }); | ||
| setShowPasswordDialog(true); | ||
| }); | ||
| if (success) { | ||
| setStatsRefreshTrigger((prev) => prev + 1); | ||
| } | ||
| }; | ||
|
|
There was a problem hiding this comment.
Unused variable handleUpdateShift.
| const handleUpdateShift = async (id: string, formData: ShiftFormData) => { | |
| const success = await updateShiftHook(id, formData, () => { | |
| setPendingAction({ type: "edit", shiftId: id, formData }); | |
| setShowPasswordDialog(true); | |
| }); | |
| if (success) { | |
| setStatsRefreshTrigger((prev) => prev + 1); | |
| } | |
| }; |
| applyPreset, | ||
| saveAsPresetHandler, | ||
| resetForm, | ||
| refetchPresets, |
There was a problem hiding this comment.
Unused variable refetchPresets.
| refetchPresets, |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
components/preset-selector.tsx (2)
58-92: Missing response validation before success toast.The
fetchcalls don't checkresponse.okbefore showing success toasts. If the API returns an error status (e.g., 400, 401, 500), the code still shows a success message.if (isCreatingNew) { - await fetch("/api/presets", { + const response = await fetch("/api/presets", { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ calendarId, ...formData, password, }), }); + if (!response.ok) { + throw new Error("Failed to create preset"); + } toast.success(t("preset.created")); } else if (editingPreset) { - await fetch(`/api/presets/${editingPreset.id}`, { + const response = await fetch(`/api/presets/${editingPreset.id}`, { method: "PATCH", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ ...formData, password }), }); + if (!response.ok) { + throw new Error("Failed to update preset"); + } toast.success(t("preset.updated"));
94-118: Delete operation missing response validation.Similar issue - the delete
fetchdoesn't checkresponse.ok.- await fetch(url, { method: "DELETE" }); + const response = await fetch(url, { method: "DELETE" }); + if (!response.ok) { + throw new Error("Failed to delete preset"); + }
🟡 Minor comments (7)
components/preset-edit-dialog.tsx-65-87 (1)
65-87: Reset form state when dialog closes.The dialog resets state when opening but doesn't clear state when closing (when
openbecomesfalse). This violates the coding guideline that dialog components must "reset internal state when open changes to false."As per coding guidelines, dialog components should reset their internal state when closed.
Apply this diff:
useEffect(() => { - if (open && preset && !isCreating) { + if (open) { + if (preset && !isCreating) { - setFormData({ - title: preset.title, - startTime: preset.startTime, - endTime: preset.endTime, - color: preset.color, - notes: preset.notes || "", - isSecondary: preset.isSecondary || false, - isAllDay: preset.isAllDay || false, - }); + setFormData({ + title: preset.title, + startTime: preset.startTime, + endTime: preset.endTime, + color: preset.color, + notes: preset.notes || "", + isSecondary: preset.isSecondary || false, + isAllDay: preset.isAllDay || false, + }); + } else if (isCreating) { - } else if (open && isCreating) { - setFormData({ - title: "", - startTime: "09:00", - endTime: "17:00", - color: PRESET_COLORS[0].value, - notes: "", - isSecondary: false, - isAllDay: false, - }); + setFormData({ + title: "", + startTime: "09:00", + endTime: "17:00", + color: PRESET_COLORS[0].value, + notes: "", + isSecondary: false, + isAllDay: false, + }); + } + } else { + // Reset to defaults when dialog closes + setFormData({ + title: "", + startTime: "09:00", + endTime: "17:00", + color: PRESET_COLORS[0].value, + notes: "", + isSecondary: false, + isAllDay: false, + }); } }, [open, preset, isCreating]);hooks/useCalendars.ts-20-27 (1)
20-27: Stale closure onselectedCalendarcheck.The condition
!selectedCalendarat line 25 captures the initial value ofselectedCalendar(which is alwaysundefinedon mount). This works for the initial fetch but would be incorrect iffetchCalendarsis called again viarefetchCalendars. Consider using a ref or functional state check.- if ( - initialCalendarId && - data.some((cal: CalendarWithCount) => cal.id === initialCalendarId) - ) { - setSelectedCalendar(initialCalendarId); - } else if (data.length > 0 && !selectedCalendar) { - setSelectedCalendar(data[0].id); - } + setSelectedCalendar((current) => { + if ( + initialCalendarId && + data.some((cal: CalendarWithCount) => cal.id === initialCalendarId) + ) { + return initialCalendarId; + } + if (data.length > 0 && !current) { + return data[0].id; + } + return current; + });hooks/usePasswordProtection.ts-15-26 (1)
15-26: Missingresponse.okcheck before parsing JSON.If the API returns a non-2xx status,
response.json()may fail or return unexpected data. Add a guard to handle error responses gracefully.const response = await fetch( `/api/calendars/${calendarId}/verify-password`, { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ password }), } ); + if (!response.ok) { + localStorage.removeItem(`calendar_password_${calendarId}`); + return false; + } + const data = await response.json();hooks/useShifts.ts-123-129 (1)
123-129: Missing dependency in useEffect.
fetchShiftsis referenced but not listed in the dependency array. Either wrapfetchShiftsinuseCallbackwith[calendarId]as dependency, or inline the fetch logic within the effect.+ const fetchShifts = useCallback(async () => { + if (!calendarId) return; + try { + const response = await fetch(`/api/shifts?calendarId=${calendarId}`); + const data = await response.json(); + setShifts(data); + } catch (error) { + console.error("Failed to fetch shifts:", error); + } + }, [calendarId]); useEffect(() => { if (calendarId) { fetchShifts(); } else { setShifts([]); } - }, [calendarId]); + }, [calendarId, fetchShifts]);Committable suggestion skipped: line range outside the PR's diff.
app/page.tsx-370-394 (1)
370-394: Optimistic UI rollback uses stale reference.When the delete fails (lines 382-394), the rollback uses
shiftsfrom closure, butshiftsmay have changed since the optimistic update on line 371. This could restore incorrect state.Consider capturing the previous state before the optimistic update:
if (existingShift) { + const previousShifts = shifts; setShifts(shifts.filter((s) => s.id !== existingShift.id)); setStatsRefreshTrigger((prev) => prev + 1); try { ... if (!response.ok) { - setShifts(shifts); + setShifts(previousShifts); ... } } catch (error) { - setShifts(shifts); + setShifts(previousShifts); ... } }hooks/useSSEConnection.ts-143-164 (1)
143-164: Potential timer leak on rapid calendarId changes.The
setTimeoutcalls inonerroraren't cleaned up ifcalendarIdchanges rapidly. The effect cleanup only closes the EventSource but doesn't clear the pending timeouts.+ let errorTimeout: NodeJS.Timeout | null = null; + let reconnectTimeout: NodeJS.Timeout | null = null; eventSource.onerror = (error) => { console.error("SSE connection error:", error); setIsConnected(false); disconnectTimeRef.current = Date.now(); eventSource.close(); - const errorTimeout = setTimeout(() => { + errorTimeout = setTimeout(() => { if (!navigator.onLine) { toast.error(t("sync.disconnected"), { duration: Infinity }); } }, 5000); - setTimeout(() => { + reconnectTimeout = setTimeout(() => { clearTimeout(errorTimeout); if (calendarId && navigator.onLine) { console.log("Attempting to reconnect and resync..."); onShiftUpdate(); onPresetUpdate(); onNoteUpdate(); } }, 3000); }; ... return () => { eventSource.close(); + if (errorTimeout) clearTimeout(errorTimeout); + if (reconnectTimeout) clearTimeout(reconnectTimeout); };hooks/useSSEConnection.ts-29-81 (1)
29-81: Both useEffect hooks are missing callback dependencies.The effect at line 29 uses
onShiftUpdate,onPresetUpdate,onNoteUpdate,onStatsRefresh, andsetIsConnectedbut excludes them from the dependency array[calendarId, t]. The same issue exists in the SSE connection effect at line 83 with dependency array[calendarId].If these callbacks are not memoized at the call site (using
useCallback), they should be added to both dependency arrays. If they are guaranteed to be stable references from the parent component, this should be documented with an ESLint disable comment explaining why.
🧹 Nitpick comments (7)
components/preset-list.tsx (1)
217-217: Move React import to the top of the file.The React import appears at the bottom of the file (line 217) but is used at line 23 with
React.useState. While this works after bundling, it violates conventional import ordering and reduces code readability.+import React from "react"; import { motion } from "motion/react"; import { useTranslations } from "next-intl"; import { Button } from "@/components/ui/button"; import { Check, Plus, Settings, ChevronDown, ChevronUp } from "lucide-react"; import { ShiftPreset } from "@/lib/db/schema"; // ... rest of file ... -import React from "react";components/app-header.tsx (1)
103-116: Consider extracting duplicated gradient styling logic.The calendar color gradient logic (lines 103-116 and 146-159) is duplicated. Consider extracting this into a helper function or a reusable component to improve maintainability.
// Helper at top of component or in a utils file const getCalendarGradient = ( calendars: CalendarWithCount[], selectedCalendar: string | undefined ) => { const color = calendars.find((c) => c.id === selectedCalendar)?.color; return color ? `linear-gradient(to bottom, ${color}, ${color}80)` : "linear-gradient(to bottom, hsl(var(--primary)), hsl(var(--primary) / 0.5))"; };components/shift-dialog.tsx (2)
90-92: Remove unnecessaryasynckeyword.
handlePresetSelectis declaredasyncbut doesn't useawait- the function body is synchronous.- const handlePresetSelect = async (preset: Parameters<typeof applyPreset>[0]) => { + const handlePresetSelect = (preset: Parameters<typeof applyPreset>[0]) => { applyPreset(preset); };
63-88: Verify error handling forsaveAsPresetHandler.The
saveAsPresetHandlercall's failure case only logs via the hook, but the dialog proceeds to reset and close regardless. If preset saving fails, the user sees no feedback except a console error.Consider surfacing the failure to the user:
// Save as preset if enabled and it's a new shift if (!shift && saveAsPreset && presetName.trim()) { const success = await saveAsPresetHandler(submitData); - if (success && onPresetsChange) { + if (success) { + onPresetsChange?.(); + } else { + // saveAsPresetHandler already logs the error; optionally notify user + } - onPresetsChange(); - } }components/shifts-list.tsx (2)
26-48: Consider memoizing filtered and grouped shifts.The
shiftsInMonthfilter andgroupedShiftscomputation run on every render. For large shift lists, this could cause performance issues.+import { useState, useMemo } from "react"; ... - const shiftsInMonth = shifts.filter((shift) => { + const shiftsInMonth = useMemo(() => shifts.filter((shift) => { if (!shift.date) return false; const shiftDate = new Date(shift.date); return ( shiftDate.getMonth() === currentDate.getMonth() && shiftDate.getFullYear() === currentDate.getFullYear() ); - }); + }), [shifts, currentDate]); - const groupedShifts = shiftsInMonth + const groupedShifts = useMemo(() => shiftsInMonth .sort(...) - .reduce(...); + .reduce(...), [shiftsInMonth]);
50-78: Collapsible button accessibility improvement.The toggle button lacks
aria-expandedfor screen readers.<button onClick={() => setShowShiftsSection(!showShiftsSection)} className="w-full px-3 sm:px-4 py-3 sm:py-3.5 flex items-center justify-between hover:bg-primary/5 transition-all" + aria-expanded={showShiftsSection} >app/page.tsx (1)
324-326: Early return could provide user feedback.When
!selectedPresetId, the function silently returns. Consider showing a toast to guide the user.- if (isTogglingShift || !selectedPresetId) return; + if (isTogglingShift) return; + if (!selectedPresetId) { + toast.info(t("shift.selectPresetFirst")); + return; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
app/page.tsx(15 hunks)components/app-header.tsx(1 hunks)components/calendar-grid.tsx(1 hunks)components/preset-edit-dialog.tsx(1 hunks)components/preset-list.tsx(1 hunks)components/preset-manage-dialog.tsx(1 hunks)components/preset-select.tsx(1 hunks)components/preset-selector.tsx(4 hunks)components/shift-dialog.tsx(3 hunks)components/shift-form-fields.tsx(1 hunks)components/shifts-list.tsx(1 hunks)hooks/useCalendars.ts(1 hunks)hooks/useNotes.ts(1 hunks)hooks/usePasswordProtection.ts(1 hunks)hooks/usePresets.ts(1 hunks)hooks/useSSEConnection.ts(1 hunks)hooks/useShiftForm.ts(1 hunks)hooks/useShifts.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
components/**/*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use PRESET_COLORS constant array for all color picker options
Files:
components/preset-select.tsxcomponents/preset-edit-dialog.tsxcomponents/shifts-list.tsxcomponents/shift-form-fields.tsxcomponents/calendar-grid.tsxcomponents/preset-manage-dialog.tsxcomponents/preset-list.tsxcomponents/shift-dialog.tsxcomponents/app-header.tsxcomponents/preset-selector.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Store colors as hex values (e.g., #3b82f6) and apply 20% opacity for backgrounds using format ${color}20
Files:
components/preset-select.tsxcomponents/preset-edit-dialog.tsxhooks/usePresets.tscomponents/shifts-list.tsxcomponents/shift-form-fields.tsxcomponents/calendar-grid.tsxhooks/useSSEConnection.tscomponents/preset-manage-dialog.tsxhooks/useShiftForm.tshooks/useNotes.tshooks/useCalendars.tscomponents/preset-list.tsxhooks/useShifts.tscomponents/shift-dialog.tsxcomponents/app-header.tsxapp/page.tsxhooks/usePasswordProtection.tscomponents/preset-selector.tsx
components/**/*-dialog.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
components/**/*-dialog.tsx: Dialog components must control open state via props, reset internal state when open changes to false, and call parent callback on form submission
Form submission in dialogs: prevent default, validate, call parent callback, close dialog
Files:
components/preset-edit-dialog.tsxcomponents/preset-manage-dialog.tsxcomponents/shift-dialog.tsx
components/**/calendar*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Calendar interaction: left-click toggles shift (requires preset selection), right-click opens note dialog with e.preventDefault()
Files:
components/calendar-grid.tsx
app/page.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
app/page.tsx: Password verification for calendar access must use pendingAction state pattern: set pending action, show PasswordDialog, retry after password verification via /api/calendars/[id]/verify-password
Main page (app/page.tsx) uses URL sync via useRouter().replace() for selected calendar
Refresh data after mutations by incrementing statsRefreshTrigger counter
Files:
app/page.tsx
app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use useTranslations() hook for all user-facing text and t("key") to access translation keys from messages/{de,en}.json
Files:
app/page.tsx
app/**/*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
app/**/*.tsx: Use formatDateToLocal() helper for consistent YYYY-MM-DD date formatting in UI
Shift toggle logic must check if matching shift exists: delete if present, create if not
Use locale-specific date formatters in components: locale === "de" ? de : enUS
Frontend uses Next.js 16 App Router (app/ directory), React 19, and client-side state management via useState/useEffect
Files:
app/page.tsx
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/**/*.tsx : Shift toggle logic must check if matching shift exists: delete if present, create if not
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/page.tsx : Main page (app/page.tsx) uses URL sync via useRouter().replace() for selected calendar
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to components/**/calendar*.tsx : Calendar interaction: left-click toggles shift (requires preset selection), right-click opens note dialog with e.preventDefault()
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to components/**/*.tsx : Use PRESET_COLORS constant array for all color picker options
Applied to files:
components/preset-select.tsxcomponents/preset-edit-dialog.tsxhooks/usePresets.tscomponents/shift-form-fields.tsxcomponents/preset-manage-dialog.tsxcomponents/preset-list.tsxcomponents/shift-dialog.tsxcomponents/preset-selector.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to components/**/calendar*.tsx : Calendar interaction: left-click toggles shift (requires preset selection), right-click opens note dialog with e.preventDefault()
Applied to files:
components/preset-select.tsxcomponents/preset-edit-dialog.tsxhooks/usePresets.tscomponents/shifts-list.tsxcomponents/shift-form-fields.tsxcomponents/calendar-grid.tsxcomponents/preset-manage-dialog.tsxhooks/useShiftForm.tscomponents/preset-list.tsxhooks/useShifts.tscomponents/shift-dialog.tsxcomponents/app-header.tsxapp/page.tsxcomponents/preset-selector.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/**/*.tsx : Shift toggle logic must check if matching shift exists: delete if present, create if not
Applied to files:
components/preset-select.tsxcomponents/shifts-list.tsxcomponents/shift-form-fields.tsxhooks/useShiftForm.tshooks/useShifts.tscomponents/shift-dialog.tsxapp/page.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to components/**/*-dialog.tsx : Dialog components must control open state via props, reset internal state when open changes to false, and call parent callback on form submission
Applied to files:
components/preset-edit-dialog.tsxcomponents/shift-form-fields.tsxcomponents/preset-manage-dialog.tsxhooks/useShiftForm.tscomponents/shift-dialog.tsxapp/page.tsxcomponents/preset-selector.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to components/**/*-dialog.tsx : Form submission in dialogs: prevent default, validate, call parent callback, close dialog
Applied to files:
components/preset-edit-dialog.tsxcomponents/preset-manage-dialog.tsxhooks/useShiftForm.tscomponents/shift-dialog.tsxapp/page.tsxcomponents/preset-selector.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/**/*.tsx : Use locale-specific date formatters in components: locale === "de" ? de : enUS
Applied to files:
components/shift-form-fields.tsxapp/page.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/**/*.tsx : Use formatDateToLocal() helper for consistent YYYY-MM-DD date formatting in UI
Applied to files:
components/calendar-grid.tsxapp/page.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/page.tsx : Password verification for calendar access must use pendingAction state pattern: set pending action, show PasswordDialog, retry after password verification via /api/calendars/[id]/verify-password
Applied to files:
hooks/useCalendars.tshooks/useShifts.tscomponents/app-header.tsxapp/page.tsxhooks/usePasswordProtection.tscomponents/preset-selector.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/page.tsx : Main page (app/page.tsx) uses URL sync via useRouter().replace() for selected calendar
Applied to files:
hooks/useCalendars.tscomponents/app-header.tsxapp/page.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/**/*.tsx : Frontend uses Next.js 16 App Router (app/ directory), React 19, and client-side state management via useState/useEffect
Applied to files:
components/shift-dialog.tsxcomponents/app-header.tsxapp/page.tsxcomponents/preset-selector.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/**/*.{ts,tsx} : Use useTranslations() hook for all user-facing text and t("key") to access translation keys from messages/{de,en}.json
Applied to files:
components/shift-dialog.tsxcomponents/preset-selector.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/page.tsx : Refresh data after mutations by incrementing statsRefreshTrigger counter
Applied to files:
app/page.tsx
🧬 Code graph analysis (14)
components/preset-select.tsx (3)
lib/db/schema.ts (1)
ShiftPreset(92-92)components/ui/label.tsx (1)
Label(24-24)components/ui/select.tsx (5)
Select(177-177)SelectTrigger(185-185)SelectValue(186-186)SelectContent(178-178)SelectItem(180-180)
hooks/usePresets.ts (1)
lib/db/schema.ts (1)
ShiftPreset(92-92)
components/shifts-list.tsx (3)
lib/types.ts (1)
ShiftWithCalendar(14-31)lib/db/schema.ts (1)
shifts(19-45)components/shift-card.tsx (1)
ShiftCard(15-68)
components/calendar-grid.tsx (2)
lib/types.ts (1)
ShiftWithCalendar(14-31)lib/db/schema.ts (1)
CalendarNote(94-94)
components/preset-manage-dialog.tsx (3)
lib/db/schema.ts (1)
ShiftPreset(92-92)components/ui/dialog.tsx (5)
Dialog(139-139)DialogContent(141-141)DialogHeader(144-144)DialogTitle(147-147)DialogDescription(142-142)components/ui/button.tsx (1)
Button(60-60)
hooks/useShiftForm.ts (4)
lib/types.ts (1)
ShiftWithCalendar(14-31)components/shift-dialog.tsx (1)
ShiftFormData(27-36)lib/date-utils.ts (1)
formatDateToLocal(5-10)lib/db/schema.ts (1)
ShiftPreset(92-92)
hooks/useNotes.ts (2)
lib/db/schema.ts (1)
CalendarNote(94-94)lib/date-utils.ts (1)
formatDateToLocal(5-10)
hooks/useCalendars.ts (2)
lib/db/schema.ts (1)
calendars(4-17)lib/types.ts (1)
CalendarWithCount(4-12)
components/preset-list.tsx (2)
lib/db/schema.ts (1)
ShiftPreset(92-92)components/ui/button.tsx (1)
Button(60-60)
hooks/useShifts.ts (3)
lib/db/schema.ts (1)
shifts(19-45)lib/types.ts (1)
ShiftWithCalendar(14-31)components/shift-dialog.tsx (1)
ShiftFormData(27-36)
components/shift-dialog.tsx (3)
hooks/useShiftForm.ts (1)
useShiftForm(14-149)components/preset-select.tsx (1)
PresetSelect(17-52)components/shift-form-fields.tsx (1)
ShiftFormFields(31-205)
app/page.tsx (10)
hooks/useCalendars.ts (1)
useCalendars(6-105)hooks/useShifts.ts (1)
useShifts(7-139)hooks/usePresets.ts (1)
usePresets(4-31)hooks/useNotes.ts (1)
useNotes(7-87)hooks/useSSEConnection.ts (1)
useSSEConnection(15-177)lib/db/schema.ts (2)
shifts(19-45)calendars(4-17)lib/date-utils.ts (1)
formatDateToLocal(5-10)components/app-header.tsx (1)
AppHeader(36-251)components/calendar-grid.tsx (1)
CalendarGrid(21-178)components/shifts-list.tsx (1)
ShiftsList(16-173)
hooks/usePasswordProtection.ts (1)
lib/password-utils.ts (1)
verifyPassword(16-23)
components/preset-selector.tsx (5)
lib/db/schema.ts (1)
ShiftPreset(92-92)hooks/usePasswordProtection.ts (1)
usePasswordProtection(8-64)components/preset-edit-dialog.tsx (2)
PresetFormData(252-252)PresetEditDialog(47-250)components/preset-list.tsx (1)
PresetList(15-126)components/preset-manage-dialog.tsx (1)
PresetManageDialog(22-102)
🔇 Additional comments (13)
components/preset-select.tsx (1)
17-52: LGTM!The PresetSelect component is well-structured with proper early returns, clean separation of concerns, and good use of translations. The color and time range display logic is clear and handles both all-day and timed shifts appropriately.
components/calendar-grid.tsx (1)
146-148: LGTM!The color application correctly uses the
${color}20format for 20% opacity backgrounds, following the coding guidelines. The color bar with left border provides good visual hierarchy.As per coding guidelines, colors are stored as hex values with 20% opacity applied using the
${color}20format.components/shift-form-fields.tsx (1)
31-205: LGTM!The ShiftFormFields component is well-structured with clean separation of concerns. The use of animated transitions for conditional fields (all-day toggle, preset name input) provides a polished UX. Form state management through props maintains proper unidirectional data flow.
hooks/usePasswordProtection.ts (1)
39-53: Consider memoization stability foronPasswordRequired.
onPasswordRequiredis included in the dependency array but is likely an inline function from the parent. This could causewithPasswordCheckto be recreated on every render. Ensure the caller wrapsonPasswordRequiredinuseCallback, or consider using a ref to hold the callback to avoid stale closure issues.components/preset-manage-dialog.tsx (1)
22-101: LGTM!The dialog correctly controls open state via props and follows the coding guidelines. The component is stateless and delegates all actions to parent callbacks, which is the correct pattern for dialog components. The preset list rendering with edit/delete actions is well-structured.
hooks/useCalendars.ts (1)
92-94: Consider memoizingfetchCalendarsor note the dependency caveat.
fetchCalendarsreferencesinitialCalendarIdandselectedCalendarbut the effect only runs once (empty deps). This is intentional for initial load, but ifrefetchCalendarsis exposed and called later, the stale closure issues noted above would manifest. If refetching should respect the current selection, consider wrapping inuseCallbackwith appropriate deps.components/shift-dialog.tsx (1)
49-61: Good refactor to hook-based form management.The extraction of form state to
useShiftFormhook improves separation of concerns. The hook properly handles state initialization when the dialog opens (as shown in the relevant code snippet foruseShiftFormlines 105-120).components/shifts-list.tsx (1)
1-8: LGTM - Proper imports and dependencies.Good use of locale-specific date formatters (
de,enUS) from date-fns, and proper integration with next-intl for translations. As per coding guidelines, this follows the patternlocale === "de" ? de : enUS.app/page.tsx (3)
53-82: Good hook-based data management refactor.Clean extraction of data management to custom hooks. The hooks follow consistent patterns for fetching, mutations, and refetching. URL sync via
useRouter().replace()is maintained as per guidelines.
115-124: SSE connection properly wired to hooks.Good integration of
useSSEConnectionwith the refetch callbacks. TheonStatsRefreshcorrectly increments the trigger counter as per guidelines.
361-408: Shift toggle logic correctly implements check-then-act pattern.The
handleAddShiftfunction properly checks if a matching shift exists (by title, startTime, endTime on the same day) and deletes if present or creates if not. This aligns with the learning: "Shift toggle logic must check if matching shift exists: delete if present, create if not."components/preset-selector.tsx (2)
7-10: Good componentization of preset management.Clean extraction of UI into
PresetList,PresetEditDialog, andPresetManageDialogcomponents. TheusePasswordProtectionhook centralizes password verification logic.
130-143: Dialog cascade handled correctly.Good pattern: closing
PresetManageDialogbefore openingPresetEditDialogprevents dialog stacking issues.
…hooks and components
…e note handling logic
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/api/notes/route.ts (1)
9-54: Critical security gap: GET route bypasses password protection.The GET endpoint allows anyone to read notes from password-protected calendars without authentication. While the POST handler (lines 82-90) correctly verifies passwords before creating notes, the GET handler has no such protection. This defeats the purpose of calendar password protection.
Apply this diff to add password verification to the GET route:
export async function GET(request: Request) { try { const { searchParams } = new URL(request.url); const calendarId = searchParams.get("calendarId"); const date = searchParams.get("date"); + const password = searchParams.get("password"); if (!calendarId) { return NextResponse.json( { error: "Calendar ID is required" }, { status: 400 } ); } + // Fetch calendar to check password + const [calendar] = await db + .select() + .from(calendars) + .where(eq(calendars.id, calendarId)); + + if (!calendar) { + return NextResponse.json( + { error: "Calendar not found" }, + { status: 404 } + ); + } + + // Verify password if calendar is protected + if (calendar.passwordHash) { + if (!password || !verifyPassword(password, calendar.passwordHash)) { + return NextResponse.json( + { error: "Invalid password" }, + { status: 401 } + ); + } + } + let query = db .select()
🧹 Nitpick comments (3)
hooks/usePresets.ts (1)
7-31: Optional: guard against stale preset responses when calendar changes quicklyIf the user switches calendars fast, an earlier
/api/presetsresponse for a previouscalendarIdcan arrive after a later one and overwritepresetswith stale data. Not a blocker, but you could tighten this by tracking the active request, e.g. with anAbortControlleror acurrentCalendarIdRefcheck before callingsetPresets.components/calendar-grid.tsx (1)
35-45: Minor: make long‑press timer typing/browser semantics a bit more robust
pressTimerRefis typed asRecord<string, NodeJS.Timeout>, which is Node‑centric; in the browsersetTimeoutreturns anumber. Not a bug today, but you can make this component more platform‑agnostic by switching to:const pressTimerRef = useRef<Record<string, ReturnType<typeof setTimeout>>>({});The rest of the logic (per‑day keys, cleanup in the unmount effect, and clearing on touch end/move) looks solid and matches the required calendar interactions.
Also applies to: 87-100
app/page.tsx (1)
382-470: Shift toggle behavior is correct; consider functional state updates for robustnessThe
handleAddShiftimplementation correctly follows the requirement: with a selected preset, left‑clicking a day checks for a matching shift (same day/title/start/end) and deletes it if present, otherwise creates a new one usingformatDateToLocal. This aligns with the documented shift toggle behavior. As per coding guidelines, this is the right logic.One minor robustness issue is that both the optimistic delete and the rollback use the captured
shiftsarray:setShifts(shifts.filter((s) => s.id !== existingShift.id)); // ... setShifts(shifts);If
shiftschanges between the click and the async DELETE completing (e.g., because SSE or some other action updated shifts), these calls can clobber newer state.You can avoid that by switching to functional updates, for example:
setShifts((prev) => prev.filter((s) => s.id !== existingShift.id)); // ... setShifts((prev) => prev); // or refetchShifts() instead of trying to roll backThe same applies to the rollback paths. Not critical, but it makes the toggle logic safer under concurrent updates.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
app/api/notes/[id]/route.ts(3 hunks)app/api/notes/route.ts(3 hunks)app/page.tsx(16 hunks)components/app-header.tsx(1 hunks)components/calendar-grid.tsx(1 hunks)components/preset-selector.tsx(3 hunks)hooks/useCalendars.ts(1 hunks)hooks/useNotes.ts(1 hunks)hooks/usePresets.ts(1 hunks)hooks/useSSEConnection.ts(1 hunks)hooks/useShiftForm.ts(1 hunks)hooks/useShifts.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- hooks/useShifts.ts
- hooks/useShiftForm.ts
- hooks/useCalendars.ts
🧰 Additional context used
📓 Path-based instructions (8)
app/api/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
app/api/**/*.ts: Always await dynamic route params in API routes and server components (Next.js 16 breaking change)
Create API route handlers for new database tables at app/api/tablename/route.ts and app/api/tablename/[id]/route.ts
Log all API errors with console.error() for debugging
Files:
app/api/notes/[id]/route.tsapp/api/notes/route.ts
app/api/**/route.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
API GET routes must validate required query parameters and return 400 status for missing params
Files:
app/api/notes/[id]/route.tsapp/api/notes/route.ts
app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use useTranslations() hook for all user-facing text and t("key") to access translation keys from messages/{de,en}.json
Files:
app/api/notes/[id]/route.tsapp/api/notes/route.tsapp/page.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Store colors as hex values (e.g., #3b82f6) and apply 20% opacity for backgrounds using format ${color}20
Files:
app/api/notes/[id]/route.tsapp/api/notes/route.tshooks/useNotes.tscomponents/calendar-grid.tsxhooks/useSSEConnection.tshooks/usePresets.tscomponents/app-header.tsxapp/page.tsxcomponents/preset-selector.tsx
components/**/*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use PRESET_COLORS constant array for all color picker options
Files:
components/calendar-grid.tsxcomponents/app-header.tsxcomponents/preset-selector.tsx
components/**/calendar*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Calendar interaction: left-click toggles shift (requires preset selection), right-click opens note dialog with e.preventDefault()
Files:
components/calendar-grid.tsx
app/page.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
app/page.tsx: Password verification for calendar access must use pendingAction state pattern: set pending action, show PasswordDialog, retry after password verification via /api/calendars/[id]/verify-password
Main page (app/page.tsx) uses URL sync via useRouter().replace() for selected calendar
Refresh data after mutations by incrementing statsRefreshTrigger counter
Files:
app/page.tsx
app/**/*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
app/**/*.tsx: Use formatDateToLocal() helper for consistent YYYY-MM-DD date formatting in UI
Shift toggle logic must check if matching shift exists: delete if present, create if not
Use locale-specific date formatters in components: locale === "de" ? de : enUS
Frontend uses Next.js 16 App Router (app/ directory), React 19, and client-side state management via useState/useEffect
Files:
app/page.tsx
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/**/*.tsx : Shift toggle logic must check if matching shift exists: delete if present, create if not
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/page.tsx : Password verification for calendar access must use pendingAction state pattern: set pending action, show PasswordDialog, retry after password verification via /api/calendars/[id]/verify-password
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/page.tsx : Main page (app/page.tsx) uses URL sync via useRouter().replace() for selected calendar
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to components/**/calendar*.tsx : Calendar interaction: left-click toggles shift (requires preset selection), right-click opens note dialog with e.preventDefault()
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/page.tsx : Password verification for calendar access must use pendingAction state pattern: set pending action, show PasswordDialog, retry after password verification via /api/calendars/[id]/verify-password
Applied to files:
app/api/notes/[id]/route.tsapp/api/notes/route.tscomponents/app-header.tsxapp/page.tsxcomponents/preset-selector.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to components/**/calendar*.tsx : Calendar interaction: left-click toggles shift (requires preset selection), right-click opens note dialog with e.preventDefault()
Applied to files:
components/calendar-grid.tsxhooks/useSSEConnection.tshooks/usePresets.tscomponents/app-header.tsxapp/page.tsxcomponents/preset-selector.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/**/*.tsx : Use formatDateToLocal() helper for consistent YYYY-MM-DD date formatting in UI
Applied to files:
components/calendar-grid.tsxapp/page.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to components/**/*.tsx : Use PRESET_COLORS constant array for all color picker options
Applied to files:
hooks/usePresets.tscomponents/preset-selector.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/page.tsx : Main page (app/page.tsx) uses URL sync via useRouter().replace() for selected calendar
Applied to files:
components/app-header.tsxapp/page.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/**/*.tsx : Frontend uses Next.js 16 App Router (app/ directory), React 19, and client-side state management via useState/useEffect
Applied to files:
components/app-header.tsxapp/page.tsxcomponents/preset-selector.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/**/*.tsx : Shift toggle logic must check if matching shift exists: delete if present, create if not
Applied to files:
app/page.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/**/*.tsx : Use locale-specific date formatters in components: locale === "de" ? de : enUS
Applied to files:
app/page.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/page.tsx : Refresh data after mutations by incrementing statsRefreshTrigger counter
Applied to files:
app/page.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to components/**/*-dialog.tsx : Form submission in dialogs: prevent default, validate, call parent callback, close dialog
Applied to files:
app/page.tsxcomponents/preset-selector.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to components/**/*-dialog.tsx : Dialog components must control open state via props, reset internal state when open changes to false, and call parent callback on form submission
Applied to files:
app/page.tsxcomponents/preset-selector.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/**/*.{ts,tsx} : Use useTranslations() hook for all user-facing text and t("key") to access translation keys from messages/{de,en}.json
Applied to files:
components/preset-selector.tsx
🧬 Code graph analysis (7)
app/api/notes/[id]/route.ts (3)
lib/db/index.ts (1)
db(19-19)lib/db/schema.ts (2)
calendarNotes(71-86)calendars(4-17)lib/password-utils.ts (1)
verifyPassword(16-23)
app/api/notes/route.ts (3)
lib/db/index.ts (1)
db(19-19)lib/db/schema.ts (1)
calendars(4-17)lib/password-utils.ts (1)
verifyPassword(16-23)
hooks/useNotes.ts (2)
lib/db/schema.ts (1)
CalendarNote(94-94)lib/date-utils.ts (1)
formatDateToLocal(5-10)
components/calendar-grid.tsx (2)
lib/types.ts (1)
ShiftWithCalendar(14-31)lib/db/schema.ts (2)
CalendarNote(94-94)shifts(19-45)
hooks/usePresets.ts (1)
lib/db/schema.ts (1)
ShiftPreset(92-92)
components/app-header.tsx (4)
lib/types.ts (1)
CalendarWithCount(4-12)lib/db/schema.ts (2)
ShiftPreset(92-92)calendars(4-17)components/calendar-selector.tsx (1)
CalendarSelector(24-86)components/preset-selector.tsx (1)
PresetSelector(25-189)
components/preset-selector.tsx (5)
lib/db/schema.ts (1)
ShiftPreset(92-92)hooks/usePasswordProtection.ts (1)
usePasswordProtection(8-64)components/preset-edit-dialog.tsx (2)
PresetFormData(252-252)PresetEditDialog(47-250)components/preset-list.tsx (1)
PresetList(15-126)components/preset-manage-dialog.tsx (1)
PresetManageDialog(22-102)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-and-push
🔇 Additional comments (7)
app/api/notes/route.ts (3)
3-3: LGTM! Imports support password verification.The new imports for
calendarsschema andverifyPasswordutility are correctly used in the POST handler's password verification logic.Also applies to: 6-6
82-90: Password verification logic is correct.The password verification properly checks for the presence of
passwordHashand validates the provided password using theverifyPasswordutility. The 401 response for invalid passwords is appropriate.
69-80: Foreign key constraint is properly configured.The database schema already enforces the foreign key constraint on
calendarNotes.calendarIdwith cascade delete (seelib/db/schema.ts:77). This provides database-level protection against orphaned notes and race conditions, making the application-level calendar existence check redundant but still good practice for early error feedback.app/api/notes/[id]/route.ts (2)
3-3: LGTM!The additional imports for
calendarsschema andverifyPasswordutility are necessary and correctly used for the password verification flow.Also applies to: 6-6
45-45: LGTM! Password verification flow is correctly implemented.The PUT handler properly:
- Extracts password from the request body (secure for PUT operations)
- Fetches the existing note to obtain the calendarId
- Loads the related calendar to check password protection
- Verifies the password before allowing updates
- Returns appropriate HTTP status codes (404 for not found, 401 for unauthorized)
This aligns with the password-protected calendar pattern used throughout the application.
Also applies to: 51-85
components/preset-selector.tsx (1)
61-153: Preset CRUD flow and password integration look solidThe refactored
handleSavePreset/handleDeletePresetlogic is cohesive:
- All mutations go through
usePasswordProtection.withPasswordCheck, so protected calendars consistently trigger the sharedonPasswordRequiredflow.- Responses are validated with
response.ok, error bodies are logged, and user feedback is surfaced via toasts for both save and delete paths.- On success you call
onPresetsChange(andonShiftsChangewhen relevant) and close/reset the edit dialog state, which keeps the UI in sync.This is a nice improvement over inline preset handling and matches the centralized password‑protection approach introduced elsewhere in the PR.
hooks/useNotes.ts (1)
11-27: No changes needed—GET /api/notes does not require password verificationThe review's concern assumes that all note API endpoints require password protection for protected calendars. However, examining the actual API implementation shows that only write operations (POST, PUT, DELETE) verify passwords. The GET endpoints for fetching notes have no password checks, so
fetchNoteswill never encounter a 401 error. This is an intentional design: read operations are public, while write operations are protected. The code is correctly aligned with this model.Likely an incorrect or invalid review comment.
…r DELETE operations
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
app/api/notes/[id]/route.ts (1)
129-140: Past security concern addressed - password now read from request body.This change correctly addresses the previous review comment about password exposure in query strings. The password is now securely read from the request body.
app/page.tsx (2)
37-37: Remove unusedShiftPresetimport.
ShiftPresetis imported but not used in this file. OnlyCalendarNoteis referenced.-import { CalendarNote, ShiftPreset } from "@/lib/db/schema"; +import { CalendarNote } from "@/lib/db/schema";
326-335: Remove unusedhandleUpdateShiftfunction.This function is defined but never called or passed to any component.
ShiftsListonly receivesonDeleteShift(line 668). If shift editing functionality is planned, consider adding it; otherwise, remove to reduce dead code.
🧹 Nitpick comments (5)
app/api/presets/[id]/route.ts (1)
114-126: Password extraction from request body looks good, but consider logging parse failures.The change from URL params to request body for password handling is a security improvement. However, the silent catch block on line 123-125 swallows parsing errors without any logging, which could make debugging difficult if clients send malformed JSON.
try { const body = await request.json(); password = body.password || null; - } catch (e) { + } catch { // If body parsing fails, continue with null password + console.warn("Failed to parse request body in DELETE /api/presets/[id]"); }app/api/shifts/[id]/route.ts (1)
147-159: Consistent password extraction pattern - same silent catch concern.The password extraction from request body matches the pattern in other DELETE handlers. Same suggestion as for presets: consider adding a warning log in the catch block for debugging purposes.
try { const body = await request.json(); password = body.password || null; - } catch (e) { + } catch { // If body parsing fails, continue with null password + console.warn("Failed to parse request body in DELETE /api/shifts/[id]"); }app/api/calendars/[id]/route.ts (1)
108-120: Consistent password extraction pattern.Same pattern as other DELETE handlers. Consider adding warning log in catch block for debugging.
try { const body = await request.json(); password = body.password || null; - } catch (e) { + } catch { // If body parsing fails, continue with null password + console.warn("Failed to parse request body in DELETE /api/calendars/[id]"); }hooks/useSSEConnection.ts (1)
5-13: Consider removing unusedisConnectedparameter.The
isConnectedparameter is destructured but never used within the hook. OnlysetIsConnectedis used. If it's not needed, removing it would simplify the interface.interface SSEConnectionOptions { calendarId: string | undefined; onShiftUpdate: () => void; onPresetUpdate: () => void; onNoteUpdate: () => void; onStatsRefresh: () => void; - isConnected: boolean; setIsConnected: (connected: boolean) => void; } export function useSSEConnection({ calendarId, onShiftUpdate, onPresetUpdate, onNoteUpdate, onStatsRefresh, - isConnected, setIsConnected, }: SSEConnectionOptions) {Also applies to: 21-21
components/app-header.tsx (1)
103-116: Consider caching the calendar lookup.The
calendars.find((c) => c.id === selectedCalendar)is called twice in the gradient style computation. While not a performance blocker, extracting to a variable would improve readability.+ const selectedCalendarData = calendars.find((c) => c.id === selectedCalendar); <div className="w-1 h-8 bg-gradient-to-b rounded-full transition-colors duration-300" style={{ backgroundImage: selectedCalendar ? `linear-gradient(to bottom, ${ - calendars.find((c) => c.id === selectedCalendar) - ?.color || "hsl(var(--primary))" + selectedCalendarData?.color || "hsl(var(--primary))" }, ${ - calendars.find((c) => c.id === selectedCalendar) - ?.color || "hsl(var(--primary))" + selectedCalendarData?.color || "hsl(var(--primary))" }80)` : "linear-gradient(to bottom, hsl(var(--primary)), hsl(var(--primary) / 0.5))", }} ></div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
app/api/calendars/[id]/route.ts(1 hunks)app/api/notes/[id]/route.ts(3 hunks)app/api/presets/[id]/route.ts(1 hunks)app/api/shifts/[id]/route.ts(1 hunks)app/page.tsx(16 hunks)components/app-header.tsx(1 hunks)components/preset-selector.tsx(3 hunks)hooks/useCalendars.ts(1 hunks)hooks/useNotes.ts(1 hunks)hooks/useSSEConnection.ts(1 hunks)hooks/useShifts.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- hooks/useNotes.ts
- hooks/useCalendars.ts
- hooks/useShifts.ts
🧰 Additional context used
📓 Path-based instructions (7)
app/api/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
app/api/**/*.ts: Always await dynamic route params in API routes and server components (Next.js 16 breaking change)
Create API route handlers for new database tables at app/api/tablename/route.ts and app/api/tablename/[id]/route.ts
Log all API errors with console.error() for debugging
Files:
app/api/notes/[id]/route.tsapp/api/calendars/[id]/route.tsapp/api/shifts/[id]/route.tsapp/api/presets/[id]/route.ts
app/api/**/route.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
API GET routes must validate required query parameters and return 400 status for missing params
Files:
app/api/notes/[id]/route.tsapp/api/calendars/[id]/route.tsapp/api/shifts/[id]/route.tsapp/api/presets/[id]/route.ts
app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use useTranslations() hook for all user-facing text and t("key") to access translation keys from messages/{de,en}.json
Files:
app/api/notes/[id]/route.tsapp/api/calendars/[id]/route.tsapp/api/shifts/[id]/route.tsapp/api/presets/[id]/route.tsapp/page.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Store colors as hex values (e.g., #3b82f6) and apply 20% opacity for backgrounds using format ${color}20
Files:
app/api/notes/[id]/route.tsapp/api/calendars/[id]/route.tsapp/api/shifts/[id]/route.tsapp/api/presets/[id]/route.tscomponents/app-header.tsxhooks/useSSEConnection.tsapp/page.tsxcomponents/preset-selector.tsx
components/**/*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use PRESET_COLORS constant array for all color picker options
Files:
components/app-header.tsxcomponents/preset-selector.tsx
app/page.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
app/page.tsx: Password verification for calendar access must use pendingAction state pattern: set pending action, show PasswordDialog, retry after password verification via /api/calendars/[id]/verify-password
Main page (app/page.tsx) uses URL sync via useRouter().replace() for selected calendar
Refresh data after mutations by incrementing statsRefreshTrigger counter
Files:
app/page.tsx
app/**/*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
app/**/*.tsx: Use formatDateToLocal() helper for consistent YYYY-MM-DD date formatting in UI
Shift toggle logic must check if matching shift exists: delete if present, create if not
Use locale-specific date formatters in components: locale === "de" ? de : enUS
Frontend uses Next.js 16 App Router (app/ directory), React 19, and client-side state management via useState/useEffect
Files:
app/page.tsx
🧠 Learnings (12)
📓 Common learnings
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/**/*.tsx : Shift toggle logic must check if matching shift exists: delete if present, create if not
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/page.tsx : Password verification for calendar access must use pendingAction state pattern: set pending action, show PasswordDialog, retry after password verification via /api/calendars/[id]/verify-password
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/page.tsx : Main page (app/page.tsx) uses URL sync via useRouter().replace() for selected calendar
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/**/*.tsx : Frontend uses Next.js 16 App Router (app/ directory), React 19, and client-side state management via useState/useEffect
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/page.tsx : Password verification for calendar access must use pendingAction state pattern: set pending action, show PasswordDialog, retry after password verification via /api/calendars/[id]/verify-password
Applied to files:
app/api/notes/[id]/route.tsapp/api/calendars/[id]/route.tsapp/api/shifts/[id]/route.tsapp/api/presets/[id]/route.tscomponents/app-header.tsxapp/page.tsxcomponents/preset-selector.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to components/**/calendar*.tsx : Calendar interaction: left-click toggles shift (requires preset selection), right-click opens note dialog with e.preventDefault()
Applied to files:
components/app-header.tsxhooks/useSSEConnection.tsapp/page.tsxcomponents/preset-selector.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/page.tsx : Main page (app/page.tsx) uses URL sync via useRouter().replace() for selected calendar
Applied to files:
components/app-header.tsxapp/page.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/**/*.tsx : Frontend uses Next.js 16 App Router (app/ directory), React 19, and client-side state management via useState/useEffect
Applied to files:
components/app-header.tsxapp/page.tsxcomponents/preset-selector.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/**/*.tsx : Use formatDateToLocal() helper for consistent YYYY-MM-DD date formatting in UI
Applied to files:
app/page.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/**/*.tsx : Shift toggle logic must check if matching shift exists: delete if present, create if not
Applied to files:
app/page.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/**/*.tsx : Use locale-specific date formatters in components: locale === "de" ? de : enUS
Applied to files:
app/page.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to components/**/*-dialog.tsx : Form submission in dialogs: prevent default, validate, call parent callback, close dialog
Applied to files:
app/page.tsxcomponents/preset-selector.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to components/**/*-dialog.tsx : Dialog components must control open state via props, reset internal state when open changes to false, and call parent callback on form submission
Applied to files:
app/page.tsxcomponents/preset-selector.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to components/**/*.tsx : Use PRESET_COLORS constant array for all color picker options
Applied to files:
components/preset-selector.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/**/*.{ts,tsx} : Use useTranslations() hook for all user-facing text and t("key") to access translation keys from messages/{de,en}.json
Applied to files:
components/preset-selector.tsx
🧬 Code graph analysis (1)
components/preset-selector.tsx (5)
lib/db/schema.ts (1)
ShiftPreset(92-92)hooks/usePasswordProtection.ts (1)
usePasswordProtection(8-64)components/preset-edit-dialog.tsx (2)
PresetFormData(252-252)PresetEditDialog(47-250)components/preset-list.tsx (1)
PresetList(15-126)components/preset-manage-dialog.tsx (1)
PresetManageDialog(22-102)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-and-push
🔇 Additional comments (25)
app/api/presets/[id]/route.ts (1)
161-173: LGTM!The cascading delete logic correctly removes related shifts before deleting the preset, and the SSE event emission ensures real-time UI updates. Password verification flow is properly implemented.
app/api/shifts/[id]/route.ts (1)
191-199: LGTM!The delete operation and SSE event emission are correctly implemented. The password verification flow properly protects the operation when the calendar is protected.
app/api/notes/[id]/route.ts (1)
51-85: LGTM! Password protection properly added to PUT handler.The implementation correctly:
- Fetches the existing note to get the calendar ID
- Verifies the calendar exists
- Validates password if the calendar is protected
This aligns with the broader PR's password-protection flow.
hooks/useSSEConnection.ts (3)
31-54: LGTM! Ref pattern properly addresses stale closure concerns.The implementation correctly uses refs to hold the latest callback references and updates them in a separate effect. This ensures SSE handlers always use current callbacks without causing unnecessary reconnections when callbacks change.
170-209: SSE error handling correctly relies on auto-reconnect.The
onerrorhandler properly avoids callingeventSource.close(), allowing the browser's built-in EventSource auto-reconnect mechanism to work. The resync timeout provides additional data refresh while waiting for reconnection. This addresses the past review concern about the stream being closed and never re-established.
228-231: LGTM! Clean hook return interface.Exposing
lastSyncTimeRefanddisconnectTimeRefallows consumers to inspect sync state when needed. The overall hook implementation is well-structured with proper cleanup.app/api/calendars/[id]/route.ts (1)
145-147: No action needed. The database schema hasON DELETE CASCADEproperly configured for all foreign key relationships to the calendars table:
shifts.calendarId→calendars.idwithonDelete: "cascade"shiftPresets.calendarId→calendars.idwithonDelete: "cascade"calendarNotes.calendarId→calendars.idwithonDelete: "cascade"The simple deletion approach in the API route correctly relies on database-level cascade deletion to clean up related entities automatically, preventing orphaned records.
Likely an incorrect or invalid review comment.
components/preset-selector.tsx (5)
1-23: LGTM! Clean imports and well-defined props interface.The component structure follows good separation of concerns by delegating UI to child components (
PresetList,PresetEditDialog,PresetManageDialog) while handling business logic (API calls, password protection) here. TheuseTranslations()hook usage aligns with coding guidelines.
45-59: LGTM! Password-protected create/edit flows.The
withPasswordCheckwrapper correctly gates actions behind password verification before showing the edit dialog. The state setup differentiates between create (isCreatingNew=true) and edit modes properly.
61-117: LGTM! Comprehensive save logic with proper error handling.The function correctly handles both create (POST) and update (PATCH) paths with appropriate error logging and user feedback via toasts. The conditional
onShiftsChange()call on update (but not create) is logical since creating a preset doesn't affect existing shifts.
119-156: LGTM! Delete with proper confirmation and cleanup.The deletion flow correctly sequences password verification before confirmation dialog, handles errors consistently, and properly deselects the preset if it was the active selection. The
onShiftsChange()call ensures any cascading effects are reflected.
158-191: LGTM! Well-orchestrated dialog composition.The render correctly composes the three UI components with proper callback wiring. The pattern of closing
PresetManageDialogbefore openingPresetEditDialog(lines 172-179) prevents dialog stacking and provides smooth UX.components/app-header.tsx (4)
17-34: LGTM! Well-structured props interface.The
AppHeaderPropsinterface clearly defines all required props with appropriate types. The separation between callbacks (on*prefixed) and state props is clean and follows React conventions.
137-145: Verified: Mobile connection indicator fix applied.The mobile connection status indicator now correctly uses
isConnectedto toggle between green and red, matching the desktop behavior. This addresses the previous review comment.
203-216: Verified: PresetSelector conditional rendering fix applied.
PresetSelectoris now only rendered whenselectedCalendaris truthy, andcalendarIdreceives the actual value without an empty string fallback. This prevents invalid API calls and ensures proper password protection flow.
221-258: LGTM! Mobile dialog with proper state management.The mobile calendar dialog correctly closes before triggering parent actions (lines 243-253), preventing UI state conflicts. The
CalendarSelectorintegration maintains consistent behavior between desktop and mobile views.app/page.tsx (9)
53-83: LGTM! Clean hook-based data management setup.The custom hooks are properly initialized with appropriate dependencies. The aliasing pattern (e.g.,
createCalendar: createCalendarHook) prevents naming conflicts with local handlers and maintains clarity.
116-125: LGTM! SSE integration for real-time updates.The
useSSEConnectionhook correctly wires refetch callbacks to handle real-time updates for shifts, presets, and notes. ThestatsRefreshTriggerincrement ensures stats recalculate on relevant changes.
142-147: LGTM! URL sync follows coding guidelines.The URL synchronization correctly uses
router.replace()withscroll: falseto update the URL query parameter when the selected calendar changes, as per the coding guidelines.
104-110: LGTM! Extended pendingAction pattern for notes.The
pendingActionstate correctly includesnoteActionto support password-protected note operations, following the established pattern for shifts and presets per coding guidelines.
383-472: LGTM! Shift toggle logic follows guidelines.The function correctly implements the shift toggle pattern: checks for an existing shift by matching date, title, and times, deletes if present, creates if not. The optimistic update pattern (lines 430-431) with rollback on failure (lines 443-445, 451-453) provides good UX. Uses
formatDateToLocal()per coding guidelines.
559-576: LGTM! AppHeader integration is complete.All required props are correctly wired to local state, handlers, and hook functions. The callback mappings properly connect UI actions to data management functions.
614-625: LGTM! CalendarGrid properly configured.The component receives all necessary data and handlers for the calendar interaction model: left-click for shift toggle (
onDayClick), right-click/long-press for notes (onDayRightClick,onLongPress), per coding guidelines.
665-669: Verify: Is shift editing intentionally removed from ShiftsList?
ShiftsListreceivesonDeleteShiftbut notonUpdateShift. ThehandleUpdateShiftfunction exists but is unused. If editing functionality is expected, ensure it's wired; if intentionally removed, delete the unused handler.
692-750: LGTM! Dialog configurations properly updated.All dialogs are correctly configured with hook-based callbacks (
createCalendarHook,refetchPresets,refetchCalendars) replacing the previous inline implementations. Conditional rendering for password-related dialogs prevents issues when no calendar is selected.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
components/calendar-grid.tsx (2)
28-28: Unused propisTogglingShift.The
isTogglingShiftprop is destructured but never used in the component. Either remove it from the interface and component signature, or implement the intended behavior (e.g., disabling interaction during shift toggle operations).interface CalendarGridProps { calendarDays: Date[]; currentDate: Date; shifts: ShiftWithCalendar[]; notes: CalendarNote[]; selectedPresetId: string | undefined; - isTogglingShift: boolean; onDayClick: (date: Date) => void; onDayRightClick: (e: React.MouseEvent, date: Date) => void; onNoteIconClick: (e: React.MouseEvent, date: Date) => void; onLongPress: (date: Date) => void; } export function CalendarGrid({ calendarDays, currentDate, shifts, notes, selectedPresetId, - isTogglingShift, onDayClick, onDayRightClick, onNoteIconClick, onLongPress, }: CalendarGridProps) {
110-110: Remove redundantdisabled={false}.The
disabledattribute defaults tofalse, so this explicit assignment is unnecessary.onTouchMove={handleTouchEnd} - disabled={false} whileTap={{ scale: 0.95 }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/calendar-grid.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
components/**/*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use PRESET_COLORS constant array for all color picker options
Files:
components/calendar-grid.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Store colors as hex values (e.g., #3b82f6) and apply 20% opacity for backgrounds using format ${color}20
Files:
components/calendar-grid.tsx
components/**/calendar*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Calendar interaction: left-click toggles shift (requires preset selection), right-click opens note dialog with e.preventDefault()
Files:
components/calendar-grid.tsx
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/**/*.tsx : Shift toggle logic must check if matching shift exists: delete if present, create if not
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/page.tsx : Password verification for calendar access must use pendingAction state pattern: set pending action, show PasswordDialog, retry after password verification via /api/calendars/[id]/verify-password
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to components/**/calendar*.tsx : Calendar interaction: left-click toggles shift (requires preset selection), right-click opens note dialog with e.preventDefault()
Applied to files:
components/calendar-grid.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/**/*.tsx : Use formatDateToLocal() helper for consistent YYYY-MM-DD date formatting in UI
Applied to files:
components/calendar-grid.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/**/*.tsx : Use locale-specific date formatters in components: locale === "de" ? de : enUS
Applied to files:
components/calendar-grid.tsx
🧬 Code graph analysis (1)
components/calendar-grid.tsx (2)
lib/types.ts (1)
ShiftWithCalendar(14-31)lib/db/schema.ts (2)
CalendarNote(94-94)shifts(19-45)
🔇 Additional comments (2)
components/calendar-grid.tsx (2)
37-45: Timer cleanup implementation looks good.The use of
useRefwith aRecord<string, NodeJS.Timeout>pattern and proper cleanup inuseEffectcorrectly addresses the memory leak concern from the previous review. Each day cell has its own timer keyed bydayKey, and all timers are cleared on unmount.Also applies to: 89-100
159-200: Shift rendering follows color conventions.The background color implementation using
${shift.color}20correctly applies 20% opacity as per coding guidelines, with appropriate fallback to the default color#3b82f620.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
components/calendar-grid.tsx (1)
35-45: Calendar interactions, long‑press, and color handling look correct
- Long‑press timers are now scoped per day via
pressTimerRefand fully cleaned up inuseEffect, addressing prior leak/scoping concerns.- Right‑click calls
e.preventDefault()before delegating toonDayRightClick, matching the calendar interaction guideline.- Shift colors use hex +
${color}20opacity and sensible fallbacks ("#3b82f620"/"#3b82f6"), aligning with the color storage/usage rules.Optionally, you could also wire
onTouchCancel={handleTouchEnd}on the button to cancel timers on interrupted touches, but the current implementation is already sound. As per coding guidelines, the interaction and color behavior here looks good.Also applies to: 87-113, 163-172
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/calendar-grid.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
components/**/*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use PRESET_COLORS constant array for all color picker options
Files:
components/calendar-grid.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Store colors as hex values (e.g., #3b82f6) and apply 20% opacity for backgrounds using format ${color}20
Files:
components/calendar-grid.tsx
components/**/calendar*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Calendar interaction: left-click toggles shift (requires preset selection), right-click opens note dialog with e.preventDefault()
Files:
components/calendar-grid.tsx
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/**/*.tsx : Shift toggle logic must check if matching shift exists: delete if present, create if not
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/page.tsx : Password verification for calendar access must use pendingAction state pattern: set pending action, show PasswordDialog, retry after password verification via /api/calendars/[id]/verify-password
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/page.tsx : Main page (app/page.tsx) uses URL sync via useRouter().replace() for selected calendar
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to components/**/calendar*.tsx : Calendar interaction: left-click toggles shift (requires preset selection), right-click opens note dialog with e.preventDefault()
Applied to files:
components/calendar-grid.tsx
📚 Learning: 2025-11-29T20:11:49.992Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T20:11:49.992Z
Learning: Applies to app/**/*.tsx : Use formatDateToLocal() helper for consistent YYYY-MM-DD date formatting in UI
Applied to files:
components/calendar-grid.tsx
🧬 Code graph analysis (1)
components/calendar-grid.tsx (2)
lib/types.ts (1)
ShiftWithCalendar(14-31)lib/db/schema.ts (2)
CalendarNote(94-94)shifts(19-45)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-and-push
| interface CalendarGridProps { | ||
| calendarDays: Date[]; | ||
| currentDate: Date; | ||
| shifts: ShiftWithCalendar[]; | ||
| notes: CalendarNote[]; | ||
| selectedPresetId: string | undefined; | ||
| isTogglingShift: boolean; | ||
| onDayClick: (date: Date) => void; | ||
| onDayRightClick: (e: React.MouseEvent, date: Date) => void; | ||
| onNoteIconClick: (e: React.MouseEvent, date: Date) => void; | ||
| onLongPress: (date: Date) => void; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the components/calendar-grid.tsx file
fd "calendar-grid.tsx" --type fRepository: panteLx/BetterShift
Length of output: 91
🏁 Script executed:
# Get the full contents of the file to check imports and type usage
cat -n components/calendar-grid.tsx | head -150Repository: panteLx/BetterShift
Length of output: 6035
Add React type import for event type annotations
The file uses React.MouseEvent (lines 17–18) and React.TouchEvent (line 89) in the CalendarGridProps interface and event handlers, but React isn't imported as a type, causing TypeScript compilation errors. Add a type-only import:
+import type React from "react";
import { useRef, useEffect } from "react";Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In components/calendar-grid.tsx around lines 9 to 20, the props use
React.MouseEvent and React.TouchEvent but React isn't imported as a type; add a
type-only import at the top of the file (for example import type React from
'react' or import type { MouseEvent, TouchEvent } from 'react') and update the
props to use those imported type names if you choose specific imports, ensuring
TypeScript recognizes the event types without bundling React at runtime.
| <motion.button | ||
| key={idx} | ||
| onClick={() => onDayClick(day)} | ||
| onContextMenu={(e) => { | ||
| e.preventDefault(); | ||
| onDayRightClick(e, day); | ||
| }} | ||
| onTouchStart={handleTouchStart} | ||
| onTouchEnd={handleTouchEnd} | ||
| onTouchMove={handleTouchEnd} | ||
| disabled={false} | ||
| whileTap={{ scale: 0.95 }} | ||
| style={{ | ||
| WebkitUserSelect: "none", | ||
| userSelect: "none", | ||
| WebkitTouchCallout: "none", | ||
| }} | ||
| className={` | ||
| min-h-25 sm:min-h-28 px-1 py-1.5 sm:p-2.5 rounded-md sm:rounded-lg text-sm transition-all relative flex flex-col border sm:border-2 | ||
| ${isCurrentMonth ? "text-foreground" : "text-muted-foreground/50"} | ||
| ${ | ||
| isTodayDate | ||
| ? "border-primary shadow-lg shadow-primary/20 bg-primary/5 ring-2 ring-primary/20" | ||
| : "border-border/30 sm:border-border/50" | ||
| } | ||
| ${ | ||
| isCurrentMonth | ||
| ? "hover:bg-accent cursor-pointer active:bg-accent/80 hover:border-border" | ||
| : selectedPresetId | ||
| ? "cursor-not-allowed" | ||
| : "cursor-pointer" | ||
| } | ||
| ${!isCurrentMonth ? "opacity-40" : ""} | ||
| `} | ||
| > | ||
| <div | ||
| className={`text-sm sm:text-sm font-semibold mb-1 flex items-center justify-between ${ | ||
| isTodayDate ? "text-primary" : "" | ||
| }`} | ||
| > | ||
| <span>{day.getDate()}</span> | ||
| {dayNote && ( | ||
| <motion.div | ||
| className="group/note relative" | ||
| onClick={(e) => onNoteIconClick(e, day)} | ||
| title={dayNote.note} | ||
| whileHover={{ scale: 1.1 }} | ||
| whileTap={{ scale: 0.9 }} | ||
| > |
There was a problem hiding this comment.
Prevent note icon clicks from also toggling shifts (stop click bubbling)
Because the note icon is inside the motion.button day cell, clicking it will fire both onNoteIconClick and the parent onDayClick, unintentionally toggling shifts when the user just wants to open/edit a note.
Stop propagation in the note icon handler:
- <motion.div
- className="group/note relative"
- onClick={(e) => onNoteIconClick(e, day)}
+ <motion.div
+ className="group/note relative"
+ onClick={(e) => {
+ e.stopPropagation();
+ onNoteIconClick(e, day);
+ }}This keeps left‑click toggling behavior on the day cell while making the note icon strictly note-focused. Based on learnings, this better matches the intended calendar interactions.
🤖 Prompt for AI Agents
In components/calendar-grid.tsx around lines 103 to 151, the note icon's onClick
resides inside the parent motion.button so clicks bubble and trigger the parent
onDayClick; update the note icon handler to stop event propagation before
delegating to onNoteIconClick (e.g., call e.stopPropagation() at start of the
click handler) so clicking the note icon only opens/edits notes and does not
toggle shifts on the parent cell. Ensure the handler accepts the event parameter
and invokes stopPropagation for mouse/touch events as appropriate.
1 participant
Summary by CodeRabbit
New Features
Refactor
Bug Fixes / Security
✏️ Tip: You can customize this high-level summary in your review settings.