-
Notifications
You must be signed in to change notification settings - Fork 216
feat: Added the ability to browser different branches and tag from th… #822
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,231 @@ | ||
| 'use client'; | ||
|
|
||
| import * as React from "react"; | ||
| import { Check, ChevronDown, GitBranch, Search, Tag } from "lucide-react"; | ||
| import { useQuery } from "@tanstack/react-query"; | ||
| import { getRefs } from "@/app/api/(client)/client"; | ||
| import { isServiceError } from "@/lib/utils"; | ||
| import { cn } from "@/lib/utils"; | ||
| import { | ||
| DropdownMenu, | ||
| DropdownMenuContent, | ||
| DropdownMenuItem, | ||
| DropdownMenuTrigger, | ||
| } from "@/components/ui/dropdown-menu"; | ||
| import { Input } from "@/components/ui/input"; | ||
| import { ScrollArea } from "@/components/ui/scroll-area"; | ||
|
|
||
| interface BranchTagSelectorProps { | ||
| repoName: string; | ||
| currentRef: string; | ||
| onRefChange: (ref: string) => void; | ||
| } | ||
|
|
||
| export function BranchTagSelector({ repoName, currentRef, onRefChange }: BranchTagSelectorProps) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. actually, I wonder if it would be better to sort by the "date of last commit" for each ref. For example, when I was looking at the tags, my expectation here would have been to see newest versions at the top:
The tradeoff with this, however, is that related branch prefixes will no longer be grouped (e.g.,
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: this component should probably live in |
||
| const [open, setOpen] = React.useState(false); | ||
| const [activeTab, setActiveTab] = React.useState<'branches' | 'tags'>('branches'); | ||
| const [searchQuery, setSearchQuery] = React.useState(''); | ||
| const inputRef = React.useRef<HTMLInputElement>(null); | ||
|
|
||
| const { data: refsData, isLoading } = useQuery({ | ||
| queryKey: ['refs', repoName], | ||
| queryFn: async () => { | ||
| const result = await getRefs({ repoName }); | ||
| if (isServiceError(result)) { | ||
| throw new Error('Failed to fetch refs'); | ||
| } | ||
| return result; | ||
| }, | ||
| enabled: open || currentRef === 'HEAD', | ||
| }); | ||
|
Comment on lines
+30
to
+40
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Check if the file exists and read it
FILE="packages/web/src/app/[domain]/components/branchTagSelector.tsx"
if [ -f "$FILE" ]; then
wc -l "$FILE"
else
echo "File not found: $FILE"
exit 1
fiRepository: sourcebot-dev/sourcebot Length of output: 133 🏁 Script executed: #!/bin/bash
# Read the relevant sections of the file
FILE="packages/web/src/app/[domain]/components/branchTagSelector.tsx"
# Read lines 25-45 to see context around line 30-40
sed -n '25,45p' "$FILE"
echo "---"
# Read lines 155-170 to see context around line 160-165
sed -n '155,170p' "$FILE"Repository: sourcebot-dev/sourcebot Length of output: 1649 🏁 Script executed: #!/bin/bash
# Search for the useQuery call to see all destructured variables
FILE="packages/web/src/app/[domain]/components/branchTagSelector.tsx"
rg "useQuery" "$FILE" -A 10 -B 2Repository: sourcebot-dev/sourcebot Length of output: 990 🏁 Script executed: #!/bin/bash
FILE="packages/web/src/app/[domain]/components/branchTagSelector.tsx"
# Read the complete conditional rendering block
sed -n '157,180p' "$FILE"Repository: sourcebot-dev/sourcebot Length of output: 1501 🏁 Script executed: #!/bin/bash
FILE="packages/web/src/app/[domain]/components/branchTagSelector.tsx"
# Check the entire file for any error handling or isError references
rg "isError|error" "$FILE" -iRepository: sourcebot-dev/sourcebot Length of output: 211 Add error state handling to surface fetch failures. The Destructure 🔧 Proposed fix- const { data: refsData, isLoading } = useQuery({
+ const { data: refsData, isLoading, isError } = useQuery({
queryKey: ['refs', repoName],
queryFn: async () => {
const result = await getRefs({ repoName });
if (isServiceError(result)) {
throw new Error('Failed to fetch refs');
}
return result;
},
enabled: open || currentRef === 'HEAD',
});
@@
- {isLoading ? (
+ {isLoading ? (
<div className="p-4 text-sm text-gray-500 text-center">
Loading...
</div>
- ) : (
+ ) : isError ? (
+ <div className="p-4 text-sm text-red-600 text-center">
+ Failed to load branches/tags
+ </div>
+ ) : (
<div className="p-1">Also applies to the same rendering pattern at lines 160-165. 🤖 Prompt for AI Agents |
||
|
|
||
| // Filter refs based on search query | ||
| const filteredBranches = React.useMemo(() => { | ||
| const branches = refsData?.branches || []; | ||
| if (!searchQuery) return branches; | ||
| return branches.filter(branch => | ||
| branch.toLowerCase().includes(searchQuery.toLowerCase()) | ||
| ); | ||
| }, [refsData?.branches, searchQuery]); | ||
|
|
||
| const filteredTags = React.useMemo(() => { | ||
| const tags = refsData?.tags || []; | ||
| if (!searchQuery) return tags; | ||
| return tags.filter(tag => | ||
| tag.toLowerCase().includes(searchQuery.toLowerCase()) | ||
| ); | ||
| }, [refsData?.tags, searchQuery]); | ||
|
|
||
| const resolvedRef = currentRef === 'HEAD' ? (refsData?.defaultBranch || 'HEAD') : currentRef; | ||
| const displayRef = resolvedRef.replace(/^refs\/(heads|tags)\//, ''); | ||
|
|
||
| const handleRefSelect = (ref: string) => { | ||
| onRefChange(ref); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As discussed in that other thread, we will want to prefix either |
||
| setOpen(false); | ||
| setSearchQuery(''); | ||
| }; | ||
|
|
||
| // Prevent dropdown items from stealing focus while user is typing | ||
| const handleItemFocus = (e: React.FocusEvent) => { | ||
| if (searchQuery) { | ||
| e.preventDefault(); | ||
| inputRef.current?.focus(); | ||
| } | ||
| }; | ||
|
|
||
| // Keep focus on the search input when typing | ||
| React.useEffect(() => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this looks..odd - do we need this? |
||
| if (open && searchQuery && inputRef.current) { | ||
| const timeoutId = setTimeout(() => { | ||
| inputRef.current?.focus(); | ||
| }, 0); | ||
| return () => clearTimeout(timeoutId); | ||
| } | ||
| }, [open, searchQuery]); | ||
|
|
||
| return ( | ||
| <DropdownMenu open={open} onOpenChange={setOpen}> | ||
| <DropdownMenuTrigger asChild> | ||
| <button | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: if possible, please use the button component s.t., we maintain visual consistency |
||
| className="flex items-center gap-1.5 px-2 py-1 text-xs font-semibold text-gray-700 hover:bg-gray-100 rounded-md transition-colors" | ||
| aria-label="Switch branches or tags" | ||
| > | ||
| <GitBranch className="h-3.5 w-3.5 flex-shrink-0" /> | ||
| <span className="truncate max-w-[150px]">{displayRef}</span> | ||
| <ChevronDown className="h-3.5 w-3.5 text-gray-500 flex-shrink-0" /> | ||
| </button> | ||
| </DropdownMenuTrigger> | ||
| <DropdownMenuContent | ||
| align="start" | ||
| className="w-[320px] p-0" | ||
| onCloseAutoFocus={(e) => e.preventDefault()} | ||
| > | ||
| <div className="flex flex-col" onKeyDown={(e) => { | ||
| // Prevent dropdown keyboard navigation from interfering with search input | ||
| if (e.key === 'ArrowDown' || e.key === 'ArrowUp') { | ||
| e.stopPropagation(); | ||
| } | ||
| }}> | ||
| {/* Search input */} | ||
| <div className="p-2 border-b"> | ||
| <div className="relative"> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: a input group would be helpful here for placing the search icon next to the input box. |
||
| <Search className="absolute left-2 top-1/2 -translate-y-1/2 h-4 w-4 text-gray-400" /> | ||
| <Input | ||
| ref={inputRef} | ||
| type="text" | ||
| placeholder={activeTab === 'branches' ? "Find a branch..." : "Find a tag..."} | ||
| value={searchQuery} | ||
| onChange={(e) => setSearchQuery(e.target.value)} | ||
| onKeyDown={(e) => { | ||
| // Prevent dropdown menu keyboard navigation | ||
| e.stopPropagation(); | ||
| }} | ||
| className="pl-8 h-8 text-sm" | ||
| autoFocus | ||
| /> | ||
| </div> | ||
| </div> | ||
|
|
||
| <div className="flex border-b"> | ||
| <button | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: if possible, please use the button component s.t., we maintain visual consistency
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Alternatively, might make sense to use a toggle group here? |
||
| onClick={() => setActiveTab('branches')} | ||
| className={cn( | ||
| "flex-1 px-4 py-2 text-sm font-medium transition-colors border-b-2", | ||
| activeTab === 'branches' | ||
| ? "border-blue-600 text-blue-600" | ||
| : "border-transparent text-gray-600 hover:text-gray-900" | ||
| )} | ||
| > | ||
| <div className="flex items-center justify-center gap-1.5"> | ||
| <GitBranch className="h-4 w-4" /> | ||
| Branches | ||
| </div> | ||
| </button> | ||
| <button | ||
| onClick={() => setActiveTab('tags')} | ||
| className={cn( | ||
| "flex-1 px-4 py-2 text-sm font-medium transition-colors border-b-2", | ||
| activeTab === 'tags' | ||
| ? "border-blue-600 text-blue-600" | ||
| : "border-transparent text-gray-600 hover:text-gray-900" | ||
| )} | ||
| > | ||
| <div className="flex items-center justify-center gap-1.5"> | ||
| <Tag className="h-4 w-4" /> | ||
| Tags | ||
| </div> | ||
| </button> | ||
| </div> | ||
|
|
||
| <ScrollArea className="h-[300px]"> | ||
| {isLoading ? ( | ||
| <div className="p-4 text-sm text-gray-500 text-center"> | ||
| Loading... | ||
| </div> | ||
| ) : ( | ||
| <div className="p-1"> | ||
| {activeTab === 'branches' && ( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: I feel like this menu item code can be broken out into a child component with a |
||
| <> | ||
| {filteredBranches.length === 0 ? ( | ||
| <div className="p-4 text-sm text-gray-500 text-center"> | ||
| {searchQuery ? 'No branches found' : 'No branches available'} | ||
| </div> | ||
| ) : ( | ||
| filteredBranches.map((branch) => ( | ||
| <DropdownMenuItem | ||
| key={branch} | ||
| onClick={() => handleRefSelect(branch)} | ||
| onFocus={handleItemFocus} | ||
| className="flex items-center justify-between px-3 py-2 cursor-pointer" | ||
| > | ||
| <div className="flex items-center gap-2 flex-1 min-w-0"> | ||
| <GitBranch className="h-4 w-4 text-gray-500 flex-shrink-0" /> | ||
| <span className="truncate text-sm">{branch}</span> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could you attach a screenshot of what this looks like with a really long branch name where truncation would occur? |
||
| {branch === refsData?.defaultBranch && ( | ||
| <span className="text-xs bg-blue-100 text-blue-700 dark:bg-blue-900 dark:text-blue-300 px-1.5 py-0.5 rounded font-medium flex-shrink-0"> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: please use the Badge component s.t., we maintain visual consistency. |
||
| default | ||
| </span> | ||
| )} | ||
| </div> | ||
| {branch === resolvedRef && ( | ||
| <Check className="h-4 w-4 text-blue-600 flex-shrink-0" /> | ||
| )} | ||
| </DropdownMenuItem> | ||
| )) | ||
| )} | ||
| </> | ||
| )} | ||
| {activeTab === 'tags' && ( | ||
| <> | ||
| {filteredTags.length === 0 ? ( | ||
| <div className="p-4 text-sm text-gray-500 text-center"> | ||
| {searchQuery ? 'No tags found' : 'No tags available'} | ||
| </div> | ||
| ) : ( | ||
| filteredTags.map((tag) => ( | ||
| <DropdownMenuItem | ||
| key={tag} | ||
| onClick={() => handleRefSelect(tag)} | ||
| onFocus={handleItemFocus} | ||
| className="flex items-center justify-between px-3 py-2 cursor-pointer" | ||
| > | ||
| <div className="flex items-center gap-2 flex-1 min-w-0"> | ||
| <Tag className="h-4 w-4 text-gray-500 flex-shrink-0" /> | ||
| <span className="truncate text-sm">{tag}</span> | ||
| </div> | ||
| {tag === resolvedRef && ( | ||
| <Check className="h-4 w-4 text-blue-600 flex-shrink-0" /> | ||
| )} | ||
| </DropdownMenuItem> | ||
| )) | ||
| )} | ||
| </> | ||
| )} | ||
| </div> | ||
| )} | ||
| </ScrollArea> | ||
| </div> | ||
| </DropdownMenuContent> | ||
| </DropdownMenu> | ||
| ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -57,7 +57,7 @@ export const PathHeader = ({ | |
| revisionName, | ||
| branchDisplayName = revisionName, | ||
| isBranchDisplayNameVisible = !!branchDisplayName, | ||
| branchDisplayTitle, | ||
| branchDisplayTitle: _branchDisplayTitle, | ||
| pathType = 'blob', | ||
| isCodeHostIconVisible = true, | ||
| isFileIconVisible = true, | ||
|
|
@@ -231,7 +231,6 @@ export const PathHeader = ({ | |
| {(isBranchDisplayNameVisible && branchDisplayName) && ( | ||
| <p | ||
| className="text-xs font-semibold text-gray-500 dark:text-gray-400 mt-[3px] flex items-center gap-0.5" | ||
| title={branchDisplayTitle} | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is there a reason you are removing this? |
||
| style={{ | ||
| marginBottom: "0.1rem", | ||
| }} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| import 'server-only'; | ||
|
|
||
| import { sew } from '@/actions'; | ||
| import { notFound, unexpectedError } from '@/lib/serviceError'; | ||
| import { withOptionalAuthV2 } from '@/withAuthV2'; | ||
| import { createLogger, getRepoPath, repoMetadataSchema } from '@sourcebot/shared'; | ||
| import { simpleGit } from 'simple-git'; | ||
|
|
||
| const logger = createLogger('refs'); | ||
|
|
||
| export const getRefs = async (params: { repoName: string }) => sew(() => | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMO, we shouldn't limit this to just branches that you have indexed, and instead allow users to view any branch. Mentioned in #461 (comment), we are planning on supporting search over any branch. |
||
| withOptionalAuthV2(async ({ org, prisma }) => { | ||
| const { repoName } = params; | ||
| const repo = await prisma.repo.findFirst({ | ||
| where: { | ||
| name: repoName, | ||
| orgId: org.id, | ||
| }, | ||
| }); | ||
|
|
||
| if (!repo) { | ||
| return notFound(); | ||
| } | ||
|
|
||
| const metadata = repoMetadataSchema.safeParse(repo.metadata); | ||
| const indexedRevisions = metadata.success ? (metadata.data.indexedRevisions || []) : []; | ||
|
|
||
| const { path: repoPath } = getRepoPath(repo); | ||
|
|
||
| const git = simpleGit().cwd(repoPath); | ||
|
|
||
| let allBranches: string[] = []; | ||
| let allTags: string[] = []; | ||
| let defaultBranch: string | null = null; | ||
|
|
||
| try { | ||
| const branchResult = await git.branch(); | ||
| allBranches = branchResult.all; | ||
| defaultBranch = branchResult.current || null; | ||
| } catch (error) { | ||
| logger.error('git branch failed.', { error }); | ||
| return unexpectedError('git branch command failed.'); | ||
| } | ||
|
|
||
| try { | ||
| const tagResult = await git.tags(); | ||
| allTags = tagResult.all; | ||
| } catch (error) { | ||
| logger.error('git tags failed.', { error }); | ||
| return unexpectedError('git tags command failed.'); | ||
| } | ||
|
|
||
| const indexedRefsSet = new Set(indexedRevisions); | ||
|
|
||
| const branches = allBranches.filter(branch => { | ||
| return indexedRefsSet.has(`refs/heads/${branch}`); | ||
| }); | ||
|
|
||
| const tags = allTags.filter(tag => { | ||
| return indexedRefsSet.has(`refs/tags/${tag}`); | ||
| }); | ||
|
|
||
| return { | ||
| branches, | ||
| tags, | ||
| defaultBranch, | ||
| }; | ||
| })); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| 'use server'; | ||
|
|
||
| import { getRefsRequestSchema } from "@/lib/schemas"; | ||
| import { requestBodySchemaValidationError, serviceErrorResponse } from "@/lib/serviceError"; | ||
| import { isServiceError } from "@/lib/utils"; | ||
| import { NextRequest } from "next/server"; | ||
| import { getRefs } from "./getRefs"; | ||
|
|
||
| export const POST = async (request: NextRequest) => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: this should be a GET request, similar to what we are doing with |
||
| const body = await request.json(); | ||
| const parsed = await getRefsRequestSchema.safeParseAsync(body); | ||
| if (!parsed.success) { | ||
| return serviceErrorResponse(requestBodySchemaValidationError(parsed.error)); | ||
| } | ||
|
|
||
| const response = await getRefs(parsed.data); | ||
| if (isServiceError(response)) { | ||
| return serviceErrorResponse(response); | ||
| } | ||
|
|
||
| return Response.json(response); | ||
|
Comment on lines
+9
to
+21
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: # First, let's understand the codebase structure and look for error handling patterns
find packages/web -name "*.ts" -o -name "*.tsx" | head -20Repository: sourcebot-dev/sourcebot Length of output: 1265 🏁 Script executed: # Search for middleware or error handling related to API routes
rg -t ts "middleware|error.*handler|catch.*json\(\)" packages/web/src --max-count 20Repository: sourcebot-dev/sourcebot Length of output: 631 🏁 Script executed: # Check the specific file and its surrounding context
cat -n packages/web/src/app/api/\(server\)/refs/route.tsRepository: sourcebot-dev/sourcebot Length of output: 966 🏁 Script executed: # Look for other API routes to understand error handling patterns
fd -t f "route.ts" packages/web/src/app/api --exec cat -n {} \;Repository: sourcebot-dev/sourcebot Length of output: 50044 🏁 Script executed: # Check if there's a global error handler or middleware setup
rg -t ts "error.*Middleware|Error.*Handler|catch.*Error" packages/web/src/app --max-count 15Repository: sourcebot-dev/sourcebot Length of output: 145 Wrap
🔧 Proposed fix export const POST = async (request: NextRequest) => {
- const body = await request.json();
+ let body: unknown = null;
+ try {
+ body = await request.json();
+ } catch {
+ // fall through to schema validation error
+ }
const parsed = await getRefsRequestSchema.safeParseAsync(body);
if (!parsed.success) {
return serviceErrorResponse(requestBodySchemaValidationError(parsed.error));
}Note: This pattern should be applied to all POST/PUT endpoints that call 🤖 Prompt for AI Agents |
||
| } | ||



There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Design feedback: it would be great if we could get better keyboard navigation support for this component. My expectation as a user is that using the up/down arrow keys or tab / shift+tab will allow me to navigate up and down the branch list:
Area.mp4