app: undo, an Edit menu, and a way back to a layout

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

@@ -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", () => {

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,
   });

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 };
 }

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; },
   };

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");
+});

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);
+});