diff --git a/movie_editor/frontend/rating_picker.js b/movie_editor/frontend/rating_picker.js index b4cf222e..8de73e7b 100644 --- a/movie_editor/frontend/rating_picker.js +++ b/movie_editor/frontend/rating_picker.js @@ -84,6 +84,10 @@ activePicker = null; activeCleanup?.(); activeCleanup = null; + // The picker is opened from a button in the timeline but mounts on document.body, so + // with "Show timeline on hover" enabled, moving onto it leaves the zone and the timeline + // would shut underneath it. Held open for as long as it is up. + window.TimelinePeek?.release?.("rating-picker"); } function makeOption(r, accent, onPick, allOptions) { @@ -191,6 +195,7 @@ setActive(currentBase, currentLoved); document.body.append(picker); + window.TimelinePeek?.hold?.("rating-picker"); const cx = event?.clientX ?? window.innerWidth / 2; const cy = event?.clientY ?? window.innerHeight / 2; diff --git a/movie_editor/frontend/timeline_peek.js b/movie_editor/frontend/timeline_peek.js index 4c44a2d1..22027ca4 100644 --- a/movie_editor/frontend/timeline_peek.js +++ b/movie_editor/frontend/timeline_peek.js @@ -54,6 +54,7 @@ // the zone, and it only means anything alongside the attribute. const zone = d.getElementById && d.getElementById("timeline-zone"); if (zone) { zone.classList.remove(DRAG_CLASS); zone.classList.remove(OPEN_CLASS); } + holds.clear(); // the preference is off; nothing is holding anything open } return !!on; } @@ -137,6 +138,33 @@ return !target.closest(".zone-head"); } + // Something on screen that BELONGS to the timeline but does not live inside it can hold it + // open. The rating picker is the case: it mounts on document.body and is opened from a + // button in the timeline, so moving onto it leaves the zone and the timeline would shut + // underneath the thing you just opened. + // + // Reasons rather than a boolean, so two holders cannot release each other's hold. + const holds = new Set(); + + function hold(reason) { + holds.add(String(reason || "hold")); + if (installedIntent && installedZone) { + installedIntent.now(); + installedZone.classList.add(OPEN_CLASS); + } + return holds.size; + } + + function release(reason) { + holds.delete(String(reason || "hold")); + // Released with the pointer elsewhere, the timeline should go back to being shut. The + // next pointer event over it reopens it by the usual rule if it is still there. + if (!holds.size && installedIntent) installedIntent.away(); + return holds.size; + } + + function isHeld() { return holds.size > 0; } + // Hover INTENT, not hover. The pointer crossing the strip on its way somewhere else — the // Mac Dock is right under it in a windowed layout — used to flash the timeline open for a // frame. Opening waits for the pointer to still be there after a beat; leaving cancels a @@ -160,7 +188,9 @@ // Pointer is over the header, or has left the zone. away() { if (pending !== null) { clearT(pending); pending = null; } - if (!open) return false; + // A hold outranks the pointer: whatever is holding it open is on screen because of + // the timeline, and closing it out from under that is the bug this prevents. + if (!open || (o.held && o.held())) return open; open = false; o.onClose && o.onClose(); return false; @@ -188,6 +218,10 @@ }); } + // The live intent, so hold()/release() can act on it from outside install(). + let installedIntent = null; + let installedZone = null; + function install(doc) { const d = doc || document; const zone = d.getElementById("timeline-zone"); @@ -207,7 +241,12 @@ const intent = makeHoverIntent({ onOpen: () => { zone.classList.add(OPEN_CLASS); markReflow(d); }, onClose: () => zone.classList.remove(OPEN_CLASS), + held: isHeld, }); + // A hold taken while the timeline is shut has to open it, not merely keep it open: the + // rating picker can be reached from the strip's own row. + installedIntent = intent; + installedZone = zone; const openIf = (e) => { if (opensOnPointer(e.target, d)) intent.point(); else intent.away(); @@ -247,6 +286,7 @@ } return { get, set, apply, install, measure, ensureStrip, opensOnPointer, isDrag, + hold, release, isHeld, makeDragTracker, makeHoverIntent, LS_KEY, ATTR, DRAG_CLASS, OPEN_CLASS, STRIP_ID, STRIP_LABEL, OPEN_DELAY_MS }; })); diff --git a/movie_editor/frontend/timeline_peek.test.js b/movie_editor/frontend/timeline_peek.test.js index e3b83738..4b826260 100644 --- a/movie_editor/frontend/timeline_peek.test.js +++ b/movie_editor/frontend/timeline_peek.test.js @@ -165,3 +165,48 @@ test("a drag cancels a pending hover rather than opening twice", () => { assert.equal(opened, 0); // the hover open never fired; the drag already opened it assert.equal(hi.isOpen, true); }); + +// --- a modal that belongs to the timeline holds it open -------------------------- + +test("a hold keeps it open when the pointer leaves", () => { + // The rating picker mounts on document.body, so opening it leaves the zone. + const T = fakeTimers(); + let closed = 0; + let held = false; + const hi = P.makeHoverIntent({ ...T, onClose: () => closed++, held: () => held }); + hi.point(); T.run(); + held = true; + hi.away(); + assert.equal(closed, 0); + assert.equal(hi.isOpen, true); +}); + +test("it closes once the hold is gone", () => { + const T = fakeTimers(); + let closed = 0; + let held = true; + const hi = P.makeHoverIntent({ ...T, onClose: () => closed++, held: () => held }); + hi.now(); + hi.away(); + assert.equal(closed, 0); + held = false; + hi.away(); + assert.equal(closed, 1); +}); + +test("two holders do not release each other", () => { + assert.equal(P.hold("rating-picker"), 1); + assert.equal(P.hold("something-else"), 2); + assert.equal(P.release("something-else"), 1); + assert.equal(P.isHeld(), true); + assert.equal(P.release("rating-picker"), 0); + assert.equal(P.isHeld(), false); +}); + +test("releasing something that never held is not a release", () => { + P.hold("rating-picker"); + P.release("never-held-this"); + assert.equal(P.isHeld(), true); + P.release("rating-picker"); + assert.equal(P.isHeld(), false); +});