From 62f92ecaf81f70cd0cf6d6388f0ca5b1e1c2ec19 Mon Sep 17 00:00:00 2001 From: digital-garbage <133010426+digital-garbage@users.noreply.github.com> Date: Sun, 30 Aug 2026 22:11:53 +0300 Subject: [PATCH] app: a menu bar, a timeline, and the prompt in a window of its own v4's arrangement, which is the one that reads as an editor: a zone head holds what acts on THAT zone, so the app grows a bar of its own for what acts on the app. * The menu bar. Brand, View and Settings, and whether ComfyUI is still on the other end. Only menus with something real in them exist -- an empty File menu is a promise the app does not keep. * The prompt is written in the CONSTRUCTOR, a window. v4 called this the Composer; that name is taken here by the element kit, and two things called Composer in one repo is one thing nobody can search for. It is the longest thing a person writes and the least often read, and a permanent third of the centre column is the room the timeline needs. The host is built at startup and outlives every opening, so a window that opens twice does not throw away what was typed. * The timeline replaces it, and Generate moves into its head with what the run is doing at the far end of the same head. No bar across the bottom: one that spans the window belongs to no region. Four faults found on the way, three of them mine and one much older: * THE OVERLAY ROOT WAS A STACKING CONTEXT. It documents at length that it must never be one -- and then set `position: fixed`, which makes one on its own. So the whole z ladder was measured inside that single element: an open menu at z 500 painted UNDER a button at z 1. Every overlay in the app was one positive z-index away from being buried, and the only reason nobody had seen it is that almost nothing claims a z-index. The root is static and zero-sized now; every layer in it is fixed already. * Generate's hover ran `filter: brightness()`, which lightens the LABEL along with the fill under it -- the one thing that had to stay readable lost contrast in both themes. Only the fill moves now, and which way it moves depends on the theme: dark's accent carries near-black text so it goes lighter, light's carries white so it goes darker. Measured: no gradient stop under 4.1:1, hovered or at rest. * A fixed-height head could not shrink below the width of everything in it, and a head now holds a title, three controls and a status. That min-content became the centre column's, so a 768px window laid out 878px wide and scrolled sideways. It wraps now, as v4's does. * A CSS patch of mine silently did nothing because its anchor text had moved and I had left the assertion off that one replacement. Every patch asserts now. --- app/boot.js | 1 + app/catalogue/demos.js | 1 + app/composer/elements/button.css | 12 +++- app/composer/elements/layout.css | 31 +++++++-- app/composer/elements/layout.js | 21 ++++-- app/composer/elements/text.css | 13 ++++ app/composer/elements/text.js | 9 +++ app/composer/internals/portal.js | 19 ++++-- app/composer/tests/portal.test.js | 13 ++++ app/composer/tokens/tokens.css | 10 +++ app/shell/constructor.js | 54 +++++++++++++++ app/shell/layout.js | 96 +++++++++++++++++---------- app/shell/menubar.js | 101 +++++++++++++++++++++++++++++ app/shell/run.js | 72 +++++++++++++++++--- app/shell/session.js | 4 ++ app/shell/tests/layout.test.js | 73 +++++++++++++++++++-- app/shell/tests/session.test.js | 10 ++- app/shell/tests/transport.test.js | 8 ++- app/shell/transport.js | 32 +++++---- app/tests-layout/_menu.js | 5 ++ app/tests-layout/alignment.spec.js | 46 +++++++++---- app/tests-layout/pipeline.spec.js | 27 +++++--- app/tests-layout/prompt.spec.js | 35 ++++++++-- app/tests-layout/workspace.spec.js | 10 +-- 24 files changed, 588 insertions(+), 115 deletions(-) create mode 100644 app/shell/constructor.js create mode 100644 app/shell/menubar.js create mode 100644 app/tests-layout/_menu.js diff --git a/app/boot.js b/app/boot.js index 30e8ec45..4d38d95e 100644 --- a/app/boot.js +++ b/app/boot.js @@ -38,6 +38,7 @@ async function start() { const page = build(root, { onGenerate: () => session.generate(), onCancel: () => run.cancel(), + onConstructor: () => page.constructor.open(), onPipeline: () => openPipeline({ load, describe, check, search, onApply: (next) => { diff --git a/app/catalogue/demos.js b/app/catalogue/demos.js index 2b20ec31..cf0159e9 100644 --- a/app/catalogue/demos.js +++ b/app/catalogue/demos.js @@ -139,6 +139,7 @@ export const DEMOS = { control: c.button.md({ label: "Reset", tone: "danger" }) }), ], }), + "brand.default": { name: "FunPack" }, "panel.default": (c) => ({ title: "Scene 3", actions: [c.iconButton.sm({ icon: "✎", label: "Rename" }), c.iconButton.sm({ icon: "✕", label: "Remove" })], diff --git a/app/composer/elements/button.css b/app/composer/elements/button.css index a16c4426..e2ec1e6a 100644 --- a/app/composer/elements/button.css +++ b/app/composer/elements/button.css @@ -22,9 +22,15 @@ color: var(--on-accent); font-weight: var(--fw-bold); box-shadow: var(--glow-accent); } -/* Brightened rather than repainted: a second accent colour for hover is a - second thing to keep in step with the first, and this one cannot drift. */ -.cx-btn-primary:hover:not(:disabled) { filter: brightness(1.08); } +/* The SURFACE brightens, not the button. + `filter: brightness()` applies to the whole element, text included -- so the + label was lightened along with the fill under it and the one thing that had + to stay readable lost contrast in both themes. The label's colour is fixed + and only the fill moves, one step up the same accent ramp. */ +.cx-btn-primary:hover:not(:disabled) { + background: var(--grad-accent-hover); border-color: var(--accent-hover-edge); +} +.cx-btn-primary:active:not(:disabled) { background: var(--accent-lo); } .cx-btn-danger { color: var(--danger); border-color: color-mix(in srgb, var(--danger) 30%, var(--line)); } .cx-btn-danger:hover:not(:disabled) { background: color-mix(in srgb, var(--danger) 12%, transparent); border-color: var(--danger); } diff --git a/app/composer/elements/layout.css b/app/composer/elements/layout.css index 846920f3..a7256b43 100644 --- a/app/composer/elements/layout.css +++ b/app/composer/elements/layout.css @@ -50,6 +50,8 @@ border: var(--bw) solid var(--line); border-radius: var(--r-4); background: var(--elev-1); overflow: hidden; box-shadow: var(--shadow-1); + /* A zone is as wide as it is given, never as wide as its head wants to be. */ + min-inline-size: 0; /* A column, so a panel given a height passes it to its body. Without this a viewer asking for 100% of its parent got 100% of `auto` and collapsed to one line, inside a panel that was the right size around it. */ @@ -66,14 +68,22 @@ .cx-zone { border: 0; border-radius: 0; box-shadow: none; } .cx-panel-head { - display: flex; align-items: center; justify-content: space-between; gap: var(--sp-5); - padding: 0 var(--sp-6); border-bottom: var(--bw) solid var(--line); + display: flex; align-items: center; gap: var(--sp-2) var(--sp-5); + padding: var(--sp-1) var(--sp-6); border-bottom: var(--bw) solid var(--line); background: var(--grad-band); - block-size: var(--menubar-h); flex: none; + /* A MINIMUM height, and it wraps. Fixed, it could not shrink below the width + of everything in it -- and a head now holds a title, two or three controls + and a status. That min-content width became the whole centre column's, so a + 768px window laid out 878px wide and scrolled sideways. v4 hit this and + landed in the same place: the right-hand block wraps, the head grows a + line, and nothing is ever clipped. */ + min-block-size: var(--menubar-h); flex: none; flex-wrap: wrap; + min-inline-size: 0; } .cx-panel-title { font-family: var(--font-display); font-size: var(--fs-3); font-weight: var(--fw-semibold); display: flex; align-items: center; gap: var(--sp-3); + min-inline-size: 0; white-space: nowrap; } /* One accent mark per panel. Carried over from v4, where it is what makes a head read as the top of something rather than as a strip of grey. */ @@ -81,7 +91,14 @@ content: ""; flex: none; inline-size: 5px; block-size: 5px; border-radius: var(--r-pill); background: var(--accent); } -.cx-panel-actions { display: flex; gap: var(--sp-2); } +.cx-panel-actions { display: flex; align-items: center; gap: var(--sp-2); } +/* Whatever the zone has to say, at the far end of its own head: a count, a + duration, where a run has got to. Quiet by default -- it is read at a glance + and never pressed. */ +.cx-panel-status { + margin-inline-start: auto; display: flex; align-items: center; gap: var(--sp-4); + min-inline-size: 0; font-size: var(--fs-6); color: var(--faint); +} /* A column with a gap, not a block: two things put in a body by two different parts of the app had whatever space their own line boxes happened to leave, which is how a prompt box ended up 8px under one label and 3px under the @@ -95,7 +112,11 @@ .cx-panel-body > .cx-viewer, .cx-panel-body > .cx-stack-fill { flex: 1 1 auto; } .cx-panel-flush > .cx-panel-body { padding: 0; } -.cx-toolbar { display: flex; flex-wrap: wrap; align-items: center; gap: var(--sp-2); } +.cx-toolbar { display: flex; align-items: center; gap: var(--sp-2); min-inline-size: 0; } +.cx-toolbar-trailing { + margin-inline-start: auto; display: flex; align-items: center; gap: var(--sp-3); + font-size: var(--fs-6); color: var(--muted); +} .cx-action-bar { display: flex; align-items: center; gap: var(--sp-6); diff --git a/app/composer/elements/layout.js b/app/composer/elements/layout.js index fc1bf7c6..3314da2c 100644 --- a/app/composer/elements/layout.js +++ b/app/composer/elements/layout.js @@ -78,11 +78,17 @@ define("group", "default", ({ label, rows = [], hint } = {}) => { * cut into areas, not a tray of cards with the desk showing between them -- * which is what an editing suite looks like, and what v4 was. */ -const panel = (variant) => ({ title, actions = [], body, flush = false } = {}) => { - const head = (title || actions.length) +const panel = (variant) => ({ title, actions = [], status = [], body, flush = false } = {}) => { + // Title, then what ACTS on this zone, then -- pushed to the far end -- what + // this zone has to SAY about itself. That order is the whole difference + // between an editing suite and a dashboard: the controls for a region live + // in that region's own head, next to the thing they change, instead of in one + // bar at the bottom that acts on whatever happens to be in front. + const head = (title || actions.length || status.length) ? el("header", { cls: "cx-panel-head", children: [ title ? el("h2", { cls: "cx-panel-title", text: title }) : null, actions.length ? el("div", { cls: "cx-panel-actions", children: actions.map((a) => nodeOf(a, `panel.${variant}`)) }) : null, + status.length ? el("div", { cls: "cx-panel-status", children: status.map((a) => nodeOf(a, `panel.${variant} status`)) }) : null, ].filter(Boolean) }) : null; const content = el("div", { cls: "cx-panel-body", children: body ? nodeOf(body, `panel.${variant}`) : null }); @@ -116,9 +122,16 @@ define("region", "stack", ({ children = [], gap = "md", label, fill = false } = return { node, set, destroy: () => node.remove() }; }); -define("toolbar", "default", ({ items = [], label } = {}) => +/** A bar has a start and an end: what you press, and what it tells you. */ +define("toolbar", "default", ({ items = [], trailing = [], label } = {}) => ({ node: el("div", { cls: "cx-toolbar", attrs: { role: "toolbar", "aria-label": label }, - children: items.map((i) => nodeOf(i, "toolbar.default")) }), + children: [ + ...items.map((i) => nodeOf(i, "toolbar.default")), + trailing.length + ? el("div", { cls: "cx-toolbar-trailing", + children: trailing.map((i) => nodeOf(i, "toolbar.default trailing")) }) + : null, + ].filter(Boolean) }), destroy() { this.node.remove(); } })); /** diff --git a/app/composer/elements/text.css b/app/composer/elements/text.css index 95cd6f4e..46b3c408 100644 --- a/app/composer/elements/text.css +++ b/app/composer/elements/text.css @@ -29,3 +29,16 @@ padding: var(--sp-4) var(--sp-5); margin: 0; overflow-x: auto; /* wide code scrolls itself, never the page */ } + +/* The one place the app names itself. The mark carries a little of its own + light, which is what stops a 5px dot reading as a bullet point. */ +.cx-brand { display: inline-flex; align-items: center; gap: var(--sp-3); } +.cx-brand-mark { + inline-size: 9px; block-size: 9px; border-radius: var(--r-pill); + border: 2px solid var(--accent); + box-shadow: 0 0 6px var(--accent-soft); +} +.cx-brand-name { + font-family: var(--font-display); font-weight: var(--fw-bold); + font-size: var(--fs-3); letter-spacing: .2px; color: var(--text); +} diff --git a/app/composer/elements/text.js b/app/composer/elements/text.js index 0fc301d0..358b2704 100644 --- a/app/composer/elements/text.js +++ b/app/composer/elements/text.js @@ -34,6 +34,15 @@ define("label", "field", ({ text, for: htmlFor } = {}) => define("label", "section", ({ text } = {}) => handle(el("div", { cls: "cx-eyebrow", text }))); +/** Who this is. One mark and one word, in the display face, at the far left of + * the menu bar -- the only place in the app that names the app. */ +define("brand", "default", ({ name = "FunPack" } = {}) => + ({ node: el("span", { cls: "cx-brand", children: [ + el("span", { cls: "cx-brand-mark", attrs: { "aria-hidden": "true" } }), + el("span", { cls: "cx-brand-name", text: name }), + ] }), + destroy() { this.node.remove(); } })); + define("code", "inline", ({ text } = {}) => handle(el("code", { cls: "cx-code", text }))); diff --git a/app/composer/internals/portal.js b/app/composer/internals/portal.js index 5e38a495..ada91d3a 100644 --- a/app/composer/internals/portal.js +++ b/app/composer/internals/portal.js @@ -15,12 +15,19 @@ export function portal() { if (!root) { root = document.createElement("div"); root.id = ROOT_ID; - // No transform, filter, backdrop-filter, contain -- OR z-index -- on this - // element, ever. Each creates a stacking context, and then the whole ladder - // is measured against this one element's number instead of against the page: - // a modal at 800 would still paint under a sticky header at 30. - root.style.position = "fixed"; - root.style.inset = "0"; + // NO POSITIONING, no transform, no filter, no contain, no z-index -- ever. + // + // Each of those makes this element a stacking context, and then the whole + // ladder is measured INSIDE it rather than against the page: every rung, + // from a tooltip at 300 to a modal at 800, collapses to whatever this one + // element resolves to. It said exactly that and then set `position: fixed`, + // which creates a stacking context all by itself -- so a menu at z 500 was + // painted under a button at z 1, and the only reason it was not obvious + // everywhere is that almost nothing in the app claims a z-index at all. + // + // Nothing is needed here anyway: every layer mounted into this root is + // itself `position: fixed`, so a static, zero-sized parent lays nothing out + // and intercepts nothing. root.style.pointerEvents = "none"; } if (!root.isConnected) document.body.appendChild(root); diff --git a/app/composer/tests/portal.test.js b/app/composer/tests/portal.test.js index 258e4455..b64e1f14 100644 --- a/app/composer/tests/portal.test.js +++ b/app/composer/tests/portal.test.js @@ -62,3 +62,16 @@ test("an existing root in the document is adopted, not duplicated", () => { assert.equal(portal(), existing); assert.equal(document.querySelectorAll(`#${ROOT_ID}`).length, 1); }); + +test("the overlay root is not a stacking context", () => { + // If it is one, every rung of the ladder is measured inside it instead of + // against the page, and the whole ordering collapses to one number. It used + // to set `position: fixed`, which creates a stacking context on its own -- + // and a menu at z 500 painted under a button at z 1. + const root = portal(); + for (const property of ["position", "zIndex", "transform", "filter", "contain", "isolation", "opacity", "willChange"]) { + const value = root.style[property]; + assert.ok(!value || value === "auto" || value === "none" || value === "static" || value === "1", + `the overlay root sets ${property}: ${value}, which can make it a stacking context`); + } +}); diff --git a/app/composer/tokens/tokens.css b/app/composer/tokens/tokens.css index 85c1c19b..7312f354 100644 --- a/app/composer/tokens/tokens.css +++ b/app/composer/tokens/tokens.css @@ -121,6 +121,11 @@ the flatness of a large area of one colour, which is most of this app. */ --grain-opacity: .035; --grain-blend: overlay; + + /* Hover moves the fill AWAY from the label, and which way that is depends on + the theme: dark's accent carries near-black text, so it goes lighter. */ + --grad-accent-hover: linear-gradient(180deg, var(--accent-hi), var(--accent)); + --accent-hover-edge: var(--accent-hi); } /* ── light: white, grey and daylight blue ─────────────────────────────── */ @@ -163,6 +168,11 @@ multiplies a much fainter one. v4 landed on the same two numbers. */ --grain-opacity: .015; --grain-blend: multiply; + + /* Light's accent carries WHITE text, so brightening it on hover walks the + label towards its own background. It goes darker instead. */ + --grad-accent-hover: linear-gradient(180deg, var(--accent), var(--accent-lo)); + --accent-hover-edge: var(--accent-lo); } /* Derived values, written once so both themes get them for free. color-mix diff --git a/app/shell/constructor.js b/app/shell/constructor.js new file mode 100644 index 00000000..104dfd36 --- /dev/null +++ b/app/shell/constructor.js @@ -0,0 +1,54 @@ +// The Constructor: where a run is written, in a window of its own. +// +// v4 called this the Composer. The name is taken here -- `app/composer` is the +// element kit -- and two things called Composer in one codebase is one thing +// nobody can search for. +// +// The prompt is not a panel on the main window. It is the longest thing a +// person writes, it is edited in bursts and read rarely, and giving it a +// permanent third of the centre column cost the timeline the room it needs. So +// it opens, is written in, and closes. +// +// The HOST is built once, at startup, and outlives every opening. Two reasons, +// and both were faults before this file existed: a mount point has to exist +// when modules mount, which is long before anyone opens a window; and a window +// that rebuilt its contents on each open would throw away whatever was typed +// and not saved. + +import { composer } from "../composer/composer.js"; + +export function createConstructor({ title = "Constructor" } = {}) { + const empty = composer.emptyState.default({ + icon: "✎", + title: "Nothing to write yet", + hint: "The pipeline decides which of its inputs are written here.", + }); + + // The host is a plain region: it is handed to the modal as a body and handed + // back when the modal closes, which is why the modal never owns it. + const host = composer.region.stack({ gap: "sm", label: "Constructor", fill: true, + children: [empty] }); + + let window_ = null; + + function open() { + if (window_) return window_; // one window onto one prompt + window_ = composer.modal.generic({ + title, + subtitle: "What this run is made of.", + size: "lg", + body: host, + actions: [composer.button.md({ label: "Done", tone: "primary", + onClick: () => window_ && window_.close("done") })], + onClose: () => { window_ = null; }, + }); + return window_; + } + + return { + host, empty, open, + /** Open, and mounted. Tests and the shell both ask. */ + get isOpen() { return Boolean(window_); }, + close() { if (window_) window_.close("close"); }, + }; +} diff --git a/app/shell/layout.js b/app/shell/layout.js index c83231da..f48f1acc 100644 --- a/app/shell/layout.js +++ b/app/shell/layout.js @@ -18,10 +18,19 @@ import { composer } from "../composer/composer.js"; import { offer } from "./mounts.js"; import { createTransport } from "./transport.js"; import { createBin } from "./bin.js"; +import { createMenubar } from "./menubar.js"; +import { createConstructor } from "./constructor.js"; export function build(root, handlers = {}) { - // Centre: the result, and the prompt under it. The prompt is the one control - // that is always on screen, because it is the one always being edited. + // The transport: Generate, Cancel, and what the run is doing. Built before the + // zones because its controls are handed to the zone that holds them. + const transport = createTransport(handlers); + + // Built here rather than on first open: a mount point has to exist when + // modules mount, which is long before anyone opens a window. + const constructor = createConstructor(); + + // Centre: the result, and the timeline under it. const viewer = composer.viewer.media({ empty: "The result of the last run appears here." }); // Left: what a project is made of, which for now is what it has produced. The @@ -33,25 +42,50 @@ export function build(root, handlers = {}) { actions: [bin.control], body: bin.host, }); + // v4's preview head carries nothing but the name of the zone: what is being + // looked at is not something you act on. const preview = composer.panel.zone({ title: "Preview", body: viewer, flush: true }); + + // The timeline: what the project IS, and where a run is started from. + // + // Generate lives here rather than in a bar across the bottom, because a bar + // that spans the window belongs to no region and acts on whatever happens to + // be in front -- which is a dashboard. v4 puts Generate in the timeline head, + // beside the thing it fills, and so does this. + // + // The warning is the first thing INSIDE the zone rather than in its head: it + // is a sentence, not a chip, and it has to sit under the button it is about + // -- an inert setting is said where the run is started, not where it was + // switched on. + const timelineEmpty = composer.emptyState.default({ + icon: "▭", + title: "Nothing on the timeline", + hint: "What you generate lands here, in the order it plays.", + }); + const timeline = composer.panel.zone({ + title: "Timeline", + actions: [ + ...transport.actions, + composer.button.sm({ + label: "Constructor", + onClick: () => { if (handlers.onConstructor) handlers.onConstructor(); }, + }), + ], + status: transport.status, + body: composer.region.stack({ gap: "sm", fill: true, + children: [transport.warning, timelineEmpty] }), + }); + // The stand-ins below are EMPTY STATES, not labels: they say a region is // empty, and settle() takes them down once it is not. A line reading "modules // appear here" left above the modules that appeared is a region explaining // itself to nobody. - const promptEmpty = composer.emptyState.default({ - icon: "✎", - title: "No prompt here", - hint: "The pipeline decides which of its inputs appear on the main window.", - }); - const prompt = composer.panel.zone({ title: "Prompt", body: promptEmpty }); - // The result gets the room. The prompt is a few lines and a button's worth of - // controls; at 38% of the height it was mostly empty space taken from the one - // thing on the page anybody is looking at. Draggable, so anyone writing a long - // prompt can take it back. + + // The result gets the room. Draggable, so a long timeline can take it back. const centre = composer.splitPane.v({ size: 74, - label: "Preview and prompt", - panes: [preview, prompt], + label: "Preview and timeline", + panes: [preview, timeline], }); // Right: everything about the run. Generation is open, settings folded away @@ -95,33 +129,24 @@ export function build(root, handlers = {}) { right: properties, }); - // The transport. One row, always visible, and the place a run is started and - // reported -- an inert setting is said HERE, next to Generate, not in the - // panel that would have enabled it. - const transport = createTransport(handlers); - // The top bar: the way into everything that is a WINDOW rather than a panel. - // It holds one button today and will hold the rest as they land -- a place - // that exists is what stops the next one being wedged into the transport row, - // which is the one row that has to keep saying what the run is doing. - const bar = composer.toolbar.default({ - label: "FunPack", - items: [ - composer.button.sm({ - label: "Models and pipeline", - onClick: () => { if (handlers.onPipeline) handlers.onPipeline(); }, - }), - ], - }); + // The menu bar: who this is, what is not a zone, and whether ComfyUI is + // still on the other end. A zone head holds what acts on THAT zone, so + // anything that acts on the app has nowhere else to live. + const menubar = createMenubar({ workspace, onPipeline: handlers.onPipeline }); + const bar = menubar; - const page = composer.frame.app({ header: bar, main: workspace, footer: transport }); + const page = composer.frame.app({ header: bar, main: workspace }); root.replaceChildren(page.node); // The mount point IS the contract with modules; the region behind it can be // rearranged freely as long as the name survives. offer("assets.library", assets.body); - offer("generation.prompt", prompt.body, promptEmpty.node); + // The prompt is written in a WINDOW, not in a zone: it is the longest thing a + // person writes and the least often read, and a permanent third of the centre + // column is the room the timeline needs. + offer("generation.prompt", constructor.host.node, constructor.empty.node); offer("settings.general", settings.node, settingsEmpty.node); // One host, five names. Each carries the same stand-in, and settle() takes it // down once anything at all has mounted into the panel they share. @@ -129,6 +154,7 @@ export function build(root, handlers = {}) { offer(`generation.${point}`, generation.node, generationEmpty.node); } - return { workspace, assets, bin, preview, viewer, prompt, generation, settings, - properties, transport, bar, page }; + return { workspace, assets, bin, preview, viewer, timeline, generation, settings, + properties, transport, constructor, menubar, + connection: menubar.connection, bar, page }; } diff --git a/app/shell/menubar.js b/app/shell/menubar.js new file mode 100644 index 00000000..289947f8 --- /dev/null +++ b/app/shell/menubar.js @@ -0,0 +1,101 @@ +// The menu bar: who this is, what is not a zone, and whether the server is +// there. +// +// v4's arrangement, and the reason an app with only zones reads as unfinished: +// a zone head holds what acts on THAT zone, so anything that acts on the app -- +// a window, a preference, the state of the connection -- has nowhere to live +// unless there is a bar that belongs to the app itself. +// +// Only menus with something real in them exist. An empty File menu is a promise +// the app does not keep, and this project's rule for an inapplicable feature is +// that it is absent rather than greyed. + +import { composer } from "../composer/composer.js"; +import { LIVE, LOST } from "./run.js"; +import { services } from "./services.js"; + +const THEMES = [ + { id: "dark", label: "Dark" }, + { id: "light", label: "Light" }, + { id: "auto", label: "Auto", hint: "Follows the system" }, +]; + +/** A dot and a word for whether ComfyUI is still on the other end. */ +export function createConnection() { + const dot = composer.dot.default({ tone: "neutral", label: "ComfyUI" }); + const text = composer.text.xs({ text: "Connecting" }); + + const SAID = { + [LIVE]: { tone: "good", words: "ComfyUI live" }, + [LOST]: { tone: "danger", words: "Reconnecting" }, + }; + + return { + /** Two handles, not a wrapper: the bar it goes in already lays out a row. */ + items: [dot, text], + dot, text, + /** Reads the same run state everything else does. */ + draw(state) { + const said = SAID[state && state.connection] || { tone: "neutral", words: "Connecting" }; + dot.node.className = `cx-dot cx-dot-${said.tone}`; + dot.node.setAttribute("aria-label", said.words); + text.setText(said.words); + }, + }; +} + +/** + * menu(label, items, onPick) -> a button that opens them. + * + * The menu is built on each press rather than kept: what is in it depends on + * what is true when it is opened -- which panel is showing, which theme is on + * -- and a menu built once is a menu that goes stale the first time anything + * changes underneath it. + */ +function menu(label, itemsOf, onPick) { + let open = null; + const button = composer.button.sm({ + label, tone: "ghost", + onClick: () => { + if (open) { open.close("toggle"); open = null; return; } + open = composer.menu.dropdown({ + anchor: button, side: "bottom", align: "start", + items: itemsOf(), + onPick: (id) => { open = null; onPick(id); }, + onClose: () => { open = null; }, + }); + }, + }); + return button; +} + +export function createMenubar({ workspace, onPipeline, theme = services.theme } = {}) { + const connection = createConnection(); + + const view = menu("View", () => [ + { id: "left", label: `${workspace && workspace.isOpen("left") ? "Hide" : "Show"} Assets` }, + { id: "right", label: `${workspace && workspace.isOpen("right") ? "Hide" : "Show"} Properties` }, + { separator: true }, + ...THEMES.map((t) => ({ + ...t, + // A tick rather than a disabled row: which one is on is a fact about the + // app, and a menu that hides it makes the user press one to find out. + icon: theme.get() === t.id ? "✓" : " ", + })), + ], (id) => { + if (id === "left" || id === "right") workspace.toggle(id); + else theme.set(id); + }); + + const settings = menu("Settings", () => [ + { id: "pipeline", label: "Models and pipeline…" }, + ], (id) => { if (id === "pipeline" && onPipeline) onPipeline(); }); + + const bar = composer.toolbar.default({ + label: "FunPack", + items: [composer.brand.default({ name: "FunPack" }), view, settings], + trailing: connection.items, + }); + + return { node: bar.node, bar, connection, view, settings }; +} diff --git a/app/shell/run.js b/app/shell/run.js index 231a50fa..bb40eafd 100644 --- a/app/shell/run.js +++ b/app/shell/run.js @@ -18,6 +18,14 @@ export const DONE = "done"; export const FAILED = "failed"; export const CANCELLED = "cancelled"; +/** The socket, which is a different question from what a run is doing. */ +export const OFFLINE = "offline"; // not opened yet +export const LIVE = "live"; +export const LOST = "lost"; // dropped, and trying again + +/** How long to wait before the next attempt: 1s, 2s, 4s ... capped at 30. */ +export const backoff = (attempt) => Math.min(30_000, 1000 * 2 ** Math.max(0, attempt)); + /** Where ComfyUI serves a produced file from. */ export function viewUrl(image, base = "") { const q = new URLSearchParams({ @@ -38,6 +46,10 @@ export function createRun({ let state = { phase: IDLE, promptId: null, progress: null, images: [], audio: [], error: null, node: null, + // Whether this page can still hear ComfyUI at all. Part of the run state + // rather than a second thing to subscribe to: one state object is what + // stops two parts of the UI disagreeing about what is happening. + connection: OFFLINE, }; const emit = (next) => { @@ -202,20 +214,56 @@ export function createRun({ } let socket = null; + let attempt = 0; + let waiting = null; + + // A CLOSED socket is still an object. "Already listening" therefore cannot be + // "there is a socket": every drop -- a laptop sleeping, a tunnel to a rented + // box hiccuping, ComfyUI restarting -- was permanent for that tab, and + // silent. Progress and results simply stopped arriving while the GPU carried + // on, and only a manual reload brought them back. + // + // readyState is undefined on a stand-in socket, and a stand-in is a live one. + const alive = (s) => Boolean(s) && (s.readyState === undefined || s.readyState <= 1); + function listen() { - if (socket || typeof connect !== "function") return socket; + if (typeof connect !== "function") return null; + if (alive(socket)) return socket; + + if (waiting) { clearTimeout(waiting); waiting = null; } socket = connect(clientId); - if (socket) { - socket.addEventListener("message", (event) => { - // Binary frames are previews, which nothing here consumes yet. A parse - // failure must not take the socket down with it. - if (typeof event.data !== "string") return; - try { handle(JSON.parse(event.data)); } catch { /* not for us */ } - }); - } + if (!socket) return null; + + socket.addEventListener("open", () => { attempt = 0; emit({ connection: LIVE }); }); + socket.addEventListener("message", (event) => { + // Binary frames are previews, which nothing here consumes yet. A parse + // failure must not take the socket down with it. + if (typeof event.data !== "string") return; + try { handle(JSON.parse(event.data)); } catch { /* not for us */ } + }); + + // close AND error: a refused connection fires error without ever closing on + // some browsers, and a dropped one closes without an error on others. + const dropped = (which) => () => { + if (which !== socket) return; // an older socket letting go + emit({ connection: LOST }); + retry(); + }; + socket.addEventListener("close", dropped(socket)); + socket.addEventListener("error", dropped(socket)); return socket; } + function retry() { + if (waiting) return; + const wait = backoff(attempt); + attempt += 1; + waiting = setTimeout(() => { waiting = null; listen(); }, wait); + // Not a reason to keep a process alive: under a test runner a pending timer + // is the difference between a suite that finishes and one that hangs. + if (waiting && typeof waiting.unref === "function") waiting.unref(); + } + return { get state() { return { ...state }; }, @@ -234,6 +282,12 @@ export function createRun({ }, handle, // for tests and for a socket owned elsewhere listen, + /** Stop trying. Only a page being torn down wants this. */ + stopListening() { + if (waiting) { clearTimeout(waiting); waiting = null; } + if (socket && typeof socket.close === "function") socket.close(); + socket = null; + }, /** Queue a prompt. Resolves with the prompt id, or throws with the reason. */ async start(prompt) { diff --git a/app/shell/session.js b/app/shell/session.js index c8ca0a30..26007572 100644 --- a/app/shell/session.js +++ b/app/shell/session.js @@ -108,6 +108,10 @@ export function wire({ run, page, check, id, queuedFor, finishedFor, slots, valu // straight back -- live-looking, and doing nothing when pressed. run.subscribe((state) => { page.transport.draw(state); + // Whether ComfyUI is still there rides on the same state object, so it is + // drawn by the same subscription: two things watching the connection is two + // things that can disagree about whether the app is talking to anything. + if (page.connection) page.connection.draw(state); // Results go to the BIN, and the bin decides what is on screen. Pointing // the viewer at the newest image from here instead put two things in charge // of it: every progress message re-showed the newest result, so clicking an diff --git a/app/shell/tests/layout.test.js b/app/shell/tests/layout.test.js index cecfafad..be0895c9 100644 --- a/app/shell/tests/layout.test.js +++ b/app/shell/tests/layout.test.js @@ -56,12 +56,30 @@ test("picking an older result in the bin changes what the viewer shows", () => { assert.match(p.viewer.node.querySelector("img").getAttribute("src"), /first\.png/); }); -test("the bin's view control sits in the Assets panel, not in the transport row", () => { - // Every control that is about a run belongs beside Generate; this one is not - // about a run, and putting it there is how that row stops being readable. +test("a control sits in the head of the zone it acts on", () => { + // The arrangement that makes this an editor rather than a dashboard: the bin's + // view control is in the Assets head, and Generate is in the head of the zone + // the run fills -- not in one bar at the bottom acting on whatever is in front. const p = page(); - assert.ok(p.assets.node.contains(p.bin.control.node)); - assert.equal(p.transport.node.contains(p.bin.control.node), false); + assert.ok(p.assets.node.contains(p.bin.control.node), "the view control left the Assets head"); + assert.ok(p.timeline.node.contains(p.transport.generate.node), "Generate is not in the Timeline head"); + assert.equal(p.assets.node.contains(p.transport.generate.node), false); + assert.equal(p.page.footer, null, "there is still a bar across the bottom"); +}); + +test("what a run is doing is said in the same head that starts it", () => { + const p = page(); + const status = p.timeline.node.querySelector(".cx-panel-status"); + assert.ok(status, "the Timeline head says nothing about the run"); + assert.ok(status.contains(p.transport.statusText.node)); + assert.ok(status.contains(p.transport.progress.node)); +}); + +test("a warning about the next run sits inside the zone that starts it", () => { + // Under the button it is about, not in the head beside it: it is a sentence + // rather than a chip, and a 34px band is not where a sentence goes. + const p = page(); + assert.ok(p.timeline.body.contains(p.transport.warning.node)); }); test("the assets mount point still exists for a module", () => { @@ -70,3 +88,48 @@ test("the assets mount point still exists for a module", () => { assert.ok(p.assets.body.contains(p.bin.host.node), "the bin is not in the panel body a module also mounts into"); }); + +test("the prompt is written in a window, not in a zone", () => { + // A permanent third of the centre column for the thing that is written in + // bursts and read rarely, taken from the timeline, which is what the project + // IS. It opens, is written in, and closes. + const p = page(); + assert.equal(p.constructor.isOpen, false, "the Constructor is open before anyone asked"); + assert.equal(document.querySelector(".cx-modal"), null); + + p.constructor.open(); + assert.equal(p.constructor.isOpen, true); + const modal = document.querySelector(".cx-modal"); + assert.ok(modal, "the Constructor did not open"); + assert.ok(modal.contains(p.constructor.host.node), "the prompt is not in the window"); + assert.match(modal.textContent, /Constructor/); + + p.constructor.close(); + assert.equal(document.querySelector(".cx-modal"), null); +}); + +test("what was typed survives the window being closed", () => { + // The host outlives every opening. A window that rebuilt its contents each + // time would throw away whatever was written and not saved -- which for a + // prompt is the whole of it. + const p = page(); + const box = document.createElement("textarea"); + p.constructor.host.node.appendChild(box); + box.value = "a cat on a rooftop"; + + p.constructor.open(); + p.constructor.close(); + p.constructor.open(); + + const found = document.querySelector(".cx-modal textarea"); + assert.equal(found, box, "the window built itself a new prompt box"); + assert.equal(found.value, "a cat on a rooftop"); + p.constructor.close(); +}); + +test("the timeline is where a run is started from", () => { + const p = page(); + assert.match(p.timeline.node.textContent, /Nothing on the timeline/); + assert.ok(p.timeline.node.querySelector(".cx-panel-actions")); + assert.match(p.timeline.node.querySelector(".cx-panel-actions").textContent, /Constructor/); +}); diff --git a/app/shell/tests/session.test.js b/app/shell/tests/session.test.js index 3daaf4de..2a9c3b1f 100644 --- a/app/shell/tests/session.test.js +++ b/app/shell/tests/session.test.js @@ -26,11 +26,17 @@ test.after(() => teardownDom()); // just before it was overwritten with "Ready" a moment after it appeared. The // test passed and the button did nothing on screen. function transport() { + // The real transport, mounted the way the shell mounts it: its controls go in + // a zone head and what it is saying goes at the far end of the same head. + // There is no bar -- an editor's controls belong to the region they act on. const t = createTransport({}); - document.body.appendChild(t.node); + const head = document.createElement("header"); + for (const handle of [...t.actions, ...t.status, t.warning]) head.appendChild(handle.node); + document.body.replaceChildren(head); return { ...t, - get text() { return t.status.node.textContent; }, + head, + get text() { return t.statusText.node.textContent; }, get disabled() { return t.generate.node.disabled; }, }; } diff --git a/app/shell/tests/transport.test.js b/app/shell/tests/transport.test.js index a756d7a1..375245af 100644 --- a/app/shell/tests/transport.test.js +++ b/app/shell/tests/transport.test.js @@ -1,4 +1,4 @@ -// The transport row, driven by run states. +// The transport, driven by run states. // // The reason this is one component reading one state object: a button here, a // progress bar there and a status somewhere else each get their own idea of @@ -24,8 +24,12 @@ const state = (over = {}) => ({ }); const built = (handlers) => { + // Mounted the way the shell mounts it: the controls go in a zone head, and + // there is no bar of its own to append. const t = createTransport(handlers); - document.body.appendChild(t.node); + const head = document.createElement("header"); + for (const handle of [...t.actions, ...t.status, t.warning]) head.appendChild(handle.node); + document.body.replaceChildren(head); return t; }; diff --git a/app/shell/transport.js b/app/shell/transport.js index 08432519..364ace54 100644 --- a/app/shell/transport.js +++ b/app/shell/transport.js @@ -1,11 +1,17 @@ // Generate, and everything that happens after it. // -// One row that owns the whole lifecycle, because the alternative is a button +// One object owns the whole lifecycle, because the alternative is a button // here and a progress bar there and a status somewhere else, each with its own // idea of what is happening. This reads ONE state object and draws it -- so // "queued" cannot appear next to a finished result, whatever order the messages // arrive in. // +// It owns the CONTROLS, not a row: they are handed out and the shell puts them +// in the head of the zone the run fills. A bar across the bottom of the window +// acts on "whatever is in front" and belongs to no region -- which is what made +// this read as a dashboard rather than an editor. v4 puts Generate in the head +// of the zone it produces, and so does this. +// // Warnings live here too, beside Generate, rather than in the panel that would // have caused them: a setting that will do nothing has to be said where the run // is started, not where it was switched on. @@ -16,24 +22,17 @@ import { IDLE, QUEUED, RUNNING, DONE, FAILED, CANCELLED } from "./run.js"; const WORKING = new Set([QUEUED, RUNNING]); export function createTransport({ onGenerate, onCancel } = {}) { - const generate = composer.button.lg({ + const generate = composer.button.md({ label: "Generate", tone: "primary", onClick: () => { if (onGenerate) onGenerate(); }, }); - const cancel = composer.button.md({ + const cancel = composer.button.sm({ label: "Cancel", tone: "ghost", onClick: () => { if (onCancel) onCancel(); }, }); const progress = composer.progress.bar({ value: 0, max: 100, label: "Generating" }); const status = composer.text.sm({ text: "Ready" }); - const bar = composer.actionBar.sticky({ - // What the run is doing at the start of the row, what starts and stops it - // at the end. The progress bar goes with the words, not with the buttons: - // it is the same statement in another form. - lead: [status, progress], - actions: [cancel, generate], - }); // Something true about the NEXT run, said before it starts and left up while // it does: a setting that will not be applied is not a status, and putting it @@ -42,7 +41,6 @@ export function createTransport({ onGenerate, onCancel } = {}) { // the panel a person has already stopped looking at. const warning = composer.banner.warn({ text: "" }); warning.node.setAttribute("hidden", ""); - const row = composer.region.stack({ gap: "none", children: [warning, bar] }); // Something to say that is not about a run in progress: why the last attempt // did not become one. It has to survive the next draw(), because a refused @@ -113,8 +111,16 @@ export function createTransport({ onGenerate, onCancel } = {}) { warning.node.toggleAttribute("hidden", !text); } - return { node: row.node, draw, say, warn, hold, release, - generate, cancel, progress, status, warning }; + return { + // What the shell puts in a zone head: the controls that start and stop a + // run, and -- at the far end of the same head -- what it is doing. + actions: [cancel, generate], + // The progress bar goes with the words rather than with the buttons: it is + // the same statement in another form. + status: [progress, status], + warning, + draw, say, warn, hold, release, generate, cancel, progress, statusText: status, + }; } /** diff --git a/app/tests-layout/_menu.js b/app/tests-layout/_menu.js new file mode 100644 index 00000000..60916c08 --- /dev/null +++ b/app/tests-layout/_menu.js @@ -0,0 +1,5 @@ +/** The window moved into the Settings menu when the app grew a menu bar. */ +export async function openPipelineWindow(page) { + await page.getByRole("button", { name: "Settings" }).click(); + await page.getByRole("menuitem", { name: /Models and pipeline/ }).click(); +} diff --git a/app/tests-layout/alignment.spec.js b/app/tests-layout/alignment.spec.js index a88fafe3..0bd5a254 100644 --- a/app/tests-layout/alignment.spec.js +++ b/app/tests-layout/alignment.spec.js @@ -7,6 +7,7 @@ // controls in one list at four different widths. import { test, expect } from "@playwright/test"; +import { openPipelineWindow } from "./_menu.js"; const WIDE = { width: 1440, height: 900 }; @@ -81,7 +82,7 @@ test("every control in a list of settings shares its two edges", async ({ page } // A