feat(model-picker): arrow-key navigation and Enter-to-select

[Sanjays2402]

The model picker has a working search input but no keyboard navigation. After you type a filter you still have to take your hand off the keyboard to click the row you want, which is the part of #2791 that bites the most in day-to-day use.

This wires three handlers onto the existing search input:

• ArrowDown / ArrowUp move a .is-highlighted class through the visible (and filter-narrowed) .model-opt rows, with wraparound at the ends.
• Enter clicks the highlighted row (or the first row if nothing is highlighted yet, so typing a filter and immediately hitting Enter just picks the top match).
• Escape keeps its existing close-dropdown behavior.

Highlight gets a small CSS rule that reuses the existing border-color and brightens the row a bit more than :hover so the selection is obvious without redoing the dropdown styling.

The maintainer's review on #2791 already broke the issue into three slices (fuzzy scorer, match highlighting, keyboard nav) and called this one out as the clean keyboard-only change. Fuzzy matching and <mark> highlighting are intentionally left for follow-ups — this PR is just the nav.

Verified

• python -m pytest tests/test_2791_model_picker_keyboard_nav.py — 4 passed.
• node --check static/ui.js — clean.
• Manual: opened picker, typed "son", arrowed down/up through filtered rows, Enter selected. Escape still closes.

Closes #2791

Sanity check: not inside a <form>

Confirmed in source: static/index.html contains no <form> element at all (grep returns no matches). #composerModelDropdown lives under #composer as a plain <div> with no form ancestor, so the preventDefault() on empty-list Enter is defensive only and won't be exercised in practice.

Review-feedback fixes (7bb7e30)

• .model-opt.active.is-highlighted rule added so the selection accent (--accent-bg) survives when the keyboard cursor lands on the currently-selected row, with outline: 1px solid var(--accent) doing the cursor indication on its own.
• CHANGELOG entry now calls out that typing into the filter resets the highlight by design (DOM is rebuilt, retyping is a fresh choice).

[nesquena-hermes]

Heads up: duplicate of #2953

You opened #2953 54 seconds after this one with the same scope (closes #2791, same static/ui.js keydown handler change, same .model-opt.is-highlighted CSS rule). Same author. Maintainer will want to pick one; close the other to keep review noise down. The two branches diverge slightly:

