Finder view merge #1213
Finder view merge #1213
Conversation
The changes in this commit introduce a new window type called "Finder" to the `HyprWindow` type in the `bindings.gen.ts` file. This change is made to accommodate the addition of a new Finder-like functionality in the application. Additionally, the corresponding changes are made in the `routeTree.gen.ts` file to add a new route for the Finder functionality, replacing the previously existing Calendar route.
When a user clicks on an event row, the application now checks if there is an existing session for that event. If a session exists, the user is navigated to the corresponding note. If no session exists, a new note is created with the event ID pre-filled. This ensures that users can easily access and manage their calendar events within the application.
This commit introduces a mechanism to prevent the handling of duplicate system events during the Apple Calendar sync process. The changes focus on the following key improvements: - Tracks the I that have been handled to avoid processing them multiple times. - Skips the processing of system events that have already been handled, either as updates or rescheduled events. - Ensures that rescheduled system events are also marked as handled to prevent duplicate creation. These changes help to improve the reliability and consistency of the Apple Calendar sync functionality by avoiding the creation of duplicate events in the database.
Deduplicates events by tracking_id to handle any sync issues. Keeps the most recent event (by database id) for each tracking_id. This ensures that the event list displayed in the app is accurate and up-to-date, even if there are any issues with event synchronization.
The changes made in this commit focus on improving the UI and navigation functionality of the note card component in the workspace calendar. The key changes are: 1. Simplified the note card header by removing the file/document icon and using a more compact heading. 2. Improved the layout and styling of the note card content, making it more visually appealing and consistent. 3. Replaced the `safeNavigate` utility with direct calls to the `windowsCommands` module to handle navigation, ensuring a smoother user experience. These changes aim to enhance the overall user interface and interaction within the workspace calendar, providing a more polished and intuitive experience for users.
The changes made in this commit are focused on improving the user experience when clicking on the date of an event or note in the left sidebar. Previously, the application would open the main window, but now it will open the Finder window instead, which is more appropriate for navigating to a specific date. The changes were made in the `events-list.tsx` and `notes-list.tsx` files, where the `windowsCommands.windowShow()` and `windowsCommands.windowEmitNavigate()` functions were updated to use the "finder" type instead of the "main" type.
The changes made in this commit focus on improving the visual appearance and layout of the event and note cards in the workspace calendar. The key changes are: - Vertically center the content within the cards to create a more balanced and consistent look. - Remove the top margin from the calendar and file icons, as they were causing the content to appear misaligned. - Remove the time display from the card previews, as the full time information is already shown in the popover content. These changes help to streamline the UI and make the cards more visually appealing and easier to scan at a glance.
📝 WalkthroughWalkthroughThis update removes the standalone calendar route and view, replacing it with a unified "finder" interface that consolidates calendar, table, folder, and contact views under a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MainWindow
participant FinderWindow
participant AppBackend
User->>MainWindow: Clicks "Finder" button or triggers finder action
MainWindow->>AppBackend: windowsCommands.windowShow({ type: "finder" })
AppBackend-->>FinderWindow: Opens Finder window at /app/finder
User->>FinderWindow: Interacts with view selector (calendar, table, folder, contact)
FinderWindow->>AppBackend: Loads data for selected view (sessions, events, contacts, etc.)
AppBackend-->>FinderWindow: Returns data for rendering
User->>FinderWindow: Selects item (e.g., person, event, session)
FinderWindow->>AppBackend: windowsCommands.windowNavigate({ url: ... })
AppBackend-->>FinderWindow: Navigates within Finder window to detail view
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (10)
plugins/apple-calendar/src/sync.rs (1)
203-239: Consider using the handled_system_event_ids HashSet in the new events section.While the current logic checks for already handled events in the
to_updatevector, it might be more efficient and consistent to also check thehandled_system_event_idsHashSet for complete duplicate prevention.for system_event in fresh_events { + // Skip if this event was already handled + if handled_system_event_ids.contains(&system_event.id) { + continue; + } + // Skip if this event was already handled as an update let already_handled = state .to_update .iter() .any(|e| e.tracking_id == system_event.id);apps/desktop/src/components/left-sidebar/notes-list.tsx (1)
236-244: Consider adding error handling for window operations.The refactored navigation logic correctly migrates from router-based to window command-based navigation. However, the async window operations could fail silently, potentially leaving users confused if the finder window doesn't open or navigate properly.
While the coding guidelines specify "no error handling," consider that window operations are external system calls that could fail due to platform limitations or resource constraints.
Consider adding basic error handling:
const handleOpenCalendar = () => { const date = new Date(currentSession.created_at); const formattedDate = format(date, "yyyy-MM-dd"); const url = `/app/finder?view=calendar&date=${formattedDate}`; - windowsCommands.windowShow({ type: "finder" }).then(() => { - windowsCommands.windowEmitNavigate({ type: "finder" }, url); - }); + windowsCommands.windowShow({ type: "finder" }).then(() => { + windowsCommands.windowEmitNavigate({ type: "finder" }, url); + }).catch((error) => { + console.error("Failed to open calendar view:", error); + }); };apps/desktop/src/components/finder/views/folder-view.tsx (1)
1-4: Address unused props to comply with coding guidelines.The props
dateandonNavigateare defined but not used in the component implementation. While this is expected for a placeholder component, it violates the coding guideline "No unused imports, variables, or functions."Consider one of these approaches:
-export function FolderView({ date, onNavigate }: FolderViewProps) { +export function FolderView({ date: _date, onNavigate: _onNavigate }: FolderViewProps) {Or add a comment explaining the placeholder nature:
export function FolderView({ date, onNavigate }: FolderViewProps) { + // Props reserved for future implementation return (apps/desktop/src/components/command-palette.tsx (1)
285-301: Consider error handling for better user experience.Similar to the notes-list component, these async window operations could fail silently, potentially leaving the command palette open when navigation fails.
Consider adding error handling:
case "human": // Open finder window and navigate to contact view with person selected windowsCommands.windowShow({ type: "finder" }).then(() => { windowsCommands.windowNavigate( { type: "finder" }, `/app/finder?view=contact&personId=${match.item.id}`, ); - }); + }).catch((error) => { + console.error("Failed to open contact view:", error); + });apps/desktop/src/components/workspace-calendar/index.tsx (1)
93-103: Consider performance optimization for large datasets.The current filtering uses nested loops (
filter+some) which results in O(n*m) complexity. For large numbers of sessions and events, this could impact performance.Consider optimizing with a Set for better performance:
const getItemsForDay = (date: Date) => { const dateStr = format(date, "yyyy-MM-dd"); const todayStr = format(today, "yyyy-MM-dd"); const isFutureDate = dateStr > todayStr; const isPastDate = dateStr < todayStr; const daySessions = getSessionsForDay(date); const dayEvents = getEventsForDay(date); + // Create a Set of event IDs for faster lookup + const eventIds = new Set(events.map(event => event.id)); + // Filter out sessions that are linked to events to prevent duplicate display // When a session is linked to an event, show only the event (not both) const unlinkedSessions = daySessions.filter(session => { // If session has calendar_event_id, check if that event exists in current events if (session.calendar_event_id) { // If the linked event exists in our events list, don't show the session separately - return !events.some(event => event.id === session.calendar_event_id); + return !eventIds.has(session.calendar_event_id); } // Session without calendar_event_id can be shown return true; });apps/desktop/src/components/finder/view-selector.tsx (1)
12-68: Consider accessibility improvements and maintainable styling.The component implementation is solid, but could benefit from:
- Accessibility: Add
aria-pressedattributes andaria-labelfor better screen reader support- Layout stability: The conditional label rendering may cause layout shifts
Consider this improvement for accessibility:
<Button variant={currentView === "folder" ? "default" : "ghost"} size="sm" + aria-pressed={currentView === "folder"} + aria-label={currentView === "folder" ? "Folder view (active)" : "Switch to folder view"} className={cn( "h-8 transition-all", currentView === "folder" ? "px-1.5 py-1 min-w-[70px]" : "w-8 px-0 py-0", )} onClick={() => onViewChange("folder")} >Apply similar changes to all buttons for consistency.
apps/desktop/src/routes/app.finder.tsx (1)
97-146: Table view loader is correctly implemented but duplicates calendar logic.The implementation works correctly with proper query key separation, but there's significant code duplication with the calendar view loader.
Consider extracting a shared function for the common data fetching logic:
const fetchMonthData = async (date: Date, userId: string, queryClient: any, keyPrefix: string) => { const [start, end] = [startOfMonth(date), endOfMonth(date)].map((v) => v.toISOString()); const [sessions, rawEvents] = await Promise.all([ queryClient.fetchQuery({ queryKey: [`${keyPrefix}-sessions`, start, end], queryFn: () => dbCommands.listSessions({ type: "dateRange", user_id: userId, start, end, limit: 100, }), }), queryClient.fetchQuery({ queryKey: [`${keyPrefix}-events`, start, end], queryFn: () => dbCommands.listEvents({ type: "dateRange", user_id: userId, start, end, limit: 100, }), }), ]); // Deduplicate events... const eventsByTrackingId = new Map(); rawEvents.forEach(event => { const existing = eventsByTrackingId.get(event.tracking_id); if (!existing || event.id > existing.id) { eventsByTrackingId.set(event.tracking_id, event); } }); return { sessions, events: Array.from(eventsByTrackingId.values()) }; };apps/desktop/src/components/finder/views/calendar-view.tsx (1)
10-38: State management and navigation logic are well-implemented.The component properly manages local date state while communicating changes to the parent. The date calculations using date-fns are correct.
Consider using more specific types instead of
any[]for better type safety:interface CalendarViewProps { date: Date; - sessions: any[]; - events: any[]; + sessions: Session[]; + events: Event[]; onNavigate: (params: { date: string }) => void; }apps/desktop/src/components/finder/views/table-view.tsx (1)
132-138: Add error logging for debugging.The error should be logged for debugging purposes before falling back to creating a new note.
} catch (error) { + console.error("Error fetching session for event:", error); // If session lookup fails, create new note const url = `/app/new?calendarEventId=${item.id}`; windowsCommands.windowShow({ type: "main" }).then(() => { windowsCommands.windowEmitNavigate({ type: "main" }, url); }); }apps/desktop/src/components/finder/views/contact-view.tsx (1)
20-900: Consider extracting nested components to separate files for better maintainability.The file contains multiple components that could be moved to separate files to improve code organization and maintainability, especially as the finder feature grows.
Consider organizing the components as:
contact-view/index.tsx(main ContactView)contact-view/EditPersonForm.tsxcontact-view/OrganizationSelector.tsxcontact-view/OrganizationForms.tsx
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
.gitignore(1 hunks)apps/desktop/src/components/command-palette.tsx(2 hunks)apps/desktop/src/components/finder/index.ts(1 hunks)apps/desktop/src/components/finder/view-selector.tsx(1 hunks)apps/desktop/src/components/finder/views/calendar-view.tsx(1 hunks)apps/desktop/src/components/finder/views/contact-view.tsx(1 hunks)apps/desktop/src/components/finder/views/folder-view.tsx(1 hunks)apps/desktop/src/components/finder/views/index.ts(1 hunks)apps/desktop/src/components/finder/views/table-view.tsx(1 hunks)apps/desktop/src/components/left-sidebar/events-list.tsx(2 hunks)apps/desktop/src/components/left-sidebar/notes-list.tsx(2 hunks)apps/desktop/src/components/left-sidebar/search-list.tsx(0 hunks)apps/desktop/src/components/left-sidebar/top-area/finder-button.tsx(2 hunks)apps/desktop/src/components/left-sidebar/top-area/index.tsx(2 hunks)apps/desktop/src/components/toolbar/bars/calendar-toolbar.tsx(0 hunks)apps/desktop/src/components/toolbar/bars/index.tsx(0 hunks)apps/desktop/src/components/toolbar/index.tsx(1 hunks)apps/desktop/src/components/workspace-calendar/event-card.tsx(4 hunks)apps/desktop/src/components/workspace-calendar/index.tsx(1 hunks)apps/desktop/src/components/workspace-calendar/note-card.tsx(3 hunks)apps/desktop/src/locales/en/messages.po(10 hunks)apps/desktop/src/locales/ko/messages.po(10 hunks)apps/desktop/src/routeTree.gen.ts(11 hunks)apps/desktop/src/routes/app.calendar.tsx(0 hunks)apps/desktop/src/routes/app.finder.tsx(1 hunks)packages/utils/src/index.ts(0 hunks)packages/utils/src/navigation.ts(0 hunks)plugins/apple-calendar/src/sync.rs(3 hunks)plugins/windows/js/bindings.gen.ts(1 hunks)plugins/windows/src/ext.rs(5 hunks)
💤 Files with no reviewable changes (6)
- packages/utils/src/index.ts
- apps/desktop/src/components/toolbar/bars/index.tsx
- apps/desktop/src/components/left-sidebar/search-list.tsx
- apps/desktop/src/routes/app.calendar.tsx
- packages/utils/src/navigation.ts
- apps/desktop/src/components/toolbar/bars/calendar-toolbar.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit Configuration File
**/*.{js,ts,tsx,rs}: 1. No error handling.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
Files:
apps/desktop/src/components/left-sidebar/top-area/index.tsxplugins/windows/js/bindings.gen.tsapps/desktop/src/components/toolbar/index.tsxplugins/apple-calendar/src/sync.rsapps/desktop/src/components/left-sidebar/top-area/finder-button.tsxapps/desktop/src/components/finder/index.tsapps/desktop/src/components/finder/view-selector.tsxapps/desktop/src/components/command-palette.tsxapps/desktop/src/components/left-sidebar/events-list.tsxapps/desktop/src/components/finder/views/index.tsapps/desktop/src/components/finder/views/folder-view.tsxapps/desktop/src/components/left-sidebar/notes-list.tsxapps/desktop/src/routes/app.finder.tsxapps/desktop/src/components/workspace-calendar/index.tsxplugins/windows/src/ext.rsapps/desktop/src/components/workspace-calendar/note-card.tsxapps/desktop/src/components/finder/views/table-view.tsxapps/desktop/src/components/finder/views/calendar-view.tsxapps/desktop/src/components/workspace-calendar/event-card.tsxapps/desktop/src/routeTree.gen.tsapps/desktop/src/components/finder/views/contact-view.tsx
⏰ 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: ci (macos, macos-latest)
- GitHub Check: ci (windows, windows-latest)
🔇 Additional comments (46)
.gitignore (1)
16-18: LGTM! Standard .gitignore maintenance.The addition of
CLAUDE.mdto the ignore list and proper formatting with newlines are appropriate housekeeping changes.apps/desktop/src/components/left-sidebar/top-area/index.tsx (2)
6-6: Import correctly updated for finder refactoring.The import change from
CalendarButtontoFinderButtonaligns with the PR's migration from calendar to finder functionality.
26-26: JSX element correctly updated.The component usage properly reflects the import change, maintaining consistency in the finder migration.
plugins/windows/js/bindings.gen.ts (1)
73-73: Generated bindings correctly reflect finder window type.The union type update from
{ type: "calendar" }to{ type: "finder" }properly reflects the backend changes for the finder refactoring.apps/desktop/src/components/finder/views/index.ts (1)
1-4: All referenced finder view modules exist and barrel export is correctVerified that the following files are present under
apps/desktop/src/components/finder/views/:
calendar-view.tsxcontact-view.tsxfolder-view.tsxtable-view.tsxNo further changes needed—this barrel export cleanly centralizes the finder view components.
plugins/apple-calendar/src/sync.rs (3)
127-128: Good addition of duplicate prevention mechanism.The
HashSetto track handled system event IDs is a solid improvement to prevent duplicate processing during synchronization.
165-166: Proper tracking of handled events.Correctly marking matched events as handled to prevent duplicate processing.
189-190: Consistent handling of rescheduled events.Properly tracking rescheduled events to prevent them from being created as duplicates.
apps/desktop/src/components/finder/index.ts (1)
1-5: Missing ContactView export mentioned in summary.The AI summary indicates that
ContactViewshould be exported alongside the other views, but it's missing from this index file. This could cause import errors if other modules expect to importContactViewfrom this barrel export.#!/bin/bash # Verify if ContactView exists and should be exported fd -t f "contact-view" apps/desktop/src/components/finder/ ast-grep --pattern $'export { ContactView }'Likely an incorrect or invalid review comment.
apps/desktop/src/components/toolbar/index.tsx (1)
5-5: Clean removal of calendar toolbar functionality.The calendar toolbar references have been properly removed without leaving any unused imports or dead code. The remaining toolbar logic is intact and well-structured.
apps/desktop/src/components/left-sidebar/top-area/finder-button.tsx (1)
8-30: Clean and consistent refactoring from calendar to finder.The component has been properly renamed with all references updated consistently:
- Function name, icon, window command, and tooltip all align with the new finder functionality
- The Folder icon is appropriate for the finder concept
- Implementation remains simple and focused
apps/desktop/src/locales/en/messages.po (1)
1-1361: Localization file properly updated for code refactoring.The PO file has been correctly updated with new source line number references following the finder view merge refactoring. The changes include:
- Updated line number references to match moved code
- Appropriate toggling of active/inactive translation entries
- No changes to actual translation strings
- Maintains proper gettext format
These appear to be automated updates that correctly reflect the codebase changes.
apps/desktop/src/locales/ko/messages.po (1)
1-1361: LGTM: Localization reference updates align with code refactoring.These changes appropriately update source code line references in the Korean localization file to reflect the structural changes from calendar-centric to finder-based navigation. The mechanical nature of these updates is expected when refactoring code that uses internationalization.
apps/desktop/src/components/finder/views/folder-view.tsx (1)
6-20: LGTM: Clean placeholder implementation.The component structure and styling are appropriate for a placeholder. The centered layout and informative messaging provide good user expectations for the upcoming feature.
apps/desktop/src/components/command-palette.tsx (1)
285-301: Align onwindowEmitNavigatefor window navigation and add error handlingTo keep navigation calls consistent across the codebase (e.g. in
notes-list.tsx,table-view.tsx, etc.), replacewindowsCommands.windowNavigatewithwindowsCommands.windowEmitNavigateinapps/desktop/src/components/command-palette.tsx. Also add error handling so the command palette reliably closes even if navigation fails:--- a/apps/desktop/src/components/command-palette.tsx +++ b/apps/desktop/src/components/command-palette.tsx @@ -285,8 +285,12 @@ switch (match.item.type) { // Open finder window and navigate to contact view with person selected - windowsCommands.windowShow({ type: "finder" }).then(() => { - windowsCommands.windowNavigate( - { type: "finder" }, - `/app/finder?view=contact&personId=${match.item.id}`, - ); - }); + windowsCommands + .windowShow({ type: "finder" }) + .then(() => + windowsCommands.windowEmitNavigate( + { type: "finder" }, + `/app/finder?view=contact&personId=${match.item.id}`, + ) + ) + .catch((err) => { + console.error("Failed to navigate finder window:", err); + }); break; case "organization": // Open finder window and navigate to contact view with organization selected @@ -294,8 +298,12 @@ switch (match.item.type) { - windowsCommands.windowShow({ type: "finder" }).then(() => { - windowsCommands.windowNavigate( - { type: "finder" }, - `/app/finder?view=contact&orgId=${match.item.id}`, - ); - }); + windowsCommands + .windowShow({ type: "finder" }) + .then(() => + windowsCommands.windowEmitNavigate( + { type: "finder" }, + `/app/finder?view=contact&orgId=${match.item.id}`, + ) + ) + .catch((err) => { + console.error("Failed to navigate finder window:", err); + }); break;
- Files requiring updates:
apps/desktop/src/components/command-palette.tsx(lines 285–292, 294–301)Likely an incorrect or invalid review comment.
apps/desktop/src/components/workspace-calendar/index.tsx (1)
93-103: LGTM: Well-implemented duplicate prevention logic.The filtering logic correctly prevents duplicate display of sessions that are linked to calendar events. The comments clearly explain the intention, and the implementation properly handles both linked and unlinked sessions.
apps/desktop/src/components/workspace-calendar/note-card.tsx (7)
2-2: Import changes look good.The updated imports properly support the new functionality: FileText icon for recorded sessions and windowsCommands for the new navigation approach.
Also applies to: 4-4, 8-8, 10-10
23-32: Linked event query implementation is well-structured.The conditional enabling and null handling are appropriate, and this will properly display calendar event names when available.
67-70: Navigation refactor aligns with the new window command pattern.The transition from router-based to window command-based navigation is implemented correctly and consistently with the broader architectural changes.
74-84: Date handling logic correctly differentiates recorded vs non-recorded sessions.The fallback logic from record timestamps to creation timestamps is appropriate and will handle both session types properly.
Also applies to: 87-88
93-106: UI improvements provide better visual distinction and context.The conditional icons and preference for calendar event names over session titles will improve user experience by providing more relevant information.
108-124: Time display logic is well-implemented.The differentiation between recorded sessions (time ranges) and non-recorded sessions (creation time) with smart cross-day handling provides clear temporal context.
133-143: Button replacement with styled div maintains functionality and styling.The div-based approach is simpler while preserving all visual feedback and interaction patterns.
apps/desktop/src/routes/app.finder.tsx (4)
11-22: Route configuration and schema validation are well-structured.The Zod schema properly defines all search parameters with appropriate defaults and the route configuration follows best practices.
31-95: Calendar view loader logic is comprehensive and well-implemented.The date handling fallbacks, parallel data fetching, and event deduplication logic properly address sync issues while maintaining performance.
148-157: Simple view cases are efficiently handled.Returning base data for folder and contact views avoids unnecessary API calls while maintaining consistent structure.
169-234: React component implementation follows best practices.The navigation handlers, conditional rendering, and prop passing are all correctly implemented. The layout structure properly handles the different view types.
apps/desktop/src/components/finder/views/calendar-view.tsx (2)
42-90: Header UI implementation is well-structured with proper i18n and accessibility.The month navigation controls, date formatting, and grid layout for weekdays are all implemented correctly with appropriate Tauri integration.
92-101: Calendar content area implementation is clean and properly integrated.The layout handling and WorkspaceCalendar integration are correctly implemented with appropriate prop passing.
apps/desktop/src/components/workspace-calendar/event-card.tsx (4)
3-3: Import changes properly support the new functionality.The added imports for icons, URL opening utility, and window commands align with the component's updated features.
Also applies to: 5-5, 9-9, 12-12
68-77: Navigation refactor is consistent with the architectural changes.The window command-based navigation properly handles both existing and new session scenarios, maintaining the same user experience.
91-108: UI improvements provide better visual clarity and functionality.The Calendar icon and clickable event title with proper error handling and visual feedback enhance the user experience.
120-147: Styling improvements and button replacements maintain functionality while simplifying structure.The participant text visibility improvement, intuitive icon choices, and styled div approach all enhance the component while maintaining proper interaction patterns.
plugins/windows/src/ext.rs (5)
21-22: LGTM!The enum variant rename from
CalendartoFinderis consistent with the PR's objective.
40-40: LGTM!Display implementation correctly updated to match the renamed variant.
55-55: LGTM!FromStr implementation correctly handles the renamed variant.
146-146: LGTM!Window title correctly updated to "Finder".
319-322: LGTM!Window configuration properly updated with appropriate path and dimensions for the finder interface.
apps/desktop/src/components/finder/views/table-view.tsx (3)
192-320: Well-implemented table UI with good UX patterns.The table implementation includes proper sorting indicators, hover states, empty state handling, and tag overflow management.
142-154: LGTM!Duration formatting logic is well-implemented with proper edge case handling.
33-98: Good use of React patterns and performance optimizations.The component properly uses
useMemofor expensive operations and maintains clean state management.apps/desktop/src/routeTree.gen.ts (1)
1-301: Auto-generated file changes are consistent.The route tree generation correctly reflects the calendar to finder migration. No manual changes should be made to this file.
apps/desktop/src/components/finder/views/contact-view.tsx (4)
1-20: LGTM!Clean imports and well-defined component interface.
528-575: LGTM!Well-implemented organization selector with proper popover handling and data fetching.
755-766: Good implementation of click outside handling.The useEffect properly sets up and cleans up the event listener for click outside detection.
28-38: Add error handling to all queries for consistency.The organizations and people queries should handle errors similar to the allPeople query.
const { data: organizations = [] } = useQuery({ queryKey: ["organizations", userId], - queryFn: () => dbCommands.listOrganizations(null), + queryFn: async () => { + try { + return await dbCommands.listOrganizations(null); + } catch (error) { + console.error("Error fetching organizations:", error); + return []; + } + }, });Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
apps/desktop/src/components/finder/views/contact-view.tsx (1)
54-83: N+1 query pattern still present for session participants.This implementation fetches all sessions and then queries participants for each one, creating performance issues with many sessions.
🧹 Nitpick comments (1)
apps/desktop/src/components/finder/views/contact-view.tsx (1)
503-507: Consider validating LinkedIn username format.The placeholder shows a full URL, but the field name is
linkedin_username. Consider either:
- Storing just the username and constructing the URL when needed
- Validating the input to extract the username from a full URL
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop/src/components/finder/views/contact-view.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit Configuration File
**/*.{js,ts,tsx,rs}: 1. No error handling.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
Files:
apps/desktop/src/components/finder/views/contact-view.tsx
⏰ 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: ci (windows, windows-latest)
- GitHub Check: ci (macos, macos-latest)
🔇 Additional comments (5)
apps/desktop/src/components/finder/views/contact-view.tsx (5)
1-19: Well-organized imports and proper TypeScript interface definition.The imports are properly structured and the ContactViewProps interface is correctly typed.
112-117: Clean navigation implementation using window commands.The navigation to notes using windowShow and windowEmitNavigate properly follows the new window management pattern.
346-362: Add error handling for organization fetch.The query should handle potential errors:
const { data: organization } = useQuery({ queryKey: ["organization", organizationId], queryFn: () => dbCommands.getOrganization(organizationId), enabled: !!organizationId, + onError: (error) => { + console.error("Error fetching organization:", error); + }, });Likely an incorrect or invalid review comment.
28-39: Add error handling for data fetching queries.The organizations and organization members queries lack error handling, unlike the allPeople query. Add error handlers to prevent silent failures:
const { data: organizations = [] } = useQuery({ queryKey: ["organizations", userId], queryFn: () => dbCommands.listOrganizations(null), + onError: (error) => { + console.error("Error fetching organizations:", error); + }, }); const { data: people = [] } = useQuery({ queryKey: ["organization-members", selectedOrganization], queryFn: () => selectedOrganization ? dbCommands.listOrganizationMembers(selectedOrganization) : Promise.resolve([]), enabled: !!selectedOrganization, + onError: (error) => { + console.error("Error fetching organization members:", error); + }, });Likely an incorrect or invalid review comment.
395-397: Improve error handling with user feedback.Instead of only logging to console, show user-friendly error messages:
onError: () => { console.error("Failed to update contact"); + // Consider adding a toast notification or error state + // to inform the user about the failure },⛔ Skipped due to learnings
Learnt from: plyght PR: fastrepl/hyprnote#921 File: plugins/location-connectivity/src/error.rs:1-41 Timestamp: 2025-06-06T16:31:46.457Z Learning: In the location-connectivity plugin (plugins/location-connectivity/), comprehensive error handling via LocationConnectivityError is necessary despite the "no error handling" guideline, as the plugin performs system-level operations (WiFi detection, persistent storage) that require proper error propagation to the frontend.
No description provided.