-
-
Notifications
You must be signed in to change notification settings - Fork 19
Enhance calendar drag-n-drop experience and add context menu #2281
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
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
WalkthroughThis pull request enhances the calendar functionality in the UI. The changes introduce a context menu for the event card with options to edit, change color, and delete events. Several components and hooks are updated, including new methods in the calendar hook to process deletions and event updates using a queued mechanism with reduced debounce. Additionally, the event overlap logic is improved, the event form now supports an Enter key save action, priority selection is removed in favor of a static default, and new CSS variables along with a constant update are integrated. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant EC as EventCard Component
participant CM as ContextMenu
participant CH as useCalendar Hook
U->>EC: Right-click on event card
EC->>CM: Open context menu
U->>CM: Select "Delete Event" or "Change Color"
alt Delete event
CM->>EC: Invoke handleDelete
EC->>CH: Call deleteEvent()
CH-->>EC: Return deletion result
else Change color
CM->>EC: Invoke handleColorChange(newColor)
EC->>CH: Call updateEvent(newColor)
CH-->>EC: Return update result
end
sequenceDiagram
participant U as User Action
participant UC as useCalendar Hook
participant Q as Update Queue
participant DB as Database
U->>UC: Trigger event update
UC->>Q: Enqueue PendingEventUpdate
UC->>UC: processUpdateQueue()
UC->>DB: Execute update request
DB-->>UC: Return success/error response
UC->>U: Resolve update promise
Possibly related PRs
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
Scope: all 3 workspace projects ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
🧹 Nitpick comments (6)
packages/ui/src/components/ui/legacy/calendar/CalendarEventMatrix.tsx (1)
181-239
: Consider optimizing the overlap grouping for large data sets.The current approach merges groups in an O(n²) manner. For significantly large schedules, a more advanced data structure (e.g., interval tree or a line sweep algorithm) could improve performance.
packages/ui/src/hooks/use-calendar.tsx (2)
78-85
: Use more specific error type for_reject
.
While this interface is effective, consider usingunknown
or a more explicit error type for_reject
to improve type safety and clarity.
378-457
: Consider optional concurrency or batching enhancements.
Your queue-based approach for updates is correct and straightforward. If concurrency or batching performance becomes a concern in high-volume scenarios, you could introduce a max concurrency limit or more granular batching.packages/ui/src/components/ui/legacy/calendar/EventCard.tsx (3)
939-949
: Color update logic is sound.
Consider reverting the color in UI if the update fails to keep the UI consistent. This can be handled by storing the old color, then reverting on error.const handleColorChange = (newColor: SupportedColor) => { + const oldColor = localEvent.color; const eventId = event._originalId || id; updateEvent(eventId, { color: newColor }) .then(() => { showStatusFeedback('success'); }) .catch((error) => { console.error('Failed to update event color:', error); showStatusFeedback('error'); + setLocalEvent((prev) => ({ ...prev, color: oldColor })); }); };
951-957
: Consider success feedback on deletion.
You handle errors gracefully inhandleDelete
. A quick success toast or message might improve user feedback once deletion completes.
959-1224
: Context menu usability.
The dynamic context menu is a great addition. For a smoother user experience, you might prompt the user to confirm event deletion, preventing accidental removals.onClick={handleDelete} className="flex items-center gap-2" > <Trash2 className="h-4 w-4" /> - <span>Delete Event</span> + <span>Delete Event...</span>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
packages/ui/src/components/ui/legacy/calendar/EventCard.tsx
(19 hunks)packages/ui/src/components/ui/legacy/calendar/UnifiedEventModal.tsx
(3 hunks)packages/ui/src/components/ui/legacy/calendar/config.ts
(1 hunks)packages/ui/src/globals.css
(1 hunks)packages/types/src/primitives/calendar-event.ts
(1 hunks)packages/ui/src/components/ui/legacy/calendar/CalendarEventMatrix.tsx
(3 hunks)packages/ui/src/components/ui/legacy/calendar/EventCard.tsx
(11 hunks)packages/ui/src/components/ui/legacy/calendar/config.ts
(1 hunks)packages/ui/src/hooks/use-calendar.tsx
(6 hunks)packages/ui/src/components/ui/context-menu.tsx
(1 hunks)packages/ui/src/components/ui/legacy/calendar/EventCard.tsx
(4 hunks)packages/ui/src/components/ui/legacy/calendar/EventFormComponents.tsx
(2 hunks)packages/ui/src/components/ui/legacy/calendar/UnifiedEventModal.tsx
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
🔇 Additional comments (19)
packages/ui/src/components/ui/legacy/calendar/config.ts (1)
5-5
: Adjusted minimum event height for better UI renderingThe change to
MIN_EVENT_HEIGHT
reduces the minimum height from 20px to 16px (20 - 4). This adjustment likely improves the visual appearance of calendar events, especially when displaying multiple overlapping events.packages/ui/src/components/ui/context-menu.tsx (1)
68-68
: Improved context menu styling for better visual hierarchyChanging from
text-sm
totext-muted-foreground
in the ContextMenuSubTrigger component improves the visual styling of the context menu items. This creates better contrast and visual hierarchy in the menu, particularly for the new calendar event context menu.packages/ui/src/globals.css (1)
176-178
: Added new light color variables to support enhanced calendar functionalityThe addition of light indigo, cyan, and gray color variables in the dark theme completes the color palette needed for the calendar's context menu and event styling. These variables follow the established naming convention and maintain consistency with the existing color system.
packages/ui/src/components/ui/legacy/calendar/EventFormComponents.tsx (2)
132-133
: Added onEnter callback to improve form submission UXAdding the optional
onEnter
callback to the EventTitleInput component enables keyboard-driven form submission, which enhances accessibility and user experience.Also applies to: 137-138
151-159
: Enhanced input field with focus and keyboard interactionsThese changes significantly improve the EventTitleInput component's usability:
- Auto-selecting text on focus makes editing existing titles more efficient
- Handling Enter key presses enables quick form submission
- Auto-focusing the input saves the user an extra click when the form opens
All these are excellent UX improvements that make the calendar event creation and editing flow more intuitive and efficient.
packages/types/src/primitives/calendar-event.ts (1)
24-27
: Great addition of overlap properties to the interface.These optional properties provide a clear, flexible way to track overlap metrics within the calendar.
packages/ui/src/components/ui/legacy/calendar/UnifiedEventModal.tsx (2)
95-95
: No issues with destructuring calendar methods.Referencing
deleteEvent
along with other methods fromuseCalendar()
appears straightforward.
344-344
: Closing the modal on successful AI event save looks good.This maintains consistency in user flow by clearing the UI once the event is successfully added.
packages/ui/src/components/ui/legacy/calendar/CalendarEventMatrix.tsx (2)
146-150
: Using maps to store levels, counts, and groups is appropriate.Keeping separate structures for event level, overlap counts, and group IDs is a clean and explicit design.
267-273
: Returning newly assigned properties ensures consistent downstream usage.Including
_level
,_overlapCount
, and_overlapGroup
directly in the returned object aligns well with the augmented interface.packages/ui/src/hooks/use-calendar.tsx (5)
13-13
: No issues with the newly added import.
102-103
: Looks good.
Using aMap<string, PendingEventUpdate>
simplifies retrieving the latest update for each event. No immediate concerns.
106-108
: Good use of refs for queued processing.
Storing queued updates inupdateQueueRef
and tracking progress withisProcessingQueueRef
is a clean approach to controlling async flows.
459-529
: Robust event update logic.
This updatedupdateEvent
method neatly handles new events and pending updates. The unique_updateId
ensures proper tracking. Great job!
659-670
: Effective cleanup logic on unmount.
Clearing pending updates and timers prevents memory leaks and dangling async calls.packages/ui/src/components/ui/legacy/calendar/EventCard.tsx (4)
2-11
: Context menu imports look fine.
Nice modular approach to implementing the context menu functionality.
13-13
: No issues with the additional import.
This import forSupportedColor
aligns with usage elsewhere in the file.
22-26
: Imports of icons are consistent.
No problems observed in addingEdit
,Palette
, andTrash2
fromlucide-react
.
68-68
: Good extension of the hook usage.
IncludingdeleteEvent
from the calendar hook is consistent with the new context menu actions.
Summary by CodeRabbit
New Features
Style
Refactor