-
-
Notifications
You must be signed in to change notification settings - Fork 170
feat: Add Enter key shortcut to execute tools, resources, and prompts #906
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
WalkthroughKeyboard handling enhancements were implemented across multiple tab components in the application. Enter key listeners were added to PromptsTab, ResourcesTab, and ToolsTab to trigger their respective actions—fetching prompts, reading resources, and executing tools—when Enter is pressed. The ParametersPanel component received similar Enter key handling on form inputs and select elements. All handlers exclude input, textarea, and contenteditable elements from global key event processing, and respect loading states to prevent unintended triggers. Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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. 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.
Actionable comments posted: 0
🧹 Nitpick comments (3)
client/src/components/ResourcesTab.tsx (1)
118-138: Well-implemented Enter key handler with proper safeguards.The implementation correctly follows accessibility guidelines and prevents unintended triggers with comprehensive exclusions. The cleanup function prevents memory leaks as recommended.
Optional refinement for dependency completeness:
The effect calls
readResource, which accessesserverNamefrom the outer scope. While the current implementation works due to component lifecycle patterns (server changes resetselectedResource), wrappingreadResourceinuseCallbackwould make dependencies explicit and prevent potential stale closures.Example refactor with useCallback
+ const readResource = useCallback(async (uri: string) => { + if (!serverName) return; setLoading(true); setError(""); // ... rest of function - }; + }, [serverName]); // Handle Enter key to read resource globally useEffect(() => { const handleKeyDown = (e: KeyboardEvent) => { if (e.key === "Enter" && !e.shiftKey && selectedResource && !loading) { // ... exclusion checks e.preventDefault(); readResource(selectedResource); } }; window.addEventListener("keydown", handleKeyDown); return () => window.removeEventListener("keydown", handleKeyDown); - }, [selectedResource, loading]); + }, [selectedResource, loading, readResource]);client/src/components/ToolsTab.tsx (1)
332-353: Consistent Enter key implementation with proper accessibility considerations.The handler correctly excludes editable elements and respects loading state. Cleanup function ensures proper resource management. Based on learnings.
Consider extracting the duplicated global handler pattern:
This implementation is nearly identical to the handlers in ResourcesTab.tsx (lines 118-138) and PromptsTab.tsx (lines 187-206). Extracting into a custom hook would improve maintainability.
Example custom hook
function useEnterKeyAction( callback: () => void, enabled: boolean, loading: boolean ) { useEffect(() => { if (!enabled) return; const handleKeyDown = (e: KeyboardEvent) => { if (e.key === "Enter" && !e.shiftKey && !loading) { const target = e.target as HTMLElement; const tagName = target.tagName; const isEditable = target.isContentEditable; if (tagName === "INPUT" || tagName === "TEXTAREA" || isEditable) { return; } e.preventDefault(); callback(); } }; window.addEventListener("keydown", handleKeyDown); return () => window.removeEventListener("keydown", handleKeyDown); }, [callback, enabled, loading]); } // Usage in ToolsTab: useEnterKeyAction(executeTool, !!selectedTool, loading);client/src/components/PromptsTab.tsx (1)
187-206: Global handler implementation aligns with established pattern.The implementation matches the handlers in ResourcesTab.tsx and ToolsTab.tsx with correct safeguards. The local handler at line 179 takes precedence for input fields, while this global handler covers interactions elsewhere.
Same refactoring opportunities apply:
The dependency array consideration and custom hook extraction suggestion from ToolsTab.tsx (lines 332-353) apply equally here, consolidating the three identical global handler patterns across PromptsTab, ResourcesTab, and ToolsTab.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
client/src/components/PromptsTab.tsx(2 hunks)client/src/components/ResourcesTab.tsx(1 hunks)client/src/components/ToolsTab.tsx(1 hunks)client/src/components/tools/ParametersPanel.tsx(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
client/src/components/PromptsTab.tsx (1)
client/src/lib/mcp-prompts-api.ts (1)
getPrompt(32-54)
client/src/components/ToolsTab.tsx (1)
sdk/src/mcp-client-manager/index.ts (1)
executeTool(504-531)
🔍 Remote MCP
Summary of Key Findings for PR Review
I've gathered additional context on best practices for keyboard event handling in React and accessibility standards. Here are the key findings relevant to this PR:
React Keyboard Event Handling Best Practices
Use onKeyDown or onKeyUp instead of the deprecated onKeyPress for more consistent and inclusive user experience. The PR implementation using onKeyDown aligns with this recommendation.
Use event.key as it's meant to be a cross-platform compatible abstraction of keyboard keys, which appears to be properly used in the implementations.
Critical Event Listener Cleanup Requirement
The cleanup function prevents memory leaks by removing the event listener when the component unmounts or before the effect re-runs. This is important to verify — the PR's global Enter-key listeners in useEffect should include proper cleanup functions that call removeEventListener to avoid memory leaks.
Keeping event listeners around is expensive, taking up memory and processing power from the browser and host computer.
Accessibility Considerations (WCAG 2.1.4)
If a keyboard shortcut uses letter, punctuation, number, or symbol characters, at least one of the following must be true: the shortcut can be turned off, it can be remapped to involve one or more non-printable keyboard characters (like Ctrl or Alt), or it's only active when the component has focus.
The PR uses bare Enter key (a non-printable character exception), which does not affect components like listboxes and drop-down menus where the shortcuts are only active when the components have focus.
If keyboard shortcuts use more than one key (using "alt" or "ctrl" + another key), then no change is needed, though an explanation of each shortcut should be available to users. Since this PR uses only the Enter key in limited contexts, ensure it only activates on the intended components (not in text inputs where user typing shouldn't trigger actions).
Key Review Checkpoints
- Verify cleanup functions — All
useEffecthandlers must return cleanup functions removing the event listeners - Verify input/textarea exclusion — The PR mentions excluding inputs and textareas, confirming focus-only behavior per accessibility guidelines
- Dependency array correctness — Ensure dependencies like
selectedResource,selectedTool, andloadingare properly specified to avoid stale closures - Event prevention — Confirm
preventDefault()is called to avoid double-execution with form submissions
⏰ 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). (1)
- GitHub Check: prettier-and-build
🔇 Additional comments (2)
client/src/components/tools/ParametersPanel.tsx (1)
34-49: Clean implementation of per-field Enter key handling.Both handlers appropriately prevent default form submission and respect the loading state. The pattern ensures pressing Enter in form controls triggers execution, complementing the global handler in the parent ToolsTab component.
client/src/components/PromptsTab.tsx (1)
178-184: Appropriate local handler for input fields.The handler provides immediate Enter key response for parameter inputs, preventing default form behavior and triggering prompt retrieval when not loading.
matteo8p
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.
Ship!
Added keyboard shortcut support to execute tools, read resources, and get prompts by pressing Enter, improving workflow efficiency and enabling keyboard-first navigation.
Fix: #885