fix(media): drag selection follows the pointer beyond the pane and auto-scrolls at edges - #342
Conversation
…to-scrolls at edges Two defects made drag-highlight unreliable in the transcript editor: 1. onMouseLeave on the listbox called handleMouseUp, silently ending the drag the moment the pointer left the pane. 2. Selection only extended via per-word mouseenter, so padding, end-of-line whitespace, and anything outside the pane never extended it - and nothing scrolled while holding at an edge. The drag now attaches document-level mousemove/mouseup listeners for its lifetime: the selection extends to the word under the clamped pointer (elementFromPoint with fallback probes), the pane auto-scrolls while the pointer is held at or past an edge, releasing outside the window ends the drag (buttons === 0 guard), and leaving the pane only cancels the pending long-press, not the drag. Regression tests fail on the previous implementation.
There was a problem hiding this comment.
Pull request overview
Improves transcript drag-selection behavior in MediaEditor so selections keep tracking the pointer outside the transcript pane and the pane auto-scrolls when dragging at its edges.
Changes:
- Switches drag-selection tracking to document-level
mousemove/mouseuplisteners for the lifetime of a drag. - Adds an animation-frame auto-scroll loop that scrolls the transcript pane when the pointer is held near/over the top/bottom edges.
- Adds regression tests covering selection extension outside the pane and ensuring
mouseleaveno longer cancels an in-progress drag.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/components/MediaEditor/MediaEditor.tsx | Implements document-level drag tracking, selection extension via elementFromPoint, and edge auto-scroll; removes onMouseLeave ending the drag. |
| src/components/MediaEditor/MediaEditor.test.tsx | Adds regression tests for dragging selection beyond the pane and preventing mouseleave from canceling a drag. |
elementFromPoint only resolves inside the viewport, but the transcript pane can extend beyond it (found on the deployed app at laptop viewport sizes: auto-scroll ran but the highlight stalled at the anchor). Probe points and the edge zones now use the intersection of the pane rect and the viewport, and a zero-area pane extends nothing.
… sits in empty pane space Dragging below the last line of a short or fully scrolled transcript put the clamped probe point in the pane's empty bottom area where no word span exists, so the selection stalled. When every probe misses, extend to the last word above the point (or the first word when above all), matching standard text-editor drag behavior.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
src/components/MediaEditor/MediaEditor.tsx:810
stepDragAutoScrollschedules a newrequestAnimationFrameunconditionally, even when the pointer is nowhere near an edge and no scrolling happens (step === 0). This creates a continuous 60fps loop for the entire drag, which is unnecessary work for long drags.
if (step !== 0) {
container.scrollTop += step;
extendSelectionToPointer();
}
dragScrollFrame.current = requestAnimationFrame(stepDragAutoScroll);
src/components/MediaEditor/MediaEditor.tsx:853
- If the auto-scroll loop is made conditional (only running while
step !== 0), it needs a way to restart when the pointer later moves back into an edge zone. A lightweight approach is to request a frame on mousemove only when no frame is currently queued.
if (e.buttons === 0) {
up();
return;
}
dragPointer.current = { x: e.clientX, y: e.clientY };
|
Two follow-up commits from testing on the deployed pulseclip dev instance:
Verified on the deployed instance: drag held below the viewport scrolls the pane to its limit and extends the selection to the final word (298/303 words, last word selected). All gates green, 399/399 tests. (SHAs updated after a history rewrite; the commits themselves are unchanged.) |
- Clear a pending long-press timer on unmount alongside the listeners and scroll frame - Clamp every probe X into the pane so narrow panes cannot resolve unrelated DOM - Guard the auto-scroll step against a fully offscreen pane (keep the loop alive, never scroll) - Extend to the first word when the pointer sits in the top padding above all spans, mirroring the past-the-end fallback
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/components/MediaEditor/MediaEditor.tsx:909
- The unmount cleanup duplicates the logic in
detachDragListeners()(remove document listeners + cancel rAF). ReusingdetachDragListeners()here reduces duplication and the risk of the two paths diverging over time.
React.useEffect(
() => () => {
if (dragListeners.current) {
document.removeEventListener('mousemove', dragListeners.current.move);
document.removeEventListener('mouseup', dragListeners.current.up);
src/components/MediaEditor/MediaEditor.tsx:882
- The document-level
mousemovehandler can leave the 500ms long-press timeout running while the user is actively dragging, as long as they don’t enter another word (e.g., dragging within whitespace). That can open the word editor mid-drag. Consider canceling the pending long-press timer on the firstmousemoveduring a drag, not only when leaving the pane.
// Leaving the pane cancels the pending long press (it used to cancel
// the whole drag), but the selection keeps tracking the pointer
const container = contentRef.current;
if (container && longPressTimer.current) {
const rect = container.getBoundingClientRect();
const outside =
e.clientX < rect.left ||
e.clientX > rect.right ||
e.clientY < rect.top ||
e.clientY > rect.bottom;
if (outside) {
clearTimeout(longPressTimer.current);
longPressTimer.current = null;
}
}
extendSelectionToPointer();
2dd88ca to
cf4e4a8
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/components/MediaEditor/MediaEditor.tsx:785
extendSelectionToPointerfalls back toquerySelectorAlland then (in the “top padding” case) callsgetBoundingClientRect()for every word span before concluding the pointer is above the first span. For long transcripts, that worst-case O(n) layout walk can happen on every mousemove / autoscroll frame and cause noticeable jank.
You can avoid the full scan by checking the first/last span rects up front (above-first => first word, below-last => last word), and only scanning when the pointer is between them.
const spans =
container.querySelectorAll<HTMLElement>('[data-word-index]');
for (let i = spans.length - 1; i >= 0; i--) {
const spanRect = spans[i].getBoundingClientRect();
if (spanRect.top <= y) {
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/components/MediaEditor/MediaEditor.tsx:785
- In
extendSelectionToPointer, when the clamped pointer is in the listbox top padding, all probe points miss and the fallback loop scans every[data-word-index]span from the end before finally selecting the first word. On large transcripts this can turn a common drag-to-top-edge action into an O(n) hot path (mousemove + auto-scroll frames). Add a quick early check against the first span’s rect to avoid the full scan in this case.
const spans =
container.querySelectorAll<HTMLElement>('[data-word-index]');
for (let i = spans.length - 1; i >= 0; i--) {
const spanRect = spans[i].getBoundingClientRect();
if (spanRect.top <= y) {
src/components/MediaEditor/MediaEditor.tsx:909
- The unmount cleanup duplicates
detachDragListenerslogic (removing document listeners + canceling the rAF). This duplication increases the chance of future drift (e.g., if another listener is added but only one cleanup path is updated). ReusedetachDragListeners()in the effect cleanup and only keep the long-press timer cleanup here.
if (dragListeners.current) {
document.removeEventListener('mousemove', dragListeners.current.move);
document.removeEventListener('mouseup', dragListeners.current.up);
dragListeners.current = null;
}
if (dragScrollFrame.current !== null) {
garrity-miepub
left a comment
There was a problem hiding this comment.
Solid fix! The root cause analysis was spot on (onMouseLeave killing the drag, plus mouseenter never firing in whitespace).
Verified locally against latest main: lint, typecheck, and all tests green. Cleanup is airtight (listeners, scroll frame, and long-press timer all detached on mouseup and unmount). Also agree with keeping the rAF loop alive during drags; stopping it would stall edge-scrolling with a motionless pointer.
Nice regression tests. Approving! 👍
Problem
Drag-selecting words in the MediaEditor transcript is unreliable at the pane boundaries:
Cause
onMouseLeaveon the listbox calledhandleMouseUp, silently ending the drag the moment the pointer left the pane.mouseenter, so padding, end-of-line whitespace, and anything outside the pane never extended it — and there was no auto-scroll loop at all.Fix
For the lifetime of one drag, document-level
mousemove/mouseuplisteners take over:elementFromPoint, with fallback probe points for whitespace misses), so it keeps tracking wherever the mouse goes.buttons === 0guard on mousemove).Tests
Two regression tests added — both fail against the previous implementation (verified by stashing the fix):
mouseleaveno longer cancels an in-progress dragGates: lint ✅ typecheck ✅ format ✅ 399/399 tests ✅