mirror of
https://gitcode.com/JianFeeeee/ModelRouter.git
synced 2026-10-03 23:54:06 +00:00
fix(ui): per-key 弹窗复用共享弹窗ID、编辑即落盘、空画布显示226个模型
截图看渲染 + 交互实测后发现的四个问题,都出在我自己新写的代码里。 一、弹窗用了全站共享的 #modal-wrap。 这个 id 是 App 内所有弹窗共用的,且允许多个同时存在("添加档位"选择器就 开在本弹窗之上)。既有代码靠 .closest() 或 closeTopModal() 从按钮往上找, 是有效的——但本弹窗还要从画布侧被查(sortCanvasEl / keyAutoClose / keyAutoInherit),那里没有按钮可走。实测点"添加档位"后 DOM 里同时有 modal-wrap=1 和 key-auto-modal=1,getElementById 会命中文档里的第一个, 也就是用户没在看的那一个:取消关掉错误的框,画布也可能解析到选择器上。 改为独立 id #key-auto-modal。 二、编辑即落盘,保存/取消形同虚设。 scrAddFromForm / sortScopeSave / scrDelSlot 等 6 处编辑后立刻调 persistAuto()。全局页这样是有意的(改动立即生效),但它和自己页面上那句 "点击保存排序后生效"的提示自相矛盾;在 per-key 弹窗里更严重——加一个槽位 就已经写进那个用户的链了,取消也无法撤销。收敛到 afterChainEdit(): 有 scope 时只提示、不落盘,保存是唯一写入口。 三、无独立链时画布显示全部 226 个模型。 注释写的是"显示空画布",实现却是 buildLanes(idx, [], 'chat')——空 rules 会走 discovery 兜底,把每个源的模型全填进去。用户打开弹窗看到 226 个色块, 会以为这些就是这个密钥的链。改为空 rules 直接给 []。 四、"使用全局链"提示不随画布变化。 加完槽位后仍显示"该密钥使用全局 AUTO 链",与旁边的色块直接矛盾。改为由 afterChainEdit 触发、按画布是否有内容决定去留。 另:批量替换时正则的缩进假设错了一次,把 afterChainEdit 自己改成了自调用 (无限递归),node --check 抓到后修掉。这类批量替换必须先过语法检查。 判据两条 + 变异:弹窗回到共享 id → 判红;去掉 scope 分支 → 判红。 CDP 实测:弹窗 id 唯一、添加后画布 0→1 块、添加后服务端 inherits=true (未落盘)、取消后仍 inherits=true、选择器与弹窗并存互不干扰。 Co-Authored-By: ModelRouter <noreply@modelrouter.dev>
This commit is contained in:
@ -3710,7 +3710,7 @@ recsState.keyNames = keyNames || {};
|
|||||||
const k = document.getElementById("key-auto-canvas");
|
const k = document.getElementById("key-auto-canvas");
|
||||||
if (k) return k;
|
if (k) return k;
|
||||||
}
|
}
|
||||||
return document.getElementById("scr-canvas");
|
return document.getElementById("scr-canvas");
|
||||||
}
|
}
|
||||||
function paintSort(affected) {
|
function paintSort(affected) {
|
||||||
const cv = sortCanvasEl();
|
const cv = sortCanvasEl();
|
||||||
@ -4133,6 +4133,25 @@ const cv = sortCanvasEl();
|
|||||||
// editor means a per-key chain gets exactly the features the global one
|
// editor means a per-key chain gets exactly the features the global one
|
||||||
// has (drag ordering, per-slot quota/period/hours) instead of a reduced
|
// has (drag ordering, per-slot quota/period/hours) instead of a reduced
|
||||||
// second implementation that would drift.
|
// second implementation that would drift.
|
||||||
|
// afterChainEdit is the single place an in-canvas edit decides whether to
|
||||||
|
// write through. On the GLOBAL page edits take effect immediately (that
|
||||||
|
// page has always behaved that way). In the per-key dialog they must NOT:
|
||||||
|
// that dialog has Save and Cancel, and auto-persisting meant a slot added
|
||||||
|
// (or dragged, or deleted) was already committed to the user's chain
|
||||||
|
// before Save — Cancel could not undo it, and the "Click Save for it to
|
||||||
|
// take effect" hint was a lie.
|
||||||
|
function afterChainEdit() {
|
||||||
|
// Every in-canvas edit lands here, so this is the one place that keeps
|
||||||
|
// the "uses the global chain" notice honest while the user works.
|
||||||
|
if (typeof keyAutoInheritNotice === "function") keyAutoInheritNotice();
|
||||||
|
if (sortState.scope) {
|
||||||
|
toast(t("sortHintSave"));
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
persistAuto()
|
||||||
|
.then(() => toast(t("sortHintSave")))
|
||||||
|
.catch((e) => toast(e.message));
|
||||||
|
}
|
||||||
async function persistAuto() {
|
async function persistAuto() {
|
||||||
const rules = lanesToRules(sortState.lanes);
|
const rules = lanesToRules(sortState.lanes);
|
||||||
if (sortState.scope) {
|
if (sortState.scope) {
|
||||||
@ -4248,9 +4267,7 @@ const cv = sortCanvasEl();
|
|||||||
if (btn) btn.closest("#modal-wrap")?.remove();
|
if (btn) btn.closest("#modal-wrap")?.remove();
|
||||||
else closeTopModal();
|
else closeTopModal();
|
||||||
paintSort([li]);
|
paintSort([li]);
|
||||||
persistAuto()
|
afterChainEdit();
|
||||||
.then(() => toast(t("sortHintSave")))
|
|
||||||
.catch((e) => toast(e.message));
|
|
||||||
}
|
}
|
||||||
function sortScopeSave(li, ji, btn) {
|
function sortScopeSave(li, ji, btn) {
|
||||||
const it = sortState.lanes[li].models[ji];
|
const it = sortState.lanes[li].models[ji];
|
||||||
@ -4263,9 +4280,7 @@ const cv = sortCanvasEl();
|
|||||||
it.meta = { quota: q, period: p, hours: p || q ? h : 0 };
|
it.meta = { quota: q, period: p, hours: p || q ? h : 0 };
|
||||||
(btn ? btn.closest("#modal-wrap") : null) || closeTopModal();
|
(btn ? btn.closest("#modal-wrap") : null) || closeTopModal();
|
||||||
paintSort([li]);
|
paintSort([li]);
|
||||||
persistAuto()
|
afterChainEdit();
|
||||||
.then(() => toast(t("sortHintSave")))
|
|
||||||
.catch((e) => toast(e.message));
|
|
||||||
}
|
}
|
||||||
function scrCtx(e, li, ji) {
|
function scrCtx(e, li, ji) {
|
||||||
if (e.button === 2) {
|
if (e.button === 2) {
|
||||||
@ -4328,9 +4343,7 @@ const cv = sortCanvasEl();
|
|||||||
meta: it.meta ? { ...it.meta } : null,
|
meta: it.meta ? { ...it.meta } : null,
|
||||||
});
|
});
|
||||||
paintSort([li]);
|
paintSort([li]);
|
||||||
persistAuto()
|
afterChainEdit();
|
||||||
.then(() => toast(t("sortHintSave")))
|
|
||||||
.catch((e) => toast(e.message));
|
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
];
|
];
|
||||||
@ -4341,9 +4354,7 @@ const cv = sortCanvasEl();
|
|||||||
fn: () => {
|
fn: () => {
|
||||||
it.meta = null;
|
it.meta = null;
|
||||||
paintSort([li]);
|
paintSort([li]);
|
||||||
persistAuto()
|
afterChainEdit();
|
||||||
.then(() => toast(t("sortHintSave")))
|
|
||||||
.catch((e) => toast(e.message));
|
|
||||||
},
|
},
|
||||||
});
|
});
|
||||||
items.push({
|
items.push({
|
||||||
@ -4359,9 +4370,7 @@ const cv = sortCanvasEl();
|
|||||||
if (!sortState.lanes[li].models.length && sortState.lanes.length > 1)
|
if (!sortState.lanes[li].models.length && sortState.lanes.length > 1)
|
||||||
sortState.lanes.splice(li, 1);
|
sortState.lanes.splice(li, 1);
|
||||||
paintSort([li]);
|
paintSort([li]);
|
||||||
persistAuto()
|
afterChainEdit();
|
||||||
.then(() => toast(t("sortHintSave")))
|
|
||||||
.catch((e) => toast(e.message));
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/* ---------- adapters tab ---------- */
|
/* ---------- adapters tab ---------- */
|
||||||
@ -5213,8 +5222,16 @@ const cv = sortCanvasEl();
|
|||||||
// /api/keys/{key}/auto instead of /api/auto. A separate reduced editor
|
// /api/keys/{key}/auto instead of /api/auto. A separate reduced editor
|
||||||
// would drift from the global one and lose features.
|
// would drift from the global one and lose features.
|
||||||
async function openKeyAutoModal(key, name) {
|
async function openKeyAutoModal(key, name) {
|
||||||
|
// This dialog deliberately does NOT reuse #modal-wrap. That id is
|
||||||
|
// shared by every dialog in the app and more than one can be open at
|
||||||
|
// once, so a lookup by id resolves to whichever comes first in the
|
||||||
|
// document — not the one the user is looking at. That is fine for
|
||||||
|
// handlers that can use `.closest()` from their own button, but this
|
||||||
|
// dialog also has to be found from the CANVAS side (sortCanvasEl,
|
||||||
|
// keyAutoClose, keyAutoInherit), so it needs an identity of its own.
|
||||||
|
document.getElementById("key-auto-modal")?.remove();
|
||||||
const wrap = document.createElement("div");
|
const wrap = document.createElement("div");
|
||||||
wrap.id = "modal-wrap";
|
wrap.id = "key-auto-modal";
|
||||||
wrap.style.cssText =
|
wrap.style.cssText =
|
||||||
"position:fixed;inset:0;background:rgba(15,22,44,.45);display:flex;align-items:flex-start;justify-content:center;overflow:auto;padding:48px 20px;z-index:50";
|
"position:fixed;inset:0;background:rgba(15,22,44,.45);display:flex;align-items:flex-start;justify-content:center;overflow:auto;padding:48px 20px;z-index:50";
|
||||||
wrap.innerHTML = `<div class="card" style="width:900px;max-width:100%">
|
wrap.innerHTML = `<div class="card" style="width:900px;max-width:100%">
|
||||||
@ -5236,25 +5253,45 @@ const cv = sortCanvasEl();
|
|||||||
const cur = await api("/api/keys/" + encodeURIComponent(key) + "/auto");
|
const cur = await api("/api/keys/" + encodeURIComponent(key) + "/auto");
|
||||||
keyAutoState.key = key;
|
keyAutoState.key = key;
|
||||||
keyAutoState.inherits = !!cur.inherits;
|
keyAutoState.inherits = !!cur.inherits;
|
||||||
// A key with no own chain shows an empty canvas rather than the
|
// A key with no own chain shows an EMPTY canvas, never the discovery
|
||||||
// discovery fallback: filling it with every available slot would
|
// fallback buildLanes uses for an unconfigured global chain: that
|
||||||
// suggest those slots are already this user's chain.
|
// fallback fills every source's models (226 blocks on a real
|
||||||
|
// deployment), and showing them inside "this key's chain" dialog
|
||||||
|
// says they already belong to this user. They do not.
|
||||||
sortState.scope = key;
|
sortState.scope = key;
|
||||||
sortState.kind = "chat";
|
sortState.kind = "chat";
|
||||||
sortState.lanes = buildLanes(idx, cur.auto || [], "chat");
|
sortState.lanes =
|
||||||
|
cur.auto && cur.auto.length ? buildLanes(idx, cur.auto, "chat") : [];
|
||||||
sortState.origin = JSON.stringify(sortState.lanes);
|
sortState.origin = JSON.stringify(sortState.lanes);
|
||||||
await loadModelPairs();
|
await loadModelPairs();
|
||||||
renderSortEditor($("#ka-canvas-host"), { canvasId: "key-auto-canvas" });
|
renderSortEditor($("#ka-canvas-host"), { canvasId: "key-auto-canvas" });
|
||||||
|
// The notice is the only thing telling the user what an empty
|
||||||
|
// canvas means here. Without it the dialog is just a blank box and
|
||||||
|
// "Use global chain" looks like a button that already did
|
||||||
|
// something. It disappears as soon as the chain has slots of its
|
||||||
|
// own, which keyAutoSave's reload then confirms.
|
||||||
if (keyAutoState.inherits) keyAutoInheritNotice();
|
if (keyAutoState.inherits) keyAutoInheritNotice();
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
toast(String(e));
|
toast(String(e));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
const keyAutoState = { key: null, inherits: true };
|
const keyAutoState = { key: null, inherits: true };
|
||||||
function keyAutoInheritNotice() {
|
// The notice describes what an EMPTY canvas means, so it must track the
|
||||||
|
// canvas rather than the state the dialog opened with: adding the first
|
||||||
|
// slot makes "this key uses the global chain" false, and leaving it up
|
||||||
|
// reads as a contradiction right next to the slot the user just added.
|
||||||
|
function keyAutoInheritNotice(showing) {
|
||||||
const host = $("#ka-canvas-host");
|
const host = $("#ka-canvas-host");
|
||||||
if (!host) return;
|
if (!host) return;
|
||||||
let n = host.querySelector(".ka-inherit-note");
|
let n = host.querySelector(".ka-inherit-note");
|
||||||
|
const hasSlots = (sortState.lanes || []).some(
|
||||||
|
(l) => (l.models || []).length,
|
||||||
|
);
|
||||||
|
const want = showing === undefined ? !hasSlots : showing && !hasSlots;
|
||||||
|
if (!want) {
|
||||||
|
if (n) n.remove();
|
||||||
|
return;
|
||||||
|
}
|
||||||
if (!n) {
|
if (!n) {
|
||||||
n = document.createElement("div");
|
n = document.createElement("div");
|
||||||
n.className = "muted ka-inherit-note";
|
n.className = "muted ka-inherit-note";
|
||||||
@ -5278,7 +5315,7 @@ const cv = sortCanvasEl();
|
|||||||
keyAutoInheritNotice();
|
keyAutoInheritNotice();
|
||||||
}
|
}
|
||||||
function keyAutoClose() {
|
function keyAutoClose() {
|
||||||
const m = document.getElementById("modal-wrap");
|
const m = document.getElementById("key-auto-modal");
|
||||||
if (m) m.remove();
|
if (m) m.remove();
|
||||||
// Leaving scope set would make the global editor save to this key.
|
// Leaving scope set would make the global editor save to this key.
|
||||||
sortState.scope = null;
|
sortState.scope = null;
|
||||||
|
|||||||
@ -204,3 +204,65 @@ func readFuncBody(t *testing.T, src, name string) string {
|
|||||||
t.Fatalf("function %s body is unterminated", name)
|
t.Fatalf("function %s body is unterminated", name)
|
||||||
return ""
|
return ""
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// The per-key dialog must not share the app-wide #modal-wrap id.
|
||||||
|
//
|
||||||
|
// Every dialog in this file uses that id, and more than one can be open at
|
||||||
|
// once (the "add slot" picker opens ON TOP of this one). Handlers resolve
|
||||||
|
// their own dialog via `.closest()` from their button, which works — but this
|
||||||
|
// dialog is also looked up from the CANVAS side (sortCanvasEl, keyAutoClose,
|
||||||
|
// keyAutoInherit), where there is no button to walk up from.
|
||||||
|
// getElementById("#modal-wrap") then returns whichever dialog comes first in
|
||||||
|
// the document, i.e. not the one being edited: Cancel closed the wrong box and
|
||||||
|
// the canvas could resolve to the picker's.
|
||||||
|
func TestUIKeyAutoModalHasOwnID(t *testing.T) {
|
||||||
|
src := uiSource(t)
|
||||||
|
body := readFuncBody(t, src, "openKeyAutoModal")
|
||||||
|
if !strings.Contains(body, `wrap.id = "key-auto-modal"`) {
|
||||||
|
t.Fatal("openKeyAutoModal must give its dialog its own id, not the shared " +
|
||||||
|
"#modal-wrap (another dialog may be open on top of it)")
|
||||||
|
}
|
||||||
|
close := readFuncBody(t, src, "keyAutoClose")
|
||||||
|
if strings.Contains(close, `"modal-wrap"`) {
|
||||||
|
t.Fatal("keyAutoClose resolves the shared #modal-wrap; it would close " +
|
||||||
|
"whichever dialog comes first in the document")
|
||||||
|
}
|
||||||
|
if !strings.Contains(close, `"key-auto-modal"`) {
|
||||||
|
t.Fatal("keyAutoClose must resolve the per-key dialog by its own id")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Editing a chain inside the per-key dialog must NOT persist it. That dialog
|
||||||
|
// has Save and Cancel, so auto-writing on add/drag/delete meant the change was
|
||||||
|
// already committed before Save and Cancel could not undo it.
|
||||||
|
//
|
||||||
|
// The global page is the opposite: it has always applied edits immediately.
|
||||||
|
func TestUIKeyAutoEditsDoNotAutoPersist(t *testing.T) {
|
||||||
|
src := uiSource(t)
|
||||||
|
body := readFuncBody(t, src, "afterChainEdit")
|
||||||
|
if !strings.Contains(body, "if (sortState.scope)") {
|
||||||
|
t.Fatal("afterChainEdit must branch on sortState.scope")
|
||||||
|
}
|
||||||
|
// The scoped branch must return before reaching persistAuto.
|
||||||
|
iScope := strings.Index(body, "if (sortState.scope)")
|
||||||
|
iReturn := strings.Index(body[iScope:], "return")
|
||||||
|
iPersist := strings.Index(body, "persistAuto()")
|
||||||
|
if iScope < 0 || iReturn < 0 || iPersist < 0 {
|
||||||
|
t.Fatal("afterChainEdit no longer has the expected shape")
|
||||||
|
}
|
||||||
|
if iScope+iReturn > iPersist {
|
||||||
|
t.Fatal("the scoped branch reaches persistAuto() — a per-key edit would " +
|
||||||
|
"be written before Save")
|
||||||
|
}
|
||||||
|
// And it must not call itself (a regex-driven rewrite did exactly that).
|
||||||
|
if strings.Contains(body, "afterChainEdit()") {
|
||||||
|
t.Fatal("afterChainEdit calls itself: infinite recursion")
|
||||||
|
}
|
||||||
|
// Every in-canvas edit goes through it, so none of them bypass the check.
|
||||||
|
for _, fn := range []string{"scrAddFromForm", "sortScopeSave", "scrDelSlot"} {
|
||||||
|
b := readFuncBody(t, src, fn)
|
||||||
|
if strings.Contains(b, "persistAuto()") {
|
||||||
|
t.Fatalf("%s calls persistAuto() directly, bypassing afterChainEdit", fn)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user