Repository navigation
🎛️ fix: Replace Stacked Capability Dropdowns with a Checkbox List - #124
dustinhealy wants to merge 2 commits into
Conversation
Inline @clickhouse/click-ui in vitest server deps and exclude .claude/** from test discovery. click-ui 0.9.1 ships ESM that imports .css files, which Node's loader cannot parse, so Vitest must process the package through Vite.
Array-of-enum config fields (the endpoint capabilities lists) previously rendered through ListField as one native select per selected value, each select listing every option, with a per-row delete button and an add button. They now render through a new EnumSetField that shows one click-ui Checkbox per enum option with a small selected count, so the whole set is visible and toggled in place. Toggles always emit the full array in canonical enum option order. The persistence contract is unchanged: every change writes the whole array at the field path. Unset fields never write on mount; the schema default is displayed as the checked hint set and only a user toggle produces the first explicit value, derived from that displayed default. ListField's now-dead enum options branch was removed along with the unused capability item locale key, and the localization test now exempts enum-item arrays from the item-key requirement.
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Thanks @dustinhealy for this, and for laying out the design options so clearly. Main now shows every array-of-enum field (the agents, assistants and Azure assistants capabilities) as a single checkbox grid. That is the MultiEnumField from #162, and #164 added plain-language labels and descriptions for agent capabilities. I checked a main build: Agents > Capabilities shows 20 checkboxes and no dropdowns, and both assistants fields show 5. Closing this as fixed on main. If you find a case it misses, or think the single-column list from this PR reads better, please comment or open a follow-up against current main. |
Summary
The capabilities fields in the AI Providers section (the only array-of-enum paths in the config schema:
endpoints.<agents|assistants|azureAssistants>.capabilities) rendered through the genericListFieldas one raw native<select>per selected capability. For agents that meant up to ~12 stacked dropdowns, each listing all 15 options, with a trash button per row; opening one covered the rows beneath it, duplicates were possible, and scanning the enabled set required opening every row. Not a click-ui regression; this is the pre-existing generic array renderer.Design decision (flagged for review): array-of-enum fields now render as a new
EnumSetField, a single vertical list of click-uiCheckboxrows, one per available capability, checked = enabled, with an "n/m enabled" count. Chosen over a consolidated MultiSelect popover (hides state, reintroduces a popover in an audit-oriented panel) and over a switch table with descriptions (heavier, needs ~20 new strings). Checkbox was chosen over Switch for set-membership semantics; the grants CapabilityPanel switch-list remains the closest in-repo precedent if Switch is preferred.The persistence contract is unchanged: the whole array is written at the field path (never indexed subpaths). Unset fields display the schema default as a purely visual checked set and never write on mount, so "inherits schema default" only flips to "explicitly pinned" on the first user toggle; toggles emit the full array in canonical enum order; unchecking the last item writes an explicit empty array.
ListField's now-dead options branch was removed (its remaining callers pass no options).Change Type
Testing
New
EnumSetField.test.tsx(8 cases, real click-ui): renders every option, reflects the value array, adds/removes options emitting the full array in canonical order, writes an empty array when the last item is unchecked, renders schema defaults as checked hints without calling onChange on mount, derives the first write from the displayed default, and shows the count indicator. Existing mock factories in FieldRenderer/EndpointsRenderer/McpServersRenderer tests gained a Checkbox mock.Manually verified in the app: AI Providers, Agents, More settings, Capabilities shows the checkbox list; toggling marks the field configured and saves correctly.
Before/after proof:
Test Configuration:
Checklist