From 37e7942e82331057ae59458f8709af0f57471f10 Mon Sep 17 00:00:00 2001
From: Thales Pereira <31625914+thcp@users.noreply.github.com>
Date: Mon, 31 Aug 2026 22:29:58 +0100
Subject: [PATCH] feat(loop): adjust a loop region instead of redrawing it
Reported in discussion #507: changing the size of a selection meant clicking
and dragging again from scratch, which restarts the whole thing and loses the
precision you had. The numeric fields are the only exact route, and finding a
seamless loop through them is guesswork.
The region could not be touched at all -- .loop-region was pointer-events:
none, so every pointerdown reached the create-drag underneath, which resets
both edges to the click position. Adjusting one edge necessarily destroyed the
other.
Three gestures now:
- a handle at either edge moves that edge alone, so a loop can be tightened one
side at a time;
- the body moves both edges together, preserving length, so a loop found by ear
can be slid without being re-measured;
- a press that does not travel still seeks, which is what clicking inside the
selection did before the region took pointer events. Losing that would have
been a regression for anyone who just wants to move the playhead.
The handles are 12px and overhang the 2px border on both sides, because an edge
you cannot reliably grab is the finnicky behaviour this is meant to replace.
They only render on hover or while dragging, so the selection looks the same at
rest.
create-drag already guarded with `e.target.closest(".loop-region")` -- dead
code until now, since the element could not receive events. It is a real guard
again, alongside the stopPropagation on the region's own handler.
The geometry lives in its own module rather than in transport.js, which cannot
be imported outside a browser: it pulls in state.js, which touches document at
module load. Same reason playbackStems.js is separate. That makes the clamping
testable, which is the half that matters -- an edge crossing its partner, or a
region pushed against either end of the track. MIN_LOOP_SEC moves with it and
now has one definition.
Pointer events throughout, so this works with touch and pen.
Closes #538.
---
static/css/waves.css | 44 +++++++++++++-
static/index.html | 2 +-
static/js/loopRegion.js | 42 ++++++++++++++
static/js/transport.js | 87 +++++++++++++++++++++++++++-
tests/js/loop-drag.test.mjs | 111 ++++++++++++++++++++++++++++++++++++
5 files changed, 283 insertions(+), 3 deletions(-)
create mode 100644 static/js/loopRegion.js
create mode 100644 tests/js/loop-drag.test.mjs
diff --git a/static/css/waves.css b/static/css/waves.css
index ebdfcfc7..6f490621 100644
--- a/static/css/waves.css
+++ b/static/css/waves.css
@@ -765,10 +765,52 @@
box-shadow:
inset 1px 0 0 rgba(255, 255, 255, 0.18),
inset -1px 0 0 rgba(255, 255, 255, 0.18);
- pointer-events: none;
+ /* Interactive so the region can be moved and its edges adjusted (#538).
+ A pointerdown that does not move still falls through to a seek, so
+ clicking inside the selection behaves as it always did. */
+ pointer-events: auto;
+ cursor: grab;
z-index: 3;
}
+.loop-region.dragging {
+ cursor: grabbing;
+}
+
+/* Wider than the 2px border they sit on: an edge you cannot reliably grab is
+ the finnicky behaviour this replaces. Extends outside the region as well as
+ in, so the handle is catchable from either side. */
+.loop-handle {
+ position: absolute;
+ top: 0;
+ bottom: 0;
+ width: 12px;
+ cursor: ew-resize;
+ z-index: 4;
+}
+
+.loop-handle-start {
+ left: -7px;
+}
+
+.loop-handle-end {
+ right: -7px;
+}
+
+/* Only visible while the pointer is on the region, so the selection reads the
+ same as before at rest. */
+.loop-region:hover .loop-handle::after,
+.loop-region.dragging .loop-handle::after {
+ content: "";
+ position: absolute;
+ top: 0;
+ bottom: 0;
+ left: 4px;
+ width: 4px;
+ background: var(--gold);
+ border-radius: 2px;
+}
+
.lane-placeholder {
height: 48px;
position: relative;
diff --git a/static/index.html b/static/index.html
index 7d9f58c8..c79d4064 100644
--- a/static/index.html
+++ b/static/index.html
@@ -617,7 +617,7 @@
diff --git a/static/js/loopRegion.js b/static/js/loopRegion.js
new file mode 100644
index 00000000..f3617dc1
--- /dev/null
+++ b/static/js/loopRegion.js
@@ -0,0 +1,42 @@
+// Loop-region geometry, kept apart from transport.js so it can be tested
+// without a DOM (same reason playbackStems.js is its own module).
+
+// Shortest loop worth having. Also the threshold that separates a click from a
+// drag, so a press that barely moves seeks instead of resizing.
+export const MIN_LOOP_SEC = 0.2;
+
+/// Where a loop-region drag lands, given where it started and where the pointer
+/// is now (#538, discussion #507).
+///
+/// `mode` is "start" or "end" to move one edge alone, or "move" to slide the
+/// whole region. The edge cases here -- an edge crossing its partner, a region
+/// pushed against either end of the track -- are where this goes wrong, not in
+/// the event plumbing, which is why this is a pure function.
+export function loopDragResult({
+ mode,
+ pointerTime,
+ grabTime,
+ fromStart,
+ fromEnd,
+ duration,
+ minLoop = MIN_LOOP_SEC,
+}) {
+ if (mode === "start") {
+ // Cannot cross the far edge: minLoop is what keeps a loop audible.
+ return {
+ start: Math.max(0, Math.min(pointerTime, fromEnd - minLoop)),
+ end: fromEnd,
+ };
+ }
+ if (mode === "end") {
+ return {
+ start: fromStart,
+ end: Math.min(duration, Math.max(pointerTime, fromStart + minLoop)),
+ };
+ }
+ // Move: clamp the region as a unit. Clamping each edge on its own would
+ // squash the loop against the track boundary instead of stopping it there.
+ const length = fromEnd - fromStart;
+ const shift = Math.max(-fromStart, Math.min(pointerTime - grabTime, duration - fromEnd));
+ return { start: fromStart + shift, end: fromStart + shift + length };
+}
diff --git a/static/js/transport.js b/static/js/transport.js
index 826456e8..e587e9d4 100644
--- a/static/js/transport.js
+++ b/static/js/transport.js
@@ -1,4 +1,5 @@
import { fmtTime, fmtTickLabel, fmtTimeMs, parseTimecode, storeGet, storeSet } from "./utils.js";
+import { MIN_LOOP_SEC, loopDragResult } from "./loopRegion.js";
import {
playBtn, playMiniBtn, stopBtn, loopBtn, timeEl, masterFader,
speedBtns,
@@ -26,7 +27,6 @@ import { isDownbeatIndex, getBeats as getGridBeats, getBars as getGridBars } fro
import { computeCountIn } from "./metronome.js";
import { t } from "./i18n.js";
-const MIN_LOOP_SEC = 0.2;
// Zoom range. 1 is the whole track fitted to the panel; there is nothing below
// it to show, so it is the floor rather than a soft default. 5 is the ceiling
// because peaks.json carries 1500 points per stem: past roughly 5x a typical
@@ -447,6 +447,90 @@ export function toggleLoop() {
// Click-drag on the timeline ruler or waveform body to define the loop
// region. Drag direction doesn't matter -- start and end get sorted.
+// Adjust an existing loop region rather than redrawing it (#538, discussion
+// #507).
+//
+// Three gestures on one element:
+// - a handle at either edge moves only that edge, so a loop can be tightened
+// one side at a time instead of being re-measured from scratch;
+// - the body moves both edges together, preserving length, so a loop found by
+// ear can be slid;
+// - a press that does not move is still a seek, which is what the region did
+// before it became interactive, and losing that would be a regression for
+// anyone who just wants to click inside their selection.
+//
+// Pointer events throughout, so this works with touch and pen as well as a
+// mouse.
+function wireLoopRegionAdjust() {
+ if (!loopRegionEl) return;
+
+ let mode = null; // "start" | "end" | "move"
+ let pointerId = null;
+ let grabTime = 0; // where in the track the pointer went down
+ let fromStart = 0;
+ let fromEnd = 0;
+ let moved = false;
+
+ const apply = (t) => {
+ const next = loopDragResult({
+ mode,
+ pointerTime: t,
+ grabTime,
+ fromStart,
+ fromEnd,
+ duration: totalDuration,
+ });
+ setLoopStart(next.start);
+ setLoopEnd(next.end);
+ updateLoopRegionVisual();
+ };
+
+ loopRegionEl.addEventListener("pointerdown", (e) => {
+ if (e.button !== 0 || !totalDuration) return;
+ const t = timeFromClientX(e.clientX);
+ if (t === null) return;
+ mode = e.target.closest("[data-loop-handle]")?.dataset.loopHandle ?? "move";
+ pointerId = e.pointerId;
+ grabTime = t;
+ fromStart = loopStart;
+ fromEnd = loopEnd;
+ moved = false;
+ loopRegionEl.classList.add("dragging");
+ loopRegionEl.setPointerCapture(e.pointerId);
+ // Stops wireLoopDrag's surface handler starting a fresh selection
+ // underneath this one.
+ e.stopPropagation();
+ e.preventDefault();
+ });
+
+ loopRegionEl.addEventListener("pointermove", (e) => {
+ if (mode === null || e.pointerId !== pointerId) return;
+ const t = timeFromClientX(e.clientX);
+ if (t === null) return;
+ // Same threshold the create-drag uses to tell a click from a drag.
+ if (Math.abs(t - grabTime) >= MIN_LOOP_SEC) moved = true;
+ apply(t);
+ e.preventDefault();
+ });
+
+ const finish = (e) => {
+ if (mode === null || e.pointerId !== pointerId) return;
+ const wasMove = mode === "move";
+ mode = null;
+ pointerId = null;
+ loopRegionEl.classList.remove("dragging");
+ if (!moved && wasMove) {
+ // A press with no travel: seek, exactly as clicking here did before the
+ // region took pointer events.
+ setPlayheadTime(grabTime);
+ }
+ syncLoopInputs();
+ };
+
+ loopRegionEl.addEventListener("pointerup", finish);
+ loopRegionEl.addEventListener("pointercancel", finish);
+}
+
// Tiny drags are treated as clicks and seek the playhead instead.
function wireLoopDrag() {
let dragging = false;
@@ -701,6 +785,7 @@ export function wireTransportButtons() {
stopBtn.addEventListener("click", stopTransport);
loopBtn.addEventListener("click", toggleLoop);
wireLoopDrag();
+ wireLoopRegionAdjust();
wireLoopInputs();
wireZoomButtons();
wireLaneScrollSync();
diff --git a/tests/js/loop-drag.test.mjs b/tests/js/loop-drag.test.mjs
new file mode 100644
index 00000000..12635037
--- /dev/null
+++ b/tests/js/loop-drag.test.mjs
@@ -0,0 +1,111 @@
+// Adjusting an existing loop region instead of redrawing it (#538).
+//
+// Only the clamping is tested: an edge crossing its partner, and a region
+// pushed against either end of the track. That is where this breaks; the
+// pointer plumbing is not something a node test can say anything useful about.
+
+import { loopDragResult } from '../../static/js/loopRegion.js';
+
+let passed = 0;
+let failed = 0;
+
+function check(name, condition, detail = '') {
+ if (condition) {
+ passed++;
+ console.log(`PASS ${name}`);
+ } else {
+ failed++;
+ console.log(`FAIL ${name}${detail ? ` -- ${detail}` : ''}`);
+ }
+}
+
+const near = (a, b) => Math.abs(a - b) < 1e-9;
+const DURATION = 100;
+// A selection from 20s to 30s, grabbed at 25s.
+const base = { grabTime: 25, fromStart: 20, fromEnd: 30, duration: DURATION, minLoop: 0.2 };
+
+// ─── dragging the start edge ───
+
+{
+ const r = loopDragResult({ ...base, mode: 'start', pointerTime: 22 });
+ check('start edge moves alone', near(r.start, 22) && near(r.end, 30),
+ `got ${r.start}..${r.end}`);
+}
+
+{
+ // The whole point of the request: nudging one edge must not disturb the other.
+ const r = loopDragResult({ ...base, mode: 'start', pointerTime: 5 });
+ check('start can be dragged earlier without moving the end', near(r.end, 30));
+}
+
+{
+ const r = loopDragResult({ ...base, mode: 'start', pointerTime: 45 });
+ check('start cannot cross the end', near(r.start, 30 - 0.2) && near(r.end, 30),
+ `got ${r.start}..${r.end}`);
+}
+
+{
+ const r = loopDragResult({ ...base, mode: 'start', pointerTime: -10 });
+ check('start clamps at zero', near(r.start, 0));
+}
+
+// ─── dragging the end edge ───
+
+{
+ const r = loopDragResult({ ...base, mode: 'end', pointerTime: 40 });
+ check('end edge moves alone', near(r.start, 20) && near(r.end, 40),
+ `got ${r.start}..${r.end}`);
+}
+
+{
+ const r = loopDragResult({ ...base, mode: 'end', pointerTime: 10 });
+ check('end cannot cross the start', near(r.start, 20) && near(r.end, 20 + 0.2),
+ `got ${r.start}..${r.end}`);
+}
+
+{
+ const r = loopDragResult({ ...base, mode: 'end', pointerTime: 500 });
+ check('end clamps at the track length', near(r.end, DURATION));
+}
+
+// ─── moving the whole region ───
+
+{
+ const r = loopDragResult({ ...base, mode: 'move', pointerTime: 35 });
+ check('move shifts both edges by the same amount',
+ near(r.start, 30) && near(r.end, 40), `got ${r.start}..${r.end}`);
+}
+
+{
+ const r = loopDragResult({ ...base, mode: 'move', pointerTime: 0 });
+ check('move stops at the start of the track without squashing',
+ near(r.start, 0) && near(r.end, 10), `got ${r.start}..${r.end}`);
+}
+
+{
+ const r = loopDragResult({ ...base, mode: 'move', pointerTime: 1000 });
+ check('move stops at the end of the track without squashing',
+ near(r.start, 90) && near(r.end, DURATION), `got ${r.start}..${r.end}`);
+}
+
+{
+ // Length is the invariant a move must never change.
+ for (const pointerTime of [-500, 0, 12, 25, 63, 99, 500]) {
+ const r = loopDragResult({ ...base, mode: 'move', pointerTime });
+ if (!near(r.end - r.start, 10)) {
+ check(`move preserves length at ${pointerTime}`, false, `got ${r.end - r.start}`);
+ break;
+ }
+ }
+ check('move preserves length everywhere', true);
+}
+
+{
+ // A press with no travel must leave the region exactly where it was, or a
+ // click-to-seek inside the selection would nudge it.
+ const r = loopDragResult({ ...base, mode: 'move', pointerTime: base.grabTime });
+ check('a move of zero changes nothing', near(r.start, 20) && near(r.end, 30));
+}
+
+console.log(`\n${passed}/${passed + failed} checks passed`);
+process.exit(failed === 0 ? 0 : 1);