app: mount every Settings section in place instead of deep-linking out

Picking Node packs, Updates, the log or Temp files used to close the
Settings window and open that feature's own separate modal, with no way
back except reopening Settings from the menu bar -- not what v4 does,
and not what "unified" is supposed to mean. Each of those five files
(plus pipeline_window.js) now exports mount({ setFooter, close }), the
same content its standalone open() already built, taking the modal
chrome's footer and close from whoever hosts it instead of owning a
modal directly. open() is unchanged for anyone opening one alone --
pipeline_window.test.js and every Playwright spec for these five prove
it, unedited, still passing.

Also fixes two findings from the adversarial pass on the earlier
deep-link version: filterList's search only matched a section's visible
label/hint, so a section findable only by a keyword (e.g. "loaders" for
"Models and pipeline") silently vanished from the filtered list; and
createSettingsWindow's "one at a time" state lived at module scope, so
a second independent instance would have redrawn the first one's modal
instead of opening its own. Both now have regression tests.
This commit is contained in:
digital-garbage
2026-09-07 01:06:02 +03:00
parent 99ce537932
commit e6e4dfa882
10 changed files with 332 additions and 162 deletions
+18 -14
View File
@@ -13,7 +13,7 @@ import { createRun, viewUrl, DONE, FAILED, CANCELLED } from "./shell/run.js";
import { clientId, connect, queuedFor, finishedFor } from "./shell/client.js";
import { wire, waitForTerminal } from "./shell/session.js";
import { check, load, describe, search } from "./shell/pipeline.js";
import { open as openPipeline } from "./shell/pipeline_window.js";
import { mount as mountPipeline } from "./shell/pipeline_window.js";
import { createPrompts } from "./shell/prompt.js";
import { createProject } from "./shell/projects.js";
import { createTimeline } from "./shell/timeline.js";
@@ -21,10 +21,10 @@ import { createInspector } from "./shell/inspector.js";
import { createWheel } from "./shell/wheel.js";
import { offerAction } from "./shell/actions.js";
import { open as openWizard } from "./shell/wizard.js";
import { open as openUpdates, status as gitStatus } from "./shell/updates.js";
import { open as openPacks } from "./shell/packs.js";
import { open as openLog } from "./shell/logwindow.js";
import { open as openTemp } from "./shell/tempfiles.js";
import { mount as mountUpdates, status as gitStatus } from "./shell/updates.js";
import { mount as mountPacks } from "./shell/packs.js";
import { mount as mountLog } from "./shell/logwindow.js";
import { mount as mountTemp } from "./shell/tempfiles.js";
import { createSettingsWindow } from "./shell/settings_window.js";
const root = document.querySelector("#app");
@@ -194,16 +194,16 @@ async function start() {
});
// One window for everything that is a PREFERENCE rather than an edit to the
// project on screen. Each of these already has its own real, tested modal --
// a settings section here is a deep link to it, not a rebuild of it.
// project on screen. Each section mounts the same content its standalone
// window would build -- pipeline_window.js etc. -- inside this one instead.
const settings = createSettingsWindow({
gitStatus,
sections: [
{
id: "pipeline", title: "Models and pipeline", subtitle: "What the run is made of.",
keywords: "pipeline models nodes slots loaders", icon: "▦",
action: () => openPipeline({
load, describe, check, search,
mount: (ctx) => mountPipeline({
load, describe, check, search, ...ctx,
onApply: (next) => {
slots = next;
// The boxes on the main window are for inputs of THESE slots. A slot
@@ -224,25 +224,29 @@ async function start() {
keywords: "update git branch restart pull checkout", icon: "⟳",
// The window asks whether a run is in flight, because the restart that
// follows an update would take it with it.
action: () => openUpdates({ running: () => ["queued", "running"].includes(run.state.phase) }),
mount: (ctx) => mountUpdates({
...ctx, running: () => ["queued", "running"].includes(run.state.phase),
}),
},
{
id: "packs", title: "Node packs", subtitle: "Install, update, or remove a custom_nodes pack.",
keywords: "node packs custom_nodes install", icon: "▣",
action: () => openPacks(),
mount: (ctx) => mountPacks(ctx),
},
{
id: "log", title: "ComfyUI log", subtitle: "The server's own log, without opening devtools.",
keywords: "log console errors output", icon: "▤",
action: () => openLog(),
mount: (ctx) => mountLog(ctx),
},
{
id: "temp", title: "Temp files", subtitle: "Where a file went when it did not land in the bin.",
keywords: "temp files output directory", icon: "▥",
// Opening one puts it in the Preview, which is where somebody hunting
// for a file wants it -- the same place a result from the bin goes.
action: () => openTemp({ onOpen: (item) => page.viewer.setSource(
item.url, item.kind, item.file ? { ...item.file, type: "temp" } : null) }),
mount: (ctx) => mountTemp({
...ctx, onOpen: (item) => page.viewer.setSource(
item.url, item.kind, item.file ? { ...item.file, type: "temp" } : null),
}),
},
],
});
+4 -1
View File
@@ -160,8 +160,11 @@ define("filterList", "md", ({ items = [], value, onChange, placeholder = "Search
function draw() {
const q = search.value.trim().toLowerCase();
// `keywords` is searched but never drawn -- a way in that does not name
// itself in the visible label/hint (a section called "About FunPack"
// found by typing "commit", say).
const shown = items.filter((i) =>
!q || `${i.label} ${i.hint || ""}`.toLowerCase().includes(q));
!q || `${i.label} ${i.hint || ""} ${i.keywords || ""}`.toLowerCase().includes(q));
list.replaceChildren();
if (!shown.length) { list.append(el("div", { cls: "cx-filter-empty", text: empty })); return; }
for (const item of shown) {
+22 -13
View File
@@ -13,8 +13,10 @@ import { composer } from "../composer/composer.js";
const EVERY = 2000;
export function open({ poll = EVERY } = {}) {
let window_ = null;
/** mount({ poll, setFooter, close }) -> { node, destroy }. See tempfiles.js's
* mount() for why setFooter/close are the only things asked for -- `destroy`
* additionally stops the poll, since this one runs a timer nothing else does. */
export function mount({ poll = EVERY, setFooter = () => {}, close = () => {} } = {}) {
let timer = null;
let follow = true; // stick to the end until the user scrolls off it
let last = "";
@@ -47,14 +49,12 @@ export function open({ poll = EVERY } = {}) {
view.setText(text);
if (follow) view.node.scrollTop = view.node.scrollHeight;
}
if (window_) window_.setFooter({ note: payload.path || "", actions: footer() });
setFooter({ note: payload.path || "", actions: footer() });
} catch (err) {
// The server being gone is the most interesting thing a log can say, so it
// is said -- and the lines already on screen are kept, because they are
// the ones from just before it went.
if (window_) {
window_.setFooter({ note: `Cannot reach ComfyUI: ${err.message}`, actions: footer() });
}
setFooter({ note: `Cannot reach ComfyUI: ${err.message}`, actions: footer() });
}
}
@@ -66,19 +66,28 @@ export function open({ poll = EVERY } = {}) {
if (navigator.clipboard) navigator.clipboard.writeText(last).catch(() => {});
},
}),
composer.button.md({ label: "Close", tone: "primary",
onClick: () => window_ && window_.close("done") }),
composer.button.md({ label: "Close", tone: "primary", onClick: () => close() }),
];
setFooter({ actions: footer() });
refresh();
timer = setInterval(refresh, poll);
return { node: body.node, destroy: () => { clearInterval(timer); timer = null; body.destroy(); } };
}
export function open({ poll = EVERY } = {}) {
let window_ = null;
const content = mount({
poll,
setFooter: (f) => window_ && window_.setFooter(f),
close: () => window_ && window_.close("done"),
});
window_ = composer.modal.generic({
title: "ComfyUI log",
subtitle: "What the server printed. Newest at the bottom.",
size: "xl",
body,
onClose: () => { clearInterval(timer); timer = null; window_ = null; },
body: content,
onClose: () => { content.destroy(); window_ = null; },
});
window_.setFooter({ actions: footer() });
refresh();
timer = setInterval(refresh, poll);
return window_;
}
+21 -14
View File
@@ -33,8 +33,9 @@ function saySince(pack, checked) {
return `${pack.branch || "?"} · up to date`;
}
export function open() {
let window_ = null;
/** mount({ setFooter, close }) -> a composer handle, no modal chrome. See
* tempfiles.js's mount() for why these two are the only things asked for. */
export function mount({ setFooter = () => {}, close = () => {} } = {}) {
let listing = null;
let checked = null;
let busy = null; // the pack currently being worked on, by name
@@ -121,17 +122,14 @@ export function open() {
function draw() {
body.set(rows());
if (window_) {
window_.setFooter({
note: listing ? listing.root : "",
actions: [
composer.button.md({ label: "Check for updates", disabled: Boolean(busy),
onClick: () => check() }),
composer.button.md({ label: "Close", tone: "primary",
onClick: () => window_.close("done") }),
],
});
}
setFooter({
note: listing ? listing.root : "",
actions: [
composer.button.md({ label: "Check for updates", disabled: Boolean(busy),
onClick: () => check() }),
composer.button.md({ label: "Close", tone: "primary", onClick: () => close() }),
],
});
}
async function load() {
@@ -154,6 +152,16 @@ export function open() {
finally { busy = null; draw(); }
}
load();
return body;
}
export function open() {
let window_ = null;
const body = mount({
setFooter: (f) => window_ && window_.setFooter(f),
close: () => window_ && window_.close("done"),
});
window_ = composer.modal.generic({
title: "Node packs",
subtitle: "Install, update and remove ComfyUI custom nodes.",
@@ -161,6 +169,5 @@ export function open() {
body,
onClose: () => { window_ = null; },
});
load();
return window_;
}
+62 -27
View File
@@ -53,19 +53,20 @@ const countOf = (n) => (n === 0 ? "empty" : `${n} node${n === 1 ? "" : "s"}`);
let current = null;
/**
* open({ load, describe, check, search, onApply }) -> handle
* mount({ load, describe, check, search, onApply, setFooter, close }) -> handle
*
* Every server call is injected. The window is then testable without a server
* and -- more to the point -- cannot reach for one that is not there.
*
* Opening while one is already open returns THAT one and focuses it, rather
* than stacking a second over it.
* `setFooter`/`close` are the two things a modal gives this that a container
* does not have for free -- see tempfiles.js's mount() for the general shape.
* This one additionally exposes its live state (slots/editing/selected/
* pending) and a few structural actions, because `open()` below and the
* jsdom suite both drive it without reading the DOM back; `open()` forwards
* every one of these rather than wrapping a second copy of this state.
*/
export function open({ load, describe, check, search, onApply } = {}) {
if (current) {
current.focus();
return current;
}
export function mount({ load, describe, check, search, onApply,
setFooter = () => {}, close = () => {} } = {}) {
let slots = [];
const nodes = new Map(); // class name -> description | null
const extraGroups = []; // made in this window, no slot in them yet
@@ -75,18 +76,6 @@ export function open({ load, describe, check, search, onApply } = {}) {
let notes = { refused: [], incomplete: [] };
const body = composer.region.stack({ gap: "md", label: "Pipeline", fill: true });
const modal = composer.modal.generic({
title: "Models and pipeline",
subtitle: "What the run is made of.",
size: "xl",
body,
// A half-finished group edit must not vanish because a click landed on the
// backdrop: the draft is the only copy of it.
closeOnOutside: false,
// However it was closed -- the button, Escape, destroy -- the next press
// opens a fresh one rather than finding this one and refusing.
onClose: () => { if (current === handle) current = null; },
});
// ---------------------------------------------------------------- server
@@ -177,11 +166,11 @@ export function open({ load, describe, check, search, onApply } = {}) {
if (editing === null) {
body.set(index());
// Nothing to save from the index: the cards are a way in, not an edit.
modal.setFooter({});
setFooter({});
return;
}
body.set(group(editing));
modal.setFooter(footer());
setFooter(footer());
}
/**
@@ -442,7 +431,7 @@ export function open({ load, describe, check, search, onApply } = {}) {
// that fired this and take the cursor out of it mid-edit; leaving the
// footer alone left it reading "No changes to save" over an edit that had
// just been made.
modal.setFooter(footer());
setFooter(footer());
}
async function saveDraft() {
@@ -556,10 +545,9 @@ export function open({ load, describe, check, search, onApply } = {}) {
})]);
});
const handle = {
node: modal.node,
close: (reason) => modal.close(reason),
focus: () => modal.node.focus ? modal.node.focus() : undefined,
return {
node: body.node,
destroy: () => body.destroy(),
ready,
// The window's own view of things, so a test does not have to infer state
// by reading the DOM back.
@@ -574,6 +562,53 @@ export function open({ load, describe, check, search, onApply } = {}) {
_moveTo: moveTo,
_remove: remove,
};
}
/**
* open({ load, describe, check, search, onApply }) -> handle
*
* Opening while one is already open returns THAT one and focuses it, rather
* than stacking a second over it.
*/
export function open({ load, describe, check, search, onApply } = {}) {
if (current) {
current.focus();
return current;
}
let window_ = null;
const content = mount({
load, describe, check, search, onApply,
setFooter: (f) => window_ && window_.setFooter(f),
close: () => window_ && window_.close("done"),
});
window_ = composer.modal.generic({
title: "Models and pipeline",
subtitle: "What the run is made of.",
size: "xl",
body: content,
// A half-finished group edit must not vanish because a click landed on the
// backdrop: the draft is the only copy of it.
closeOnOutside: false,
// However it was closed -- the button, Escape, destroy -- the next press
// opens a fresh one rather than finding this one and refusing.
onClose: () => { if (current === handle) current = null; },
});
const handle = {
node: window_.node,
close: (reason) => window_.close(reason),
focus: () => window_.node.focus ? window_.node.focus() : undefined,
ready: content.ready,
get slots() { return content.slots; },
get editing() { return content.editing; },
get selected() { return content.selected; },
get pending() { return content.pending; },
enter: content.enter,
leave: content.leave,
_replace: content._replace,
_moveTo: content._moveTo,
_remove: content._remove,
};
current = handle;
return handle;
}
+29 -25
View File
@@ -3,34 +3,40 @@
// menu items (Models and pipeline, Updates, Node packs, Log, Temp files) that
// nothing tied together and nothing let you search.
//
// A section is either MOUNTED here (About, today) or a DEEP LINK: picking it
// closes this window and opens the real one. Every deep-linked window here
// (pipeline_window, updates, packs, log, tempfiles) already owns real,
// tested modal chrome of its own -- rebuilding each into a mountable pane is
// real surgery on working code for a visual nicety, not a fix for anything
// broken. This gets the one thing that mattered (one findable, searchable
// place to start) without touching any of that.
// Every section is MOUNTED here, in place, beside the same always-visible
// nav list -- picking "Node packs" does not leave this window any more than
// clicking a different System Settings pane leaves System Settings. Each of
// pipeline_window.js/updates.js/packs.js/logwindow.js/tempfiles.js already
// had its content built separately from its own modal chrome (a `body`
// region.stack, footer buttons set on the modal handle); their `mount()`
// exports are that same content, taking `setFooter`/`close` from whoever is
// hosting them instead of owning a modal directly. Their `open()` exports
// are unchanged for anyone opening one standalone.
import { composer } from "../composer/composer.js";
let current = null;
/**
* createSettingsWindow({ sections, gitStatus }) -> { open, close }
*
* `sections`: [{ id, title, subtitle, keywords, icon,
* mount() -> handle } -- OR -- { action() }].
* A section with `action` is a deep link: picking it closes this window and
* calls `action`. A section with `mount` is drawn in place; `mount()` returns
* a composer element handle (its own `destroy()` is the cleanup, called when
* another section is picked or the window closes).
* mount({ setFooter, close }) -> handle }].
* `mount()` returns a composer element handle (its own `destroy()` is the
* cleanup, called when another section is picked or the window closes).
* `setFooter`/`close` act on this window's own modal chrome -- a section
* does not know or care whether it is standing alone or hosted here.
*
* `gitStatus` is injected, not imported, for the same reason every other
* shell window takes its network calls as props: testable without a server,
* and unable to reach for one that was not handed to it.
*
* `current` lives HERE, per call, not at module scope: a module-level
* singleton would let a second independent settings window (there is only
* ever one in this app today, but nothing enforced that) silently redraw
* the FIRST one's modal instead of opening its own.
*/
export function createSettingsWindow({ sections = [], gitStatus } = {}) {
const all = [aboutSection(gitStatus), ...sections];
let current = null;
function open(id) {
if (current) { current.show(id); return current; }
@@ -42,15 +48,13 @@ export function createSettingsWindow({ sections = [], gitStatus } = {}) {
const spec = all.find((s) => s.id === pick) || all[0];
if (!spec) return;
if (activeCleanup) { activeCleanup.destroy?.(); activeCleanup = null; }
if (spec.action) {
// Deep link: this window is not what is being asked for, only how
// it was found. Close first -- two full-size modals stacked is not
// "unified", it is one more window in front of the one just opened.
close();
spec.action();
return;
}
const content = spec.mount();
// Cleared before mounting: a section that sets no footer of its own
// (About) must not go on showing the PREVIOUS section's buttons.
modal.setFooter({});
const content = spec.mount({
setFooter: (f) => modal.setFooter(f),
close: () => modal.close("done"),
});
activeCleanup = content;
contentStack.set([
composer.header.md({ text: spec.title }),
@@ -61,7 +65,7 @@ export function createSettingsWindow({ sections = [], gitStatus } = {}) {
}
const nav = composer.filterList.md({
items: all.map((s) => ({ id: s.id, label: s.title, hint: s.subtitle, icon: s.icon })),
items: all.map((s) => ({ id: s.id, label: s.title, hint: s.subtitle, icon: s.icon, keywords: s.keywords })),
value: id,
onChange: showSection,
placeholder: "Search settings",
@@ -69,7 +73,7 @@ export function createSettingsWindow({ sections = [], gitStatus } = {}) {
const body = composer.splitPane.h({ panes: [nav, contentStack], size: 25, min: 15, label: "Settings" });
const modal = composer.modal.generic({
title: "Settings", size: "lg", body,
title: "Settings", size: "xl", body,
onClose: () => {
if (activeCleanup) { activeCleanup.destroy?.(); activeCleanup = null; }
if (current === handle) current = null;
+31 -14
View File
@@ -23,8 +23,18 @@ function size(bytes) {
const when = (mtime) => new Date((Number(mtime) || 0) * 1000).toLocaleTimeString();
export function open({ onOpen } = {}) {
let window_ = null;
/**
* mount({ onOpen, setFooter, close }) -> a composer handle, no modal chrome.
*
* `setFooter`/`close` are the two things a modal gives this window that a
* container does not have for free: somewhere for Refresh/Close to live
* outside the scrolling body, and a way to leave when a file is picked.
* `open()` below is this plus a modal wired to itself; the unified Settings
* window mounts this directly, wired to ITS OWN footer and close instead --
* so picking a file from Settings closes Settings, and standalone Temp
* files behaves exactly as it always did.
*/
export function mount({ onOpen, setFooter = () => {}, close = () => {} } = {}) {
const body = composer.region.stack({ gap: "sm", fill: true, label: "Temp files" });
const cells = (files) => files.map((f) => ({
@@ -60,7 +70,7 @@ export function open({ onOpen } = {}) {
const file = files.find((f) => `${f.subfolder}/${f.filename}` === cell.id);
if (!file || !onOpen) return;
onOpen({ url: viewUrl({ ...file, type: "temp" }), kind: file.kind, file });
if (window_) window_.close("opened");
close();
},
})
// The reason there is nothing, which is not the same as "nothing was made".
@@ -69,18 +79,26 @@ export function open({ onOpen } = {}) {
hint: payload.detail || "",
})]);
if (window_) {
window_.setFooter({
note: payload.path || "",
actions: [
composer.button.md({ label: "Refresh", onClick: () => load() }),
composer.button.md({ label: "Close", tone: "primary",
onClick: () => window_.close("done") }),
],
});
}
setFooter({
note: payload.path || "",
actions: [
composer.button.md({ label: "Refresh", onClick: () => load() }),
composer.button.md({ label: "Close", tone: "primary", onClick: () => close() }),
],
});
}
load();
return body;
}
export function open({ onOpen } = {}) {
let window_ = null;
const body = mount({
onOpen,
setFooter: (f) => window_ && window_.setFooter(f),
close: () => window_ && window_.close("done"),
});
window_ = composer.modal.generic({
title: "Temp files",
subtitle: "Transient ComfyUI outputs. Wiped when the server restarts.",
@@ -88,6 +106,5 @@ export function open({ onOpen } = {}) {
body,
onClose: () => { window_ = null; },
});
load();
return window_;
}
+67 -19
View File
@@ -1,6 +1,5 @@
// The unified Settings window: one modal, a searchable list of sections, and
// two kinds of section -- mounted in place (About), or a deep link that
// closes this window and opens the real one.
// The unified Settings window: one modal, a searchable nav list, and every
// section mounted in place beside it -- picking one never leaves the window.
import test from "node:test";
import assert from "node:assert/strict";
@@ -15,15 +14,17 @@ test.before(async () => {
});
test.after(() => teardownDom());
function fakeSection(id, title) {
function fakeSection(id, title, { keywords } = {}) {
const destroyed = [];
return {
id, title, subtitle: `${title} subtitle`, keywords: title,
id, title, subtitle: `${title} subtitle`, keywords: keywords ?? "",
calls: destroyed,
mount() {
const stack = { node: document.createElement("div"), destroy: () => destroyed.push("destroyed") };
stack.node.textContent = `${title} content`;
return stack;
lastCtx: null,
mount(ctx) {
this.lastCtx = ctx;
const node = document.createElement("div");
node.textContent = `${title} content`;
return { node, destroy: () => destroyed.push("destroyed") };
},
};
}
@@ -37,26 +38,40 @@ test("opening draws the first section by default", () => {
w.close();
});
test("picking a section in the nav swaps the content and tears down the last one", () => {
test("picking a section in the nav swaps the content and tears down the last one, without closing the window", () => {
const alpha = fakeSection("a", "Alpha");
const w = createSettingsWindow({ sections: [alpha] });
w.open("a");
assert.match(document.querySelector(".cx-modal").textContent, /Alpha content/);
document.querySelector('.cx-filter-row[aria-selected="false"]').click(); // About
assert.equal(document.querySelectorAll(".cx-modal").length, 1, "picking a section opened or closed a modal");
assert.match(document.querySelector(".cx-modal").textContent, /About FunPack/);
assert.deepEqual(alpha.calls, ["destroyed"]);
w.close();
});
test("a deep-link section closes this window and calls its own action instead of drawing anything", () => {
let called = 0;
const w = createSettingsWindow({
sections: [{ id: "packs", title: "Node packs", keywords: "packs", action: () => { called += 1; } }],
});
w.open("packs");
assert.equal(called, 1);
assert.equal(document.querySelector(".cx-modal"), null, "the settings window is still open over the real one");
test("a mounted section gets a setFooter and a close, wired to this window's own modal", () => {
const alpha = fakeSection("a", "Alpha");
const w = createSettingsWindow({ sections: [alpha] });
w.open("a");
assert.equal(typeof alpha.lastCtx.setFooter, "function");
alpha.lastCtx.setFooter({ note: "hello" });
assert.match(document.querySelector(".cx-modal-foot").textContent, /hello/);
alpha.lastCtx.close();
assert.equal(document.querySelector(".cx-modal"), null, "the section's own close did not close the window");
});
test("switching sections clears the previous one's footer instead of leaving it showing", () => {
const alpha = fakeSection("a", "Alpha");
const beta = fakeSection("b", "Beta");
const w = createSettingsWindow({ sections: [alpha, beta] });
w.open("a");
alpha.lastCtx.setFooter({ note: "alpha's own note" });
w.open("b");
assert.doesNotMatch(document.querySelector(".cx-modal-foot").textContent, /alpha's own note/);
w.close();
});
test("opening again while already open jumps to the requested section instead of stacking a second modal", () => {
@@ -68,7 +83,7 @@ test("opening again while already open jumps to the requested section instead of
w.close();
});
test("the search box narrows the list to matching sections", () => {
test("the search box narrows the list to sections matching the title", () => {
const w = createSettingsWindow({ sections: [fakeSection("a", "Alpha"), fakeSection("b", "Beta")] });
w.open();
const search = document.querySelector(".cx-search");
@@ -79,6 +94,23 @@ test("the search box narrows the list to matching sections", () => {
w.close();
});
test("the search box also matches a section's keywords, not only its visible title and subtitle", () => {
// The bug this guards: an earlier version only searched `label` + `hint`,
// so a section findable by a word that appears ONLY in its keywords list
// (e.g. "loaders" for a section titled "Models and pipeline") silently
// vanished from the filtered list instead of matching.
const w = createSettingsWindow({
sections: [fakeSection("a", "Alpha", { keywords: "xyzzy" }), fakeSection("b", "Beta")],
});
w.open();
const search = document.querySelector(".cx-search");
search.value = "xyzzy";
search.dispatchEvent(new window.Event("input", { bubbles: true }));
const labels = [...document.querySelectorAll(".cx-filter-label")].map((n) => n.textContent);
assert.deepEqual(labels, ["Alpha"]);
w.close();
});
test("a gitStatus rejection is shown as a hint rather than left blank or thrown", async () => {
const w = createSettingsWindow({ sections: [], gitStatus: () => Promise.reject(new Error("offline")) });
w.open("about");
@@ -95,3 +127,19 @@ test("closing tears down the active section and a later open starts fresh", () =
assert.equal(document.querySelector(".cx-modal"), null);
assert.deepEqual(alpha.calls, ["destroyed"]);
});
test("two independent windows do not share which section is open", () => {
// The bug this guards: `current` used to live at module scope, so a second
// createSettingsWindow() instance's open() would redraw the FIRST
// instance's already-open modal instead of opening its own.
const w1 = createSettingsWindow({ sections: [fakeSection("a", "Alpha")] });
const w2 = createSettingsWindow({ sections: [fakeSection("z", "Zeta")] });
w1.open("a");
w2.open("z");
assert.equal(document.querySelectorAll(".cx-modal").length, 2);
const texts = [...document.querySelectorAll(".cx-modal")].map((m) => m.textContent);
assert.ok(texts.some((t) => t.includes("Alpha content")), "the first window lost its own section");
assert.ok(texts.some((t) => t.includes("Zeta content")), "the second window did not open its own section");
w1.close();
w2.close();
});
+35 -24
View File
@@ -65,13 +65,14 @@ export function waitForRestart({ poll = 1500, give_up = 180000 } = {}) {
}
/**
* open() -> the window. One at a time.
*
* Built from a live status every time it opens: what branch you are on and how
* far behind are exactly the facts that go stale while a window is closed.
* mount({ onRestart, running, setFooter, close }) -> a composer handle, no
* modal chrome. See tempfiles.js's mount() for why setFooter/close are the
* only things asked for. Built from a live status the moment it is mounted:
* what branch you are on and how far behind are exactly the facts that go
* stale while this was not on screen.
*/
export function open({ onRestart = waitForRestart, running = () => false } = {}) {
let window_ = null;
export function mount({ onRestart = waitForRestart, running = () => false,
setFooter = () => {}, close = () => {} } = {}) {
const body = composer.region.stack({ gap: "sm", label: "Updates" });
const act = async (run) => {
@@ -88,7 +89,7 @@ export function open({ onRestart = waitForRestart, running = () => false } = {})
// Update while already up to date is a normal thing to do, and waiting for
// a restart that is not coming would hang on an overlay for three minutes.
if (!result || result.restarting === false) { load(); return; }
if (window_) window_.close("restarting");
close();
onRestart();
} catch (err) {
// Said in the window, where the button was pressed. Every refusal this can
@@ -136,8 +137,8 @@ export function open({ onRestart = waitForRestart, running = () => false } = {})
onClick: () => act(() => ask("POST", "/restart")) }),
}));
body.set([composer.group.default({ rows })]);
if (window_) window_.setFooter({ actions: [
composer.button.md({ label: "Close", tone: "ghost", onClick: () => window_.close("done") }),
setFooter({ actions: [
composer.button.md({ label: "Close", tone: "ghost", onClick: () => close() }),
] });
return;
}
@@ -173,20 +174,17 @@ export function open({ onRestart = waitForRestart, running = () => false } = {})
}
body.set([composer.group.default({ rows })]);
if (window_) {
window_.setFooter({
note: s.ok ? "" : "FunPack is not a git checkout, so it cannot update itself.",
actions: [
composer.button.md({ label: "Close", tone: "ghost",
onClick: () => window_.close("done") }),
composer.button.md({
label: s.behind ? `Update (${s.behind})` : "Update", tone: "primary",
disabled: !s.ok || Boolean(s.dirty) || busy,
onClick: () => act(() => ask("POST", "/update", {})),
}),
],
});
}
setFooter({
note: s.ok ? "" : "FunPack is not a git checkout, so it cannot update itself.",
actions: [
composer.button.md({ label: "Close", tone: "ghost", onClick: () => close() }),
composer.button.md({
label: s.behind ? `Update (${s.behind})` : "Update", tone: "primary",
disabled: !s.ok || Boolean(s.dirty) || busy,
onClick: () => act(() => ask("POST", "/update", {})),
}),
],
});
}
async function load() {
@@ -207,6 +205,20 @@ export function open({ onRestart = waitForRestart, running = () => false } = {})
catch { /* the local answer stands, and says it has not checked */ }
}
load();
return body;
}
/**
* open() -> the window. One at a time.
*/
export function open({ onRestart, running } = {}) {
let window_ = null;
const body = mount({
onRestart, running,
setFooter: (f) => window_ && window_.setFooter(f),
close: () => window_ && window_.close("done"),
});
window_ = composer.modal.generic({
title: "Updates",
subtitle: "FunPack updates itself from git, then restarts ComfyUI.",
@@ -214,6 +226,5 @@ export function open({ onRestart = waitForRestart, running = () => false } = {})
body,
onClose: () => { window_ = null; },
});
load();
return window_;
}
+43 -11
View File
@@ -1,5 +1,5 @@
// The unified Settings window: one button, a searchable list, About drawn in
// place, everything else a deep link to its own real window.
// The unified Settings window: one button, a searchable nav list, and every
// section mounted in place beside it -- picking one never leaves the window.
import { test, expect } from "@playwright/test";
@@ -26,22 +26,54 @@ test("search narrows the section list to what matches", async ({ page }) => {
await expect(rows).toContainText("Node packs");
});
test("picking a deep-link section closes Settings and opens that window instead", async ({ page }) => {
test("search also matches a section by its keywords, not only its title", async ({ page }) => {
await app(page);
await page.getByRole("button", { name: "Settings" }).click();
await page.locator(".cx-filter-row", { hasText: "Node packs" }).click();
// Only one modal ever exists at a time -- the deep link replaces this
// window rather than stacking over it.
await expect(page.locator(".cx-modal")).toHaveCount(1);
await expect(page.locator(".cx-modal-title")).toHaveText("Node packs");
// "loaders" is on the pipeline section's keywords, not its visible title
// ("Models and pipeline") or subtitle ("What the run is made of.").
await page.locator(".cx-modal .cx-search").fill("loaders");
const rows = page.locator(".cx-modal .cx-filter-row");
await expect(rows).toHaveCount(1);
await expect(rows).toContainText("Models and pipeline");
});
test("a fresh open always starts on About, even after a deep link was used last", async ({ page }) => {
test("picking a section swaps the content in place -- the window stays open, and there is only ever one", async ({ page }) => {
await app(page);
await page.getByRole("button", { name: "Settings" }).click();
await page.locator(".cx-filter-row", { hasText: "Node packs" }).click();
await page.locator(".cx-modal .cx-icon-btn").click(); // close the packs window
await expect(page.locator(".cx-modal")).toHaveCount(1);
// "Settings" is the window's own title throughout; the section names
// itself inside the body, beside the still-visible nav list.
await expect(page.locator(".cx-modal-title")).toHaveText("Settings");
await expect(page.locator(".cx-modal")).toContainText("Node packs");
await expect(page.locator(".cx-filter-row")).toHaveCount(6); // About + the five, still all there
});
test("switching sections tears down the last one -- its own polling stops", async ({ page }) => {
// The log window polls while mounted. Picking it, then picking something
// else, must stop that poll rather than leaving it running against a
// detached view -- proven by leaving the log mounted only briefly and
// confirming the window still works normally afterwards, requesting
// nothing further from a section no longer on screen.
await app(page);
await page.getByRole("button", { name: "Settings" }).click();
let logRequests = 0;
await page.route("**/api/log**", (route) => { logRequests += 1; route.fulfill({ json: { lines: ["hi"] } }); });
await page.locator(".cx-filter-row", { hasText: "ComfyUI log" }).click();
await expect.poll(() => logRequests).toBeGreaterThan(0);
await page.locator(".cx-filter-row", { hasText: "About FunPack" }).click();
const before = logRequests;
await page.waitForTimeout(2200); // longer than the log's own 2s poll
expect(logRequests, "the log kept polling after it was navigated away from").toBe(before);
});
test("closing and reopening always starts fresh, on About", async ({ page }) => {
await app(page);
await page.getByRole("button", { name: "Settings" }).click();
await page.locator(".cx-filter-row", { hasText: "Node packs" }).click();
await page.locator(".cx-modal .cx-icon-btn").click(); // the one close button, for the whole window
await page.getByRole("button", { name: "Settings" }).click();
await expect(page.locator(".cx-modal")).toContainText("About FunPack");