mirror of
https://github.com/stablyai/orca.git
synced 2026-10-21 00:03:46 +00:00
fix(cmd-j): re-emit section headers for interleaved palette remainder ro
When both open tabs and worktrees overflow their first-screen slice, the layout interleaves remainder rows. The trailing-section header renders before the leading remainder, so unlabeled rows read as the wrong section. Re-emit headers before each remainder with a distinct suffix for React keys. Refactor type-alias matching into a reusable utility that prefers earliest match position over declaration order.
This commit is contained in:
@@ -268,6 +268,8 @@ const EMPTY_QUERY_RECENT_TAB_CAP = 6
|
||||
const EMPTY_QUERY_ROW_BUDGET = 10
|
||||
const EMPTY_QUERY_WORKTREE_CAP = 5
|
||||
const EMPTY_RECENT_TAB_ORDER: readonly string[] = []
|
||||
// Why: the interleaved layout emits a section header twice; the second copy needs a distinct entry id.
|
||||
const CONTINUED_SECTION_HEADER_ID_SUFFIX = '__continued'
|
||||
|
||||
function isCurrentOpenTabItem(item: OpenTabPaletteItem): boolean {
|
||||
return item.type === 'browser-page' ? item.result.isCurrentPage : item.result.isCurrentTab
|
||||
@@ -1494,12 +1496,13 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null {
|
||||
hasQuery && visibleProjectTargetItems.length > 0 && populatedSectionCount > 1
|
||||
const showMiddleHeader = hasQuery && visibleMiddleItems.length > 0 && populatedSectionCount > 1
|
||||
|
||||
const pushOpenTabsHeader = (): void => {
|
||||
// idSuffix: the interleaved layout re-emits a header for the section's remainder, which needs its own key.
|
||||
const pushOpenTabsHeader = (idSuffix = ''): void => {
|
||||
if (!showOpenTabsHeader) {
|
||||
return
|
||||
}
|
||||
entries.push({
|
||||
id: '__header_open_tabs__',
|
||||
id: `__header_open_tabs__${idSuffix}`,
|
||||
type: 'section-header',
|
||||
label: hasQuery
|
||||
? translate('auto.components.WorktreeJumpPalette.50a1d11d5b', 'Open Tabs')
|
||||
@@ -1510,12 +1513,12 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null {
|
||||
})
|
||||
}
|
||||
|
||||
const pushWorktreesHeader = (): void => {
|
||||
const pushWorktreesHeader = (idSuffix = ''): void => {
|
||||
if (!showWorktreeHeader) {
|
||||
return
|
||||
}
|
||||
entries.push({
|
||||
id: '__header_worktrees__',
|
||||
id: `__header_worktrees__${idSuffix}`,
|
||||
type: 'section-header',
|
||||
label: hasQuery
|
||||
? translate('auto.components.WorktreeJumpPalette.worktreesHeader', 'Worktrees')
|
||||
@@ -1600,24 +1603,42 @@ export default function WorktreeJumpPalette(): React.JSX.Element | null {
|
||||
? '__hint_worktree_overflow__'
|
||||
: '__hint_open_tab_overflow__'
|
||||
|
||||
if (openTabsLeadSections) {
|
||||
pushOpenTabsHeader()
|
||||
} else {
|
||||
pushWorktreesHeader()
|
||||
const pushLeadingHeader = (idSuffix = ''): void => {
|
||||
if (openTabsLeadSections) {
|
||||
pushOpenTabsHeader(idSuffix)
|
||||
} else {
|
||||
pushWorktreesHeader(idSuffix)
|
||||
}
|
||||
}
|
||||
const pushTrailingHeader = (idSuffix = ''): void => {
|
||||
if (openTabsLeadSections) {
|
||||
pushWorktreesHeader(idSuffix)
|
||||
} else {
|
||||
pushOpenTabsHeader(idSuffix)
|
||||
}
|
||||
}
|
||||
|
||||
pushLeadingHeader()
|
||||
appendPaletteListEntries(entries, multiPrimaryLayout.leadingPreview as PaletteItem[])
|
||||
// Soft more for the leading section (scrollable rest + hard-cap tail).
|
||||
// Soft more for the leading section (rows resuming below + hard-cap tail).
|
||||
pushOverflowHint(leadingHintId, multiPrimaryLayout.leadingMoreCount)
|
||||
if (openTabsLeadSections) {
|
||||
pushWorktreesHeader()
|
||||
} else {
|
||||
pushOpenTabsHeader()
|
||||
}
|
||||
pushTrailingHeader()
|
||||
// Floor first, then remaining leading rows, then trailing rest — same order
|
||||
// as orderMultiPrimaryPaletteItems / keyboard selection.
|
||||
// as orderMultiPrimaryPaletteItems / keyboard selection. Each remainder
|
||||
// re-emits its own header so no row sits under the other section's label.
|
||||
appendPaletteListEntries(entries, multiPrimaryLayout.trailingFloor as PaletteItem[])
|
||||
appendPaletteListEntries(entries, multiPrimaryLayout.leadingRest as PaletteItem[])
|
||||
appendPaletteListEntries(entries, multiPrimaryLayout.trailingRest as PaletteItem[])
|
||||
const hasLeadingRest = multiPrimaryLayout.leadingRest.length > 0
|
||||
if (hasLeadingRest) {
|
||||
pushLeadingHeader(CONTINUED_SECTION_HEADER_ID_SUFFIX)
|
||||
appendPaletteListEntries(entries, multiPrimaryLayout.leadingRest as PaletteItem[])
|
||||
}
|
||||
if (multiPrimaryLayout.trailingRest.length > 0) {
|
||||
// Only re-label when the leading remainder split the trailing section.
|
||||
if (hasLeadingRest) {
|
||||
pushTrailingHeader(CONTINUED_SECTION_HEADER_ID_SUFFIX)
|
||||
}
|
||||
appendPaletteListEntries(entries, multiPrimaryLayout.trailingRest as PaletteItem[])
|
||||
}
|
||||
// Trailing rest is already on screen; only hard-cap overflow needs a hint.
|
||||
pushOverflowHint(trailingHintId, multiPrimaryLayout.trailingHardOverflowCount)
|
||||
pushProjectAndMiddleSections()
|
||||
|
||||
@@ -64,7 +64,12 @@ export function softSplitPaletteSection<T>(
|
||||
* it is not buried under ~50 leading rows. Remaining rows of both sections
|
||||
* follow (still under the hard cap). Projects/middle stay after both primaries.
|
||||
*
|
||||
* `leadingMoreCount` is the mid-list soft hint (rest + hard-cap overflow).
|
||||
* Callers must re-emit the section header before each remainder — the interleave
|
||||
* puts `leadingRest` after the trailing header, so unlabelled rows read as the
|
||||
* wrong section.
|
||||
*
|
||||
* `leadingMoreCount` is the mid-list soft hint (rest resuming below + hard-cap
|
||||
* overflow).
|
||||
* `trailingHardOverflowCount` is only rows past the hard cap — trailing rest is
|
||||
* already rendered, so a soft “more” would double-count scrollable rows.
|
||||
*/
|
||||
|
||||
@@ -0,0 +1,279 @@
|
||||
// @vitest-environment happy-dom
|
||||
|
||||
import { act } from 'react'
|
||||
import { createRoot, type Root } from 'react-dom/client'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type * as ReactI18Next from 'react-i18next'
|
||||
import type { Repo, Tab, TabGroup, TerminalTab, Worktree } from '../../../shared/types'
|
||||
import { useAppStore } from '@/store'
|
||||
import type { AppState } from '@/store/types'
|
||||
import WorktreeJumpPalette from './WorktreeJumpPalette'
|
||||
|
||||
vi.mock('react-i18next', async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof ReactI18Next>()
|
||||
return {
|
||||
...actual,
|
||||
useTranslation: () => ({
|
||||
t: (_key: string, fallback?: string) => fallback ?? _key
|
||||
})
|
||||
}
|
||||
})
|
||||
|
||||
vi.mock('sonner', () => ({
|
||||
toast: {
|
||||
success: vi.fn(),
|
||||
error: vi.fn(),
|
||||
info: vi.fn(),
|
||||
warning: vi.fn(),
|
||||
message: vi.fn()
|
||||
}
|
||||
}))
|
||||
|
||||
vi.mock('@/hooks/useSettingsNavigationMetadata', () => ({
|
||||
useSettingsNavigationMetadata: () => []
|
||||
}))
|
||||
|
||||
vi.mock('@/components/sidebar/StatusIndicator', () => ({
|
||||
default: () => <span data-status-indicator="true" />
|
||||
}))
|
||||
|
||||
vi.mock('@/components/repo/RepoBadgeLabel', () => ({
|
||||
RepoBadgeMark: () => <span data-repo-badge-mark="true" />
|
||||
}))
|
||||
|
||||
vi.mock('@/components/cmd-j/palette-host-badge', () => ({
|
||||
getPaletteHostBadge: () => null
|
||||
}))
|
||||
|
||||
vi.mock('@/components/ui/command', async () => {
|
||||
const React = await import('react')
|
||||
return {
|
||||
CommandDialog: ({ children, open }: { children: React.ReactNode; open?: boolean }) =>
|
||||
open ? <div data-command-dialog="true">{children}</div> : null,
|
||||
CommandInput: ({
|
||||
value,
|
||||
onValueChange
|
||||
}: {
|
||||
value?: string
|
||||
onValueChange?: (next: string) => void
|
||||
}) => {
|
||||
setCommandQuery = onValueChange ?? null
|
||||
return <input data-command-input="true" value={value} onChange={() => {}} />
|
||||
},
|
||||
CommandList: React.forwardRef(function CommandList(
|
||||
{ children }: { children: React.ReactNode },
|
||||
ref: React.ForwardedRef<HTMLDivElement>
|
||||
) {
|
||||
return (
|
||||
<div ref={ref} data-command-list="true">
|
||||
{children}
|
||||
</div>
|
||||
)
|
||||
}),
|
||||
CommandEmpty: ({ children }: { children: React.ReactNode }) => (
|
||||
<div data-command-empty="true">{children}</div>
|
||||
),
|
||||
CommandItem: ({ children, value }: { children: React.ReactNode; value?: string }) => (
|
||||
<button data-command-item={value ?? ''} type="button">
|
||||
{children}
|
||||
</button>
|
||||
)
|
||||
}
|
||||
})
|
||||
|
||||
const initialAppState = useAppStore.getInitialState()
|
||||
let testRoot: Root
|
||||
let testContainer: HTMLDivElement
|
||||
let setCommandQuery: ((next: string) => void) | null = null
|
||||
|
||||
function makeRepo(): Repo {
|
||||
return {
|
||||
id: 'repo-1',
|
||||
path: '/repos/repo-1',
|
||||
displayName: 'Repo 1',
|
||||
badgeColor: '#000000',
|
||||
addedAt: 0
|
||||
}
|
||||
}
|
||||
|
||||
function makeWorktree(id: string, displayName: string): Worktree {
|
||||
return {
|
||||
id,
|
||||
repoId: 'repo-1',
|
||||
path: `/tmp/${id}`,
|
||||
head: 'abc123',
|
||||
branch: 'refs/heads/main',
|
||||
isBare: false,
|
||||
isMainWorktree: false,
|
||||
displayName,
|
||||
comment: '',
|
||||
linkedIssue: null,
|
||||
linkedPR: null,
|
||||
linkedLinearIssue: null,
|
||||
isArchived: false,
|
||||
isUnread: false,
|
||||
isPinned: false,
|
||||
sortOrder: 0,
|
||||
lastActivityAt: 0
|
||||
}
|
||||
}
|
||||
|
||||
function makeTerminalTab(id: string, worktreeId: string, title: string): TerminalTab {
|
||||
return {
|
||||
id,
|
||||
ptyId: `pty-${id}`,
|
||||
worktreeId,
|
||||
title,
|
||||
customTitle: null,
|
||||
color: null,
|
||||
sortOrder: 0,
|
||||
createdAt: 0
|
||||
}
|
||||
}
|
||||
|
||||
function makeUnifiedTab(id: string, worktreeId: string, entityId: string, label: string): Tab {
|
||||
return {
|
||||
id,
|
||||
entityId,
|
||||
groupId: `group-${worktreeId}`,
|
||||
worktreeId,
|
||||
contentType: 'terminal',
|
||||
label,
|
||||
customLabel: null,
|
||||
color: null,
|
||||
sortOrder: 0,
|
||||
createdAt: 0
|
||||
}
|
||||
}
|
||||
|
||||
function makeGroup(worktreeId: string, tabIds: string[]): TabGroup {
|
||||
return {
|
||||
id: `group-${worktreeId}`,
|
||||
worktreeId,
|
||||
activeTabId: tabIds[0] ?? null,
|
||||
tabOrder: tabIds,
|
||||
recentTabIds: tabIds
|
||||
}
|
||||
}
|
||||
|
||||
/** Both primaries overflow their first-screen slice, so the layout interleaves. */
|
||||
function makeInterleavedQueryState(): Partial<AppState> {
|
||||
const tabIds = Array.from({ length: 8 }, (_, index) => `${index}`)
|
||||
return {
|
||||
worktreesByRepo: {
|
||||
'repo-1': [
|
||||
makeWorktree('wt-tabs', 'tab-host'),
|
||||
...Array.from({ length: 5 }, (_, index) =>
|
||||
makeWorktree(`wt-${index}`, `improve-perf-${index}`)
|
||||
)
|
||||
]
|
||||
},
|
||||
showSleepingWorkspaces: true,
|
||||
ptyIdsByTabId: Object.fromEntries(tabIds.map((id) => [`term-${id}`, [`pty-${id}`]])),
|
||||
tabsByWorktree: {
|
||||
'wt-tabs': tabIds.map((id) => makeTerminalTab(`term-${id}`, 'wt-tabs', `Perf chat ${id}`))
|
||||
},
|
||||
unifiedTabsByWorktree: {
|
||||
'wt-tabs': tabIds.map((id) =>
|
||||
makeUnifiedTab(`tab-${id}`, 'wt-tabs', `term-${id}`, `Perf chat ${id}`)
|
||||
)
|
||||
},
|
||||
groupsByWorktree: {
|
||||
'wt-tabs': [
|
||||
makeGroup(
|
||||
'wt-tabs',
|
||||
tabIds.map((id) => `tab-${id}`)
|
||||
)
|
||||
]
|
||||
},
|
||||
activeGroupIdByWorktree: { 'wt-tabs': 'group-wt-tabs' }
|
||||
}
|
||||
}
|
||||
|
||||
async function flushEffects(): Promise<void> {
|
||||
await act(async () => {
|
||||
await Promise.resolve()
|
||||
await Promise.resolve()
|
||||
})
|
||||
}
|
||||
|
||||
async function renderPalette(overrides: Partial<AppState>): Promise<void> {
|
||||
useAppStore.setState({
|
||||
activeModal: 'worktree-palette',
|
||||
activeWorktreeId: null,
|
||||
repos: [makeRepo()],
|
||||
tabsByWorktree: {},
|
||||
browserTabsByWorktree: {},
|
||||
browserPagesByWorkspace: {},
|
||||
unifiedTabsByWorktree: {},
|
||||
hideDefaultBranchWorkspace: false,
|
||||
hideAutomationGeneratedWorkspaces: false,
|
||||
alwaysShowDefaultBranchWorkspace: true,
|
||||
lastVisitedAtByWorktreeId: {},
|
||||
...overrides
|
||||
} as Partial<AppState>)
|
||||
|
||||
await act(async () => {
|
||||
testRoot.render(<WorktreeJumpPalette />)
|
||||
})
|
||||
await flushEffects()
|
||||
}
|
||||
|
||||
/** Each primary row paired with the section header rendered above it, in DOM order. */
|
||||
function getPrimaryRowsBySectionHeader(): { header: string; rowId: string }[] {
|
||||
const headerLabels = new Set(['Open Tabs', 'Worktrees'])
|
||||
const pairs: { header: string; rowId: string }[] = []
|
||||
let header = ''
|
||||
for (const node of testContainer.querySelectorAll<HTMLElement>(
|
||||
'[data-command-item], .uppercase'
|
||||
)) {
|
||||
const rowId = node.dataset.commandItem
|
||||
if (rowId) {
|
||||
if (rowId.startsWith('workspace-tab:') || rowId.startsWith('worktree:')) {
|
||||
pairs.push({ header, rowId })
|
||||
}
|
||||
continue
|
||||
}
|
||||
const label = node.textContent?.trim() ?? ''
|
||||
if (!node.closest('[data-command-item]') && headerLabels.has(label)) {
|
||||
header = label
|
||||
}
|
||||
}
|
||||
return pairs
|
||||
}
|
||||
|
||||
describe('WorktreeJumpPalette interleaved primary sections', () => {
|
||||
beforeEach(() => {
|
||||
globalThis.IS_REACT_ACT_ENVIRONMENT = true
|
||||
setCommandQuery = null
|
||||
useAppStore.setState(initialAppState, true)
|
||||
testContainer = document.createElement('div')
|
||||
document.body.appendChild(testContainer)
|
||||
testRoot = createRoot(testContainer)
|
||||
})
|
||||
|
||||
afterEach(async () => {
|
||||
await act(async () => {
|
||||
testRoot.unmount()
|
||||
})
|
||||
document.body.replaceChildren()
|
||||
useAppStore.setState(initialAppState, true)
|
||||
})
|
||||
|
||||
it('keeps every interleaved row under its own section header', async () => {
|
||||
await renderPalette(makeInterleavedQueryState())
|
||||
|
||||
await act(async () => {
|
||||
setCommandQuery?.('perf')
|
||||
})
|
||||
await flushEffects()
|
||||
|
||||
const rows = getPrimaryRowsBySectionHeader()
|
||||
// Why the counts: both remainders must still render, just under a re-emitted header.
|
||||
expect(rows.filter((row) => row.rowId.startsWith('workspace-tab:'))).toHaveLength(8)
|
||||
expect(rows.filter((row) => row.rowId.startsWith('worktree:'))).toHaveLength(5)
|
||||
for (const { header, rowId } of rows) {
|
||||
expect(header).toBe(rowId.startsWith('workspace-tab:') ? 'Open Tabs' : 'Worktrees')
|
||||
}
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,33 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { selectPaletteTypeAliasMatch } from './palette-type-alias-match'
|
||||
|
||||
const ALIASES = ['mobile emulator tab', 'mobile emulator', 'ios simulator', 'emulator'] as const
|
||||
|
||||
describe('selectPaletteTypeAliasMatch', () => {
|
||||
it('prefers the alias with the earliest match, not declaration order', () => {
|
||||
expect(selectPaletteTypeAliasMatch(ALIASES, 'emulator')).toEqual({
|
||||
text: 'emulator',
|
||||
range: { start: 0, end: 8 }
|
||||
})
|
||||
})
|
||||
|
||||
it('keeps the first alias when several match at the same offset', () => {
|
||||
expect(selectPaletteTypeAliasMatch(ALIASES, 'mobile')).toEqual({
|
||||
text: 'mobile emulator tab',
|
||||
range: { start: 0, end: 6 }
|
||||
})
|
||||
})
|
||||
|
||||
it('still reports a mid-string hit when no alias starts with the query', () => {
|
||||
expect(selectPaletteTypeAliasMatch(ALIASES, 'simulator')).toEqual({
|
||||
text: 'ios simulator',
|
||||
range: { start: 4, end: 13 }
|
||||
})
|
||||
})
|
||||
|
||||
it('returns null for an empty query or no match', () => {
|
||||
expect(selectPaletteTypeAliasMatch(ALIASES, '')).toBeNull()
|
||||
expect(selectPaletteTypeAliasMatch(ALIASES, 'browser')).toBeNull()
|
||||
expect(selectPaletteTypeAliasMatch([], 'emulator')).toBeNull()
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,28 @@
|
||||
import type { MatchRange } from './worktree-palette-search'
|
||||
|
||||
export type PaletteTypeAliasMatch = { text: string; range: MatchRange }
|
||||
|
||||
/**
|
||||
* Earliest match start wins, not declaration order: "emulator" has to score as a
|
||||
* prefix hit on 'emulator' rather than a mid-string hit inside 'mobile emulator
|
||||
* tab'. Ties keep the first alias so the longest phrasing stays the label.
|
||||
*/
|
||||
export function selectPaletteTypeAliasMatch(
|
||||
aliases: readonly string[],
|
||||
lowercasedQuery: string
|
||||
): PaletteTypeAliasMatch | null {
|
||||
if (!lowercasedQuery) {
|
||||
return null
|
||||
}
|
||||
let best: PaletteTypeAliasMatch | null = null
|
||||
for (const alias of aliases) {
|
||||
const start = alias.toLowerCase().indexOf(lowercasedQuery)
|
||||
if (start === -1) {
|
||||
continue
|
||||
}
|
||||
if (!best || start < best.range.start) {
|
||||
best = { text: alias, range: { start, end: start + lowercasedQuery.length } }
|
||||
}
|
||||
}
|
||||
return best
|
||||
}
|
||||
@@ -119,6 +119,11 @@ describe('simulator-palette-search', () => {
|
||||
expect(searchSimulatorTabs(entries, 'simulator')).toHaveLength(1)
|
||||
expect(searchSimulatorTabs(entries, 'ios')).toHaveLength(1)
|
||||
expect(searchSimulatorTabs(entries, 'emulator')).toHaveLength(1)
|
||||
// Why: "emulator" must score as a prefix hit, not mid-string in 'mobile emulator tab'.
|
||||
expect(searchSimulatorTabs(entries, 'emulator')[0]?.typeAliasMatch).toEqual({
|
||||
text: 'emulator',
|
||||
range: { start: 0, end: 8 }
|
||||
})
|
||||
})
|
||||
|
||||
it('searches worktree and repo metadata', () => {
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import type { Tab, TabGroup, Worktree } from '../../../shared/types'
|
||||
import { isClipboardTextByteLengthOverLimit } from '../../../shared/clipboard-text'
|
||||
import { selectPaletteTypeAliasMatch } from './palette-type-alias-match'
|
||||
import { resolveWorktreeDisplayName } from './worktree-default-display-name'
|
||||
import type { MatchRange } from './worktree-palette-search'
|
||||
|
||||
@@ -239,14 +240,7 @@ export function searchSimulatorTabs(
|
||||
continue
|
||||
}
|
||||
|
||||
let typeAliasHit: { text: string; range: MatchRange } | null = null
|
||||
for (const alias of SIMULATOR_TYPE_SEARCH_ALIASES) {
|
||||
const range = findRange(alias, trimmedQuery)
|
||||
if (range) {
|
||||
typeAliasHit = { text: alias, range }
|
||||
break
|
||||
}
|
||||
}
|
||||
const typeAliasHit = selectPaletteTypeAliasMatch(SIMULATOR_TYPE_SEARCH_ALIASES, trimmedQuery)
|
||||
if (typeAliasHit) {
|
||||
results.push({
|
||||
...baseResult,
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import { selectPaletteTypeAliasMatch } from './palette-type-alias-match'
|
||||
import { resolveWorktreeDisplayName } from './worktree-default-display-name'
|
||||
import type { MatchRange } from './worktree-palette-search'
|
||||
import type {
|
||||
@@ -187,14 +188,7 @@ export function searchWorkspaceTabs(
|
||||
}
|
||||
|
||||
// Why after display secondaries: path/file matches should beat bare type labels.
|
||||
let typeAliasHit: { text: string; range: MatchRange } | null = null
|
||||
for (const alias of entry.typeSearchAliases ?? []) {
|
||||
const range = findRange(alias, trimmedQuery)
|
||||
if (range) {
|
||||
typeAliasHit = { text: alias, range }
|
||||
break
|
||||
}
|
||||
}
|
||||
const typeAliasHit = selectPaletteTypeAliasMatch(entry.typeSearchAliases ?? [], trimmedQuery)
|
||||
if (typeAliasHit) {
|
||||
results.push({
|
||||
...baseResult,
|
||||
|
||||
Reference in New Issue
Block a user