From 087c694c8d417987caf157b0b4793534def505de Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Fri, 3 Jul 2026 22:07:52 -0400 Subject: [PATCH] feat(#5497): reuse the rich model picker in Preferences --- static/index.html | 2 + static/panels.js | 6 + static/style.css | 39 +++++ static/ui.js | 151 ++++++++++++++++-- tests/test_2791_model_picker_keyboard_nav.py | 2 +- ...est_issue1103_reasoning_chip_visibility.py | 9 +- ...test_issue5497_preferences_model_picker.py | 101 ++++++++++++ 7 files changed, 290 insertions(+), 20 deletions(-) create mode 100644 tests/test_issue5497_preferences_model_picker.py diff --git a/static/index.html b/static/index.html index 4d40fed8..8fa14207 100644 --- a/static/index.html +++ b/static/index.html @@ -1136,8 +1136,10 @@
+ +
Used for new conversations. Existing conversations keep their selected model.
diff --git a/static/panels.js b/static/panels.js index ac8390e8..5a7d9f62 100644 --- a/static/panels.js +++ b/static/panels.js @@ -8644,7 +8644,13 @@ async function loadSettingsPanel(){ }else{ modelSel.value=_settingsHermesDefaultModelOnOpen; } + if(typeof closeSettingsModelDropdown==='function') closeSettingsModelDropdown(); + if(typeof mountSettingsModelPicker==='function') mountSettingsModelPicker(); modelSel.addEventListener('change',_markSettingsDirty,{once:false}); + if(!modelSel._settingsChipSyncBound){ + modelSel._settingsChipSyncBound=true; + modelSel.addEventListener('change',()=>{if(typeof syncSettingsModelChip==='function') syncSettingsModelChip();},{once:false}); + } } // Auxiliary models — load task assignments and provider/model options _bindMainAdvancedOptionsButton(); diff --git a/static/style.css b/static/style.css index b49a6b01..5de172f8 100644 --- a/static/style.css +++ b/static/style.css @@ -4840,16 +4840,55 @@ main.main > #mainPlugin{display:none;} #mainSettings .settings-field > div[style*="color: var(--muted)"]{font-size:12px;color:var(--muted);line-height:1.5;} .model-advanced-row{ + position:relative; display:grid; grid-template-columns:minmax(0,1fr) 34px; gap:8px; align-items:center; } .model-advanced-row #settingsModel{ + display:none; width:100%!important; min-width:0; box-sizing:border-box; } +.settings-model-chip{ + width:100%; + min-width:0; + box-sizing:border-box; + padding:8px 10px; + border:1px solid var(--border2); + border-radius:6px; + background:var(--code-bg); + color:var(--text); + font:inherit; + font-size:13px; + text-align:left; + cursor:pointer; + overflow:hidden; + text-overflow:ellipsis; + white-space:nowrap; +} +.settings-model-chip:hover,.settings-model-chip.active,.settings-model-chip:focus-visible{ + border-color:var(--accent); + outline:none; +} +.settings-model-dropdown{ + top:calc(100% + 4px); + bottom:auto; + left:0; + width:calc(100% - 42px); + min-width:min(360px,100%); + max-width:100%; + max-height:360px; + z-index:260; +} +.settings-model-dropdown .model-search-row{ + position:sticky; + top:0; + background:var(--surface); + z-index:1; +} .model-advanced-btn{ width:32px; height:32px; diff --git a/static/ui.js b/static/ui.js index 6af31688..11b29386 100644 --- a/static/ui.js +++ b/static/ui.js @@ -2430,23 +2430,36 @@ function _refreshOpenModelDropdown(){ renderModelDropdown(); if(typeof _positionModelDropdown==='function') _positionModelDropdown(); } + const sdd=$('settingsModelDropdown'); + if(sdd&&sdd.classList&&sdd.classList.contains('open')&&typeof renderModelDropdown==='function'){ + renderModelDropdown({ + dropdownId:'settingsModelDropdown', + selectId:'settingsModel', + forceOpenKey:'settingsModel', + closeDropdown:closeSettingsModelDropdown, + selectModel:selectSettingsModelFromDropdown, + scopeNoteText:t('settings_desc_model')||'Used for new conversations. Existing conversations keep their selected model.', + }); + } } function _applyModelToDropdown(modelId, sel, preferredProviderId, opts){ if(!modelId||!sel) return null; - const currentState=(sel.id==='modelSelect'&&typeof _modelStateForSelect==='function') + const isRichPickerSelect=sel.id==='modelSelect'||sel.id==='settingsModel'; + const currentState=(isRichPickerSelect&&typeof _modelStateForSelect==='function') ? _modelStateForSelect(sel, sel.value) : null; const resolved=_findModelInDropdown(modelId,sel,preferredProviderId); if(resolved){ sel.value=resolved; - if(sel.id==='modelSelect'){ + if(isRichPickerSelect){ const resolvedState=typeof _modelStateForSelect==='function' ? _modelStateForSelect(sel, resolved) : {model:resolved,model_provider:preferredProviderId||null}; const pickerChanged= !!(opts&&opts.forceRefresh) || !currentState || String(currentState.model||'')!==String(resolvedState.model||'') || String(currentState.model_provider||'')!==String(resolvedState.model_provider||''); - if(typeof syncModelChip==='function') syncModelChip(); + if(sel.id==='modelSelect'&&typeof syncModelChip==='function') syncModelChip(); + if(sel.id==='settingsModel'&&typeof syncSettingsModelChip==='function') syncSettingsModelChip(); if(pickerChanged) _refreshOpenModelDropdown(); } return resolved; @@ -2472,6 +2485,10 @@ function _ensureModelOptionInDropdown(modelId, sel, preferredProviderId){ if(typeof syncModelChip==='function') syncModelChip(); _refreshOpenModelDropdown(); } + if(sel.id==='settingsModel'){ + if(typeof syncSettingsModelChip==='function') syncSettingsModelChip(); + _refreshOpenModelDropdown(); + } return modelId; } function _modelStateFromAppliedDropdown(sel, modelValue){ @@ -3078,9 +3095,16 @@ function _mountSearchableModelSelect(opts={}){ } function renderModelDropdown(){ - const dd=$('composerModelDropdown'); - const sel=$('modelSelect'); + const opts=arguments[0]||{}; + const dd=$(opts.dropdownId||'composerModelDropdown'); + const sel=$(opts.selectId||'modelSelect'); if(!dd||!sel) return; + const selectFromDropdown=typeof opts.selectModel==='function' + ? opts.selectModel + : (value,provider)=>selectModelFromDropdown(value,provider); + const closeDropdown=typeof opts.closeDropdown==='function' + ? opts.closeDropdown + : closeModelDropdown; // Group(s) that must render OPEN even though they aren't the selected group — // set when the user expands a group's overflow via "Show more" so a later full // re-render doesn't re-collapse it (_groupOpenState is rebuilt per render, so @@ -3089,8 +3113,10 @@ function renderModelDropdown(){ // #3691 node test driver evals the function body without module scope). const _forceOpenGroups=(()=>{ const _g=(typeof window!=='undefined')?window:(typeof globalThis!=='undefined'?globalThis:{}); - if(!_g.__modelGroupForceOpen) _g.__modelGroupForceOpen=new Set(); - return _g.__modelGroupForceOpen; + const key=opts.forceOpenKey||'composer'; + if(!_g.__modelGroupForceOpenByPicker) _g.__modelGroupForceOpenByPicker={}; + if(!_g.__modelGroupForceOpenByPicker[key]) _g.__modelGroupForceOpenByPicker[key]=new Set(); + return _g.__modelGroupForceOpenByPicker[key]; })(); const _modelData=[]; const _groupMeta=new Map(); @@ -3187,7 +3213,7 @@ function renderModelDropdown(){ // Create search input FIRST before filterModels definition const _scopeNote=document.createElement('div'); _scopeNote.className='model-scope-note'; - _scopeNote.textContent=t('model_scope_advisory')||'Applies to this conversation from your next message.'; + _scopeNote.textContent=opts.scopeNoteText||(t('model_scope_advisory')||'Applies to this conversation from your next message.'); const _searchRow=document.createElement('div'); _searchRow.className='model-search-row'; _searchRow.innerHTML=``; @@ -3227,7 +3253,7 @@ function renderModelDropdown(){ const _plainGroup=m.group?String(m.group).replace(/\s*\(\d+\s+of\s+\d+\)\s*$/,''):''; const providerChip=(_plainGroup&&withProviderChip)?`${esc(_plainGroup)}`:''; row.innerHTML=`
${esc(m.name)}${badgeHtml}${providerChip}
${esc(m.id)}`; - row.onclick=()=>selectModelFromDropdown(m.value,m.providerId||(m.badge&&m.badge.provider)||null); + row.onclick=()=>selectFromDropdown(m.value,m.providerId||(m.badge&&m.badge.provider)||null); return row; }; const _expandOverflowGroup=(groupMetaEntry)=>{ @@ -3251,7 +3277,7 @@ function renderModelDropdown(){ // expander gone, search term reapplied. const _fullReRender=()=>{ const _term=(_si&&_si.value)||''; - renderModelDropdown(); + renderModelDropdown(opts); const ns=dd.querySelector('.model-search-input'); if(ns){ ns.value=_term; (ns._listeners&&ns._listeners.input)?ns._listeners.input():ns.dispatchEvent(new Event('input')); } }; @@ -3280,7 +3306,7 @@ function renderModelDropdown(){ for(const m of extraModels){ if(!m||!m.id) continue; if(_alreadyShown.has(esc(m.id))) continue; - const row=_buildModelRow({value:m.id,name:m.label||m.id,id:m.id,group:_plainLabel,groupKey,providerId:(og.dataset&&og.dataset.provider)||''},$('modelSelect'),false); + const row=_buildModelRow({value:m.id,name:m.label||m.id,id:m.id,group:_plainLabel,groupKey,providerId:(og.dataset&&og.dataset.provider)||''},sel,false); wrap.insertBefore(row,moreEl); if(!firstNewRow) firstNewRow=row; } @@ -3346,7 +3372,7 @@ function renderModelDropdown(){ const _underOwnHeading=shouldRenderHeading&&!!(m.groupKey&&_groupWrappers[m.groupKey]); const providerChip=(_plainGroup&&!_underOwnHeading)?`${esc(_plainGroup)}`:''; row.innerHTML=`
${esc(m.name)}${badgeHtml}${providerChip}
${esc(m.id)}`; - row.onclick=()=>selectModelFromDropdown(m.value,m.providerId||(m.badge&&m.badge.provider)||null); + row.onclick=()=>selectFromDropdown(m.value,m.providerId||(m.badge&&m.badge.provider)||null); return row; }; const _filterModels=(term)=>{ @@ -3441,7 +3467,7 @@ function renderModelDropdown(){ } const badgeHtml=m.badge?`${esc(badgeLabel)}`:''; row.innerHTML=`
${esc(modelName)}${badgeHtml}
${esc(m.id)}`; - row.onclick=()=>selectModelFromDropdown(m.value,(m.badge&&m.badge.provider)||m.providerId||null); + row.onclick=()=>selectFromDropdown(m.value,(m.badge&&m.badge.provider)||m.providerId||null); dd.appendChild(row); } } @@ -3609,7 +3635,7 @@ function renderModelDropdown(){ if(typeof row.scrollIntoView==='function') row.scrollIntoView({block:'nearest'}); }; _si.addEventListener('keydown',e=>{ - if(e.key==='Escape'){closeModelDropdown();return;} + if(e.key==='Escape'){closeDropdown();return;} if(e.key==='ArrowDown'||e.key==='ArrowUp'||e.key==='Enter'){ const rows=_visibleModelRows(); if(!rows.length){if(e.key==='Enter') e.preventDefault();return;} @@ -3626,9 +3652,9 @@ function renderModelDropdown(){ _si.addEventListener('click',e=>e.stopPropagation()); _sc.onclick=()=>{ _si.value=''; _filterModels(''); _si.focus(); }; _sc.addEventListener('keydown',e=>{if(e.key==='Enter'||e.key===' '){ _si.value=''; _filterModels(''); _si.focus(); e.preventDefault(); }}); - const _applyCustom=()=>{const v=_ci.value.trim();if(!v)return;selectModelFromDropdown(v);_ci.value='';}; + const _applyCustom=()=>{const v=_ci.value.trim();if(!v)return;selectFromDropdown(v,null);_ci.value='';}; _cb.onclick=_applyCustom; - _ci.addEventListener('keydown',e=>{if(e.key==='Enter'){e.preventDefault();_applyCustom();}if(e.key==='Escape'){closeModelDropdown();}}); + _ci.addEventListener('keydown',e=>{if(e.key==='Enter'){e.preventDefault();_applyCustom();}if(e.key==='Escape'){closeDropdown();}}); _ci.addEventListener('click',e=>e.stopPropagation()); dd.appendChild(_scopeNote); dd.appendChild(_searchRow); @@ -3693,12 +3719,105 @@ function closeModelDropdown(){ if(mobileAction) mobileAction.classList.remove('active'); } +function closeSettingsModelDropdown(){ + const dd=$('settingsModelDropdown'); + const chip=$('settingsModelChip'); + if(dd) dd.classList.remove('open'); + if(chip){ + chip.classList.remove('active'); + chip.setAttribute('aria-expanded','false'); + } +} + +function syncSettingsModelChip(){ + const sel=$('settingsModel'); + const chip=$('settingsModelChip'); + if(!sel||!chip) return; + const opt=sel.selectedOptions&&sel.selectedOptions[0]; + const text=(opt&&opt.textContent)||getModelLabel(sel.value||'')||t('settings_label_model')||'Default Model'; + chip.textContent=text; + chip.title=sel.value||text; +} + +function selectSettingsModelFromDropdown(value,preferredProviderId){ + const sel=$('settingsModel'); + if(!sel){closeSettingsModelDropdown();return;} + const provider=String(preferredProviderId||'').trim()||null; + if(typeof _ensureModelOptionInDropdown==='function'){ + _ensureModelOptionInDropdown(value,sel,provider); + }else{ + sel.value=value; + if(typeof syncSettingsModelChip==='function') syncSettingsModelChip(); + } + closeSettingsModelDropdown(); + try{ + if(typeof Event==='function') sel.dispatchEvent(new Event('change',{bubbles:true})); + else if(typeof sel.onchange==='function') sel.onchange(); + }catch(_){} +} + +function openSettingsModelDropdown(){ + const dd=$('settingsModelDropdown'); + const sel=$('settingsModel'); + const chip=$('settingsModelChip'); + if(!dd||!sel) return; + renderModelDropdown({ + dropdownId:'settingsModelDropdown', + selectId:'settingsModel', + forceOpenKey:'settingsModel', + closeDropdown:closeSettingsModelDropdown, + selectModel:selectSettingsModelFromDropdown, + scopeNoteText:t('settings_desc_model')||'Used for new conversations. Existing conversations keep their selected model.', + }); + dd.classList.add('open'); + if(chip){ + chip.classList.add('active'); + chip.setAttribute('aria-expanded','true'); + } + setTimeout(()=>{ + const input=dd.querySelector('.model-search-input'); + if(input) input.focus(); + },0); +} + +function toggleSettingsModelDropdown(){ + const dd=$('settingsModelDropdown'); + if(dd&&dd.classList.contains('open')){closeSettingsModelDropdown();return;} + openSettingsModelDropdown(); +} + +function mountSettingsModelPicker(){ + const chip=$('settingsModelChip'); + const sel=$('settingsModel'); + if(!chip||!sel) return; + syncSettingsModelChip(); + if(!chip._settingsModelPickerBound){ + chip._settingsModelPickerBound=true; + chip.addEventListener('click',e=>{ + e.preventDefault(); + e.stopPropagation(); + toggleSettingsModelDropdown(); + }); + chip.addEventListener('keydown',e=>{ + if(e.key==='Enter'||e.key===' '||e.key==='ArrowDown'){ + e.preventDefault(); + toggleSettingsModelDropdown(); + } + }); + } +} + document.addEventListener('click',e=>{ if( !e.target.closest('#composerModelChip') && !e.target.closest('#composerMobileModelAction') && !e.target.closest('#composerModelDropdown') ) closeModelDropdown(); + if( + !e.target.closest('#settingsModelChip') && + !e.target.closest('#settingsModel') && + !e.target.closest('#settingsModelDropdown') + ) closeSettingsModelDropdown(); }); window.addEventListener('resize',()=>{ const dd=$('composerModelDropdown'); diff --git a/tests/test_2791_model_picker_keyboard_nav.py b/tests/test_2791_model_picker_keyboard_nav.py index 74910024..1846c695 100644 --- a/tests/test_2791_model_picker_keyboard_nav.py +++ b/tests/test_2791_model_picker_keyboard_nav.py @@ -24,7 +24,7 @@ def test_highlight_class_has_style(): def test_escape_still_closes_dropdown(): # Existing Escape-to-close behavior must not regress. - assert "if(e.key==='Escape'){closeModelDropdown();return;}" in UI_JS + assert "if(e.key==='Escape'){closeDropdown();return;}" in UI_JS def test_enter_picks_highlighted_row(): diff --git a/tests/test_issue1103_reasoning_chip_visibility.py b/tests/test_issue1103_reasoning_chip_visibility.py index 5c8ac758..a0acc0b7 100644 --- a/tests/test_issue1103_reasoning_chip_visibility.py +++ b/tests/test_issue1103_reasoning_chip_visibility.py @@ -135,10 +135,13 @@ def test_selectModelFromDropdown_defers_reasoning_sync_to_onchange(): def test_model_dropdown_passes_provider_to_select(): - """Composer model rows must pass provider context into selectModelFromDropdown.""" + """Composer model rows must pass provider context through the shared picker callback.""" with open("static/ui.js") as f: src = f.read() + assert "selectModelFromDropdown(value,provider)" in src, ( + "composer default picker callback must still route to selectModelFromDropdown" + ) assert re.search( - r"selectModelFromDropdown\(m\.value,\s*m\.providerId", + r"selectFromDropdown\(m\.value,\s*m\.providerId", src, - ), "model dropdown rows must pass providerId to selectModelFromDropdown" + ), "model dropdown rows must pass providerId through the shared callback" diff --git a/tests/test_issue5497_preferences_model_picker.py b/tests/test_issue5497_preferences_model_picker.py new file mode 100644 index 00000000..aae665fa --- /dev/null +++ b/tests/test_issue5497_preferences_model_picker.py @@ -0,0 +1,101 @@ +"""Regression coverage for #5497 Preferences default model picker parity.""" + +from __future__ import annotations + +import re +from pathlib import Path + + +REPO = Path(__file__).resolve().parents[1] +INDEX_HTML = (REPO / "static" / "index.html").read_text(encoding="utf-8") +PANELS_JS = (REPO / "static" / "panels.js").read_text(encoding="utf-8") +STYLE_CSS = (REPO / "static" / "style.css").read_text(encoding="utf-8") +UI_JS = (REPO / "static" / "ui.js").read_text(encoding="utf-8") + + +def _function_body(src: str, name: str) -> str: + match = re.search(rf"function\s+{re.escape(name)}\s*\([^)]*\)\s*\{{", src) + assert match, f"{name} not found" + start = match.end() + depth = 1 + pos = start + while pos < len(src) and depth: + if src[pos] == "{": + depth += 1 + elif src[pos] == "}": + depth -= 1 + pos += 1 + assert depth == 0, f"{name} body did not terminate" + return src[start : pos - 1] + + +def test_preferences_default_model_field_keeps_source_select_and_adds_rich_picker_shell(): + field_start = INDEX_HTML.index('id="settingsModelChip"') + field_end = INDEX_HTML.index('data-i18n="settings_desc_model"', field_start) + field = INDEX_HTML[field_start:field_end] + + assert 'id="settingsModelChip"' in field + assert 'id="settingsModel"' in field + assert 'id="settingsModelDropdown"' in field + assert 'class="model-dropdown settings-model-dropdown"' in field + assert 'id="mainAdvancedBtn"' in field + assert field.index('id="settingsModelChip"') < field.index('id="mainAdvancedBtn"') + + +def test_settings_model_select_is_hidden_but_remains_advanced_options_source(): + assert ".model-advanced-row #settingsModel" in STYLE_CSS + assert "display:none;" in STYLE_CSS[STYLE_CSS.index(".model-advanced-row #settingsModel") : STYLE_CSS.index(".model-advanced-btn")] + + bind_body = _function_body(PANELS_JS, "_bindMainAdvancedOptionsButton") + assert "const modelSel=$('settingsModel');" in bind_body + assert "modelSel.parentElement" in bind_body + assert "mainAdvancedBtn" in bind_body + + +def test_render_model_dropdown_accepts_settings_select_and_callbacks(): + body = _function_body(UI_JS, "renderModelDropdown") + + assert "opts.dropdownId||'composerModelDropdown'" in body + assert "opts.selectId||'modelSelect'" in body + assert "opts.selectModel" in body + assert "opts.closeDropdown" in body + assert "selectFromDropdown(m.value" in body + assert "row.onclick=()=>selectModelFromDropdown" not in body + assert "closeDropdown();return;" in body + assert "opts.forceOpenKey||'composer'" in body + assert "renderModelDropdown(opts)" in body + assert "opts.scopeNoteText||" in body + + +def test_preferences_picker_routes_through_shared_renderer_without_panel_copy(): + mount_body = _function_body(UI_JS, "openSettingsModelDropdown") + select_body = _function_body(UI_JS, "selectSettingsModelFromDropdown") + panels_model_block = PANELS_JS[ + PANELS_JS.index("const modelSel=$('settingsModel');") : PANELS_JS.index("// Auxiliary models", PANELS_JS.index("const modelSel=$('settingsModel');")) + ] + + assert "renderModelDropdown({" in mount_body + assert "dropdownId:'settingsModelDropdown'" in mount_body + assert "selectId:'settingsModel'" in mount_body + assert "selectModel:selectSettingsModelFromDropdown" in mount_body + assert "scopeNoteText:t('settings_desc_model')" in mount_body + assert "_ensureModelOptionInDropdown(value,sel,provider)" in select_body + assert "sel.dispatchEvent(new Event('change',{bubbles:true}))" in select_body + assert "syncSettingsModelChip" in select_body + assert "closeSettingsModelDropdown" in panels_model_block + assert "mountSettingsModelPicker" in panels_model_block + assert "_settingsChipSyncBound" in panels_model_block + assert ".model-opt" not in PANELS_JS + assert "model-search-input" not in PANELS_JS + + +def test_preferences_picker_refreshes_existing_save_and_dirty_contracts(): + autosave_body = _function_body(PANELS_JS, "_autosavePreferencesSettings") + save_body = _function_body(PANELS_JS, "saveSettings") + refresh_body = _function_body(UI_JS, "_refreshOpenModelDropdown") + + assert "_captureModelDropdownSelection(modelSel)" in autosave_body + assert "_captureModelDropdownSelection($('settingsModel'))" in save_body + assert "await api('/api/default-model',{method:'POST',body:JSON.stringify({model,provider:modelState.model_provider||null})});" in save_body + assert "settingsModelDropdown" in refresh_body + assert "selectModel:selectSettingsModelFromDropdown" in refresh_body