fix: address review findings (#4358)

This commit is contained in:
Jinjing
2026-05-31 22:33:20 -07:00
committed by GitHub
parent ae95fd84e1
commit 4e162dff4e
2 changed files with 84 additions and 11 deletions
@@ -3,7 +3,8 @@ import {
getInitialProjectAddedChoice,
getInitialProjectAddedWorktreeName,
getProjectAddedChoiceOrder,
getProjectAddedPrimaryBranchName
getProjectAddedPrimaryBranchName,
getSyncedProjectAddedChoice
} from './AddRepoSetupStep'
describe('getInitialProjectAddedWorktreeName', () => {
@@ -19,17 +20,36 @@ describe('getInitialProjectAddedWorktreeName', () => {
})
describe('getInitialProjectAddedChoice', () => {
it('defaults to creating a worktree when Orca found linked worktrees', () => {
expect(getInitialProjectAddedChoice(1)).toBe('create')
it('defaults to using existing worktrees when Orca found fewer than 10 linked worktrees', () => {
expect(getInitialProjectAddedChoice(1)).toBe('existing')
expect(getInitialProjectAddedChoice(9)).toBe('existing')
})
it('defaults to creating a worktree when no linked worktrees were found', () => {
expect(getInitialProjectAddedChoice(0)).toBe('create')
})
it('defaults to creating a worktree when the repo has a named primary branch', () => {
it('defaults to creating a worktree when Orca found 10 or more linked worktrees', () => {
expect(getInitialProjectAddedChoice(10)).toBe('create')
expect(getInitialProjectAddedChoice(11)).toBe('create')
})
it('keeps the discovered-worktree threshold when the repo has a named primary branch', () => {
expect(getInitialProjectAddedChoice(0, 'main')).toBe('create')
expect(getInitialProjectAddedChoice(2, 'main')).toBe('create')
expect(getInitialProjectAddedChoice(2, 'main')).toBe('existing')
expect(getInitialProjectAddedChoice(10, 'main')).toBe('create')
})
})
describe('getSyncedProjectAddedChoice', () => {
it('updates the default choice when worktree detection arrives later', () => {
expect(getSyncedProjectAddedChoice('create', false, 2)).toBe('existing')
expect(getSyncedProjectAddedChoice('existing', false, 10)).toBe('create')
})
it('preserves a manual selection when worktree detection changes', () => {
expect(getSyncedProjectAddedChoice('create', true, 2)).toBe('create')
expect(getSyncedProjectAddedChoice('existing', true, 10)).toBe('existing')
})
})
@@ -1,4 +1,4 @@
import React, { useCallback, useRef, useState } from 'react'
import React, { useCallback, useEffect, useRef, useState } from 'react'
import { GitBranch, GitBranchPlus, Settings } from 'lucide-react'
import { DialogHeader, DialogTitle, DialogDescription } from '@/components/ui/dialog'
import { Button } from '@/components/ui/button'
@@ -21,6 +21,7 @@ type ProjectAddedContentProps = {
type SetupStepProps = ProjectAddedContentProps
const DEFAULT_PROJECT_ADDED_WORKTREE_NAME = 'new-workspace-1'
const EXISTING_WORKTREE_DEFAULT_LIMIT = 10
export function getInitialProjectAddedWorktreeName(
defaultWorktreeName: string | undefined
@@ -50,12 +51,29 @@ export function getProjectAddedChoiceOrder(
}
export function getInitialProjectAddedChoice(
_hiddenWorktreeCount: number,
hiddenWorktreeCount: number,
_primaryBranchName?: string
): ProjectAddedChoice {
// Why: a small discovered set is usually worth importing directly; larger
// repos are noisy enough that starting fresh should remain the default.
if (hiddenWorktreeCount > 0 && hiddenWorktreeCount < EXISTING_WORKTREE_DEFAULT_LIMIT) {
return 'existing'
}
return 'create'
}
export function getSyncedProjectAddedChoice(
currentChoice: ProjectAddedChoice,
choiceManuallySelected: boolean,
hiddenWorktreeCount: number,
primaryBranchName?: string
): ProjectAddedChoice {
if (choiceManuallySelected) {
return currentChoice
}
return getInitialProjectAddedChoice(hiddenWorktreeCount, primaryBranchName)
}
function formatWorktreeCount(count: number): string {
return `${count} ${count === 1 ? 'worktree' : 'worktrees'}`
}
@@ -142,6 +160,7 @@ export function ProjectAddedContent({
const [choice, setChoice] = useState<ProjectAddedChoice>(() =>
getInitialProjectAddedChoice(hiddenWorktreeCount, primaryBranchName)
)
const [choiceManuallySelected, setChoiceManuallySelected] = useState(false)
const radioGroupRef = useRef<HTMLDivElement>(null)
const radioFocusFrameRef = useRef<number | null>(null)
const trimmedName = worktreeName.trim()
@@ -150,6 +169,19 @@ export function ProjectAddedContent({
const choices = getProjectAddedChoiceOrder(hiddenWorktreeCount, normalizedPrimaryBranchName)
const selectedChoice = choices.includes(choice) ? choice : (choices[0] ?? 'create')
useEffect(() => {
// Why: hidden worktree detection can finish after this step mounts, but
// should not override an explicit user choice.
setChoice((currentChoice) =>
getSyncedProjectAddedChoice(
currentChoice,
choiceManuallySelected,
hiddenWorktreeCount,
primaryBranchName
)
)
}, [choiceManuallySelected, hiddenWorktreeCount, primaryBranchName])
const cancelRadioFocusFrame = useCallback((): void => {
if (radioFocusFrameRef.current === null) {
return
@@ -172,6 +204,7 @@ export function ProjectAddedContent({
const cycleChoice = useCallback(() => {
const index = choices.indexOf(selectedChoice)
const nextChoice = choices[(index + 1) % choices.length] ?? 'create'
setChoiceManuallySelected(true)
setChoice(nextChoice)
cancelRadioFocusFrame()
radioFocusFrameRef.current = requestAnimationFrame(() => {
@@ -182,6 +215,13 @@ export function ProjectAddedContent({
})
}, [cancelRadioFocusFrame, choices, selectedChoice])
const markCreateChoiceSelected = useCallback(() => {
// Why: editing the name means the user has committed to the create flow,
// even if hidden worktree detection finishes afterward.
setChoiceManuallySelected(true)
setChoice('create')
}, [])
const handlePrimaryAction = (): void => {
if (selectedChoice === 'primary') {
onStartPrimaryWorktree()
@@ -216,7 +256,10 @@ export function ProjectAddedContent({
<StartChoiceCard
value="primary"
selected={selectedChoice === 'primary'}
onSelect={() => setChoice('primary')}
onSelect={() => {
setChoiceManuallySelected(true)
setChoice('primary')
}}
onArrowNav={cycleChoice}
icon={<GitBranch className="size-4" />}
title={`Start from ${normalizedPrimaryBranchName}`}
@@ -227,7 +270,10 @@ export function ProjectAddedContent({
<StartChoiceCard
value="existing"
selected={selectedChoice === 'existing'}
onSelect={() => setChoice('existing')}
onSelect={() => {
setChoiceManuallySelected(true)
setChoice('existing')
}}
onArrowNav={cycleChoice}
icon={<GitBranch className="size-4" />}
title="Use existing worktrees"
@@ -237,7 +283,10 @@ export function ProjectAddedContent({
<StartChoiceCard
value="create"
selected={selectedChoice === 'create'}
onSelect={() => setChoice('create')}
onSelect={() => {
setChoiceManuallySelected(true)
setChoice('create')
}}
onArrowNav={cycleChoice}
icon={<GitBranchPlus className="size-4" />}
title="Create a new worktree"
@@ -257,7 +306,11 @@ export function ProjectAddedContent({
<Input
id="project-added-worktree-name"
value={worktreeName}
onChange={(event) => setWorktreeName(event.target.value)}
onFocus={markCreateChoiceSelected}
onChange={(event) => {
markCreateChoiceSelected()
setWorktreeName(event.target.value)
}}
placeholder="new-workspace"
className="h-9"
/>