From b85c79a6a2adb8cd8e58caf64ef961431ed29a8d Mon Sep 17 00:00:00 2001 From: digital-garbage <133010426+digital-garbage@users.noreply.github.com> Date: Sun, 6 Sep 2026 22:53:34 +0300 Subject: [PATCH] app, core: save/load a result, save/load a project, generate every scene Player: Save to bin (whatever the viewer shows, if it has a real file identity) and Save frame (canvas-capture the video on screen, upload through ComfyUI's own /upload/image, no new route needed). Both add without navigating the viewer -- an upload is a real round trip, and the result is added to the bin, not shown as a stale finished pick. Save/Load Project File: a real download route with Content-Disposition, and an import route that never trusts an id in the file -- an import always lands as its own project, never overwrites one already here. Generate All: every scene in order, waiting for each to finish before starting the next -- not a montage (v5 has no render/stitch stage), N separate generations each landing on its own scene. Both project-switching (open/newProject/importProject) and the earlier run-reattach path shared a bug class this session kept finding: state captured at one moment, applied at a later async moment with no check that nothing had moved on in between. Closed it properly this time -- a generation counter every switch bumps and checks, including start() (the page's own handlers are live before it resolves) and undo/redo (also "a different project is open now"), and scheduleSave() captures the object an edit actually touched instead of re-reading a variable that can point somewhere else by the time its debounce timer fires. Four adversarial rounds against this range, each fixed before landing. --- app/boot.js | 103 ++++++++++- app/composer/elements/gallery.js | 34 +++- app/shell/bin.js | 9 +- app/shell/layout.js | 77 +++++++- app/shell/menubar.js | 5 + app/shell/projects.js | 96 +++++++++- app/shell/session.js | 35 ++++ app/shell/tests/bin.test.js | 24 +++ app/shell/tests/layout.test.js | 120 +++++++++++++ app/shell/tests/projects.test.js | 237 +++++++++++++++++++++++++ app/shell/tests/session.test.js | 53 +++++- app/tests-layout/alignment.spec.js | 8 +- app/tests-layout/generate_all.spec.js | 65 +++++++ app/tests-layout/player.spec.js | 62 +++++++ app/tests-layout/project_file.spec.js | 86 +++++++++ app/tests-layout/project_video.spec.js | 7 +- app/tests-layout/workspace.spec.js | 10 +- core/routes.py | 35 ++++ core/tests/test_projects.py | 91 ++++++++++ 19 files changed, 1125 insertions(+), 32 deletions(-) create mode 100644 app/tests-layout/generate_all.spec.js create mode 100644 app/tests-layout/player.spec.js create mode 100644 app/tests-layout/project_file.spec.js diff --git a/app/boot.js b/app/boot.js index ffc72f3c..410c6a0d 100644 --- a/app/boot.js +++ b/app/boot.js @@ -9,9 +9,9 @@ import { mountAll } from "./shell/panels.js"; import { settle } from "./shell/mounts.js"; import { all as allValues } from "./shell/values.js"; import { composer } from "./composer/composer.js"; -import { createRun, viewUrl, DONE } from "./shell/run.js"; +import { createRun, viewUrl, DONE, FAILED, CANCELLED } from "./shell/run.js"; import { clientId, connect, queuedFor, finishedFor } from "./shell/client.js"; -import { wire } from "./shell/session.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 { createPrompts } from "./shell/prompt.js"; @@ -113,13 +113,85 @@ async function start() { // Before the page, because the properties column is composed with it in. inspector = createInspector({ project, onRename: (name) => project.rename(name) }); + // "Load Project File..." trigger. One input, kept and reused rather than + // built fresh per click -- a fresh one would need its `change` listener + // rewired every time, which is a listener leaked on every click but the last. + const loadProjectFile = document.createElement("input"); + loadProjectFile.type = "file"; + loadProjectFile.accept = "application/json,.json"; + loadProjectFile.hidden = true; + document.body.appendChild(loadProjectFile); + loadProjectFile.addEventListener("change", async () => { + const file = loadProjectFile.files[0]; + loadProjectFile.value = ""; // the same file picked twice must still change + if (!file) return; + try { + await project.importProject(JSON.parse(await file.text())); + } catch (err) { + page.transport.say(`Could not load that project file: ${err.message}`); + } + }); + + // What Generate does for whichever scene is selected right now -- shared + // with "Generate All" below, so the two never disagree about how a run gets + // attributed to its scene. + const generateCurrentScene = () => { + ranFor = project.selectedId; + ranForProject = project.project ? project.project.id : null; + return session.generate(); + }; + + // Every scene, one at a time, waiting for each to actually finish before + // starting the next. Not a montage -- v5 has no render/stitch stage -- this + // is N separate generations, each landing on its own scene, the same as + // pressing Generate N times without having to sit at the keyboard for each. + let batchCancelled = false; + async function generateAll() { + const scenes = [...project.scenes]; + if (!scenes.length) return; + generateAllBtn.setDisabled(true); + batchCancelled = false; + page.transport.hold(`Generating scene 1 of ${scenes.length}…`); + const failed = []; + try { + for (let i = 0; i < scenes.length; i++) { + if (batchCancelled) break; + const scene = scenes[i]; + // A scene removed mid-batch by whoever is not at the keyboard right + // now is not a scene left to generate. + if (!project.scenes.some((s) => s.id === scene.id)) continue; + project.select(scene.id); + showScene(project.selected); + page.transport.say(`Generating scene ${i + 1} of ${scenes.length}…`); + const waiting = waitForTerminal(run); + const queued = await generateCurrentScene(); + if (!queued) { + // Refused before it ever reached run.start() -- nothing will ever + // transition the run, so nothing will ever resolve `waiting` either. + waiting.cancel(); + failed.push(scene); + continue; + } + const phase = await waiting; + if (phase === CANCELLED) { batchCancelled = true; break; } + if (phase === FAILED) failed.push(scene); + } + } finally { + generateAllBtn.setDisabled(false); + page.transport.release(run.state); + } + if (batchCancelled) page.transport.say("Generate All was cancelled."); + else if (failed.length) page.transport.say(`${failed.length} of ${scenes.length} scenes did not generate.`); + } + const generateAllBtn = composer.button.sm({ + label: "Generate All", tone: "ghost", + onClick: () => generateAll(), + }); + const page = build(root, { inspector, - onGenerate: () => { - ranFor = project.selectedId; - ranForProject = project.project ? project.project.id : null; - session.generate(); - }, + onGenerate: generateCurrentScene, + generateAll: generateAllBtn, onCancel: () => run.cancel(), onConstructor: () => page.constructor.open(), // The window asks whether a run is in flight, because the restart that @@ -129,7 +201,8 @@ async function start() { onLog: () => openLog(), // 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. - onTemp: () => openTemp({ onOpen: (item) => page.viewer.setSource(item.url, item.kind) }), + onTemp: () => openTemp({ onOpen: (item) => page.viewer.setSource( + item.url, item.kind, item.file ? { ...item.file, type: "temp" } : null) }), // What the Edit menu offers, and what the keyboard reaches. One list, so a // menu item and its shortcut cannot drift apart. edits: { @@ -156,6 +229,19 @@ async function start() { const name = await asked.result; if (name === null) return; // cancelled await project.newProject(name); + } else if (id === "save-file") { + const url = project.downloadUrl(); + if (!url) return; + // Navigation, not a fetch: the server's own Content-Disposition is + // what makes this a save-as instead of a page full of JSON. + const a = document.createElement("a"); + a.href = url; + a.download = ""; + document.body.appendChild(a); + a.click(); + a.remove(); + } else if (id === "load-file") { + loadProjectFile.click(); } else if (id !== (project.project || {}).id) { await project.open(id); } else { @@ -331,6 +417,7 @@ async function start() { window.FunPack = { manifest, values: allValues, failed, hidden, run, bin: page.bin, project, wheel, + viewer: page.viewer, prompts: () => (prompts ? prompts.overrides() : {}), mounted: mounted.map((m) => m.id), }; diff --git a/app/composer/elements/gallery.js b/app/composer/elements/gallery.js index aa3005b5..e40b8533 100644 --- a/app/composer/elements/gallery.js +++ b/app/composer/elements/gallery.js @@ -304,9 +304,15 @@ define("ruler", "default", ({ marks = [], total = 1, label } = {}) => { define("viewer", "media", ({ src, kind = "image", empty = "Nothing yet", onError } = {}) => { const stage = el("div", { cls: "cx-viewer-stage" }); let current = null; + let currentKind = kind; + let currentFile = null; + let mediaEl = null; - function show(next, nextKind = kind) { + function show(next, nextKind = kind, file = null) { current = next || null; + currentKind = nextKind; + currentFile = current ? file : null; + mediaEl = null; stage.replaceChildren(); if (!current) { stage.append(el("p", { cls: "cx-list-empty", text: empty })); @@ -315,10 +321,16 @@ define("viewer", "media", ({ src, kind = "image", empty = "Nothing yet", onError const media = nextKind === "video" ? el("video", { cls: "cx-viewer-media", attrs: { src: current, controls: "", loop: "", playsinline: "" } }) : el("img", { cls: "cx-viewer-media", attrs: { src: current, alt: "" } }); + mediaEl = media; // A failed load is reported rather than left as a broken icon: the file is // produced by a run, so it not arriving is a fault worth naming. media.addEventListener("error", () => { stage.replaceChildren(el("p", { cls: "cx-list-empty", text: "This result could not be loaded." })); + mediaEl = null; + // Nothing is actually showing any more -- `file` saying otherwise is + // what let "Save to bin" silently bin a dead reference from this exact + // state, the one place the panel itself says there is nothing here. + currentFile = null; if (onError) onError(current); }); stage.append(media); @@ -329,8 +341,26 @@ define("viewer", "media", ({ src, kind = "image", empty = "Nothing yet", onError return { node, get value() { return current; }, + get kind() { return currentKind; }, + // The file this came from -- {filename, subfolder, type} -- when the + // caller had one to give. Null for anything shown by URL alone, which is + // not enough to name a file back to the server with. + get file() { return currentFile; }, setValue: (next) => show(next, kind), - setSource: (next, nextKind) => show(next, nextKind || kind), + setSource: (next, nextKind, file) => show(next, nextKind || kind, file), + /** + * The video frame on screen right now, as a PNG blob -- null if nothing is + * playing or there is no frame yet to draw (a video that has not loaded + * its first frame has no dimensions to capture at). + */ + captureFrame() { + if (currentKind !== "video" || !mediaEl || !mediaEl.videoWidth) return Promise.resolve(null); + const canvas = document.createElement("canvas"); + canvas.width = mediaEl.videoWidth; + canvas.height = mediaEl.videoHeight; + canvas.getContext("2d").drawImage(mediaEl, 0, 0); + return new Promise((resolve) => canvas.toBlob(resolve, "image/png")); + }, destroy: () => node.remove(), }; }); diff --git a/app/shell/bin.js b/app/shell/bin.js index 388efd73..2ecc9671 100644 --- a/app/shell/bin.js +++ b/app/shell/bin.js @@ -132,8 +132,13 @@ export function createBin({ onOpen, view = recallView(), persist = true, * anything new arrives -- so it must be silent when there is nothing new. It * was not, once: redrawing on each message walked the selection back to the * newest result the moment the user clicked an older one. + * + * `open: false` is for a save action, not a run: something ADDED to the bin + * a while after it was asked for (an upload's own round trip) must not then + * yank the viewer back to it -- the user may have moved on to something else + * in the meantime, and a save was never a request to look at what got saved. */ - function absorb(files = []) { + function absorb(files = [], { open: shouldOpen = true } = {}) { const added = []; for (const file of files || []) { const id = keyOf(file); @@ -151,7 +156,7 @@ export function createBin({ onOpen, view = recallView(), persist = true, // its own result is the whole point; an older selection is not preserved // through it, because nobody generates in order to keep looking at the last // one. - open(added[added.length - 1].id); + if (shouldOpen) open(added[added.length - 1].id); return added; } diff --git a/app/shell/layout.js b/app/shell/layout.js index e211e98c..acac9fd6 100644 --- a/app/shell/layout.js +++ b/app/shell/layout.js @@ -38,15 +38,81 @@ export function build(root, handlers = {}) { // Left: what a project is made of, which for now is what it has produced. The // bin owns the choice of what the viewer shows -- one place decides, so a // click on an older result and a run finishing cannot both be right at once. - const bin = createBin({ onOpen: (item) => viewer.setSource(item.url, item.kind) }); + const bin = createBin({ + onOpen: (item) => viewer.setSource(item.url, item.kind, item.file), + }); const assets = composer.panel.zone({ title: "Assets", actions: [bin.control], body: bin.host, }); + + // ComfyUI's own route, not FunPack's -- ordinary image upload already exists + // and there is nothing about a captured frame that needs its own. + async function uploadFrame(blob) { + const form = new FormData(); + form.append("image", blob, `frame-${Date.now()}.png`); + form.append("type", "input"); + const res = await fetch("/upload/image", { method: "POST", body: form }); + if (!res.ok) throw new Error(`the upload was refused (${res.status})`); + const body = await res.json(); + return { filename: body.name, subfolder: body.subfolder || "", type: body.type || "input" }; + } + + // `transport.say` is the Timeline's own line, overwritten by the next + // progress message a run in flight sends -- often inside a second, which + // silently erased a save's own confirmation or error whenever anything was + // generating. This one is the Preview zone's, and nothing else writes to it. + const previewStatus = composer.text.sm({ text: "" }); + let previewStatusTimer = null; + function sayPreview(text, ms = 3000) { + previewStatus.setText(text); + if (previewStatusTimer) clearTimeout(previewStatusTimer); + previewStatusTimer = setTimeout(() => previewStatus.setText(""), ms); + // Not a reason to keep a process alive: under a test runner, a page built + // and abandoned mid-test (a new one built for the next test, this one's + // DOM never explicitly torn down) left a timer that fired minutes later, + // reaching for a `document` the test harness had already removed. + if (previewStatusTimer && typeof previewStatusTimer.unref === "function") previewStatusTimer.unref(); + } + + // What's shown here came from somewhere already server-side (a run's own + // result, or an already-open bin/temp item) OR is a video the viewer can + // still draw a frame from -- either way, "keep this" should not mean a + // download and a re-upload by hand. + const saveToBin = composer.iconButton.sm({ + icon: "➕", label: "Save to bin", + onClick: () => { + if (!viewer.file) { sayPreview("Nothing here has a file to save."); return; } + // Already showing, so nothing needs to move -- but a save is a save, + // never a navigation, even when it happens to name what's on screen. + const added = bin.absorb([viewer.file], { open: false }); + sayPreview(added.length ? "Saved to the bin." : "Already in the bin."); + }, + }); + const saveFrame = composer.iconButton.sm({ + icon: "⛶", label: "Save this frame", + onClick: async () => { + const blob = await viewer.captureFrame(); + if (!blob) { sayPreview("Play a video first -- there is no frame to save."); return; } + try { + // Not `open: true`: the upload is a real round trip, and by the time + // it lands the viewer may be showing something else entirely -- a + // save must not yank it back to what it saved. + const added = bin.absorb([await uploadFrame(blob)], { open: false }); + sayPreview(added.length ? "Frame saved to the bin." : "Already in the bin."); + } catch (err) { + sayPreview(`Could not save the frame: ${err.message}`); + } + }, + }); // 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 }); + // looked at is not something you act on -- except now it is, the two + // actions above, which only ever touch what is ALREADY on screen. + const preview = composer.panel.zone({ + title: "Preview", body: viewer, flush: true, + actions: [saveToBin, saveFrame], status: [previewStatus], + }); // The timeline: what the project IS, and where a run is started from. // @@ -101,6 +167,11 @@ export function build(root, handlers = {}) { title: "Timeline", actions: [ ...transport.actions, + // A ready-made widget, not a callback: the loop it runs needs the run, + // the session and the project all at once, and none of those exist yet + // at the point build() is called -- boot.js is the one place all three + // do, so boot.js builds the button too. + ...(handlers.generateAll ? [handlers.generateAll] : []), regionToggle("Assets", () => Boolean(ws && ws.isOpen("left")), () => ws && ws.toggle("left"), { overlaid: true }), regionToggle("Properties", () => Boolean(ws && ws.isOpen("right")), diff --git a/app/shell/menubar.js b/app/shell/menubar.js index c0275cbf..136a310a 100644 --- a/app/shell/menubar.js +++ b/app/shell/menubar.js @@ -76,6 +76,11 @@ export function createMenubar({ workspace, onPipeline, onProject, projects = () const file = menu("File", () => [ { id: "new", label: "New project…" }, + { separator: true }, + // Moving a project between machines -- a git checkout does not carry + // ~/Documents/ComfyUI/user/default/funpack_projects with it. + { id: "save-file", label: "Save Project File…", hint: "⬇" }, + { id: "load-file", label: "Load Project File…" }, ...(projects().length ? [{ separator: true }] : []), // A tick rather than leaving the current one out: which project is open is // a fact about the app, and a menu that hides it makes you press one to find out. diff --git a/app/shell/projects.js b/app/shell/projects.js index c77238d8..7270676b 100644 --- a/app/shell/projects.js +++ b/app/shell/projects.js @@ -76,6 +76,12 @@ export function createProject({ onChange, onError, onOpen } = {}) { let timer = null; let saving = null; // the PUT in flight, so a queued one waits for it let dirty = false; + // Bumped by every open/newProject/importProject, BEFORE their own await -- + // so a switch that starts and then loses a race (two imports, or an import + // overlapping an open()) can tell, once its fetch finally answers, that a + // NEWER switch already won and abandon its own result instead of clobbering + // whatever the user is actually looking at now. + let generation = 0; const changed = () => { if (onChange) onChange(); }; @@ -94,6 +100,12 @@ export function createProject({ onChange, onError, onOpen } = {}) { if (!project || !from.length) return false; to.push(copy()); const was = from.pop(); + // Same reason open/newProject/importProject/start bump it: an undo/redo + // is also "a different project is open now" (the comment on opened() two + // lines below already says so), and an in-flight switch that started + // before this must not still be allowed to land on top of it once its + // own fetch catches up. + generation += 1; project = was.project; selected = was.selected; // The scene that was selected may be gone -- stepping FORWARD into a state @@ -113,24 +125,39 @@ export function createProject({ onChange, onError, onOpen } = {}) { const forget = () => { past = []; future = []; }; function scheduleSave() { + // The object THIS edit actually touched, captured now -- not read back + // later from `project`, which can point somewhere else entirely by the + // time this timer fires (a project switch started and finished in + // between). Without this, an edit made in the gap between a switch + // starting and completing was silently lost: mutated on the right object, + // then saved -- once the timer finally fired -- against whatever project + // had since replaced it. + // + // ponytail: `dirty`/`timer`/`saving` stay single, shared flags rather than + // one per target. A second edit landing on a DIFFERENT project before this + // one's timer fires still cancels it (real, narrower race: an edit during + // a switch's gap, then another edit to the new project, both inside one + // debounce window). Upgrade to a per-target queue if that ever shows up + // outside a deliberately adversarial test. + const target = project; dirty = true; if (timer) clearTimeout(timer); - timer = setTimeout(flush, SAVE_AFTER); + timer = setTimeout(() => flush(target), SAVE_AFTER); } - async function flush() { + async function flush(target = project) { if (timer) { clearTimeout(timer); timer = null; } - if (!project || !dirty) return; + if (!target || !dirty) return; // One PUT at a time. Two overlapping writes of a whole project can land in // either order, and the loser is a version of the project the user has // already moved past. if (saving) { await saving.catch(() => {}); if (!dirty) return; } dirty = false; - const body = { ...project }; + const body = { ...target }; saving = put(body).then((saved) => { // Only the fields the server owns are taken back: replacing the whole // project would overwrite whatever was typed while the PUT was in flight. - if (project && saved && project.id === saved.id) project.updated_at = saved.updated_at; + if (target && saved && target.id === saved.id) target.updated_at = saved.updated_at; }).catch((err) => { dirty = true; // it is still unsaved, so say so if (onError) onError(err); @@ -150,9 +177,19 @@ export function createProject({ onChange, onError, onOpen } = {}) { get recent() { return recent; }, async start() { + // Guarded like open/newProject/importProject, and for the same reason: + // the page's own handlers (the File menu, the hidden file input) are + // wired up by build() well before this is awaited, and both of start()'s + // own network calls are real gaps a user's own click can finish inside + // of. Losing this race used to mean start() caught up later and + // silently reverted a switch the user had already made, with no error. + const mine = ++generation; const found = await list(); + if (mine !== generation) return project; recent = found; - project = found.length ? await read(found[0].id) : await create("Untitled"); + const opened_ = found.length ? await read(found[0].id) : await create("Untitled"); + if (mine !== generation) return project; + project = opened_; if (!found.length) recent = [{ id: project.id, name: project.name }]; selected = (project.scenes || [])[0]?.id ?? null; forget(); // a project just opened has no past @@ -161,8 +198,14 @@ export function createProject({ onChange, onError, onOpen } = {}) { }, async open(id) { + const mine = ++generation; await flush(); - project = await read(id); + const found = await read(id); + // A newer switch already won this race (another open/new/import started + // after this one and has already landed) -- applying this one now would + // silently pull the user back to a project they already left. + if (mine !== generation) return project; + project = found; selected = (project.scenes || [])[0]?.id ?? null; forget(); // a project just opened has no past opened(); @@ -170,8 +213,18 @@ export function createProject({ onChange, onError, onOpen } = {}) { }, async newProject(name) { + const mine = ++generation; await flush(); - project = await create(name); + const made = await create(name); + if (mine !== generation) { + // Superseded by a newer switch -- this one already created a real + // project on the server that nothing will ever show. Best-effort: + // a failed cleanup leaves an orphan, which is the pre-existing + // (worse) behaviour, not a new one. + remove(made.id).catch(() => {}); + return project; + } + project = made; recent = [{ id: project.id, name: project.name }, ...recent]; selected = (project.scenes || [])[0]?.id ?? null; forget(); // a project just opened has no past @@ -179,6 +232,33 @@ export function createProject({ onChange, onError, onOpen } = {}) { return project; }, + /** Where "Save Project File" downloads FROM, for the current project. */ + downloadUrl() { return project ? `${BASE}/${encodeURIComponent(project.id)}/download` : null; }, + + /** + * A project file from disk, as its own new project -- moving a project + * between machines, not restoring one in place. The file's own id is + * never kept (the server strips it too, but a project already open in + * THIS browser with that id must not be what a stale double-check relies + * on): imported always means "another one", even from the very checkout + * it came from. + */ + async importProject(data) { + const mine = ++generation; + await flush(); + const imported = await json("POST", `${BASE}/import`, data); + if (mine !== generation) { + remove(imported.id).catch(() => {}); // superseded -- see newProject() + return project; + } + project = imported; + recent = [{ id: project.id, name: project.name }, ...recent]; + selected = (project.scenes || [])[0]?.id ?? null; + forget(); + opened(); + return project; + }, + select(id) { if (!sceneAt(id) || id === selected) return; selected = id; diff --git a/app/shell/session.js b/app/shell/session.js index 2634a09a..e9a4e850 100644 --- a/app/shell/session.js +++ b/app/shell/session.js @@ -8,6 +8,41 @@ // live during a round trip, and a socket listening before anyone knew which run // was ours. +import { DONE, FAILED, CANCELLED } from "./run.js"; + +const TERMINAL = new Set([DONE, FAILED, CANCELLED]); + +/** + * Resolves with the phase a run ends at -- done, failed, or cancelled. + * + * Subscribe FIRST, before whatever starts the run: subscribing after risks + * missing the very transition being waited for. subscribe() delivers the + * CURRENT state immediately, which is not a new transition and is not what + * this is waiting for -- if a page reload adopts a run that already finished, + * that first delivery would resolve this before the caller's own "start" + * step has even run. Only a state delivered AFTER this call counts. + * + * The returned promise carries its own `.cancel()`: subscribing before the + * caller knows whether a run will actually start is the whole point (see + * above), but a start attempt that gets refused before it ever reaches + * run.start() -- an incomplete pipeline, a queue that says no -- never + * transitions the run at all, so nothing would ever resolve this and the + * subscription would sit on `run` forever. Call `.cancel()` when the attempt + * this was waiting on turns out never to have started. + */ +export function waitForTerminal(run) { + let unsubscribe = () => {}; + const promise = new Promise((resolve) => { + let first = true; + unsubscribe = run.subscribe((state) => { + if (first) { first = false; return; } + if (TERMINAL.has(state.phase)) { unsubscribe(); resolve(state.phase); } + }); + }); + promise.cancel = () => unsubscribe(); + return promise; +} + /** * onGenerate: ask what to queue, then queue it, once at a time. * diff --git a/app/shell/tests/bin.test.js b/app/shell/tests/bin.test.js index 51d92c36..6e47b24c 100644 --- a/app/shell/tests/bin.test.js +++ b/app/shell/tests/bin.test.js @@ -99,6 +99,30 @@ test("the newest of a batch is the one shown", () => { assert.deepEqual(b.items.map((i) => i.label), ["three.png", "two.png", "one.png"]); }); +test("open: false adds a file to the bin without moving what the viewer shows", () => { + // A save action's own round trip (an upload) can finish well after the user + // has moved on to looking at something else -- absorb()'s normal auto-open + // exists for a RUN's own result appearing, not for that. `open: false` is + // what a save uses instead. + const b = bin(); + b.absorb([file("already-open.png")]); // the normal path: this IS opened + assert.equal(b.opened.length, 1); + + b.absorb([file("saved-later.png")], { open: false }); + + assert.equal(b.items.length, 2, "the file was not added to the bin at all"); + assert.equal(b.opened.length, 1, "opened a second time despite open: false"); +}); + +test("open: false still dedupes -- a file already in the bin is not added twice", () => { + const b = bin(); + b.absorb([file("a.png")]); + const added = b.absorb([file("a.png")], { open: false }); + + assert.equal(added.length, 0); + assert.equal(b.items.length, 1); +}); + test("a video is known as one, and never becomes a