From 9ef2707a363aa1c9e26de27a4ff29c1dc422ea3f Mon Sep 17 00:00:00 2001 From: digital-garbage <133010426+digital-garbage@users.noreply.github.com> Date: Sun, 6 Sep 2026 13:32:08 +0300 Subject: [PATCH] app: undo, an Edit menu, and a way back to a layout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First off the port list. Every edit in v5 was final -- v4 has had undo since store.js, which is why it never showed up in a file-by-file inventory: there is no undo.js to find. A project is plain JSON and a few kilobytes, so undo is a snapshot before each edit rather than an inverse per action. Capped at 40. The SELECTION travels with the snapshot: undoing a removal and landing on a different scene than the one that came back is not what anyone means by "take that back". Two things it deliberately does not do. A new edit ends the redo line, or a redo walks into a version that never followed from what is on screen. And a project just opened has no past -- otherwise undo reaches back across a project switch and replaces the open project with a version of a different one. Edit menu: undo, redo, add and delete scene, move clip either way. Built on each press so Undo is offered only when there is something behind it; a menu that always offers it teaches people it does nothing. ⌘Z and ⇧⌘Z do the same, except inside a text box, where the browser's own undo is the right one -- taking it over means one keystroke un-typing a paragraph instead of a word. View gains Reset layout, for the person who closed both panels and cannot remember what brings them back. --- app/boot.js | 30 +++++++ app/shell/layout.js | 1 + app/shell/menubar.js | 26 +++++- app/shell/projects.js | 57 ++++++++++++ app/shell/tests/projects.test.js | 116 +++++++++++++++++++++++++ app/tests-layout/project_video.spec.js | 56 ++++++++++++ 6 files changed, 283 insertions(+), 3 deletions(-) diff --git a/app/boot.js b/app/boot.js index afba6977..cc4a7781 100644 --- a/app/boot.js +++ b/app/boot.js @@ -106,6 +106,22 @@ async function start() { onGenerate: () => { ranFor = project.selectedId; session.generate(); }, onCancel: () => run.cancel(), onConstructor: () => page.constructor.open(), + // What the Edit menu offers, and what the keyboard reaches. One list, so a + // menu item and its shortcut cannot drift apart. + edits: { + canUndo: () => project.canUndo, + canRedo: () => project.canRedo, + run: (id) => { + if (id === "undo") project.undo(); + else if (id === "redo") project.redo(); + else if (id === "scene") { project.addScene(); showScene(project.selected); } + else if (id === "remove" && project.scenes.length > 1) { + project.removeScene(project.selectedId); + showScene(project.selected); + } else if (id === "earlier") project.move(project.selectedId, -1); + else if (id === "later") project.move(project.selectedId, 1); + }, + }, projects: () => project.recent, currentProject: () => (project.project ? project.project.id : null), onProject: async (id) => { @@ -153,6 +169,20 @@ async function start() { ranFor = null; }); + // Undo from the keyboard, which is where anyone will reach for it first. + // + // Not inside a field: a text box has its own undo stack and taking that over + // would mean one keystroke un-typing a whole paragraph instead of a word. + window.addEventListener("keydown", (event) => { + if (!(event.metaKey || event.ctrlKey) || event.key.toLowerCase() !== "z") return; + const on = event.target; + const typing = on && (on.tagName === "INPUT" || on.tagName === "TEXTAREA" || on.isContentEditable); + if (typing) return; + event.preventDefault(); + if (event.shiftKey) project.redo(); + else project.undo(); + }); + // Saved on the way out. A debounce that has not fired yet is work the user // did and cannot see anywhere, and a reload is exactly when it is lost. window.addEventListener("pagehide", () => { diff --git a/app/shell/layout.js b/app/shell/layout.js index 625df9cc..0464a71a 100644 --- a/app/shell/layout.js +++ b/app/shell/layout.js @@ -188,6 +188,7 @@ export function build(root, handlers = {}) { workspace, onPipeline: handlers.onPipeline, onProject: handlers.onProject, + edits: handlers.edits, projects: handlers.projects, current: handlers.currentProject, }); diff --git a/app/shell/menubar.js b/app/shell/menubar.js index 7ba3d8a4..772abc23 100644 --- a/app/shell/menubar.js +++ b/app/shell/menubar.js @@ -70,7 +70,8 @@ function menu(label, itemsOf, onPick) { } export function createMenubar({ workspace, onPipeline, onProject, projects = () => [], - current = () => null, theme = services.theme } = {}) { + current = () => null, edits = null, + theme = services.theme } = {}) { const connection = createConnection(); const file = menu("File", () => [ @@ -81,10 +82,25 @@ export function createMenubar({ workspace, onPipeline, onProject, projects = () ...projects().map((p) => ({ id: p.id, label: p.name, icon: p.id === current() ? "✓" : " " })), ], (id) => { if (onProject) onProject(id); }); + // Built on each press, so "Undo" is offered only when there is something to + // undo -- a menu that always offers it teaches people it does nothing. + const edit = edits && menu("Edit", () => [ + { id: "undo", label: "Undo", hint: "⌘Z", disabled: !edits.canUndo() }, + { id: "redo", label: "Redo", hint: "⇧⌘Z", disabled: !edits.canRedo() }, + { separator: true }, + { id: "scene", label: "Add scene" }, + { id: "remove", label: "Delete scene" }, + { separator: true }, + { id: "earlier", label: "Move clip left" }, + { id: "later", label: "Move clip right" }, + ], (id) => edits.run(id)); + 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 }, + { id: "reset", label: "Reset layout" }, + { separator: true }, ...THEMES.map((t) => ({ ...t, // A tick rather than a disabled row: which one is on is a fact about the @@ -93,6 +109,9 @@ export function createMenubar({ workspace, onPipeline, onProject, projects = () })), ], (id) => { if (id === "left" || id === "right") workspace.toggle(id); + // Cheap insurance: a person who has closed both panels and forgotten which + // control brings them back has no way home otherwise. + else if (id === "reset") { workspace.open("left"); workspace.open("right"); } else theme.set(id); }); @@ -102,9 +121,10 @@ export function createMenubar({ workspace, onPipeline, onProject, projects = () const bar = composer.toolbar.default({ label: "FunPack", - items: [composer.brand.default({ name: "FunPack" }), file, view, settings], + items: [composer.brand.default({ name: "FunPack" }), + ...[file, edit, view, settings].filter(Boolean)], trailing: connection.items, }); - return { node: bar.node, bar, connection, file, view, settings }; + return { node: bar.node, bar, connection, file, edit, view, settings }; } diff --git a/app/shell/projects.js b/app/shell/projects.js index 862f6eaa..1ed4a607 100644 --- a/app/shell/projects.js +++ b/app/shell/projects.js @@ -60,15 +60,57 @@ export function createProject({ onChange, onError, onOpen } = {}) { // startup and kept in step here rather than re-fetched each time the menu // opens: a listing is not a thing that changes behind the user's back. let recent = []; + // Undo, the cheap way: a project is plain JSON and a few kilobytes, so a + // snapshot before each edit costs nothing worth measuring and needs no + // per-action inverse. Bounded, or a long session keeps every version of a + // project it has ever had. + // ponytail: whole-project snapshots, capped at 40. Per-field diffs only if a + // project ever gets big enough for the copy to show up in a profile. + const LIMIT = 40; + let past = []; + let future = []; + // The selection travels with the snapshot. It is view state and not saved, + // but undoing a removal and landing on a different scene than the one that + // came back is not what anyone means by "take that back". + const copy = () => ({ project: JSON.parse(JSON.stringify(project)), selected }); let timer = null; let saving = null; // the PUT in flight, so a queued one waits for it let dirty = false; const changed = () => { if (onChange) onChange(); }; + + /** Before an edit. Anything that changes what is SAVED calls this; selecting + * a scene does not, because a view is not an edit. */ + function remember() { + if (!project) return; + past.push(copy()); + if (past.length > LIMIT) past.shift(); + // A new edit ends the redo line. Keeping it would let a user redo their way + // into a version that never followed from what they are looking at. + future = []; + } + + function step(from, to) { + if (!project || !from.length) return false; + to.push(copy()); + const was = from.pop(); + project = was.project; + selected = was.selected; + // The scene that was selected may be gone -- stepping FORWARD into a state + // where it had been removed. Fall back rather than leaving a selection that + // names nothing, which reads as "no scene" in every panel that draws it. + if (!(project.scenes || []).some((sc) => sc.id === selected)) { + selected = (project.scenes || [])[0]?.id ?? null; + } + scheduleSave(); + opened(); // the project was REPLACED, not edited + return true; + } // A DIFFERENT project is open now -- which is not the same event as a field // changing, and the things that follow a project are not the things that // follow an edit. Fired by every route in: start, open, new. const opened = () => { changed(); if (onOpen) onOpen(project); }; + const forget = () => { past = []; future = []; }; function scheduleSave() { dirty = true; @@ -113,6 +155,7 @@ export function createProject({ onChange, onError, onOpen } = {}) { project = found.length ? await read(found[0].id) : await create("Untitled"); if (!found.length) recent = [{ id: project.id, name: project.name }]; selected = (project.scenes || [])[0]?.id ?? null; + forget(); // a project just opened has no past opened(); return project; }, @@ -121,6 +164,7 @@ export function createProject({ onChange, onError, onOpen } = {}) { await flush(); project = await read(id); selected = (project.scenes || [])[0]?.id ?? null; + forget(); // a project just opened has no past opened(); return project; }, @@ -130,6 +174,7 @@ export function createProject({ onChange, onError, onOpen } = {}) { project = await create(name); recent = [{ id: project.id, name: project.name }, ...recent]; selected = (project.scenes || [])[0]?.id ?? null; + forget(); // a project just opened has no past opened(); return project; }, @@ -145,6 +190,7 @@ export function createProject({ onChange, onError, onOpen } = {}) { rename(name) { const clean = String(name || "").trim(); if (!project || !clean || project.name === clean) return; + remember(); project.name = clean; const listed = recent.find((p) => p.id === project.id); if (listed) listed.name = clean; @@ -159,6 +205,7 @@ export function createProject({ onChange, onError, onOpen } = {}) { if (!project) return; project.video = project.video || {}; if (project.video[key] === value) return; + remember(); project.video[key] = value; scheduleSave(); // The timeline's clip widths and its ruler are computed from the project's @@ -176,6 +223,7 @@ export function createProject({ onChange, onError, onOpen } = {}) { // A new scene lands AFTER the selected one, not at the end: adding partway // through a timeline is how a scene gets inserted, and appending silently // would put it somewhere the user was not looking. + remember(); const at = project.scenes.findIndex((s) => s.id === selected); const scene = { id: newId(), text: "", result: null, length: null, rating: null }; project.scenes.splice(at < 0 ? project.scenes.length : at + 1, 0, scene); @@ -187,6 +235,7 @@ export function createProject({ onChange, onError, onOpen } = {}) { removeScene(id) { if (!project || !sceneAt(id)) return; + remember(); const at = project.scenes.findIndex((s) => s.id === id); project.scenes.splice(at, 1); if (selected === id) { @@ -202,6 +251,7 @@ export function createProject({ onChange, onError, onOpen } = {}) { const at = project.scenes.findIndex((s) => s.id === id); const to = at + by; if (at < 0 || to < 0 || to >= project.scenes.length) return; + remember(); const [scene] = project.scenes.splice(at, 1); project.scenes.splice(to, 0, scene); scheduleSave(); @@ -213,6 +263,7 @@ export function createProject({ onChange, onError, onOpen } = {}) { setScene(id, key, value) { const scene = sceneAt(id); if (!scene || scene[key] === value) return; + remember(); scene[key] = value; scheduleSave(); changed(); @@ -221,6 +272,12 @@ export function createProject({ onChange, onError, onOpen } = {}) { /** Attach what a run produced to the scene it was started from. */ setResult(id, result) { this.setScene(id, "result", result); }, + /** Step back, or forward. True when something moved. */ + undo() { return step(past, future); }, + redo() { return step(future, past); }, + get canUndo() { return Boolean(project) && past.length > 0; }, + get canRedo() { return Boolean(project) && future.length > 0; }, + flush, get unsaved() { return dirty; }, }; diff --git a/app/shell/tests/projects.test.js b/app/shell/tests/projects.test.js index 96237917..c9fa440b 100644 --- a/app/shell/tests/projects.test.js +++ b/app/shell/tests/projects.test.js @@ -134,3 +134,119 @@ test("changing a project setting tells whoever draws from it", async () => { p.setVideo("length", 97); assert.ok(drew.length > before, "nothing was told the project changed"); }); + +// --- undo --------------------------------------------------------------- + +test("an edit can be taken back, and put back again", async () => { + server(); + const p = createProject({}); + await p.start(); + p.addScene(); + const id = p.selectedId; + + p.setText(id, "a cat"); + p.setText(id, "a cat on a roof"); + assert.equal(p.selected.text, "a cat on a roof"); + + assert.equal(p.undo(), true); + assert.equal(p.selected.text, "a cat"); + assert.equal(p.undo(), true); + assert.equal(p.selected.text, ""); + + assert.equal(p.redo(), true); + assert.equal(p.selected.text, "a cat"); +}); + +test("undo puts back a scene that was removed, and the selection with it", async () => { + server(); + const p = createProject({}); + await p.start(); + p.addScene(); + p.addScene(); + const [first, second] = p.scenes.map((s) => s.id); + p.select(second); + + p.removeScene(second); + assert.equal(p.scenes.length, 1); + + p.undo(); + assert.equal(p.scenes.length, 2); + assert.ok(p.scenes.some((s) => s.id === second), "the scene came back as a different one"); + assert.equal(p.selectedId, second, "the selection did not follow the scene back"); + void first; +}); + +test("a selection that no longer names anything falls back rather than emptying", async () => { + // Undo can remove the scene that is selected. A selection naming nothing + // reads as "no scene" in every panel that draws from it. + server(); + const p = createProject({}); + await p.start(); + p.addScene(); + p.addScene(); + const added = p.selectedId; + + p.undo(); // takes the second scene away again + assert.ok(p.selectedId, "the selection was left naming nothing"); + assert.notEqual(p.selectedId, added); + assert.ok(p.scenes.some((s) => s.id === p.selectedId)); +}); + +test("a new edit ends the redo line", async () => { + // Otherwise a redo walks into a version that never followed from what is on + // screen. + server(); + const p = createProject({}); + await p.start(); + p.addScene(); + const id = p.selectedId; + + p.setText(id, "one"); + p.undo(); + assert.equal(p.canRedo, true); + + p.setText(id, "two"); + assert.equal(p.canRedo, false); + assert.equal(p.redo(), false); +}); + +test("undo with nothing behind it does nothing, and says so", async () => { + server(); + const p = createProject({}); + await p.start(); + assert.equal(p.canUndo, false); + assert.equal(p.undo(), false); + assert.equal(p.redo(), false); +}); + +test("a project just opened has no past to step into", async () => { + // Undoing across a project switch would replace the open project with a + // version of a DIFFERENT one. + server(); + const p = createProject({}); + await p.start(); + p.addScene(); + assert.equal(p.canUndo, true); + + await p.newProject("Another"); + assert.equal(p.canUndo, false, "undo reached back past the project switch"); +}); + +test("taking an edit back saves it, and tells whoever draws it", async () => { + const { sent } = server(); + const drew = []; + const opens = []; + const p = createProject({ onChange: () => drew.push(1), onOpen: () => opens.push(1) }); + await p.start(); + p.addScene(); + p.setText(p.selectedId, "a cat"); + await p.flush(); + + const before = drew.length; + p.undo(); + assert.ok(drew.length > before, "nothing was told the project moved"); + assert.ok(opens.length > 1, "the controls that follow a project were not put back"); + + await p.flush(); + assert.equal(sent.at(-1).scenes.at(-1).text, "", "the version saved was not the one on screen"); +}); diff --git a/app/tests-layout/project_video.spec.js b/app/tests-layout/project_video.spec.js index 9030986e..243856a9 100644 --- a/app/tests-layout/project_video.spec.js +++ b/app/tests-layout/project_video.spec.js @@ -156,3 +156,59 @@ test("switching projects switches what the run will use, not just what is stored await openConstructor(page); await expect(widthBox(page)).toHaveValue("640"); }); + +test("an edit can be taken back from the menu and from the keyboard", async ({ page }) => { + await page.goto("/funpack/"); + await page.waitForFunction(() => window.FunPack !== undefined); + + const scenes = () => page.evaluate(() => window.FunPack.project.scenes.length); + const before = await scenes(); + + await page.getByRole("button", { name: "Edit" }).click(); + await page.getByRole("menuitem", { name: "Add scene" }).click(); + expect(await scenes()).toBe(before + 1); + + await page.getByRole("button", { name: "Edit" }).click(); + await page.getByRole("menuitem", { name: "Undo" }).click(); + expect(await scenes(), "the menu did not undo").toBe(before); + + // And the shortcut anyone reaches for first. + await page.getByRole("button", { name: "Edit" }).click(); + await page.getByRole("menuitem", { name: "Add scene" }).click(); + expect(await scenes()).toBe(before + 1); + await page.keyboard.press("ControlOrMeta+z"); + expect(await scenes(), "the keyboard did not undo").toBe(before); + await page.keyboard.press("ControlOrMeta+Shift+z"); + expect(await scenes(), "redo did not put it back").toBe(before + 1); +}); + +test("Undo is not offered when there is nothing behind it", async ({ page }) => { + // A menu that always offers it teaches people it does nothing. + await page.goto("/funpack/"); + await page.waitForFunction(() => window.FunPack !== undefined); + + await page.getByRole("button", { name: "Edit" }).click(); + await expect(page.getByRole("menuitem", { name: "Undo" })).toBeDisabled(); + await page.keyboard.press("Escape"); + + await page.getByRole("button", { name: "Edit" }).click(); + await page.getByRole("menuitem", { name: "Add scene" }).click(); + await page.getByRole("button", { name: "Edit" }).click(); + await expect(page.getByRole("menuitem", { name: "Undo" })).toBeEnabled(); +}); + +test("typing in a box keeps its own undo", async ({ page }) => { + // Taking the shortcut over inside a field means one keystroke un-typing a + // paragraph instead of a word. + await page.goto("/funpack/"); + await page.waitForFunction(() => window.FunPack !== undefined); + const scenes = await page.evaluate(() => window.FunPack.project.scenes.length); + + await page.locator(".cx-panel-head").getByRole("button", { name: "Constructor" }).click(); + const box = page.locator(".cx-modal textarea").first(); + await box.click(); + await page.keyboard.press("ControlOrMeta+z"); + + expect(await page.evaluate(() => window.FunPack.project.scenes.length), + "the project was undone from inside a text box").toBe(scenes); +});