Add composer model selector#316
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new reusable ComposerModelSelector UI primitive under src/components/AI for selecting an AI provider/model pair in compact “composer” UIs, along with Storybook coverage.
Changes:
- Added
ComposerModelSelectorcomponent with provider filtering, grouped model rendering, and constrained/portaled dropdown positioning. - Exported the component and related types from the AI barrel.
- Added Storybook stories covering single/multi-provider, long labels, and constrained container behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/components/AI/index.ts |
Exports the new selector and its public types from the AI entrypoint. |
src/components/AI/ComposerModelSelector.tsx |
Implements the portaled, filterable, grouped provider/model selector UI. |
src/components/AI/ComposerModelSelector.stories.tsx |
Adds Storybook examples for common and edge display cases. |
2bab230 to
7abf153
Compare
7abf153 to
3717801
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
src/components/AI/ComposerModelSelector.tsx:385
- The listbox uses an
aria-activedescendantpattern (focus stays on the listbox), but each option is a tabbable<button>. This makes Tab move into the options (and arrow-key handling on the listbox stops working), which breaks the expected listbox keyboard model. Make the options not tabbable and let the listbox be the single tab stop.
key={model.id ?? optionKey(model)}
id={getOptionId(index)}
type="button"
role="option"
aria-selected={selected}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
src/components/AI/ComposerModelSelector.tsx:386
- The listbox uses
aria-activedescendantwith focus managed on the list container, but the option elements are plain<button>s that will take focus on mouse interaction and are also tabbable by default. This can break keyboard navigation/highlight behavior and is inconsistent with other listbox implementations in the repo (e.g. CodeLookup keeps focus on the list owner and makes options non-tabbable). AddtabIndex={-1}and prevent default on mouse down so focus remains on the listbox container.
type="button"
role="option"
aria-selected={selected}
onClick={() => selectModel(model)}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
src/components/AI/ComposerModelSelector.tsx:235
- ComposerModelSelector introduces a fair amount of interaction logic (portal positioning, provider filtering, outside-click close, and Escape handling) but there are no automated tests covering these behaviors. Given there are existing Vitest/RTL tests in src/components/AI, adding a focused test file would help prevent regressions in keyboard/mouse interaction.
React.useEffect(() => {
if (!open) return;
updateMenuPosition();
window.addEventListener('resize', updateMenuPosition);
window.addEventListener('scroll', updateMenuPosition, true);
return () => {
window.removeEventListener('resize', updateMenuPosition);
window.removeEventListener('scroll', updateMenuPosition, true);
};
}, [open, updateMenuPosition]);
React.useEffect(() => {
if (!open) return;
const handlePointerDown = (
event: globalThis.MouseEvent | globalThis.TouchEvent
) => {
const target = event.target as Node;
if (
triggerRef.current?.contains(target) ||
menuRef.current?.contains(target)
) {
return;
}
close();
};
document.addEventListener('mousedown', handlePointerDown);
document.addEventListener('touchstart', handlePointerDown);
return () => {
document.removeEventListener('mousedown', handlePointerDown);
document.removeEventListener('touchstart', handlePointerDown);
};
}, [close, open]);
useEscapeKey(close, open);
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
src/components/AI/ComposerModelSelector.tsx:408
- The listbox options are rendered as
<button role="option">, which creates nested interactive semantics inside arole="listbox"and leaves each option in the tab order. This can confuse assistive tech and keyboard users. Prefer non-interactive elements (div/li) withrole="option"and keep keyboard interaction on the listbox container (viaaria-activedescendant) or adopt the roving-tabindex pattern used inSelect.
<button
key={model.id ?? optionKey(model)}
id={getOptionId(index)}
type="button"
role="option"
| className | ||
| )} | ||
| > | ||
| <span className="max-w-40 min-w-0 truncate"> |
| type ControlledProviderFilterProps = { | ||
| providerFilter: string | 'any'; | ||
| onProviderFilterChange: (provider: string | 'any') => void; | ||
| }; |
| const renderedModelIndexes = React.useMemo( | ||
| () => | ||
| new Map(renderedModels.map((model, index) => [optionKey(model), index])), | ||
| [renderedModels] |
Closes #317
Adds a reusable ComposerModelSelector primitive for compact provider/model selection in composer-style UIs.
What changed:
Scope:
Verification: