mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 08:02:28 +00:00
Refactor editor header file rename to breadcrumb morph UI (#21265)
* Refactor editor header file rename to a breadcrumb morph UI - Display full breadcrumb (repo name + parent dirs) during rename for context - Separate basename field from extension suffix to clarify what users edit - Replace blur-to-commit with explicit confirm/cancel buttons - Auto-attach extension to basename; respect explicitly typed extensions - Add comprehensive tests for rename scenarios and edge cases * Fix markdown rename: drop blur-commit, handle IME, track active file - Blur no longer commits, preventing accidental renames when focus moves - IME composition keys properly handled for CJK input support - File switches during rename now cancel the operation - Extension display improved to show final filename - Comprehensive test coverage added for edge cases * Simplify markdown rename to inline field without breadcrumb or buttons Replaces breadcrumb-morph rename with confirm/cancel buttons with a simple inline field accepting full filenames. Commits on Enter, blur, or Escape to cancel, matching tab bar and file explorer behavior. Adds renameCancelledRef to prevent blur-commit after Escape. Removes unused i18n strings for buttons and simplifies state by dropping extension pinning and breadcrumb display. * Handle blur race when file changes during rename When switching files mid-rename, React may deliver the old input's blur event after the new file renders, causing a stale rename commit. Mark the rename as cancelled when the active file changes, and add test coverage verifying stale blur events are ignored. * Test blur-race condition in hook unit test Move blur-commit-after-file-change test from EditorPanelHeaderPath integration tests to useEditorHeaderFileRename unit test. Tests the blur-handling logic at the hook level where it belongs.
This commit is contained in:
@@ -72,7 +72,7 @@ Orca targets macOS, Linux, and Windows. Keep all platform-dependent behavior beh
|
||||
- **Keyboard shortcuts**: Never hardcode `e.metaKey`. Use a platform check (`navigator.userAgent.includes('Mac')`) to pick `metaKey` on Mac and `ctrlKey` on Linux/Windows. Electron menu accelerators should use `CmdOrCtrl`.
|
||||
- **Shortcut labels in UI**: Display `⌘` / `⇧` on Mac and `Ctrl+` / `Shift+` on other platforms.
|
||||
- **File paths**: Use `path.join` or Electron/Node path utilities — never assume `/` or `\`.
|
||||
- **Windows terminal shells**: `--shell` picks the shell a terminal *is*; `--command` is typed into whatever shell the host spawned, so a shell choice routed through `command` silently becomes a child process. See [`docs/reference/windows-terminal-shell-selection.md`](./docs/reference/windows-terminal-shell-selection.md).
|
||||
- **Windows terminal shells**: `--shell` picks the shell a terminal _is_; `--command` is typed into whatever shell the host spawned, so a shell choice routed through `command` silently becomes a child process. See [`docs/reference/windows-terminal-shell-selection.md`](./docs/reference/windows-terminal-shell-selection.md).
|
||||
- **Windows setup scripts**: the setup/issue-command runner is a `.cmd` batch file unless the script starts with a `#!` line — never derive that from the user's terminal-shell preference, and never launch a `.cmd` runner with a bare `cmd.exe /c` from a Git Bash pane (MSYS rewrites the `/c`). See [`docs/reference/windows-setup-shell.md`](./docs/reference/windows-setup-shell.md).
|
||||
- **Windows child processes**: start them through `runProcess`/`spawnProcess` in `src/shared/child-process/` — never `child_process` directly. It pins `windowsHide`, refuses `shell: true`, and encodes `.cmd`/`.bat` arguments so neither `CommandLineToArgvW` nor `cmd.exe` mangles them. A ratchet test fails on any new direct import. Recognised npm/pnpm `.cmd` shims are resolved to their real target so the spawn skips `cmd.exe` entirely; see [`docs/reference/windows-cmd-shim-resolution.md`](./docs/reference/windows-cmd-shim-resolution.md) before adding a shim shape or debugging one.
|
||||
- **Windows process enumeration**: read the table through `src/main/windows/windows-process-table.ts`, never by forking `powershell.exe`. See [`docs/reference/windows-process-enumeration.md`](./docs/reference/windows-process-enumeration.md).
|
||||
|
||||
@@ -102,7 +102,7 @@ gate, and a conflicted worktree that looks clean is granted a hosted-review crea
|
||||
should not have. Withholding an affordance is a degrade; removing the evidence a gate
|
||||
reads is not.
|
||||
|
||||
A fallback is only ever allowed to shape a *reading*. If the member is sent back to the
|
||||
A fallback is only ever allowed to shape a _reading_. If the member is sent back to the
|
||||
host — a token the client echoes into a later call's params — pass it through as
|
||||
`z.string()` and let the send site keep it verbatim. `hostedReview`'s `provider` is the
|
||||
case: the eligibility reply names it and the create call returns it, so an
|
||||
|
||||
@@ -33,7 +33,12 @@ const EXPECTED_EVENTS = FILE_COUNT * EVENTS_PER_FILE
|
||||
const TOKENS_PER_EVENT = 200
|
||||
|
||||
const WORKTREES: UsageScanWorktreeRef[] = [
|
||||
{ repoId: 'repo-1', worktreeId: 'wt-1', path: '/tmp/orca-usage-oracle-project', displayName: 'demo' }
|
||||
{
|
||||
repoId: 'repo-1',
|
||||
worktreeId: 'wt-1',
|
||||
path: '/tmp/orca-usage-oracle-project',
|
||||
displayName: 'demo'
|
||||
}
|
||||
]
|
||||
|
||||
let corpusRoot = ''
|
||||
|
||||
@@ -0,0 +1,228 @@
|
||||
// @vitest-environment happy-dom
|
||||
|
||||
import { cleanup, fireEvent, render, screen } from '@testing-library/react'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { OpenFile } from '@/store/slices/editor'
|
||||
import { EditorPanelHeaderPath } from './EditorPanelHeaderPath'
|
||||
|
||||
const renameFileOnDiskMock = vi.hoisted(() => vi.fn())
|
||||
|
||||
vi.mock('@/store/selectors', () => ({
|
||||
useWorktreeById: () => ({ path: '/repo', repoId: 'repo-1' })
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/rename-file', () => ({
|
||||
renameFileOnDisk: renameFileOnDiskMock
|
||||
}))
|
||||
|
||||
vi.mock('@/hooks/useShortcutLabel', () => ({
|
||||
useShortcutLabel: () => ''
|
||||
}))
|
||||
|
||||
vi.mock('@/i18n/i18n', async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import('@/i18n/i18n')>() // eslint-disable-line @typescript-eslint/consistent-type-imports -- vi.mock requires an inline import
|
||||
return {
|
||||
...actual,
|
||||
translate: (_key: string, fallback: string, options?: { value0?: string }) =>
|
||||
fallback.replace('{{value0}}', options?.value0 ?? '')
|
||||
}
|
||||
})
|
||||
|
||||
afterEach(cleanup)
|
||||
|
||||
function baseFile(overrides: Partial<OpenFile> = {}): OpenFile {
|
||||
return {
|
||||
id: '/repo/notes.md',
|
||||
filePath: '/repo/notes.md',
|
||||
relativePath: 'notes.md',
|
||||
worktreeId: 'wt-1',
|
||||
language: 'markdown',
|
||||
isDirty: false,
|
||||
mode: 'edit',
|
||||
...overrides
|
||||
}
|
||||
}
|
||||
|
||||
function renderPath(file: OpenFile): (next: OpenFile) => void {
|
||||
const view = render(
|
||||
<EditorPanelHeaderPath
|
||||
activeFile={file}
|
||||
copiedPathVisible={false}
|
||||
canShowMarkdownPreview={false}
|
||||
onCopyPath={vi.fn()}
|
||||
onOpenMarkdownPreview={vi.fn()}
|
||||
onOpenContainingFolder={vi.fn()}
|
||||
/>
|
||||
)
|
||||
return (next) =>
|
||||
view.rerender(
|
||||
<EditorPanelHeaderPath
|
||||
activeFile={next}
|
||||
copiedPathVisible={false}
|
||||
canShowMarkdownPreview={false}
|
||||
onCopyPath={vi.fn()}
|
||||
onOpenMarkdownPreview={vi.fn()}
|
||||
onOpenContainingFolder={vi.fn()}
|
||||
/>
|
||||
)
|
||||
}
|
||||
|
||||
function getRenameInput(label: string): HTMLInputElement {
|
||||
const input = screen.getByLabelText(label)
|
||||
if (!(input instanceof HTMLInputElement)) {
|
||||
throw new Error(`Missing rename input: ${label}`)
|
||||
}
|
||||
return input
|
||||
}
|
||||
|
||||
function openRenameInput(): void {
|
||||
const pathRow = document.querySelector('.editor-header-path-row')
|
||||
if (!pathRow) {
|
||||
throw new Error('Missing editor header path row')
|
||||
}
|
||||
fireEvent.contextMenu(pathRow)
|
||||
fireEvent.click(screen.getByRole('menuitem', { name: 'Rename' }))
|
||||
}
|
||||
|
||||
describe('EditorPanelHeaderPath inline rename', () => {
|
||||
beforeEach(() => {
|
||||
renameFileOnDiskMock.mockReset()
|
||||
Object.assign(window, { api: { ui: { writeClipboardText: vi.fn() } } })
|
||||
vi.stubGlobal('requestAnimationFrame', (callback: FrameRequestCallback) => {
|
||||
callback(0)
|
||||
return 1
|
||||
})
|
||||
vi.stubGlobal('cancelAnimationFrame', vi.fn())
|
||||
})
|
||||
|
||||
it('opens a field holding the whole name', () => {
|
||||
renderPath(baseFile())
|
||||
openRenameInput()
|
||||
|
||||
const input = getRenameInput('Rename file notes.md')
|
||||
expect(input.value).toBe('notes.md')
|
||||
})
|
||||
|
||||
it('lets the field claim the full header width', () => {
|
||||
renderPath(baseFile())
|
||||
openRenameInput()
|
||||
|
||||
const input = getRenameInput('Rename file notes.md')
|
||||
expect(input.className).toContain('w-full')
|
||||
expect(input.className).toContain('max-w-full')
|
||||
})
|
||||
|
||||
it('selects the basename so typing replaces just the name', () => {
|
||||
renderPath(baseFile())
|
||||
openRenameInput()
|
||||
|
||||
const input = getRenameInput('Rename file notes.md')
|
||||
expect(document.activeElement).toBe(input)
|
||||
expect(input.selectionStart).toBe(0)
|
||||
expect(input.selectionEnd).toBe('notes'.length)
|
||||
})
|
||||
|
||||
it('renames to the typed name verbatim', () => {
|
||||
renderPath(baseFile())
|
||||
openRenameInput()
|
||||
|
||||
const input = getRenameInput('Rename file notes.md')
|
||||
fireEvent.change(input, { target: { value: 'renamed.mdx' } })
|
||||
fireEvent.keyDown(input, { key: 'Enter' })
|
||||
|
||||
expect(renameFileOnDiskMock).toHaveBeenCalledWith({
|
||||
oldPath: '/repo/notes.md',
|
||||
newName: 'renamed.mdx',
|
||||
worktreeId: 'wt-1',
|
||||
worktreePath: '/repo'
|
||||
})
|
||||
})
|
||||
|
||||
it('commits on blur like the tab bar and file explorer', () => {
|
||||
renderPath(baseFile())
|
||||
openRenameInput()
|
||||
|
||||
const input = getRenameInput('Rename file notes.md')
|
||||
fireEvent.change(input, { target: { value: 'renamed.md' } })
|
||||
fireEvent.blur(input)
|
||||
|
||||
expect(renameFileOnDiskMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ newName: 'renamed.md' })
|
||||
)
|
||||
})
|
||||
|
||||
it('does not request a rename when the name was not edited', () => {
|
||||
renderPath(baseFile())
|
||||
openRenameInput()
|
||||
|
||||
fireEvent.keyDown(getRenameInput('Rename file notes.md'), { key: 'Enter' })
|
||||
|
||||
expect(renameFileOnDiskMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('ignores an Enter that only confirms an IME candidate', () => {
|
||||
renderPath(baseFile())
|
||||
openRenameInput()
|
||||
|
||||
const input = getRenameInput('Rename file notes.md')
|
||||
fireEvent.change(input, { target: { value: 'renamed.md' } })
|
||||
fireEvent.keyDown(input, { key: 'Enter', keyCode: 229 })
|
||||
expect(renameFileOnDiskMock).not.toHaveBeenCalled()
|
||||
|
||||
fireEvent.keyDown(input, { key: 'Enter', keyCode: 13 })
|
||||
expect(renameFileOnDiskMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ newName: 'renamed.md' })
|
||||
)
|
||||
})
|
||||
|
||||
it('cancels on Escape without a trailing blur-commit, and ignores empty renames', () => {
|
||||
renderPath(baseFile())
|
||||
openRenameInput()
|
||||
|
||||
const input = getRenameInput('Rename file notes.md')
|
||||
fireEvent.change(input, { target: { value: 'renamed.md' } })
|
||||
fireEvent.keyDown(input, { key: 'Escape' })
|
||||
fireEvent.blur(input)
|
||||
expect(renameFileOnDiskMock).not.toHaveBeenCalled()
|
||||
expect(screen.queryByLabelText('Rename file notes.md')).toBeNull()
|
||||
|
||||
openRenameInput()
|
||||
fireEvent.change(getRenameInput('Rename file notes.md'), { target: { value: ' ' } })
|
||||
fireEvent.keyDown(getRenameInput('Rename file notes.md'), { key: 'Enter' })
|
||||
expect(renameFileOnDiskMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('drops rename mode when the active file changes', () => {
|
||||
const rerenderPath = renderPath(baseFile())
|
||||
openRenameInput()
|
||||
|
||||
const input = getRenameInput('Rename file notes.md')
|
||||
fireEvent.change(input, { target: { value: 'renamed.md' } })
|
||||
rerenderPath(
|
||||
baseFile({
|
||||
id: '/repo/other.md',
|
||||
filePath: '/repo/other.md',
|
||||
relativePath: 'other.md'
|
||||
})
|
||||
)
|
||||
|
||||
expect(screen.queryByLabelText('Rename file notes.md')).toBeNull()
|
||||
expect(screen.queryByLabelText('Rename file other.md')).toBeNull()
|
||||
|
||||
expect(renameFileOnDiskMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('selects the whole name when there is no extension', () => {
|
||||
const file = baseFile({
|
||||
id: '/repo/Makefile',
|
||||
filePath: '/repo/Makefile',
|
||||
relativePath: 'Makefile'
|
||||
})
|
||||
renderPath(file)
|
||||
openRenameInput()
|
||||
|
||||
const input = getRenameInput('Rename file Makefile')
|
||||
expect(input.selectionStart).toBe(0)
|
||||
expect(input.selectionEnd).toBe('Makefile'.length)
|
||||
})
|
||||
})
|
||||
@@ -8,8 +8,8 @@ import {
|
||||
DropdownMenuShortcut,
|
||||
DropdownMenuTrigger
|
||||
} from '@/components/ui/dropdown-menu'
|
||||
import { Input } from '@/components/ui/input'
|
||||
import { useShortcutLabel } from '@/hooks/useShortcutLabel'
|
||||
import { isImeCompositionKeyDown } from '@/lib/ime-composition-keyboard-event'
|
||||
import { translate } from '@/i18n/i18n'
|
||||
import type { OpenFile } from '@/store/slices/editor'
|
||||
import { CLOSE_ALL_CONTEXT_MENUS_EVENT } from '../tab-bar/SortableTab'
|
||||
@@ -88,7 +88,7 @@ export function EditorPanelHeaderPath({
|
||||
}}
|
||||
>
|
||||
{isRenaming ? (
|
||||
<Input
|
||||
<input
|
||||
ref={renameInputRef}
|
||||
data-editor-header-rename-input="true"
|
||||
aria-label={translate(
|
||||
@@ -97,15 +97,20 @@ export function EditorPanelHeaderPath({
|
||||
{ value0: currentFileName }
|
||||
)}
|
||||
defaultValue={currentFileName}
|
||||
// Why: the header is narrow in floating mode; this keeps the
|
||||
// edit field aligned with the path label without growing chrome.
|
||||
className="h-6 w-[16ch] min-w-[104px] max-w-full rounded-sm bg-input/40 px-1.5 py-0 font-mono text-xs text-foreground md:text-xs focus-visible:ring-[1px]"
|
||||
// Why: the field spans the header rather than sizing to the name —
|
||||
// a long path is exactly when the rename field needs the room.
|
||||
className="h-6 w-full min-w-0 max-w-full rounded-md border border-accent/40 bg-input/40 px-1.5 font-mono text-xs text-foreground outline-none focus:border-accent focus:ring-1 focus:ring-ring"
|
||||
spellCheck={false}
|
||||
onPointerDown={(event) => event.stopPropagation()}
|
||||
onMouseDown={(event) => event.stopPropagation()}
|
||||
onClick={(event) => event.stopPropagation()}
|
||||
onDoubleClick={(event) => event.stopPropagation()}
|
||||
onKeyDown={(event) => {
|
||||
// Why: an Enter that only confirms a CJK IME candidate must not
|
||||
// commit the rename; wait for a non-composition Enter.
|
||||
if (isImeCompositionKeyDown(event)) {
|
||||
return
|
||||
}
|
||||
if (event.key === 'Enter') {
|
||||
event.preventDefault()
|
||||
event.stopPropagation()
|
||||
@@ -130,12 +135,17 @@ export function EditorPanelHeaderPath({
|
||||
<span className="editor-header-path-file">{displayPath.fileName}</span>
|
||||
</button>
|
||||
)}
|
||||
<span
|
||||
className={`editor-header-copy-toast${copiedPathVisible ? ' is-visible' : ''}`}
|
||||
aria-live="polite"
|
||||
>
|
||||
{headerCopyState.copyToastLabel}
|
||||
</span>
|
||||
{/* Why: the toast is opacity-0 rather than display-none, so leaving it
|
||||
mounted reserves ~100px of the row from the rename field for a
|
||||
message that cannot fire while renaming. */}
|
||||
{!isRenaming && (
|
||||
<span
|
||||
className={`editor-header-copy-toast${copiedPathVisible ? ' is-visible' : ''}`}
|
||||
aria-live="polite"
|
||||
>
|
||||
{headerCopyState.copyToastLabel}
|
||||
</span>
|
||||
)}
|
||||
</div>
|
||||
<DropdownMenu open={pathMenuOpen} onOpenChange={setPathMenuOpen} modal={false}>
|
||||
<DropdownMenuTrigger asChild>
|
||||
|
||||
@@ -0,0 +1,73 @@
|
||||
// @vitest-environment happy-dom
|
||||
|
||||
import { act, renderHook } from '@testing-library/react'
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { OpenFile } from '@/store/slices/editor'
|
||||
import { useEditorHeaderFileRename } from './editor-header-file-rename'
|
||||
|
||||
const renameFileOnDiskMock = vi.hoisted(() => vi.fn())
|
||||
|
||||
vi.mock('@/store/selectors', () => ({
|
||||
useWorktreeById: () => ({ path: '/repo', repoId: 'repo-1' })
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/rename-file', () => ({
|
||||
renameFileOnDisk: renameFileOnDiskMock
|
||||
}))
|
||||
|
||||
function baseFile(overrides: Partial<OpenFile> = {}): OpenFile {
|
||||
return {
|
||||
id: '/repo/notes.md',
|
||||
filePath: '/repo/notes.md',
|
||||
relativePath: 'notes.md',
|
||||
worktreeId: 'wt-1',
|
||||
language: 'markdown',
|
||||
isDirty: false,
|
||||
mode: 'edit',
|
||||
...overrides
|
||||
}
|
||||
}
|
||||
|
||||
function renameInputStub(value: string): HTMLInputElement {
|
||||
const input = document.createElement('input')
|
||||
input.value = value
|
||||
return input
|
||||
}
|
||||
|
||||
describe('useEditorHeaderFileRename', () => {
|
||||
beforeEach(() => {
|
||||
renameFileOnDiskMock.mockReset()
|
||||
vi.stubGlobal('requestAnimationFrame', (callback: FrameRequestCallback) => {
|
||||
callback(0)
|
||||
return 1
|
||||
})
|
||||
vi.stubGlobal('cancelAnimationFrame', vi.fn())
|
||||
})
|
||||
|
||||
// Unmounting the focused input emits a trailing focusout, so commitRename can
|
||||
// still run after the active file changed — with the old input as ref target.
|
||||
it('ignores a blur-commit that arrives after the active file changed', () => {
|
||||
const { result, rerender } = renderHook((file: OpenFile) => useEditorHeaderFileRename(file), {
|
||||
initialProps: baseFile()
|
||||
})
|
||||
|
||||
act(() => {
|
||||
result.current.openRenameInput()
|
||||
})
|
||||
act(() => {
|
||||
result.current.renameInputRef(renameInputStub('renamed.md'))
|
||||
})
|
||||
|
||||
rerender(
|
||||
baseFile({ id: '/repo/other.md', filePath: '/repo/other.md', relativePath: 'other.md' })
|
||||
)
|
||||
|
||||
expect(result.current.isRenaming).toBe(false)
|
||||
|
||||
act(() => {
|
||||
result.current.commitRename()
|
||||
})
|
||||
|
||||
expect(renameFileOnDiskMock).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
@@ -19,9 +19,21 @@ type EditorHeaderFileRenameState = {
|
||||
export function useEditorHeaderFileRename(activeFile: OpenFile): EditorHeaderFileRenameState {
|
||||
const worktree = useWorktreeById(activeFile.worktreeId)
|
||||
const [isRenaming, setIsRenaming] = useState(false)
|
||||
const [renameFilePath, setRenameFilePath] = useState(activeFile.filePath)
|
||||
const renameInputElementRef = useRef<HTMLInputElement | null>(null)
|
||||
const renameCancelledRef = useRef(false)
|
||||
const renameFocusFrameRef = useRef<number | null>(null)
|
||||
// Escape fires setIsRenaming(false), which unmounts the input. The browser
|
||||
// still fires focusout as the focused node is removed, so onBlur can invoke
|
||||
// commitRename *after* cancel — committing the typed value against the
|
||||
// user's intent. This flag suppresses the trailing blur-commit.
|
||||
const renameCancelledRef = useRef(false)
|
||||
// Why: the header renders one unkeyed path strip for every file, so a file
|
||||
// switch mid-rename would otherwise commit the typed name against the new path.
|
||||
if (renameFilePath !== activeFile.filePath) {
|
||||
renameCancelledRef.current = true
|
||||
setRenameFilePath(activeFile.filePath)
|
||||
setIsRenaming(false)
|
||||
}
|
||||
const currentFileName = basename(activeFile.filePath)
|
||||
// Why: read-only tabs (AI Vault View Log) are never renameable — rename would
|
||||
// rewrite the agent-owned artifact's backing path.
|
||||
@@ -88,8 +100,10 @@ export function useEditorHeaderFileRename(activeFile: OpenFile): EditorHeaderFil
|
||||
return
|
||||
}
|
||||
|
||||
// Why: focus belongs to the rename input mount; the frame preserves the
|
||||
// previous timing so header layout settles before selecting text.
|
||||
// Why: focus belongs to the rename input mount; the frame lets the header
|
||||
// layout settle before selecting text. Preselect the basename only, so
|
||||
// typing replaces the name and leaves the extension — same as the tab bar
|
||||
// and the file explorer.
|
||||
renameFocusFrameRef.current = requestAnimationFrame(() => {
|
||||
renameFocusFrameRef.current = null
|
||||
if (renameInputElementRef.current !== el) {
|
||||
|
||||
Reference in New Issue
Block a user