Skip to content

Commit efd8b42

Browse files
committed
fix(editor): prevent scroll-to-top when clicking toolbar buttons (SD-1780)
Toolbar buttons had tabindex="0" but no mousedown prevention, so clicking them caused the browser to transfer focus away from the hidden ProseMirror editor. The subsequent refocus triggered ProseMirror's selectionToDOM, and the browser asynchronously scrolled to make the DOM selection visible inside the hidden editor at position:fixed top:0. This only manifested when the window was the scroll container (not a div with overflow:auto). Two fixes applied: 1. Toolbar mousedown preventDefault for non-input elements. This is the standard pattern used by ProseMirror's example editor, Tiptap, and most WYSIWYG editors. It keeps the PM editor focused throughout toolbar interactions. 2. requestAnimationFrame safety net in wrapHiddenEditorFocus. After the synchronous scroll restoration, a RAF callback catches any async browser scroll caused by layout reflow post-focus.
1 parent 39e1477 commit efd8b42

3 files changed

Lines changed: 57 additions & 1 deletion

File tree

packages/super-editor/src/components/toolbar/Toolbar.vue

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,32 @@ const handleCommand = ({ item, argument, option }) => {
7777
const restoreSelection = () => {
7878
proxy.$toolbar.activeEditor?.commands?.restoreSelection();
7979
};
80+
81+
/**
82+
* Prevents the browser's default focus-transfer behavior when clicking toolbar buttons.
83+
*
84+
* Without this, clicking a toolbar button moves focus from the hidden ProseMirror editor
85+
* to the toolbar button element. The subsequent refocus of the PM editor can trigger
86+
* browser-native scroll adjustments that jump the page to the top — especially when
87+
* the window (not a div) is the scroll container.
88+
*
89+
* Input elements are excluded so they still receive native focus and cursor placement.
90+
*/
91+
const handleToolbarMousedown = (e) => {
92+
if (e.target.closest('input, textarea, [contenteditable="true"]')) return;
93+
e.preventDefault();
94+
};
8095
</script>
8196
8297
<template>
83-
<div class="superdoc-toolbar" :key="toolbarKey" role="toolbar" aria-label="Toolbar" data-editor-ui-surface>
98+
<div
99+
class="superdoc-toolbar"
100+
:key="toolbarKey"
101+
role="toolbar"
102+
aria-label="Toolbar"
103+
data-editor-ui-surface
104+
@mousedown="handleToolbarMousedown"
105+
>
84106
<ButtonGroup
85107
tabindex="0"
86108
v-if="showLeftSide"

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -775,6 +775,15 @@ export class PresentationEditor extends EventEmitter {
775775
if (win.scrollX !== beforeX || win.scrollY !== beforeY) {
776776
win.scrollTo(beforeX, beforeY);
777777
}
778+
779+
// Safety net: the browser may asynchronously scroll after ProseMirror's
780+
// selectionToDOM() modifies the DOM selection inside the hidden editor.
781+
// A single requestAnimationFrame catches this post-layout scroll.
782+
win.requestAnimationFrame(() => {
783+
if (win.scrollX !== beforeX || win.scrollY !== beforeY) {
784+
win.scrollTo(beforeX, beforeY);
785+
}
786+
});
778787
};
779788
}
780789

@@ -2523,6 +2532,15 @@ export class PresentationEditor extends EventEmitter {
25232532
}, 'Layout RAF');
25242533
}
25252534

2535+
// Cancel pending focus-scroll safety net RAF
2536+
if (this.#focusScrollRafId != null) {
2537+
safeCleanup(() => {
2538+
const win = this.#visibleHost?.ownerDocument?.defaultView ?? window;
2539+
win.cancelAnimationFrame(this.#focusScrollRafId!);
2540+
this.#focusScrollRafId = null;
2541+
}, 'Focus scroll RAF');
2542+
}
2543+
25262544
// Cancel pending decoration sync RAF
25272545
if (this.#decorationSyncRafHandle != null) {
25282546
safeCleanup(() => {

packages/super-editor/src/core/presentation-editor/tests/PresentationEditor.focusWrapping.test.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -355,6 +355,22 @@ describe('PresentationEditor - Focus Wrapping (#wrapHiddenEditorFocus)', () => {
355355
editor.editor.view.focus();
356356
}).not.toThrow();
357357
});
358+
359+
it('schedules requestAnimationFrame as async scroll safety net', () => {
360+
editor = new PresentationEditor({
361+
element: container,
362+
documentId: 'test-doc',
363+
pageSize: { w: 612, h: 792 },
364+
});
365+
366+
const rafSpy = vi.spyOn(window, 'requestAnimationFrame');
367+
368+
editor.editor.view.focus();
369+
370+
// RAF should be scheduled to catch async browser scroll after focus
371+
expect(rafSpy).toHaveBeenCalledTimes(1);
372+
expect(rafSpy).toHaveBeenCalledWith(expect.any(Function));
373+
});
358374
});
359375

360376
describe('mock detection', () => {

0 commit comments

Comments
 (0)