-
Notifications
You must be signed in to change notification settings - Fork 2.9k
fix: allow command palette search results to open in new tab #8000
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
base: preview
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ | |
| import { Command } from "cmdk"; | ||
| import { observer } from "mobx-react"; | ||
| import { useParams } from "next/navigation"; | ||
| import type { MouseEvent } from "react"; | ||
| // plane imports | ||
| import type { IWorkspaceSearchResults } from "@plane/types"; | ||
| // hooks | ||
|
|
@@ -35,28 +36,43 @@ export const CommandPaletteSearchResults: React.FC<Props> = observer((props) => | |
| if (section.length > 0) { | ||
| return ( | ||
| <Command.Group key={key} heading={`${currentSection.title} search`}> | ||
| {section.map((item: any) => ( | ||
| <Command.Item | ||
| key={item.id} | ||
| onSelect={() => { | ||
| closePalette(); | ||
| router.push(currentSection.path(item, projectId)); | ||
| const itemProjectId = | ||
| item?.project_id || | ||
| (Array.isArray(item?.project_ids) && item?.project_ids?.length > 0 | ||
| ? item?.project_ids[0] | ||
| : undefined); | ||
| if (itemProjectId) openProjectAndScrollToSidebar(itemProjectId); | ||
| }} | ||
| value={`${key}-${item?.id}-${item.name}-${item.project__identifier ?? ""}-${item.sequence_id ?? ""}`} | ||
| className="focus:outline-none" | ||
| > | ||
| <div className="flex items-center gap-2 overflow-hidden text-custom-text-200"> | ||
| {currentSection.icon} | ||
| <p className="block flex-1 truncate">{currentSection.itemName(item)}</p> | ||
| </div> | ||
| </Command.Item> | ||
| ))} | ||
| {section.map((item: any) => { | ||
| const targetPath = currentSection.path(item, projectId); | ||
| const itemProjectId = | ||
| item?.project_id || | ||
| (Array.isArray(item?.project_ids) && item?.project_ids?.length > 0 | ||
| ? item?.project_ids[0] | ||
| : undefined); | ||
|
|
||
| const handleMetaClick = (event: MouseEvent<HTMLDivElement>) => { | ||
| if (!event.metaKey && !event.ctrlKey) return; | ||
|
|
||
| event.preventDefault(); | ||
| event.stopPropagation(); | ||
|
|
||
| closePalette(); | ||
| window.open(targetPath, "_blank", "noopener,noreferrer"); | ||
| }; | ||
|
Comment on lines
+47
to
+55
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainVerify that onMouseDown preventDefault prevents onSelect from firing. The Test both scenarios to confirm the behavior:
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 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) 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 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 💡 Result: Short answer
Why this matters
Practical recommendations
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 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 Move the meta+click logic into 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 🤖 Prompt for AI Agents |
||
|
|
||
| return ( | ||
| <Command.Item | ||
| key={item.id} | ||
| onMouseDown={handleMetaClick} | ||
| onSelect={() => { | ||
| closePalette(); | ||
| router.push(targetPath); | ||
| if (itemProjectId) openProjectAndScrollToSidebar(itemProjectId); | ||
| }} | ||
| value={`${key}-${item?.id}-${item.name}-${item.project__identifier ?? ""}-${item.sequence_id ?? ""}`} | ||
| className="focus:outline-none" | ||
| > | ||
| <div className="flex items-center gap-2 overflow-hidden text-custom-text-200"> | ||
| {currentSection.icon} | ||
| <p className="block flex-1 truncate">{currentSection.itemName(item)}</p> | ||
| </div> | ||
| </Command.Item> | ||
| ); | ||
| })} | ||
| </Command.Group> | ||
| ); | ||
| } | ||
|
|
||
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.
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.