diff --git a/app/catalogue/demos.js b/app/catalogue/demos.js index 3cf4d768..2b538c93 100644 --- a/app/catalogue/demos.js +++ b/app/catalogue/demos.js @@ -334,6 +334,11 @@ export const DEMOS = { { at: 0, label: "1", hint: "0" }, { at: 0.4, label: "2", hint: "96" }, { at: 0.75, label: "3", hint: "180" }, ] }, + "track.default": { label: "Timeline", pxPerSecond: 40, playhead: 6.5, selection: ["2"], items: [ + { id: "1", icon: "▦", label: "Scene 1", badge: "1", start: 0, duration: 4 }, + { id: "2", icon: "▦", label: "Scene 2", badge: "2", start: 4, duration: 6, rating: "liked" }, + { id: "3", icon: "▦", label: "Scene 3", badge: "3", start: 10, duration: 3, excluded: true }, + ] }, "wheel.half": { edge: "right", items: [ { icon: "✂", label: "Split" }, { icon: "⧉", label: "Duplicate" }, { icon: "★", label: "Rate" }, { icon: "◐", label: "Anchor" }, diff --git a/app/composer/composer.css b/app/composer/composer.css index 6d74b0f3..ff344820 100644 --- a/app/composer/composer.css +++ b/app/composer/composer.css @@ -14,4 +14,5 @@ @import "./elements/status.css"; @import "./elements/overlay.css"; @import "./elements/gallery.css"; +@import "./elements/track.css"; @import "./elements/wheel.css"; diff --git a/app/composer/elements/gallery.js b/app/composer/elements/gallery.js index 624eb788..6b1b4f6f 100644 --- a/app/composer/elements/gallery.js +++ b/app/composer/elements/gallery.js @@ -12,6 +12,7 @@ import { define } from "../internals/register.js"; import { el } from "../internals/el.js"; import { applyDensity, recallCols, rememberCols, normaliseCols } from "../internals/density.js"; +import { wireDragReorder } from "../internals/dragReorder.js"; const EMPTY = "Nothing here yet"; @@ -222,45 +223,7 @@ define("gallery", "strip", ({ onReorder, ...props } = {}) => { return cell; }); - // Delegated on the container rather than per-cell: cells are torn down and - // rebuilt on every setItems/setValue, and listeners bound to them would have - // to be reattached every time. One set here survives all of that. - // - // The dragged item's id lives on `dataTransfer`, not in a closure variable. - // A closure var tracks by NOTHING the browser understands, so nothing keeps - // it in step with the actual drag session: `dragend` fires on the ORIGINAL - // source element, and a redraw mid-drag (an unrelated remove/undo firing on - // its own input channel while the mouse is still held down) can detach that - // element from the tree -- its `dragend` then never bubbles here, and a - // closure var set at dragstart is never cleared, ready to be picked up by - // the next unrelated drop that lands on this strip from somewhere else - // entirely. `dataTransfer` has no such gap: it belongs to the actual OS-level - // drag session, unaffected by which DOM node currently receives the events, - // and a drop from any OTHER session simply never had this MIME type set. - const MIME = "text/x-funpack-reorder"; - const cellOf = (e) => e.target.closest(".cx-strip-cell"); - node.addEventListener("dragstart", (e) => { - const cell = cellOf(e); - if (!cell || !e.dataTransfer) return; - e.dataTransfer.effectAllowed = "move"; - e.dataTransfer.setData(MIME, cell.dataset.id || ""); - cell.classList.add("cx-dragging"); - }); - node.addEventListener("dragend", (e) => { cellOf(e)?.classList.remove("cx-dragging"); }); - node.addEventListener("dragover", (e) => { - // `getData` only returns the real value on "drop" (a browser security - // restriction during drag) -- `types` is what dragover can actually read. - if (e.dataTransfer && e.dataTransfer.types.includes(MIME)) e.preventDefault(); - }); - node.addEventListener("drop", (e) => { - if (!e.dataTransfer) return; - const draggedId = e.dataTransfer.getData(MIME); - if (!draggedId) return; - e.preventDefault(); - const cell = cellOf(e); - const targetId = cell ? cell.dataset.id : null; - if (onReorder && targetId && targetId !== draggedId) onReorder(draggedId, targetId); - }); + wireDragReorder(node, ".cx-strip-cell", onReorder); return { node, diff --git a/app/composer/elements/index.js b/app/composer/elements/index.js index ef005a1e..78e56aa3 100644 --- a/app/composer/elements/index.js +++ b/app/composer/elements/index.js @@ -16,4 +16,5 @@ import "./modal.js"; import "./popover.js"; import "./floating.js"; import "./gallery.js"; +import "./track.js"; import "./wheel.js"; diff --git a/app/composer/elements/track.css b/app/composer/elements/track.css new file mode 100644 index 00000000..f85501f4 --- /dev/null +++ b/app/composer/elements/track.css @@ -0,0 +1,50 @@ +/* A real timeline: a ruler in seconds, clips as wide as they play for, and a + * playhead over both. Pixel-positioned rather than percent/flex-grow -- what + * makes the ruler's ticks and the clips underneath actually line up. */ +.cx-track { overflow-x: auto; min-inline-size: 0; } +.cx-track-inner { position: relative; min-inline-size: 100%; } + +.cx-track-ruler { + position: relative; block-size: var(--lh-8); min-inline-size: 100%; + border-block-end: var(--bw) solid var(--line-2); +} +.cx-track-tick { + position: absolute; inset-block-start: 0; + padding-inline-start: var(--sp-1); + border-inline-start: var(--bw) solid var(--line); + font-size: var(--fs-8); line-height: var(--lh-8); color: var(--faint); + font-variant-numeric: tabular-nums; +} + +/* A fixed row height, not aspect-ratio off the clip's own width -- a clip's + width now tracks real duration and can run to hundreds of pixels, and + 16:10 off THAT would make a half-minute scene taller than the window. */ +.cx-track-clips { display: flex; min-inline-size: 100%; block-size: 45px; } +.cx-track-clip { + flex: none; padding: 0; block-size: 100%; + border: var(--bw) solid var(--line); border-radius: var(--r-2); + background: var(--void); overflow: hidden; cursor: grab; + display: flex; align-items: center; justify-content: center; +} +.cx-track-clip + .cx-track-clip { border-inline-start: none; } +.cx-track-face { position: relative; inline-size: 100%; block-size: 100%; display: flex; align-items: center; justify-content: center; } +.cx-track-clip:hover { border-color: var(--accent); } +.cx-track-clip.cx-on { border-color: var(--accent); box-shadow: inset 0 0 0 1px var(--accent); } +.cx-track-clip.cx-dragging { opacity: 0.4; cursor: grabbing; } +/* Still on the timeline, still there to bring back -- excluded from a batch + run reads as dimmed, not gone, the way a deleted scene would be. */ +.cx-track-clip.cx-excluded { opacity: 0.45; } +/* A clip carries its rating as a colour on its edge -- the one thing about a + result you want to see without selecting it. */ +.cx-track-clip.cx-rated { border-color: var(--muted); } +.cx-track-clip[data-rating="liked"] { border-color: var(--good); } +.cx-track-clip[data-rating="disliked"] { border-color: var(--danger); } + +/* Spans both rows, so where the ruler reads and where the clip sits are + visibly the same point -- a plain marker on just one of them would not + read as "this is where you are" the way a line through both does. */ +.cx-track-playhead { + position: absolute; inset-block: 0; inline-size: 0; + border-inline-start: 2px solid var(--accent-hi); + pointer-events: none; z-index: 1; +} diff --git a/app/composer/elements/track.js b/app/composer/elements/track.js new file mode 100644 index 00000000..de28ccc4 --- /dev/null +++ b/app/composer/elements/track.js @@ -0,0 +1,164 @@ +// A single time-based track: clips laid out by real duration, a ruler in +// seconds, and a playhead -- the shape of an actual video timeline, not a +// filmstrip of equal-sized thumbnails with a tick per item. +// +// One track, on purpose: v5 generates one clip per scene, in order, with no +// separate audio/overlay layers to line up against it. A second track is a +// different, larger feature (waveforms, compositing, independent trims) -- +// this is only ever asked to place things that already play back to back. + +import { define } from "../internals/register.js"; +import { el } from "../internals/el.js"; +import { wireDragReorder } from "../internals/dragReorder.js"; + +/** A clip's face: the result so far, or the glyph that stands in for one -- + * the same visual language cx-cell-thumb uses elsewhere in the kit, but + * filling the clip's own fixed-height box rather than a 16:10 box of its + * own: a clip's width tracks real duration, and that box can run to + * hundreds of pixels wide. */ +function faceOf(item) { + const thumb = el("div", { cls: "cx-track-face" }); + const glyph = () => el("span", { cls: "cx-cell-glyph", text: item.icon || "▦", attrs: { "aria-hidden": "true" } }); + if (item.thumb) { + const img = el("img", { cls: "cx-cell-img", attrs: { src: item.thumb, alt: "", loading: "lazy" } }); + // A thumbnail that fails to load falls back to the glyph, not the + // browser's broken-image icon -- which would read as a damaged file. + img.addEventListener("error", () => img.replaceWith(glyph())); + thumb.append(img); + } else { + thumb.append(glyph()); + } + if (item.badge) thumb.append(el("span", { cls: "cx-cell-badge", text: item.badge })); + return thumb; +} + +// A tick every 1/2/5/10... seconds, whichever is the smallest that still +// leaves room to read the label beside it. Fixed intervals rather than a tick +// per clip: what a person reads off a ruler is elapsed TIME, and a tick per +// scene is the scene-index ruler this replaces. +const NICE_INTERVALS = [1, 2, 5, 10, 15, 30, 60, 120, 300, 600, 900, 1800, 3600]; +const MIN_TICK_PX = 56; + +function tickInterval(pxPerSecond) { + for (const s of NICE_INTERVALS) if (s * pxPerSecond >= MIN_TICK_PX) return s; + return NICE_INTERVALS[NICE_INTERVALS.length - 1]; +} + +function formatTime(seconds) { + const whole = Math.max(0, Math.round(seconds)); + const m = Math.floor(whole / 60); + const s = whole % 60; + return `${m}:${String(s).padStart(2, "0")}`; +} + +// A floor, not a cap: a scene under a second would otherwise render as a +// sliver nothing could click or even see next to a normal one. Everything +// else stays exactly proportional -- this only rescues the degenerate case. +const MIN_CLIP_PX = 24; + +/** + * track.default({ items, pxPerSecond, selection, playhead, label, + * onSeek, onReorder }) + * + * `items`: [{ id, label, thumb, icon, badge, rating, excluded, start, + * duration }] -- start/duration in SECONDS, contiguous (each + * item's start is the previous one's start + duration), which is what lets + * one click handler double as both "seek" and "select": every point on the + * track belongs to exactly one clip. The track's own span is read off the + * items themselves (the last one's start + duration) -- nothing else here + * knows the project's length. + * + * `onSeek(seconds, item)` fires on any click on the track -- the ruler, the + * empty background, or a clip. `item` is whichever clip's span the point + * (or, for a clip itself, the clip clicked) falls in. + */ +define("track", "default", ({ + items = [], pxPerSecond: initialPxPerSecond = 40, selection = [], playhead = 0, + label, onSeek, onReorder, +} = {}) => { + let pxPerSecond = initialPxPerSecond; + const chosen = new Set(selection); + let current = items; + const spanOf = () => current.reduce((max, i) => Math.max(max, i.start + i.duration), 0); + + const node = el("div", { cls: "cx-track", attrs: { role: "presentation", "aria-label": label } }); + const inner = el("div", { cls: "cx-track-inner" }); + const ruler = el("div", { cls: "cx-track-ruler", attrs: { role: "presentation" } }); + const clips = el("div", { cls: "cx-track-clips", attrs: { role: "listbox", "aria-label": label } }); + const head = el("div", { cls: "cx-track-playhead", attrs: { "aria-hidden": "true" } }); + inner.append(ruler, clips, head); + node.append(inner); + + function drawRuler(span) { + ruler.replaceChildren(); + const interval = tickInterval(pxPerSecond); + for (let t = 0; t <= span; t += interval) { + const tick = el("span", { cls: "cx-track-tick", text: formatTime(t) }); + tick.style.insetInlineStart = `${t * pxPerSecond}px`; + ruler.append(tick); + } + } + + function drawClips() { + clips.replaceChildren(); + for (const item of current) { + const on = chosen.has(item.id); + const cell = el("button", { + cls: ["cx-track-clip", on ? "cx-on" : null, item.rating ? "cx-rated" : null, + item.excluded ? "cx-excluded" : null, "cx-focusable"], + attrs: { + type: "button", role: "option", "aria-selected": String(on), title: item.label, + "aria-label": item.label, "data-rating": item.rating || undefined, + "data-id": item.id, + // Draggable whether or not `onReorder` is given, matching the + // strip: a drag with nowhere to report to is inert, not wired. + draggable: "true", + }, + children: [faceOf(item)], + }); + cell.style.width = `${Math.max(MIN_CLIP_PX, item.duration * pxPerSecond)}px`; + clips.append(cell); + } + } + + function setPlayhead(seconds) { + head.style.insetInlineStart = `${Math.max(0, seconds) * pxPerSecond}px`; + } + + function draw() { + const span = spanOf(); + inner.style.width = `${span * pxPerSecond}px`; + drawRuler(span); + drawClips(); + setPlayhead(playhead); + } + + wireDragReorder(clips, ".cx-track-clip", onReorder); + + // One handler for the whole track: a click's OWN position is turned into a + // time, the same way a scrub bar would, whether it landed on the ruler, the + // empty background, or a clip -- a clip covers most of the track's area, so + // reading its id instead of where on it the click landed would make the + // ruler the only place a real, exact-second seek ever reaches. Keyboard + // activation (Enter/Space on a focused clip) still resolves correctly: the + // browser reports the click at that button's own on-screen position, which + // is well inside the right clip's span even if not at its exact start. + inner.addEventListener("click", (e) => { + if (!onSeek) return; + const rect = inner.getBoundingClientRect(); + const seconds = Math.min(spanOf(), Math.max(0, (e.clientX - rect.left) / pxPerSecond)); + const item = current.find((i) => seconds < i.start + i.duration) || current[current.length - 1] || null; + onSeek(seconds, item); + }); + + draw(); + + return { + node, + setItems(next = []) { current = next; draw(); }, + setValue(next = []) { chosen.clear(); for (const id of next) chosen.add(id); drawClips(); }, + setPlayhead(seconds) { playhead = seconds; setPlayhead(seconds); }, + setZoom(next) { pxPerSecond = next; draw(); }, + destroy: () => node.remove(), + }; +}); diff --git a/app/composer/internals/dragReorder.js b/app/composer/internals/dragReorder.js new file mode 100644 index 00000000..e0e93af2 --- /dev/null +++ b/app/composer/internals/dragReorder.js @@ -0,0 +1,46 @@ +// Reordering a row of id-tagged children by drag, shared by every element that +// has one (the scene strip, the timeline track). +// +// Delegated on the container: cells are torn down and rebuilt on every redraw, +// and listeners bound to them would have to be reattached every time. One set +// here survives all of that. +// +// The dragged item's id lives on `dataTransfer`, not in a closure variable. A +// closure var tracks by NOTHING the browser understands, so nothing keeps it in +// step with the actual drag session: `dragend` fires on the ORIGINAL source +// element, and a redraw mid-drag (an unrelated remove/undo firing on its own +// input channel while the mouse is still held down) can detach that element +// from the tree -- its `dragend` then never bubbles here, and a closure var set +// at dragstart is never cleared, ready to be picked up by the next unrelated +// drop that lands on this row from somewhere else entirely. `dataTransfer` has +// no such gap: it belongs to the actual OS-level drag session, unaffected by +// which DOM node currently receives the events, and a drop from any OTHER +// session simply never had this MIME type set. +const MIME = "text/x-funpack-reorder"; + +/** wireDragReorder(node, ".cx-cell-selector", (draggedId, targetId) => {}) */ +export function wireDragReorder(node, cellSelector, onReorder) { + const cellOf = (e) => e.target.closest(cellSelector); + node.addEventListener("dragstart", (e) => { + const cell = cellOf(e); + if (!cell || !e.dataTransfer) return; + e.dataTransfer.effectAllowed = "move"; + e.dataTransfer.setData(MIME, cell.dataset.id || ""); + cell.classList.add("cx-dragging"); + }); + node.addEventListener("dragend", (e) => { cellOf(e)?.classList.remove("cx-dragging"); }); + node.addEventListener("dragover", (e) => { + // `getData` only returns the real value on "drop" (a browser security + // restriction during drag) -- `types` is what dragover can actually read. + if (e.dataTransfer && e.dataTransfer.types.includes(MIME)) e.preventDefault(); + }); + node.addEventListener("drop", (e) => { + if (!e.dataTransfer) return; + const draggedId = e.dataTransfer.getData(MIME); + if (!draggedId) return; + e.preventDefault(); + const cell = cellOf(e); + const targetId = cell ? cell.dataset.id : null; + if (onReorder && targetId && targetId !== draggedId) onReorder(draggedId, targetId); + }); +} diff --git a/app/composer/tests/elements_behaviour.test.js b/app/composer/tests/elements_behaviour.test.js index 09b9f259..52cb493d 100644 --- a/app/composer/tests/elements_behaviour.test.js +++ b/app/composer/tests/elements_behaviour.test.js @@ -498,6 +498,124 @@ test("a bin's thumbnails are lazy", () => { } }); +// --- track ------------------------------------------------------------- + +const CLIPS = [ + { id: "a", label: "Scene 1", start: 0, duration: 4 }, + { id: "b", label: "Scene 2", start: 4, duration: 6 }, + { id: "c", label: "Scene 3", start: 10, duration: 2 }, +]; + +test("a clip's width is proportional to its duration, not equal to its neighbours'", () => { + // The third clip is 2s at 10px/s = 20px, under the 24px floor -- widened to + // stay clickable, which is why it is not exactly proportional to the other + // two the way the first two are to each other. + const t = mount(composer.track.default({ items: CLIPS, pxPerSecond: 10 })); + const widths = [...t.node.querySelectorAll(".cx-track-clip")].map((c) => parseFloat(c.style.width)); + assert.deepEqual(widths, [40, 60, 24]); +}); + +test("a clip under the pixel floor is not squeezed to nothing", () => { + const t = mount(composer.track.default({ + items: [{ id: "a", label: "Sliver", start: 0, duration: 0.1 }], pxPerSecond: 10, + })); + const width = parseFloat(t.node.querySelector(".cx-track-clip").style.width); + assert.ok(width >= 24, `a 0.1s clip at 10px/s rendered ${width}px wide`); +}); + +test("the ruler reads real elapsed time, not a tick per clip", () => { + // Three clips, but the ruler's ticks are seconds -- there is no reason their + // count should match the clip count in either direction. + const t = mount(composer.track.default({ items: CLIPS, pxPerSecond: 40 })); + const ticks = [...t.node.querySelectorAll(".cx-track-tick")].map((n) => n.textContent); + assert.deepEqual(ticks, ["0:00", "0:02", "0:04", "0:06", "0:08", "0:10", "0:12"]); +}); + +test("clicking partway into a clip seeks to that exact second, not to the clip's start", () => { + // A clip covers most of the track's own area -- if a click on one only + // ever resolved to its START, an exact-second seek would only ever be + // reachable from the thin ruler strip above it. + const seen = []; + const t = mount(composer.track.default({ + items: CLIPS, pxPerSecond: 40, onSeek: (seconds, item) => seen.push([seconds, item && item.id]), + })); + // Clip "b" spans 4s-10s -> 160px-400px at 40px/s. 250px is 6.25s into it. + t.node.querySelector('[data-id="b"]') + .dispatchEvent(new window.MouseEvent("click", { bubbles: true, clientX: 250 })); + assert.deepEqual(seen, [[6.25, "b"]]); +}); + +test("clicking empty track (the ruler, the gap past the clips) seeks by position", () => { + const seen = []; + const t = mount(composer.track.default({ + items: CLIPS, pxPerSecond: 10, onSeek: (seconds, item) => seen.push([seconds, item && item.id]), + })); + const inner = t.node.querySelector(".cx-track-inner"); + inner.dispatchEvent(new window.MouseEvent("click", { bubbles: true, clientX: 45 })); + // 45px at 10px/s = 4.5s, which falls inside clip "b" (4s..10s). + assert.deepEqual(seen, [[4.5, "b"]]); +}); + +test("a seek past the last clip clamps to the track's own end", () => { + const seen = []; + const t = mount(composer.track.default({ + items: CLIPS, pxPerSecond: 10, onSeek: (seconds, item) => seen.push([seconds, item && item.id]), + })); + t.node.querySelector(".cx-track-inner") + .dispatchEvent(new window.MouseEvent("click", { bubbles: true, clientX: 9999 })); + assert.deepEqual(seen, [[12, "c"]]); +}); + +test("setPlayhead moves the marker without redrawing the clips", () => { + const t = mount(composer.track.default({ items: CLIPS, pxPerSecond: 10, playhead: 0 })); + const before = t.node.querySelector(".cx-track-clip"); + t.setPlayhead(7); + const head = t.node.querySelector(".cx-track-playhead"); + assert.equal(head.style.insetInlineStart, "70px"); + assert.equal(t.node.querySelector(".cx-track-clip"), before, "the clip row was rebuilt for a playhead move"); +}); + +test("setValue redraws the selected clip without a fresh setItems call", () => { + const t = mount(composer.track.default({ items: CLIPS, pxPerSecond: 10 })); + assert.equal(t.node.querySelectorAll('[aria-selected="true"]').length, 0); + t.setValue(["b"]); + const on = t.node.querySelector('[aria-selected="true"]'); + assert.equal(on && on.dataset.id, "b"); +}); + +test("dragging a clip onto another reports the reorder by id, not by position", () => { + const reordered = []; + const t = mount(composer.track.default({ items: CLIPS, pxPerSecond: 10, onReorder: (from, to) => reordered.push([from, to]) })); + const cells = t.node.querySelectorAll(".cx-track-clip"); + const store = {}; + const dt = { + effectAllowed: null, + setData: (type, v) => { store[type] = v; }, + getData: (type) => store[type] || "", + get types() { return Object.keys(store); }, + }; + const fireDrag = (node, type) => { + const e = new window.Event(type, { bubbles: true, cancelable: true }); + e.dataTransfer = dt; + node.dispatchEvent(e); + }; + fireDrag(cells[0], "dragstart"); + fireDrag(cells[2], "drop"); + assert.deepEqual(reordered, [["a", "c"]]); +}); + +test("an excluded clip is dimmed, a rated one carries its rating", () => { + const t = mount(composer.track.default({ + items: [ + { id: "a", label: "A", start: 0, duration: 1, excluded: true }, + { id: "b", label: "B", start: 1, duration: 1, rating: "liked" }, + ], pxPerSecond: 10, + })); + const [a, b] = t.node.querySelectorAll(".cx-track-clip"); + assert.equal(a.classList.contains("cx-excluded"), true); + assert.equal(b.getAttribute("data-rating"), "liked"); +}); + test("Enter is a newline in a textarea, not a commit", () => { // A one-line field commits on Enter. A textarea holds the longest thing // anyone types here -- the prompt -- and stealing Enter there means a diff --git a/app/shell/projects.js b/app/shell/projects.js index 1e232d21..1e220870 100644 --- a/app/shell/projects.js +++ b/app/shell/projects.js @@ -368,7 +368,14 @@ export function createProject({ onChange, onError, onOpen } = {}) { // 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 }; + const scene = { + id: newId(), text: "", result: null, length: null, rating: null, + // Matching core's Scene dataclass shape exactly: the Inspector reads + // scene.references unconditionally (a list to map over, never a + // maybe-undefined field), and a scene born here has to look the same + // as one that came back from the server. + source_image: null, references: [], + }; project.scenes.splice(at < 0 ? project.scenes.length : at + 1, 0, scene); selected = scene.id; // you add a scene in order to fill it scheduleSave(); diff --git a/app/shell/tests/projects.test.js b/app/shell/tests/projects.test.js index 2cccb28f..4926fb9b 100644 --- a/app/shell/tests/projects.test.js +++ b/app/shell/tests/projects.test.js @@ -122,6 +122,20 @@ test("setting the negative prompt to what it already is saves nothing", async () assert.equal(sent.length, 1); }); +test("a scene added on the client has the same shape as one the server sent", async () => { + // core's Scene dataclass always has source_image/references -- the + // Inspector reads scene.references unconditionally (a list to map over, + // never a maybe-undefined field). A scene born here that skipped them + // crashed the very next thing that tried to draw it. + server(); + const p = createProject({}); + await p.start(); + p.addScene(); + const scene = p.selected; + assert.equal(scene.source_image, null); + assert.deepEqual(scene.references, []); +}); + test("a run's result lands on the scene it was started for, not the current one", async () => { // A run takes minutes and the user goes on clicking. Reading "the selected // scene" when it finishes attaches the picture to whatever they wandered to. diff --git a/app/shell/tests/timeline.test.js b/app/shell/tests/timeline.test.js index 0b99b7c5..577977d4 100644 --- a/app/shell/tests/timeline.test.js +++ b/app/shell/tests/timeline.test.js @@ -1,4 +1,5 @@ -// The timeline: reorder by drag, and the zoom that changes clip width. +// The timeline: real seconds, drag reorder, and the zoom that changes clip +// width. import test from "node:test"; import assert from "node:assert/strict"; @@ -30,15 +31,17 @@ test.before(async () => { }); test.after(() => teardownDom()); -/** Just enough of the store for the timeline to read and act on. */ -function fakeProject(scenes) { +/** Just enough of the store for the timeline to read and act on. Video length + * is 24 frames at the 24fps fallback -- one second per scene by default, + * which keeps the maths in each test readable. */ +function fakeProject(scenes, video = { length: 24 }) { let selected = scenes[0] ? scenes[0].id : null; const moved = []; return { get scenes() { return scenes; }, get selectedId() { return selected; }, get selected() { return scenes.find((s) => s.id === selected) || null; }, - video: {}, + video, moved, addScene() { const s = { id: `s${scenes.length}`, text: "", result: null, length: null }; scenes.push(s); selected = s.id; }, removeScene(id) { const at = scenes.findIndex((s) => s.id === id); if (at >= 0) scenes.splice(at, 1); }, @@ -50,7 +53,7 @@ function fakeProject(scenes) { const [scene] = scenes.splice(at, 1); scenes.splice(to, 0, scene); }, - select(id) { selected = id; }, + select(id) { if (!scenes.some((s) => s.id === id) || id === selected) return; selected = id; }, }; } @@ -61,7 +64,7 @@ test("dragging a clip onto another reorders by the distance dropped, not just on const t = createTimeline({ project }); document.body.append(t.node); - const cells = t.node.querySelectorAll(".cx-strip-cell"); + const cells = t.node.querySelectorAll(".cx-track-clip"); assert.equal(cells.length, 4); // Drag the first clip (s0) onto the fourth cell's position. @@ -77,7 +80,7 @@ test("dragging a clip onto another reorders by the distance dropped, not just on test("a redraw mid-drag (a remove fired from the keyboard, say) still resolves to the clip actually picked up", () => { // The mouse held down and a keyboard shortcut are two different input - // channels -- nothing stops project.removeScene firing, and the strip + // channels -- nothing stops project.removeScene firing, and the track // redrawing, while a drag is still in flight. Tracking BY POSITION would // have this act on whatever now sits where the drag started, not on what // was actually grabbed. @@ -86,12 +89,12 @@ test("a redraw mid-drag (a remove fired from the keyboard, say) still resolves t document.body.append(t.node); const dt = fakeDataTransfer(); - dragFire(t.node.querySelectorAll(".cx-strip-cell")[1], "dragstart", dt); // grabs s1 + dragFire(t.node.querySelectorAll(".cx-track-clip")[1], "dragstart", dt); // grabs s1 project.removeScene("s0"); // s1 s2 s3 -- s1 is now cell 0 t.draw(); // the redraw a real remove triggers - const after = t.node.querySelectorAll(".cx-strip-cell"); + const after = t.node.querySelectorAll(".cx-track-clip"); dragFire(after[2], "dragover", dt); // drop on whatever cell now shows s3 dragFire(after[2], "drop", dt); @@ -106,7 +109,7 @@ test("dropping a clip on itself does nothing", () => { const t = createTimeline({ project }); document.body.append(t.node); - const cells = t.node.querySelectorAll(".cx-strip-cell"); + const cells = t.node.querySelectorAll(".cx-track-clip"); const dt = fakeDataTransfer(); dragFire(cells[1], "dragstart", dt); dragFire(cells[1], "drop", dt); @@ -120,79 +123,140 @@ test("a drop outside any cell is not a reorder", () => { const t = createTimeline({ project }); document.body.append(t.node); - const strip = t.node.querySelector(".cx-strip"); - const cells = t.node.querySelectorAll(".cx-strip-cell"); + const track = t.node.querySelector(".cx-track"); + const cells = t.node.querySelectorAll(".cx-track-clip"); const dt = fakeDataTransfer(); dragFire(cells[0], "dragstart", dt); - dragFire(strip, "drop", dt); // the strip's own background, not a cell + dragFire(track, "drop", dt); // the track's own background, not a cell assert.deepEqual(project.moved, []); t.destroy(); }); test("dragend firing on a node a redraw already detached does not leave anything for an unrelated drop to pick up", () => { - // dragend fires on the ORIGINAL source element, wherever it now is -- a - // redraw mid-drag can detach it from the tree, and an event on a detached - // node does not bubble to a listener on an ancestor it no longer has. A - // closure variable set at dragstart and only ever cleared by that dragend - // would be left stuck; the fix does not depend on dragend firing at all, so - // there is nothing to leave stuck. This drop's dataTransfer belongs to a - // DIFFERENT, unrelated drag session (its own fresh, empty one) -- exactly - // what a later, disconnected drag looks like. const project = fakeProject(scenes(4)); // s0 s1 s2 s3 const t = createTimeline({ project }); document.body.append(t.node); const started = fakeDataTransfer(); - dragFire(t.node.querySelectorAll(".cx-strip-cell")[1], "dragstart", started); // grabs s1 + dragFire(t.node.querySelectorAll(".cx-track-clip")[1], "dragstart", started); // grabs s1 project.addScene(); // unrelated change, still redraws t.draw(); - // No drop ever reached the strip for `started` -- dropped outside the - // window, Escape, anything. Its dragend, even if it fires, cannot reach - // this delegated listener once its source node is gone. const unrelated = fakeDataTransfer(); // a completely separate drag - const after = t.node.querySelectorAll(".cx-strip-cell"); + const after = t.node.querySelectorAll(".cx-track-clip"); dragFire(after[3], "drop", unrelated); assert.deepEqual(project.moved, [], "an unrelated drop moved a scene from an abandoned drag"); t.destroy(); }); -test("zoom sets the clip width and remembers it for next time", () => { +test("zoom sets pixels-per-second and remembers it for next time", () => { window.localStorage.clear(); const project = fakeProject(scenes(2)); const t = createTimeline({ project }); document.body.append(t.node); - const strip = t.node.querySelector(".cx-strip"); + const before = parseFloat(t.node.querySelector(".cx-track-clip").style.width); t.node.querySelector('[role="radio"][aria-checked="false"]').click(); // any non-default level - assert.ok(strip.style.getPropertyValue("--strip-w"), "zoom did not touch the strip"); + const after = parseFloat(t.node.querySelector(".cx-track-clip").style.width); + assert.notEqual(after, before, "zoom did not change a clip's width"); assert.ok(window.localStorage.getItem("funpack.timeline.zoom")); t.destroy(); }); test("zoom picked earlier is applied when the timeline is rebuilt", () => { window.localStorage.setItem("funpack.timeline.zoom", "lg"); - const project = fakeProject(scenes(2)); + const project = fakeProject(scenes(2)); // 1s scenes, lg = 80px/s const t = createTimeline({ project }); document.body.append(t.node); - const strip = t.node.querySelector(".cx-strip"); - assert.equal(strip.style.getPropertyValue("--strip-w"), "128px"); + assert.equal(t.node.querySelector(".cx-track-clip").style.width, "80px"); t.destroy(); window.localStorage.clear(); }); -test("an excluded scene is dimmed in the strip, an included one is not", () => { +test("an excluded scene is dimmed in the track, an included one is not", () => { const project = fakeProject(scenes(2)); project.scenes[1].excluded = true; const t = createTimeline({ project }); document.body.append(t.node); - const cells = t.node.querySelectorAll(".cx-strip-cell"); + const cells = t.node.querySelectorAll(".cx-track-clip"); assert.equal(cells[0].classList.contains("cx-excluded"), false); assert.equal(cells[1].classList.contains("cx-excluded"), true); t.destroy(); }); + +// --- real time --------------------------------------------------------- + +test("a clip's width comes from its own length in frames, not a fixed size", () => { + const s = scenes(2); + s[0].length = 48; // 2s at the 24fps fallback + s[1].length = 24; // 1s + const project = fakeProject(s); + const t = createTimeline({ project }); + document.body.append(t.node); + + const widths = [...t.node.querySelectorAll(".cx-track-clip")].map((c) => parseFloat(c.style.width)); + // md zoom = 40px/s. + assert.deepEqual(widths, [80, 40]); + t.destroy(); +}); + +test("a scene with no length of its own uses the project's", () => { + const s = scenes(1); + const project = fakeProject(s, { length: 48 }); // 2s at 24fps + const t = createTimeline({ project }); + document.body.append(t.node); + + assert.equal(parseFloat(t.node.querySelector(".cx-track-clip").style.width), 80); + t.destroy(); +}); + +test("clicking a scene's clip selects it and reports the change", () => { + const seen = []; + const project = fakeProject(scenes(3)); // 1s scenes at 40px/s: s0 0-40, s1 40-80, s2 80-120 + const t = createTimeline({ project, onSelect: (scene) => seen.push(scene && scene.id) }); + document.body.append(t.node); + + t.node.querySelector(".cx-track-inner") + .dispatchEvent(new window.MouseEvent("click", { bubbles: true, clientX: 100 })); + assert.equal(project.selectedId, "s2"); + assert.deepEqual(seen, ["s2"]); + t.destroy(); +}); + +test("the playhead follows a selection made from outside the track", () => { + // Add/Remove/Move and a pick made elsewhere in the app all just change + // project.selectedId -- the same path this exercises directly, rather than + // depending on the fake store's own reselection fallback. + const project = fakeProject(scenes(3)); // 1s scenes: s0 0-1, s1 1-2, s2 2-3 + const t = createTimeline({ project }); + document.body.append(t.node); + + t.node.querySelector(".cx-track-inner") // s2 starts at 80px (2s) + .dispatchEvent(new window.MouseEvent("click", { bubbles: true, clientX: 80 })); + assert.equal(t.node.querySelector(".cx-track-playhead").style.insetInlineStart, "80px"); + + project.select("s0"); + t.draw(); + assert.equal(t.node.querySelector(".cx-track-playhead").style.insetInlineStart, "0px", + "the playhead did not follow a selection made outside the track"); + t.destroy(); +}); + +test("clicking mid-clip moves the playhead to that exact second without changing the selection", () => { + const project = fakeProject(scenes(2)); // s0 0-1s, s1 1-2s + const t = createTimeline({ project }); // starts selected on s0 + document.body.append(t.node); + + const inner = t.node.querySelector(".cx-track-inner"); + // 20px at 40px/s = 0.5s, inside s0's own span -- a pure seek, no reselect. + inner.dispatchEvent(new window.MouseEvent("click", { bubbles: true, clientX: 20 })); + + assert.equal(project.selectedId, "s0", "a click within the selected clip changed the selection"); + assert.equal(t.node.querySelector(".cx-track-playhead").style.insetInlineStart, "20px"); + t.destroy(); +}); diff --git a/app/shell/timeline.js b/app/shell/timeline.js index 4a47eecb..a0e91b70 100644 --- a/app/shell/timeline.js +++ b/app/shell/timeline.js @@ -8,6 +8,10 @@ // scene. v4 had one prompt for the whole project and split it by markers in the // text, which meant the text was both the content and the structure: editing a // scene could re-cut every other one. +// +// A real single-track timeline: clips as wide as they play for, a ruler in +// seconds, and a playhead -- composer's `track.default` does the drawing, this +// file only turns scenes into seconds and seconds back into a selected scene. import { composer } from "../composer/composer.js"; @@ -17,11 +21,20 @@ const EMPTY = { hint: "What you generate lands here, in the order it plays.", }; -// How wide a clip is at rest, in pixels -- what "zoom" changes. Persisted per +// How wide a second is, in pixels -- what "zoom" changes. Persisted per // browser like the media bin's own view choice: a per-viewer convenience, not // state a run depends on, so nothing breaks if storage is unavailable. const ZOOM_KEY = "funpack.timeline.zoom"; -const ZOOM_LEVELS = { sm: 48, md: 72, lg: 128 }; +const ZOOM_LEVELS = { sm: 20, md: 40, lg: 80 }; + +// No module declares an `fps` role today -- the one pipeline that ships +// (minimax_h3) hands CreateVideo a literal 24.0 rather than a project.video +// input, so there is nothing to read it FROM yet. This mirrors that literal +// rather than inventing a setting nothing produces. +// ponytail: becomes a real per-pipeline value the day a module declares a +// role at project.video.fps, the same way length/target_width/target_height +// already are -- read here with a fallback so that day needs no change here. +const FPS_FALLBACK = 24; function recallZoom() { try { return ZOOM_LEVELS[window.localStorage.getItem(ZOOM_KEY)] ? window.localStorage.getItem(ZOOM_KEY) : "md"; } @@ -40,7 +53,15 @@ function rememberZoom(level) { export function createTimeline({ project, onSelect } = {}) { const host = composer.region.stack({ gap: "sm", fill: true }); const empty = composer.emptyState.default(EMPTY); - let strip = null; + let track = null; + let pxPerSecond = ZOOM_LEVELS[recallZoom()]; + // The playhead's own position, in seconds -- decoupled from which scene is + // selected so a click partway through a clip reads as that exact second, + // not snapped to whichever scene it landed in. Re-synced to the selected + // scene's start whenever the selection changes from somewhere OTHER than a + // track click (Add/Remove/Move, or a selection made elsewhere in the app) -- + // see draw(). + let playhead = 0; // Rebuilt on every draw, so a stale handler cannot act on a scene that has // moved. The buttons that act on "the current scene" read it at click time @@ -53,20 +74,20 @@ export function createTimeline({ project, onSelect } = {}) { composer.iconButton.sm({ icon: "◀", label: "Move earlier", onClick: () => move(-1) }), composer.iconButton.sm({ icon: "▶", label: "Move later", onClick: () => move(1) }), ], - // Zoom changes clip WIDTH, not the frame counts behind them -- a bigger - // strip to work with, not a different one. + // Zoom changes seconds->pixels, not the durations behind them -- a bigger + // track to work with, not a different one. trailing: [composer.segmented.sm({ label: "Zoom", value: recallZoom(), options: [{ value: "sm", label: "S" }, { value: "md", label: "M" }, { value: "lg", label: "L" }], - onChange: (level) => { rememberZoom(level); applyZoom(level); }, + onChange: (level) => { + rememberZoom(level); + pxPerSecond = ZOOM_LEVELS[level] || ZOOM_LEVELS.md; + if (track) track.setZoom(pxPerSecond); + }, })], }); - function applyZoom(level) { - if (strip) strip.node.style.setProperty("--strip-w", `${ZOOM_LEVELS[level] || ZOOM_LEVELS.md}px`); - } - function add() { project.addScene(); announce(); @@ -90,67 +111,79 @@ export function createTimeline({ project, onSelect } = {}) { } /** Frames this clip runs for: its own crop, or the project's length. */ - const lengthOf = (scene) => scene.length || project.video.length || 1; + const framesOf = (scene) => scene.length || project.video.length || 1; + const fps = () => (Number(project.video.fps) > 0 ? Number(project.video.fps) : FPS_FALLBACK); + const secondsOf = (scene) => framesOf(scene) / fps(); - function items() { - return project.scenes.map((scene, i) => ({ - id: scene.id, - // The number is what a scene is called before it has any text, and the - // text is what it is called after -- a strip of "Scene 1..8" is a strip - // nobody can read their own project off. - label: scene.text ? `${i + 1}. ${scene.text}` : `Scene ${i + 1}`, - badge: String(i + 1), - thumb: scene.result || null, - icon: "▦", - // A clip as wide as it is long, so the strip reads as time rather than as - // a row of equal boxes. Bounded: one very long scene beside several short - // ones must not squeeze the rest to nothing. - weight: lengthOf(scene), - rating: scene.rating || null, - excluded: Boolean(scene.excluded), - })); - } - - /** Where each scene starts, in frames. The ruler is drawn from this. */ - function marks() { + /** Where each scene starts, in seconds -- contiguous, so a click anywhere + * on the track belongs to exactly one of them. */ + function spans() { let at = 0; return project.scenes.map((scene) => { const start = at; - at += lengthOf(scene); - return { id: scene.id, start, length: lengthOf(scene) }; + const duration = secondsOf(scene); + at += duration; + return { scene, start, duration }; }); } - // The ruler. Its ticks are the scene boundaries rather than a fixed interval: - // what a person looks for on this timeline is where one scene becomes the - // next, and at 24fps a tick per second is a picket fence. - const ruler = composer.ruler.default({ label: "Scenes" }); - - function drawRuler() { - const spans = marks(); - const total = spans.reduce((sum, m) => sum + m.length, 0) || 1; - ruler.set(spans.map((m, i) => ({ - at: m.start / total, - label: `${i + 1}`, - hint: `${m.start}`, - })), total); + function items() { + return project.scenes.map((scene, i) => { + const { start, duration } = spans()[i]; + return { + id: scene.id, + // The number is what a scene is called before it has any text, and the + // text is what it is called after -- a strip nobody can read their own + // project off. + label: scene.text ? `${i + 1}. ${scene.text}` : `Scene ${i + 1}`, + badge: String(i + 1), + thumb: scene.result || null, + icon: "▦", + start, duration, + rating: scene.rating || null, + excluded: Boolean(scene.excluded), + }; + }); } function draw() { const scenes = project.scenes; if (!scenes.length) { host.set([empty]); - strip = null; + track = null; return; } - if (!strip) { - strip = composer.gallery.strip({ - label: "Scenes", - items: items(), + const built = items(); + // The invariant: the playhead always lies within the SELECTED scene's + // span. A track click asserts both at once (see onSeek below) and stays + // exactly where it was clicked; anything else that changes the selection + // -- Add, Remove, Move, a pick made elsewhere -- only moves the selection, + // so this is what pulls the playhead back to match it. + const current = built.find((i) => i.id === project.selectedId); + if (current && !(playhead >= current.start && playhead < current.start + current.duration)) { + playhead = current.start; + } + if (!track) { + track = composer.track.default({ + label: "Timeline", + items: built, selection: project.selectedId ? [project.selectedId] : [], - onActivate: (item) => { project.select(item.id); announce(); }, + playhead, + pxPerSecond, + onSeek: (seconds, item) => { + playhead = seconds; + // select() no-ops, without announcing, when the click landed + // inside the scene already current -- the common case for a pure + // seek. The playhead still needs to move, so it is set directly + // rather than only through whatever redraw a real selection + // change would have triggered. + if (item) project.select(item.id); + announce(); + track.setPlayhead(playhead); + track.setValue(project.selectedId ? [project.selectedId] : []); + }, // Ids, resolved to a position HERE, now -- not carried from whenever - // the drag started. A remove or an undo can redraw the strip while a + // the drag started. A remove or an undo can redraw the track while a // drag is still in flight (the mouse held down is a different input // channel from the keyboard shortcut that triggers one), and a // position captured at dragstart would then name whatever has since @@ -163,17 +196,12 @@ export function createTimeline({ project, onSelect } = {}) { announce(); }, }); - host.set([controls, ruler, strip]); - applyZoom(recallZoom()); - drawRuler(); + host.set([controls, track]); return; } - drawRuler(); - // setItems then setValue: setValue redraws against whatever items are - // current, so seeding the selection first marks a row that is about to be - // replaced and the strip comes back with nothing on. - strip.setItems(items()); - strip.setValue(project.selectedId ? [project.selectedId] : []); + track.setItems(built); + track.setValue(project.selectedId ? [project.selectedId] : []); + track.setPlayhead(playhead); } draw(); @@ -182,8 +210,7 @@ export function createTimeline({ project, onSelect } = {}) { node: host.node, draw, destroy() { - if (strip) strip.destroy(); - ruler.destroy(); + if (track) track.destroy(); controls.destroy(); empty.destroy(); host.destroy();