fix: allow command palette search results to open in new tab #8000
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR enables Command+Click / Ctrl+Click functionality on command palette search results to open entries in a new tab, while preserving the existing behavior for normal clicks.
Key Changes:
- Added meta/ctrl+click detection to intercept new tab requests
- Extracted target path calculation for reuse in both click handlers
- Implemented onMouseDown handler to open results in new tab with appropriate security attributes
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if (!event.metaKey && !event.ctrlKey) return; | ||
|
|
||
| event.preventDefault(); | ||
| event.stopPropagation(); | ||
|
|
||
| closePalette(); | ||
| window.open(targetPath, "_blank", "noopener,noreferrer"); |
There was a problem hiding this comment.
The early return on line 48 doesn't prevent the default behavior. When neither metaKey nor ctrlKey is pressed, the function returns but onMouseDown still fires, which can interfere with the normal onSelect flow. Consider calling event.preventDefault() before the early return, or restructure to only call preventDefault/stopPropagation when meta/ctrl keys are detected.
| if (!event.metaKey && !event.ctrlKey) return; | |
| event.preventDefault(); | |
| event.stopPropagation(); | |
| closePalette(); | |
| window.open(targetPath, "_blank", "noopener,noreferrer"); | |
| if (event.metaKey || event.ctrlKey) { | |
| event.preventDefault(); | |
| event.stopPropagation(); | |
| closePalette(); | |
| window.open(targetPath, "_blank", "noopener,noreferrer"); | |
| } |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughRefactored click handling in the search results component to support meta/ctrl-click for opening items in new tabs. The targetPath is now computed once upfront and reused across regular navigation and meta-click flows, with added handleMetaClick to detect modifier keys and open paths in new windows. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/web/core/components/command-palette/actions/search-results.tsx (1)
47-65: Consider adding keyboard and middle-click support for new-tab behavior.Currently, only meta/ctrl+click opens items in a new tab. Users navigating via keyboard might expect Cmd/Ctrl+Enter to work similarly, and many users rely on middle-click (mouse button 2) as the standard way to open links in new tabs.
Add keyboard support:
Check for meta/ctrl in the existing
onSelect:onSelect={() => { + // Note: check if cmdk provides the keyboard event; if not, track key state separately + if (/* event.metaKey || event.ctrlKey */) { + closePalette(); + window.open(targetPath, "_blank", "noopener,noreferrer"); + return; + } closePalette(); router.push(targetPath); if (itemProjectId) openProjectAndScrollToSidebar(itemProjectId); }}Add middle-click support:
+const handleAuxClick = (event: MouseEvent<HTMLDivElement>) => { + if (event.button === 1) { // middle mouse button + event.preventDefault(); + closePalette(); + window.open(targetPath, "_blank", "noopener,noreferrer"); + } +}; <Command.Item ... onMouseDown={handleMetaClick} + onAuxClick={handleAuxClick} onSelect={...} >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/core/components/command-palette/actions/search-results.tsx(2 hunks)
⏰ 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). (2)
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
apps/web/core/components/command-palette/actions/search-results.tsx (2)
6-6: LGTM!The MouseEvent type import is correctly added to support typing for the new meta-click handler.
39-45: Good refactoring to compute path and project ID once.Computing
targetPathanditemProjectIdupfront and reusing them in both the meta-click handler andonSelecteliminates duplicate logic and improves maintainability.
| const handleMetaClick = (event: MouseEvent<HTMLDivElement>) => { | ||
| if (!event.metaKey && !event.ctrlKey) return; | ||
|
|
||
| event.preventDefault(); | ||
| event.stopPropagation(); | ||
|
|
||
| closePalette(); | ||
| window.open(targetPath, "_blank", "noopener,noreferrer"); | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify that onMouseDown preventDefault prevents onSelect from firing.
The preventDefault() and stopPropagation() calls in onMouseDown may not prevent the subsequent click event and onSelect from firing—standard DOM behavior allows click events to fire even after mousedown is prevented. This could result in both the new-tab open and the router.push executing when meta/ctrl+click is used.
Test both scenarios to confirm the behavior:
- Normal click: should only trigger
onSelect(navigate in same tab) - Meta/Ctrl+click: should only trigger
handleMetaClick(open in new tab), notonSelect
If both handlers fire on meta+click, consider these alternatives:
Option 1: Use onClick instead of onMouseDown and check for default prevented
const handleClick = (event: MouseEvent<HTMLDivElement>) => {
if (event.metaKey || event.ctrlKey) {
event.preventDefault();
event.stopPropagation();
closePalette();
window.open(targetPath, "_blank", "noopener,noreferrer");
}
};Then set onClick={handleClick} and let onSelect check if default was prevented.
Option 2: Track meta/ctrl state and check in onSelect
let isMetaClick = false;
const handleMetaClick = (event: MouseEvent<HTMLDivElement>) => {
if (event.metaKey || event.ctrlKey) {
isMetaClick = true;
event.preventDefault();
event.stopPropagation();
closePalette();
window.open(targetPath, "_blank", "noopener,noreferrer");
}
};
// In onSelect:
onSelect={() => {
if (isMetaClick) {
isMetaClick = false;
return;
}
// existing logic...
}}Option 3: Move meta/ctrl check into onSelect
onSelect={(event?: any) => {
closePalette();
if (event?.metaKey || event?.ctrlKey) {
window.open(targetPath, "_blank", "noopener,noreferrer");
} else {
router.push(targetPath);
if (itemProjectId) openProjectAndScrollToSidebar(itemProjectId);
}
}}(Check if cmdk passes the original event to onSelect)
---
🏁 Script executed:
```shell
#!/bin/bash
# Inspect the target file to see full context and component structure
cat apps/web/core/components/command-palette/actions/search-results.tsx
Length of output: 2942
🏁 Script executed:
#!/bin/bash
# Search for onSelect handlers and how they interact with mouse events
rg -nP -C5 '\bonSelect\b' apps/web/core/components/command-palette/ --type=ts --type=tsxLength of output: 27
🏁 Script executed:
#!/bin/bash
# Check how cmdk is used in the codebase to understand event handling
rg -nP 'import.*cmdk|from.*cmdk' apps/web --type=ts --type=tsx | head -20Length of output: 27
🏁 Script executed:
#!/bin/bash
# Search for other examples of meta/ctrl click handling in the codebase
rg -nP '\b(metaKey|ctrlKey)\b.*\bonMouseDown' apps/web --type=ts --type=tsx -B2 -A5Length of output: 27
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for cmdk imports to find how it's being used
rg -n 'import.*cmdk|from.*cmdk' apps/web --type=ts
Length of output: 1081
🏁 Script executed:
#!/bin/bash
# Search for other meta/ctrl click handling patterns in codebase
rg -n 'metaKey|ctrlKey' apps/web --type=ts -B2 -A2 | head -40Length of output: 3607
🏁 Script executed:
#!/bin/bash
# Look for Command.Item usage to understand cmdk patterns
rg -n 'Command\.Item' apps/web --type=ts -B1 -A5 | head -60Length of output: 6591
🏁 Script executed:
#!/bin/bash
# Check package.json for cmdk version to understand its API
rg -n '"cmdk"' . -B1 -A1Length of output: 3649
---
🌐 Web query:
```web
cmdk Command.Item onSelect event handling mousedown click
💡 Result:
Short answer
- cmdk's Command.Item uses its onSelect callback as the selection API (it intentionally triggers selection on pointer down / activation rather than waiting for native click). Use onSelect for command handling. [1]
Why this matters
- Stopping or changing mousedown/click ordering can be browser-dependent (calling preventDefault() on mousedown affects default actions and :active behavior in some browsers — notably Firefox). That can change whether click or focus behaviors run. See MDN on preventDefault and a writeup about Firefox mousedown behavior. [2][3]
Practical recommendations
- Handle selection with Command.Item's onSelect — that's the supported, cross-browser hook. [1]
- Avoid depending on the native click event for command behavior inside cmdk; relying on onSelect prevents race/ordering bugs. [1]
- If you must intercept pointer interactions (for example to prevent focus styling), prefer pointer events (pointerdown/pointerup) or manage focus programmatically — and be careful with event.preventDefault() on mousedown because it can change browser behavior (especially Firefox). Test in target browsers. [2][3]
- If you need to stop cmdk selection for a specific item, use disabled/aria-disabled or conditionally render the item rather than preventing events. [1]
If you want, paste the minimal code you’re using and I’ll show the exact changes (e.g., moving logic into onSelect or using pointerdown with guarded preventDefault).
Sources
[1] cmdk README / Command.Item docs.
[2] MDN: Event.preventDefault().
[3] Article: Firefox behavior when using preventDefault() on mousedown.
[4] StackOverflow: preventDefault and mousedown/click interaction.
Move meta+click handling into onSelect to prevent both handlers from executing.
The current implementation has a critical bug: cmdk's Command.Item triggers onSelect on pointer down activation, not on click. When a user meta+clicks, both handleMetaClick (on onMouseDown) and onSelect execute sequentially, causing both window.open() and router.push() to run. The preventDefault() on mousedown does not prevent the subsequent onSelect firing.
Move the meta+click logic into onSelect to consolidate routing decisions into a single handler:
onSelect={(event?: any) => {
closePalette();
if (event?.metaKey || event?.ctrlKey) {
window.open(targetPath, "_blank", "noopener,noreferrer");
} else {
router.push(targetPath);
if (itemProjectId) openProjectAndScrollToSidebar(itemProjectId);
}
}}Remove the onMouseDown={handleMetaClick} handler. If cmdk doesn't pass the original MouseEvent to onSelect, track the modifier keys in a ref or state variable during onMouseDown and check it in onSelect.
🤖 Prompt for AI Agents
In apps/web/core/components/command-palette/actions/search-results.tsx around
lines 47 to 55, the meta+click logic currently lives in an onMouseDown handler
which does not prevent cmdk's onSelect from also firing; move the meta/ctrl
modifier handling into the onSelect handler so routing decisions happen once:
call closePalette(), then if metaKey or ctrlKey open the target path in a new
tab via window.open(...), else call router.push(targetPath) and call
openProjectAndScrollToSidebar(itemProjectId) when applicable; remove the
onMouseDown={handleMetaClick} binding entirely; if cmdk’s onSelect does not
receive the original MouseEvent, record modifier keys into a ref/state on mouse
down and read that ref inside onSelect to decide between window.open and
router.push.
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_68fa0caa8aec832a9b2eb5bb478c09eb
Summary by CodeRabbit