fix(editor): arrow key navigation across page boundaries and auto-scroll (SD-1950) (#2191)

* fix(editor): arrow key navigation across page boundaries and auto-scroll (SD-1950)

Fix vertical arrow key navigation that would jump sections or get stuck
at page boundaries, and add auto-scroll to keep the caret visible during
keyboard navigation.

Two bugs in the vertical-navigation extension:

1. ArrowDown would jump thousands of characters when crossing page
   boundaries because the adjacent line's getBoundingClientRect() returns
   off-screen coordinates, causing hitTest() to map to the wrong position.

2. ArrowUp would get stuck at certain positions because the adjacent
   line's DOM center Y falls in a zone where hitTest() maps to the
   current fragment instead of the adjacent one (fragment boundary
   misalignment), so the cursor stays at the same position.

The fix reads data-pm-start/data-pm-end from the adjacent line element
and validates the hit test result against this range. When the hit
falls outside the line's PM range (with a tight tolerance of 5), a
binary search using computeCaretLayoutRect resolves the correct position
at the goal X coordinate within the line. This avoids relying on
screen-space hit testing when it produces unreliable results.

Additionally adds scrollCaretIntoViewIfNeeded() to PresentationEditor
to auto-scroll the viewport after selection changes during keyboard
navigation.

* fix(editor): improve binary search resilience in vertical navigation

Skip unmeasurable positions in resolvePositionAtGoalX instead of
breaking the entire search. Positions at inline node boundaries may
return null from computeCaretLayoutRect, and breaking falls back to
pmStart causing the caret to jump to line start.

Also document the LTR assumption in the binary search.

* fix(editor): scope auto-scroll to arrow keys and add page-mount guard

Address review feedback for SD-1950:

- Scope auto-scroll to arrow-key navigation only. Add
  requestScrollCaretIntoView() public method that the vertical-nav
  extension calls before dispatch. #scrollCaretIntoViewIfNeeded only
  runs when this flag is set, preventing unexpected scroll jumps from
  collab edits, undo, find-and-replace, or other programmatic changes.

- Add page-mount guard in #scrollCaretIntoViewIfNeeded. The caret
  element may exist with estimated coordinates even when the page isn't
  visible (virtualized). Check for the page element in the DOM before
  using the caret rect, falling back to scrollPageIntoView otherwise.

- Add 7 unit tests for resolvePositionAtGoalX covering: closest match,
  exact match, goalX before/after all positions, null midpoints (inline
  node boundaries), all-null positions, and single-position ranges.

- Export resolvePositionAtGoalX for direct testing.

* fix(editor): revert scroll gating, keep unconditional auto-scroll

The flag-based scroll gating (requestScrollCaretIntoView) was
over-engineered. The 20px margin check in scrollCaretIntoViewIfNeeded
already prevents scrolling when the caret is visible. Scrolling when
the caret is off-screen is correct for all selection change sources
(arrow keys, undo, collab, find-and-replace).

Revert to unconditional scroll after caret rendering. Keep the
page-mount guard and resolvePositionAtGoalX tests from the previous
commit.

* fix(editor): remove page-mount guard from scrollCaretIntoViewIfNeeded

The DOM query for page mount status was failing (wrong selector after
main merge), causing scrollPageIntoView to fire on every selection
update instead of precise scroll. Restore the original guard that
only checks caret element existence.

* fix(editor): add page-mount guard using painterHost reference

Check if the caret's target page is mounted before using the caret
element's rect for scroll calculations. The caret overlay can exist
with estimated coordinates even when the page is virtualized. Query
this.#painterHost (stable class field) for the page element instead
of the previous fragile querySelector('.superdoc-container') approach.

* test(editor): add integration tests for hit validation and fallback path

Add two tests exercising the data-pm-start/data-pm-end validation
block in handleKeyDown:

- Hit lands within PM range: uses hit test result directly, no fallback
- Hit lands outside PM range: triggers resolvePositionAtGoalX fallback,
  binary search resolves position within the line's range

* fix(editor): scroll selection head into view for Shift+Arrow navigation

Refactor scroll logic to handle both collapsed and range selections:

- Extract #scrollScreenRectIntoView for reusable scroll-bounds check
- Replace #scrollCaretIntoViewIfNeeded with #scrollActiveEndIntoView
  that works for both carets and range selections
- For range selections, pick the rendered rect nearest the selection
  head: first child for backward (Shift+ArrowUp), last child for
  forward (Shift+ArrowDown)
- Call scroll after range selection rendering so Shift+Arrow across
  page boundaries follows the active end

* chore: fix behavior tests

* chore: fix pnpm lock

* chore: fix pnpm dedupe

Author: tupizz

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

@@ -269,6 +269,8 @@ export class PresentationEditor extends EventEmitter {
   #selectionOverlay: HTMLElement;
   #permissionOverlay: HTMLElement | null = null;
   #hiddenHost: HTMLElement;
+  /** Scroll-isolating wrapper around #hiddenHost. Append/remove this from the DOM. */
+  #hiddenHostWrapper: HTMLElement;
   #layoutOptions: LayoutEngineOptions;
   #layoutState: LayoutState = { blocks: [], measures: [], layout: null, bookmarks: new Map() };
   /** Cache for incremental toFlowBlocks conversion */
@@ -287,6 +289,13 @@ export class PresentationEditor extends EventEmitter {
   #pendingMapping: Mapping | null = null;
   #isRerendering = false;
   #selectionSync = new SelectionSyncCoordinator();
+  /**
+   * When true, the next selection render scrolls the caret/selection head into view.
+   * Only set for user-initiated actions (keyboard/mouse selection, image click, zoom).
+   * Passive re-renders (virtualization remounts, layout completions, DOM rebuilds) leave
+   * this unset so they don't fight the user's scroll position.
+   */
+  #shouldScrollSelectionIntoView = false;
   #epochMapper = new EpochPositionMapper();
   #layoutEpoch = 0;
   #htmlAnnotationHeights: Map<string, number> = new Map();
@@ -583,11 +592,16 @@ export class PresentationEditor extends EventEmitter {
     });
     this.#visibleHost.appendChild(this.#ariaLiveRegion);
 
-    this.#hiddenHost = createHiddenHost(doc, this.#layoutOptions.pageSize?.w ?? DEFAULT_PAGE_SIZE.w);
+    const { wrapper: hiddenHostWrapper, host: hiddenHost } = createHiddenHost(
+      doc,
+      this.#layoutOptions.pageSize?.w ?? DEFAULT_PAGE_SIZE.w,
+    );
+    this.#hiddenHostWrapper = hiddenHostWrapper;
+    this.#hiddenHost = hiddenHost;
     if (doc.body) {
-      doc.body.appendChild(this.#hiddenHost);
+      doc.body.appendChild(this.#hiddenHostWrapper);
     } else {
-      this.#visibleHost.appendChild(this.#hiddenHost);
+      this.#visibleHost.appendChild(this.#hiddenHostWrapper);
     }
 
     const { layoutEngineOptions: _layoutEngineOptions, element: _element, ...editorOptions } = options;
@@ -2436,6 +2450,7 @@ export class PresentationEditor extends EventEmitter {
     // Notify DomPainter so virtualization accounts for the CSS transform scale
     this.#domPainter?.setZoom?.(zoom);
     this.emit('zoomChange', { zoom });
+    this.#shouldScrollSelectionIntoView = true;
     this.#scheduleSelectionUpdate();
     // Trigger cursor updates on zoom changes
     if (this.#remoteCursorManager?.hasRemoteCursors()) {
@@ -2557,7 +2572,7 @@ export class PresentationEditor extends EventEmitter {
     this.#dragDropManager = null;
     this.#selectionOverlay?.remove();
     this.#painterHost?.remove();
-    this.#hiddenHost?.remove();
+    this.#hiddenHostWrapper?.remove();
     this.#hoverOverlay = null;
     this.#hoverTooltip = null;
     this.#modeBanner?.remove();
@@ -2682,6 +2697,8 @@ export class PresentationEditor extends EventEmitter {
       }
     };
     const handleSelection = () => {
+      // User-initiated selection change (keyboard, mouse) — scroll caret into view.
+      this.#shouldScrollSelectionIntoView = true;
       // Use immediate rendering for selection-only changes (clicks, arrow keys).
       // Without immediate, the render is RAF-deferred — leaving a window where
       // a remote collaborator's edit can cancel the pending render via
@@ -3028,6 +3045,7 @@ export class PresentationEditor extends EventEmitter {
    * @returns {void}
    */
   #focusEditorAfterImageSelection(): void {
+    this.#shouldScrollSelectionIntoView = true;
     this.#scheduleSelectionUpdate();
     if (document.activeElement instanceof HTMLElement) {
       document.activeElement.blur();
@@ -4205,6 +4223,13 @@ export class PresentationEditor extends EventEmitter {
    * @private
    */
   #updateSelection() {
+    // Consume the scroll intent before any early returns. Passive re-renders
+    // (virtualization remounts, layout completions) never set this flag, so
+    // they won't scroll the viewport to the caret — only real user-initiated
+    // selection changes (keyboard, mouse, image click, zoom) will.
+    const shouldScrollIntoView = this.#shouldScrollSelectionIntoView;
+    this.#shouldScrollSelectionIntoView = false;
+
     // In header/footer mode, the ProseMirror editor handles its own caret
     const sessionMode = this.#headerFooterSession?.session?.mode ?? 'body';
     if (sessionMode !== 'body') {
@@ -4324,6 +4349,9 @@ export class PresentationEditor extends EventEmitter {
           console.warn('[PresentationEditor] Failed to render caret overlay:', error);
         }
       }
+      if (shouldScrollIntoView) {
+        this.#scrollActiveEndIntoView(caretLayout.pageIndex);
+      }
       return;
     }
 
@@ -4366,6 +4394,99 @@ export class PresentationEditor extends EventEmitter {
         console.warn('[PresentationEditor] Failed to render selection rects:', error);
       }
     }
+
+    // Scroll to keep the selection head visible (Shift+Arrow across page boundaries).
+    // Use the head's layout rect to determine the target page.
+    if (shouldScrollIntoView) {
+      const head = activeEditor?.view?.state?.selection?.head ?? to;
+      const headLayout = this.#computeCaretLayoutRect(head);
+      if (headLayout) {
+        this.#scrollActiveEndIntoView(headLayout.pageIndex);
+      }
+    }
+  }
+
+  /**
+   * Scrolls the scroll container minimally so that a screen-space rect is visible,
+   * keeping a small margin (20px) for comfortable viewing. No-ops when the rect
+   * is already within the visible bounds.
+   */
+  #scrollScreenRectIntoView(screenTop: number, screenBottom: number): void {
+    const scrollContainer = this.#scrollContainer;
+    if (!scrollContainer) return;
+
+    let containerTop: number;
+    let containerBottom: number;
+
+    if (scrollContainer instanceof Window) {
+      containerTop = 0;
+      containerBottom = scrollContainer.innerHeight;
+    } else {
+      const r = (scrollContainer as Element).getBoundingClientRect();
+      containerTop = r.top;
+      containerBottom = r.bottom;
+    }
+
+    const SCROLL_MARGIN = 20;
+
+    if (screenBottom > containerBottom - SCROLL_MARGIN) {
+      const delta = screenBottom - containerBottom + SCROLL_MARGIN;
+      if (scrollContainer instanceof Window) {
+        scrollContainer.scrollBy({ top: delta });
+      } else {
+        (scrollContainer as Element).scrollTop += delta;
+      }
+    } else if (screenTop < containerTop + SCROLL_MARGIN) {
+      const delta = containerTop + SCROLL_MARGIN - screenTop;
+      if (scrollContainer instanceof Window) {
+        scrollContainer.scrollBy({ top: -delta });
+      } else {
+        (scrollContainer as Element).scrollTop -= delta;
+      }
+    }
+  }
+
+  /**
+   * Scrolls the scroll container so the caret or selection head remains visible
+   * after selection changes. Works for both collapsed (caret) and range selections.
+   *
+   * For collapsed selections, uses the rendered caret element's screen position.
+   * For range selections, uses the rendered selection rect nearest to the head.
+   *
+   * If the target page isn't mounted (virtualized), falls back to scrolling the
+   * page into view to trigger mount; the next selection update handles precise scroll.
+   */
+  #scrollActiveEndIntoView(pageIndex: number): void {
+    // Check if the target page is mounted before trusting rendered element positions.
+    const pageIsMounted = !!this.#painterHost.querySelector(`[data-page-index="${pageIndex}"]`);
+    if (!pageIsMounted) {
+      this.#scrollPageIntoView(pageIndex);
+      return;
+    }
+
+    // Try caret element first (collapsed selection)
+    const caretEl = this.#localSelectionLayer?.querySelector(
+      '.presentation-editor__selection-caret',
+    ) as HTMLElement | null;
+    if (caretEl) {
+      const r = caretEl.getBoundingClientRect();
+      this.#scrollScreenRectIntoView(r.top, r.bottom);
+      return;
+    }
+
+    // Range selection: pick the rendered rect nearest the selection head.
+    // Rects are rendered in document order. head < anchor means the user is
+    // extending backward (Shift+ArrowUp) → first child. head >= anchor means
+    // extending forward (Shift+ArrowDown) → last child.
+    const sel = this.getActiveEditor()?.view?.state?.selection;
+    const headIsForward = !sel || sel.head >= sel.anchor;
+    const headRect = (
+      headIsForward ? this.#localSelectionLayer?.lastElementChild : this.#localSelectionLayer?.firstElementChild
+    ) as HTMLElement | null;
+    if (headRect) {
+      const r = headRect.getBoundingClientRect();
+      this.#scrollScreenRectIntoView(r.top, r.bottom);
+    }
   }
 
   /**

packages/super-editor/src/core/presentation-editor/dom/HiddenHost.ts

@@ -1,5 +1,26 @@
 /**
- * Creates a hidden host element for the ProseMirror editor.
+ * Result of creating the hidden ProseMirror editor host.
+ *
+ * The hidden host is wrapped in a scroll-isolation container that prevents the
+ * browser's native caret-tracking scroll from leaking to the page. Without this
+ * wrapper, when the focused contenteditable has a deep caret position (e.g., end
+ * of a long document), the browser continuously scrolls the page to reveal it —
+ * fighting any programmatic scroll the user or virtualization system performs.
+ *
+ * @remarks
+ * The wrapper is the element to insert into the DOM tree. The host is the element
+ * passed to ProseMirror's Editor as its container.
+ */
+export type HiddenHostElements = {
+  /** Outer wrapper — append this to the DOM. Provides scroll isolation via overflow:hidden. */
+  wrapper: HTMLElement;
+  /** Inner host — pass this to ProseMirror as the editor container. */
+  host: HTMLElement;
+};
+
+/**
+ * Creates a hidden host element for the ProseMirror editor, wrapped in a
+ * scroll-isolating container.
  *
  * The hidden host contains the actual ProseMirror editor DOM, which provides semantic
  * document structure for accessibility (screen readers, keyboard navigation) while being
@@ -8,37 +29,55 @@
  *
  * @param doc - The document object to create the element in
  * @param widthPx - The width of the hidden host in pixels (should match document width)
- * @returns A configured hidden host div element
+ * @returns The wrapper (for DOM insertion) and the host (for ProseMirror)
  *
  * @remarks
- * - Uses position: fixed with left: -9999px to move off-screen without affecting scroll
- * - Uses opacity: 0 (NOT visibility: hidden) to keep content focusable
- * - Does NOT set aria-hidden="true" because the editor must remain accessible
- * - Sets pointer-events: none and z-index: -1 to prevent interaction
- * - Sets user-select: none to prevent text selection in the hidden editor
- * - Sets overflow-anchor: none to prevent scroll anchoring issues when content changes
- * - The viewport host is aria-hidden, but this host provides semantic structure
+ * **Scroll isolation (wrapper):**
+ * - `position: fixed; overflow: hidden; width: 1px; height: 1px` — creates a tiny,
+ *   off-screen scroll container. When the browser's native caret-tracking tries to
+ *   scroll to the focused contenteditable's caret, it adjusts the wrapper's scrollTop
+ *   (a no-op since the wrapper is 1×1) instead of the page's scrollTop.
+ *
+ * **Hidden host (inner element):**
+ * - `position: absolute` inside the wrapper — takes the full document width for accurate
+ *   text measurement while being visually clipped by the wrapper.
+ * - Inherits invisibility and non-interactivity from the wrapper (`opacity: 0`,
+ *   `pointer-events: none`). Does NOT use `visibility: hidden` — that prevents focusing.
+ * - Does NOT set `aria-hidden="true"` because the editor must remain accessible.
+ * - Sets `user-select: none` to prevent text selection in the hidden editor.
+ * - Sets `overflow-anchor: none` to prevent scroll anchoring issues when content changes.
  */
-export function createHiddenHost(doc: Document, widthPx: number): HTMLElement {
+export function createHiddenHost(doc: Document, widthPx: number): HiddenHostElements {
+  // --- Scroll-isolation wrapper ---
+  const wrapper = doc.createElement('div');
+  wrapper.className = 'presentation-editor__hidden-host-wrapper';
+  wrapper.style.setProperty('position', 'fixed');
+  wrapper.style.setProperty('left', '-9999px');
+  wrapper.style.setProperty('top', '0');
+  wrapper.style.setProperty('width', '1px');
+  wrapper.style.setProperty('height', '1px');
+  wrapper.style.setProperty('overflow', 'hidden');
+  wrapper.style.setProperty('opacity', '0');
+  wrapper.style.setProperty('z-index', '-1');
+  wrapper.style.setProperty('pointer-events', 'none');
+
+  // --- Inner host for ProseMirror ---
   const host = doc.createElement('div');
   host.className = 'presentation-editor__hidden-host';
-  host.style.setProperty('position', 'fixed');
-  host.style.setProperty('left', '-9999px');
+  host.style.setProperty('position', 'absolute');
+  host.style.setProperty('left', '0');
   host.style.setProperty('top', '0');
-  // Only set valid (non-negative) width values
   if (widthPx >= 0) {
     host.style.setProperty('width', `${widthPx}px`);
   }
   host.style.setProperty('overflow-anchor', 'none');
-  host.style.setProperty('pointer-events', 'none');
   // DO NOT use visibility:hidden - it prevents focusing!
-  // Instead use opacity:0 and z-index to hide while keeping focusable
-  host.style.setProperty('opacity', '0');
-  host.style.setProperty('z-index', '-1');
   host.style.setProperty('user-select', 'none');
   // DO NOT set aria-hidden="true" on this element.
   // This hidden host contains the actual ProseMirror editor which must remain accessible
   // to screen readers and keyboard navigation. The viewport (#viewportHost) is aria-hidden
   // because it's purely visual, but this editor provides the semantic document structure.
-  return host;
+
+  wrapper.appendChild(host);
+  return { wrapper, host };
 }