fix: a newly added model reaches every picker, including the user-facing ones
Some checks failed
Forgejo Android APK / Root app tests (push) Successful in 56s
Forgejo Docker Build / Root app tests (push) Successful in 45s
Forgejo Android APK / Build signed APK (push) Successful in 1m53s
Forgejo Docker Build / Build Docker image (push) Successful in 8s
Forgejo Docker Build / Deploy to the host (push) Failing after 2s
Some checks failed
Forgejo Android APK / Root app tests (push) Successful in 56s
Forgejo Docker Build / Root app tests (push) Successful in 45s
Forgejo Android APK / Build signed APK (push) Successful in 1m53s
Forgejo Docker Build / Build Docker image (push) Successful in 8s
Forgejo Docker Build / Deploy to the host (push) Failing after 2s
The first pass at this covered the two admin cards. It missed the pickers that matter most: the per-tab model selectors in app.js, which every clinical tab uses, and the My Resources model dropdown. Both were filled once at page load, so a model added in Admin was still invisible where people actually choose one. app.js's boot fetch is now a named loadModelList() that also runs on models-changed; My Resources re-runs loadOptions(), which is the same call that decides whether the model row is shown at all. Both rebuilds keep a choice already made. These selects can be rebuilt while someone is halfway through a form, and silently moving them off the model they picked would be worse than not refreshing. Verified against a mutation: removing the app.js listener fails the test that says every picker listens. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dv6sqaY6Vq3ChZHMem3cnU
This commit is contained in:
parent
7305443243
commit
b1e039d834
3 changed files with 49 additions and 2 deletions
|
|
@ -572,7 +572,11 @@ document.addEventListener('DOMContentLoaded', function() {
|
|||
window._currentProvider = 'openrouter';
|
||||
window._defaultModelId = '';
|
||||
|
||||
fetch('/api/models')
|
||||
// Re-read on demand, so a model an administrator adds reaches every per-tab
|
||||
// selector without a page reload. The same call runs at boot and on
|
||||
// models-changed; it is idempotent and rebuilds whatever selectors exist now.
|
||||
function loadModelList() {
|
||||
return fetch('/api/models')
|
||||
.then(function(r) { return r.json(); })
|
||||
.then(function(data) {
|
||||
window._currentModels = data.models || [];
|
||||
|
|
@ -593,12 +597,22 @@ document.addEventListener('DOMContentLoaded', function() {
|
|||
var defaultModelId = data.defaultModel || (window._currentModels.length > 0 ? window._currentModels[0].id : '');
|
||||
window._defaultModelId = defaultModelId;
|
||||
|
||||
// Populate all per-tab model selectors already in DOM
|
||||
// Populate all per-tab model selectors already in DOM. A selector that
|
||||
// already holds a valid choice keeps it: rebuilding must not silently
|
||||
// move someone off the model they picked.
|
||||
document.querySelectorAll('.tab-model-select').forEach(function(sel) {
|
||||
var chosen = sel.value;
|
||||
window._buildModelOptions(sel);
|
||||
if (chosen && Array.prototype.some.call(sel.options, function(o) { return o.value === chosen; })) {
|
||||
sel.value = chosen;
|
||||
}
|
||||
});
|
||||
})
|
||||
.catch(function(err) { console.warn('Models load failed:', err); });
|
||||
}
|
||||
|
||||
loadModelList();
|
||||
document.addEventListener('models-changed', function() { loadModelList(); });
|
||||
|
||||
console.log('✅ App.js DOM ready');
|
||||
|
||||
|
|
|
|||
|
|
@ -11,6 +11,13 @@
|
|||
(function () {
|
||||
var inited = false;
|
||||
|
||||
// A model an administrator adds has to show up here too, without a reload.
|
||||
// loadOptions also decides whether the model row is shown at all, so this is
|
||||
// the same call rather than a partial refresh of the select.
|
||||
document.addEventListener('models-changed', function () {
|
||||
if (inited) loadOptions();
|
||||
});
|
||||
|
||||
document.addEventListener('tabChanged', function (e) {
|
||||
if (!e.detail || e.detail.tab !== 'myresources') return;
|
||||
if (!inited) { init(); inited = true; }
|
||||
|
|
@ -121,6 +128,10 @@
|
|||
var modelRow = document.getElementById('mr-model-row');
|
||||
var models = data.models || [];
|
||||
if (select) {
|
||||
// Rebuilt, but a choice already made is kept: this also runs when an
|
||||
// administrator adds a model, and moving someone off the model they
|
||||
// picked mid-form would be worse than not refreshing at all.
|
||||
var chosen = select.value;
|
||||
select.textContent = '';
|
||||
models.forEach(function (id) {
|
||||
var option = document.createElement('option');
|
||||
|
|
@ -129,6 +140,7 @@
|
|||
if (id === data.defaultModel) option.selected = true;
|
||||
select.appendChild(option);
|
||||
});
|
||||
if (chosen && models.indexOf(chosen) !== -1) select.value = chosen;
|
||||
}
|
||||
if (modelRow) modelRow.hidden = models.length < 2;
|
||||
|
||||
|
|
|
|||
|
|
@ -45,6 +45,27 @@ test('the pickers listen, and re-run a loader that is otherwise once-per-visit',
|
|||
assert.match(images, /if \(loading\) return;/, 'no reload mid-flight');
|
||||
});
|
||||
|
||||
test('every model picker listens, not only the two in the admin cards', () => {
|
||||
// The point is that a model added in Admin is selectable wherever models are
|
||||
// chosen — the per-tab selectors and My Resources included, not just the
|
||||
// admin's own dropdowns.
|
||||
const app = read('public/js/app.js');
|
||||
assert.match(app, /document\.addEventListener\('models-changed'/);
|
||||
assert.match(app, /function loadModelList\(\)/, 'the boot fetch is reusable, not inline');
|
||||
|
||||
const resources = read('public/js/myResources.js');
|
||||
assert.match(resources, /document\.addEventListener\('models-changed'/);
|
||||
assert.match(resources, /if \(inited\) loadOptions\(\);/, 'nothing to refresh before the tab has initialised');
|
||||
});
|
||||
|
||||
test('a refresh never moves someone off the model they picked', () => {
|
||||
// These selects are rebuilt while a form may be half filled in.
|
||||
const app = read('public/js/app.js');
|
||||
assert.match(app, /var chosen = sel\.value;[\s\S]{0,260}sel\.value = chosen;/);
|
||||
const resources = read('public/js/myResources.js');
|
||||
assert.match(resources, /var chosen = select\.value;[\s\S]{0,420}select\.value = chosen;/);
|
||||
});
|
||||
|
||||
test('the toast no longer tells people to go and set a default', () => {
|
||||
// It used to say "now select it as default and click Set Default", which was
|
||||
// advice for the one dropdown that did refresh.
|
||||
|
|
|
|||
Loading…
Reference in a new issue