fix: adress pr reviews

This commit is contained in:
Kacper
2025-12-23 21:07:36 +01:00
parent 4958ee1dda
commit e0c5f55fe7
14 changed files with 325 additions and 345 deletions

View File

@@ -0,0 +1,44 @@
import type { ReactElement, ReactNode } from 'react';
import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from '@/components/ui/tooltip';
interface TooltipWrapperProps {
/** The element to wrap with a tooltip */
children: ReactElement;
/** The content to display in the tooltip */
tooltipContent: ReactNode;
/** Whether to show the tooltip (if false, renders children without tooltip) */
showTooltip: boolean;
/** The side where the tooltip should appear */
side?: 'top' | 'right' | 'bottom' | 'left';
}
/**
* A reusable wrapper that conditionally adds a tooltip to its children.
* When showTooltip is false, it renders the children directly without any tooltip.
* This is useful for adding tooltips to disabled elements that need to show
* a reason for being disabled.
*/
export function TooltipWrapper({
children,
tooltipContent,
showTooltip,
side = 'left',
}: TooltipWrapperProps) {
if (!showTooltip) {
return children;
}
return (
<TooltipProvider>
<Tooltip>
<TooltipTrigger asChild>
{/* The div wrapper is necessary for tooltips to work on disabled elements */}
<div>{children}</div>
</TooltipTrigger>
<TooltipContent side={side}>
<p>{tooltipContent}</p>
</TooltipContent>
</Tooltip>
</TooltipProvider>
);
}

View File

