Add keyboard shortcut for creating event #175
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughIntroduces a reusable useKeyboardShortcut hook and refactors components to use it. Adds global “n” key handling to open create modes in task form and calendar. Updates the calendar’s “New” button to show a Kbd “N” hint on larger screens. Removes inline keydown logic from task component. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant D as Document
participant H as useKeyboardShortcut
participant C1 as TaskForm
participant C2 as Calendar
U->>D: Press "n"
D-->>H: keydown event
H->>H: Validate key, modifiers, repeat, and editing context
alt Valid "n" and not typing
H-->>C1: callback() (if mounted) -> setIsCreating(true)
H-->>C2: callback() (if mounted) -> setCreate(true)
C1->>C1: Open create dialog (task)
C2->>C2: Open create state (calendar)
else Ignored
H-->>D: No action
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
✨ 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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
hooks/use-keyboard-shortcut.ts (3)
3-3: Drop the explicit boolean type onenabled(Biome hint).The default value makes the type trivially inferrable. This quiets the linter without changing behavior.
-export function useKeyboardShortcut(key: string, callback: () => void, enabled: boolean = true) { +export function useKeyboardShortcut(key: string, callback: () => void, enabled = true) {
7-23: Stabilize callback, make key matching case-insensitive via normalization, and guard IME composition.
- Avoid re-subscribing on every render by storing the latest callback in a ref.
- Normalize both
e.keyandkeyto lowercase instead of comparing both cases.- Ignore events while composing (
e.isComposing) to play nice with IMEs.- Add a defensive
instanceof Elementcheck before callingclosest.-import { useEffect } from "react"; +import { useEffect, useRef } from "react"; -export function useKeyboardShortcut(key: string, callback: () => void, enabled: boolean = true) { - useEffect(() => { - if (!enabled) return; +export function useKeyboardShortcut(key: string, callback: () => void, enabled = true) { + const cbRef = useRef(callback); + // Keep latest callback without resubscribing + useEffect(() => { + cbRef.current = callback; + }, [callback]); + + useEffect(() => { + if (!enabled) return; + const normalized = key.toLowerCase(); const handleKeyDown = (e: KeyboardEvent) => { - if ((e.key === key || e.key === key.toUpperCase()) && !e.metaKey && !e.ctrlKey && !e.altKey && !e.repeat) { - const target = e.target as HTMLElement; + if (e.isComposing) return; + const pressed = (e.key || "").toLowerCase(); + if (pressed === normalized && !e.metaKey && !e.ctrlKey && !e.altKey && !e.repeat) { + const target = e.target; - if ( - target.tagName === "INPUT" || - target.tagName === "TEXTAREA" || - target.isContentEditable || - target.closest('[contenteditable="true"]') - ) { - return; - } + if (target && target instanceof Element) { + const tag = (target as HTMLElement).tagName; + if ( + tag === "INPUT" || + tag === "TEXTAREA" || + (target as HTMLElement).isContentEditable || + target.closest('[contenteditable="true"]') + ) { + return; + } + } e.preventDefault(); - callback(); + cbRef.current(); } }; document.addEventListener("keydown", handleKeyDown); return () => document.removeEventListener("keydown", handleKeyDown); - }, [key, callback, enabled]); + }, [key, enabled]); }
11-16: Consider broadening the “typing context” guard (optional).If you expect rich editors that render as role="textbox" without contentEditable, or custom inputs (combobox, select), you could expand the guard to also skip
[role="textbox"],SELECT, and elements witharia-haspopup="listbox". Not required for this PR, but it’ll make the hook safer to reuse across the app.components/form/task.tsx (1)
21-21: Scope the shortcut to when the dialog is closed and use a stable callback.Prevents redundant re-subscriptions and avoids firing “N” while the create dialog is already open.
- useKeyboardShortcut("n", () => setIsCreating(true)); + const openCreate = useCallback(() => setIsCreating(true), []); + useKeyboardShortcut("n", openCreate, !isCreating);components/project/events/full-calendar.tsx (2)
80-81: Stabilize the callback (and optionally disable on mobile).
- Using a memoized handler avoids listener churn.
- If you prefer not to advertise/use keyboard shortcuts on small screens, you can gate with
!isMobile(optional).If both the calendar and the inline task form can be mounted together, pressing “N” will trigger both create flows. Confirm they don’t co-exist; if they do, gate one via the
enabledflag based on active context/route.- useKeyboardShortcut("n", () => setCreate(true)); + const openCreate = useCallback(() => setCreate(true), [setCreate]); + // If desired, disable on mobile keyboards: + useKeyboardShortcut("n", openCreate /*, !isMobile */);
237-243: Expose the shortcut to assistive technologies.Add
aria-keyshortcuts="N"to the New button so screen readers and ATs can announce the shortcut.- <Button + <Button + aria-keyshortcuts="N" className="w-full gap-2 md:w-auto" onClick={() => setCreate(true)} > <span>New</span> <Kbd className="hidden md:inline-flex">N</Kbd>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
components/form/task.tsx(2 hunks)components/project/events/full-calendar.tsx(4 hunks)hooks/use-keyboard-shortcut.ts(1 hunks)
🧰 Additional context used
🪛 Biome (2.1.2)
hooks/use-keyboard-shortcut.ts
[error] 3-3: This type annotation is trivially inferred from its initialization.
Safe fix: Remove the type annotation.
(lint/style/noInferrableTypes)
🔇 Additional comments (3)
hooks/use-keyboard-shortcut.ts (1)
3-27: Solid, reusable shortcut hook.Clean lifecycle management and sensible default guards (meta/ctrl/alt/repeat and text-entry contexts). Good foundation for app-wide shortcuts.
components/form/task.tsx (1)
9-9: LGTM: Using the shared hook improves consistency and reduces duplication.Moving to a common hook removes bespoke listeners and centralizes the input guards.
components/project/events/full-calendar.tsx (1)
5-6: Consistency and UX win.Importing and displaying the Kbd “N” hint alongside wiring the shortcut via the shared hook is a nice, discoverable pattern.
Also applies to: 35-36, 242-243
Summary by CodeRabbit