From 05ade5db82c7406df23de7ec66764a4260f378c2 Mon Sep 17 00:00:00 2001 From: digital-garbage <133010426+digital-garbage@users.noreply.github.com> Date: Tue, 6 Oct 2026 09:43:41 +0300 Subject: [PATCH] fix: typed node values go live at once; number arrows commit; closing a window or starting Generate commits the focused field; Composer button toggles; padded popovers Co-Authored-By: Claude Opus 5.5 --- app/ui/composer/elements/input.js | 7 +++++-- app/ui/composer/elements/overlay.css | 2 ++ app/ui/composer/internals/portal.js | 3 +++ app/ui/composer/tests/elements_behaviour.test.js | 14 +++++++++++++- app/ui/composer/tests/overlays.test.js | 10 ++++++++++ app/ui/features/composer/index.js | 2 +- app/ui/features/generate/index.js | 2 ++ app/ui/shell/pipeline_state.js | 10 ++++++++++ app/ui/shell/tests/pipeline_state.test.js | 16 ++++++++++++++++ 9 files changed, 62 insertions(+), 4 deletions(-) diff --git a/app/ui/composer/elements/input.js b/app/ui/composer/elements/input.js index e08394a7..61879ff0 100644 --- a/app/ui/composer/elements/input.js +++ b/app/ui/composer/elements/input.js @@ -77,11 +77,14 @@ define("number", "md", ({ value = 0, min, max, step = 1, precision, unit, onChan const commit = () => { const raw = input.value.trim(); const parsed = raw === "" ? NaN : Number(raw); - const next = Number.isFinite(parsed) ? clamp(parsed) : committed; + const next = Number.isFinite(parsed) ? clamp(parsed) : committed, changed = next !== committed; committed = next; input.value = String(next); - if (onChange) onChange(next); + if (changed && onChange) onChange(next); }; + // "change" is the browser's commit: each spinner-arrow click, the wheel, and leaving a typed edit. Blur alone missed + // the arrows, so a redraw put the old value back. Typing does not fire it, so mid-typing values stay typeable. + input.addEventListener("change", commit); input.addEventListener("blur", commit); input.addEventListener("keydown", (e) => { if (e.key === "Enter") { e.preventDefault(); commit(); } }); diff --git a/app/ui/composer/elements/overlay.css b/app/ui/composer/elements/overlay.css index bfae2b00..f85348e3 100644 --- a/app/ui/composer/elements/overlay.css +++ b/app/ui/composer/elements/overlay.css @@ -91,6 +91,8 @@ animation: cx-pop var(--dur-1) var(--ease-standard); } .cx-popover-menu { min-width: 180px; } +/* a popover holding a panel (settings, fields), not a list of items, needs room to the sides */ +.cx-popover[role="dialog"] { padding: var(--sp-5) var(--sp-6); } .cx-menu { display: flex; flex-direction: column; gap: 1px; } .cx-menu-item { diff --git a/app/ui/composer/internals/portal.js b/app/ui/composer/internals/portal.js index ada91d3a..c13063e5 100644 --- a/app/ui/composer/internals/portal.js +++ b/app/ui/composer/internals/portal.js @@ -44,6 +44,9 @@ export function mount(node) { export function unmount(node) { // Both halves matter: with no root yet, an unparented node's parentNode is // also null, and `null === null` would send removeChild to nothing. + // A field still focused would vanish without its blur, and a value typed there with it: let it commit first. + const active = typeof document !== "undefined" && document.activeElement; + if (node && active && active !== document.body && node.contains(active) && typeof active.blur === "function") active.blur(); if (root && node && node.parentNode === root) root.removeChild(node); } diff --git a/app/ui/composer/tests/elements_behaviour.test.js b/app/ui/composer/tests/elements_behaviour.test.js index 52cb493d..8d149cb5 100644 --- a/app/ui/composer/tests/elements_behaviour.test.js +++ b/app/ui/composer/tests/elements_behaviour.test.js @@ -340,7 +340,19 @@ test("clearing a number field keeps the last committed value", () => { fire(input, "blur"); assert.equal(n.value, 7, "an emptied field is not a zero"); assert.equal(input.value, "7", "and the field shows what is actually held"); - assert.deepEqual(seen, [7, 7]); + assert.deepEqual(seen, [7], "nothing changed, so nothing is saved again"); +}); + +test("a spinner-arrow click commits at once; leaving afterwards does not save it twice", () => { + // the arrows fire "change" and no blur: committed on blur alone, a redraw put the old value back + const seen = []; + const n = mount(composer.number.md({ value: 3, min: 1, max: 10, onChange: (v) => seen.push(v) })); + const input = n.node.querySelector("input") || n.node; + input.value = "4"; + fire(input, "change"); + assert.deepEqual(seen, [4]); + fire(input, "blur"); + assert.deepEqual(seen, [4]); }); test("a number field ignores unparseable text on commit", () => { diff --git a/app/ui/composer/tests/overlays.test.js b/app/ui/composer/tests/overlays.test.js index f5da4540..8e3c0df4 100644 --- a/app/ui/composer/tests/overlays.test.js +++ b/app/ui/composer/tests/overlays.test.js @@ -636,3 +636,13 @@ test("a popover whose content grows after opening is placed again for its new si pop.close(); anchor.remove(); } finally { delete globalThis.ResizeObserver; } }); + +test("closing a window while a field in it is focused still commits what was typed", () => { + let got = null; + const field = composer.input.md({ label: "Taste key", onCommit: (v) => { got = v; } }); + const win = composer.floating.window({ id: "commit-on-close", title: "T", body: composer.region.stack({ children: [field] }) }); + field.node.focus(); + field.node.value = "portraits"; + win.close(); + assert.equal(got, "portraits"); +}); diff --git a/app/ui/features/composer/index.js b/app/ui/features/composer/index.js index 2dd6ffa0..fc3ea607 100644 --- a/app/ui/features/composer/index.js +++ b/app/ui/features/composer/index.js @@ -71,7 +71,7 @@ export default { win = c.floating.window({ id: "composer", title: "Composer", subtitle: "prompt craft", body, width: 420, height: 520, x: 320, y: 90, onClose: () => { cleanup(); win = null; } }); }; - host.append(c.button.sm({ label: "◆ Composer", tone: "neutral", onClick: open }).node); + host.append(c.button.sm({ label: "◆ Composer", tone: "neutral", onClick: () => (win ? win.close() : open()) }).node); // the button toggles it, as v4 app.inputHooks.push(chatHook); // last, so a setup that failed above leaves nothing behind return () => { cleanup(); app.inputHooks.splice(app.inputHooks.indexOf(chatHook) >>> 0, 1); }; }, diff --git a/app/ui/features/generate/index.js b/app/ui/features/generate/index.js index 2d453714..e69d55ca 100644 --- a/app/ui/features/generate/index.js +++ b/app/ui/features/generate/index.js @@ -57,6 +57,8 @@ export default { let made = 0; said = false; try { + const typing = document.activeElement; // a field still being typed in commits first, however Generate was started + if (typing && typing !== document.body && typeof typing.blur === "function") typing.blur(); await Promise.all((app.beforeRun || []).map((f) => f().catch(() => {}))); // an edit still on its way (the Story box) lands first // A project's pipeline still going in is the one this run uses, not the last one's. if (app.pipeline.settled && !(await app.pipeline.settled())) { tell("Not started: this project's pipeline is still loading. ComfyUI is slow to answer; try again in a moment."); return; } diff --git a/app/ui/shell/pipeline_state.js b/app/ui/shell/pipeline_state.js index 076bd8f5..c5631da7 100644 --- a/app/ui/shell/pipeline_state.js +++ b/app/ui/shell/pipeline_state.js @@ -209,9 +209,18 @@ export function createPipelineState(rawApi, { answerMs = ANSWER_MS } = {}) { let editGuard = () => null; const blocked = (kind = "edit") => { const why = editGuard(kind); if (why) { saveNotes = [why]; _changed("load"); } return why; }; // said, and views redraw to what is true + // A typed node value is live at once: a run reads these slots now, and the server's answer only confirms it. + function placeInputs(inputs) { + if (!inputs || !slots) return; + Object.entries(inputs).forEach(([id, kv]) => { + const slot = slots.find((x) => x.id === id); + if (slot && kv && typeof kv === "object" && !Array.isArray(kv)) slot.inputs = { ...(slot.inputs || {}), ...kv }; + }); + } async function save(body) { if (adoptGate) await adoptGate; // an edit made while a project's pipeline goes in lands on THAT pipeline if (blocked()) return; + placeInputs(body && body.inputs); pendingBody = mergeBodies(pendingBody, body); if (saving) { pending = true; return; } saving = true; @@ -227,6 +236,7 @@ export function createPipelineState(rawApi, { answerMs = ANSWER_MS } = {}) { // this answer is about the old one and must not overwrite the new. if (mine !== epoch) continue; if (res && res.slots) slots = res.slots; + placeInputs(pendingBody && pendingBody.inputs); // typed while this was in flight: still what is on screen if (Object.keys(pendingValues).length) mirrorValues(currentValues()); // an edit queued behind this one stays in the slots a run reads applyGroups(); incomplete = (res && res.incomplete) || []; diff --git a/app/ui/shell/tests/pipeline_state.test.js b/app/ui/shell/tests/pipeline_state.test.js index e286ba01..6cc75bf8 100644 --- a/app/ui/shell/tests/pipeline_state.test.js +++ b/app/ui/shell/tests/pipeline_state.test.js @@ -629,3 +629,19 @@ test("Revert to what is already live changes nothing and is not a change of shap assert.deepStrictEqual((await PS.restore(PS.snapshot())).refused, []); assert.deepStrictEqual([posts.length, heard], [0, []]); }); + +test("a typed node value is in the live slots at once, not when the server answers; one typed during the round trip survives it", async () => { + const posts = []; + const PS = load(posts, { delay: 30 }); + await PS.ensureLoaded(); + const steps = () => PS.slots().find((s) => s.id === "gen").inputs.steps; + const first = PS.save({ inputs: { gen: { steps: 12 } } }); + assert.strictEqual(steps(), 12, "Generate now reads what was typed"); + await new Promise((r) => setTimeout(r, 5)); + const second = PS.save({ inputs: { gen: { steps: 20 } } }); // typed while the first is on its way + assert.strictEqual(steps(), 20); + await new Promise((r) => setTimeout(r, 32)); // the first answer lands, carrying 12 + assert.strictEqual(steps(), 20, "the older answer does not put the old value back on screen"); + await Promise.all([first, second]); + assert.strictEqual(steps(), 20); +});