fix(sidebar): order repo headers by recency in Recent/Smart sort
This commit is contained in:
parent
9217cf625d
commit
ac1a4e44e9
|
|
@ -30,10 +30,12 @@ import { track } from '@/lib/telemetry'
|
|||
import { tabHasLivePty } from '@/lib/tab-has-live-pty'
|
||||
import {
|
||||
type GroupHeaderRow,
|
||||
type RepoGroupOrdering,
|
||||
type Row,
|
||||
type WorktreeGroupBy,
|
||||
PINNED_GROUP_KEY,
|
||||
buildRows,
|
||||
getRepoGroupOrdering,
|
||||
getGroupKeyForWorktree
|
||||
} from './worktree-list-groups'
|
||||
import {
|
||||
|
|
@ -93,6 +95,7 @@ type VirtualizedWorktreeViewportProps = {
|
|||
rows: Row[]
|
||||
activeWorktreeId: string | null
|
||||
groupBy: WorktreeGroupBy
|
||||
repoGroupOrdering: RepoGroupOrdering
|
||||
toggleGroup: (key: string) => void
|
||||
collapsedGroups: Set<string>
|
||||
handleCreateForRepo: (repoId: string) => void
|
||||
|
|
@ -132,6 +135,7 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp
|
|||
rows,
|
||||
activeWorktreeId,
|
||||
groupBy,
|
||||
repoGroupOrdering,
|
||||
toggleGroup,
|
||||
collapsedGroups,
|
||||
handleCreateForRepo,
|
||||
|
|
@ -156,10 +160,10 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp
|
|||
const scrollRef = useRef<HTMLDivElement>(null)
|
||||
const [dragOverStatus, setDragOverStatus] = useState<WorkspaceStatus | null>(null)
|
||||
const [pinDragOver, setPinDragOver] = useState(false)
|
||||
const canReorderRepoHeaders = groupBy === 'repo' && repoGroupOrdering === 'manual'
|
||||
|
||||
// Drag is only meaningful when the user is grouping by repo. When inert
|
||||
// (groupBy !== 'repo'), the controller is still constructed for hook order
|
||||
// stability but the handle is never rendered.
|
||||
// Drag is only meaningful when repo headers are using manual order. The
|
||||
// controller is still constructed for hook order stability when inert.
|
||||
const repoDrag = useRepoHeaderDrag({
|
||||
orderedRepoIds: allRepoIds,
|
||||
onCommit: reorderRepos,
|
||||
|
|
@ -321,7 +325,8 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp
|
|||
prCache,
|
||||
new Set<string>(),
|
||||
repoOrder,
|
||||
workspaceStatuses
|
||||
workspaceStatuses,
|
||||
repoGroupOrdering
|
||||
).filter((r): r is Extract<Row, { type: 'item' }> => r.type === 'item')
|
||||
if (worktreeRows.length === 0) {
|
||||
return
|
||||
|
|
@ -359,6 +364,7 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp
|
|||
activeWorktreeId,
|
||||
virtualizer,
|
||||
groupBy,
|
||||
repoGroupOrdering,
|
||||
worktrees,
|
||||
repoMap,
|
||||
prCache,
|
||||
|
|
@ -511,7 +517,9 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp
|
|||
className="relative w-full"
|
||||
style={{ height: `${virtualizer.getTotalSize()}px` }}
|
||||
>
|
||||
{repoDrag.state.draggingRepoId !== null && repoDrag.state.dropIndicatorY !== null ? (
|
||||
{canReorderRepoHeaders &&
|
||||
repoDrag.state.draggingRepoId !== null &&
|
||||
repoDrag.state.dropIndicatorY !== null ? (
|
||||
<div
|
||||
role="presentation"
|
||||
className="pointer-events-none absolute left-2 right-2 z-10 border-t border-dashed border-muted-foreground/70"
|
||||
|
|
@ -525,6 +533,7 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp
|
|||
const isRepoHeader = groupBy === 'repo' && row.repo !== undefined
|
||||
const repoIdForHeader = isRepoHeader ? row.repo!.id : undefined
|
||||
const isDraggingThis =
|
||||
canReorderRepoHeaders &&
|
||||
repoDrag.state.draggingRepoId !== null &&
|
||||
repoDrag.state.draggingRepoId === repoIdForHeader
|
||||
const headerWorkspaceStatus =
|
||||
|
|
@ -592,7 +601,7 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp
|
|||
{row.icon ? (
|
||||
<div
|
||||
onPointerDown={
|
||||
isRepoHeader && repoIdForHeader
|
||||
canReorderRepoHeaders && isRepoHeader && repoIdForHeader
|
||||
? (e) => repoDrag.onHandlePointerDown(e, repoIdForHeader)
|
||||
: undefined
|
||||
}
|
||||
|
|
@ -1013,9 +1022,8 @@ const WorktreeList = React.memo(function WorktreeList() {
|
|||
const collapsedGroups = useAppStore((s) => s.collapsedGroups)
|
||||
const toggleGroup = useAppStore((s) => s.toggleCollapsedGroup)
|
||||
|
||||
// Why: header order in groupBy='repo' is bound to state.repos array order so
|
||||
// manual reorder is the single source of truth. The Map lets buildRows do an
|
||||
// O(1) rank lookup per group without depending on Repo identity.
|
||||
// Why: manual repo header order is bound to state.repos. Recent/Smart derive
|
||||
// header order from the sorted visible worktree stream instead.
|
||||
const repos = useAppStore((s) => s.repos)
|
||||
const repoOrder = useMemo(() => {
|
||||
const map = new Map<string, number>()
|
||||
|
|
@ -1024,6 +1032,7 @@ const WorktreeList = React.memo(function WorktreeList() {
|
|||
}, [repos])
|
||||
const allRepoIds = useMemo(() => repos.map((r) => r.id), [repos])
|
||||
const reorderReposAction = useAppStore((s) => s.reorderRepos)
|
||||
const repoGroupOrdering = getRepoGroupOrdering(groupBy, sortBy)
|
||||
|
||||
// Build flat row list for rendering
|
||||
const rows: Row[] = useMemo(
|
||||
|
|
@ -1035,9 +1044,19 @@ const WorktreeList = React.memo(function WorktreeList() {
|
|||
prCache,
|
||||
collapsedGroups,
|
||||
repoOrder,
|
||||
workspaceStatuses
|
||||
workspaceStatuses,
|
||||
repoGroupOrdering
|
||||
),
|
||||
[groupBy, worktrees, repoMap, prCache, collapsedGroups, repoOrder, workspaceStatuses]
|
||||
[
|
||||
groupBy,
|
||||
worktrees,
|
||||
repoMap,
|
||||
prCache,
|
||||
collapsedGroups,
|
||||
repoOrder,
|
||||
workspaceStatuses,
|
||||
repoGroupOrdering
|
||||
]
|
||||
)
|
||||
// Why: rows.length alone can stay the same when items migrate between
|
||||
// groups (e.g., PR cache loads on restart and a collapsed group absorbs
|
||||
|
|
@ -1240,6 +1259,7 @@ const WorktreeList = React.memo(function WorktreeList() {
|
|||
rows={rows}
|
||||
activeWorktreeId={selectedSidebarWorktreeId}
|
||||
groupBy={groupBy}
|
||||
repoGroupOrdering={repoGroupOrdering}
|
||||
toggleGroup={toggleGroup}
|
||||
collapsedGroups={collapsedGroups}
|
||||
handleCreateForRepo={handleCreateForRepo}
|
||||
|
|
|
|||
|
|
@ -6,7 +6,7 @@ import type {
|
|||
import { tabHasLivePty } from '@/lib/tab-has-live-pty'
|
||||
import { IDLE, buildAttentionByWorktree, type WorktreeAttention } from './smart-attention'
|
||||
|
||||
type SortBy = 'name' | 'smart' | 'recent' | 'repo'
|
||||
export type SortBy = 'name' | 'smart' | 'recent' | 'repo'
|
||||
|
||||
// Why: a newly-created worktree's lastActivityAt is stamped at the moment
|
||||
// createLocalWorktree finishes git + setup-runner prep (often several seconds
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
import { readFileSync } from 'node:fs'
|
||||
import { fileURLToPath } from 'node:url'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { buildRows, getPRGroupKey } from './worktree-list-groups'
|
||||
import { buildRows, getPRGroupKey, getRepoGroupOrdering } from './worktree-list-groups'
|
||||
import type { Repo, Worktree } from '../../../../shared/types'
|
||||
|
||||
const repo: Repo = {
|
||||
|
|
@ -209,6 +209,7 @@ describe('buildRows repo grouping order', () => {
|
|||
[repoC.id, repoC]
|
||||
])
|
||||
const wA: Worktree = { ...worktree, id: 'wt-a', repoId: repoA.id, displayName: 'a' }
|
||||
const wAStale: Worktree = { ...worktree, id: 'wt-a-stale', repoId: repoA.id, displayName: 'a2' }
|
||||
const wB: Worktree = { ...worktree, id: 'wt-b', repoId: repoB.id, displayName: 'b' }
|
||||
const wC: Worktree = { ...worktree, id: 'wt-c', repoId: repoC.id, displayName: 'c' }
|
||||
|
||||
|
|
@ -231,6 +232,82 @@ describe('buildRows repo grouping order', () => {
|
|||
const headerKeys = rows.filter((r) => r.type === 'header').map((r) => r.key)
|
||||
expect(headerKeys).toEqual(['repo:repo-b', 'repo:repo-a', 'repo:repo-c'])
|
||||
})
|
||||
|
||||
it('orders repo headers by first encounter when caller uses visible worktree order', () => {
|
||||
// Caller already sorted worktrees by recency: C is freshest, then A, then B.
|
||||
// Even though repoOrder pins B, A, C, dynamic sorts must follow the freshest
|
||||
// worktree out of each repo so a just-active worktree's parent group
|
||||
// bubbles to the top of the sidebar.
|
||||
const repoOrder = new Map([
|
||||
[repoB.id, 0],
|
||||
[repoA.id, 1],
|
||||
[repoC.id, 2]
|
||||
])
|
||||
const rows = buildRows(
|
||||
'repo',
|
||||
[wC, wA, wB],
|
||||
map,
|
||||
null,
|
||||
new Set(),
|
||||
repoOrder,
|
||||
undefined,
|
||||
'visible-worktree-order'
|
||||
)
|
||||
const headerKeys = rows.filter((r) => r.type === 'header').map((r) => r.key)
|
||||
expect(headerKeys).toEqual(['repo:repo-c', 'repo:repo-a', 'repo:repo-b'])
|
||||
})
|
||||
|
||||
it('orders repo headers by each repo highest-ranked visible child', () => {
|
||||
const repoOrder = new Map([
|
||||
[repoB.id, 0],
|
||||
[repoA.id, 1],
|
||||
[repoC.id, 2]
|
||||
])
|
||||
const rows = buildRows(
|
||||
'repo',
|
||||
[wA, wB, wAStale, wC],
|
||||
map,
|
||||
null,
|
||||
new Set(),
|
||||
repoOrder,
|
||||
undefined,
|
||||
'visible-worktree-order'
|
||||
)
|
||||
|
||||
expect(rows).toMatchObject([
|
||||
{ type: 'header', key: 'repo:repo-a' },
|
||||
{ type: 'item', worktree: { id: 'wt-a' } },
|
||||
{ type: 'item', worktree: { id: 'wt-a-stale' } },
|
||||
{ type: 'header', key: 'repo:repo-b' },
|
||||
{ type: 'item', worktree: { id: 'wt-b' } },
|
||||
{ type: 'header', key: 'repo:repo-c' },
|
||||
{ type: 'item', worktree: { id: 'wt-c' } }
|
||||
])
|
||||
})
|
||||
|
||||
it('keeps repoOrder for manual repo group ordering', () => {
|
||||
const repoOrder = new Map([
|
||||
[repoB.id, 0],
|
||||
[repoA.id, 1],
|
||||
[repoC.id, 2]
|
||||
])
|
||||
const rows = buildRows('repo', [wC, wA, wB], map, null, new Set(), repoOrder)
|
||||
const headerKeys = rows.filter((r) => r.type === 'header').map((r) => r.key)
|
||||
expect(headerKeys).toEqual(['repo:repo-b', 'repo:repo-a', 'repo:repo-c'])
|
||||
})
|
||||
})
|
||||
|
||||
describe('getRepoGroupOrdering', () => {
|
||||
it.each([
|
||||
['repo', 'recent', 'visible-worktree-order'],
|
||||
['repo', 'smart', 'visible-worktree-order'],
|
||||
['repo', 'name', 'manual'],
|
||||
['repo', 'repo', 'manual'],
|
||||
['none', 'recent', 'manual'],
|
||||
['pr-status', 'recent', 'manual']
|
||||
] as const)('uses %s/%s -> %s', (groupBy, sortBy, expected) => {
|
||||
expect(getRepoGroupOrdering(groupBy, sortBy)).toBe(expected)
|
||||
})
|
||||
})
|
||||
|
||||
describe('WorktreeList header styles', () => {
|
||||
|
|
|
|||
|
|
@ -9,10 +9,18 @@ import {
|
|||
getWorkspaceStatusVisualMeta
|
||||
} from './workspace-status'
|
||||
import { cloneDefaultWorkspaceStatuses } from '../../../../shared/workspace-statuses'
|
||||
import type { SortBy } from './smart-sort'
|
||||
|
||||
export { branchName }
|
||||
|
||||
export type WorktreeGroupBy = 'none' | 'repo' | 'pr-status'
|
||||
export type RepoGroupOrdering = 'manual' | 'visible-worktree-order'
|
||||
|
||||
export function getRepoGroupOrdering(groupBy: WorktreeGroupBy, sortBy: SortBy): RepoGroupOrdering {
|
||||
return groupBy === 'repo' && (sortBy === 'recent' || sortBy === 'smart')
|
||||
? 'visible-worktree-order'
|
||||
: 'manual'
|
||||
}
|
||||
|
||||
export type GroupHeaderRow = {
|
||||
type: 'header'
|
||||
|
|
@ -146,7 +154,8 @@ export function buildRows(
|
|||
prCache: Record<string, unknown> | null,
|
||||
collapsedGroups: Set<string>,
|
||||
repoOrder?: Map<string, number>,
|
||||
workspaceStatuses: readonly WorkspaceStatusDefinition[] = cloneDefaultWorkspaceStatuses()
|
||||
workspaceStatuses: readonly WorkspaceStatusDefinition[] = cloneDefaultWorkspaceStatuses(),
|
||||
repoGroupOrdering: RepoGroupOrdering = 'manual'
|
||||
): Row[] {
|
||||
const result: Row[] = []
|
||||
|
||||
|
|
@ -199,13 +208,11 @@ export function buildRows(
|
|||
}
|
||||
}
|
||||
} else {
|
||||
// Why: header order must follow the canonical state.repos array order, not
|
||||
// first-encounter from the smart-sorted worktree stream — otherwise sorting
|
||||
// or filtering side effects could shuffle which repo header appears first,
|
||||
// and manual reorder would have nothing to bind to. Unknown ids (no entry
|
||||
// in repoOrder) sort last by label so they remain deterministic.
|
||||
// Why: dynamic sorts need repo headers to follow their highest-ranked
|
||||
// visible child. Manual ordering still uses the canonical state.repos
|
||||
// order so repo-header drag has a stable source of truth.
|
||||
const entries = Array.from(grouped.entries())
|
||||
if (repoOrder) {
|
||||
if (repoGroupOrdering === 'manual' && repoOrder) {
|
||||
const rankFor = (key: string): number => {
|
||||
const repoId = key.startsWith('repo:') ? key.slice('repo:'.length) : key
|
||||
const rank = repoOrder.get(repoId)
|
||||
|
|
|
|||
Loading…
Reference in New Issue