• This PR (feat(model-picker): arrow-key navigation and Enter-to-select #2952) factors out three helpers (_visibleModelRows, _activeRowIndex, _highlightRow) and calls row.click() to fire the selection.
• feat(ui): keyboard navigation (arrows + enter) in model picker #2953 inlines the same logic in one block and calls row.onclick() directly.

Both work against the rendered DOM at static/ui.js:1481 and :1520:

row.onclick=()=>selectModelFromDropdown(m.value);

I'll do the substantive review on this one since it landed first; please consider closing #2953 (or rebasing #2953 onto whichever variant you prefer and closing this one).

Code reference

The added handler at static/ui.js:1538-1561 (PR head):

_si.addEventListener('keydown',e=>{
  if(e.key==='Escape'){closeModelDropdown();return;}
  if(e.key==='ArrowDown'||e.key==='ArrowUp'||e.key==='Enter'){
    const rows=_visibleModelRows();
    if(!rows.length){if(e.key==='Enter') e.preventDefault();return;}
    const cur=_activeRowIndex(rows);
    if(e.key==='ArrowDown'){e.preventDefault();_highlightRow(rows,cur<0?0:Math.min(rows.length-1,cur+1));return;}
    ...
  }
});

And the supporting style rule at static/style.css:2039:

.model-opt.is-highlighted{background:rgba(255,255,255,.1);outline:1px solid var(--border2);outline-offset:-1px;}

Diagnosis

1. row.click() is the right call (vs row.onclick() in #2953)

Both rows are bound only via row.onclick=... today (static/ui.js:1481,1520), so either path fires. But row.click() dispatches a real synthetic click event, so if anyone later attaches a second handler via addEventListener (e.g. for telemetry on row selection, which is exactly the shape recent issues asked about), only row.click() continues to work. row.onclick() would silently skip the new listener. Small thing, but it makes this PR's variant more future-proof than #2953's — worth keeping if reviewers ask.

2. Highlight is lost on every keystroke (intentional or not?)

_filterModels() at static/ui.js:1447 does dd.innerHTML='' and rebuilds all rows. Since is-highlighted is a class on DOM nodes that get destroyed on filter, typing any character clears the highlight. Then Enter picks "the first row" because _activeRowIndex returns -1.

This is actually probably the right behavior — the user retyped, so their previous arrow-key choice is stale. But it means the documented "ArrowDown / ArrowUp moves the highlight, Enter picks highlighted (or first)" UX has an implicit "until you type again" caveat. Worth one line in the CHANGELOG so it's not surprising. The alternative (preserve highlight by re-applying it after filter rebuild, e.g. by remembering m.value rather than a DOM ref) is more code than the PR is worth.

3. Inert no-op on Enter when there are no rows

The branch:

if(!rows.length){if(e.key==='Enter') e.preventDefault();return;}

is right — without preventDefault(), an Enter with the search input focused submits the enclosing form (if any) or fires a default behavior. Worth confirming that the dropdown isn't ever rendered inside a form (the only one that comes to mind is the composer, and the picker opens in a fixed-position overlay so it shouldn't be a child of the composer form, but worth a sanity check before merge).

4. The Configured row's "active" class clashes visually with the highlight

static/style.css:2039-2040 after the patch:

.model-opt.is-highlighted{background:rgba(255,255,255,.1);outline:1px solid var(--border2);outline-offset:-1px;}
.model-opt.active{background:var(--accent-bg);}

.active is applied at static/ui.js:1466,1515 to the row whose value matches sel.value — i.e. the currently-selected model. If the user navigates with arrows over their currently-selected row, both classes apply and the .active background loses to the later-declared .is-highlighted background:rgba(255,255,255,.1) (because both have the same specificity and .is-highlighted is later in source). The selection accent is lost while the row is highlighted.

Two cheap fixes:

/* Either: make highlight win additively without replacing the selection accent */
.model-opt.active.is-highlighted{background:var(--accent-bg);outline:1px solid var(--accent);}

/* Or: bump the highlight rule below .active in source order so .active wins, and rely on
   the outline alone to indicate the keyboard cursor over the selected row */

The first variant is what I'd ship — the selected model deserves to keep its accent color even when the keyboard cursor is on it, and the outline alone (no background change) is a sufficient keyboard-focus indicator.

5. Tests are correctly source-level

tests/test_2791_model_picker_keyboard_nav.py asserts string presence in the JS/CSS. That's the right shape for this kind of change (no JS test runner in the repo) and matches how tests/test_chinese_locale.py and friends are structured. Good.

Test plan after fixes

1. Open the model picker, type a filter. Highlight should reset to nothing (each keystroke). Hit Enter — first match selects.
2. Open the picker without typing. ArrowDown highlights the first row. Down arrow walks to the last row then wraps to the first. ArrowUp wraps the other way. Enter picks. Escape closes.
3. Navigate the keyboard cursor over your currently-selected model row (.active). The accent background should remain visible under the highlight outline.
4. Open the picker with zero models loaded (e.g. provider not configured). Hit Enter. No selection, no form submission, no console error.
5. After typing a non-matching filter ("zzzzz"), confirm Enter doesn't crash and the "No models found" row is not selectable.
6. Verify dropdown isn't a descendant of a <form> that would submit on Enter (sanity check the parent chain in DevTools).

Optional polish

• Consider Home / End / PageUp / PageDown for big provider lists. Not blocking — arrows + Enter cover 95% of the workflow.
• The maintainer's feat: fuzzy search for model selection dropdown #2791 review broke this work into three slices (keyboard nav, fuzzy scorer, <mark> highlighting). Keyboard nav is the right first slice and you correctly didn't bundle the others — good scope discipline.

LGTM in concept. Decide which of #2952/#2953 to land, fix the .active+.is-highlighted cascade collision, then this is mergeable.

[Sanjays2402]

Fixed the cascade collision in 7bb7e30: added .model-opt.active.is-highlighted{background:var(--accent-bg);outline:1px solid var(--accent);outline-offset:-1px;} so the selection accent survives when the keyboard cursor lands on the active row, with the outline alone serving as the cursor indicator. Added a CHANGELOG note that typing into the filter resets the highlight by design. Confirmed in source: #composerModelDropdown has no <form> ancestor anywhere in static/index.html, so the preventDefault() on empty Enter is belt-and-braces only.