feat: prioritize selected picker options (#2935)
This commit is contained in:
@@ -0,0 +1,81 @@
|
||||
import { cleanup, fireEvent, render, screen } from "@testing-library/react";
|
||||
import { TooltipProvider } from "@kandev/ui/tooltip";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
import { Combobox, type ComboboxOption } from "@/components/combobox";
|
||||
|
||||
afterEach(() => cleanup());
|
||||
|
||||
const options: ComboboxOption[] = [
|
||||
{ value: "first", label: "First" },
|
||||
{ value: "current", label: "Current" },
|
||||
{ value: "last", label: "Last" },
|
||||
];
|
||||
const selectedAttribute = "data-selected";
|
||||
|
||||
describe("Combobox", () => {
|
||||
it("puts the current option first with a persistent selected surface", () => {
|
||||
render(
|
||||
<Combobox
|
||||
options={options}
|
||||
value="current"
|
||||
onValueChange={vi.fn()}
|
||||
ariaLabel="Choose an agent"
|
||||
dropdownLabel="Agents"
|
||||
/>,
|
||||
);
|
||||
|
||||
fireEvent.click(screen.getByRole("combobox", { name: "Choose an agent" }));
|
||||
|
||||
const renderedOptions = screen.getAllByRole("option");
|
||||
expect(renderedOptions.map((option) => option.textContent?.trim())).toEqual([
|
||||
"Current",
|
||||
"First",
|
||||
"Last",
|
||||
]);
|
||||
expect(renderedOptions[0].className).toContain("bg-card");
|
||||
expect(renderedOptions[0].className).toContain("border-primary/50");
|
||||
|
||||
const searchInput = document.querySelector("[cmdk-input]");
|
||||
expect(searchInput).not.toBeNull();
|
||||
expect(renderedOptions[0].getAttribute(selectedAttribute)).toBe("true");
|
||||
|
||||
fireEvent.keyDown(searchInput!, { key: "ArrowDown" });
|
||||
expect(renderedOptions[0].getAttribute(selectedAttribute)).toBe("false");
|
||||
expect(renderedOptions[1].getAttribute(selectedAttribute)).toBe("true");
|
||||
|
||||
fireEvent.keyDown(searchInput!, { key: "ArrowUp" });
|
||||
expect(renderedOptions[0].getAttribute(selectedAttribute)).toBe("true");
|
||||
});
|
||||
|
||||
it("keeps a disabled current option first and non-selectable", () => {
|
||||
render(
|
||||
<TooltipProvider>
|
||||
<Combobox
|
||||
options={[
|
||||
{ value: "available", label: "Available" },
|
||||
{
|
||||
value: "current",
|
||||
label: "Unavailable current",
|
||||
disabled: true,
|
||||
disabledReason: "No longer available",
|
||||
},
|
||||
]}
|
||||
value="current"
|
||||
onValueChange={vi.fn()}
|
||||
ariaLabel="Choose an agent"
|
||||
/>
|
||||
</TooltipProvider>,
|
||||
);
|
||||
|
||||
fireEvent.click(screen.getByRole("combobox", { name: "Choose an agent" }));
|
||||
|
||||
const renderedOptions = screen.getAllByRole("option");
|
||||
expect(renderedOptions.map((option) => option.textContent?.trim())).toEqual([
|
||||
"Unavailable current",
|
||||
"Available",
|
||||
]);
|
||||
expect(renderedOptions[0].getAttribute("aria-disabled")).toBe("true");
|
||||
expect(renderedOptions[0].className).toContain("bg-card");
|
||||
});
|
||||
});
|
||||
@@ -4,6 +4,7 @@ import { memo, useState } from "react";
|
||||
import { IconCheck, IconChevronDown, IconLoader2 } from "@tabler/icons-react";
|
||||
|
||||
import { cn } from "@/lib/utils";
|
||||
import { prioritizeSelectedOption, selectorOptionClassName } from "@/lib/utils/selector-options";
|
||||
import { Button } from "@kandev/ui/button";
|
||||
import {
|
||||
Command,
|
||||
@@ -92,8 +93,13 @@ function OptionsList({
|
||||
value: string;
|
||||
onSelect: (value: string) => void;
|
||||
}) {
|
||||
const enabled = options.filter((o) => !o.disabled);
|
||||
const disabled = options.filter((o) => o.disabled);
|
||||
const orderedOptions = prioritizeSelectedOption(options, value, (option) => option.value);
|
||||
const selected = orderedOptions.find((option) => option.value === value);
|
||||
const remaining = selected
|
||||
? orderedOptions.filter((option) => option.value !== value)
|
||||
: orderedOptions;
|
||||
const enabled = remaining.filter((o) => !o.disabled);
|
||||
const disabled = remaining.filter((o) => o.disabled);
|
||||
|
||||
const renderItem = (option: ComboboxOption) => {
|
||||
const item = (
|
||||
@@ -103,7 +109,7 @@ function OptionsList({
|
||||
keywords={option.keywords ?? [option.label, option.description ?? ""]}
|
||||
onSelect={() => !option.disabled && onSelect(option.value)}
|
||||
disabled={option.disabled}
|
||||
className={cn("relative pr-7", option.disabled && "opacity-40 cursor-not-allowed")}
|
||||
className={selectorOptionClassName(option.value === value, option.disabled)}
|
||||
>
|
||||
<div className="flex min-w-0 flex-1 items-center">
|
||||
{option.renderLabel ? option.renderLabel() : option.label}
|
||||
@@ -133,7 +139,8 @@ function OptionsList({
|
||||
|
||||
return (
|
||||
<>
|
||||
<CommandGroup>{enabled.map(renderItem)}</CommandGroup>
|
||||
{selected && <CommandGroup>{renderItem(selected)}</CommandGroup>}
|
||||
{enabled.length > 0 && <CommandGroup>{enabled.map(renderItem)}</CommandGroup>}
|
||||
{disabled.length > 0 && (
|
||||
<>
|
||||
<CommandSeparator />
|
||||
|
||||
@@ -0,0 +1,310 @@
|
||||
"use client";
|
||||
|
||||
import { useEffect, useRef } from "react";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import { IconCheck, IconChevronLeft, IconChevronRight } from "@tabler/icons-react";
|
||||
|
||||
import { cn } from "@/lib/utils";
|
||||
import * as selectorOptions from "@/lib/utils/selector-options";
|
||||
import type { ModelSelectorOption, SelectConfigOption } from "@/components/model-config-selector";
|
||||
import { Button } from "@kandev/ui/button";
|
||||
import {
|
||||
Command,
|
||||
CommandEmpty,
|
||||
CommandGroup,
|
||||
CommandInput,
|
||||
CommandItem,
|
||||
CommandList,
|
||||
} from "@kandev/ui/command";
|
||||
import { ScrollArea } from "@kandev/ui/scroll-area";
|
||||
import { Separator } from "@kandev/ui/separator";
|
||||
import { Spinner } from "@kandev/ui/spinner";
|
||||
import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip";
|
||||
|
||||
function currentOptionName(option: SelectConfigOption): string {
|
||||
return (
|
||||
option.options.find((item) => item.value === option.currentValue)?.name ?? option.currentValue
|
||||
);
|
||||
}
|
||||
|
||||
function ModelRow({
|
||||
model,
|
||||
selected,
|
||||
loading,
|
||||
onSelect,
|
||||
}: {
|
||||
model: ModelSelectorOption;
|
||||
selected: boolean;
|
||||
loading: boolean;
|
||||
onSelect: (value: string) => void;
|
||||
}) {
|
||||
const item = (
|
||||
<CommandItem
|
||||
value={model.id}
|
||||
keywords={[model.name, model.description ?? "", model.id]}
|
||||
onSelect={() => !model.disabled && onSelect(model.id)}
|
||||
disabled={model.disabled}
|
||||
data-testid={selected ? "model-config-selected-row" : undefined}
|
||||
className={selectorOptions.selectorOptionClassName(selected, model.disabled)}
|
||||
>
|
||||
<div className="flex min-w-0 flex-1 items-center">
|
||||
<div className="min-w-0 flex-1">
|
||||
<div className="truncate">{model.name}</div>
|
||||
{model.description && (
|
||||
<div className="truncate text-xs text-muted-foreground" title={model.description}>
|
||||
{model.description}
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
{model.usageMultiplier && (
|
||||
<span className="shrink-0 text-xs text-muted-foreground">{model.usageMultiplier}</span>
|
||||
)}
|
||||
</div>
|
||||
{selected && loading ? (
|
||||
<Spinner aria-hidden="true" className="absolute right-2" />
|
||||
) : (
|
||||
<IconCheck
|
||||
className={cn("absolute right-2 h-4 w-4", selected ? "opacity-100" : "opacity-0")}
|
||||
/>
|
||||
)}
|
||||
</CommandItem>
|
||||
);
|
||||
// cmdk's CommandItem swallows pointer events with no native tooltip slot;
|
||||
// keep disabled items in a focusable wrapper so their unavailable reason is reachable.
|
||||
if (model.disabled && model.disabledReason) {
|
||||
return (
|
||||
<Tooltip>
|
||||
<TooltipTrigger asChild>
|
||||
<div
|
||||
tabIndex={0}
|
||||
aria-label={`${model.name}: ${model.disabledReason}`}
|
||||
className="rounded outline-none focus-visible:ring-2 focus-visible:ring-ring/35"
|
||||
>
|
||||
{item}
|
||||
</div>
|
||||
</TooltipTrigger>
|
||||
<TooltipContent side="right">{model.disabledReason}</TooltipContent>
|
||||
</Tooltip>
|
||||
);
|
||||
}
|
||||
return item;
|
||||
}
|
||||
|
||||
function ConfigOptionTrigger({
|
||||
option,
|
||||
onSelect,
|
||||
triggerRef,
|
||||
}: {
|
||||
option: SelectConfigOption;
|
||||
onSelect: () => void;
|
||||
triggerRef?: (element: HTMLButtonElement | null) => void;
|
||||
}) {
|
||||
return (
|
||||
<button
|
||||
type="button"
|
||||
ref={triggerRef}
|
||||
data-testid={`config-option-trigger-${option.id}`}
|
||||
className="flex min-h-9 w-full cursor-pointer items-center justify-between gap-3 rounded-md px-2.5 py-2 text-left text-xs/relaxed hover:bg-muted focus-visible:ring-2 focus-visible:ring-ring/35 focus-visible:outline-none"
|
||||
onClick={onSelect}
|
||||
>
|
||||
<span className="min-w-0 flex-1">
|
||||
<span className="block font-medium">{option.name}</span>
|
||||
</span>
|
||||
<span className="flex min-w-0 items-center gap-2 text-muted-foreground">
|
||||
<span className="truncate">{currentOptionName(option)}</span>
|
||||
<IconChevronRight className="h-3.5 w-3.5 shrink-0" />
|
||||
</span>
|
||||
</button>
|
||||
);
|
||||
}
|
||||
|
||||
function ConfigOptionSubSelector({
|
||||
option,
|
||||
onBack,
|
||||
onChange,
|
||||
}: {
|
||||
option: SelectConfigOption;
|
||||
onBack: () => void;
|
||||
onChange?: (configId: string, value: string) => void;
|
||||
}) {
|
||||
const { t } = useTranslation();
|
||||
const orderedOptions = selectorOptions.prioritizeValueOption(option.options, option.currentValue);
|
||||
return (
|
||||
<div className="flex min-h-0 flex-col gap-2">
|
||||
<button
|
||||
type="button"
|
||||
aria-label={t("agents:backToModelSettings", { name: option.name })}
|
||||
autoFocus
|
||||
className="flex min-h-9 w-full cursor-pointer items-center gap-2 rounded-md px-2 text-left text-xs/relaxed hover:bg-muted focus-visible:ring-2 focus-visible:ring-ring/35 focus-visible:outline-none"
|
||||
onClick={onBack}
|
||||
>
|
||||
<IconChevronLeft className="h-3.5 w-3.5 shrink-0 text-muted-foreground" />
|
||||
<span className="min-w-0">
|
||||
<span className="block truncate font-medium">{option.name}</span>
|
||||
{option.description && (
|
||||
<span className="block whitespace-normal text-muted-foreground">
|
||||
{option.description}
|
||||
</span>
|
||||
)}
|
||||
</span>
|
||||
</button>
|
||||
<div
|
||||
className="max-h-[min(18rem,calc(100dvh-8rem))] overflow-y-auto overscroll-contain pr-2"
|
||||
data-testid={`config-option-section-${option.id}`}
|
||||
>
|
||||
<div className="space-y-1">
|
||||
{orderedOptions.map((item, index) => {
|
||||
const descriptionId = item.description
|
||||
? `config-option-value-description-${option.id}-${index}`
|
||||
: undefined;
|
||||
return (
|
||||
<Button
|
||||
key={item.value}
|
||||
type="button"
|
||||
aria-label={item.name}
|
||||
aria-describedby={descriptionId}
|
||||
variant={item.value === option.currentValue ? "secondary" : "ghost"}
|
||||
size="sm"
|
||||
className={selectorOptions.configClassName(item.value === option.currentValue)}
|
||||
disabled={!onChange}
|
||||
onClick={() => {
|
||||
onChange?.(option.id, item.value);
|
||||
onBack();
|
||||
}}
|
||||
>
|
||||
<span className="min-w-0 flex-1">
|
||||
<span className="block truncate">{item.name}</span>
|
||||
{item.description && (
|
||||
<span
|
||||
id={descriptionId}
|
||||
className="block whitespace-normal text-xs text-muted-foreground"
|
||||
>
|
||||
{item.description}
|
||||
</span>
|
||||
)}
|
||||
</span>
|
||||
<IconCheck
|
||||
className={cn(
|
||||
"ml-auto h-4 w-4 shrink-0",
|
||||
item.value === option.currentValue ? "opacity-100" : "opacity-0",
|
||||
)}
|
||||
/>
|
||||
</Button>
|
||||
);
|
||||
})}
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
export type ModelConfigSelectorContentProps = {
|
||||
activeConfig: SelectConfigOption | undefined;
|
||||
modelOptions: ModelSelectorOption[];
|
||||
currentModelValue: string;
|
||||
extraConfigOptions: SelectConfigOption[];
|
||||
onModelSelect: (value: string) => void;
|
||||
onConfigSelect: (configId: string) => void;
|
||||
onConfigBack: () => void;
|
||||
onConfigChange?: (configId: string, value: string) => void;
|
||||
configOptionsLoading: boolean;
|
||||
};
|
||||
|
||||
export function ModelConfigSelectorContent({
|
||||
activeConfig,
|
||||
modelOptions,
|
||||
currentModelValue,
|
||||
extraConfigOptions,
|
||||
onModelSelect,
|
||||
onConfigSelect,
|
||||
onConfigBack,
|
||||
onConfigChange,
|
||||
configOptionsLoading,
|
||||
}: ModelConfigSelectorContentProps) {
|
||||
const { t } = useTranslation();
|
||||
const pendingFocusConfigId = useRef<string | null>(null);
|
||||
const triggerRefs = useRef<Record<string, HTMLButtonElement | null>>({});
|
||||
const showModelFilter = modelOptions.length > 5;
|
||||
const orderedModelOptions = selectorOptions.prioritizeIdOption(modelOptions, currentModelValue);
|
||||
|
||||
useEffect(() => {
|
||||
if (activeConfig) return;
|
||||
const configId = pendingFocusConfigId.current;
|
||||
if (!configId) return;
|
||||
pendingFocusConfigId.current = null;
|
||||
triggerRefs.current[configId]?.focus();
|
||||
}, [activeConfig]);
|
||||
|
||||
const returnToConfigTrigger = () => {
|
||||
if (activeConfig) {
|
||||
pendingFocusConfigId.current = activeConfig.id;
|
||||
}
|
||||
onConfigBack();
|
||||
};
|
||||
|
||||
if (activeConfig) {
|
||||
return (
|
||||
<ConfigOptionSubSelector
|
||||
option={activeConfig}
|
||||
onBack={returnToConfigTrigger}
|
||||
onChange={onConfigChange}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
return (
|
||||
<>
|
||||
<Command>
|
||||
{showModelFilter && <CommandInput placeholder={t("agents:filterModels")} className="h-8" />}
|
||||
<CommandList className="max-h-60">
|
||||
<CommandEmpty>{t("agents:noModelsFound")}</CommandEmpty>
|
||||
<CommandGroup heading={t("agents:modelHeading")}>
|
||||
{orderedModelOptions.map((model) => (
|
||||
<ModelRow
|
||||
key={model.id}
|
||||
model={model}
|
||||
selected={model.id === currentModelValue}
|
||||
loading={configOptionsLoading}
|
||||
onSelect={onModelSelect}
|
||||
/>
|
||||
))}
|
||||
</CommandGroup>
|
||||
</CommandList>
|
||||
</Command>
|
||||
{configOptionsLoading ? (
|
||||
<>
|
||||
<Separator />
|
||||
<div
|
||||
className="flex min-h-9 items-center gap-2 px-2 text-xs text-muted-foreground"
|
||||
data-testid="model-config-options-loading"
|
||||
role="status"
|
||||
aria-label={t("agents:resolvingModelOptions")}
|
||||
>
|
||||
<Spinner aria-hidden="true" className="h-3.5 w-3.5" />
|
||||
<span aria-hidden="true">{t("agents:resolvingModelOptions")}</span>
|
||||
</div>
|
||||
</>
|
||||
) : (
|
||||
extraConfigOptions.length > 0 && (
|
||||
<>
|
||||
<Separator />
|
||||
<ScrollArea className="max-h-40 pr-2">
|
||||
<div className="space-y-1">
|
||||
{extraConfigOptions.map((option) => (
|
||||
<ConfigOptionTrigger
|
||||
key={option.id}
|
||||
option={option}
|
||||
onSelect={() => onConfigSelect(option.id)}
|
||||
triggerRef={(element) => {
|
||||
triggerRefs.current[option.id] = element;
|
||||
}}
|
||||
/>
|
||||
))}
|
||||
</div>
|
||||
</ScrollArea>
|
||||
</>
|
||||
)
|
||||
)}
|
||||
</>
|
||||
);
|
||||
}
|
||||
@@ -102,6 +102,58 @@ describe("ModelConfigSelector", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("ModelConfigSelector selected options", () => {
|
||||
it("puts the current model and nested config value first", () => {
|
||||
render(
|
||||
<ModelConfigSelector
|
||||
modelOptions={[
|
||||
{ id: "model-1", name: "Model 1" },
|
||||
{ id: "model-2", name: "Model 2" },
|
||||
{ id: "model-3", name: "Model 3" },
|
||||
]}
|
||||
currentModel="model-2"
|
||||
onModelChange={() => {}}
|
||||
onConfigChange={() => {}}
|
||||
configOptions={[
|
||||
{
|
||||
type: "select",
|
||||
id: "effort",
|
||||
name: "Effort",
|
||||
currentValue: "medium",
|
||||
options: [
|
||||
{ value: "low", name: "Low" },
|
||||
{ value: "medium", name: "Medium" },
|
||||
{ value: "high", name: "High" },
|
||||
],
|
||||
},
|
||||
]}
|
||||
/>,
|
||||
);
|
||||
|
||||
fireEvent.click(screen.getByRole("button", { name: modelSettingsButtonName }));
|
||||
expect(screen.getAllByRole("option").map((option) => option.textContent?.trim())).toEqual([
|
||||
"Model 2",
|
||||
"Model 1",
|
||||
"Model 3",
|
||||
]);
|
||||
expect(screen.getByTestId("model-config-selected-row").className).toContain("bg-card");
|
||||
expect(screen.getByTestId("model-config-selected-row").className).toContain(
|
||||
"border-primary/50",
|
||||
);
|
||||
|
||||
fireEvent.click(screen.getByTestId(effortTriggerTestId));
|
||||
const effortSection = screen.getByTestId(effortSectionTestId);
|
||||
expect(
|
||||
within(effortSection)
|
||||
.getAllByRole("button")
|
||||
.map((button) => button.textContent?.trim()),
|
||||
).toEqual(["Medium", "Low", "High"]);
|
||||
expect(within(effortSection).getByRole("button", { name: "Medium" }).className).toContain(
|
||||
"bg-card",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("ModelConfigSelector loading behavior", () => {
|
||||
it("keeps the picker open after selecting a model without existing dependent options", () => {
|
||||
const onModelChange = vi.fn();
|
||||
|
||||
@@ -1,25 +1,15 @@
|
||||
"use client";
|
||||
|
||||
import { memo, useEffect, useRef, useState } from "react";
|
||||
import { memo, useState } from "react";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import type { TFunction } from "i18next";
|
||||
import { IconCheck, IconChevronDown, IconChevronLeft, IconChevronRight } from "@tabler/icons-react";
|
||||
import { IconChevronDown } from "@tabler/icons-react";
|
||||
|
||||
import { cn } from "@/lib/utils";
|
||||
import { settingsControlClassName } from "@/components/settings/settings-control";
|
||||
import { ModelConfigSelectorContent } from "@/components/model-config-selector-content";
|
||||
import { Button } from "@kandev/ui/button";
|
||||
import {
|
||||
Command,
|
||||
CommandEmpty,
|
||||
CommandGroup,
|
||||
CommandInput,
|
||||
CommandItem,
|
||||
CommandList,
|
||||
} from "@kandev/ui/command";
|
||||
import { Popover, PopoverContent, PopoverTrigger } from "@kandev/ui/popover";
|
||||
import { ScrollArea } from "@kandev/ui/scroll-area";
|
||||
import { Separator } from "@kandev/ui/separator";
|
||||
import { Spinner } from "@kandev/ui/spinner";
|
||||
import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip";
|
||||
|
||||
export type ModelSelectorOption = {
|
||||
@@ -27,8 +17,7 @@ export type ModelSelectorOption = {
|
||||
name: string;
|
||||
description?: string;
|
||||
usageMultiplier?: string;
|
||||
/** When true the model is unavailable ("gone") — rendered greyed out and
|
||||
* not selectable, with `disabledReason` shown in a tooltip. */
|
||||
/** Unavailable models are dimmed and non-selectable with a reason tooltip. */
|
||||
disabled?: boolean;
|
||||
disabledReason?: string;
|
||||
};
|
||||
@@ -184,286 +173,6 @@ function triggerDetails(
|
||||
);
|
||||
}
|
||||
|
||||
function ModelRow({
|
||||
model,
|
||||
selected,
|
||||
loading,
|
||||
onSelect,
|
||||
}: {
|
||||
model: ModelSelectorOption;
|
||||
selected: boolean;
|
||||
loading: boolean;
|
||||
onSelect: (value: string) => void;
|
||||
}) {
|
||||
const item = (
|
||||
<CommandItem
|
||||
value={model.id}
|
||||
keywords={[model.name, model.description ?? "", model.id]}
|
||||
onSelect={() => !model.disabled && onSelect(model.id)}
|
||||
disabled={model.disabled}
|
||||
data-testid={selected ? "model-config-selected-row" : undefined}
|
||||
className={cn("relative pr-7", model.disabled && "opacity-40 cursor-not-allowed")}
|
||||
>
|
||||
<div className="flex min-w-0 flex-1 items-center">
|
||||
<div className="min-w-0 flex-1">
|
||||
<div className="truncate">{model.name}</div>
|
||||
{model.description && (
|
||||
<div className="truncate text-xs text-muted-foreground" title={model.description}>
|
||||
{model.description}
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
{model.usageMultiplier && (
|
||||
<span className="shrink-0 text-xs text-muted-foreground">{model.usageMultiplier}</span>
|
||||
)}
|
||||
</div>
|
||||
{selected && loading ? (
|
||||
<Spinner aria-hidden="true" className="absolute right-2" />
|
||||
) : (
|
||||
<IconCheck
|
||||
className={cn("absolute right-2 h-4 w-4", selected ? "opacity-100" : "opacity-0")}
|
||||
/>
|
||||
)}
|
||||
</CommandItem>
|
||||
);
|
||||
// cmdk's CommandItem swallows pointer events with no native tooltip slot;
|
||||
// keep disabled items in a focusable wrapper so their gone reason is reachable.
|
||||
if (model.disabled && model.disabledReason) {
|
||||
return (
|
||||
<Tooltip>
|
||||
<TooltipTrigger asChild>
|
||||
<div
|
||||
tabIndex={0}
|
||||
aria-label={`${model.name}: ${model.disabledReason}`}
|
||||
className="rounded outline-none focus-visible:ring-2 focus-visible:ring-ring/35"
|
||||
>
|
||||
{item}
|
||||
</div>
|
||||
</TooltipTrigger>
|
||||
<TooltipContent side="right">{model.disabledReason}</TooltipContent>
|
||||
</Tooltip>
|
||||
);
|
||||
}
|
||||
return item;
|
||||
}
|
||||
|
||||
function ConfigOptionTrigger({
|
||||
option,
|
||||
onSelect,
|
||||
triggerRef,
|
||||
}: {
|
||||
option: SelectConfigOption;
|
||||
onSelect: () => void;
|
||||
triggerRef?: (element: HTMLButtonElement | null) => void;
|
||||
}) {
|
||||
return (
|
||||
<button
|
||||
type="button"
|
||||
ref={triggerRef}
|
||||
data-testid={`config-option-trigger-${option.id}`}
|
||||
className="flex min-h-9 w-full cursor-pointer items-center justify-between gap-3 rounded-md px-2.5 py-2 text-left text-xs/relaxed hover:bg-muted focus-visible:ring-2 focus-visible:ring-ring/35 focus-visible:outline-none"
|
||||
onClick={onSelect}
|
||||
>
|
||||
<span className="min-w-0 flex-1">
|
||||
<span className="block font-medium">{option.name}</span>
|
||||
</span>
|
||||
<span className="flex min-w-0 items-center gap-2 text-muted-foreground">
|
||||
<span className="truncate">{currentOptionName(option)}</span>
|
||||
<IconChevronRight className="h-3.5 w-3.5 shrink-0" />
|
||||
</span>
|
||||
</button>
|
||||
);
|
||||
}
|
||||
|
||||
function ConfigOptionSubSelector({
|
||||
option,
|
||||
onBack,
|
||||
onChange,
|
||||
}: {
|
||||
option: SelectConfigOption;
|
||||
onBack: () => void;
|
||||
onChange?: (configId: string, value: string) => void;
|
||||
}) {
|
||||
const { t } = useTranslation();
|
||||
return (
|
||||
<div className="flex min-h-0 flex-col gap-2">
|
||||
<button
|
||||
type="button"
|
||||
aria-label={t("agents:backToModelSettings", { name: option.name })}
|
||||
autoFocus
|
||||
className="flex min-h-9 w-full cursor-pointer items-center gap-2 rounded-md px-2 text-left text-xs/relaxed hover:bg-muted focus-visible:ring-2 focus-visible:ring-ring/35 focus-visible:outline-none"
|
||||
onClick={onBack}
|
||||
>
|
||||
<IconChevronLeft className="h-3.5 w-3.5 shrink-0 text-muted-foreground" />
|
||||
<span className="min-w-0">
|
||||
<span className="block truncate font-medium">{option.name}</span>
|
||||
{option.description && (
|
||||
<span className="block whitespace-normal text-muted-foreground">
|
||||
{option.description}
|
||||
</span>
|
||||
)}
|
||||
</span>
|
||||
</button>
|
||||
<div
|
||||
className="max-h-[min(18rem,calc(100dvh-8rem))] overflow-y-auto overscroll-contain pr-2"
|
||||
data-testid={`config-option-section-${option.id}`}
|
||||
>
|
||||
<div className="space-y-1">
|
||||
{option.options.map((item, index) => {
|
||||
const descriptionId = item.description
|
||||
? `config-option-value-description-${option.id}-${index}`
|
||||
: undefined;
|
||||
return (
|
||||
<Button
|
||||
key={item.value}
|
||||
type="button"
|
||||
aria-label={item.name}
|
||||
aria-describedby={descriptionId}
|
||||
variant={item.value === option.currentValue ? "secondary" : "ghost"}
|
||||
size="sm"
|
||||
className="h-auto min-h-9 w-full min-w-0 cursor-pointer justify-start px-2 py-2 text-left"
|
||||
disabled={!onChange}
|
||||
onClick={() => {
|
||||
onChange?.(option.id, item.value);
|
||||
onBack();
|
||||
}}
|
||||
>
|
||||
<span className="min-w-0 flex-1">
|
||||
<span className="block truncate">{item.name}</span>
|
||||
{item.description && (
|
||||
<span
|
||||
id={descriptionId}
|
||||
className="block whitespace-normal text-xs text-muted-foreground"
|
||||
>
|
||||
{item.description}
|
||||
</span>
|
||||
)}
|
||||
</span>
|
||||
<IconCheck
|
||||
className={cn(
|
||||
"ml-auto h-4 w-4 shrink-0",
|
||||
item.value === option.currentValue ? "opacity-100" : "opacity-0",
|
||||
)}
|
||||
/>
|
||||
</Button>
|
||||
);
|
||||
})}
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
type ModelConfigSelectorContentProps = {
|
||||
activeConfig: SelectConfigOption | undefined;
|
||||
modelOptions: ModelSelectorOption[];
|
||||
currentModelValue: string;
|
||||
extraConfigOptions: SelectConfigOption[];
|
||||
onModelSelect: (value: string) => void;
|
||||
onConfigSelect: (configId: string) => void;
|
||||
onConfigBack: () => void;
|
||||
onConfigChange?: (configId: string, value: string) => void;
|
||||
configOptionsLoading: boolean;
|
||||
};
|
||||
|
||||
function ModelConfigSelectorContent({
|
||||
activeConfig,
|
||||
modelOptions,
|
||||
currentModelValue,
|
||||
extraConfigOptions,
|
||||
onModelSelect,
|
||||
onConfigSelect,
|
||||
onConfigBack,
|
||||
onConfigChange,
|
||||
configOptionsLoading,
|
||||
}: ModelConfigSelectorContentProps) {
|
||||
const { t } = useTranslation();
|
||||
const pendingFocusConfigId = useRef<string | null>(null);
|
||||
const triggerRefs = useRef<Record<string, HTMLButtonElement | null>>({});
|
||||
const showModelFilter = modelOptions.length > 5;
|
||||
|
||||
useEffect(() => {
|
||||
if (activeConfig) return;
|
||||
const configId = pendingFocusConfigId.current;
|
||||
if (!configId) return;
|
||||
pendingFocusConfigId.current = null;
|
||||
triggerRefs.current[configId]?.focus();
|
||||
}, [activeConfig]);
|
||||
|
||||
const returnToConfigTrigger = () => {
|
||||
if (activeConfig) {
|
||||
pendingFocusConfigId.current = activeConfig.id;
|
||||
}
|
||||
onConfigBack();
|
||||
};
|
||||
|
||||
if (activeConfig) {
|
||||
return (
|
||||
<ConfigOptionSubSelector
|
||||
option={activeConfig}
|
||||
onBack={returnToConfigTrigger}
|
||||
onChange={onConfigChange}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
return (
|
||||
<>
|
||||
<Command>
|
||||
{showModelFilter && <CommandInput placeholder={t("agents:filterModels")} className="h-8" />}
|
||||
<CommandList className="max-h-60">
|
||||
<CommandEmpty>{t("agents:noModelsFound")}</CommandEmpty>
|
||||
<CommandGroup heading={t("agents:modelHeading")}>
|
||||
{modelOptions.map((model) => (
|
||||
<ModelRow
|
||||
key={model.id}
|
||||
model={model}
|
||||
selected={model.id === currentModelValue}
|
||||
loading={configOptionsLoading}
|
||||
onSelect={onModelSelect}
|
||||
/>
|
||||
))}
|
||||
</CommandGroup>
|
||||
</CommandList>
|
||||
</Command>
|
||||
{configOptionsLoading ? (
|
||||
<>
|
||||
<Separator />
|
||||
<div
|
||||
className="flex min-h-9 items-center gap-2 px-2 text-xs text-muted-foreground"
|
||||
data-testid="model-config-options-loading"
|
||||
role="status"
|
||||
aria-label={t("agents:resolvingModelOptions")}
|
||||
>
|
||||
<Spinner aria-hidden="true" className="h-3.5 w-3.5" />
|
||||
<span aria-hidden="true">{t("agents:resolvingModelOptions")}</span>
|
||||
</div>
|
||||
</>
|
||||
) : (
|
||||
extraConfigOptions.length > 0 && (
|
||||
<>
|
||||
<Separator />
|
||||
<ScrollArea className="max-h-40 pr-2">
|
||||
<div className="space-y-1">
|
||||
{extraConfigOptions.map((option) => (
|
||||
<ConfigOptionTrigger
|
||||
key={option.id}
|
||||
option={option}
|
||||
onSelect={() => onConfigSelect(option.id)}
|
||||
triggerRef={(element) => {
|
||||
triggerRefs.current[option.id] = element;
|
||||
}}
|
||||
/>
|
||||
))}
|
||||
</div>
|
||||
</ScrollArea>
|
||||
</>
|
||||
)
|
||||
)}
|
||||
</>
|
||||
);
|
||||
}
|
||||
|
||||
export type ModelConfigSelectorProps = {
|
||||
modelOptions: ModelSelectorOption[];
|
||||
currentModel: string | null;
|
||||
@@ -592,9 +301,7 @@ export const ModelConfigSelector = memo(function ModelConfigSelector({
|
||||
triggerSummary === "changed"
|
||||
? triggerDetails(modelOptions, currentModel, modelConfig, extraConfigOptions, t)
|
||||
: undefined;
|
||||
// The (fallback) marker is a live WS signal, not replayed on reload. When
|
||||
// it is active, explain on the trigger that the note is transient so users
|
||||
// are not surprised when the marker disappears after a refresh.
|
||||
// The fallback marker is live-only, so explain when it is active.
|
||||
const triggerTitle = currentModelSuffix ? t("settings:fallbackNoteLive") : undefined;
|
||||
|
||||
const hasExtraConfigOptions = extraConfigOptions.length > 0;
|
||||
|
||||
@@ -0,0 +1,79 @@
|
||||
import { Badge } from "@kandev/ui/badge";
|
||||
|
||||
import type { Branch } from "@/lib/types/http";
|
||||
import { t } from "@/lib/i18n";
|
||||
import type { PillOption } from "./task-create-dialog-pill";
|
||||
|
||||
// Conventional default branches surfaced at the top of the dropdown when no
|
||||
// search term is active. cmdk preserves option order on empty queries, so a
|
||||
// stable sort here lifts main/master/develop above feature branches.
|
||||
const PREFERRED_BRANCH_NAMES = ["main", "master", "develop"];
|
||||
|
||||
function branchPriority(b: Branch): number {
|
||||
const idx = PREFERRED_BRANCH_NAMES.indexOf(b.name);
|
||||
if (idx === -1) return PREFERRED_BRANCH_NAMES.length;
|
||||
return idx;
|
||||
}
|
||||
|
||||
export function sortBranches(branches: Branch[]): Branch[] {
|
||||
return [...branches].sort((a, b) => {
|
||||
const pa = branchPriority(a);
|
||||
const pb = branchPriority(b);
|
||||
if (pa !== pb) return pa - pb;
|
||||
// Within the same priority bucket, locals before remotes. This matches
|
||||
// the auto-select preference (`main` over `origin/main`).
|
||||
if (a.type !== b.type) return a.type === "local" ? -1 : 1;
|
||||
return 0;
|
||||
});
|
||||
}
|
||||
|
||||
const BRANCH_SEGMENT_RE = /[/_.\-\s]+/;
|
||||
|
||||
export function buildBranchKeywords(name: string, remote?: string): string[] {
|
||||
const out = new Set<string>();
|
||||
out.add(name);
|
||||
const leafIdx = name.lastIndexOf("/");
|
||||
if (leafIdx >= 0) out.add(name.slice(leafIdx + 1));
|
||||
for (const seg of name.split(BRANCH_SEGMENT_RE)) {
|
||||
if (seg) out.add(seg);
|
||||
}
|
||||
if (remote) out.add(remote);
|
||||
return Array.from(out);
|
||||
}
|
||||
|
||||
export function branchToOption(b: Branch): PillOption {
|
||||
// Remote branches keep their "origin/" prefix so they're distinguishable
|
||||
// from local branches with the same short name (e.g. "main" vs "origin/main").
|
||||
// Without the prefix, the dropdown shows two indistinguishable rows.
|
||||
const display = b.type === "remote" && b.remote ? `${b.remote}/${b.name}` : b.name;
|
||||
// `||` (not `??`) so an empty-string `remote` falls back too. Provider-backed
|
||||
// workspace repos (URL-added) list branches without a tracking remote, so the
|
||||
// backend sends `remote: ""`; `??` would render an invisible empty badge.
|
||||
const badge = b.type === "local" ? "local" : b.remote || "remote";
|
||||
return {
|
||||
value: display,
|
||||
label: display,
|
||||
keywords: buildBranchKeywords(b.name, b.remote),
|
||||
renderLabel: () => (
|
||||
<span className="flex min-w-0 flex-1 items-center justify-between gap-2">
|
||||
<span className="truncate" title={display}>
|
||||
{display}
|
||||
</span>
|
||||
<Badge variant="outline" className="text-xs shrink-0">
|
||||
{badge}
|
||||
</Badge>
|
||||
</span>
|
||||
),
|
||||
};
|
||||
}
|
||||
|
||||
export function computeBranchPlaceholder(
|
||||
hasRepo: boolean,
|
||||
loading: boolean,
|
||||
optionCount: number,
|
||||
): string {
|
||||
if (!hasRepo) return "branch";
|
||||
if (loading) return "loading…";
|
||||
if (optionCount === 0) return t("task:noBranchesShort");
|
||||
return "branch";
|
||||
}
|
||||
@@ -32,7 +32,7 @@ import { getExecutorIcon } from "@/lib/executor-icons";
|
||||
import { AgentLogo } from "@/components/agent-logo";
|
||||
import { getCapabilityWarning } from "@/lib/capability-warning";
|
||||
import { useTouchDrawer } from "@/hooks/use-compact-task-chrome";
|
||||
import { buildBranchKeywords } from "./task-create-dialog-pill";
|
||||
import { buildBranchKeywords } from "./task-create-dialog-branch-options";
|
||||
|
||||
type OptionItem = {
|
||||
value: string;
|
||||
|
||||
@@ -80,13 +80,12 @@ vi.mock("@kandev/ui/tooltip", async () => {
|
||||
};
|
||||
});
|
||||
|
||||
import { Pill, type PillOption } from "./task-create-dialog-pill";
|
||||
import {
|
||||
sortBranches,
|
||||
branchToOption,
|
||||
computeBranchPlaceholder,
|
||||
Pill,
|
||||
type PillOption,
|
||||
} from "./task-create-dialog-pill";
|
||||
sortBranches,
|
||||
} from "./task-create-dialog-branch-options";
|
||||
|
||||
const CREATE_REPOSITORY = "Create new repository";
|
||||
|
||||
@@ -319,6 +318,37 @@ describe("Pill tooltip", () => {
|
||||
});
|
||||
|
||||
describe("Pill popover", () => {
|
||||
it("puts the current option first with a persistent selected surface", () => {
|
||||
render(
|
||||
<Pill
|
||||
icon={<span aria-hidden="true" />}
|
||||
value="Current label"
|
||||
selectedValue="current"
|
||||
placeholder="repository"
|
||||
options={[
|
||||
{ value: "first", label: "First" },
|
||||
{ value: "current", label: "Current" },
|
||||
{ value: "last", label: "Last" },
|
||||
]}
|
||||
onSelect={vi.fn()}
|
||||
searchPlaceholder="Search repositories..."
|
||||
emptyMessage="No repositories"
|
||||
/>,
|
||||
);
|
||||
|
||||
fireEvent.click(screen.getByText("Current label"));
|
||||
|
||||
const options = screen.getAllByRole("option");
|
||||
expect(options.map((option) => option.textContent?.trim())).toEqual([
|
||||
"Current",
|
||||
"First",
|
||||
"Last",
|
||||
]);
|
||||
expect(options[0].className).toContain("bg-card");
|
||||
expect(options[0].className).toContain("border-primary/50");
|
||||
expect(options[0].querySelector("svg")).not.toBeNull();
|
||||
});
|
||||
|
||||
it("portals selector content outside the trigger container", () => {
|
||||
render(
|
||||
<div data-testid="clipping-host" className="overflow-hidden">
|
||||
|
||||
@@ -1,10 +1,11 @@
|
||||
"use client";
|
||||
|
||||
import { useCallback, useEffect, useRef, useState } from "react";
|
||||
import { useCallback, useRef, useState } from "react";
|
||||
import { IconCheck } from "@tabler/icons-react";
|
||||
import { cn } from "@/lib/utils";
|
||||
import { prioritizeSelectedOption, selectorOptionClassName } from "@/lib/utils/selector-options";
|
||||
import { Popover, PopoverContent, PopoverTrigger } from "@kandev/ui/popover";
|
||||
import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip";
|
||||
import { Badge } from "@kandev/ui/badge";
|
||||
import {
|
||||
Command,
|
||||
CommandEmpty,
|
||||
@@ -13,11 +14,10 @@ import {
|
||||
CommandItem,
|
||||
CommandList,
|
||||
} from "@kandev/ui/command";
|
||||
import type { Branch } from "@/lib/types/http";
|
||||
import { BranchRefreshButton } from "@/components/branch-refresh-button";
|
||||
import { useTaskCreateDialogPopoverContainer } from "@/hooks/use-task-create-dialog-popover-container";
|
||||
import { usePillTooltipSuppression } from "@/hooks/use-pill-tooltip-suppression";
|
||||
import { useTooltipMountGate } from "@/hooks/use-tooltip-mount-gate";
|
||||
import { t } from "@/lib/i18n";
|
||||
|
||||
export type PillOption = {
|
||||
value: string;
|
||||
@@ -43,6 +43,8 @@ export type PillAction = {
|
||||
type PillProps = {
|
||||
icon: React.ReactNode;
|
||||
value: string;
|
||||
/** Option value used for selected-first ordering when the trigger label differs. */
|
||||
selectedValue?: string;
|
||||
placeholder: string;
|
||||
options: PillOption[];
|
||||
onSelect: (value: string) => void;
|
||||
@@ -87,35 +89,48 @@ function pillActiveClass(flat: boolean): string {
|
||||
|
||||
function PillCommandList({
|
||||
options,
|
||||
value,
|
||||
onSelect,
|
||||
onPointerSelect,
|
||||
setOpen,
|
||||
emptyMessage,
|
||||
}: {
|
||||
options: PillOption[];
|
||||
value: string;
|
||||
onSelect: (value: string) => void;
|
||||
onPointerSelect: (pointerType: string) => void;
|
||||
setOpen: (open: boolean) => void;
|
||||
emptyMessage: string;
|
||||
}) {
|
||||
const orderedOptions = prioritizeSelectedOption(options, value, (option) => option.value);
|
||||
|
||||
return (
|
||||
<CommandList>
|
||||
<CommandEmpty>{emptyMessage}</CommandEmpty>
|
||||
<CommandGroup>
|
||||
{options.map((option) => (
|
||||
<CommandItem
|
||||
key={option.value}
|
||||
value={option.value}
|
||||
keywords={[option.label, ...(option.keywords ?? [])]}
|
||||
onPointerDown={(event) => onPointerSelect(event.pointerType)}
|
||||
onSelect={() => {
|
||||
onSelect(option.value);
|
||||
setOpen(false);
|
||||
}}
|
||||
>
|
||||
{option.renderLabel ? option.renderLabel() : option.label}
|
||||
</CommandItem>
|
||||
))}
|
||||
{orderedOptions.map((option) => {
|
||||
const selected = option.value === value;
|
||||
return (
|
||||
<CommandItem
|
||||
key={option.value}
|
||||
value={option.value}
|
||||
keywords={[option.label, ...(option.keywords ?? [])]}
|
||||
onPointerDown={(event) => onPointerSelect(event.pointerType)}
|
||||
onSelect={() => {
|
||||
onSelect(option.value);
|
||||
setOpen(false);
|
||||
}}
|
||||
className={selectorOptionClassName(selected)}
|
||||
>
|
||||
<div className="min-w-0 flex-1">
|
||||
{option.renderLabel ? option.renderLabel() : option.label}
|
||||
</div>
|
||||
<IconCheck
|
||||
className={cn("absolute right-2 h-4 w-4", selected ? "opacity-100" : "opacity-0")}
|
||||
/>
|
||||
</CommandItem>
|
||||
);
|
||||
})}
|
||||
</CommandGroup>
|
||||
</CommandList>
|
||||
);
|
||||
@@ -134,88 +149,6 @@ function pillTriggerClass(disabled: boolean, flat: boolean, hasValue: boolean):
|
||||
);
|
||||
}
|
||||
|
||||
function usePillTooltipSuppression(open: boolean) {
|
||||
const [suppressTooltip, setSuppressTooltip] = useState(false);
|
||||
const suppressTooltipRef = useRef(false);
|
||||
const pointerInsideTriggerRef = useRef(false);
|
||||
const suppressionReleaseOnExitRef = useRef(true);
|
||||
const suppressionReleaseArmedRef = useRef(false);
|
||||
const suppressionReleaseFrameRef = useRef<number | null>(null);
|
||||
const clearTooltipSuppression = useCallback(() => {
|
||||
if (suppressionReleaseFrameRef.current !== null) {
|
||||
cancelAnimationFrame(suppressionReleaseFrameRef.current);
|
||||
suppressionReleaseFrameRef.current = null;
|
||||
}
|
||||
suppressionReleaseArmedRef.current = false;
|
||||
suppressionReleaseOnExitRef.current = true;
|
||||
suppressTooltipRef.current = false;
|
||||
setSuppressTooltip(false);
|
||||
}, []);
|
||||
const suppressTooltipUntilLeave = useCallback(
|
||||
(releaseOnExit = true) => {
|
||||
if (suppressionReleaseFrameRef.current !== null) {
|
||||
cancelAnimationFrame(suppressionReleaseFrameRef.current);
|
||||
}
|
||||
suppressionReleaseOnExitRef.current = releaseOnExit;
|
||||
suppressionReleaseArmedRef.current = false;
|
||||
suppressTooltipRef.current = true;
|
||||
setSuppressTooltip(true);
|
||||
suppressionReleaseFrameRef.current = requestAnimationFrame(() => {
|
||||
suppressionReleaseFrameRef.current = requestAnimationFrame(() => {
|
||||
suppressionReleaseFrameRef.current = null;
|
||||
if (!pointerInsideTriggerRef.current && suppressionReleaseOnExitRef.current) {
|
||||
clearTooltipSuppression();
|
||||
return;
|
||||
}
|
||||
suppressionReleaseArmedRef.current = true;
|
||||
});
|
||||
});
|
||||
},
|
||||
[clearTooltipSuppression],
|
||||
);
|
||||
useEffect(
|
||||
() => () => {
|
||||
if (suppressionReleaseFrameRef.current !== null) {
|
||||
cancelAnimationFrame(suppressionReleaseFrameRef.current);
|
||||
}
|
||||
},
|
||||
[],
|
||||
);
|
||||
const handlePointerEnter = useCallback(
|
||||
(event: React.PointerEvent) => {
|
||||
pointerInsideTriggerRef.current = true;
|
||||
if (
|
||||
event.pointerType !== "touch" &&
|
||||
!suppressionReleaseOnExitRef.current &&
|
||||
suppressTooltipRef.current
|
||||
) {
|
||||
clearTooltipSuppression();
|
||||
}
|
||||
},
|
||||
[clearTooltipSuppression],
|
||||
);
|
||||
const handlePointerLeave = useCallback(() => {
|
||||
pointerInsideTriggerRef.current = false;
|
||||
if (!open && suppressionReleaseOnExitRef.current && suppressionReleaseArmedRef.current) {
|
||||
clearTooltipSuppression();
|
||||
}
|
||||
}, [clearTooltipSuppression, open]);
|
||||
const handleBlur = useCallback(() => {
|
||||
if (!open && suppressionReleaseOnExitRef.current && suppressionReleaseArmedRef.current) {
|
||||
clearTooltipSuppression();
|
||||
}
|
||||
}, [clearTooltipSuppression, open]);
|
||||
|
||||
return {
|
||||
suppressTooltip,
|
||||
suppressTooltipRef,
|
||||
suppressTooltipUntilLeave,
|
||||
handlePointerEnter,
|
||||
handlePointerLeave,
|
||||
handleBlur,
|
||||
};
|
||||
}
|
||||
|
||||
function DisabledPillTooltip({
|
||||
open,
|
||||
onOpenChange,
|
||||
@@ -241,6 +174,22 @@ function DisabledPillTooltip({
|
||||
);
|
||||
}
|
||||
|
||||
function renderDisabledPillTooltip(
|
||||
tooltipOpenState: boolean,
|
||||
onOpenChange: (open: boolean) => void,
|
||||
triggerButton: React.ReactNode,
|
||||
disabledReason: string,
|
||||
): React.ReactElement {
|
||||
return (
|
||||
<DisabledPillTooltip
|
||||
open={tooltipOpenState}
|
||||
onOpenChange={onOpenChange}
|
||||
triggerButton={triggerButton}
|
||||
disabledReason={disabledReason}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
function PillPopoverContent({
|
||||
filter,
|
||||
searchPlaceholder,
|
||||
@@ -248,6 +197,7 @@ function PillPopoverContent({
|
||||
refreshing,
|
||||
refreshLabel,
|
||||
options,
|
||||
value,
|
||||
onSelect,
|
||||
onPointerSelect,
|
||||
setOpen,
|
||||
@@ -261,6 +211,7 @@ function PillPopoverContent({
|
||||
refreshing?: boolean;
|
||||
refreshLabel?: string;
|
||||
options: PillOption[];
|
||||
value: string;
|
||||
onSelect: (value: string) => void;
|
||||
onPointerSelect: (pointerType: string) => void;
|
||||
setOpen: (open: boolean) => void;
|
||||
@@ -310,6 +261,7 @@ function PillPopoverContent({
|
||||
</div>
|
||||
<PillCommandList
|
||||
options={options}
|
||||
value={value}
|
||||
onSelect={onSelect}
|
||||
onPointerSelect={onPointerSelect}
|
||||
setOpen={setOpen}
|
||||
@@ -330,6 +282,7 @@ function PillPopover({
|
||||
refreshing,
|
||||
refreshLabel,
|
||||
options,
|
||||
value,
|
||||
onSelect,
|
||||
onPointerSelect,
|
||||
emptyMessage,
|
||||
@@ -345,6 +298,7 @@ function PillPopover({
|
||||
refreshing?: boolean;
|
||||
refreshLabel?: string;
|
||||
options: PillOption[];
|
||||
value: string;
|
||||
onSelect: (value: string) => void;
|
||||
onPointerSelect: (pointerType: string) => void;
|
||||
emptyMessage: string;
|
||||
@@ -361,6 +315,7 @@ function PillPopover({
|
||||
refreshing={refreshing}
|
||||
refreshLabel={refreshLabel}
|
||||
options={options}
|
||||
value={value}
|
||||
onSelect={onSelect}
|
||||
onPointerSelect={onPointerSelect}
|
||||
setOpen={setOpen}
|
||||
@@ -372,6 +327,88 @@ function PillPopover({
|
||||
);
|
||||
}
|
||||
|
||||
type PillPopoverShellProps = {
|
||||
open: boolean;
|
||||
setOpen: (open: boolean) => void;
|
||||
triggerButton: React.ReactElement;
|
||||
filter?: PillProps["filter"];
|
||||
searchPlaceholder: string;
|
||||
onRefresh?: () => void;
|
||||
refreshing?: boolean;
|
||||
refreshLabel?: string;
|
||||
options: PillOption[];
|
||||
value: string;
|
||||
onSelect: (value: string) => void;
|
||||
onPointerSelect: (pointerType: string) => void;
|
||||
emptyMessage: string;
|
||||
portalContainer: HTMLElement | null;
|
||||
action?: PillAction;
|
||||
tooltip?: string;
|
||||
tooltipOpenState: boolean;
|
||||
suppressTooltip: boolean;
|
||||
suppressTooltipRef: { current: boolean };
|
||||
handlePillTooltipOpenChange: (open: boolean) => void;
|
||||
suppressForSelection: () => void;
|
||||
};
|
||||
|
||||
function renderPillPopover({
|
||||
open,
|
||||
setOpen,
|
||||
triggerButton,
|
||||
filter,
|
||||
searchPlaceholder,
|
||||
onRefresh,
|
||||
refreshing,
|
||||
refreshLabel,
|
||||
options,
|
||||
value,
|
||||
onSelect,
|
||||
onPointerSelect,
|
||||
emptyMessage,
|
||||
portalContainer,
|
||||
action,
|
||||
tooltip,
|
||||
tooltipOpenState,
|
||||
suppressTooltip,
|
||||
suppressTooltipRef,
|
||||
handlePillTooltipOpenChange,
|
||||
suppressForSelection,
|
||||
}: PillPopoverShellProps): React.ReactElement {
|
||||
const popover = (
|
||||
<PillPopover
|
||||
open={open}
|
||||
setOpen={setOpen}
|
||||
triggerButton={triggerButton}
|
||||
filter={filter}
|
||||
searchPlaceholder={searchPlaceholder}
|
||||
onRefresh={onRefresh}
|
||||
refreshing={refreshing}
|
||||
refreshLabel={refreshLabel}
|
||||
options={options}
|
||||
value={value}
|
||||
onPointerSelect={onPointerSelect}
|
||||
onSelect={(selectedValue) => {
|
||||
if (tooltip) suppressForSelection();
|
||||
onSelect(selectedValue);
|
||||
}}
|
||||
emptyMessage={emptyMessage}
|
||||
portalContainer={portalContainer}
|
||||
action={action}
|
||||
/>
|
||||
);
|
||||
|
||||
if (!tooltip) return popover;
|
||||
|
||||
const tooltipOpen =
|
||||
open || suppressTooltip || suppressTooltipRef.current ? false : tooltipOpenState;
|
||||
return (
|
||||
<Tooltip open={tooltipOpen} onOpenChange={handlePillTooltipOpenChange}>
|
||||
{popover}
|
||||
<TooltipContent className="max-w-[calc(100vw-2rem)] break-all">{tooltip}</TooltipContent>
|
||||
</Tooltip>
|
||||
);
|
||||
}
|
||||
|
||||
function renderPillTriggerButton({
|
||||
icon,
|
||||
value,
|
||||
@@ -445,6 +482,7 @@ function usePillOpenHandlers(
|
||||
export function Pill({
|
||||
icon,
|
||||
value,
|
||||
selectedValue,
|
||||
placeholder,
|
||||
options,
|
||||
onSelect,
|
||||
@@ -498,130 +536,41 @@ export function Pill({
|
||||
onPointerLeave: tooltip ? handlePointerLeave : undefined,
|
||||
onBlur: tooltip ? handleBlur : undefined,
|
||||
});
|
||||
|
||||
// Disabled buttons swallow pointer/focus events, so the wrapper owns tooltip
|
||||
// focus while the button stays disabled.
|
||||
// Disabled buttons swallow events, so the wrapper owns tooltip focus.
|
||||
if (disabled && disabledReason && !open) {
|
||||
return (
|
||||
<DisabledPillTooltip
|
||||
open={tooltipOpenState}
|
||||
onOpenChange={handleTooltipOpenChange}
|
||||
triggerButton={triggerButton}
|
||||
disabledReason={disabledReason}
|
||||
/>
|
||||
return renderDisabledPillTooltip(
|
||||
tooltipOpenState,
|
||||
handleTooltipOpenChange,
|
||||
triggerButton,
|
||||
disabledReason,
|
||||
);
|
||||
}
|
||||
|
||||
const popover = (
|
||||
<PillPopover
|
||||
open={open}
|
||||
setOpen={setOpen}
|
||||
triggerButton={
|
||||
tooltip ? <TooltipTrigger asChild>{triggerButton}</TooltipTrigger> : triggerButton
|
||||
}
|
||||
filter={filter}
|
||||
searchPlaceholder={searchPlaceholder}
|
||||
onRefresh={onRefresh}
|
||||
refreshing={refreshing}
|
||||
refreshLabel={refreshLabel}
|
||||
options={options}
|
||||
onPointerSelect={recordPointerSelection}
|
||||
onSelect={(selectedValue) => {
|
||||
if (tooltip) suppressForSelection();
|
||||
onSelect(selectedValue);
|
||||
}}
|
||||
emptyMessage={emptyMessage}
|
||||
portalContainer={portalContainer}
|
||||
action={action}
|
||||
/>
|
||||
);
|
||||
|
||||
if (!tooltip) return popover;
|
||||
|
||||
// Suppress the hover tooltip while the popover is open and until the pointer
|
||||
// leaves after close, so a selection cannot disclose a tooltip underneath it.
|
||||
const tooltipOpen =
|
||||
open || suppressTooltip || suppressTooltipRef.current ? false : tooltipOpenState;
|
||||
return (
|
||||
<Tooltip open={tooltipOpen} onOpenChange={handlePillTooltipOpenChange}>
|
||||
{popover}
|
||||
<TooltipContent className="max-w-[calc(100vw-2rem)] break-all">{tooltip}</TooltipContent>
|
||||
</Tooltip>
|
||||
);
|
||||
}
|
||||
|
||||
// --- Branch utilities ---
|
||||
|
||||
// Conventional default branches surfaced at the top of the dropdown when no
|
||||
// search term is active. cmdk preserves option order on empty queries, so a
|
||||
// stable sort here lifts main/master/develop above feature branches.
|
||||
const PREFERRED_BRANCH_NAMES = ["main", "master", "develop"];
|
||||
|
||||
function branchPriority(b: Branch): number {
|
||||
const idx = PREFERRED_BRANCH_NAMES.indexOf(b.name);
|
||||
if (idx === -1) return PREFERRED_BRANCH_NAMES.length;
|
||||
return idx;
|
||||
}
|
||||
|
||||
export function sortBranches(branches: Branch[]): Branch[] {
|
||||
return [...branches].sort((a, b) => {
|
||||
const pa = branchPriority(a);
|
||||
const pb = branchPriority(b);
|
||||
if (pa !== pb) return pa - pb;
|
||||
// Within the same priority bucket, locals before remotes — matches the
|
||||
// auto-select preference (`main` over `origin/main`).
|
||||
if (a.type !== b.type) return a.type === "local" ? -1 : 1;
|
||||
return 0;
|
||||
return renderPillPopover({
|
||||
open,
|
||||
setOpen,
|
||||
triggerButton: tooltip ? (
|
||||
<TooltipTrigger asChild>{triggerButton}</TooltipTrigger>
|
||||
) : (
|
||||
triggerButton
|
||||
),
|
||||
filter,
|
||||
searchPlaceholder,
|
||||
onRefresh,
|
||||
refreshing,
|
||||
refreshLabel,
|
||||
options,
|
||||
value: selectedValue ?? value,
|
||||
onPointerSelect: recordPointerSelection,
|
||||
onSelect,
|
||||
emptyMessage,
|
||||
portalContainer,
|
||||
action,
|
||||
tooltip,
|
||||
tooltipOpenState,
|
||||
suppressTooltip,
|
||||
suppressTooltipRef,
|
||||
handlePillTooltipOpenChange,
|
||||
suppressForSelection,
|
||||
});
|
||||
}
|
||||
|
||||
const BRANCH_SEGMENT_RE = /[/_.\-\s]+/;
|
||||
|
||||
export function buildBranchKeywords(name: string, remote?: string): string[] {
|
||||
const out = new Set<string>();
|
||||
out.add(name);
|
||||
const leafIdx = name.lastIndexOf("/");
|
||||
if (leafIdx >= 0) out.add(name.slice(leafIdx + 1));
|
||||
for (const seg of name.split(BRANCH_SEGMENT_RE)) {
|
||||
if (seg) out.add(seg);
|
||||
}
|
||||
if (remote) out.add(remote);
|
||||
return Array.from(out);
|
||||
}
|
||||
|
||||
export function branchToOption(b: Branch): PillOption {
|
||||
// Remote branches keep their "origin/" prefix so they're distinguishable
|
||||
// from local branches with the same short name (e.g. "main" vs "origin/main").
|
||||
// Without the prefix, the dropdown shows two indistinguishable rows.
|
||||
const display = b.type === "remote" && b.remote ? `${b.remote}/${b.name}` : b.name;
|
||||
// `||` (not `??`) so an empty-string `remote` falls back too. Provider-backed
|
||||
// workspace repos (URL-added) list branches without a tracking remote, so the
|
||||
// backend sends `remote: ""`; `??` would render an invisible empty badge.
|
||||
const badge = b.type === "local" ? "local" : b.remote || "remote";
|
||||
return {
|
||||
value: display,
|
||||
label: display,
|
||||
keywords: buildBranchKeywords(b.name, b.remote),
|
||||
renderLabel: () => (
|
||||
<span className="flex min-w-0 flex-1 items-center justify-between gap-2">
|
||||
<span className="truncate" title={display}>
|
||||
{display}
|
||||
</span>
|
||||
<Badge variant="outline" className="text-xs shrink-0">
|
||||
{badge}
|
||||
</Badge>
|
||||
</span>
|
||||
),
|
||||
};
|
||||
}
|
||||
|
||||
export function computeBranchPlaceholder(
|
||||
hasRepo: boolean,
|
||||
loading: boolean,
|
||||
optionCount: number,
|
||||
): string {
|
||||
if (!hasRepo) return "branch";
|
||||
if (loading) return "loading…";
|
||||
if (optionCount === 0) return t("task:noBranchesShort");
|
||||
return "branch";
|
||||
}
|
||||
|
||||
@@ -10,12 +10,12 @@ import { Button } from "@kandev/ui/button";
|
||||
import { Popover, PopoverContent, PopoverTrigger } from "@kandev/ui/popover";
|
||||
import { Spinner } from "@kandev/ui/spinner";
|
||||
import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip";
|
||||
import { Pill } from "@/components/task-create-dialog-pill";
|
||||
import {
|
||||
Pill,
|
||||
branchToOption,
|
||||
computeBranchPlaceholder,
|
||||
sortBranches,
|
||||
} from "@/components/task-create-dialog-pill";
|
||||
} from "@/components/task-create-dialog-branch-options";
|
||||
import { scoreBranch } from "@/lib/utils/branch-filter";
|
||||
import type {
|
||||
RemoteRepository,
|
||||
|
||||
@@ -17,14 +17,12 @@ import type { TaskRepoRow } from "@/components/task-create-dialog-types";
|
||||
import { cn, formatUserHomePath } from "@/lib/utils";
|
||||
import { scoreBranch } from "@/lib/utils/branch-filter";
|
||||
import { scoreRepo } from "@/lib/utils/repo-filter";
|
||||
import { Pill, type PillAction, type PillOption } from "@/components/task-create-dialog-pill";
|
||||
import {
|
||||
Pill,
|
||||
sortBranches,
|
||||
branchToOption,
|
||||
computeBranchPlaceholder,
|
||||
type PillAction,
|
||||
type PillOption,
|
||||
} from "@/components/task-create-dialog-pill";
|
||||
sortBranches,
|
||||
} from "@/components/task-create-dialog-branch-options";
|
||||
import {
|
||||
computeBranchIntent,
|
||||
computeBranchPrefix,
|
||||
@@ -444,6 +442,7 @@ function RepoChip({
|
||||
<Pill
|
||||
icon={<IconCode className="h-3 w-3 shrink-0 text-muted-foreground" />}
|
||||
value={repoLabel}
|
||||
selectedValue={row.repositoryId || row.localPath || ""}
|
||||
placeholder={t("task:repository")}
|
||||
options={repoOptions}
|
||||
onSelect={onRepositoryChange}
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { cleanup, render, screen } from "@testing-library/react";
|
||||
import { cleanup, fireEvent, render, screen } from "@testing-library/react";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import type { Branch } from "@/lib/types/http";
|
||||
import { BranchPickerList } from "./branch-picker-list";
|
||||
@@ -6,6 +6,54 @@ import { BranchPickerList } from "./branch-picker-list";
|
||||
afterEach(() => cleanup());
|
||||
|
||||
describe("BranchPickerList", () => {
|
||||
it("puts the current branch first and keeps its selected surface", () => {
|
||||
const branches: Branch[] = [
|
||||
{ name: "feature/local", type: "local" },
|
||||
{ name: "main", type: "remote", remote: "origin" },
|
||||
{ name: "develop", type: "remote", remote: "origin" },
|
||||
];
|
||||
|
||||
render(
|
||||
<BranchPickerList
|
||||
branches={branches}
|
||||
isLoadingBranches={false}
|
||||
currentBase="main"
|
||||
onSelect={() => {}}
|
||||
/>,
|
||||
);
|
||||
|
||||
const options = screen.getAllByRole("option");
|
||||
expect(options.map((option) => option.textContent?.trim())).toEqual([
|
||||
"main",
|
||||
"feature/local",
|
||||
"develop",
|
||||
]);
|
||||
expect(options[0].getAttribute("aria-selected")).toBe("true");
|
||||
expect(options[0].className).toContain("bg-card");
|
||||
expect(options[0].className).toContain("border-primary/50");
|
||||
});
|
||||
|
||||
it("does not inject the current branch into filtered results", () => {
|
||||
render(
|
||||
<BranchPickerList
|
||||
branches={[
|
||||
{ name: "main", type: "remote", remote: "origin" },
|
||||
{ name: "develop", type: "remote", remote: "origin" },
|
||||
]}
|
||||
isLoadingBranches={false}
|
||||
currentBase="main"
|
||||
onSelect={() => {}}
|
||||
/>,
|
||||
);
|
||||
|
||||
const filter = screen.getByTestId("base-branch-picker-filter");
|
||||
fireEvent.change(filter, { target: { value: "develop" } });
|
||||
|
||||
expect(screen.getAllByRole("option").map((option) => option.textContent?.trim())).toEqual([
|
||||
"develop",
|
||||
]);
|
||||
});
|
||||
|
||||
it("can limit recovery choices to remote branches", () => {
|
||||
const branches: Branch[] = [
|
||||
{ name: "feature/local-only", type: "local" },
|
||||
|
||||
@@ -3,6 +3,7 @@
|
||||
import { useMemo, useState } from "react";
|
||||
import { IconLoader2 } from "@tabler/icons-react";
|
||||
import { cn } from "@/lib/utils";
|
||||
import { prioritizeSelectedOption } from "@/lib/utils/selector-options";
|
||||
import type { Branch } from "@/lib/types/http";
|
||||
import { useTranslation } from "react-i18next";
|
||||
|
||||
@@ -41,6 +42,10 @@ export function BranchPickerList({
|
||||
? uniqueByName.filter((branch) => branch.name.toLowerCase().includes(query))
|
||||
: uniqueByName;
|
||||
}, [filter, uniqueByName]);
|
||||
const orderedBranches = useMemo(() => {
|
||||
if (filter.trim()) return filtered;
|
||||
return prioritizeSelectedOption(filtered, currentBase, (branch) => branch.name);
|
||||
}, [currentBase, filter, filtered]);
|
||||
|
||||
return (
|
||||
<div className="flex min-w-0 flex-col gap-1">
|
||||
@@ -54,7 +59,7 @@ export function BranchPickerList({
|
||||
autoFocus
|
||||
/>
|
||||
<BranchPickerListBody
|
||||
branches={filtered}
|
||||
branches={orderedBranches}
|
||||
isLoadingBranches={isLoadingBranches}
|
||||
currentBase={currentBase}
|
||||
onSelect={onSelect}
|
||||
@@ -104,7 +109,8 @@ function BranchPickerListBody({
|
||||
data-testid={`${testIdPrefix}-option-${branch.name}`}
|
||||
className={cn(
|
||||
"flex min-h-11 w-full items-center gap-2 rounded border border-transparent px-2 py-1.5 text-left text-xs cursor-pointer hover:bg-muted sm:min-h-8",
|
||||
branch.name === currentBase && "border-primary/50 bg-card font-medium",
|
||||
branch.name === currentBase &&
|
||||
"border-primary/50 bg-card font-medium ring-1 ring-inset ring-primary/20",
|
||||
)}
|
||||
onClick={() => onSelect(branch.name)}
|
||||
>
|
||||
|
||||
@@ -16,6 +16,7 @@ import { useAvailableAgents } from "@/hooks/domains/settings/use-available-agent
|
||||
import { useSettingsData } from "@/hooks/domains/settings/use-settings-data";
|
||||
import { setSessionMode } from "@/lib/api/domains/session-api";
|
||||
import { cn } from "@/lib/utils";
|
||||
import { prioritizeSelectedOption } from "@/lib/utils/selector-options";
|
||||
import type { Agent, AgentProfile, AvailableAgent } from "@/lib/types/http";
|
||||
import { useTranslation } from "react-i18next";
|
||||
|
||||
@@ -84,7 +85,10 @@ function buildModeState(
|
||||
}
|
||||
if (availableModes.length <= 1) return undefined;
|
||||
if (availableModes.some((m) => m.id === currentModeId)) {
|
||||
return { currentModeId, availableModes };
|
||||
return {
|
||||
currentModeId,
|
||||
availableModes: prioritizeSelectedOption(availableModes, currentModeId, (mode) => mode.id),
|
||||
};
|
||||
}
|
||||
return {
|
||||
currentModeId,
|
||||
@@ -199,7 +203,11 @@ export const ModeSelector = memo(function ModeSelector({
|
||||
<DropdownMenuItem
|
||||
key={mode.id}
|
||||
onClick={() => handleModeChange(mode.id)}
|
||||
className={`cursor-pointer relative pr-7 ${mode.id === modeState.currentModeId ? "bg-muted" : ""}`}
|
||||
className={cn(
|
||||
"relative min-h-11 cursor-pointer border border-transparent pr-7 sm:min-h-8",
|
||||
mode.id === modeState.currentModeId &&
|
||||
"border-primary/50 bg-card font-medium hover:bg-card",
|
||||
)}
|
||||
>
|
||||
<div className="min-w-0 flex-1">
|
||||
<div>{mode.name}</div>
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { cleanup, render, screen } from "@testing-library/react";
|
||||
import { cleanup, fireEvent, render, screen } from "@testing-library/react";
|
||||
import { TooltipProvider } from "@kandev/ui/tooltip";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
@@ -38,8 +38,8 @@ const mocks = vi.hoisted(() => {
|
||||
"session-1": {
|
||||
currentModeId: "full-access",
|
||||
availableModes: [
|
||||
{ id: "full-access", name: "Full access" },
|
||||
{ id: "read-only", name: "Read only" },
|
||||
{ id: "full-access", name: "Full access" },
|
||||
],
|
||||
},
|
||||
},
|
||||
@@ -156,4 +156,21 @@ describe("task selector trigger styling", () => {
|
||||
|
||||
expect(screen.getByTestId("session-mode-selector").className).toContain("max-w-mode");
|
||||
});
|
||||
|
||||
it("puts the current mode first with a persistent selected surface", () => {
|
||||
render(
|
||||
<TooltipProvider>
|
||||
<ModeSelector sessionId="session-1" />
|
||||
</TooltipProvider>,
|
||||
);
|
||||
|
||||
const trigger = screen.getByTestId("session-mode-selector");
|
||||
fireEvent.pointerDown(trigger);
|
||||
fireEvent.click(trigger);
|
||||
|
||||
const modes = screen.getAllByRole("menuitem");
|
||||
expect(modes.map((mode) => mode.textContent?.trim())).toEqual(["Full access", "Read only"]);
|
||||
expect(modes[0].className).toContain("bg-card");
|
||||
expect(modes[0].className).toContain("border-primary/50");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -121,14 +121,21 @@ test.describe("Mobile chat model selector", () => {
|
||||
await expect(trigger).toHaveText("Mock Fast", { timeout: 15_000 });
|
||||
|
||||
await trigger.tap();
|
||||
await expect(testPage.getByRole("option", { name: /Mock Smart/ })).toBeVisible({
|
||||
const modelListbox = testPage.getByRole("listbox");
|
||||
await expect(modelListbox.getByRole("option", { name: /Mock Smart/ })).toBeVisible({
|
||||
timeout: 5_000,
|
||||
});
|
||||
const modelOptions = modelListbox.getByRole("option");
|
||||
await expect(modelOptions.first()).toContainText("Mock Fast");
|
||||
await expect(modelOptions.first()).toHaveClass(/bg-card/);
|
||||
await expect(modelOptions.first()).toHaveClass(/border-primary\/50/);
|
||||
await expect(
|
||||
testPage.getByRole("option", { name: /Mock Fast/ }).getByTitle("Fast mock model for testing"),
|
||||
modelListbox
|
||||
.getByRole("option", { name: /Mock Fast/ })
|
||||
.getByTitle("Fast mock model for testing"),
|
||||
).toBeVisible();
|
||||
await expect(
|
||||
testPage
|
||||
modelListbox
|
||||
.getByRole("option", { name: /Mock Smart/ })
|
||||
.getByTitle("Smart mock model for testing"),
|
||||
).toBeVisible();
|
||||
@@ -205,5 +212,8 @@ test.describe("Mobile chat model selector", () => {
|
||||
expect(popoverBox).not.toBeNull();
|
||||
expect(popoverBox!.x).toBeGreaterThanOrEqual(0);
|
||||
expect(popoverBox!.x + popoverBox!.width).toBeLessThanOrEqual(viewport!.width);
|
||||
expect(await testPage.evaluate(() => document.documentElement.scrollWidth)).toBeLessThanOrEqual(
|
||||
viewport!.width,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -67,7 +67,12 @@ test.describe("Chat model selector — RPC failure", () => {
|
||||
await testPage.route("**/set-config-option", fail);
|
||||
|
||||
await trigger.click();
|
||||
const smartRow = testPage.getByRole("option", { name: /Mock Smart/ });
|
||||
const modelListbox = testPage.getByRole("listbox");
|
||||
const modelOptions = modelListbox.getByRole("option");
|
||||
await expect(modelOptions.first()).toContainText("Mock Fast");
|
||||
await expect(modelOptions.first()).toHaveClass(/bg-card/);
|
||||
await expect(modelOptions.first()).toHaveClass(/border-primary\/50/);
|
||||
const smartRow = modelListbox.getByRole("option", { name: /Mock Smart/ });
|
||||
await expect(smartRow).toBeVisible({ timeout: 5_000 });
|
||||
await smartRow.click();
|
||||
|
||||
|
||||
@@ -664,7 +664,12 @@ test.describe("Branch refresh + filter", () => {
|
||||
|
||||
// The enabled button and a rendered option together establish that both
|
||||
// the branch request and the popover's selected-repository state settled.
|
||||
await expect(testPage.getByRole("option").first()).toBeVisible({ timeout: 10_000 });
|
||||
const branchListbox = testPage.getByRole("listbox");
|
||||
const firstBranchOption = branchListbox.getByRole("option").first();
|
||||
await expect(firstBranchOption).toBeVisible({ timeout: 10_000 });
|
||||
await expect(firstBranchOption).toHaveAttribute("data-value", "main");
|
||||
await expect(firstBranchOption).toHaveClass(/bg-card/);
|
||||
await expect(firstBranchOption).toHaveClass(/border-primary\/50/);
|
||||
|
||||
await expect(testPage.getByTestId("repo-chip").first()).toHaveAttribute(
|
||||
"data-repository-id",
|
||||
|
||||
@@ -97,6 +97,29 @@ test.describe("Task creation", () => {
|
||||
await expect(testPage.getByTestId("agent-profile-selector")).not.toContainText(
|
||||
"Select agent...",
|
||||
);
|
||||
|
||||
const repositorySelector = testPage.getByTestId("repo-chip-trigger").first();
|
||||
await repositorySelector.click();
|
||||
const repositoryListbox = testPage.getByRole("listbox");
|
||||
const repositoryOptions = repositoryListbox.getByRole("option");
|
||||
await expect(repositoryOptions.first()).toHaveAttribute("aria-selected", "true");
|
||||
await expect(repositoryOptions.first()).toHaveClass(/bg-card/);
|
||||
await testPage.keyboard.press("Escape");
|
||||
|
||||
const agentSelector = testPage.getByTestId("agent-profile-selector");
|
||||
await agentSelector.click();
|
||||
const agentListbox = testPage.getByRole("listbox");
|
||||
const agentOptions = agentListbox.getByRole("option");
|
||||
await expect(agentOptions.first()).toHaveAttribute("aria-selected", "true");
|
||||
await expect(agentOptions.first()).toHaveClass(/bg-card/);
|
||||
await testPage.keyboard.press("Escape");
|
||||
|
||||
const branchSelector = testPage.getByTestId("branch-chip-trigger").first();
|
||||
await branchSelector.click();
|
||||
const branchListbox = testPage.getByRole("listbox");
|
||||
const branchOptions = branchListbox.getByRole("option");
|
||||
await expect(branchOptions.first()).toHaveAttribute("aria-selected", "true");
|
||||
await expect(branchOptions.first()).toHaveClass(/bg-card/);
|
||||
});
|
||||
|
||||
test("dialog remembers selections after creating a task", async ({ testPage }) => {
|
||||
|
||||
@@ -103,6 +103,10 @@ test.describe("mobile task launch failure recovery", () => {
|
||||
const branchOption = testPage.getByTestId("task-launch-branch-picker-option-main");
|
||||
await expect(branchOption).toBeVisible({ timeout: 30_000 });
|
||||
await expect(branchOption).toBeInViewport();
|
||||
await expect(pickerScroll.getByRole("option").first()).toHaveAttribute(
|
||||
"data-testid",
|
||||
"task-launch-branch-picker-option-main",
|
||||
);
|
||||
const optionBox = await branchOption.boundingBox();
|
||||
expect(optionBox).not.toBeNull();
|
||||
expect(optionBox!.height).toBeGreaterThanOrEqual(44);
|
||||
|
||||
@@ -0,0 +1,83 @@
|
||||
import { useCallback, useEffect, useRef, useState } from "react";
|
||||
|
||||
export function usePillTooltipSuppression(open: boolean) {
|
||||
const [suppressTooltip, setSuppressTooltip] = useState(false);
|
||||
const suppressTooltipRef = useRef(false);
|
||||
const pointerInsideTriggerRef = useRef(false);
|
||||
const suppressionReleaseOnExitRef = useRef(true);
|
||||
const suppressionReleaseArmedRef = useRef(false);
|
||||
const suppressionReleaseFrameRef = useRef<number | null>(null);
|
||||
const clearTooltipSuppression = useCallback(() => {
|
||||
if (suppressionReleaseFrameRef.current !== null) {
|
||||
cancelAnimationFrame(suppressionReleaseFrameRef.current);
|
||||
suppressionReleaseFrameRef.current = null;
|
||||
}
|
||||
suppressionReleaseArmedRef.current = false;
|
||||
suppressionReleaseOnExitRef.current = true;
|
||||
suppressTooltipRef.current = false;
|
||||
setSuppressTooltip(false);
|
||||
}, []);
|
||||
const suppressTooltipUntilLeave = useCallback(
|
||||
(releaseOnExit = true) => {
|
||||
if (suppressionReleaseFrameRef.current !== null) {
|
||||
cancelAnimationFrame(suppressionReleaseFrameRef.current);
|
||||
}
|
||||
suppressionReleaseOnExitRef.current = releaseOnExit;
|
||||
suppressionReleaseArmedRef.current = false;
|
||||
suppressTooltipRef.current = true;
|
||||
setSuppressTooltip(true);
|
||||
suppressionReleaseFrameRef.current = requestAnimationFrame(() => {
|
||||
suppressionReleaseFrameRef.current = requestAnimationFrame(() => {
|
||||
suppressionReleaseFrameRef.current = null;
|
||||
if (!pointerInsideTriggerRef.current && suppressionReleaseOnExitRef.current) {
|
||||
clearTooltipSuppression();
|
||||
return;
|
||||
}
|
||||
suppressionReleaseArmedRef.current = true;
|
||||
});
|
||||
});
|
||||
},
|
||||
[clearTooltipSuppression],
|
||||
);
|
||||
useEffect(
|
||||
() => () => {
|
||||
if (suppressionReleaseFrameRef.current !== null) {
|
||||
cancelAnimationFrame(suppressionReleaseFrameRef.current);
|
||||
}
|
||||
},
|
||||
[],
|
||||
);
|
||||
const handlePointerEnter = useCallback(
|
||||
(event: React.PointerEvent) => {
|
||||
pointerInsideTriggerRef.current = true;
|
||||
if (
|
||||
event.pointerType !== "touch" &&
|
||||
!suppressionReleaseOnExitRef.current &&
|
||||
suppressTooltipRef.current
|
||||
) {
|
||||
clearTooltipSuppression();
|
||||
}
|
||||
},
|
||||
[clearTooltipSuppression],
|
||||
);
|
||||
const handlePointerLeave = useCallback(() => {
|
||||
pointerInsideTriggerRef.current = false;
|
||||
if (!open && suppressionReleaseOnExitRef.current && suppressionReleaseArmedRef.current) {
|
||||
clearTooltipSuppression();
|
||||
}
|
||||
}, [clearTooltipSuppression, open]);
|
||||
const handleBlur = useCallback(() => {
|
||||
if (!open && suppressionReleaseOnExitRef.current && suppressionReleaseArmedRef.current) {
|
||||
clearTooltipSuppression();
|
||||
}
|
||||
}, [clearTooltipSuppression, open]);
|
||||
|
||||
return {
|
||||
suppressTooltip,
|
||||
suppressTooltipRef,
|
||||
suppressTooltipUntilLeave,
|
||||
handlePointerEnter,
|
||||
handlePointerLeave,
|
||||
handleBlur,
|
||||
};
|
||||
}
|
||||
@@ -0,0 +1,59 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import {
|
||||
configClassName,
|
||||
prioritizeSelectedOption,
|
||||
selectorOptionClassName,
|
||||
} from "@/lib/utils/selector-options";
|
||||
|
||||
describe("prioritizeSelectedOption", () => {
|
||||
it("moves the selected option first without changing the source", () => {
|
||||
const source = [{ id: "first" }, { id: "current" }, { id: "last" }];
|
||||
|
||||
expect(prioritizeSelectedOption(source, "current", (option) => option.id)).toEqual([
|
||||
{ id: "current" },
|
||||
{ id: "first" },
|
||||
{ id: "last" },
|
||||
]);
|
||||
expect(source).toEqual([{ id: "first" }, { id: "current" }, { id: "last" }]);
|
||||
});
|
||||
|
||||
it("preserves source order when there is no selected option", () => {
|
||||
const source = [{ id: "first" }, { id: "last" }];
|
||||
|
||||
expect(prioritizeSelectedOption(source, "missing", (option) => option.id)).toEqual(source);
|
||||
expect(prioritizeSelectedOption(source, "", (option) => option.id)).toEqual(source);
|
||||
});
|
||||
});
|
||||
|
||||
describe("selectorOptionClassName", () => {
|
||||
it("returns only base classes when not selected", () => {
|
||||
const className = selectorOptionClassName(false);
|
||||
|
||||
expect(className).toContain("border-transparent");
|
||||
expect(className).not.toContain("border-primary");
|
||||
expect(className).not.toContain("opacity-40");
|
||||
});
|
||||
|
||||
it("adds a persistent surface and active highlight when selected", () => {
|
||||
const className = selectorOptionClassName(true);
|
||||
|
||||
expect(className).toContain("border-primary/50");
|
||||
expect(className).toContain("bg-card");
|
||||
expect(className).toContain("data-[selected=true]:ring-2");
|
||||
});
|
||||
|
||||
it("adds disabled classes to a selected option", () => {
|
||||
const className = selectorOptionClassName(true, true);
|
||||
|
||||
expect(className).toContain("opacity-40");
|
||||
expect(className).toContain("cursor-not-allowed");
|
||||
});
|
||||
});
|
||||
|
||||
describe("configClassName", () => {
|
||||
it("adds the selected surface only when selected", () => {
|
||||
expect(configClassName(true)).toContain("border-primary/50");
|
||||
expect(configClassName(false)).not.toContain("border-primary");
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,50 @@
|
||||
/** Return a new list with the current option first when it is present. */
|
||||
export function prioritizeSelectedOption<T>(
|
||||
options: readonly T[],
|
||||
selectedValue: string | null | undefined,
|
||||
getValue: (option: T) => string,
|
||||
): T[] {
|
||||
if (!selectedValue) return [...options];
|
||||
|
||||
const selectedIndex = options.findIndex((option) => getValue(option) === selectedValue);
|
||||
if (selectedIndex <= 0) return [...options];
|
||||
|
||||
const selected = options[selectedIndex];
|
||||
if (selected === undefined) return [...options];
|
||||
|
||||
return [selected, ...options.slice(0, selectedIndex), ...options.slice(selectedIndex + 1)];
|
||||
}
|
||||
|
||||
export function prioritizeValueOption<T extends { value: string }>(
|
||||
options: readonly T[],
|
||||
selectedValue: string | null | undefined,
|
||||
): T[] {
|
||||
return prioritizeSelectedOption(options, selectedValue, (option) => option.value);
|
||||
}
|
||||
|
||||
export function prioritizeIdOption<T extends { id: string }>(
|
||||
options: readonly T[],
|
||||
selectedValue: string | null | undefined,
|
||||
): T[] {
|
||||
return prioritizeSelectedOption(options, selectedValue, (option) => option.id);
|
||||
}
|
||||
|
||||
export function selectorOptionClassName(selected: boolean, disabled = false): string {
|
||||
return [
|
||||
"relative min-h-11 border border-transparent pr-7 sm:min-h-7",
|
||||
selected &&
|
||||
"border-primary/50 bg-card font-medium data-[selected=true]:ring-2 data-[selected=true]:ring-primary/40",
|
||||
disabled && "opacity-40 cursor-not-allowed",
|
||||
]
|
||||
.filter(Boolean)
|
||||
.join(" ");
|
||||
}
|
||||
|
||||
export function configClassName(selected: boolean): string {
|
||||
return [
|
||||
"h-auto min-h-11 w-full min-w-0 cursor-pointer justify-start border border-transparent px-2 py-2 text-left sm:min-h-9",
|
||||
selected && "border-primary/50 bg-card font-medium hover:bg-card",
|
||||
]
|
||||
.filter(Boolean)
|
||||
.join(" ");
|
||||
}
|
||||
@@ -0,0 +1,171 @@
|
||||
---
|
||||
spec: docs/specs/ui/selected-option-picker-prominence.md
|
||||
created: 2026-08-22
|
||||
status: implemented
|
||||
---
|
||||
|
||||
# Implementation Plan: Selected option prominence in single-choice pickers
|
||||
|
||||
## Overview
|
||||
|
||||
Add one pure ordering helper and apply it at the existing single-choice picker
|
||||
boundaries. The model/config picker, shared combobox, compact task-create pill,
|
||||
mode menu, and dedicated branch list will keep their current data and selection
|
||||
handlers while showing the current value first and using the repository's
|
||||
neutral selected-surface semantics. Focused component tests cover stable
|
||||
ordering and selected styling; desktop and mobile Playwright checks cover the
|
||||
rendered user flow.
|
||||
|
||||
## Frontend
|
||||
|
||||
### Shared selected-option ordering
|
||||
|
||||
- `apps/web/lib/utils/selector-options.ts`
|
||||
- `apps/web/lib/utils/selector-options.test.ts`
|
||||
|
||||
Add a generic helper that returns a selected-first copy without mutating the
|
||||
source collection, preserves the relative order of every other entry, and
|
||||
leaves collections with no selected value unchanged.
|
||||
|
||||
### Shared combobox
|
||||
|
||||
- `apps/web/components/combobox.tsx`
|
||||
- `apps/web/components/combobox.test.tsx`
|
||||
|
||||
Apply selected-first ordering to the unfiltered option list used by agent,
|
||||
branch, repository, executor, and profile consumers. Keep unavailable options
|
||||
non-selectable, ensure a selected unavailable option is still first, and give
|
||||
the selected option a reserved neutral/card surface with a primary boundary or
|
||||
inset ring. Keep keyboard highlighting and existing enabled/disabled grouping
|
||||
behavior readable.
|
||||
|
||||
### Compact task-create pills
|
||||
|
||||
- `apps/web/components/task-create-dialog-pill.tsx`
|
||||
- `apps/web/components/task-create-dialog-pill.test.tsx`
|
||||
|
||||
Apply the same selected-first order and persistent selected row treatment to
|
||||
the compact repository and branch chip pickers. Preserve their custom filter
|
||||
scoring, refresh/action controls, tooltip behavior, and selection handlers.
|
||||
|
||||
### Model and dependent configuration picker
|
||||
|
||||
- `apps/web/components/model-config-selector.tsx`
|
||||
- `apps/web/components/model-config-selector-content.tsx`
|
||||
- `apps/web/components/model-config-selector.test.tsx`
|
||||
|
||||
Prioritize the current model in the top-level model list and the current value
|
||||
in each nested configuration list. Add persistent selected-row styling that is
|
||||
separate from cmdk's transient highlighted-row styling. Preserve model loading,
|
||||
gone-model, provider-description, and keep-open behavior.
|
||||
|
||||
### Mode and dedicated branch pickers
|
||||
|
||||
- `apps/web/components/task/mode-selector.tsx`
|
||||
- `apps/web/components/task/branch-picker-list.tsx`
|
||||
- `apps/web/components/task/branch-picker-list.test.tsx`
|
||||
|
||||
Use the shared ordering helper for the current mode and current branch when no
|
||||
filter is active. Apply the same selected-surface semantics without changing
|
||||
branch filtering/ranking, branch saving, mode persistence, or the existing
|
||||
`MobilePickerSheet` composition used by task-launch recovery.
|
||||
|
||||
## Tests
|
||||
|
||||
- **What:** the ordering helper moves an existing selected value first without
|
||||
mutating input, preserves non-selected order, and leaves no-selection lists
|
||||
unchanged.
|
||||
**File:** `apps/web/lib/utils/selector-options.test.ts`.
|
||||
**How:** focused Vitest unit cases.
|
||||
- **What:** shared combobox consumers render the current option first and keep
|
||||
the selected row visibly distinct, including an unavailable current value.
|
||||
**File:** `apps/web/components/combobox.test.tsx`.
|
||||
**How:** render the real combobox, open it, and assert option order,
|
||||
selected styling, keyboard highlight transitions, and disabled behavior.
|
||||
- **What:** model and nested configuration lists show their current values
|
||||
first while preserving selection callbacks and loading/description behavior.
|
||||
**File:** `apps/web/components/model-config-selector.test.tsx`.
|
||||
**How:** focused component tests using the existing test fixtures.
|
||||
- **What:** the dedicated branch list puts the current branch first, preserves
|
||||
the source order of other branches, and keeps `aria-selected` and the
|
||||
selected surface correct.
|
||||
**File:** `apps/web/components/task/branch-picker-list.test.tsx`.
|
||||
**How:** render tests with local and remote branch fixtures, including a
|
||||
filtered list.
|
||||
|
||||
## E2E Tests
|
||||
|
||||
- **Scenario:** the task chat model selector opens with its current model first,
|
||||
keeps the selected surface visible, and still allows a different model and
|
||||
dependent option to be selected.
|
||||
**File:** `apps/web/e2e/tests/chat/mobile-model-selector.spec.ts` and the
|
||||
existing desktop model-selector coverage in
|
||||
`apps/web/e2e/tests/chat/model-selector-error.spec.ts`.
|
||||
**What to verify:** touch and mouse opening, first-option identity, selected
|
||||
row styling, dependent-option interaction, and viewport containment.
|
||||
- **Scenario:** task creation opens the agent, repository, and branch selectors
|
||||
with their current values first.
|
||||
**File:** `apps/web/e2e/tests/task/create-task.spec.ts` and
|
||||
`apps/web/e2e/tests/task/create-task-branch-selector.spec.ts`.
|
||||
**What to verify:** selected agent/branch order, current selected styling,
|
||||
and unchanged branch filter ranking.
|
||||
- **Scenario:** the phone recovery branch picker keeps the first available
|
||||
replacement branch visible in its existing bottom sheet.
|
||||
**File:** `apps/web/e2e/tests/task/mobile-launch-failure-recovery.spec.ts`.
|
||||
**What to verify:** the first replacement row is visible before scrolling,
|
||||
touch-sized, viewport-contained, and free of document horizontal overflow.
|
||||
|
||||
## Mobile design contract
|
||||
|
||||
- **Desktop outcome and mobile entry:** users confirm or change the current
|
||||
model, config value, agent, or branch. Desktop opens the existing popover or
|
||||
menu; phone opens the existing compact toolbar picker or branch
|
||||
`MobilePickerSheet` through the current visible trigger.
|
||||
- **Nearest shipped exemplars:** `apps/web/e2e/tests/chat/mobile-model-selector.spec.ts`
|
||||
for the task toolbar picker and
|
||||
`apps/web/components/task/mobile/mobile-picker-sheet.tsx` plus
|
||||
`apps/web/components/task/task-launch-branch-picker.tsx` for the phone branch
|
||||
surface. Reuse their trigger, focus, containment, and scroll behavior.
|
||||
- **Mobile hierarchy and primary action:** the picker title/current context is
|
||||
already supplied by the existing trigger or sheet header; the first list row
|
||||
is the current choice, followed by alternatives. Tapping a row remains the
|
||||
primary action.
|
||||
- **Presentation and rationale:** keep the current popover/command surface for
|
||||
short searchable choices and the existing inset bottom sheet for branch
|
||||
recovery. This is a short selection task, so a new route or full-height
|
||||
surface would add unnecessary navigation.
|
||||
- **Scroll owner and geometry:** preserve the existing command list or
|
||||
`BranchPickerList` scroll owner, internal overflow behavior, dynamic viewport
|
||||
sizing, safe-area padding, and document horizontal-overflow guarantees.
|
||||
- **Shared logic:** ordering, selected-value matching, filtering, and handlers
|
||||
are shared across viewports. Only the existing presentation wrapper differs.
|
||||
- **Mobile proof:** the mobile model selector test asserts the current row is
|
||||
first and visibly selected before any scroll; the branch-sheet path asserts
|
||||
containment where that path is available.
|
||||
|
||||
## Verification Results
|
||||
|
||||
Completed on 2026-08-22.
|
||||
|
||||
- `cd apps && rtk pnpm --filter @kandev/web test -- --run lib/utils/selector-options.test.ts components/combobox.test.tsx components/model-config-selector.test.tsx components/task-create-dialog-pill.test.tsx components/task/branch-picker-list.test.tsx components/task/selector-trigger-class.test.tsx` - 6 files, 55 tests passed.
|
||||
- `cd apps/web && rtk pnpm run typecheck` - passed.
|
||||
- `cd apps/web && rtk pnpm e2e:run --project chromium tests/chat/model-selector-error.spec.ts tests/task/create-task.spec.ts tests/task/create-task-branch-selector.spec.ts` - fresh backend/frontend build, 35 tests passed.
|
||||
- `cd apps/web && rtk pnpm e2e:run --project mobile-chrome tests/chat/mobile-model-selector.spec.ts tests/settings/mobile-no-silent-model-fallback.spec.ts tests/task/mobile-launch-failure-recovery.spec.ts` - fresh backend/frontend build, 3 tests passed.
|
||||
- Targeted ESLint with `--max-warnings 0` and Prettier checks for all changed frontend and E2E files - passed.
|
||||
- `rtk git diff --check` - passed.
|
||||
- Fixup review findings addressed: active listbox scoping, distinct keyboard
|
||||
highlight styling, focused module extraction for size limits, and direct
|
||||
helper-class coverage.
|
||||
|
||||
## Implementation Waves And Parallel Candidates
|
||||
|
||||
Wave 1:
|
||||
|
||||
- [x] [task-01-selected-option-picker-ui](task-01-selected-option-picker-ui.md)
|
||||
|
||||
Wave 2:
|
||||
|
||||
- [x] [task-02-selected-option-picker-e2e](task-02-selected-option-picker-e2e.md)
|
||||
|
||||
Tasks are sequential because the E2E selectors and assertions depend on the
|
||||
final selected-row markup.
|
||||
@@ -0,0 +1,83 @@
|
||||
---
|
||||
id: "01-selected-option-picker-ui"
|
||||
title: "Prioritize selected picker options"
|
||||
status: done
|
||||
wave: 1
|
||||
depends_on: []
|
||||
plan: "plan.md"
|
||||
spec: "../../specs/ui/selected-option-picker-prominence.md"
|
||||
---
|
||||
|
||||
# Task 01: Prioritize selected picker options
|
||||
|
||||
## Acceptance
|
||||
|
||||
- The current value is first in every unfiltered model/config, shared
|
||||
combobox, compact task-create pill, mode, and dedicated branch option list;
|
||||
all other values preserve source order, and no-current-value lists remain
|
||||
unchanged.
|
||||
- Every current row has a persistent neutral/card selected surface with a
|
||||
primary boundary or inset ring and its check indicator, while hover and
|
||||
keyboard highlight remain distinct and unavailable current values stay
|
||||
disabled.
|
||||
- Existing filtering, loading, dependent-option, mobile-sheet, and selection
|
||||
behavior remains intact.
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
cd apps && rtk pnpm install --frozen-lockfile
|
||||
cd apps && rtk pnpm --filter @kandev/web test -- --run lib/utils/selector-options.test.ts components/combobox.test.tsx components/model-config-selector.test.tsx components/task-create-dialog-pill.test.tsx components/task/branch-picker-list.test.tsx components/task/selector-trigger-class.test.tsx
|
||||
cd apps/web && rtk pnpm run typecheck
|
||||
```
|
||||
|
||||
## Files likely touched
|
||||
|
||||
- `apps/web/lib/utils/selector-options.ts`
|
||||
- `apps/web/lib/utils/selector-options.test.ts`
|
||||
- `apps/web/components/combobox.tsx`
|
||||
- `apps/web/components/combobox.test.tsx`
|
||||
- `apps/web/components/task-create-dialog-pill.tsx`
|
||||
- `apps/web/components/task-create-dialog-pill.test.tsx`
|
||||
- `apps/web/components/task-create-dialog-branch-options.tsx`
|
||||
- `apps/web/hooks/use-pill-tooltip-suppression.ts`
|
||||
- `apps/web/components/model-config-selector.tsx`
|
||||
- `apps/web/components/model-config-selector.test.tsx`
|
||||
- `apps/web/components/task/mode-selector.tsx`
|
||||
- `apps/web/components/task/branch-picker-list.tsx`
|
||||
- `apps/web/components/task/branch-picker-list.test.tsx`
|
||||
|
||||
## Dependencies
|
||||
|
||||
None.
|
||||
|
||||
## Parallelism
|
||||
|
||||
Sequential. These components share the selected-option contract and styling.
|
||||
|
||||
## Inputs
|
||||
|
||||
- All `What` and `Scenarios` sections of the spec.
|
||||
- `docs/decisions/2026-07-29-interactive-accent-surface-semantics.md`.
|
||||
- Existing `CommandItem`, `Combobox`, `ModelConfigSelector`,
|
||||
`BranchPickerList`, `ModeSelector`, and mobile picker patterns.
|
||||
- The mobile design contract in `plan.md`.
|
||||
|
||||
## Output contract
|
||||
|
||||
Report the helper and component files changed, the exact selected-first and
|
||||
selected-surface tests run, mobile implications, blockers, risks, and the
|
||||
synchronized task/plan status.
|
||||
|
||||
## Results
|
||||
|
||||
Focused implementation tests pass: 6 files, 55 tests. TypeScript typecheck
|
||||
passes. The shared combobox, compact task-create pills, model/config picker,
|
||||
mode menu, and dedicated branch picker now share selected-first ordering and
|
||||
persistent selected-row styling. Keyboard navigation coverage confirms cmdk's
|
||||
transient highlight remains distinct from the persistent current-row surface.
|
||||
The model content, branch option utilities, and pill tooltip suppression are
|
||||
split into focused modules so the changed files remain within repository size
|
||||
limits. Mobile behavior keeps the existing command and branch-sheet
|
||||
compositions, with 44px touch rows and no new overflow. The changed frontend
|
||||
files pass targeted ESLint with zero warnings and Prettier.
|
||||
@@ -0,0 +1,79 @@
|
||||
---
|
||||
id: "02-selected-option-picker-e2e"
|
||||
title: "Verify selected picker prominence"
|
||||
status: done
|
||||
wave: 2
|
||||
depends_on: ["01-selected-option-picker-ui"]
|
||||
plan: "plan.md"
|
||||
spec: "../../specs/ui/selected-option-picker-prominence.md"
|
||||
---
|
||||
|
||||
# Task 02: Verify selected picker prominence
|
||||
|
||||
## Acceptance
|
||||
|
||||
- Desktop model, agent, repository, and branch picker flows show the current
|
||||
value first with the selected surface and preserve existing filtering and
|
||||
selection behavior.
|
||||
- The mobile model flow proves the current option is visible first before
|
||||
scrolling, remains touch-selectable, and stays within the viewport without
|
||||
document horizontal overflow.
|
||||
- The focused frontend tests and Playwright projects pass against a fresh
|
||||
production web build.
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
cd apps && rtk pnpm install --frozen-lockfile
|
||||
cd apps/web && rtk pnpm e2e:run --project chromium tests/chat/model-selector-error.spec.ts tests/task/create-task.spec.ts tests/task/create-task-branch-selector.spec.ts
|
||||
cd apps/web && rtk pnpm e2e:run --project mobile-chrome tests/chat/mobile-model-selector.spec.ts tests/settings/mobile-no-silent-model-fallback.spec.ts tests/task/mobile-launch-failure-recovery.spec.ts
|
||||
cd apps/web && rtk pnpm run typecheck
|
||||
rtk git diff --check
|
||||
```
|
||||
|
||||
The managed E2E runner must rebuild the production web assets. If an existing
|
||||
task-launch recovery test is the correct branch-sheet fixture, extend that
|
||||
test rather than creating a duplicate seed flow.
|
||||
|
||||
## Files likely touched
|
||||
|
||||
- `apps/web/e2e/tests/chat/mobile-model-selector.spec.ts`
|
||||
- `apps/web/e2e/tests/chat/model-selector-error.spec.ts`
|
||||
- `apps/web/e2e/tests/task/create-task.spec.ts`
|
||||
- `apps/web/e2e/tests/task/create-task-branch-selector.spec.ts`
|
||||
- `apps/web/e2e/tests/task/mobile-launch-failure-recovery.spec.ts`
|
||||
- `apps/web/e2e/tests/settings/mobile-no-silent-model-fallback.spec.ts` only if
|
||||
its existing agent-picker flow is the minimal mobile proof.
|
||||
|
||||
## Dependencies
|
||||
|
||||
Task 01.
|
||||
|
||||
## Parallelism
|
||||
|
||||
Sequential. E2E selectors and assertions depend on the final UI markup.
|
||||
|
||||
## Inputs
|
||||
|
||||
- The spec scenarios for model, combobox, branch, unavailable values, and
|
||||
narrow touch viewports.
|
||||
- The mobile parity contract in `plan.md`.
|
||||
- Existing fixture, page-object, causal-wait, and selector-scoping guidance in
|
||||
`.agents/skills/e2e/SKILL.md`.
|
||||
|
||||
## Output contract
|
||||
|
||||
Report exact desktop and mobile commands, test counts and outcomes, fresh-build
|
||||
evidence, screenshots or artifacts if captured, cleanup evidence, changed test
|
||||
files, blockers, risks, and synchronized task/plan status.
|
||||
|
||||
## Results
|
||||
|
||||
Desktop fresh production-build E2E passed 35 tests. Mobile fresh
|
||||
production-build E2E passed 3 tests covering the model picker, settings agent
|
||||
picker, and the existing launch-recovery branch sheet. The recovery fixture
|
||||
starts with a missing branch, so its assertion verifies that the first
|
||||
available replacement branch is visible, touch-sized, and viewport-contained;
|
||||
selected styling is covered by the current-value branch tests. The fixup pass
|
||||
also scopes option assertions through each active listbox to avoid matching a
|
||||
different mounted picker.
|
||||
@@ -0,0 +1,90 @@
|
||||
---
|
||||
status: implemented
|
||||
created: 2026-08-22
|
||||
owner: kandev
|
||||
---
|
||||
|
||||
# Selected option prominence in single-choice pickers
|
||||
|
||||
## Why
|
||||
|
||||
When a picker contains several models, branches, agents, or related choices,
|
||||
the current value can appear in the middle of a long list. The check mark alone
|
||||
does not make the current choice easy to find, so users must scan or scroll
|
||||
before they can confirm the active configuration.
|
||||
|
||||
## What
|
||||
|
||||
- Single-choice option lists show the current value as the first row when the
|
||||
list is opened without an active search query. The remaining options keep
|
||||
their existing source order.
|
||||
- The ordering behavior applies to the model/configuration picker, the shared
|
||||
`Combobox` used by agent, branch, repository, executor, and profile pickers,
|
||||
the compact `Pill` used by task-create repository/branch chips, the task mode
|
||||
picker, and the dedicated branch picker list.
|
||||
- If the current value is represented by an unavailable or disabled option, it
|
||||
still appears first and remains non-selectable. If there is no current value,
|
||||
the source order is unchanged.
|
||||
- The current row has a persistent selected appearance: a neutral or card-based
|
||||
background, a primary boundary or inset ring, normal readable foreground
|
||||
text, and the existing check indicator. Hover and keyboard-highlight states
|
||||
remain distinct transient states and do not remove the persistent selected
|
||||
appearance.
|
||||
- Selection styling reserves its boundary space so moving between rows does
|
||||
not change the list layout. Existing selection, filtering, loading, disabled,
|
||||
and dependent-option behavior remains unchanged.
|
||||
- With an active search or filter query, the picker keeps its existing filtering
|
||||
and ranking behavior. When the selected value matches the query, it may be
|
||||
prioritized within the matching results; a query that excludes it does not
|
||||
force it into the results.
|
||||
- The same selected-first and selected-state behavior is available on the
|
||||
existing phone surfaces. The current choice is visible before scrolling, and
|
||||
the change does not add document-level horizontal scrolling or require hover.
|
||||
|
||||
## Scenarios
|
||||
|
||||
- **GIVEN** the model list is ordered `Default`, `Sonnet`, `Fable`, `Opus` and
|
||||
`Opus` is current, **WHEN** the model picker opens without a search query,
|
||||
**THEN** `Opus` is the first option, has the persistent selected surface and
|
||||
check indicator, and the other models retain their relative order.
|
||||
- **GIVEN** a dynamic configuration list has `Low`, `Medium`, and `High` and
|
||||
`Medium` is current, **WHEN** its nested picker opens, **THEN** `Medium` is
|
||||
the first option with the same selected treatment and all three values remain
|
||||
selectable according to their existing rules.
|
||||
- **GIVEN** an agent or repository `Combobox` has its second option selected,
|
||||
**WHEN** the list opens without a query, **THEN** that option is first and
|
||||
the remaining options retain their original order.
|
||||
- **GIVEN** a task-create repository or branch chip has a current value after
|
||||
other options, **WHEN** its compact picker opens without a query, **THEN**
|
||||
the current value is first with the same selected treatment.
|
||||
- **GIVEN** a branch picker contains the current `main` branch after other
|
||||
branches, **WHEN** the branch list opens in a popover or phone sheet,
|
||||
**THEN** `main` is the first row and exposes its selected state without
|
||||
changing branch selection behavior.
|
||||
- **GIVEN** a task mode picker has a current mode that is not first in the
|
||||
provider list, **WHEN** the menu opens, **THEN** the current mode is first and
|
||||
remains visually distinct from the keyboard-highlighted row.
|
||||
- **GIVEN** a picker has no current value, **WHEN** it opens, **THEN** its
|
||||
options retain the existing source order and no row receives selected styling
|
||||
solely because it is first.
|
||||
- **GIVEN** the current value is represented by a disabled unavailable option,
|
||||
**WHEN** the picker opens, **THEN** that option is first, visibly selected,
|
||||
and still cannot be chosen.
|
||||
- **GIVEN** a user types a filter query, **WHEN** the current value does not
|
||||
match the query, **THEN** the current value is not injected into the results
|
||||
and the existing filter ranking is preserved.
|
||||
- **GIVEN** a narrow touch viewport, **WHEN** the user opens the model or
|
||||
branch picker, **THEN** the current row is visible without scrolling, its
|
||||
selected treatment is readable, and the picker remains contained within the
|
||||
viewport.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- Changing model, branch, agent, executor, repository, or mode persistence,
|
||||
APIs, labels, or selection side effects.
|
||||
- Reordering multi-select lists, chips, checkboxes, tabs, navigation menus, or
|
||||
lists whose order communicates a domain ranking rather than a current choice.
|
||||
- Adding a new mobile drawer, route, or picker composition. Existing responsive
|
||||
surfaces and scroll owners remain in place.
|
||||
- Ranking options by popularity, capability, provider, or any other heuristic
|
||||
beyond prioritizing the current value when no query is active.
|
||||
Reference in New Issue
Block a user