Skip to content

fix(media): drag selection follows the pointer beyond the pane and auto-scrolls at edges - #342

Merged
garrity-miepub merged 7 commits into
mieweb:mainfrom
jlocala1:fix/transcript-drag-autoscroll
Aug 4, 2026
Merged

fix(media): drag selection follows the pointer beyond the pane and auto-scrolls at edges#342
garrity-miepub merged 7 commits into
mieweb:mainfrom
jlocala1:fix/transcript-drag-autoscroll

Conversation

@jlocala1

@jlocala1 jlocala1 commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

Problem

Drag-selecting words in the MediaEditor transcript is unreliable at the pane boundaries:

  • Drag the highlight toward the bottom (or top) of the pane and keep going — the selection stops following the mouse.
  • Hold the drag at the edge — the pane never scrolls, so you can't select past what's currently visible.

Cause

  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 there was no auto-scroll loop at all.

Fix

For the lifetime of one drag, document-level mousemove/mouseup listeners take over:

  • The selection extends to the word under the clamped pointer position (elementFromPoint, with fallback probe points for whitespace misses), so it keeps tracking wherever the mouse goes.
  • While the pointer is held at or beyond a pane edge, an animation-frame loop scrolls the pane (speed scales with overshoot, capped) and keeps extending the selection as new words scroll under the pointer.
  • Releasing outside the window ends the drag (buttons === 0 guard on mousemove).
  • Leaving the pane now only cancels the pending long-press (its previous side effect), never the drag itself.
  • Listeners and the scroll frame are detached on mouseup and on unmount.

Tests

Two regression tests added — both fail against the previous implementation (verified by stashing the fix):

  • selection keeps extending while the pointer is outside the pane, and stops changing after mouseup
  • mouseleave no longer cancels an in-progress drag

Gates: lint ✅ typecheck ✅ format ✅ 399/399 tests ✅

…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.
Copilot AI review requested due to automatic review settings July 27, 2026 17:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/mouseup listeners 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 mouseleave no 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.

Comment thread src/components/MediaEditor/MediaEditor.tsx
Comment thread src/components/MediaEditor/MediaEditor.tsx
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.
Copilot AI review requested due to automatic review settings July 27, 2026 17:52

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comment thread src/components/MediaEditor/MediaEditor.tsx Outdated
Comment thread src/components/MediaEditor/MediaEditor.tsx
… 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.
Copilot AI review requested due to automatic review settings July 27, 2026 18:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

  • stepDragAutoScroll schedules a new requestAnimationFrame unconditionally, 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 };

Comment thread src/components/MediaEditor/MediaEditor.tsx
@jlocala1

jlocala1 commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator Author

Two follow-up commits from testing on the deployed pulseclip dev instance:

  • 78fc0ad — clamp probe points and auto-scroll edge zones to the visible pane (viewport ∩ container): elementFromPoint only resolves inside the viewport, so panes extending past it stalled the highlight while scroll kept running.
  • 954100d — when every probe misses because the pointer sits in empty pane space (below the last line of a short or fully scrolled transcript), extend to the nearest end — standard editor behavior.

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
Copilot AI review requested due to automatic review settings July 27, 2026 18:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Copilot AI review requested due to automatic review settings August 3, 2026 19:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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). Reusing detachDragListeners() 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 mousemove handler 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 first mousemove during 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();

@jlocala1
jlocala1 force-pushed the fix/transcript-drag-autoscroll branch from 2dd88ca to cf4e4a8 Compare August 3, 2026 19:54
Copilot AI review requested due to automatic review settings August 3, 2026 22:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

  • extendSelectionToPointer falls back to querySelectorAll and then (in the “top padding” case) calls getBoundingClientRect() 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) {

@garrity-miepub
garrity-miepub self-requested a review August 4, 2026 00:23
Copilot AI review requested due to automatic review settings August 4, 2026 01:12

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 detachDragListeners logic (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). Reuse detachDragListeners() 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 garrity-miepub left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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! 👍

@garrity-miepub
garrity-miepub merged commit bc04723 into mieweb:main Aug 4, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants