feat: Adds calendar comparison mode and related UI/logic#85
Conversation
Adds a comparison mode to view and interact with multiple calendars side-by-side to simplify cross-calendar review and editing. Implements a compare selector and comparison view that load shifts, notes, presets and external syncs for selected calendars; enables creating/deleting shifts via presets with password verification and reloads compare-specific data after edits. Integrates compare controls into header and selector, adds localized strings, and adjusts several UI helpers (manage button hiding, sheet save label). Also includes small backend/UI fixes to ensure counts and pending actions include calendar context.
|
Caution Review failedThe pull request is closed. 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. WalkthroughAdds a calendar compare mode (select 2–3 calendars to view side‑by‑side) with new compare UI components, per‑calendar parallel data loading and password‑aware actions, a small API response alias change ( Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AppHeader
participant CalendarSelector
participant CalendarCompareSheet
participant AppPage
participant API
participant CalendarCompareView
User->>AppHeader: open header
AppHeader->>CalendarSelector: render (onCompare wired)
User->>CalendarSelector: click Compare
CalendarSelector->>CalendarCompareSheet: open modal
User->>CalendarCompareSheet: select 2–3 calendars + Start
CalendarCompareSheet->>AppPage: onStartCompare(selectedIds)
AppPage->>AppPage: set isCompareMode, selectedCompareIds
AppPage->>API: fetch per-calendar data (shifts, notes, externalSyncs, presets) [parallel]
API-->>AppPage: return per-calendar payloads (includes `_count`)
AppPage->>CalendarCompareView: render compare UI with data & handlers
User->>CalendarCompareView: interact (day click / note / shift)
CalendarCompareView->>AppPage: invoke handler (may require password)
AppPage->>API: create/update/delete shift or note
API-->>AppPage: confirm
AppPage->>AppPage: refresh affected calendar data
User->>CalendarCompareView: click Exit Compare
CalendarCompareView->>AppPage: onExitCompare
AppPage->>AppPage: unset isCompareMode
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested labels
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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 pull request introduces a calendar comparison mode that allows users to view and interact with 2-3 calendars side-by-side. The implementation includes a comparison selector sheet, a dedicated comparison view, localization strings for German/English/Italian, and infrastructure to manage data loading and interactions across multiple calendars simultaneously.
Key changes:
- New comparison UI components (CalendarCompareView, CalendarCompareSheet) with responsive grid layouts
- Calendar-specific data management using Map structures for shifts, notes, presets, and external syncs
- Password verification support for multi-calendar operations in compare mode
- Integration of compare mode toggle into calendar selector (desktop and mobile)
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| messages/de.json | Adds German translations for comparison mode UI strings |
| messages/en.json | Adds English translations for comparison mode UI strings |
| messages/it.json | Adds Italian translations for comparison mode UI strings |
| hooks/usePasswordManagement.ts | Extends PendingAction interface with calendarId field for multi-calendar support |
| components/ui/base-sheet.tsx | Adds saveLabel prop to customize save button text |
| components/preset-selector.tsx | Adds hideManageButton prop to hide preset management in compare mode |
| components/preset-list.tsx | Implements conditional rendering of manage button based on hideManageButton prop |
| components/calendar-selector.tsx | Adds compare button with "Copy" icon to both desktop and mobile layouts |
| components/calendar-compare-view.tsx | New component rendering side-by-side calendar views with shared preset selection |
| components/calendar-compare-sheet.tsx | New component for selecting 2-3 calendars to compare with visual feedback |
| components/app-header.tsx | Propagates onCompare handler to calendar selectors |
| app/page.tsx | Implements compare mode state management, multi-calendar data loading, and interaction handlers |
| app/api/calendars/route.ts | Renames SQL alias from "shift_count" to "_count" for consistency |
Comments suppressed due to low confidence (1)
app/page.tsx:350
- In compare mode, when a password is required for shift toggling, the handlePasswordSuccess function only reloads notes data, not shifts or presets. This means after entering a password and the pending action executes (creating/deleting a shift), the local state won't be updated because handlePasswordSuccess doesn't reload shifts for the specific calendar. The pendingAction should include the calendarId so that shifts can be reloaded for that calendar after password entry.
const handlePasswordSuccess = async () => {
if (isCompareMode && compareNoteCalendarId) {
// In compare mode, reload notes for the specific calendar
await refetchNotes();
const password = getCachedPassword(compareNoteCalendarId);
const passwordParam = password ? `&password=${password}` : "";
const notesRes = await fetch(
`/api/notes?calendarId=${compareNoteCalendarId}${passwordParam}`
);
const notesData = notesRes.ok ? await notesRes.json() : [];
setCompareCalendarData((prev) => {
const updated = new Map(prev);
const data = updated.get(compareNoteCalendarId);
if (data) {
updated.set(compareNoteCalendarId, { ...data, notes: notesData });
}
return updated;
});
} else {
await Promise.all([
refetchShifts(),
refetchPresets(),
refetchNotes(),
fetchExternalSyncs(),
fetchSyncErrorStatus(),
]);
setStatsRefreshTrigger((prev) => prev + 1);
}
baseHandlePasswordSuccess();
// Execute pending action if exists
if (pendingAction?.action) {
await pendingAction.action();
setPendingAction(null);
}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/api/calendars/route.ts (2)
10-27: CRITICAL: Remove passwordHash from API response.The GET endpoint exposes
passwordHashfor all calendars, creating a serious security vulnerability. Even though passwords are hashed, exposing hashes allows attackers to perform offline brute-force attacks or rainbow table lookups.🔎 Recommended fix
Remove
passwordHashfrom the select statement and only return metadata needed by the client:const allCalendars = await db .select({ id: calendars.id, name: calendars.name, color: calendars.color, - passwordHash: calendars.passwordHash, isLocked: calendars.isLocked, createdAt: calendars.createdAt, updatedAt: calendars.updatedAt, _count: sql<number>`(SELECT COUNT(*) FROM ${shifts} WHERE ${shifts.calendarId} = ${calendars.id})`.as( "_count" ), }) .from(calendars) .orderBy(calendars.createdAt);The
isLockedfield is sufficient for the client to know whether a calendar requires password verification.
58-68: CRITICAL: Remove passwordHash from creation response.The POST endpoint returns the created calendar including
passwordHash, exposing the hash to the client. This creates the same security vulnerability as the GET endpoint.🔎 Recommended fix
Explicitly select only the fields that should be returned to the client:
const [calendar] = await db .insert(calendars) .values({ name, color: color || "#3b82f6", passwordHash: password ? hashPassword(password) : null, isLocked: isLocked || false, }) - .returning(); + .returning({ + id: calendars.id, + name: calendars.name, + color: calendars.color, + isLocked: calendars.isLocked, + createdAt: calendars.createdAt, + updatedAt: calendars.updatedAt, + });
🧹 Nitpick comments (6)
components/calendar-compare-sheet.tsx (1)
72-79: Consider removing redundantonCheckedChangehandler.The
Checkboxhaspointer-events-none(line 78), which prevents mouse events from reaching it. This means theonCheckedChangehandler (lines 75-77) will never fire. All interaction is handled by the parentmotion.divonClick(line 67).While this doesn't cause bugs, the unused handler adds cognitive overhead.
🔎 Proposed refactor to remove unused handler
<Checkbox checked={isSelected} disabled={isDisabled} - onCheckedChange={() => - !isDisabled && onToggleCalendar(calendar.id) - } className="pointer-events-none" />components/calendar-compare-view.tsx (1)
81-90: Consider simplifying the IIFE pattern for finding preset owner.The IIFE is functional but could be more readable using
Array.from().find().🔎 Proposed refactor
- const selectedPresetCalendarId = props.selectedPresetId - ? (() => { - for (const [calendarId, presets] of props.presetsMap.entries()) { - if (presets.some((p) => p.id === props.selectedPresetId)) { - return calendarId; - } - } - return null; - })() - : null; + const selectedPresetCalendarId = props.selectedPresetId + ? Array.from(props.presetsMap.entries()).find(([, presets]) => + presets.some((p) => p.id === props.selectedPresetId) + )?.[0] ?? null + : null;app/page.tsx (4)
141-166: Extract duplicate fetch-and-update logic for compare notes.
handleNoteSubmitandhandleNoteDeleteshare nearly identical logic for reloading notes in compare mode. Consider extracting a helper function.🔎 Proposed refactor
// Add helper function before the handlers const reloadCompareNotes = async (calendarId: string) => { const password = getCachedPassword(calendarId); const passwordParam = password ? `&password=${password}` : ""; try { const notesRes = await fetch( `/api/notes?calendarId=${calendarId}${passwordParam}` ); const notesData = notesRes.ok ? await notesRes.json() : []; setCompareCalendarData((prev) => { const updated = new Map(prev); const data = updated.get(calendarId); if (data) { updated.set(calendarId, { ...data, notes: notesData }); } return updated; }); } catch (error) { console.error("Failed to reload notes:", error); } }; // Then simplify handlers: const handleNoteSubmit = async (noteText: string) => { await noteActions.handleNoteSubmit(noteText); if (isCompareMode && compareNoteCalendarId) { await reloadCompareNotes(compareNoteCalendarId); } };
244-306: Consider parallelizing compare data fetches for better performance.Currently, data for each calendar is fetched sequentially in a
for...ofloop. UsingPromise.allwould load all calendars in parallel.🔎 Proposed refactor
const loadCompareData = async () => { - const dataMap = new Map(); - - for (const calendarId of selectedCompareIds) { - try { - const password = getCachedPassword(calendarId); - const passwordParam = password ? `&password=${password}` : ""; - - // Fetch shifts - const shiftsRes = await fetch( - `/api/shifts?calendarId=${calendarId}${passwordParam}` - ); - // ... more sequential fetches - } catch (error) { - // ... - } - } - - setCompareCalendarData(dataMap); + const entries = await Promise.all( + selectedCompareIds.map(async (calendarId) => { + try { + const password = getCachedPassword(calendarId); + const passwordParam = password ? `&password=${password}` : ""; + + const [shiftsRes, notesRes, syncsRes, presetsRes] = await Promise.all([ + fetch(`/api/shifts?calendarId=${calendarId}${passwordParam}`), + fetch(`/api/notes?calendarId=${calendarId}${passwordParam}`), + fetch(`/api/external-syncs?calendarId=${calendarId}${passwordParam}`), + fetch(`/api/presets?calendarId=${calendarId}${passwordParam}`), + ]); + + const [shiftsData, notesData, syncsData, presetsData] = await Promise.all([ + shiftsRes.ok ? shiftsRes.json() : [], + notesRes.ok ? notesRes.json() : [], + syncsRes.ok ? syncsRes.json() : [], + presetsRes.ok ? presetsRes.json() : [], + ]); + + return [calendarId, { + shifts: shiftsData, + notes: notesData, + externalSyncs: syncsData, + presets: presetsData, + togglingDates: new Set<string>(), + }] as const; + } catch (error) { + console.error(`Error loading data for calendar ${calendarId}:`, error); + return [calendarId, { + shifts: [], + notes: [], + externalSyncs: [], + presets: [], + togglingDates: new Set<string>(), + }] as const; + } + }) + ); + + setCompareCalendarData(new Map(entries)); };
474-632: Large handler function - consider extracting shift toggle logic.
handleCompareDayClickat ~158 lines handles multiple responsibilities: preset validation, toggling state, password verification, shift existence check, delete/create operations, and state updates. Consider extracting the core shift toggle logic into a separate function or custom hook for better maintainability.
710-750: Maps created inline in JSX props cause unnecessary re-renders.Creating new
Mapobjects inside the JSX props (e.g.,shiftsMap,notesMap, etc.) will create new object references on every render, potentially causing unnecessary re-renders ofCalendarCompareView.🔎 Proposed refactor using useMemo
// Add these before the return statement in the compare mode branch const shiftsMap = useMemo( () => new Map( Array.from(compareCalendarData.entries()).map(([id, data]) => [id, data.shifts]) ), [compareCalendarData] ); const notesMap = useMemo( () => new Map( Array.from(compareCalendarData.entries()).map(([id, data]) => [id, data.notes]) ), [compareCalendarData] ); // ... similar for externalSyncsMap, presetsMap, togglingDatesMap // Then in JSX: <CalendarCompareView shiftsMap={shiftsMap} notesMap={notesMap} // ... />
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
app/api/calendars/route.ts(1 hunks)app/page.tsx(11 hunks)components/app-header.tsx(4 hunks)components/calendar-compare-sheet.tsx(1 hunks)components/calendar-compare-view.tsx(1 hunks)components/calendar-selector.tsx(6 hunks)components/preset-list.tsx(4 hunks)components/preset-selector.tsx(3 hunks)components/ui/base-sheet.tsx(3 hunks)hooks/usePasswordManagement.ts(1 hunks)messages/de.json(2 hunks)messages/en.json(2 hunks)messages/it.json(2 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
components/**/*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
components/**/*.tsx: UseBaseSheetcomponent fromcomponents/ui/base-sheet.tsxfor simple sheets (create, edit, settings) with props:open,onOpenChange,title,description,showSaveButton,onSave,isSaving,saveDisabled,hasUnsavedChanges,maxWidth
Dirty state tracking in custom sheets must useuseDirtyStatehook withisDirty,handleClose,showConfirmDialog,setShowConfirmDialog,handleConfirmClosefor unsaved changes confirmation
Custom sheet components must follow the structure: SheetContent with flex layout, SheetHeader with gradient background and border, scrollable content area with flex-1, and SheetFooter with border and action buttons
Calendar left-click toggles shift with selected preset (delete if exists, create if not), right-click opens note dialog preventing default context menu, and day indicators show<StickyNote>icon for days with notes
All components must support real-time updates via Server-Sent Events (SSE) by listening to relevant events and using silent refresh patterns with counter-based refresh triggers
New components with sheets must useBaseSheetfor simple forms or follow the custom sheet pattern for complex layouts, with props:open,onOpenChange,onSubmit, optionalonDelete, reset state on open change
Files:
components/ui/base-sheet.tsxcomponents/calendar-compare-sheet.tsxcomponents/preset-selector.tsxcomponents/preset-list.tsxcomponents/calendar-compare-view.tsxcomponents/calendar-selector.tsxcomponents/app-header.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Never use nativeconfirm()- always useConfirmationDialogcomponent with state management for confirmation dialogs
UsePRESET_COLORSarray with hex format (#3b82f6) for colors, and use 20% opacity for backgrounds (e.g.,${color}20)
UseformatDateToLocal()for date formatting to YYYY-MM-DD format in all date display components
UseuseTranslations()for all user-facing text and ensure all new translation keys are added tomessages/de.json,messages/en.json, andmessages/it.json
Use centralized translation keys for common messages:common.created,common.updated,common.deleted,common.createError,common.updateError,common.deleteError,common.deleteConfirm,common.deleteConfirmWithWarningwith{item}parameter instead of duplicating messages
Use centralized validation translation keys:validation.passwordMatch,validation.passwordIncorrect,validation.passwordRequired,validation.fileRequired,validation.fileTooLarge,validation.urlRequired,validation.urlInvalid,validation.urlAlreadyExists
Use centralized form field translation keys:form.nameLabel,form.namePlaceholder,form.colorLabel,form.passwordLabel,form.passwordPlaceholder,form.notesLabel,form.notesPlaceholder,form.urlLabel,form.urlPlaceholder
Date locale formatting must use:locale === "de" ? de : (locale === "it" ? it : enUS)based on the next-intl locale setting
Files:
components/ui/base-sheet.tsxcomponents/calendar-compare-sheet.tsxcomponents/preset-selector.tsxcomponents/preset-list.tsxhooks/usePasswordManagement.tsapp/api/calendars/route.tscomponents/calendar-compare-view.tsxcomponents/calendar-selector.tsxapp/page.tsxcomponents/app-header.tsx
**/*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.tsx: React components must use React 19 hooks for state management withuseStatefor shifts/presets/notes/calendars anduseEffectfor data fetching on calendar/date changes, anduseRouter().replace()for URL state sync
UsestatsRefreshTriggercounter pattern to track mutations and trigger data refreshes in components without page reloads
Mobile UI must separate calendar selector withshowMobileCalendarDialogto prevent cramped interfaces on small screens
Files:
components/ui/base-sheet.tsxcomponents/calendar-compare-sheet.tsxcomponents/preset-selector.tsxcomponents/preset-list.tsxcomponents/calendar-compare-view.tsxcomponents/calendar-selector.tsxapp/page.tsxcomponents/app-header.tsx
hooks/**.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
All data-fetching hooks (
useShifts,usePresets,useNotes) must automatically callgetCachedPassword()before each fetch and append password as query parameter if present, returning empty arrays on 401 responses
Files:
hooks/usePasswordManagement.ts
app/api/**/route.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
app/api/**/route.ts: Passwords must be SHA-256 hashed using utilities fromlib/password-utils.ts
API route GET endpoints must check bothpasswordHashANDisLockedfields for full protection, while POST/PUT/PATCH/DELETE endpoints must check onlypasswordHashfor write-only protection
Files:
app/api/calendars/route.ts
messages/de.json
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
German translations must always use informal "du" form (never formal "Sie" form) in all user-facing messages, descriptions, hints, and instructions
Files:
messages/de.json
app/**/*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Client-side password caching must use utilities from
lib/password-cache.ts:getCachedPassword(),verifyAndCachePassword(),setCachedPassword(),removeCachedPassword()
Files:
app/page.tsx
🧠 Learnings (14)
📓 Common learnings
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T13:30:34.344Z
Learning: Applies to components/**/*.tsx : Calendar left-click toggles shift with selected preset (delete if exists, create if not), right-click opens note dialog preventing default context menu, and day indicators show `<StickyNote>` icon for days with notes
📚 Learning: 2025-12-20T13:30:34.343Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T13:30:34.343Z
Learning: Applies to components/**/*.tsx : Use `BaseSheet` component from `components/ui/base-sheet.tsx` for simple sheets (create, edit, settings) with props: `open`, `onOpenChange`, `title`, `description`, `showSaveButton`, `onSave`, `isSaving`, `saveDisabled`, `hasUnsavedChanges`, `maxWidth`
Applied to files:
components/ui/base-sheet.tsxcomponents/calendar-compare-sheet.tsxapp/page.tsx
📚 Learning: 2025-12-20T13:30:34.344Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T13:30:34.344Z
Learning: Applies to components/**/*.tsx : New components with sheets must use `BaseSheet` for simple forms or follow the custom sheet pattern for complex layouts, with props: `open`, `onOpenChange`, `onSubmit`, optional `onDelete`, reset state on open change
Applied to files:
components/ui/base-sheet.tsxcomponents/calendar-compare-sheet.tsx
📚 Learning: 2025-12-20T13:30:34.343Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T13:30:34.343Z
Learning: Applies to components/**/*.tsx : Dirty state tracking in custom sheets must use `useDirtyState` hook with `isDirty`, `handleClose`, `showConfirmDialog`, `setShowConfirmDialog`, `handleConfirmClose` for unsaved changes confirmation
Applied to files:
components/ui/base-sheet.tsxcomponents/calendar-compare-sheet.tsx
📚 Learning: 2025-12-20T13:30:34.344Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T13:30:34.344Z
Learning: Applies to **/*.{ts,tsx} : Use centralized form field translation keys: `form.nameLabel`, `form.namePlaceholder`, `form.colorLabel`, `form.passwordLabel`, `form.passwordPlaceholder`, `form.notesLabel`, `form.notesPlaceholder`, `form.urlLabel`, `form.urlPlaceholder`
Applied to files:
components/ui/base-sheet.tsx
📚 Learning: 2025-12-20T13:30:34.344Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T13:30:34.344Z
Learning: Applies to components/**/*.tsx : Calendar left-click toggles shift with selected preset (delete if exists, create if not), right-click opens note dialog preventing default context menu, and day indicators show `<StickyNote>` icon for days with notes
Applied to files:
components/calendar-compare-sheet.tsxcomponents/preset-selector.tsxcomponents/preset-list.tsxcomponents/calendar-compare-view.tsxcomponents/calendar-selector.tsxapp/page.tsxcomponents/app-header.tsx
📚 Learning: 2025-12-20T13:30:34.344Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T13:30:34.344Z
Learning: Applies to **/*.tsx : Mobile UI must separate calendar selector with `showMobileCalendarDialog` to prevent cramped interfaces on small screens
Applied to files:
components/calendar-compare-sheet.tsxcomponents/calendar-compare-view.tsxcomponents/calendar-selector.tsxapp/page.tsxcomponents/app-header.tsx
📚 Learning: 2025-12-20T13:30:34.344Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T13:30:34.344Z
Learning: Applies to **/*.tsx : React components must use React 19 hooks for state management with `useState` for shifts/presets/notes/calendars and `useEffect` for data fetching on calendar/date changes, and `useRouter().replace()` for URL state sync
Applied to files:
components/calendar-compare-sheet.tsxcomponents/calendar-compare-view.tsxcomponents/calendar-selector.tsxapp/page.tsx
📚 Learning: 2025-12-20T13:30:34.343Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T13:30:34.343Z
Learning: Applies to components/**/*.tsx : Custom sheet components must follow the structure: SheetContent with flex layout, SheetHeader with gradient background and border, scrollable content area with flex-1, and SheetFooter with border and action buttons
Applied to files:
components/calendar-compare-sheet.tsxapp/page.tsx
📚 Learning: 2025-12-20T13:30:34.343Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T13:30:34.343Z
Learning: Applies to hooks/**.ts : All data-fetching hooks (`useShifts`, `usePresets`, `useNotes`) must automatically call `getCachedPassword()` before each fetch and append password as query parameter if present, returning empty arrays on 401 responses
Applied to files:
hooks/usePasswordManagement.tsapp/page.tsx
📚 Learning: 2025-12-20T13:30:34.344Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T13:30:34.344Z
Learning: Applies to lib/db/schema.ts : Database cascading relationships: `calendars` cascade delete to `shifts`, `shiftPresets`, `calendarNotes`; `shiftPresets` set null on delete to `shifts`
Applied to files:
hooks/usePasswordManagement.tsapp/api/calendars/route.ts
📚 Learning: 2025-12-20T13:30:34.344Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T13:30:34.344Z
Learning: Applies to **/*.{ts,tsx} : Use `useTranslations()` for all user-facing text and ensure all new translation keys are added to `messages/de.json`, `messages/en.json`, and `messages/it.json`
Applied to files:
messages/de.jsonmessages/it.json
📚 Learning: 2025-12-20T13:30:34.344Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T13:30:34.344Z
Learning: Applies to **/*.{ts,tsx} : Use centralized translation keys for common messages: `common.created`, `common.updated`, `common.deleted`, `common.createError`, `common.updateError`, `common.deleteError`, `common.deleteConfirm`, `common.deleteConfirmWithWarning` with `{item}` parameter instead of duplicating messages
Applied to files:
messages/de.jsonmessages/it.jsonmessages/en.json
📚 Learning: 2025-12-20T13:30:34.343Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T13:30:34.343Z
Learning: Applies to app/**/*.tsx : Client-side password caching must use utilities from `lib/password-cache.ts`: `getCachedPassword()`, `verifyAndCachePassword()`, `setCachedPassword()`, `removeCachedPassword()`
Applied to files:
app/page.tsx
🧬 Code graph analysis (2)
components/calendar-compare-sheet.tsx (4)
lib/types.ts (1)
CalendarWithCount(4-13)components/ui/base-sheet.tsx (1)
BaseSheet(41-142)lib/db/schema.ts (1)
calendars(4-18)components/ui/checkbox.tsx (1)
Checkbox(32-32)
components/preset-list.tsx (1)
components/ui/button.tsx (1)
Button(60-60)
⏰ 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). (2)
- GitHub Check: Agent
- GitHub Check: build-dev
🔇 Additional comments (21)
app/api/calendars/route.ts (1)
19-22: LGTM: Alias consistency improvement.Updating the SQL alias from
"shift_count"to"_count"aligns it with the field name in the select object, improving code consistency.components/ui/base-sheet.tsx (1)
29-29: LGTM! Clean implementation of customizable save button label.The new
saveLabelprop allows consumers to override the default "Save" button text while maintaining backward compatibility through proper fallback logic.Also applies to: 53-53, 123-125
messages/de.json (1)
85-93: LGTM! Translations correctly use informal "du" form.The new translation keys for compare mode and preset hints are properly localized and consistently follow the informal "du" form requirement for German translations.
Based on coding guidelines, German translations must use informal "du" form.
Also applies to: 164-165
components/calendar-selector.tsx (2)
13-13: LGTM! Proper capability gating for compare functionality.The
canComparecheck ensures the compare feature is only enabled when at least 2 calendars are available, preventing runtime errors and providing clear UX expectations.Also applies to: 24-24, 37-37, 48-48
118-128: LGTM! Consistent compare button implementation across desktop and mobile.The compare action is properly integrated into both layouts:
- Desktop: Icon-only button with tooltip for compact header
- Mobile: Full button with icon + text in restructured grid layout with conditional spanning
The grid layout correctly handles the Create button spanning both columns when compare is unavailable.
Also applies to: 216-238
components/app-header.tsx (1)
40-40: LGTM! Proper threading of compare action with mobile dialog handling.The
onCompareprop is correctly wired to theCalendarSelectorin both desktop and mobile contexts. The mobile implementation properly closes the calendar dialog before invoking the compare action, maintaining clean UI state transitions.Also applies to: 67-67, 154-154, 339-346
messages/it.json (1)
85-93: LGTM! Italian translations complete the multilingual support for compare mode.The new translation keys are consistent with the English and German versions, ensuring full localization support for the calendar comparison feature.
Also applies to: 164-165
components/calendar-compare-sheet.tsx (1)
1-107: LGTM! Well-structured calendar comparison selection component.The component follows the custom sheet pattern correctly using
BaseSheetwith the newsaveLabelprop. The capability gating (2-3 calendars), mobile warning, and disabled states are all properly implemented.messages/en.json (1)
85-93: LGTM! English translations complete the base language support.The new translation keys provide clear, user-facing text for the calendar comparison feature and are consistent across all three supported languages (English, German, Italian).
Also applies to: 164-165
hooks/usePasswordManagement.ts (1)
11-11: Verify that the newcalendarIdfield is utilized in the compare mode flow.The optional
calendarIdfield extendsPendingActionto support calendar-scoped actions. This field is set during compare mode password verification scenarios (inhandleCompareDayClick) and is consumed when rendering the ShiftDialog to maintain the correct calendar context for the pending action.components/preset-selector.tsx (1)
24-24: LGTM!The new
hideManageButtonprop is cleanly added with proper typing, a sensible default value offalsefor backward compatibility, and correctly forwarded toPresetList.Also applies to: 41-41, 77-77
components/preset-list.tsx (3)
32-32: LGTM!The
hideManageButtonprop is properly typed and destructured with a default value.Also applies to: 47-47
134-146: LGTM!The conditional rendering in the empty state correctly hides the "Create Your First" preset button when
hideManageButtonis true.
211-222: LGTM!The manage button in the control row is correctly hidden when either
hidePresetHeaderorhideManageButtonis true.components/calendar-compare-view.tsx (4)
125-132: Good UX: Mobile warning for non-optimized experience.This appropriately sets user expectations for the compare view on smaller screens.
191-216: LGTM!The
PresetSelectoris correctly configured per-calendar withhideManageButton={true}to prevent managing presets in compare mode, which aligns with the compare view's read-focused design.
263-294: LGTM!Optional callback handling with conditional wrappers correctly adapts callbacks to include
calendarIdcontext for compare mode interactions.
153-156: Pattern is correct and intentional.The logic
(passwordHash && isLocked) && !hasPasswordin calendar-compare-view aligns with API route protection, which consistently checks bothpasswordHash && isLockedbefore verifying passwords. This dual-field check is the correct pattern for read operations per the coding guidelines.Note:
PresetListonly checkspasswordHashwithout verifyingisLocked, creating an inconsistency in client-side protection logic.app/page.tsx (3)
991-1005: LGTM!The
AnimatePresencewrapper correctly handles the compare selector overlay animation, and the cancel handler properly resets both the selector visibility and selected IDs.
1023-1023: LGTM!The
onCompareprop is correctly wired to trigger the compare selector.
523-531: No recursive call pattern exists; password dialog dismissal doesn't trigger action retry.When the password dialog is dismissed without entering a password, the
pendingActionremains set but is never executed. The action only runs viahandlePasswordSuccess, which is only called on successful password verification. There's no automatic retry mechanism or loop—the user would need to click the day again to trigger a new action.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (5)
components/calendar-compare-view.tsx (1)
268-327: Previous concern has been addressed.The
selectedPresetIdis now correctly set toundefinedfor disabled calendars (line 274-276), andonDayClickis set to a no-op function when disabled (lines 290-294). This prevents confusion and unnecessary clicks on calendars where the selected preset doesn't belong.messages/de.json (1)
85-96: German translations correctly use informal "du" form.The translations appropriately use informal German ("nutze bitte" instead of formal "nutzen Sie bitte"), which aligns with the coding guideline requiring informal "du" form in all German user-facing text.
app/page.tsx (3)
260-313: Parallel data loading is well-implemented.The compare mode data loading correctly uses
Promise.allfor both calendar-level parallelism and per-calendar fetch parallelism. This addresses previous performance concerns about sequential API calls.
561-590: Toggling state cleanup before password dialog is now properly handled.The code now correctly clears the toggling state (lines 568-580) before showing the password dialog, preventing the cell from remaining in a "toggling" state indefinitely if the user cancels or enters an incorrect password.
885-928: Stale closure issue inonShiftsChangehas been addressed.The implementation now correctly:
- Builds updates in parallel with
Promise.all(lines 890-911)- Collects results into a local
updatesMap- Uses the functional state updater pattern (lines 913-925) to apply updates to the latest state
This avoids the stale closure risk from the previous implementation.
🧹 Nitpick comments (1)
components/calendar-compare-view.tsx (1)
93-105: Consider adding error handling for clipboard API.The share link handler has basic error handling, but the
.catch()shows a generic error. Consider providing more specific feedback if clipboard access is denied.🔎 Optional enhancement
const handleShareLink = () => { const url = new URL(window.location.href); url.searchParams.set("compare", props.selectedIds.join(",")); navigator.clipboard .writeText(url.toString()) .then(() => { toast.success(t("calendar.linkCopied")); }) .catch(() => { - toast.error(t("common.error")); + // Fallback: show URL in a prompt if clipboard fails + window.prompt("Copy link:", url.toString()); }); };
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
README.md(2 hunks)app/page.tsx(10 hunks)components/calendar-compare-skeleton.tsx(1 hunks)components/calendar-compare-view.tsx(1 hunks)messages/de.json(2 hunks)messages/en.json(2 hunks)messages/it.json(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- messages/it.json
🧰 Additional context used
📓 Path-based instructions (5)
components/**/*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
components/**/*.tsx: UseBaseSheetcomponent fromcomponents/ui/base-sheet.tsxfor simple sheets (create, edit, settings) with props:open,onOpenChange,title,description,showSaveButton,onSave,isSaving,saveDisabled,hasUnsavedChanges,maxWidth
Dirty state tracking in custom sheets must useuseDirtyStatehook withisDirty,handleClose,showConfirmDialog,setShowConfirmDialog,handleConfirmClosefor unsaved changes confirmation
Custom sheet components must follow the structure: SheetContent with flex layout, SheetHeader with gradient background and border, scrollable content area with flex-1, and SheetFooter with border and action buttons
Calendar left-click toggles shift with selected preset (delete if exists, create if not), right-click opens note dialog preventing default context menu, and day indicators show<StickyNote>icon for days with notes
All components must support real-time updates via Server-Sent Events (SSE) by listening to relevant events and using silent refresh patterns with counter-based refresh triggers
New components with sheets must useBaseSheetfor simple forms or follow the custom sheet pattern for complex layouts, with props:open,onOpenChange,onSubmit, optionalonDelete, reset state on open change
Files:
components/calendar-compare-skeleton.tsxcomponents/calendar-compare-view.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Never use nativeconfirm()- always useConfirmationDialogcomponent with state management for confirmation dialogs
UsePRESET_COLORSarray with hex format (#3b82f6) for colors, and use 20% opacity for backgrounds (e.g.,${color}20)
UseformatDateToLocal()for date formatting to YYYY-MM-DD format in all date display components
UseuseTranslations()for all user-facing text and ensure all new translation keys are added tomessages/de.json,messages/en.json, andmessages/it.json
Use centralized translation keys for common messages:common.created,common.updated,common.deleted,common.createError,common.updateError,common.deleteError,common.deleteConfirm,common.deleteConfirmWithWarningwith{item}parameter instead of duplicating messages
Use centralized validation translation keys:validation.passwordMatch,validation.passwordIncorrect,validation.passwordRequired,validation.fileRequired,validation.fileTooLarge,validation.urlRequired,validation.urlInvalid,validation.urlAlreadyExists
Use centralized form field translation keys:form.nameLabel,form.namePlaceholder,form.colorLabel,form.passwordLabel,form.passwordPlaceholder,form.notesLabel,form.notesPlaceholder,form.urlLabel,form.urlPlaceholder
Date locale formatting must use:locale === "de" ? de : (locale === "it" ? it : enUS)based on the next-intl locale setting
Files:
components/calendar-compare-skeleton.tsxcomponents/calendar-compare-view.tsxapp/page.tsx
**/*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.tsx: React components must use React 19 hooks for state management withuseStatefor shifts/presets/notes/calendars anduseEffectfor data fetching on calendar/date changes, anduseRouter().replace()for URL state sync
UsestatsRefreshTriggercounter pattern to track mutations and trigger data refreshes in components without page reloads
Mobile UI must separate calendar selector withshowMobileCalendarDialogto prevent cramped interfaces on small screens
Files:
components/calendar-compare-skeleton.tsxcomponents/calendar-compare-view.tsxapp/page.tsx
app/**/*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Client-side password caching must use utilities from
lib/password-cache.ts:getCachedPassword(),verifyAndCachePassword(),setCachedPassword(),removeCachedPassword()
Files:
app/page.tsx
messages/de.json
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
German translations must always use informal "du" form (never formal "Sie" form) in all user-facing messages, descriptions, hints, and instructions
Files:
messages/de.json
🧠 Learnings (9)
📓 Common learnings
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T13:30:34.344Z
Learning: Applies to components/**/*.tsx : Calendar left-click toggles shift with selected preset (delete if exists, create if not), right-click opens note dialog preventing default context menu, and day indicators show `<StickyNote>` icon for days with notes
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T13:30:34.344Z
Learning: Applies to **/*.tsx : Mobile UI must separate calendar selector with `showMobileCalendarDialog` to prevent cramped interfaces on small screens
📚 Learning: 2025-12-20T13:30:34.344Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T13:30:34.344Z
Learning: Applies to components/**/*.tsx : Calendar left-click toggles shift with selected preset (delete if exists, create if not), right-click opens note dialog preventing default context menu, and day indicators show `<StickyNote>` icon for days with notes
Applied to files:
components/calendar-compare-skeleton.tsxcomponents/calendar-compare-view.tsxapp/page.tsx
📚 Learning: 2025-12-20T13:30:34.344Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T13:30:34.344Z
Learning: Applies to **/*.tsx : Mobile UI must separate calendar selector with `showMobileCalendarDialog` to prevent cramped interfaces on small screens
Applied to files:
components/calendar-compare-skeleton.tsxcomponents/calendar-compare-view.tsxapp/page.tsx
📚 Learning: 2025-12-20T13:30:34.344Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T13:30:34.344Z
Learning: Applies to **/*.tsx : React components must use React 19 hooks for state management with `useState` for shifts/presets/notes/calendars and `useEffect` for data fetching on calendar/date changes, and `useRouter().replace()` for URL state sync
Applied to files:
components/calendar-compare-skeleton.tsxcomponents/calendar-compare-view.tsxapp/page.tsx
📚 Learning: 2025-12-20T13:30:34.343Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T13:30:34.343Z
Learning: Applies to hooks/**.ts : All data-fetching hooks (`useShifts`, `usePresets`, `useNotes`) must automatically call `getCachedPassword()` before each fetch and append password as query parameter if present, returning empty arrays on 401 responses
Applied to files:
app/page.tsx
📚 Learning: 2025-12-20T13:30:34.343Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T13:30:34.343Z
Learning: Applies to components/**/*.tsx : Use `BaseSheet` component from `components/ui/base-sheet.tsx` for simple sheets (create, edit, settings) with props: `open`, `onOpenChange`, `title`, `description`, `showSaveButton`, `onSave`, `isSaving`, `saveDisabled`, `hasUnsavedChanges`, `maxWidth`
Applied to files:
app/page.tsx
📚 Learning: 2025-12-20T13:30:34.344Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T13:30:34.344Z
Learning: Applies to **/*.{ts,tsx} : Use centralized translation keys for common messages: `common.created`, `common.updated`, `common.deleted`, `common.createError`, `common.updateError`, `common.deleteError`, `common.deleteConfirm`, `common.deleteConfirmWithWarning` with `{item}` parameter instead of duplicating messages
Applied to files:
messages/en.jsonmessages/de.json
📚 Learning: 2025-12-20T13:30:34.344Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T13:30:34.344Z
Learning: Applies to **/*.{ts,tsx} : Use `useTranslations()` for all user-facing text and ensure all new translation keys are added to `messages/de.json`, `messages/en.json`, and `messages/it.json`
Applied to files:
messages/de.json
📚 Learning: 2025-12-20T13:30:34.344Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T13:30:34.344Z
Learning: Applies to messages/de.json : German translations must always use informal "du" form (never formal "Sie" form) in all user-facing messages, descriptions, hints, and instructions
Applied to files:
messages/de.json
🧬 Code graph analysis (1)
app/page.tsx (6)
lib/locales.ts (1)
getDateLocale(13-15)lib/password-cache.ts (2)
getCachedPassword(9-14)verifyAndCachePassword(36-89)lib/types.ts (3)
ShiftWithCalendar(15-35)ExternalSync(2-2)Shift(2-2)lib/db/schema.ts (5)
CalendarNote(160-160)ExternalSync(154-154)ShiftPreset(158-158)calendars(4-18)Shift(156-156)hooks/useVersionInfo.ts (1)
useVersionInfo(9-45)lib/date-utils.ts (1)
formatDateToLocal(5-10)
🪛 GitHub Check: Tests & Checks
components/calendar-compare-skeleton.tsx
[warning] 2-2:
'CalendarContentSkeleton' is defined but never used
app/page.tsx
[warning] 336-336:
React Hook useEffect has a missing dependency: 'isCompareMode'. Either include it or remove the dependency array
⏰ 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). (2)
- GitHub Check: Agent
- GitHub Check: build-dev
🔇 Additional comments (7)
components/calendar-compare-skeleton.tsx (1)
10-92: Well-structured skeleton component.The component correctly implements responsive grid layouts based on the
countprop and properly forwards thehidePresetHeaderprop to thePresetListSkeleton. The skeleton structure mirrors the actualCalendarCompareViewlayout nicely.messages/en.json (2)
85-96: Translation keys are well-structured.The new compare mode translations follow the existing conventions and appropriately use parameterized strings (
{count}) for dynamic content. The keys align with the new compare UI components.
167-168: LGTM for preset translations.The
cannotAddPresetHintkey provides clear guidance for users when a preset from a different calendar is selected.components/calendar-compare-view.tsx (1)
74-338: Well-structured compare view component.The component correctly implements:
- Lock state handling with
getCachedPassword- Responsive grid layout (1-3 columns)
- Per-calendar preset selection with proper disabling logic
- Proper forwarding of callbacks with calendar context
- Mobile warning banner for non-optimized view
messages/de.json (1)
167-168: LGTM for German preset translations.The
cannotAddPresetHinttranslation is clear and follows German language conventions.app/page.tsx (2)
148-200: Note action wrappers for compare mode are well-structured.The
handleNoteSubmitandhandleNoteDeletewrappers properly handle both regular and compare mode scenarios, reloading notes for the specific calendar in compare mode using the functional state updater pattern.
517-690: Comprehensive compare day click handler.The
handleCompareDayClickfunction properly handles:
- No preset selected (shows existing shifts)
- Toggling state management with proper cleanup
- Password verification with state cleanup before dialog
- Shift creation/deletion with appropriate toasts
- Data reload after mutations
The implementation follows the coding guidelines for using
formatDateToLocal,getCachedPassword,verifyAndCachePassword, and centralized translation keys.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
app/page.tsx:389
- The password success handler has incomplete logic for compare mode. When
isCompareMode && compareNoteCalendarIdis true, it only reloads notes data. However, if the pending action is a shift toggle action (set in handleCompareDayClick at line 581), executing that action after password success won't properly refresh the UI because the shift data reload happens inside the pending action itself, not here. This creates inconsistent behavior compared to normal mode where all data is refreshed.
Consider checking the pendingAction?.calendarId to determine which calendar's data needs reloading, or ensure the pending action itself handles all necessary data reloads.
const handlePasswordSuccess = async () => {
if (isCompareMode && compareNoteCalendarId) {
// In compare mode, reload notes for the specific calendar
await refetchNotes();
const password = getCachedPassword(compareNoteCalendarId);
const passwordParam = password ? `&password=${password}` : "";
const notesRes = await fetch(
`/api/notes?calendarId=${compareNoteCalendarId}${passwordParam}`
);
const notesData = notesRes.ok ? await notesRes.json() : [];
setCompareCalendarData((prev) => {
const updated = new Map(prev);
const data = updated.get(compareNoteCalendarId);
if (data) {
updated.set(compareNoteCalendarId, { ...data, notes: notesData });
}
return updated;
});
} else {
await Promise.all([
refetchShifts(),
refetchPresets(),
refetchNotes(),
fetchExternalSyncs(),
fetchSyncErrorStatus(),
]);
setStatsRefreshTrigger((prev) => prev + 1);
}
baseHandlePasswordSuccess();
// Execute pending action if exists
if (pendingAction?.action) {
await pendingAction.action();
setPendingAction(null);
}
};
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
app/page.tsx (1)
315-335: Add missingisCompareModedependency to useEffect.The effect reads
isCompareMode(lines 317, 329) but doesn't include it in the dependency array. This can cause stale closure issues where the effect doesn't properly respond to compare mode state changes.🔎 Proposed fix
useEffect(() => { const compareParam = searchParams.get("compare"); if (compareParam && !isCompareMode) { const calendarIds = compareParam.split(",").filter((id) => id.trim()); if (calendarIds.length >= 2 && calendarIds.length <= 3) { // Verify that all calendars exist const validIds = calendarIds.filter((id) => calendars.some((cal) => cal.id === id) ); if (validIds.length >= 2) { setSelectedCompareIds(validIds); setIsCompareMode(true); } } } else if (!compareParam && isCompareMode) { // If compare param is removed from URL, exit compare mode setIsCompareMode(false); setSelectedCompareIds([]); setCompareCalendarData(new Map()); } - }, [searchParams, calendars]); + }, [searchParams, calendars, isCompareMode]);
🧹 Nitpick comments (4)
app/page.tsx (4)
148-200: Extract duplicated note reload logic.The logic for reloading notes in compare mode (lines 152-172 and 179-199) is duplicated between
handleNoteSubmitandhandleNoteDelete. Extract this into a helper function.🔎 Proposed refactor
+ // Helper to reload notes for a calendar in compare mode + const reloadCompareNotes = async (calendarId: string) => { + const password = getCachedPassword(calendarId); + const passwordParam = password ? `&password=${password}` : ""; + try { + const notesRes = await fetch( + `/api/notes?calendarId=${calendarId}${passwordParam}` + ); + const notesData = notesRes.ok ? await notesRes.json() : []; + setCompareCalendarData((prev) => { + const updated = new Map(prev); + const data = updated.get(calendarId); + if (data) { + updated.set(calendarId, { ...data, notes: notesData }); + } + return updated; + }); + } catch (error) { + console.error("Failed to reload notes:", error); + } + }; + // Wrapper for note submit that reloads compare data const handleNoteSubmit = async (noteText: string) => { await noteActions.handleNoteSubmit(noteText); - // Reload notes for the specific calendar in compare mode if (isCompareMode && compareNoteCalendarId) { - const password = getCachedPassword(compareNoteCalendarId); - const passwordParam = password ? `&password=${password}` : ""; - try { - const notesRes = await fetch( - `/api/notes?calendarId=${compareNoteCalendarId}${passwordParam}` - ); - const notesData = notesRes.ok ? await notesRes.json() : []; - setCompareCalendarData((prev) => { - const updated = new Map(prev); - const data = updated.get(compareNoteCalendarId); - if (data) { - updated.set(compareNoteCalendarId, { ...data, notes: notesData }); - } - return updated; - }); - } catch (error) { - console.error("Failed to reload notes:", error); - } + await reloadCompareNotes(compareNoteCalendarId); } }; // Wrapper for note delete that reloads compare data const handleNoteDelete = async () => { await noteActions.handleNoteDelete(); - // Reload notes for the specific calendar in compare mode if (isCompareMode && compareNoteCalendarId) { - const password = getCachedPassword(compareNoteCalendarId); - const passwordParam = password ? `&password=${password}` : ""; - try { - const notesRes = await fetch( - `/api/notes?calendarId=${compareNoteCalendarId}${passwordParam}` - ); - const notesData = notesRes.ok ? await notesRes.json() : []; - setCompareCalendarData((prev) => { - const updated = new Map(prev); - const data = updated.get(compareNoteCalendarId); - if (data) { - updated.set(compareNoteCalendarId, { ...data, notes: notesData }); - } - return updated; - }); - } catch (error) { - console.error("Failed to reload notes:", error); - } + await reloadCompareNotes(compareNoteCalendarId); } };
517-689: Consider extracting logic fromhandleCompareDayClick.This function is 172 lines and handles multiple responsibilities: password verification, shift creation/deletion, data reloading, and UI feedback. For better maintainability and testability, consider extracting:
- Password verification flow into a helper
- Shift creation logic
- Shift deletion logic
- Compare data reload logic
848-883: Consider extracting inlineonPresetsChangehandler.The
onPresetsChangehandler (36 lines) is defined inline within theCalendarCompareViewprops. For better readability, consider extracting it as a named function similar to other handlers.
933-984: Consider extracting inlineonUnlockCalendarhandler.The
onUnlockCalendarhandler (52 lines) is defined inline within theCalendarCompareViewprops. For better readability, consider extracting it as a named function similar to other handlers.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/page.tsx(10 hunks)components/calendar-compare-skeleton.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- components/calendar-compare-skeleton.tsx
🧰 Additional context used
📓 Path-based instructions (3)
app/**/*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Client-side password caching must use utilities from
lib/password-cache.ts:getCachedPassword(),verifyAndCachePassword(),setCachedPassword(),removeCachedPassword()
Files:
app/page.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Never use nativeconfirm()- always useConfirmationDialogcomponent with state management for confirmation dialogs
UsePRESET_COLORSarray with hex format (#3b82f6) for colors, and use 20% opacity for backgrounds (e.g.,${color}20)
UseformatDateToLocal()for date formatting to YYYY-MM-DD format in all date display components
UseuseTranslations()for all user-facing text and ensure all new translation keys are added tomessages/de.json,messages/en.json, andmessages/it.json
Use centralized translation keys for common messages:common.created,common.updated,common.deleted,common.createError,common.updateError,common.deleteError,common.deleteConfirm,common.deleteConfirmWithWarningwith{item}parameter instead of duplicating messages
Use centralized validation translation keys:validation.passwordMatch,validation.passwordIncorrect,validation.passwordRequired,validation.fileRequired,validation.fileTooLarge,validation.urlRequired,validation.urlInvalid,validation.urlAlreadyExists
Use centralized form field translation keys:form.nameLabel,form.namePlaceholder,form.colorLabel,form.passwordLabel,form.passwordPlaceholder,form.notesLabel,form.notesPlaceholder,form.urlLabel,form.urlPlaceholder
Date locale formatting must use:locale === "de" ? de : (locale === "it" ? it : enUS)based on the next-intl locale setting
Files:
app/page.tsx
**/*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.tsx: React components must use React 19 hooks for state management withuseStatefor shifts/presets/notes/calendars anduseEffectfor data fetching on calendar/date changes, anduseRouter().replace()for URL state sync
UsestatsRefreshTriggercounter pattern to track mutations and trigger data refreshes in components without page reloads
Mobile UI must separate calendar selector withshowMobileCalendarDialogto prevent cramped interfaces on small screens
Files:
app/page.tsx
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T13:30:34.344Z
Learning: Applies to components/**/*.tsx : Calendar left-click toggles shift with selected preset (delete if exists, create if not), right-click opens note dialog preventing default context menu, and day indicators show `<StickyNote>` icon for days with notes
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T13:30:34.344Z
Learning: Applies to **/*.tsx : Mobile UI must separate calendar selector with `showMobileCalendarDialog` to prevent cramped interfaces on small screens
📚 Learning: 2025-12-20T13:30:34.344Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T13:30:34.344Z
Learning: Applies to components/**/*.tsx : Calendar left-click toggles shift with selected preset (delete if exists, create if not), right-click opens note dialog preventing default context menu, and day indicators show `<StickyNote>` icon for days with notes
Applied to files:
app/page.tsx
📚 Learning: 2025-12-20T13:30:34.344Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T13:30:34.344Z
Learning: Applies to **/*.tsx : Mobile UI must separate calendar selector with `showMobileCalendarDialog` to prevent cramped interfaces on small screens
Applied to files:
app/page.tsx
📚 Learning: 2025-12-20T13:30:34.344Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T13:30:34.344Z
Learning: Applies to **/*.tsx : React components must use React 19 hooks for state management with `useState` for shifts/presets/notes/calendars and `useEffect` for data fetching on calendar/date changes, and `useRouter().replace()` for URL state sync
Applied to files:
app/page.tsx
📚 Learning: 2025-12-20T13:30:34.343Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T13:30:34.343Z
Learning: Applies to hooks/**.ts : All data-fetching hooks (`useShifts`, `usePresets`, `useNotes`) must automatically call `getCachedPassword()` before each fetch and append password as query parameter if present, returning empty arrays on 401 responses
Applied to files:
app/page.tsx
📚 Learning: 2025-12-20T13:30:34.343Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-20T13:30:34.343Z
Learning: Applies to components/**/*.tsx : Use `BaseSheet` component from `components/ui/base-sheet.tsx` for simple sheets (create, edit, settings) with props: `open`, `onOpenChange`, `title`, `description`, `showSaveButton`, `onSave`, `isSaving`, `saveDisabled`, `hasUnsavedChanges`, `maxWidth`
Applied to files:
app/page.tsx
🧬 Code graph analysis (1)
app/page.tsx (9)
lib/locales.ts (1)
getDateLocale(13-15)hooks/useNotes.ts (1)
useNotes(8-189)lib/password-cache.ts (2)
getCachedPassword(9-14)verifyAndCachePassword(36-89)lib/types.ts (3)
ShiftWithCalendar(15-35)ExternalSync(2-2)Shift(2-2)lib/db/schema.ts (5)
CalendarNote(160-160)ExternalSync(154-154)ShiftPreset(158-158)calendars(4-18)Shift(156-156)lib/date-utils.ts (1)
formatDateToLocal(5-10)components/calendar-compare-skeleton.tsx (1)
CalendarCompareSkeleton(9-91)components/calendar-compare-view.tsx (1)
CalendarCompareView(74-338)components/calendar-compare-sheet.tsx (1)
CalendarCompareSheet(19-107)
🪛 GitHub Check: Tests & Checks
app/page.tsx
[warning] 335-335:
React Hook useEffect has a missing dependency: 'isCompareMode'. Either include it or remove the dependency array
⏰ 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-dev
| } = useNotes(selectedCalendar); | ||
|
|
||
| // External sync management | ||
| } = useNotes(isCompareMode ? compareNoteCalendarId : selectedCalendar); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find and examine useNotes implementation
if [ -f "hooks/useNotes.ts" ]; then
wc -l hooks/useNotes.ts
echo "---"
cat -n hooks/useNotes.ts
elif [ -f "src/hooks/useNotes.ts" ]; then
wc -l src/hooks/useNotes.ts
echo "---"
cat -n src/hooks/useNotes.ts
else
# Search for useNotes definition
find . -name "*.ts" -o -name "*.tsx" | xargs grep -l "export.*useNotes" 2>/dev/null | head -5
fiRepository: panteLx/BetterShift
Length of output: 6896
useNotes handles undefined gracefully for the main flow, but updateNote and deleteNote lack defensive checks.
The hook correctly accepts calendarId: string | undefined and guards against undefined in fetchNotes() (line 13: if (!calendarId) return) and the useEffect dependency (lines 173-178). When undefined, notes remain empty and no API calls are made—which safely covers the scenario at line 109.
However, updateNote (line 96) and deleteNote (line 135) call getCachedPassword(calendarId) without first checking if calendarId is defined. Add guards like if (!calendarId) return false; at the start of both methods to prevent potential runtime errors if these functions are called when the calendar ID is undefined.
🤖 Prompt for AI Agents
In app/page.tsx around lines 96, 109 and 135, updateNote and deleteNote call
getCachedPassword(calendarId) without ensuring calendarId is defined; add a
defensive guard at the start of both functions (e.g., if (!calendarId) return
false;) so they return early when calendarId is undefined and avoid runtime
errors, leaving the existing main flow handling for undefined calendarId
unchanged.
1 participant
Adds a comparison mode to view and interact with multiple calendars side-by-side to simplify cross-calendar review and editing.
Implements a compare selector and comparison view that load shifts, notes, presets and external syncs for selected calendars; enables creating/deleting shifts via presets with password verification and reloads compare-specific data after edits. Integrates compare controls into header and selector, adds localized strings, and adjusts several UI helpers (manage button hiding, sheet save label). Also includes small backend/UI fixes to ensure counts and pending actions include calendar context.
Summary by CodeRabbit
New Features
UI
Localization
Bug Fixes
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.