Skip to content

Commit afd8f0f

Browse files
committed
fix(review): address PR feedback - hooks, types, UX clarity
- Move useMemo before conditional returns (React hooks rule violation) - Disable empty state button when onAddClick handler not provided - Use discriminated union types for AddRepoForm controlled mode - Hide trigger button in controlled mode (parent provides trigger) - Rename 'try again' to 'use a different email' for clarity
1 parent 1b3295e commit afd8f0f

4 files changed

Lines changed: 55 additions & 28 deletions

File tree

frontend/src/components/AddRepoForm.tsx

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,21 +5,30 @@ import { Button } from '@/components/ui/button'
55
import { Input } from '@/components/ui/input'
66
import { Label } from '@/components/ui/label'
77

8-
interface AddRepoFormProps {
8+
// Discriminated union: if isOpen is provided, onOpenChange is required
9+
type UncontrolledProps = {
10+
isOpen?: undefined
11+
onOpenChange?: undefined
12+
}
13+
14+
type ControlledProps = {
15+
isOpen: boolean
16+
onOpenChange: (open: boolean) => void
17+
}
18+
19+
type AddRepoFormProps = {
920
onAdd: (gitUrl: string, branch: string) => Promise<void>
1021
loading: boolean
11-
isOpen?: boolean
12-
onOpenChange?: (open: boolean) => void
13-
}
22+
} & (UncontrolledProps | ControlledProps)
1423

1524
export function AddRepoForm({ onAdd, loading, isOpen, onOpenChange }: AddRepoFormProps) {
1625
const [gitUrl, setGitUrl] = useState('')
1726
const [branch, setBranch] = useState('main')
1827
const [internalOpen, setInternalOpen] = useState(false)
1928

20-
// Support both controlled and uncontrolled modes
21-
const showForm = isOpen !== undefined ? isOpen : internalOpen
22-
const setShowForm = onOpenChange || setInternalOpen
29+
const isControlled = isOpen !== undefined
30+
const showForm = isControlled ? isOpen : internalOpen
31+
const setShowForm = isControlled ? onOpenChange : setInternalOpen
2332

2433
const handleSubmit = async (e: React.FormEvent) => {
2534
e.preventDefault()
@@ -32,14 +41,17 @@ export function AddRepoForm({ onAdd, loading, isOpen, onOpenChange }: AddRepoFor
3241

3342
return (
3443
<>
35-
<Button
36-
onClick={() => setShowForm(true)}
37-
disabled={loading}
38-
className="bg-primary hover:bg-primary/90 text-primary-foreground gap-2"
39-
>
40-
<Plus className="w-4 h-4" />
41-
Add Repository
42-
</Button>
44+
{/* Only show trigger button in uncontrolled mode */}
45+
{!isControlled && (
46+
<Button
47+
onClick={() => setShowForm(true)}
48+
disabled={loading}
49+
className="bg-primary hover:bg-primary/90 text-primary-foreground gap-2"
50+
>
51+
<Plus className="w-4 h-4" />
52+
Add Repository
53+
</Button>
54+
)}
4355

4456
<AnimatePresence>
4557
{showForm && (

frontend/src/components/RepoList.tsx

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -96,17 +96,30 @@ const RepoCard = ({ repo, index, onSelect }: {
9696
}
9797

9898
export function RepoList({ repos, selectedRepo, onSelect, onAddClick, loading }: RepoListProps) {
99+
// Hooks must be called before any conditional returns
100+
const sortedRepos = useMemo(() => {
101+
return [...repos].sort((a, b) => {
102+
if (a.status === 'indexed' && b.status !== 'indexed') return -1
103+
if (b.status === 'indexed' && a.status !== 'indexed') return 1
104+
return (b.file_count || 0) - (a.file_count || 0)
105+
})
106+
}, [repos])
107+
99108
if (loading) return <RepoGridSkeleton count={3} />
100109

101110
if (repos.length === 0) {
111+
const isClickable = !!onAddClick
102112
return (
103113
<motion.button
104114
onClick={onAddClick}
115+
disabled={!isClickable}
105116
initial={{ opacity: 0 }}
106117
animate={{ opacity: 1 }}
107-
whileHover={{ scale: 1.01 }}
108-
whileTap={{ scale: 0.99 }}
109-
className="w-full bg-card border border-dashed border-border hover:border-primary/40 rounded-xl p-16 text-center transition-colors cursor-pointer focus:outline-none focus:ring-2 focus:ring-primary/50"
118+
whileHover={isClickable ? { scale: 1.01 } : undefined}
119+
whileTap={isClickable ? { scale: 0.99 } : undefined}
120+
className={`w-full bg-card border border-dashed border-border rounded-xl p-16 text-center transition-colors focus:outline-none focus:ring-2 focus:ring-primary/50 ${
121+
isClickable ? 'hover:border-primary/40 cursor-pointer' : 'cursor-default'
122+
}`}
110123
>
111124
<div className="w-14 h-14 mx-auto mb-4 rounded-xl bg-primary/10 border border-primary/20 flex items-center justify-center">
112125
<Plus className="w-6 h-6 text-primary" />
@@ -119,14 +132,6 @@ export function RepoList({ repos, selectedRepo, onSelect, onAddClick, loading }:
119132
)
120133
}
121134

122-
const sortedRepos = useMemo(() => {
123-
return [...repos].sort((a, b) => {
124-
if (a.status === 'indexed' && b.status !== 'indexed') return -1
125-
if (b.status === 'indexed' && a.status !== 'indexed') return 1
126-
return (b.file_count || 0) - (a.file_count || 0)
127-
})
128-
}, [repos])
129-
130135
return (
131136
<div className="grid grid-cols-1 md:grid-cols-2 lg:grid-cols-3 gap-4">
132137
{sortedRepos.map((repo, index) => (

frontend/src/components/auth/SignupForm.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ export function SignupForm() {
153153
onClick={() => setEmailSent(false)}
154154
className="text-primary hover:underline font-medium"
155155
>
156-
try again
156+
use a different email
157157
</button>
158158
</p>
159159
</div>

frontend/src/components/dashboard/DashboardHome.tsx

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,11 @@ import {
99
Zap,
1010
ArrowLeft,
1111
FolderGit2,
12-
ExternalLink
12+
ExternalLink,
13+
Plus
1314
} from 'lucide-react'
1415
import { useAuth } from '../../contexts/AuthContext'
16+
import { Button } from '../ui/button'
1517
import { RepoList } from '../RepoList'
1618
import { AddRepoForm } from '../AddRepoForm'
1719
import { SearchPanel } from '../SearchPanel'
@@ -137,6 +139,14 @@ export function DashboardHome() {
137139
Semantic code search powered by AI
138140
</p>
139141
</div>
142+
<Button
143+
onClick={() => setShowAddForm(true)}
144+
disabled={loading}
145+
className="bg-primary hover:bg-primary/90 text-primary-foreground gap-2"
146+
>
147+
<Plus className="w-4 h-4" />
148+
Add Repository
149+
</Button>
140150
<AddRepoForm
141151
onAdd={handleAddRepo}
142152
loading={loading}

0 commit comments

Comments
 (0)