From 1d8a30aafa3ca6f0b5a99ca10307a2212d62610f Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Sat, 8 Aug 2026 12:16:08 -0700 Subject: [PATCH] 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. --- .../src/components/WorktreeJumpPalette.tsx | 55 ++-- .../cmd-j/palette-section-render-cap.ts | 7 +- ...jump-palette-interleaved-sections.test.tsx | 279 ++++++++++++++++++ .../src/lib/palette-type-alias-match.test.ts | 33 +++ .../src/lib/palette-type-alias-match.ts | 28 ++ .../src/lib/simulator-palette-search.test.ts | 5 + .../src/lib/simulator-palette-search.ts | 10 +- .../src/lib/workspace-tab-palette-results.ts | 10 +- 8 files changed, 393 insertions(+), 34 deletions(-) create mode 100644 src/renderer/src/components/worktree-jump-palette-interleaved-sections.test.tsx create mode 100644 src/renderer/src/lib/palette-type-alias-match.test.ts create mode 100644 src/renderer/src/lib/palette-type-alias-match.ts diff --git a/src/renderer/src/components/WorktreeJumpPalette.tsx b/src/renderer/src/components/WorktreeJumpPalette.tsx index bd163a9c4fb..1c901b165bd 100644 --- a/src/renderer/src/components/WorktreeJumpPalette.tsx +++ b/src/renderer/src/components/WorktreeJumpPalette.tsx @@ -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() diff --git a/src/renderer/src/components/cmd-j/palette-section-render-cap.ts b/src/renderer/src/components/cmd-j/palette-section-render-cap.ts index 334e2c8c1ff..710b40d7e1d 100644 --- a/src/renderer/src/components/cmd-j/palette-section-render-cap.ts +++ b/src/renderer/src/components/cmd-j/palette-section-render-cap.ts @@ -64,7 +64,12 @@ export function softSplitPaletteSection( * 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. */ diff --git a/src/renderer/src/components/worktree-jump-palette-interleaved-sections.test.tsx b/src/renderer/src/components/worktree-jump-palette-interleaved-sections.test.tsx new file mode 100644 index 00000000000..1cc3989e37a --- /dev/null +++ b/src/renderer/src/components/worktree-jump-palette-interleaved-sections.test.tsx @@ -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() + 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: () => +})) + +vi.mock('@/components/repo/RepoBadgeLabel', () => ({ + RepoBadgeMark: () => +})) + +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 ?
{children}
: null, + CommandInput: ({ + value, + onValueChange + }: { + value?: string + onValueChange?: (next: string) => void + }) => { + setCommandQuery = onValueChange ?? null + return {}} /> + }, + CommandList: React.forwardRef(function CommandList( + { children }: { children: React.ReactNode }, + ref: React.ForwardedRef + ) { + return ( +
+ {children} +
+ ) + }), + CommandEmpty: ({ children }: { children: React.ReactNode }) => ( +
{children}
+ ), + CommandItem: ({ children, value }: { children: React.ReactNode; value?: string }) => ( + + ) + } +}) + +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 { + 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 { + await act(async () => { + await Promise.resolve() + await Promise.resolve() + }) +} + +async function renderPalette(overrides: Partial): Promise { + useAppStore.setState({ + activeModal: 'worktree-palette', + activeWorktreeId: null, + repos: [makeRepo()], + tabsByWorktree: {}, + browserTabsByWorktree: {}, + browserPagesByWorkspace: {}, + unifiedTabsByWorktree: {}, + hideDefaultBranchWorkspace: false, + hideAutomationGeneratedWorkspaces: false, + alwaysShowDefaultBranchWorkspace: true, + lastVisitedAtByWorktreeId: {}, + ...overrides + } as Partial) + + await act(async () => { + testRoot.render() + }) + 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( + '[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') + } + }) +}) diff --git a/src/renderer/src/lib/palette-type-alias-match.test.ts b/src/renderer/src/lib/palette-type-alias-match.test.ts new file mode 100644 index 00000000000..43d3ca16411 --- /dev/null +++ b/src/renderer/src/lib/palette-type-alias-match.test.ts @@ -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() + }) +}) diff --git a/src/renderer/src/lib/palette-type-alias-match.ts b/src/renderer/src/lib/palette-type-alias-match.ts new file mode 100644 index 00000000000..830a99759c7 --- /dev/null +++ b/src/renderer/src/lib/palette-type-alias-match.ts @@ -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 +} diff --git a/src/renderer/src/lib/simulator-palette-search.test.ts b/src/renderer/src/lib/simulator-palette-search.test.ts index ee74978b101..52393f15422 100644 --- a/src/renderer/src/lib/simulator-palette-search.test.ts +++ b/src/renderer/src/lib/simulator-palette-search.test.ts @@ -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', () => { diff --git a/src/renderer/src/lib/simulator-palette-search.ts b/src/renderer/src/lib/simulator-palette-search.ts index e458f62ae7f..1c2f7efc89e 100644 --- a/src/renderer/src/lib/simulator-palette-search.ts +++ b/src/renderer/src/lib/simulator-palette-search.ts @@ -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, diff --git a/src/renderer/src/lib/workspace-tab-palette-results.ts b/src/renderer/src/lib/workspace-tab-palette-results.ts index 4d0e5dc8379..18570802abc 100644 --- a/src/renderer/src/lib/workspace-tab-palette-results.ts +++ b/src/renderer/src/lib/workspace-tab-palette-results.ts @@ -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,