From ad2fba8827460f8b5d195da2d3a9c7cfd76f9d35 Mon Sep 17 00:00:00 2001 From: digital-garbage <133010426+digital-garbage@users.noreply.github.com> Date: Tue, 25 Aug 2026 16:26:56 +0300 Subject: [PATCH] fix(editor): the rating picker holds the timeline open MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The picker is opened from a button in the timeline but mounts on document.body, so moving onto it left the zone and the timeline shut underneath the thing that had just been opened from it. TimelinePeek gains hold(reason) / release(reason). A hold outranks the pointer: whatever is holding it open is on screen because of the timeline. Reasons rather than a boolean so two holders cannot release each other's hold, and a hold taken while the timeline is shut opens it — the picker can be reached from the strip's own row. Any other panel that belongs to the timeline but lives outside it can use the same two calls. Verified in the browser: open by hover, hold, pointer leaves — stays open; release — closes; hold while shut — opens; release — closes; turning the preference off clears the holds. Co-Authored-By: Claude Opus 5 --- movie_editor/frontend/rating_picker.js | 5 +++ movie_editor/frontend/timeline_peek.js | 42 ++++++++++++++++++- movie_editor/frontend/timeline_peek.test.js | 45 +++++++++++++++++++++ 3 files changed, 91 insertions(+), 1 deletion(-) 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); +});