Skip to content

Bug - Select migration display value fixes#2519

Merged
SpicyPete merged 3 commits into
masterfrom
bug/select-items
Jun 19, 2026
Merged

Bug - Select migration display value fixes#2519
SpicyPete merged 3 commits into
masterfrom
bug/select-items

Conversation

@SpicyPete

Copy link
Copy Markdown
Contributor
  • Fixes the display value of Selects
  • Fixes from previous migration work

@SpicyPete SpicyPete requested review from a team, malinskibeniamin and r-vasquez June 19, 2026 18:08
@SpicyPete SpicyPete self-assigned this Jun 19, 2026
@SpicyPete SpicyPete added the bug Something isn't working label Jun 19, 2026
@SpicyPete SpicyPete requested review from datamali and eblairmckee and removed request for a team June 19, 2026 18:08
@SpicyPete

Copy link
Copy Markdown
Contributor Author

@claude review

@github-actions

github-actions Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Claude finished @SpicyPete's task in 4m 16s —— View job


Claude PR Review — Select migration display value fixes

  • Read the full diff against origin/master
  • Understand the Select migration fixes
  • Review for bugs, correctness & consistency
  • Post review feedback

Summary

The change is sound and addresses a real bug. Base UI's Select only resolves an item's label after the popup first mounts; until then it stringifies the raw value, so a controlled, enum-backed select flashes the underlying value (e.g. 1, TOPIC, a model id) instead of its label. Passing an items map to <Select> (Base UI's Select.Root) lets it resolve the label up front. This matches the documented contract in select.tsx:56-59.

I verified the items payloads against the actually-rendered SelectItems in every changed file:

  • Single-source-of-truth (excellent): select-slot (slot.options), create-topic-dialog (options), advanced-topic-settings (units), transcript-filter-bar (TIME_RANGES/OPERATOR_OPTIONS), observability-toolbar & pipeline-throughput-card (TIME_RANGES), ai-agent-transcripts-tab (STATUS_FILTER_OPTIONS), resource-tier-select, secret-selector, llm-config provider select, install-rpk, remote-mcp, schema-create references (contextSelectOptions). These reuse the same array that renders the items, so they can't drift. 👍
  • Refactors that de-duplicated branching logic: claude-code.tsx / gemini.tsx — collapsing the selectedScope === 'x' && ... chains into a SCOPE_OPTIONS lookup is a nice cleanup, and configFile for local/user both stay ~/.claude.json (no regression).
  • Hand-written maps that match their SelectItems exactly: schema-create Strategy, all three create-acl selects (including the dynamic getResourceName(...) labels).

No correctness bugs found in the changed code.

Main finding — a missed sibling select (not in this PR's diff)

frontend/src/components/ui/ai-agent/llm-config-section.tsx:288 — the model select is enum-backed: value={model.value} (a model id) but it displays {model.name}. It has neither an items map nor a render-prop SelectValue, so it still exhibits exactly the bug this PR fixes — when editing an agent with a pre-selected model, the trigger flashes the raw model id until the popup opens. Notably, the provider select in the same file (line 201) was fixed in this PR, but this one right below it was not.

<Select
  disabled={hasNoProviders || hasNoModels}
  items={filteredModels.map((m) => ({ value: m.value, label: m.name }))}
  onValueChange={field.onChange}
  value={field.value ?? ''}
>

Fix this →

Other controlled selects I checked (no action needed)

For completeness I swept every remaining <Select value=…> without items. All are already correct because they either use a render-prop SelectValue (the other documented escape hatch) or have value === label:

  • ui/ai/chat-model-select, embedding-model-select, reranker-model-select — render-prop child showing {value}.
  • redpanda-ui/.../auto-form/fields/select.tsx — render-prop via renderStaticSelectedValue.
  • template-gallery/slot-fields/secret-slot.tsx — render-prop child.
  • ui/user/user-selector, add-user-step (SASL mechanism), schema-create topic-name select — value === label.

Minor / nits (non-blocking)

  • DRY in schema-create & create-acl: the items={{…}} literals duplicate the hardcoded <SelectItem> children. The create-acl Selector case is the most fragile — both the items map and the SelectItems independently call getResourceName(rule.resourceType) / getPluralResourceName(...), so a future label tweak must be made in two places or the trigger and dropdown will disagree. Consider deriving one from the other (or mapping over a shared array, as the other files do).
  • I did not run bun run type:check / lint / test (no bun install in this review environment), but the items shapes all conform to Base UI's { value, label }[] / Record<value,label> contract and reuse existing typed arrays, so I'd expect them to pass.
    · bug/select-items

@SpicyPete SpicyPete enabled auto-merge (squash) June 19, 2026 18:33
@SpicyPete SpicyPete merged commit ec19e48 into master Jun 19, 2026
17 checks passed
@SpicyPete SpicyPete deleted the bug/select-items branch June 19, 2026 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants