fix: prioritize filenames in cmd+p quick open (#12679)
* fix: show full paths in quick open results * refactor: use native file path tooltips * fix: position file path tooltips * refactor: share the cursor path tooltip with quick open Co-authored-by: Orca <help@stably.ai> * fix: let path tooltips run wider before wrapping Co-authored-by: Orca <help@stably.ai> --------- Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Neil
GitHub
parent
ead6b8e225
commit
f71373953b
|
|
@ -11,6 +11,7 @@ import {
|
|||
CommandEmpty,
|
||||
CommandItem
|
||||
} from '@/components/ui/command'
|
||||
import { FilePathCursorTooltip, splitTrailingSegment } from '@/components/file-path-cursor-tooltip'
|
||||
import { prepareQuickOpenFiles, rankQuickOpenFiles } from '@/components/quick-open-search'
|
||||
import { useRuntimeFileListForWorktree } from '@/components/quick-open-file-list'
|
||||
import { useModalReturnFocus } from '@/hooks/useModalReturnFocus'
|
||||
|
|
@ -146,9 +147,7 @@ export default function QuickOpen(): React.JSX.Element | null {
|
|||
</CommandEmpty>
|
||||
) : (
|
||||
filtered.map((item) => {
|
||||
const lastSlash = item.path.lastIndexOf('/')
|
||||
const dir = lastSlash >= 0 ? item.path.slice(0, lastSlash) : ''
|
||||
const filename = item.path.slice(lastSlash + 1)
|
||||
const { directory, filename } = splitTrailingSegment(item.path)
|
||||
const FileIcon = getFileTypeIcon(item.path)
|
||||
|
||||
return (
|
||||
|
|
@ -156,11 +155,24 @@ export default function QuickOpen(): React.JSX.Element | null {
|
|||
key={item.path}
|
||||
value={item.path}
|
||||
onSelect={() => handleSelect(item.path)}
|
||||
className="flex items-center gap-2 px-3 py-1.5"
|
||||
className="min-w-0 p-0"
|
||||
>
|
||||
<FileIcon className="size-3.5 text-muted-foreground flex-shrink-0" />
|
||||
<span className="truncate text-foreground">{filename}</span>
|
||||
{dir && <span className="truncate text-muted-foreground ml-1">{dir}</span>}
|
||||
{/* Why: the trigger is this inner element, not the CommandItem.
|
||||
cmdk sets its own onPointerMove after spreading props, which
|
||||
drops the one Radix needs to open the tooltip. */}
|
||||
<FilePathCursorTooltip path={item.path}>
|
||||
<div className="flex w-full min-w-0 items-center gap-2 px-3 py-1.5">
|
||||
<FileIcon className="size-3.5 shrink-0 text-muted-foreground" />
|
||||
{/* shrink-0 + max-w-full: the directory gives up all of its
|
||||
width before the filename loses a character. */}
|
||||
<span className="min-w-0 max-w-full shrink-0 truncate text-foreground">
|
||||
{filename}
|
||||
</span>
|
||||
{directory ? (
|
||||
<span className="min-w-0 truncate text-muted-foreground">{directory}</span>
|
||||
) : null}
|
||||
</div>
|
||||
</FilePathCursorTooltip>
|
||||
</CommandItem>
|
||||
)
|
||||
})
|
||||
|
|
|
|||
|
|
@ -0,0 +1,55 @@
|
|||
// @vitest-environment happy-dom
|
||||
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { cursorTooltipOffsets, splitTrailingSegment } from './file-path-cursor-tooltip'
|
||||
|
||||
describe('cursorTooltipOffsets', () => {
|
||||
const row = { bottom: 120, left: 400 }
|
||||
|
||||
it('places the tooltip under the cursor rather than the row', () => {
|
||||
// Row-anchored placement would be align 0; the cursor is 64px into the row.
|
||||
expect(cursorTooltipOffsets({ x: 464, y: 108 }, row)).toEqual({ align: 64, side: 6 })
|
||||
})
|
||||
|
||||
it('tracks the cursor across the row', () => {
|
||||
const left = cursorTooltipOffsets({ x: 410, y: 108 }, row)
|
||||
const right = cursorTooltipOffsets({ x: 610, y: 108 }, row)
|
||||
|
||||
expect(right.align - left.align).toBe(200)
|
||||
expect(right.side).toBe(left.side)
|
||||
})
|
||||
|
||||
it('stays anchored to the cursor when the row moves under it', () => {
|
||||
// The dropdown reflows while results stream in; re-measuring the row must
|
||||
// keep the tooltip on the cursor, not drag it along with the row.
|
||||
const before = cursorTooltipOffsets({ x: 464, y: 108 }, row)
|
||||
const after = cursorTooltipOffsets({ x: 464, y: 108 }, { bottom: 148, left: 576 })
|
||||
|
||||
expect(row.left + before.align).toBe(576 + after.align)
|
||||
expect(row.bottom + before.side).toBe(148 + after.side)
|
||||
})
|
||||
})
|
||||
|
||||
describe('splitTrailingSegment', () => {
|
||||
it('keeps the separator on the directory', () => {
|
||||
expect(splitTrailingSegment('app/src/SecondaryNav.tsx')).toEqual({
|
||||
directory: 'app/src/',
|
||||
filename: 'SecondaryNav.tsx'
|
||||
})
|
||||
})
|
||||
|
||||
it('does not duplicate the root separator', () => {
|
||||
expect(splitTrailingSegment('/foo')).toEqual({ directory: '/', filename: 'foo' })
|
||||
})
|
||||
|
||||
it('preserves Windows separators', () => {
|
||||
expect(splitTrailingSegment('C:\\repo\\src\\a.ts')).toEqual({
|
||||
directory: 'C:\\repo\\src\\',
|
||||
filename: 'a.ts'
|
||||
})
|
||||
})
|
||||
|
||||
it('returns no directory for a bare filename', () => {
|
||||
expect(splitTrailingSegment('README.md')).toEqual({ directory: '', filename: 'README.md' })
|
||||
})
|
||||
})
|
||||
|
|
@ -0,0 +1,100 @@
|
|||
import React from 'react'
|
||||
import { Slot } from 'radix-ui'
|
||||
import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip'
|
||||
|
||||
// Clears the pointer without putting the label under the cursor's own hotspot.
|
||||
const CURSOR_TOOLTIP_GAP = 18
|
||||
|
||||
/**
|
||||
* Radix positions the tooltip against the trigger, so a cursor position has to
|
||||
* be restated as offsets from the trigger's bottom-left corner to land under
|
||||
* the pointer.
|
||||
*/
|
||||
export function cursorTooltipOffsets(
|
||||
pointer: { x: number; y: number },
|
||||
trigger: { bottom: number; left: number }
|
||||
): { align: number; side: number } {
|
||||
return {
|
||||
align: pointer.x - trigger.left,
|
||||
side: pointer.y + CURSOR_TOOLTIP_GAP - trigger.bottom
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Splits a path for filename-first display, keeping the separator attached to
|
||||
* the directory so `/foo` renders `foo` + `/` and Windows paths keep `\`.
|
||||
*/
|
||||
export function splitTrailingSegment(path: string): { directory: string; filename: string } {
|
||||
const separatorIndex = Math.max(path.lastIndexOf('/'), path.lastIndexOf('\\'))
|
||||
|
||||
return separatorIndex === -1
|
||||
? { directory: '', filename: path }
|
||||
: { directory: path.slice(0, separatorIndex + 1), filename: path.slice(separatorIndex + 1) }
|
||||
}
|
||||
|
||||
/**
|
||||
* Wraps a single result row so its full path appears in a system-styled tooltip
|
||||
* anchored under the cursor. The child is the trigger; it receives a ref and a
|
||||
* pointer-move handler.
|
||||
*/
|
||||
export function FilePathCursorTooltip({
|
||||
children,
|
||||
path
|
||||
}: {
|
||||
children: React.ReactNode
|
||||
path: string
|
||||
}): React.JSX.Element {
|
||||
const triggerRef = React.useRef<HTMLElement>(null)
|
||||
const pointerRef = React.useRef<{ x: number; y: number } | null>(null)
|
||||
const [open, setOpen] = React.useState(false)
|
||||
const [offset, setOffset] = React.useState({ align: 0, side: 0 })
|
||||
|
||||
// Why: the offsets are only valid for the trigger's position at the moment
|
||||
// Radix positions it. Results stream in and move rows while the open delay
|
||||
// runs, so re-measure on open rather than trusting the pointer-move rect.
|
||||
React.useLayoutEffect(() => {
|
||||
const rect = triggerRef.current?.getBoundingClientRect()
|
||||
const pointer = pointerRef.current
|
||||
if (!open || !rect || !pointer) {
|
||||
return
|
||||
}
|
||||
const next = cursorTooltipOffsets(pointer, rect)
|
||||
setOffset((current) =>
|
||||
current.align === next.align && current.side === next.side ? current : next
|
||||
)
|
||||
}, [open])
|
||||
|
||||
return (
|
||||
<Tooltip open={open} onOpenChange={setOpen}>
|
||||
<TooltipTrigger asChild>
|
||||
<Slot.Root
|
||||
ref={triggerRef}
|
||||
// Why: a ref, not state — read once when the tooltip opens, so
|
||||
// re-rendering per pointer move would be churn for nothing.
|
||||
// pointermove (not mousemove) because Radix opens off pointermove and
|
||||
// Slot runs the child handler first, keeping this fresh on instant open.
|
||||
onPointerMove={(event: React.PointerEvent) => {
|
||||
if (open) {
|
||||
return
|
||||
}
|
||||
pointerRef.current = { x: event.clientX, y: event.clientY }
|
||||
}}
|
||||
>
|
||||
{children}
|
||||
</Slot.Root>
|
||||
</TooltipTrigger>
|
||||
{/* Anchored under the cursor the way a system tooltip is, rather than to
|
||||
the row. Collision shifting still applies near the viewport edge. */}
|
||||
<TooltipContent
|
||||
side="bottom"
|
||||
align="start"
|
||||
sideOffset={offset.side}
|
||||
alignOffset={offset.align}
|
||||
showArrow={false}
|
||||
className="max-w-[min(90vw,800px)] rounded-md border border-border/80 bg-popover px-2 py-1 text-[11px] leading-[15px] break-words text-popover-foreground shadow-[0_10px_24px_rgba(0,0,0,0.18)]"
|
||||
>
|
||||
{path}
|
||||
</TooltipContent>
|
||||
</Tooltip>
|
||||
)
|
||||
}
|
||||
|
|
@ -5,7 +5,7 @@ import { createRoot, type Root } from 'react-dom/client'
|
|||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { TooltipProvider } from '@/components/ui/tooltip'
|
||||
import type { ActiveOption } from './tab-create-entry-active-option'
|
||||
import { cursorTooltipOffsets, EntryActionRow } from './TabBarCreateEntryRow'
|
||||
import { EntryActionRow } from './TabBarCreateEntryRow'
|
||||
|
||||
;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
|
||||
|
||||
|
|
@ -89,30 +89,3 @@ describe('EntryActionRow', () => {
|
|||
expect(container.textContent).not.toContain('/')
|
||||
})
|
||||
})
|
||||
|
||||
describe('cursorTooltipOffsets', () => {
|
||||
const row = { bottom: 120, left: 400 }
|
||||
|
||||
it('places the tooltip under the cursor rather than the row', () => {
|
||||
// Row-anchored placement would be align 0; the cursor is 64px into the row.
|
||||
expect(cursorTooltipOffsets({ x: 464, y: 108 }, row)).toEqual({ align: 64, side: 6 })
|
||||
})
|
||||
|
||||
it('tracks the cursor across the row', () => {
|
||||
const left = cursorTooltipOffsets({ x: 410, y: 108 }, row)
|
||||
const right = cursorTooltipOffsets({ x: 610, y: 108 }, row)
|
||||
|
||||
expect(right.align - left.align).toBe(200)
|
||||
expect(right.side).toBe(left.side)
|
||||
})
|
||||
|
||||
it('stays anchored to the cursor when the row moves under it', () => {
|
||||
// The dropdown reflows while results stream in; re-measuring the row must
|
||||
// keep the tooltip on the cursor, not drag it along with the row.
|
||||
const before = cursorTooltipOffsets({ x: 464, y: 108 }, row)
|
||||
const after = cursorTooltipOffsets({ x: 464, y: 108 }, { bottom: 148, left: 576 })
|
||||
|
||||
expect(row.left + before.align).toBe(576 + after.align)
|
||||
expect(row.bottom + before.side).toBe(148 + after.side)
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -2,29 +2,12 @@ import React from 'react'
|
|||
import { FilePlus, FileText, Globe, Loader2, Smartphone, TerminalSquare } from 'lucide-react'
|
||||
import { AgentIcon } from '@/lib/agent-catalog'
|
||||
import { cn } from '@/lib/utils'
|
||||
import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip'
|
||||
import { FilePathCursorTooltip, splitTrailingSegment } from '@/components/file-path-cursor-tooltip'
|
||||
import { translate } from '@/i18n/i18n'
|
||||
import type { ActiveOption } from './tab-create-entry-active-option'
|
||||
|
||||
export const RESULT_LISTBOX_ID = 'tab-create-entry-results'
|
||||
|
||||
// Clears the macOS pointer without putting the label under the cursor's own hotspot.
|
||||
const CURSOR_TOOLTIP_GAP = 18
|
||||
|
||||
/**
|
||||
* Radix positions the tooltip against the row, so a cursor position has to be
|
||||
* restated as offsets from the row's bottom-left corner to land under the pointer.
|
||||
*/
|
||||
export function cursorTooltipOffsets(
|
||||
pointer: { x: number; y: number },
|
||||
row: { bottom: number; left: number }
|
||||
): { align: number; side: number } {
|
||||
return {
|
||||
align: pointer.x - row.left,
|
||||
side: pointer.y + CURSOR_TOOLTIP_GAP - row.bottom
|
||||
}
|
||||
}
|
||||
|
||||
// Index-based (not the option id, which may contain spaces/slashes from file
|
||||
// paths) so it is always a valid aria-activedescendant IDREF.
|
||||
export function resultOptionDomId(index: number): string {
|
||||
|
|
@ -58,43 +41,13 @@ export function EntryActionRow({
|
|||
selected: boolean
|
||||
}): React.JSX.Element {
|
||||
const presentation = getActionPresentation(option)
|
||||
const rowRef = React.useRef<HTMLButtonElement>(null)
|
||||
const pointerRef = React.useRef<{ x: number; y: number } | null>(null)
|
||||
const [tooltipOpen, setTooltipOpen] = React.useState(false)
|
||||
const [pointerOffset, setPointerOffset] = React.useState({ align: 0, side: 0 })
|
||||
|
||||
// Why: Radix positions against the row, so the offsets are only valid for the
|
||||
// row's position at the moment it positions. Results stream in while the open
|
||||
// delay runs, so re-measure on open rather than trusting the pointer-move rect.
|
||||
React.useLayoutEffect(() => {
|
||||
const rect = rowRef.current?.getBoundingClientRect()
|
||||
const pointer = pointerRef.current
|
||||
if (!tooltipOpen || !rect || !pointer) {
|
||||
return
|
||||
}
|
||||
const next = cursorTooltipOffsets(pointer, rect)
|
||||
setPointerOffset((current) =>
|
||||
current.align === next.align && current.side === next.side ? current : next
|
||||
)
|
||||
}, [tooltipOpen])
|
||||
|
||||
const row = (
|
||||
<button
|
||||
ref={rowRef}
|
||||
type="button"
|
||||
id={id}
|
||||
role="option"
|
||||
aria-selected={selected}
|
||||
// Why: a ref, not state — this is read once when the tooltip opens, so
|
||||
// re-rendering the row on every pointer move would be churn for nothing.
|
||||
// pointermove (not mousemove) because Radix opens off pointermove and Slot
|
||||
// runs the child handler first, keeping this fresh even on instant open.
|
||||
onPointerMove={(event) => {
|
||||
if (tooltipOpen) {
|
||||
return
|
||||
}
|
||||
pointerRef.current = { x: event.clientX, y: event.clientY }
|
||||
}}
|
||||
className={cn(
|
||||
'flex h-6 w-full items-center gap-1.5 rounded-[7px] px-1 text-left text-[11px] leading-5 outline-none',
|
||||
selected
|
||||
|
|
@ -128,35 +81,7 @@ export function EntryActionRow({
|
|||
return row
|
||||
}
|
||||
|
||||
return (
|
||||
<Tooltip open={tooltipOpen} onOpenChange={setTooltipOpen}>
|
||||
<TooltipTrigger asChild>{row}</TooltipTrigger>
|
||||
{/* Anchored under the cursor the way a system tooltip is, rather than to
|
||||
the row. Radix anchors to the trigger, so the cursor is expressed as
|
||||
offsets off the row's bottom-left corner; collision flipping still
|
||||
applies near the viewport edge. */}
|
||||
<TooltipContent
|
||||
side="bottom"
|
||||
align="start"
|
||||
sideOffset={pointerOffset.side}
|
||||
alignOffset={pointerOffset.align}
|
||||
showArrow={false}
|
||||
className="max-w-[420px] rounded-md border border-border/80 bg-popover px-2 py-1 text-[11px] leading-[15px] break-words text-popover-foreground shadow-[0_10px_24px_rgba(0,0,0,0.18)]"
|
||||
>
|
||||
{presentation.detail}
|
||||
</TooltipContent>
|
||||
</Tooltip>
|
||||
)
|
||||
}
|
||||
|
||||
// Why: keeps the separator attached to the directory, so `/foo` renders as
|
||||
// `foo` + `/` rather than re-deriving a separator that may not match the path.
|
||||
function splitTrailingSegment(path: string): { directory: string; filename: string } {
|
||||
const separatorIndex = Math.max(path.lastIndexOf('/'), path.lastIndexOf('\\'))
|
||||
|
||||
return separatorIndex === -1
|
||||
? { directory: '', filename: path }
|
||||
: { directory: path.slice(0, separatorIndex + 1), filename: path.slice(separatorIndex + 1) }
|
||||
return <FilePathCursorTooltip path={presentation.detail}>{row}</FilePathCursorTooltip>
|
||||
}
|
||||
|
||||
function FilenameFirstPath({ path }: { path: string }): React.JSX.Element {
|
||||
|
|
|
|||
|
|
@ -205,9 +205,14 @@ function CommandGroup({
|
|||
)
|
||||
}
|
||||
|
||||
function CommandItem({ className, ...props }: React.ComponentProps<typeof CommandPrimitive.Item>) {
|
||||
function CommandItem({
|
||||
className,
|
||||
ref,
|
||||
...props
|
||||
}: React.ComponentProps<typeof CommandPrimitive.Item>) {
|
||||
return (
|
||||
<CommandPrimitive.Item
|
||||
ref={ref}
|
||||
data-slot="command-item"
|
||||
className={cn(
|
||||
'relative flex cursor-default gap-2 select-none items-center rounded-sm px-2 py-1.5 text-sm outline-none data-[selected=true]:bg-accent data-[selected=true]:text-accent-foreground data-[disabled=true]:pointer-events-none data-[disabled=true]:opacity-50 [&_svg]:pointer-events-none [&_svg]:shrink-0 [&_svg:not([class*="size-"])]:size-4',
|
||||
|
|
|
|||
|
|
@ -0,0 +1,57 @@
|
|||
import { mkdirSync, writeFileSync } from 'node:fs'
|
||||
import path from 'node:path'
|
||||
import { expect, test } from './helpers/orca-app'
|
||||
import { ensureTerminalVisible, waitForActiveWorktree, waitForSessionReady } from './helpers/store'
|
||||
|
||||
const relativeFilePath =
|
||||
'packages/orca/src/renderer/src/components/navigation/worktree/quick-open/long-path-fixtures/very-deeply-nested-folder/QuickOpenTarget.tsx'
|
||||
|
||||
test('cmd+p quick open prioritizes the filename and reveals the full path on hover', async ({
|
||||
electronApp,
|
||||
orcaPage,
|
||||
testRepoPath
|
||||
}) => {
|
||||
const filePath = path.join(testRepoPath, ...relativeFilePath.split('/'))
|
||||
mkdirSync(path.dirname(filePath), { recursive: true })
|
||||
writeFileSync(filePath, 'export const QuickOpenTarget = true\n')
|
||||
|
||||
await waitForSessionReady(orcaPage)
|
||||
await waitForActiveWorktree(orcaPage)
|
||||
await ensureTerminalVisible(orcaPage)
|
||||
|
||||
// Headless Playwright keyboard events bypass Electron’s before-input-event shortcut path.
|
||||
await electronApp.evaluate(({ BrowserWindow }) => {
|
||||
BrowserWindow.getAllWindows()[0]?.webContents.send('ui:openQuickOpen')
|
||||
})
|
||||
const dialog = orcaPage.getByRole('dialog', { name: 'Go to file' })
|
||||
await expect(dialog).toBeVisible()
|
||||
const input = dialog.locator('input[placeholder="Go to file..."]')
|
||||
await input.fill('QuickOpenTarget')
|
||||
|
||||
const row = dialog.getByRole('option').filter({ hasText: 'QuickOpenTarget.tsx' }).first()
|
||||
await expect(row).toBeVisible()
|
||||
await expect(row).toContainText('packages/orca/src/renderer/src/components/navigation/')
|
||||
const rowText = await row.textContent()
|
||||
expect(rowText?.indexOf('QuickOpenTarget.tsx')).toBeLessThan(
|
||||
rowText?.indexOf('packages/orca/src/renderer/src/components/navigation/') ?? -1
|
||||
)
|
||||
|
||||
// Two hovers on purpose: results stream in and remount the row, and Radix only
|
||||
// opens on a pointermove it actually receives. A single hover can land before
|
||||
// the remount and leave the cursor sitting still over a row that never saw it.
|
||||
await row.hover({ position: { x: 20, y: 12 } })
|
||||
await orcaPage.waitForTimeout(250)
|
||||
await row.hover({ position: { x: 40, y: 12 } })
|
||||
|
||||
// Exact cursor placement is arithmetic, unit-tested via cursorTooltipOffsets.
|
||||
// Asserting it here measures the app mid-reflow and is flaky; what E2E is
|
||||
// uniquely good for is that the tooltip really opens with the whole path.
|
||||
await expect(
|
||||
orcaPage.locator('[data-slot="tooltip-content"]').filter({ hasText: relativeFilePath })
|
||||
).toBeVisible()
|
||||
|
||||
const proofPath = process.env.ORCA_QUICK_OPEN_PROOF_PATH
|
||||
if (proofPath) {
|
||||
await orcaPage.screenshot({ path: proofPath })
|
||||
}
|
||||
})
|
||||
Loading…
Reference in New Issue