-
-
Notifications
You must be signed in to change notification settings - Fork 186
feat: show/hide calendars and filter events #56
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
* feat: include calendar id into each event
|
@iboughtbed is attempting to deploy a commit to the Analog Interface Team on Vercel. A member of the Team first needs to authorize it. |
a-rebets
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Solid change! Just needs some polish
fix: use hooks instead of atoms
WalkthroughThis update introduces a persistent state mechanism for calendar visibility using Jotai atoms, including new types, atoms, and custom hooks. The calendar event data model is extended to include a Changes
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (6)
🪧 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
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
apps/web/src/atoms/calendars-visbility.ts (1)
1-11:⚠️ Potential issueFix the filename typo.
The filename contains a typo:
calendars-visbility.tsshould becalendars-visibility.ts(missing 'i' in "visibility").The implementation follows good patterns with Jotai's
atomWithStorageand adheres to the agreed naming convention with the "analog-" prefix.-// File: apps/web/src/atoms/calendars-visbility.ts +// File: apps/web/src/atoms/calendars-visibility.tsMake sure to update all imports of this file when renaming it.
♻️ Duplicate comments (1)
packages/api/src/routers/events.ts (1)
37-40: Formatting improvements enhance readability.The multi-line formatting makes the code more readable and easier to follow, especially for complex operations like filtering, error handling, and chaining operations.
Note: This aligns with the previous reviewer's observation about formatting inconsistencies across the codebase.
Also applies to: 48-52, 55-58, 64-68, 136-140
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
apps/web/src/atoms/calendars-visbility.ts(1 hunks)apps/web/src/atoms/index.ts(1 hunks)apps/web/src/components/calendar-view.tsx(1 hunks)apps/web/src/components/calendars.tsx(3 hunks)apps/web/src/components/event-calendar/event-calendar.tsx(3 hunks)apps/web/src/components/event-calendar/hooks/index.ts(1 hunks)apps/web/src/components/event-calendar/hooks/use-calendars-visibility.ts(1 hunks)apps/web/src/components/event-calendar/types.ts(1 hunks)apps/web/src/components/event-calendar/utils/event.ts(12 hunks)apps/web/src/components/ui/checkbox.tsx(1 hunks)packages/api/src/routers/events.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
apps/web/src/components/event-calendar/hooks/use-calendars-visibility.ts (3)
apps/web/src/components/event-calendar/hooks/index.ts (1)
useCalendarsVisibility(18-18)apps/web/src/atoms/index.ts (1)
calendarsVisibilityAtom(6-6)apps/web/src/atoms/calendars-visbility.ts (1)
calendarsVisibilityAtom(7-10)
packages/api/src/routers/events.ts (1)
packages/api/src/providers/google-calendar.ts (1)
events(33-46)
apps/web/src/components/calendars.tsx (4)
apps/web/src/components/event-calendar/hooks/use-calendars-visibility.ts (1)
useCalendarsVisibility(4-6)apps/web/src/components/ui/checkbox.tsx (1)
Checkbox(59-59)apps/web/src/components/ui/sidebar.tsx (1)
SidebarMenuButton(715-715)apps/web/src/components/ui/tooltip.tsx (2)
TooltipTrigger(60-60)TooltipContent(60-60)
apps/web/src/components/event-calendar/utils/event.ts (1)
apps/web/src/components/event-calendar/types.ts (1)
CalendarEvent(3-13)
🔇 Additional comments (13)
apps/web/src/components/ui/checkbox.tsx (1)
17-17: LGTM! Clean syntax formatting.Good cleanup removing the unnecessary trailing comma. This improves code consistency without affecting functionality.
apps/web/src/components/event-calendar/hooks/index.ts (1)
18-18: LGTM! Proper hook export organization.Good addition following the existing pattern. The placement in the "Utility hooks" section is appropriate since this hook manages state utilities for calendar visibility.
apps/web/src/components/calendar-view.tsx (1)
85-85: LGTM! Calendar ID integration looks correct.The addition of
calendarIdto the transformed event object properly supports the calendar visibility filtering feature. This change aligns perfectly with the updatedCalendarEventinterface and enables downstream filtering logic.apps/web/src/atoms/index.ts (1)
4-8: LGTM! Clean atom exports following established patterns.The addition of
calendarsVisibilityAtomandCalendarsVisibilitytype exports follows the same pattern as existing atoms and provides proper centralized access to the new calendar visibility state.apps/web/src/components/event-calendar/hooks/use-calendars-visibility.ts (1)
1-6: LGTM! Excellent custom hook implementation.This custom hook provides a clean abstraction over the Jotai atom, following React best practices. The wrapper pattern makes it easy for components to access calendar visibility state while maintaining flexibility for future implementation changes.
apps/web/src/components/event-calendar/utils/event.ts (2)
40-45: LGTM! Clean and efficient event filtering implementation.The
filterVisibleEventsfunction correctly filters out events from hidden calendars using a clear and efficient approach. The logic properly handles edge cases (empty hiddenCalendars array) and integrates well with the calendar visibility feature.
32-32: LGTM! Consistent formatting improvements.The removal of trailing commas throughout the file improves code consistency and readability while maintaining the same functionality.
Also applies to: 49-49, 61-61, 78-78, 95-95, 106-106, 131-131, 160-160, 176-176, 191-192, 215-215, 225-225, 231-231, 238-238, 263-263, 266-266
packages/api/src/routers/events.ts (1)
53-53: Excellent addition of calendarId to event objects.This change enables the frontend to filter events by calendar visibility, which is essential for the show/hide calendars feature. The spread operator correctly preserves all existing event properties while adding the calendar identifier.
apps/web/src/components/event-calendar/event-calendar.tsx (2)
21-21: Good adoption of custom hooks.Replacing direct atom usage with
useViewPreferencesanduseCalendarsVisibilityhooks improves code organization and reusability. This addresses the previous review feedback about using hooks instead of direct atom access.
42-47: Well-structured filtering chain.The filtering logic correctly chains
filterPastEventsandfilterVisibleEvents, applying both user preferences and calendar visibility state. The dependency array properly includes all relevant dependencies for the memoization.apps/web/src/components/calendars.tsx (3)
39-50: Excellent use of useCallback for event handling.The callback function is properly memoized and handles calendar visibility state correctly. The logic appropriately adds/removes calendar IDs from the hiddenCalendars array based on the checkbox state. This addresses the previous review feedback about extracting logic into useCallback.
82-95: Interactive checkboxes improve user experience.The replacement of static checkmarks with interactive checkboxes enables users to toggle calendar visibility dynamically. The checked state correctly reflects whether a calendar is hidden (!hiddenCalendars.includes(item.id)), and the onCheckedChange handler properly updates the visibility state.
101-106: Nice tooltip enhancement.The tooltip provides better user interaction feedback with appropriate styling and positioning. The bottom-aligned tooltip with start alignment works well for sidebar menu items.
* chore: remove unused imports
a-rebets
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool! Thanks for updating, a few small things left
cfd534d to
c1e2fb1
Compare
a-rebets
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM let's go
|
@JeanMeijer this one is ready |
|
formatted, merged, resolved conflicts, ready to merge, @JeanMeijer |
Summary by CodeRabbit
New Features
Improvements
Documentation