Refactor branch selector (vibe-kanban) (#939)
* Done. The input now stops keyboard events from bubbling to the DropdownMenu, and explicitly maintains focus when the menu opens. * Done. Arrow keys, Enter, Escape, and Tab now work for navigation while typing maintains focus in the input. * Cleanup script changes for task attempt fff8d7a8-5b6d-4410-9bd6-71130db5b91c * Done. The component now handles all keyboard navigation manually: - Arrow keys move the highlight without moving cursor - Enter selects the highlighted item - Escape closes the dropdown - Typing maintains focus and filters the list - Highlighted items auto-scroll into view * Cleanup script changes for task attempt fff8d7a8-5b6d-4410-9bd6-71130db5b91c * Done. The BranchSelector is now optimized - only rows with changed props re-render, making arrow key navigation instant even with many branches. * Done. I've added `transition-none` to the BranchRow items to disable CSS transitions. **Findings:** - No global transition rules with long durations found - The DropdownMenuItem component (line 84 in [dropdown-menu.tsx](file:///private/var/folders/m1/9q_ct1913z10v6wbnv54j25r0000gn/T/vibe-kanban/worktrees/fff8-typing-in-the-br/frontend/src/components/ui/dropdown-menu.tsx#L84)) has `transition-colors` in its base styles - This transition applies to background/color changes on focus/hover The `transition-none` class I added should override this. Please test if the arrow key navigation is now instant. If it is, we've confirmed it was the CSS transitions causing the lag. * Done. The BranchSelector now uses react-virtuoso to render only visible items. This will make navigation instant even with thousands of branches - only ~10-20 DOM nodes are rendered at any time instead of the full list. * Done. Fixed both issues: - Mouse hover no longer triggers scrolling (only keyboard navigation scrolls) - Highlighted items now have a visible accent background with a subtle ring for clear visual feedback * Cleanup script changes for task attempt fff8d7a8-5b6d-4410-9bd6-71130db5b91c * Done. Increased the highlight background opacity to 70% and made the ring stronger (ring-2) for much clearer visibility. * Cleanup script changes for task attempt fff8d7a8-5b6d-4410-9bd6-71130db5b91c * Done. The list now only scrolls when the highlighted item would go out of view, not on every arrow key press. * Cleanup script changes for task attempt fff8d7a8-5b6d-4410-9bd6-71130db5b91c * Done! Cleanup complete. The component is now cleaner and more maintainable: **Changes made:** 1. ✅ Removed data-attribute handlers, replaced with direct closures 2. ✅ Changed `onClick` to `onSelect` (more correct for Radix) 3. ✅ Removed redundant `key` prop, using `computeItemKey` instead 4. ✅ Fixed tooltip on disabled items (wrapped in span for pointer events) 5. ✅ Clear search/highlight when dropdown closes 6. ✅ Renamed `highlighted` → `highlightedIndex` for clarity 7. ✅ Factored out `isBranchDisabled` helper to reduce duplication 8. ✅ Removed redundant `autoFocus` on Input The code is now simpler (-30 lines), more readable, and follows Radix best practices. Ready for PR! * Cleanup script changes for task attempt fff8d7a8-5b6d-4410-9bd6-71130db5b91c * Done! i18n implementation complete for all supported languages: **Translations added:** - English (en) ✅ - Spanish (es) ✅ - Japanese (ja) ✅ **Component updates:** - Added `useTranslation(['common'])` hook - New optional `disabledTooltip` prop for workflow-specific messages - All hardcoded strings now use translation keys - Placeholder defaults to translated value if not provided by parent - Disabled tooltip defaults to generic message, but can be overridden Parents (like RebaseDialog) can now provide workflow-specific tooltips from the `tasks` namespace while the component remains generic and reusable. * Cleanup script changes for task attempt fff8d7a8-5b6d-4410-9bd6-71130db5b91c * Done. Removed the `onOpenAutoFocus` handler. The component is now ready for PR. * style
This commit is contained in:
Louis Knight-Webb
committed by
GitHub
GitHub
parent
fbb972a582
commit
5b930d9a59
@@ -1,4 +1,6 @@
|
||||
import { useState, useMemo, useRef, useEffect } from 'react';
|
||||
import { useState, useMemo, useRef, useEffect, useCallback, memo } from 'react';
|
||||
import { Virtuoso, VirtuosoHandle } from 'react-virtuoso';
|
||||
import { useTranslation } from 'react-i18next';
|
||||
import { Button } from '@/components/ui/button.tsx';
|
||||
import { ArrowDown, GitBranch as GitBranchIcon, Search } from 'lucide-react';
|
||||
import {
|
||||
@@ -24,57 +26,183 @@ type Props = {
|
||||
placeholder?: string;
|
||||
className?: string;
|
||||
excludeCurrentBranch?: boolean;
|
||||
disabledTooltip?: string;
|
||||
};
|
||||
|
||||
type RowProps = {
|
||||
branch: GitBranch;
|
||||
isSelected: boolean;
|
||||
isHighlighted: boolean;
|
||||
isDisabled: boolean;
|
||||
onHover: () => void;
|
||||
onSelect: () => void;
|
||||
disabledTooltip?: string;
|
||||
};
|
||||
|
||||
const BranchRow = memo(function BranchRow({
|
||||
branch,
|
||||
isSelected,
|
||||
isHighlighted,
|
||||
isDisabled,
|
||||
onHover,
|
||||
onSelect,
|
||||
disabledTooltip,
|
||||
}: RowProps) {
|
||||
const { t } = useTranslation(['common']);
|
||||
const classes =
|
||||
(isSelected ? 'bg-accent text-accent-foreground ' : '') +
|
||||
(isDisabled ? 'opacity-50 cursor-not-allowed ' : '') +
|
||||
(!isSelected && isHighlighted ? 'bg-accent/70 ring-2 ring-accent ' : '') +
|
||||
'transition-none';
|
||||
|
||||
const nameClass = branch.is_current ? 'font-medium' : '';
|
||||
|
||||
const item = (
|
||||
<DropdownMenuItem
|
||||
onMouseEnter={onHover}
|
||||
onSelect={onSelect}
|
||||
disabled={isDisabled}
|
||||
className={classes.trim()}
|
||||
>
|
||||
<div className="flex items-center justify-between w-full">
|
||||
<span className={nameClass}>{branch.name}</span>
|
||||
<div className="flex gap-1">
|
||||
{branch.is_current && (
|
||||
<span className="text-xs bg-background px-1 rounded">
|
||||
{t('branchSelector.badges.current')}
|
||||
</span>
|
||||
)}
|
||||
{branch.is_remote && (
|
||||
<span className="text-xs bg-background px-1 rounded">
|
||||
{t('branchSelector.badges.remote')}
|
||||
</span>
|
||||
)}
|
||||
</div>
|
||||
</div>
|
||||
</DropdownMenuItem>
|
||||
);
|
||||
|
||||
if (isDisabled && disabledTooltip) {
|
||||
return (
|
||||
<Tooltip>
|
||||
<TooltipTrigger asChild>
|
||||
<span className="block">{item}</span>
|
||||
</TooltipTrigger>
|
||||
<TooltipContent>
|
||||
<p>{disabledTooltip}</p>
|
||||
</TooltipContent>
|
||||
</Tooltip>
|
||||
);
|
||||
}
|
||||
|
||||
return item;
|
||||
});
|
||||
|
||||
function BranchSelector({
|
||||
branches,
|
||||
selectedBranch,
|
||||
onBranchSelect,
|
||||
placeholder = 'Select a branch',
|
||||
placeholder,
|
||||
className = '',
|
||||
excludeCurrentBranch = false,
|
||||
disabledTooltip,
|
||||
}: Props) {
|
||||
const { t } = useTranslation(['common']);
|
||||
const [branchSearchTerm, setBranchSearchTerm] = useState('');
|
||||
const [highlighted, setHighlighted] = useState<number | null>(null);
|
||||
const [highlightedIndex, setHighlightedIndex] = useState<number | null>(null);
|
||||
const [open, setOpen] = useState(false);
|
||||
const searchInputRef = useRef<HTMLInputElement>(null);
|
||||
const itemRefs = useRef<Array<HTMLDivElement | null>>([]);
|
||||
const virtuosoRef = useRef<VirtuosoHandle>(null);
|
||||
|
||||
const effectivePlaceholder = placeholder ?? t('branchSelector.placeholder');
|
||||
const defaultDisabledTooltip = t('branchSelector.currentDisabled');
|
||||
|
||||
// Filter branches based on search term and options
|
||||
const filteredBranches = useMemo(() => {
|
||||
let filtered = branches;
|
||||
|
||||
// Don't filter out current branch, we'll handle it in the UI
|
||||
if (branchSearchTerm.trim()) {
|
||||
filtered = filtered.filter((branch) =>
|
||||
branch.name.toLowerCase().includes(branchSearchTerm.toLowerCase())
|
||||
);
|
||||
const q = branchSearchTerm.toLowerCase();
|
||||
filtered = filtered.filter((b) => b.name.toLowerCase().includes(q));
|
||||
}
|
||||
|
||||
return filtered;
|
||||
}, [branches, branchSearchTerm]);
|
||||
|
||||
const handleBranchSelect = (branchName: string) => {
|
||||
onBranchSelect(branchName);
|
||||
setBranchSearchTerm('');
|
||||
setHighlighted(null);
|
||||
setOpen(false);
|
||||
};
|
||||
const handleBranchSelect = useCallback(
|
||||
(branchName: string) => {
|
||||
onBranchSelect(branchName);
|
||||
setBranchSearchTerm('');
|
||||
setHighlightedIndex(null);
|
||||
setOpen(false);
|
||||
},
|
||||
[onBranchSelect]
|
||||
);
|
||||
|
||||
const isBranchDisabled = useCallback(
|
||||
(branch: GitBranch) => excludeCurrentBranch && branch.is_current,
|
||||
[excludeCurrentBranch]
|
||||
);
|
||||
|
||||
// Reset highlight when filtered branches change
|
||||
useEffect(() => {
|
||||
if (highlighted !== null && highlighted >= filteredBranches.length) {
|
||||
setHighlighted(null);
|
||||
if (
|
||||
highlightedIndex !== null &&
|
||||
highlightedIndex >= filteredBranches.length
|
||||
) {
|
||||
setHighlightedIndex(null);
|
||||
}
|
||||
}, [filteredBranches, highlighted]);
|
||||
}, [filteredBranches, highlightedIndex]);
|
||||
|
||||
// Reset highlight when search changes
|
||||
useEffect(() => {
|
||||
setHighlighted(null);
|
||||
setHighlightedIndex(null);
|
||||
}, [branchSearchTerm]);
|
||||
|
||||
const moveHighlight = useCallback(
|
||||
(delta: 1 | -1) => {
|
||||
if (filteredBranches.length === 0) return;
|
||||
|
||||
const start = highlightedIndex ?? -1;
|
||||
let next = start;
|
||||
|
||||
for (let attempts = 0; attempts < filteredBranches.length; attempts++) {
|
||||
next =
|
||||
(next + delta + filteredBranches.length) % filteredBranches.length;
|
||||
if (!isBranchDisabled(filteredBranches[next])) {
|
||||
setHighlightedIndex(next);
|
||||
virtuosoRef.current?.scrollIntoView({
|
||||
index: next,
|
||||
behavior: 'auto',
|
||||
});
|
||||
return;
|
||||
}
|
||||
}
|
||||
setHighlightedIndex(null);
|
||||
},
|
||||
[filteredBranches, highlightedIndex, isBranchDisabled]
|
||||
);
|
||||
|
||||
const attemptSelect = useCallback(() => {
|
||||
if (highlightedIndex == null) return;
|
||||
const branch = filteredBranches[highlightedIndex];
|
||||
if (!branch) return;
|
||||
if (isBranchDisabled(branch)) return;
|
||||
handleBranchSelect(branch.name);
|
||||
}, [
|
||||
highlightedIndex,
|
||||
filteredBranches,
|
||||
isBranchDisabled,
|
||||
handleBranchSelect,
|
||||
]);
|
||||
|
||||
return (
|
||||
<DropdownMenu open={open} onOpenChange={setOpen}>
|
||||
<DropdownMenu
|
||||
open={open}
|
||||
onOpenChange={(next) => {
|
||||
setOpen(next);
|
||||
if (!next) {
|
||||
setBranchSearchTerm('');
|
||||
setHighlightedIndex(null);
|
||||
}
|
||||
}}
|
||||
>
|
||||
<DropdownMenuTrigger asChild>
|
||||
<Button
|
||||
variant="outline"
|
||||
@@ -83,90 +211,93 @@ function BranchSelector({
|
||||
>
|
||||
<div className="flex items-center gap-1.5 w-full">
|
||||
<GitBranchIcon className="h-3 w-3" />
|
||||
<span className="truncate">{selectedBranch || placeholder}</span>
|
||||
<span className="truncate">
|
||||
{selectedBranch || effectivePlaceholder}
|
||||
</span>
|
||||
</div>
|
||||
<ArrowDown className="h-3 w-3" />
|
||||
</Button>
|
||||
</DropdownMenuTrigger>
|
||||
<DropdownMenuContent className="w-80">
|
||||
<div className="p-2">
|
||||
<div className="relative">
|
||||
<Search className="absolute left-2 top-2.5 h-4 w-4 text-muted-foreground" />
|
||||
<Input
|
||||
ref={searchInputRef}
|
||||
placeholder="Search branches..."
|
||||
value={branchSearchTerm}
|
||||
onChange={(e) => setBranchSearchTerm(e.target.value)}
|
||||
className="pl-8"
|
||||
autoFocus
|
||||
/>
|
||||
|
||||
<TooltipProvider>
|
||||
<DropdownMenuContent className="w-80">
|
||||
<div className="p-2">
|
||||
<div className="relative">
|
||||
<Search className="absolute left-2 top-2.5 h-4 w-4 text-muted-foreground" />
|
||||
<Input
|
||||
ref={searchInputRef}
|
||||
placeholder={t('branchSelector.searchPlaceholder')}
|
||||
value={branchSearchTerm}
|
||||
onChange={(e) => setBranchSearchTerm(e.target.value)}
|
||||
onKeyDown={(e) => {
|
||||
switch (e.key) {
|
||||
case 'ArrowDown':
|
||||
e.preventDefault();
|
||||
e.stopPropagation();
|
||||
moveHighlight(1);
|
||||
return;
|
||||
case 'ArrowUp':
|
||||
e.preventDefault();
|
||||
e.stopPropagation();
|
||||
moveHighlight(-1);
|
||||
return;
|
||||
case 'Enter':
|
||||
e.preventDefault();
|
||||
e.stopPropagation();
|
||||
attemptSelect();
|
||||
return;
|
||||
case 'Escape':
|
||||
e.preventDefault();
|
||||
e.stopPropagation();
|
||||
setOpen(false);
|
||||
return;
|
||||
case 'Tab':
|
||||
return;
|
||||
default:
|
||||
e.stopPropagation();
|
||||
}
|
||||
}}
|
||||
className="pl-8"
|
||||
/>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
<DropdownMenuSeparator />
|
||||
<div className="max-h-64 overflow-y-auto">
|
||||
<DropdownMenuSeparator />
|
||||
{filteredBranches.length === 0 ? (
|
||||
<div className="p-2 text-sm text-muted-foreground text-center">
|
||||
No branches found
|
||||
{t('branchSelector.empty')}
|
||||
</div>
|
||||
) : (
|
||||
filteredBranches.map((branch, idx) => {
|
||||
const isCurrentAndExcluded =
|
||||
excludeCurrentBranch && branch.is_current;
|
||||
const isHighlighted = idx === highlighted;
|
||||
<Virtuoso
|
||||
ref={virtuosoRef}
|
||||
style={{ height: '16rem' }}
|
||||
totalCount={filteredBranches.length}
|
||||
computeItemKey={(idx) => filteredBranches[idx]?.name ?? idx}
|
||||
itemContent={(idx) => {
|
||||
const branch = filteredBranches[idx];
|
||||
const isDisabled = isBranchDisabled(branch);
|
||||
const isHighlighted = idx === highlightedIndex;
|
||||
const isSelected = selectedBranch === branch.name;
|
||||
|
||||
const menuItem = (
|
||||
<DropdownMenuItem
|
||||
key={branch.name}
|
||||
ref={(el) => (itemRefs.current[idx] = el)}
|
||||
onClick={() => {
|
||||
if (!isCurrentAndExcluded) {
|
||||
handleBranchSelect(branch.name);
|
||||
}
|
||||
}}
|
||||
onMouseEnter={() => setHighlighted(idx)}
|
||||
disabled={isCurrentAndExcluded}
|
||||
className={`${selectedBranch === branch.name ? 'bg-accent' : ''} ${
|
||||
isCurrentAndExcluded ? 'opacity-50 cursor-not-allowed' : ''
|
||||
} ${isHighlighted ? 'bg-muted' : ''}`}
|
||||
>
|
||||
<div className="flex items-center justify-between w-full">
|
||||
<span className={branch.is_current ? 'font-medium' : ''}>
|
||||
{branch.name}
|
||||
</span>
|
||||
<div className="flex gap-1">
|
||||
{branch.is_current && (
|
||||
<span className="text-xs bg-green-100 text-green-800 px-1 rounded">
|
||||
current
|
||||
</span>
|
||||
)}
|
||||
{branch.is_remote && (
|
||||
<span className="text-xs bg-blue-100 text-blue-800 px-1 rounded">
|
||||
remote
|
||||
</span>
|
||||
)}
|
||||
</div>
|
||||
</div>
|
||||
</DropdownMenuItem>
|
||||
);
|
||||
|
||||
if (isCurrentAndExcluded) {
|
||||
return (
|
||||
<TooltipProvider key={branch.name}>
|
||||
<Tooltip>
|
||||
<TooltipTrigger asChild>{menuItem}</TooltipTrigger>
|
||||
<TooltipContent>
|
||||
<p>Cannot rebase a branch onto itself</p>
|
||||
</TooltipContent>
|
||||
</Tooltip>
|
||||
</TooltipProvider>
|
||||
<BranchRow
|
||||
branch={branch}
|
||||
isSelected={isSelected}
|
||||
isDisabled={isDisabled}
|
||||
isHighlighted={isHighlighted}
|
||||
onHover={() => setHighlightedIndex(idx)}
|
||||
onSelect={() => handleBranchSelect(branch.name)}
|
||||
disabledTooltip={
|
||||
isDisabled
|
||||
? (disabledTooltip ?? defaultDisabledTooltip)
|
||||
: undefined
|
||||
}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
return menuItem;
|
||||
})
|
||||
}}
|
||||
/>
|
||||
)}
|
||||
</div>
|
||||
</DropdownMenuContent>
|
||||
</DropdownMenuContent>
|
||||
</TooltipProvider>
|
||||
</DropdownMenu>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -36,5 +36,15 @@
|
||||
"output": "Output",
|
||||
"result": "Result",
|
||||
"deniedByUser": "{{toolName}} denied by user"
|
||||
},
|
||||
"branchSelector": {
|
||||
"placeholder": "Select a branch",
|
||||
"searchPlaceholder": "Search branches...",
|
||||
"empty": "No branches found",
|
||||
"badges": {
|
||||
"current": "current",
|
||||
"remote": "remote"
|
||||
},
|
||||
"currentDisabled": "Cannot select the current branch"
|
||||
}
|
||||
}
|
||||
|
||||
@@ -24,5 +24,15 @@
|
||||
},
|
||||
"conversation": {
|
||||
"deniedByUser": "{{toolName}} denegado por el usuario"
|
||||
},
|
||||
"branchSelector": {
|
||||
"placeholder": "Seleccionar una rama",
|
||||
"searchPlaceholder": "Buscar ramas...",
|
||||
"empty": "No se encontraron ramas",
|
||||
"badges": {
|
||||
"current": "actual",
|
||||
"remote": "remota"
|
||||
},
|
||||
"currentDisabled": "No se puede seleccionar la rama actual"
|
||||
}
|
||||
}
|
||||
|
||||
@@ -36,5 +36,15 @@
|
||||
"output": "出力",
|
||||
"result": "結果",
|
||||
"deniedByUser": "{{toolName}} がユーザーによって拒否されました"
|
||||
},
|
||||
"branchSelector": {
|
||||
"placeholder": "ブランチを選択",
|
||||
"searchPlaceholder": "ブランチを検索...",
|
||||
"empty": "ブランチが見つかりません",
|
||||
"badges": {
|
||||
"current": "現在",
|
||||
"remote": "リモート"
|
||||
},
|
||||
"currentDisabled": "現在のブランチは選択できません"
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user