Skip to content

Commit c947433

Browse files
authored
SD-2541 - fix: avoid scrolling back when dragging selection (#2809)
* fix: avoid scrolling back when dragging selection * chore: removed references to tickets * chore: removed references to ticket
1 parent bc6e5bf commit c947433

3 files changed

Lines changed: 61 additions & 3 deletions

File tree

packages/super-editor/src/editors/v1/core/presentation-editor/PresentationEditor.ts

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,8 @@ export class PresentationEditor extends EventEmitter {
333333
/**
334334
* When true, the next selection render scrolls the caret/selection head into view.
335335
* Only set for user-initiated actions (keyboard/mouse selection, image click, zoom).
336+
* Not set on each `selectionUpdate` while a pointer drag is active — edge auto-scroll
337+
* owns the viewport then; `notifyDragSelectionEnded` restores one scroll after mouseup.
336338
* Passive re-renders (virtualization remounts, layout completions, DOM rebuilds) leave
337339
* this unset so they don't fight the user's scroll position.
338340
*/
@@ -3245,8 +3247,11 @@ export class PresentationEditor extends EventEmitter {
32453247
}
32463248
};
32473249
const handleSelection = () => {
3248-
// User-initiated selection change (keyboard, mouse) — scroll caret into view.
3249-
this.#shouldScrollSelectionIntoView = true;
3250+
// User-initiated selection change — scroll caret/head into view once, except during
3251+
// pointer drag: EditorInputManager edge auto-scroll must not fight #scrollActiveEndIntoView.
3252+
if (!this.#editorInputManager?.isDragging) {
3253+
this.#shouldScrollSelectionIntoView = true;
3254+
}
32503255
// Use immediate rendering for selection-only changes (clicks, arrow keys).
32513256
// Without immediate, the render is RAF-deferred — leaving a window where
32523257
// a remote collaborator's edit can cancel the pending render via
@@ -3566,6 +3571,10 @@ export class PresentationEditor extends EventEmitter {
35663571
selectParagraphAt: (pos: number) => this.#selectParagraphAt(pos),
35673572
finalizeDragSelectionWithDom: (pointer, dragAnchor, dragMode) =>
35683573
this.#finalizeDragSelectionWithDom(pointer, dragAnchor, dragMode),
3574+
notifyDragSelectionEnded: () => {
3575+
this.#shouldScrollSelectionIntoView = true;
3576+
this.#scheduleSelectionUpdate({ immediate: true });
3577+
},
35693578
hitTestTable: (x: number, y: number) => this.#hitTestTable(x, y),
35703579
});
35713580
}
@@ -4990,7 +4999,8 @@ export class PresentationEditor extends EventEmitter {
49904999
// (virtualization remounts, layout completions) never set this flag, so
49915000
// they won't scroll the viewport to the caret — only real user-initiated
49925001
// selection changes (keyboard, mouse, image click, zoom) will.
4993-
const shouldScrollIntoView = this.#shouldScrollSelectionIntoView;
5002+
// Belt-and-suspenders: never scroll from this path while pointer-drag is active.
5003+
const shouldScrollIntoView = this.#shouldScrollSelectionIntoView && !this.#editorInputManager?.isDragging;
49945004
this.#shouldScrollSelectionIntoView = false;
49955005

49965006
const sessionMode = this.#headerFooterSession?.session?.mode ?? 'body';

packages/super-editor/src/editors/v1/core/presentation-editor/pointer-events/EditorInputManager.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -359,6 +359,12 @@ export type EditorInputCallbacks = {
359359
dragAnchor: number,
360360
dragMode: 'char' | 'word' | 'para',
361361
) => void;
362+
/**
363+
* Called when a pointer text-drag selection ends.
364+
* Used to scroll the selection into view once after auto-scroll stops; during drag,
365+
* selection-driven scroll is suppressed to avoid fighting edge auto-scroll.
366+
*/
367+
notifyDragSelectionEnded?: () => void;
362368
/** Hit test table at coordinates */
363369
hitTestTable?: (x: number, y: number) => TableHitResult | null;
364370
};
@@ -1406,6 +1412,8 @@ export class EditorInputManager {
14061412
this.#callbacks.finalizeDragSelectionWithDom?.(pointer, dragAnchor, dragMode);
14071413
}
14081414

1415+
this.#callbacks.notifyDragSelectionEnded?.();
1416+
14091417
this.#callbacks.scheduleA11ySelectionAnnouncement?.({ immediate: true });
14101418

14111419
this.#dragLastPointer = null;

packages/super-editor/src/editors/v1/core/presentation-editor/tests/EditorInputManager.dragAutoScroll.test.ts

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@ describe('EditorInputManager - Drag Auto Scroll', () => {
135135
normalizeClientPoint: vi.fn((clientX: number, clientY: number) => ({ x: clientX, y: clientY })),
136136
updateSelectionVirtualizationPins: vi.fn(),
137137
scheduleSelectionUpdate: vi.fn(),
138+
notifyDragSelectionEnded: vi.fn(),
138139
};
139140

140141
manager = new EditorInputManager();
@@ -254,6 +255,8 @@ describe('EditorInputManager - Drag Auto Scroll', () => {
254255

255256
// Auto-scroll should be stopped
256257
expect(rafCallback).toBeNull();
258+
// one post-drag hook so PresentationEditor can scroll selection into view after auto-scroll stops
259+
expect(mockCallbacks.notifyDragSelectionEnded).toHaveBeenCalledTimes(1);
257260
});
258261

259262
it('does not auto-scroll in header/footer mode', () => {
@@ -328,4 +331,41 @@ describe('EditorInputManager - Drag Auto Scroll', () => {
328331
expect(scrollContainer.scrollLeft).toBe(0);
329332
});
330333
});
334+
335+
describe('notifyDragSelectionEnded (selection scroll after drag)', () => {
336+
it('invokes notifyDragSelectionEnded exactly once when a text drag ends after movement', () => {
337+
startDrag(10, 10);
338+
moveDrag(40, 25);
339+
endDrag(40, 25);
340+
341+
expect(mockCallbacks.notifyDragSelectionEnded).toHaveBeenCalledTimes(1);
342+
});
343+
344+
it('invokes notifyDragSelectionEnded when pointer goes down and up without move (click-hold-release)', () => {
345+
startDrag(10, 10);
346+
endDrag(10, 10);
347+
348+
expect(mockCallbacks.notifyDragSelectionEnded).toHaveBeenCalledTimes(1);
349+
});
350+
351+
it('does not invoke notifyDragSelectionEnded on pointer up if no drag was started', () => {
352+
endDrag(10, 10);
353+
354+
expect(mockCallbacks.notifyDragSelectionEnded).not.toHaveBeenCalled();
355+
});
356+
357+
it('invokes notifyDragSelectionEnded once per completed drag gesture', () => {
358+
startDrag(10, 10);
359+
moveDrag(20, 15);
360+
endDrag(20, 15);
361+
expect(mockCallbacks.notifyDragSelectionEnded).toHaveBeenCalledTimes(1);
362+
363+
(mockCallbacks.notifyDragSelectionEnded as ReturnType<typeof vi.fn>).mockClear();
364+
365+
startDrag(50, 50);
366+
moveDrag(60, 55);
367+
endDrag(60, 55);
368+
expect(mockCallbacks.notifyDragSelectionEnded).toHaveBeenCalledTimes(1);
369+
});
370+
});
331371
});

0 commit comments

Comments
 (0)