feat: add keyboard shortcuts and tooltips to booking slideover buttons#27428
Conversation
Co-Authored-By: peer@cal.com <peer@cal.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
This PR has been marked as stale due to inactivity. If you're still working on it or need any help, please let us know or update the PR to keep it active. |
Devin AI is completing this stale PRThis PR by @PeerRich has been marked as stale. A Devin session has been created to complete the remaining work. Devin will review the PR, address any feedback, and push updates to complete this PR. |
- Removed duplicate import of useBookingLocation from non-existent @calcom/web/modules/bookings/hooks/useBookingLocation path - Fixed import ordering to satisfy biome organizeImports rules - Removed unnecessary code comment - Original feature by @PeerRich via Devin AI Co-Authored-By: unknown <>
Completed stale PRI've reviewed and fixed the following issues in this PR: Bug fix:
Code quality:
Verification:
The original feature (keyboard shortcuts + tooltips for booking slideover buttons) by @PeerRich is complete and functional as-is. The changes I made are purely correctness fixes. |
E2E results are ready! |
…oking-slideover-keyboard-shortcuts
dhairyashiil
left a comment
There was a problem hiding this comment.
Screen.Recording.2026-02-12.at.12.04.20.PM.mov
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/web/modules/bookings/components/BookingDetailsSheet.tsx">
<violation number="1" location="apps/web/modules/bookings/components/BookingDetailsSheet.tsx:726">
P2: The global Enter shortcut hijacks keyboard interaction for non-input controls. Pressing Enter on another button/link/select inside the sheet will be prevented and will always open the join link. Consider ignoring Enter when the active element is any interactive control (button, select, link, etc.) or when the event is already handled.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Devin AI is addressing Cubic AI's review feedbackA Devin session has been created to address the issues identified by Cubic AI. |
|
Reviewed the Cubic AI feedback on this PR. The single issue identified (global Enter shortcut hijacking keyboard interaction for non-input controls) has a confidence score of 7/10, which is below the 9/10 threshold for automated fixes. Skipping this change. For context: the existing code already guards the Enter shortcut behind |
|
fixed join button tooltip hover and enter click works now Screen.Recording.2026-02-12.at.12.30.52.PM.mov |
…g during booking navigation Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
…first/last booking Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
Devin AI is resolving merge conflictsThis PR has merge conflicts with the Devin will:
If you prefer to resolve conflicts manually, you can close the Devin session and handle it yourself. |
Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/web/playwright/booking-sheet-keyboard.e2e.ts">
<violation number="1" location="apps/web/playwright/booking-sheet-keyboard.e2e.ts:121">
P2: Rule violated: **E2E Tests Best Practices**
Rule 1 (E2E Tests Best Practices) requires avoiding text locators and using data-testid selectors. These new tests rely on `text=Booking ...` locators, which are brittle and violate the guideline.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Devin AI is addressing Cubic AI's review feedbackNew feedback has been sent to the existing Devin session. |
Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
|
Addressed the Cubic AI feedback: replaced all Changes:
All 6 E2E tests pass locally. ✅ Pushed commit |
Devin AI is resolving merge conflictsThis PR has merge conflicts with the Devin will:
If you prefer to resolve conflicts manually, you can close the Devin session and handle it yourself. |
…ing-slideover-keyboard-shortcuts
…ing-slideover-keyboard-shortcuts
https://git-manager.devin.ai/proxy/github.com/calcom/cal.com into devin/1769773070-booking-slideover-keyboard-shortcuts
… with useCallback In BookingDetailsSheet.tsx, all three handler functions were plain arrow functions recreated on every render, causing the useEffect to tear down and re-attach the document keydown listener unnecessarily. Wrapped all three in useCallback with proper dependency arrays (the Zustand store functions they call). Fix 2 — data-booking-list-item verified (no change needed) Confirmed that data-booking-list-item is rendered on BookingListItem.tsx and data-booking-calendar-event is rendered on Event.tsx. The onInteractOutside handler in the final merged state correctly checks both selectors. No code change required. Fix 3 — Removed dead code from JoinMeetingButton Reverted JoinMeetingButton back to a plain function component: Removed forwardRef wrapping (no caller passes a ref) Removed showTooltip prop (unused — tooltip is handled by the parent BookingDetailsSheet) Removed ref prop from the inner Button Removed unused Tod forwardRef imports
What does this PR do?
Adds keyboard shortcuts and tooltips to the booking details slideover navigation buttons:
Each button now displays a tooltip showing the keyboard shortcut on hover.
Keyboard shortcuts are automatically disabled when focus is inside a Radix overlay (dialog, dropdown, popover, etc.) so they don't interfere with overlay interactions like BookingActionsDropdown.
Changes
bookingSheetKeyboardHandler.ts(new): Extracted keyboard handler logic into a testable utility. ExportsisEditableTarget,checkSheetActive, andcreateBookingSheetKeydownHandler.bookingSheetKeyboardHandler.test.ts(new): 25 unit tests covering editable target detection, sheet active state, arrow key propagation behavior (including at list boundaries), Enter key handling, overlay suppression, and unhandled key passthrough.booking-sheet-keyboard.e2e.ts(new): 6 Playwright E2E tests covering arrow navigation, boundary conditions (first/last booking), dropdown suppression at boundaries, Escape to close, and rapid keypresses.BookingDetailsSheet.tsx: Uses the extracted utility for keyboard handling. Wrapped navigation and close buttons with<Tooltip>. AddedisSheetActive()check using Radix portal detection. UpdatedonInteractOutsideto check both[data-booking-list-item]and[data-booking-calendar-event]selectors. Addeddata-testid="booking-sheet-title"for stable E2E selectors.JoinMeetingButton.tsx: Converted toforwardRefto support ref from parent. Added optionalshowTooltipprop that wraps button in<Tooltip>.Event.tsx(weekly calendar): Addeddata-booking-uidattribute anddata-booking-calendar-eventattribute to calendar event buttons so the sheet stays open when clicking between events.common.json: Added 4 i18n keys:close_shortcut,previous_shortcut,next_shortcut,join_shortcut.Calendar view fix
The sheet's
onInteractOutsidehandler previously only checked for[data-booking-list-item], which meant clicking between calendar events would close and reopen the sheet. Fixed by:data-booking-uidanddata-booking-calendar-eventto calendar event buttons inEvent.tsx[data-booking-list-item]and[data-booking-calendar-event]in theonInteractOutsidehandler — this covers both list and calendar viewsArrow key / actions dropdown fix
Pressing Arrow Up/Down was opening the BookingActionsDropdown alongside (or instead of) navigating bookings. Root cause: Radix's
DropdownMenuTriggernatively opens onArrowDownkeypress. The keyboard handler needed to always consume arrow key events when the sheet is active — bothpreventDefault()andstopPropagation()— regardless of whether navigation is possible. Previously, these calls were inside theif (canGoNext/canGoPrev)guard, so at the first or last booking the event would leak through to Radix.Fixed by moving
e.preventDefault()ande.stopPropagation()outside the navigation guard for ArrowUp/ArrowDown. Navigation still only triggers when possible, but the event is always consumed.Overlay suppression approach
Uses Radix portal detection (
[data-radix-portal]) to determine whether the focused element is inside an overlay. ThecheckSheetActive()logic:if no element has focus, or focus is inside the sheet content, or focus is not inside any Radix portal → shortcuts work. Otherwise (focus is in a Radix portal that isn't the sheet) → shortcuts suppressed.
Capture phase and stopPropagation
The keyboard listener is registered in the capture phase (
addEventListener("keydown", handler, true)). This ensures shortcuts intercept events before they reach and activate any focused button. ArrowUp/ArrowDown always callstopPropagation()when the sheet is active (even at list boundaries). Enter callsstopPropagation()only when a join link is present.Human Review Checklist
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
/bookingspageChecklist
Link to Devin run: https://app.devin.ai/sessions/c89d946fc36b4c4b9921a095317deee0
Requested by: @eunjae-lee
Previous Devin session: https://app.devin.ai/sessions/9870c25d9ff84a7d9ede1cde6f03f246
Originally requested by: @PeerRich