@@ -7,7 +7,6 @@ import {
DropdownMenuTrigger,
DropdownMenuLabel,
} from '@/components/ui/dropdown-menu';
import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from '@/components/ui/tooltip';
import {
Trash2,
MoreHorizontal,
@@ -25,6 +24,7 @@ import {
} from 'lucide-react';
import { cn } from '@/lib/utils';
import type { WorktreeInfo, DevServerInfo, PRInfo, GitRepoStatus } from '../types';
import { TooltipWrapper } from './tooltip-wrapper';
interface WorktreeActionsDropdownProps {
worktree: WorktreeInfo;
@@ -146,92 +146,58 @@ export function WorktreeActionsDropdown({
<DropdownMenuSeparator />
</>
)}
<TooltipProvider>
<Tooltip>
<TooltipTrigger asChild>
<div>
<DropdownMenuItem
onClick={() => canPerformGitOps && onPull(worktree)}
disabled={isPulling || !canPerformGitOps}
className={cn('text-xs', !canPerformGitOps && 'opacity-50 cursor-not-allowed')}
>
<Download className={cn('w-3.5 h-3.5 mr-2', isPulling && 'animate-pulse')} />
{isPulling ? 'Pulling...' : 'Pull'}
{!canPerformGitOps && (
<AlertCircle className="w-3 h-3 ml-auto text-muted-foreground" />
)}
{canPerformGitOps && behindCount > 0 && (
<span className="ml-auto text-[10px] bg-muted px-1.5 py-0.5 rounded">
{behindCount} behind
</span>
)}
</DropdownMenuItem>
</div>
</TooltipTrigger>
{gitOpsDisabledReason && (
<TooltipContent side="left">
<p>{gitOpsDisabledReason}</p>
</TooltipContent>
<TooltipWrapper showTooltip={!!gitOpsDisabledReason} tooltipContent={gitOpsDisabledReason}>
<DropdownMenuItem
onClick={() => canPerformGitOps && onPull(worktree)}
disabled={isPulling || !canPerformGitOps}
className={cn('text-xs', !canPerformGitOps && 'opacity-50 cursor-not-allowed')}
>
<Download className={cn('w-3.5 h-3.5 mr-2', isPulling && 'animate-pulse')} />
{isPulling ? 'Pulling...' : 'Pull'}
{!canPerformGitOps && <AlertCircle className="w-3 h-3 ml-auto text-muted-foreground" />}
{canPerformGitOps && behindCount > 0 && (
<span className="ml-auto text-[10px] bg-muted px-1.5 py-0.5 rounded">
{behindCount} behind
</span>
)}
</Tooltip>
</TooltipProvider>
<TooltipProvider>
<Tooltip>
<TooltipTrigger asChild>
<div>
<DropdownMenuItem
onClick={() => canPerformGitOps && onPush(worktree)}
disabled={isPushing || aheadCount === 0 || !canPerformGitOps}
className={cn('text-xs', !canPerformGitOps && 'opacity-50 cursor-not-allowed')}
>
<Upload className={cn('w-3.5 h-3.5 mr-2', isPushing && 'animate-pulse')} />
{isPushing ? 'Pushing...' : 'Push'}
{!canPerformGitOps && (
<AlertCircle className="w-3 h-3 ml-auto text-muted-foreground" />
)}
{canPerformGitOps && aheadCount > 0 && (
<span className="ml-auto text-[10px] bg-primary/20 text-primary px-1.5 py-0.5 rounded">
{aheadCount} ahead
</span>
)}
</DropdownMenuItem>
</div>
</TooltipTrigger>
{gitOpsDisabledReason && (
<TooltipContent side="left">
<p>{gitOpsDisabledReason}</p>
</TooltipContent>
</DropdownMenuItem>
</TooltipWrapper>
<TooltipWrapper showTooltip={!!gitOpsDisabledReason} tooltipContent={gitOpsDisabledReason}>
<DropdownMenuItem
onClick={() => canPerformGitOps && onPush(worktree)}
disabled={isPushing || aheadCount === 0 || !canPerformGitOps}
className={cn('text-xs', !canPerformGitOps && 'opacity-50 cursor-not-allowed')}
>
<Upload className={cn('w-3.5 h-3.5 mr-2', isPushing && 'animate-pulse')} />
{isPushing ? 'Pushing...' : 'Push'}
{!canPerformGitOps && <AlertCircle className="w-3 h-3 ml-auto text-muted-foreground" />}
{canPerformGitOps && aheadCount > 0 && (
<span className="ml-auto text-[10px] bg-primary/20 text-primary px-1.5 py-0.5 rounded">
{aheadCount} ahead
</span>
)}
</Tooltip>
</TooltipProvider>
</DropdownMenuItem>
</TooltipWrapper>
{!worktree.isMain && (
<TooltipProvider>
<Tooltip>
<TooltipTrigger asChild>
<div>
<DropdownMenuItem
onClick={() => canPerformGitOps && onResolveConflicts(worktree)}
disabled={!canPerformGitOps}
className={cn(
'text-xs text-purple-500 focus:text-purple-600',
!canPerformGitOps && 'opacity-50 cursor-not-allowed'
)}
>
<GitMerge className="w-3.5 h-3.5 mr-2" />
Pull & Resolve Conflicts
{!canPerformGitOps && (
<AlertCircle className="w-3 h-3 ml-auto text-muted-foreground" />
)}
</DropdownMenuItem>
</div>
</TooltipTrigger>
{gitOpsDisabledReason && (
<TooltipContent side="left">
<p>{gitOpsDisabledReason}</p>
</TooltipContent>
<TooltipWrapper
showTooltip={!!gitOpsDisabledReason}
tooltipContent={gitOpsDisabledReason}
>
<DropdownMenuItem
onClick={() => canPerformGitOps && onResolveConflicts(worktree)}
disabled={!canPerformGitOps}
className={cn(
'text-xs text-purple-500 focus:text-purple-600',
!canPerformGitOps && 'opacity-50 cursor-not-allowed'
)}
</Tooltip>
</TooltipProvider>
>
<GitMerge className="w-3.5 h-3.5 mr-2" />
Pull & Resolve Conflicts
{!canPerformGitOps && (
<AlertCircle className="w-3 h-3 ml-auto text-muted-foreground" />
)}
</DropdownMenuItem>
</TooltipWrapper>
)}
<DropdownMenuSeparator />
<DropdownMenuItem onClick={() => onOpenInEditor(worktree)} className="text-xs">
@@ -240,60 +206,41 @@ export function WorktreeActionsDropdown({
</DropdownMenuItem>
<DropdownMenuSeparator />
{worktree.hasChanges && (
<TooltipProvider>
<Tooltip>
<TooltipTrigger asChild>
<div>
<DropdownMenuItem
onClick={() => gitRepoStatus.isGitRepo && onCommit(worktree)}
disabled={!gitRepoStatus.isGitRepo}
className={cn(
'text-xs',
!gitRepoStatus.isGitRepo && 'opacity-50 cursor-not-allowed'
)}
>
<GitCommit className="w-3.5 h-3.5 mr-2" />
Commit Changes
{!gitRepoStatus.isGitRepo && (
<AlertCircle className="w-3 h-3 ml-auto text-muted-foreground" />
)}
</DropdownMenuItem>
</div>
</TooltipTrigger>
<TooltipWrapper
showTooltip={!gitRepoStatus.isGitRepo}
tooltipContent="Not a git repository"
>
<DropdownMenuItem
onClick={() => gitRepoStatus.isGitRepo && onCommit(worktree)}
disabled={!gitRepoStatus.isGitRepo}
className={cn('text-xs', !gitRepoStatus.isGitRepo && 'opacity-50 cursor-not-allowed')}
>
<GitCommit className="w-3.5 h-3.5 mr-2" />
Commit Changes
{!gitRepoStatus.isGitRepo && (
<TooltipContent side="left">
<p>Not a git repository</p>
</TooltipContent>
<AlertCircle className="w-3 h-3 ml-auto text-muted-foreground" />
)}
</Tooltip>
</TooltipProvider>
</DropdownMenuItem>
</TooltipWrapper>
)}
{/* Show PR option for non-primary worktrees, or primary worktree with changes */}
{(!worktree.isMain || worktree.hasChanges) && !hasPR && (
<TooltipProvider>
<Tooltip>
<TooltipTrigger asChild>
<div>
<DropdownMenuItem
onClick={() => canPerformGitOps && onCreatePR(worktree)}
disabled={!canPerformGitOps}
className={cn('text-xs', !canPerformGitOps && 'opacity-50 cursor-not-allowed')}
>
<GitPullRequest className="w-3.5 h-3.5 mr-2" />
Create Pull Request
{!canPerformGitOps && (
<AlertCircle className="w-3 h-3 ml-auto text-muted-foreground" />
)}
</DropdownMenuItem>
</div>
</TooltipTrigger>
{gitOpsDisabledReason && (
<TooltipContent side="left">
<p>{gitOpsDisabledReason}</p>
</TooltipContent>
<TooltipWrapper
showTooltip={!!gitOpsDisabledReason}
tooltipContent={gitOpsDisabledReason}
>
<DropdownMenuItem
onClick={() => canPerformGitOps && onCreatePR(worktree)}
disabled={!canPerformGitOps}
className={cn('text-xs', !canPerformGitOps && 'opacity-50 cursor-not-allowed')}
>
<GitPullRequest className="w-3.5 h-3.5 mr-2" />
Create Pull Request
{!canPerformGitOps && (
<AlertCircle className="w-3 h-3 ml-auto text-muted-foreground" />
)}
</Tooltip>
</TooltipProvider>
</DropdownMenuItem>
</TooltipWrapper>
)}
{/* Show PR info and Address Comments button if PR exists */}
{!worktree.isMain && hasPR && worktree.pr && (

View File

@@ -13,49 +13,53 @@ export function useBranches() {
hasCommits: true,
});
const fetchBranches = useCallback(async (worktreePath: string) => {
setIsLoadingBranches(true);
try {
const api = getElectronAPI();
if (!api?.worktree?.listBranches) {
console.warn('List branches API not available');
return;
}
const result = await api.worktree.listBranches(worktreePath);
if (result.success && result.result) {
setBranches(result.result.branches);
setAheadCount(result.result.aheadCount || 0);
setBehindCount(result.result.behindCount || 0);
setGitRepoStatus({ isGitRepo: true, hasCommits: true });
} else if (result.code === 'NOT_GIT_REPO') {
// Not a git repository - clear branches silently without logging an error
setBranches([]);
setAheadCount(0);
setBehindCount(0);
setGitRepoStatus({ isGitRepo: false, hasCommits: false });
} else if (result.code === 'NO_COMMITS') {
// Git repo but no commits yet - clear branches silently without logging an error
setBranches([]);
setAheadCount(0);
setBehindCount(0);
setGitRepoStatus({ isGitRepo: true, hasCommits: false });
} else if (!result.success) {
// Other errors - log them
console.warn('Failed to fetch branches:', result.error);
setBranches([]);
setAheadCount(0);
setBehindCount(0);
}
} catch (error) {
console.error('Failed to fetch branches:', error);
setBranches([]);
setAheadCount(0);
setBehindCount(0);
} finally {
setIsLoadingBranches(false);
}
/** Helper to reset branch state to initial values */
const resetBranchState = useCallback(() => {
setBranches([]);
setAheadCount(0);
setBehindCount(0);
}, []);
const fetchBranches = useCallback(
async (worktreePath: string) => {
setIsLoadingBranches(true);
try {
const api = getElectronAPI();
if (!api?.worktree?.listBranches) {
console.warn('List branches API not available');
return;
}
const result = await api.worktree.listBranches(worktreePath);
if (result.success && result.result) {
setBranches(result.result.branches);
setAheadCount(result.result.aheadCount || 0);
setBehindCount(result.result.behindCount || 0);
setGitRepoStatus({ isGitRepo: true, hasCommits: true });
} else if (result.code === 'NOT_GIT_REPO') {
// Not a git repository - clear branches silently without logging an error
resetBranchState();
setGitRepoStatus({ isGitRepo: false, hasCommits: false });
} else if (result.code === 'NO_COMMITS') {
// Git repo but no commits yet - clear branches silently without logging an error
resetBranchState();
setGitRepoStatus({ isGitRepo: true, hasCommits: false });
} else if (!result.success) {
// Other errors - log them
console.warn('Failed to fetch branches:', result.error);
resetBranchState();
}
} catch (error) {
console.error('Failed to fetch branches:', error);
resetBranchState();
// Reset git status to unknown state on network/API errors
setGitRepoStatus({ isGitRepo: true, hasCommits: true });
} finally {
setIsLoadingBranches(false);
}
},
[resetBranchState]
);
const resetBranchFilter = useCallback(() => {
setBranchFilter('');
}, []);

View File

@@ -5,13 +5,27 @@ import type { WorktreeInfo } from '../types';
// Error codes that need special user-friendly handling
const GIT_STATUS_ERROR_CODES = ['NOT_GIT_REPO', 'NO_COMMITS'] as const;
type GitStatusErrorCode = (typeof GIT_STATUS_ERROR_CODES)[number];
// User-friendly messages for git status errors
const GIT_STATUS_ERROR_MESSAGES: Record<string, string> = {
const GIT_STATUS_ERROR_MESSAGES: Record<GitStatusErrorCode, string> = {
NOT_GIT_REPO: 'This directory is not a git repository',
NO_COMMITS: 'Repository has no commits yet. Create an initial commit first.',
};
/**
* Helper to handle git status errors with user-friendly messages.
* @returns true if the error was a git status error and was handled, false otherwise.
*/
function handleGitStatusError(result: { code?: string; error?: string }): boolean {
const errorCode = result.code as GitStatusErrorCode | undefined;
if (errorCode && GIT_STATUS_ERROR_CODES.includes(errorCode)) {
toast.info(GIT_STATUS_ERROR_MESSAGES[errorCode] || result.error);
return true;
}
return false;
}
interface UseWorktreeActionsOptions {
fetchWorktrees: () => Promise<Array<{ path: string; branch: string }> | undefined>;
fetchBranches: (worktreePath: string) => Promise<void>;
@@ -38,15 +52,7 @@ export function useWorktreeActions({ fetchWorktrees, fetchBranches }: UseWorktre
toast.success(result.result.message);
fetchWorktrees();
} else {
// Handle git status errors with informative messages
const errorCode = (result as { code?: string }).code;
if (
errorCode &&
GIT_STATUS_ERROR_CODES.includes(errorCode as (typeof GIT_STATUS_ERROR_CODES)[number])
) {
toast.info(GIT_STATUS_ERROR_MESSAGES[errorCode] || result.error);
return;
}
if (handleGitStatusError(result)) return;
toast.error(result.error || 'Failed to switch branch');
}
} catch (error) {
@@ -74,15 +80,7 @@ export function useWorktreeActions({ fetchWorktrees, fetchBranches }: UseWorktre
toast.success(result.result.message);
fetchWorktrees();
} else {
// Handle git status errors with informative messages
const errorCode = (result as { code?: string }).code;
if (
errorCode &&
GIT_STATUS_ERROR_CODES.includes(errorCode as (typeof GIT_STATUS_ERROR_CODES)[number])
) {
toast.info(GIT_STATUS_ERROR_MESSAGES[errorCode] || result.error);
return;
}
if (handleGitStatusError(result)) return;
toast.error(result.error || 'Failed to pull latest changes');
}
} catch (error) {
@@ -111,15 +109,7 @@ export function useWorktreeActions({ fetchWorktrees, fetchBranches }: UseWorktre
fetchBranches(worktree.path);
fetchWorktrees();
} else {
// Handle git status errors with informative messages
const errorCode = (result as { code?: string }).code;
if (
errorCode &&
GIT_STATUS_ERROR_CODES.includes(errorCode as (typeof GIT_STATUS_ERROR_CODES)[number])
) {
toast.info(GIT_STATUS_ERROR_MESSAGES[errorCode] || result.error);
return;
}
if (handleGitStatusError(result)) return;
toast.error(result.error || 'Failed to push changes');
}
} catch (error) {