Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions .changeset/11865-p-listview-chatbot-shared-select.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
---
'@object-ui/plugin-list': patch
'@object-ui/plugin-chatbot': patch
---

Three more controls pick with the shared `Select`, the control the rest of the console picks with (objectui#11865, the list view and the chatbot): `ListView`'s "Color by field" and its "Rows per page" selector (the fallback for views without a grid pager), and `ChatbotEnhanced`'s model picker.

The three were browser-native selects, so they looked and behaved differently from the console's other dropdowns. They now use the shared Radix `Select`: the same trigger, dropdown and keyboard behaviour. "Color by field" now matches its twin in the compact toolbar's View settings popover, which made the same change earlier.

What they write is unchanged. Each option gives the same value as before: "None" still clears the row-colour config, a field keeps the config's colours, each page size still reaches `onPageSizeChange` and refetches at that size, and each model still reaches `onModelChange` as its id. Re-picking the current option writes nothing. The model picker keeps its accessible name, the `model` label. A row-colour rule on a field the caller may not read still shows as "None" and is never offered.

One display change: a value none of a picker's options carries now shows as itself. The native select showed its first option instead: "None" for a row-colour field outside the list's columns, the first size for a page size in force that is not one of the options (an undeclared size, for instance), and the first model for a selected model the environment no longer offers.

**Clause-②: no.** No published face moves: the package entries export the same names, the two components take the same props, and no i18n key is added. What moves is the three controls' own markup, described above.
76 changes: 63 additions & 13 deletions packages/plugin-chatbot/src/ChatbotEnhanced.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
* - streaming markdown via streamdown (used by Message internals)
*/
import * as React from 'react';
import { cn } from '@object-ui/components';
import { cn, Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '@object-ui/components';
import { SchemaRenderer } from '@object-ui/react';
import { useObjectTranslation, useSafeTranslate } from '@object-ui/i18n';
import { AlertCircle, ArrowRight, Copy, Check, RefreshCw, CornerDownLeft, Bot, Eye, GitCompareArrows, Rocket, Clock3, CheckCircle2, XCircle, Loader2, ShieldCheck, TriangleAlert, ClipboardList, HelpCircle, Table2, WifiOff, Sparkles, Hourglass } from 'lucide-react';
Expand Down Expand Up @@ -925,6 +925,63 @@ export interface ChatbotModelOption {
provider?: string;
}

/** The item a selected model none of the offered models carries is shown by. */
const OUTSIDE_MODELS = 'outside';

/**
* objectui#11865 — the composer's model picker, drawn with the shared `Select`,
* the control the rest of the console picks with. It used to be a
* browser-native `<select>`. What a pick writes is unchanged: `onPick` receives
* the picked model's `id`, the string the native control's `change` carried,
* and its accessible name is the same `aria-label`. Re-picking the current
* model writes nothing, as it did there.
*
* - Items carry the model's INDEX, not its id, as the card's other pickers do.
* - A selected id none of the offered models carries gets an item of its own,
* labelled with the id, so the trigger shows the model the host holds. The
* native control showed the first model there. Picking that item writes
* nothing.
*/
function ModelPicker({
label,
models,
value,
onPick,
}: {
label: string;
models: ReadonlyArray<ChatbotModelOption>;
value: string;
onPick: (modelId: string) => void;
}) {
const at = models.findIndex((m) => m.id === value);
return (
<Select
value={at !== -1 ? String(at) : OUTSIDE_MODELS}
onValueChange={(token) => {
// `undefined` for the outside item: it is the host's own model, so there is nothing to write.
const picked = models[Number(token)];
if (picked) onPick(picked.id);
}}
>
<SelectTrigger
aria-label={label}
className="h-7 w-auto gap-1 px-2 text-xs text-muted-foreground hover:text-foreground focus:ring-1 focus:ring-offset-0"
>
<SelectValue />
</SelectTrigger>
<SelectContent>
{at === -1 && <SelectItem value={OUTSIDE_MODELS}>{value}</SelectItem>}
{models.map((m, i) => (
<SelectItem key={`${i}:${m.id}`} value={String(i)}>
{m.label ?? m.id}
{m.provider ? ` · ${m.provider}` : ''}
</SelectItem>
))}
</SelectContent>
</Select>
);
}

function formatMessageProps(role: ChatMessage['role']): MessageProps['from'] {
// The vendored Message only knows user/assistant — render system as assistant
// (FloatingChatbotProvider already renders system messages inline elsewhere).
Expand Down Expand Up @@ -3557,19 +3614,12 @@ const ChatbotEnhanced = React.forwardRef<HTMLDivElement, ChatbotEnhancedProps>(
envs (the backend returns one entry) get no dropdown — the
lone model is still sent via `selectedModelId`. */}
{models && models.length > 1 ? (
<select
aria-label={L.model}
<ModelPicker
label={L.model}
models={models}
value={selectedModelId ?? models[0].id}
onChange={(e) => onModelChange?.(e.target.value)}
className="h-7 rounded-md border bg-background px-2 text-xs text-muted-foreground hover:text-foreground focus:outline-none focus:ring-1 focus:ring-ring"
>
{models.map((m) => (
<option key={m.id} value={m.id}>
{m.label ?? m.id}
{m.provider ? ` · ${m.provider}` : ''}
</option>
))}
</select>
onPick={(modelId) => onModelChange?.(modelId)}
/>
) : null}
{/* #2458 UX#7 — the composer sends on PLAIN Enter (Shift+Enter =
newline); the old `⌘` glyph implied Cmd+Enter and misled users.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

/**
* The composer's model picker picks with the shared `Select` (objectui#11865).
*
* The control was a browser-native select beside the shared Radix `Select`
* the rest of the console picks with. The card asks for one control for one
* kind of choice, surface by surface.
*
* What is pinned:
* - it IS the primitive (a Radix combobox trigger, with no visible native
* select beside it), keeps its accessible name, the `aria-label` the
* `model` label gives it, shows the model in force and lists the native
* control's options, with their text, in their order;
* - every option writes what the native control wrote: the model's `id`,
* handed to `onModelChange`, compared as JSON text over three selections
* (none, so the first model is in force; an offered model; a model none
* of the options carries). Re-picking the current model writes nothing;
* - a selected model none of the options carries is what the trigger shows;
* - the keyboard alone opens the picker and selects, inside the composer's
* form, without sending the draft;
* - the read-only transcript has no composer, so no picker, as before.
*
* DIRECTION, observed against the native control: every pin here reads the
* control as the primitive's trigger, so each is red there except the name
* row and the read-only row (green there by design: they keep a behaviour,
* the `aria-label` name and the composer-less transcript). What makes the
* write rows guards of "the conversion changed nothing the composer writes" is
* the literal each compares against: a `change` event on the pre-conversion
* native control wrote that same JSON, read once on this component with these
* fixtures. That probe's `change` event fired for the current option too,
* which a browser's native select does not do, so the re-pick rows pin the
* primitive.
*/

import '@testing-library/jest-dom/vitest';
import React from 'react';
import { describe, it, expect, vi, afterEach } from 'vitest';
import { render, screen, fireEvent, cleanup, within } from '@testing-library/react';
import { ChatbotEnhanced } from '../ChatbotEnhanced';

afterEach(() => cleanup());

const MODELS = [
{ id: 'gpt-4o-mini', label: 'GPT-4o mini', provider: 'openai' },
{ id: 'claude-3-5-sonnet', label: 'Claude 3.5', provider: 'anthropic' },
{ id: 'local-llm' },
];

const SELECTIONS: Record<string, string | undefined> = {
unset: undefined,
claude: 'claude-3-5-sonnet',
ghost: 'ghost-model',
};

function mount(selected: string | undefined, extra: Record<string, unknown> = {}) {
const onModelChange = vi.fn();
const onSendMessage = vi.fn();
const view = render(
<ChatbotEnhanced
models={MODELS}
selectedModelId={selected}
onModelChange={onModelChange}
onSendMessage={onSendMessage}
{...extra}
/>,
);
return { onModelChange, onSendMessage, view, trigger: () => screen.getByRole('combobox', { name: 'Model' }) };
}

async function openPicker(trigger: HTMLElement): Promise<HTMLElement[]> {
fireEvent.keyDown(trigger, { key: 'ArrowDown' });
const listbox = await screen.findByRole('listbox');
return within(listbox).getAllByRole('option');
}

async function pick(trigger: HTMLElement, label: string): Promise<void> {
const options = await openPicker(trigger);
const option = options.find((o) => o.textContent === label);
if (!option) throw new Error(`the picker lists no "${label}": ${options.map((o) => o.textContent).join(' | ')}`);
fireEvent.click(option);
}

/**
* [selection, option label, what the native control handed `onModelChange`].
* `null` marks the model in force: re-picking it writes nothing.
*/
const WRITES: ReadonlyArray<readonly [string, string, string | null]> = [
['unset', 'GPT-4o mini · openai', null],
['unset', 'Claude 3.5 · anthropic', '[["claude-3-5-sonnet"]]'],
['unset', 'local-llm', '[["local-llm"]]'],
['claude', 'GPT-4o mini · openai', '[["gpt-4o-mini"]]'],
['claude', 'Claude 3.5 · anthropic', null],
['claude', 'local-llm', '[["local-llm"]]'],
['ghost', 'GPT-4o mini · openai', '[["gpt-4o-mini"]]'],
['ghost', 'Claude 3.5 · anthropic', '[["claude-3-5-sonnet"]]'],
['ghost', 'local-llm', '[["local-llm"]]'],
];

describe('ChatbotEnhanced — the model picker is the shared Select (objectui#11865)', () => {
it('is the primitive, keeps its name, shows the model in force, and lists the native options in order', async () => {
const { trigger, view } = mount(SELECTIONS.claude);
expect(trigger().tagName).toBe('BUTTON');
// Radix mirrors the value into a hidden native select inside a form; none is visible.
expect(view.container.querySelectorAll('select:not([aria-hidden="true"])')).toHaveLength(0);
expect(trigger().textContent).toBe('Claude 3.5 · anthropic');
const options = await openPicker(trigger());
expect(options.map((o) => o.textContent)).toEqual(['GPT-4o mini · openai', 'Claude 3.5 · anthropic', 'local-llm']);
});

it('with no selection the first model is in force', () => {
expect(mount(SELECTIONS.unset).trigger().textContent).toBe('GPT-4o mini · openai');
});

it('takes its name from the `model` label', () => {
mount(SELECTIONS.unset, { labels: { model: 'Modèle' } });
expect(screen.getByRole('combobox', { name: 'Modèle' })).toHaveAttribute('aria-label', 'Modèle');
});

it.each(WRITES)('selection "%s", picking "%s" writes what the native control wrote', async (selection, label, json) => {
const { onModelChange, trigger } = mount(SELECTIONS[selection]);
await pick(trigger(), label);
expect(JSON.stringify(onModelChange.mock.calls)).toBe(json ?? '[]');
});

it('a selected model none of the options carries is what the trigger shows, and re-picking it writes nothing', async () => {
const { onModelChange, trigger } = mount(SELECTIONS.ghost);
// The native control showed the first model here.
expect(trigger().textContent).toBe('ghost-model');
const options = await openPicker(trigger());
expect(options.map((o) => o.textContent)).toEqual(['ghost-model', 'GPT-4o mini · openai', 'Claude 3.5 · anthropic', 'local-llm']);
fireEvent.click(options[0]);
expect(onModelChange).not.toHaveBeenCalled();
});

it('Enter opens the picker and Enter on a model selects it, without sending the draft', async () => {
const { onModelChange, onSendMessage, trigger } = mount(SELECTIONS.unset);
fireEvent.change(screen.getByRole('textbox'), { target: { value: 'a draft' } });
fireEvent.keyDown(trigger(), { key: 'Enter' });
const listbox = await screen.findByRole('listbox');
fireEvent.keyDown(within(listbox).getByRole('option', { name: 'local-llm' }), { key: 'Enter' });
expect(onModelChange.mock.calls).toEqual([['local-llm']]);
expect(onSendMessage).not.toHaveBeenCalled();
});

it('the read-only transcript has no composer, so no picker', () => {
mount(SELECTIONS.claude, { readOnly: true });
expect(screen.queryByRole('combobox')).toBeNull();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -678,7 +678,7 @@ describe('ChatbotEnhanced (AI Elements composition)', () => {
expect(detail).toHaveTextContent('Add an options array.');
});

it('renders a model picker and forwards changes', () => {
it('renders a model picker and forwards changes', async () => {
const onModelChange = vi.fn();
render(
<ChatbotEnhanced
Expand All @@ -690,8 +690,11 @@ describe('ChatbotEnhanced (AI Elements composition)', () => {
onModelChange={onModelChange}
/>
);
const picker = screen.getByLabelText(/Model/i) as HTMLSelectElement;
fireEvent.change(picker, { target: { value: 'claude-3-5-sonnet' } });
// The shared `Select` (objectui#11865), still named by its aria-label.
const picker = screen.getByLabelText(/Model/i);
expect(picker).toHaveAttribute('role', 'combobox');
fireEvent.keyDown(picker, { key: 'ArrowDown' });
fireEvent.click(await screen.findByRole('option', { name: 'Claude 3.5 · anthropic' }));
expect(onModelChange).toHaveBeenCalledWith('claude-3-5-sonnet');
});

Expand Down
Loading
Loading