Skip to content

feat(dashboard): Done button in model picker for combo creation - #4496

Merged
diegosouzapw merged 1 commit into
release/v3.8.33from
feat/port-pr-1031-done-button-modelselect
Jun 21, 2026
Merged

diegosouzapw merged 1 commit into
release/v3.8.33from
feat/port-pr-1031-done-button-modelselect

Conversation

@diegosouzapw

@diegosouzapw diegosouzapw commented Jun 21, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Adds an opt-in keepOpenOnSelect prop to ModelSelectModal. When the
combos page sets it, picking a model no longer auto-closes the modal and
a full-width Done button is rendered in the footer (via Modal's
footer slot) so users can add several models in a row and confirm
explicitly. Inspired by upstream PR the upstream fix.

Why

Combo creation typically adds 2-5 models. Today the modal auto-closes
on every pick, forcing the user to reopen the picker for each model —
the same UX complaint the upstream PR fixes for the JS variant.
OmniRoute already shows an "added" check-mark on previously selected
models (via addedModelValues), so the only missing piece was the
ability to keep the modal open and confirm the batch.

What changed

  • src/shared/components/ModelSelectModal.tsx — new keepOpenOnSelect?: boolean
    prop (default false, so all 10+ existing single-select callers stay
    on auto-close). When set and !multiSelect, the modal renders a
    full-width Done button in its footer. The multiSelect Clear+Done
    footer wins when both are set, to avoid two competing footers.
  • src/app/(dashboard)/dashboard/combos/page.tsx — passes keepOpenOnSelect
    to the picker. Single-line change.
  • tests/unit/ui/model-select-modal-keep-open.test.tsx — new Vitest
    unit test (4 cases) covering: default has no Done button, prop adds
    it, Done click triggers onClose (not a stray onSelect),
    multiSelect still renders exactly one Done button.
  • CHANGELOG.md — entry under [3.8.32] → ✨ New Features.

Notes on the port

  • Upstream PR uses a JS modal with closeOnSelect={false}. OmniRoute's
    TypeScript modal already had a similar multiSelect mode whose footer
    includes a Clear button (calls onSelect(null)), but Clear is
    incompatible with the combo handler shape (combo entries are
    {model, providerId, weight}, not bare values). So instead of
    reusing multiSelect, the port adds a small sibling prop
    (keepOpenOnSelect) that only suppresses auto-close and renders the
    Done button — no Clear, no selectedModels semantics.
  • The t("done") translation key already exists in all locales
    (common.done was added when multiSelect shipped). No new i18n
    keys.
  • Co-authored-by trailer credits the upstream author (@zanuartri).

Test plan

  • npx vitest run tests/unit/ui/model-select-modal-keep-open.test.tsx — 4/4 pass
  • npx vitest run tests/unit/ui/model-select-modal-zero-config.test.tsx — 3/3 pass (no regression)
  • npm run typecheck:core — clean
  • npx eslint on touched files — clean
  • Manual UX expectation: opening the picker in combo creation,
    adding several models, then clicking Done closes the modal. Other
    callers (CLI tool cards) keep auto-close.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a keepOpenOnSelect prop to ModelSelectModal, allowing callers like the combo creation form to keep the modal open for sequential selections, accompanied by a "Done" button in the footer. Unit tests have been added to cover this new functionality. The review feedback recommends minor cleanups in the test file, specifically replacing an unnecessary dynamic import with a standard static import and removing a redundant empty act block.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

useTranslations: () => (key: string) => key,
}));

const { default: ModelSelectModal } = await import("@/shared/components/ModelSelectModal");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using a dynamic import with top-level await is unnecessary here. Standard static imports are more idiomatic in Vitest/TypeScript test files and are fully compatible with Vitest's hoisted mocking system.

Suggested change
const { default: ModelSelectModal } = await import("@/shared/components/ModelSelectModal");
import ModelSelectModal from "@/shared/components/ModelSelectModal";

);
});

await act(async () => {});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This empty act block is redundant because the preceding act block on lines 28-39 already handles the rendering and flushes any pending microtasks/effects. Removing it keeps the test clean and efficient.

@diegosouzapw
diegosouzapw force-pushed the feat/port-pr-1031-done-button-modelselect branch from 65b6ca7 to fa37b2b Compare June 21, 2026 12:03
@diegosouzapw
diegosouzapw changed the base branch from release/v3.8.32 to release/v3.8.33 June 21, 2026 14:02
Adds an opt-in `keepOpenOnSelect` prop to `ModelSelectModal`. When the
combos page sets it, picking a model no longer auto-closes the modal and
a full-width "Done" button is rendered in the footer (via Modal's
`footer` slot) so users can add several models in a row and confirm
explicitly. All other single-select callers keep the existing
auto-close-on-select behaviour (the prop is off by default). When
`multiSelect` is on, the existing Clear+Done footer wins to avoid two
competing footers.

The upstream JS variant gates the Done button on `!closeOnSelect`.

TDD coverage: tests/unit/ui/model-select-modal-keep-open.test.tsx
asserts the Done button is absent by default, present when
`keepOpenOnSelect=true`, fires only `onClose` (not a stray `onSelect`),
and that `multiSelect` still renders exactly one Done button.

Inspired-by: decolua/9router#1031
Co-authored-by: Zanuar Tri Romadon <triromadon@gmail.com>
@diegosouzapw
diegosouzapw force-pushed the feat/port-pr-1031-done-button-modelselect branch from fa37b2b to c49757b Compare June 21, 2026 17:09
@diegosouzapw
diegosouzapw merged commit 247e578 into release/v3.8.33 Jun 21, 2026
3 checks passed
diegosouzapw added a commit that referenced this pull request Jun 21, 2026
#4498)

Rebuilt onto release/v3.8.33; reconciled with the already-merged keepOpenOnSelect (#4496) — deselection now rides the existing keepOpenOnSelect + onDeselect wiring instead of a redundant closeOnSelect prop. Integrated into release/v3.8.33.
@diegosouzapw diegosouzapw mentioned this pull request Jun 22, 2026
@diegosouzapw
diegosouzapw deleted the feat/port-pr-1031-done-button-modelselect branch June 22, 2026 02:48
tkgo11 pushed a commit to tkgo11/OmniRoute that referenced this pull request Sep 23, 2026
…per-inference#4496)

Rebuilt onto release/v3.8.33 (squash-base-stale). Integrated into release/v3.8.33.
tkgo11 pushed a commit to tkgo11/OmniRoute that referenced this pull request Sep 23, 2026
cheaper-inference#4498)

Rebuilt onto release/v3.8.33; reconciled with the already-merged keepOpenOnSelect (cheaper-inference#4496) — deselection now rides the existing keepOpenOnSelect + onDeselect wiring instead of a redundant closeOnSelect prop. Integrated into release/v3.8.33.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant