fix(editor): scroll keys work on load without clicking the document (#1647) (#1648)

* fix(editor): route scroll keys to the document when the browser has no target (#1647)

On load nothing has focus and <main>, not the window, scrolls, so Down,
PageDown and Space did nothing until the reader clicked the document.
useScrollKeyRouting scrolls the document viewport for unmodified scroll
keys only when focus is on body, no earlier listener handled the key, no
dialog/popover/menu/listbox is open, and the last click left the browser
without a scrollable target. It never moves focus. Disabled on HTML and
live-app surfaces and while vim mode owns the keyboard.

* fix(ui): match the native page step for routed PageDown/Space (#1647)

Chromium, WebKit and Firefox all page by max(87.5% of the viewport,
viewport - 40px). Using the fraction alone made the first routed PageDown
on load shorter than every native one after a click (746px vs 812px at
900px tall).

* test(ui): run the scrollKeyAction mapping tests without a DOM

The pure mapping block runs in the non-DOM test job too, where
KeyboardEvent is undefined, so both tests failed in CI. scrollKeyAction now
takes the keydown fields it reads and the tests pass plain objects.

Author: backnotprop

.github/workflows/test.yml

@@ -257,6 +257,7 @@ jobs:
           packages/ui/hooks/useViewportEnvironment.test.tsx
           packages/ui/hooks/useIsMobile.test.tsx
           packages/ui/hooks/useVimDocumentFocus.test.tsx
+          packages/ui/hooks/useScrollKeyRouting.test.tsx
           packages/ui/hooks/useScrollViewport.test.ts
           packages/review-editor/components/ExpandedCommentDialog.mobile.test.tsx
           packages/review-editor/components/DiffTypePicker.openState.test.tsx

packages/editor/App.tsx

@@ -89,6 +89,7 @@ import {
   getDocumentScrollViewport,
   ScrollViewportProvider,
 } from '@plannotator/ui/hooks/useScrollViewport';
+import { useScrollKeyRouting } from '@plannotator/ui/hooks/useScrollKeyRouting';
 import { useOverlayViewport } from '@plannotator/ui/hooks/useOverlayViewport';
 import { useCompactTouchLayout, useIsMobile } from '@plannotator/ui/hooks/useIsMobile';
 import { useViewportEnvironment } from '@plannotator/ui/hooks/useViewportEnvironment';
@@ -927,6 +928,16 @@ const App: React.FC = () => {
       : mainViewportRef.current);
   }, [handleViewportReady, usesDocumentScroll]);
 
+  // #1647: Down/PageDown/Space do nothing on load because <main> scrolls, not
+  // the window, and nothing has focus yet. Route those keys to the document
+  // only while the browser has no scroll target of its own. HTML/live-app
+  // surfaces are excluded (the framed page owns its keys), and vim owns
+  // keyboard focus when enabled.
+  useScrollKeyRouting({
+    viewport: scrollViewport,
+    enabled: !isHtmlSurface && !liveApp && !vimModeEnabled,
+  });
+
   usePrintMode();
 
   // Sidebar (shared TOC + Version Browser)

packages/ui/hooks/useScrollKeyRouting.test.tsx

@@ -0,0 +1,191 @@
+/**
+ * #1647: scroll keys do nothing on load because the document scrolls inside
+ * <main>, not the window. These tests guard the routing that fills that gap
+ * and, just as much, every owner it must stay out of the way of.
+ *
+ * DOM-gated: run with DOM_TESTS=1 (listed in .github/workflows/test.yml).
+ */
+import { afterEach, describe, expect, test } from 'bun:test';
+import React, { act } from 'react';
+import { createRoot, type Root } from 'react-dom/client';
+import { scrollKeyAction, useScrollKeyRouting, type ScrollKeyInput } from './useScrollKeyRouting';
+
+const hasDom = typeof document !== 'undefined';
+let root: Root | null = null;
+let host: HTMLElement | null = null;
+
+interface ScrollCall { readonly top?: number }
+
+/** A viewport that reports overflow and records the scrolls asked of it. */
+function makeViewport(): { el: HTMLElement; calls: ScrollCall[] } {
+  const el = document.createElement('main');
+  el.style.overflowY = 'auto';
+  Object.defineProperty(el, 'scrollHeight', { configurable: true, value: 5000 });
+  Object.defineProperty(el, 'clientHeight', { configurable: true, value: 800 });
+  const calls: ScrollCall[] = [];
+  el.scrollBy = ((options: ScrollToOptions) => { calls.push({ top: options.top }); }) as HTMLElement['scrollBy'];
+  el.scrollTo = ((options: ScrollToOptions) => { calls.push({ top: options.top }); }) as HTMLElement['scrollTo'];
+  const paragraph = document.createElement('p');
+  paragraph.textContent = 'Document text';
+  el.appendChild(paragraph);
+  document.body.appendChild(el);
+  return { el, calls };
+}
+
+function Harness({ viewport, enabled }: { viewport: HTMLElement; enabled: boolean }) {
+  useScrollKeyRouting({ viewport, enabled });
+  return null;
+}
+
+async function mount(viewport: HTMLElement, enabled = true): Promise<void> {
+  host = document.createElement('div');
+  document.body.appendChild(host);
+  root = createRoot(host);
+  await act(async () => root?.render(<Harness viewport={viewport} enabled={enabled} />));
+}
+
+function key(target: EventTarget, init: KeyboardEventInit): KeyboardEvent {
+  const event = new KeyboardEvent('keydown', { bubbles: true, cancelable: true, ...init });
+  target.dispatchEvent(event);
+  return event;
+}
+
+function pointerDown(target: EventTarget): void {
+  target.dispatchEvent(new PointerEvent('pointerdown', { bubbles: true, cancelable: true }));
+}
+
+// Plain objects, not `new KeyboardEvent`: this block runs in the non-DOM job too.
+const k = (init: Partial<ScrollKeyInput> & { key: string }) => scrollKeyAction({
+  isComposing: false, ctrlKey: false, metaKey: false, altKey: false, shiftKey: false, ...init,
+});
+
+describe('scrollKeyAction', () => {
+  test('maps the plain scroll keys and Shift+Space', () => {
+    expect(k({ key: 'ArrowDown' })).toEqual({ kind: 'line', direction: 1 });
+    expect(k({ key: 'ArrowUp' })).toEqual({ kind: 'line', direction: -1 });
+    expect(k({ key: 'PageDown' })).toEqual({ kind: 'page', direction: 1 });
+    expect(k({ key: 'PageUp' })).toEqual({ kind: 'page', direction: -1 });
+    expect(k({ key: ' ' })).toEqual({ kind: 'page', direction: 1 });
+    expect(k({ key: ' ', shiftKey: true })).toEqual({ kind: 'page', direction: -1 });
+    expect(k({ key: 'Home' })).toEqual({ kind: 'edge', edge: 'start' });
+    expect(k({ key: 'End' })).toEqual({ kind: 'edge', edge: 'end' });
+  });
+
+  test('leaves chords, selection-extending keys and IME composition alone', () => {
+    expect(k({ key: 'ArrowDown', metaKey: true })).toBeNull();
+    expect(k({ key: 'ArrowDown', ctrlKey: true })).toBeNull();
+    expect(k({ key: 'ArrowDown', altKey: true })).toBeNull();
+    expect(k({ key: 'ArrowDown', shiftKey: true })).toBeNull();
+    expect(k({ key: 'End', shiftKey: true })).toBeNull();
+    expect(k({ key: ' ', isComposing: true })).toBeNull();
+    expect(k({ key: 'Enter' })).toBeNull();
+    expect(k({ key: 'j' })).toBeNull();
+  });
+});
+
+describe('useScrollKeyRouting', () => {
+  afterEach(async () => {
+    const mounted = root;
+    if (mounted) await act(async () => mounted.unmount());
+    root = null;
+    host = null;
+    if (hasDom) document.body.replaceChildren();
+  });
+
+  test.skipIf(!hasDom)('scrolls the document viewport when nothing owns the key', async () => {
+    const { el, calls } = makeViewport();
+    await mount(el);
+
+    const pageDown = key(document.body, { key: 'PageDown' });
+    expect(pageDown.defaultPrevented).toBe(true);
+    key(document.body, { key: 'ArrowDown' });
+    key(document.body, { key: ' ', shiftKey: true });
+
+    expect(calls.map(c => Math.sign(c.top ?? 0))).toEqual([1, 1, -1]);
+    expect(calls[0].top).toBeGreaterThan(calls[1].top ?? 0);
+  });
+
+  test.skipIf(!hasDom)('never takes a key from a focused control, input or editor', async () => {
+    const { el, calls } = makeViewport();
+    await mount(el);
+
+    const input = document.createElement('textarea');
+    document.body.appendChild(input);
+    input.focus();
+    const space = key(input, { key: ' ' });
+    expect(space.defaultPrevented).toBe(false);
+
+    const button = document.createElement('button');
+    document.body.appendChild(button);
+    button.focus();
+    expect(key(button, { key: ' ' }).defaultPrevented).toBe(false);
+
+    expect(calls).toEqual([]);
+  });
+
+  test.skipIf(!hasDom)('yields to a listener that already handled the key', async () => {
+    const { el, calls } = makeViewport();
+    await mount(el);
+    const consume = (event: KeyboardEvent) => event.preventDefault();
+    window.addEventListener('keydown', consume, true);
+    try {
+      key(document.body, { key: 'ArrowDown' });
+    } finally {
+      window.removeEventListener('keydown', consume, true);
+    }
+    expect(calls).toEqual([]);
+  });
+
+  test.skipIf(!hasDom)('does not scroll the document behind a dialog, menu or listbox', async () => {
+    const { el, calls } = makeViewport();
+    await mount(el);
+
+    for (const role of ['dialog', 'menu', 'listbox']) {
+      const overlay = document.createElement('div');
+      overlay.setAttribute('role', role);
+      document.body.appendChild(overlay);
+      expect(key(document.body, { key: 'PageDown' }).defaultPrevented).toBe(false);
+      overlay.remove();
+    }
+    const legacyModal = document.createElement('div');
+    legacyModal.className = 'fixed inset-0';
+    document.body.appendChild(legacyModal);
+    key(document.body, { key: ' ' });
+    legacyModal.remove();
+
+    expect(calls).toEqual([]);
+  });
+
+  test.skipIf(!hasDom)('leaves the key to the browser once a click gave it a scroll target', async () => {
+    const { el, calls } = makeViewport();
+    await mount(el);
+
+    // A click in the document: the browser now scrolls <main> natively, so
+    // routing too would move it twice.
+    pointerDown(el.querySelector('p')!);
+    expect(key(document.body, { key: 'PageDown' }).defaultPrevented).toBe(false);
+    expect(calls).toEqual([]);
+
+    // A click on chrome with nothing scrollable above it (the header).
+    const header = document.createElement('header');
+    document.body.appendChild(header);
+    pointerDown(header);
+    key(document.body, { key: 'PageDown' });
+    expect(calls).toHaveLength(1);
+
+    // A click on a node that then goes away (a dismissed first-run dialog).
+    const dialogButton = document.createElement('button');
+    el.appendChild(dialogButton);
+    pointerDown(dialogButton);
+    dialogButton.remove();
+    key(document.body, { key: 'PageDown' });
+    expect(calls).toHaveLength(2);
+  });
+
+  test.skipIf(!hasDom)('does nothing while disabled', async () => {
+    const { el, calls } = makeViewport();
+    await mount(el, false);
+    expect(key(document.body, { key: 'PageDown' }).defaultPrevented).toBe(false);
+    expect(calls).toEqual([]);
+  });
+});

packages/ui/hooks/useScrollKeyRouting.ts

@@ -0,0 +1,157 @@
+import { useEffect } from 'react';
+import { hasBlockingOverlay } from './useVimDocumentFocus';
+import { isDocumentScrollViewport } from './useScrollViewport';
+
+/**
+ * Scroll keys with no browser target (#1647).
+ *
+ * The desktop editor scrolls inside an element (`<main>`), not the window. On
+ * load nothing is focused and the user has not clicked anywhere, so the
+ * browser's keyboard-scroll starting point is the root scroller, which cannot
+ * scroll: ArrowDown, PageDown and Space do nothing until the reader clicks into
+ * the document. The same dead state returns after the node the user last
+ * clicked is removed (a dismissed first-run dialog) or when that click landed
+ * on chrome with nothing scrollable above it (the header).
+ *
+ * This hook fills exactly that gap and nothing else. It never moves focus, so
+ * dialogs, popovers, screen readers and `:focus-visible` are untouched. It
+ * only acts when all of these hold:
+ *   - focus is neutral (body), so no control, input, editor or iframe owns
+ *     the key;
+ *   - no earlier listener consumed the key (`defaultPrevented`);
+ *   - no modifier except Shift+Space (page up), and not mid-composition;
+ *   - no dialog, modal overlay, popover, menu or listbox is open;
+ *   - the browser has no scroll target of its own: the last pointerdown is
+ *     absent, detached, or has no vertically scrollable ancestor. When it has
+ *     one (a click in the document, the annotations panel, the sidebar), the
+ *     native key scroll already works and is left alone.
+ */
+
+const LINE_STEP_PX = 40;
+/**
+ * Page step as Chromium and WebKit compute it: the larger of 87.5% of the
+ * viewport (kMinFractionToStepWhenPaging) and the viewport minus a 40px
+ * overlap (kMaxOverlapBetweenPages). On a normal window the overlap rule wins,
+ * so a routed PageDown matches the native one taken after a click.
+ */
+const PAGE_FRACTION = 0.875;
+const PAGE_OVERLAP_PX = 40;
+
+const OPEN_POPUP_SELECTOR = '[role="menu"], [role="listbox"]';
+
+export type ScrollKeyAction =
+  | { readonly kind: 'line'; readonly direction: 1 | -1 }
+  | { readonly kind: 'page'; readonly direction: 1 | -1 }
+  | { readonly kind: 'edge'; readonly edge: 'start' | 'end' };
+
+/** The keydown fields the mapping reads (pure, so it runs without a DOM). */
+export type ScrollKeyInput = Pick<
+  KeyboardEvent,
+  'key' | 'isComposing' | 'ctrlKey' | 'metaKey' | 'altKey' | 'shiftKey'
+>;
+
+/** Map a keydown to the scroll it asks for, or null when it is not a plain scroll key. */
+export function scrollKeyAction(event: ScrollKeyInput): ScrollKeyAction | null {
+  if (event.isComposing || event.ctrlKey || event.metaKey || event.altKey) return null;
+  const isSpace = event.key === ' ' || event.key === 'Spacebar';
+  if (isSpace) return { kind: 'page', direction: event.shiftKey ? -1 : 1 };
+  if (event.shiftKey) return null;
+  switch (event.key) {
+    case 'ArrowDown': return { kind: 'line', direction: 1 };
+    case 'ArrowUp': return { kind: 'line', direction: -1 };
+    case 'PageDown': return { kind: 'page', direction: 1 };
+    case 'PageUp': return { kind: 'page', direction: -1 };
+    case 'Home': return { kind: 'edge', edge: 'start' };
+    case 'End': return { kind: 'edge', edge: 'end' };
+    default: return null;
+  }
+}
+
+function canScrollVertically(element: Element): boolean {
+  if (!(element instanceof HTMLElement)) return false;
+  if (element.scrollHeight <= element.clientHeight) return false;
+  const overflowY = getComputedStyle(element).overflowY;
+  return overflowY === 'auto' || overflowY === 'scroll' || overflowY === 'overlay';
+}
+
+/**
+ * Whether the browser already has a keyboard-scroll target: the last clicked
+ * node is still in the page and sits inside a vertically scrollable element.
+ */
+export function browserHasScrollTarget(lastPointerTarget: Node | null): boolean {
+  if (!lastPointerTarget || !lastPointerTarget.isConnected) return false;
+  const root = lastPointerTarget.ownerDocument?.documentElement ?? null;
+  let node: Node | null = lastPointerTarget;
+  while (node && node !== root) {
+    if (node instanceof Element && canScrollVertically(node)) return true;
+    const parent: Node | null = node.parentNode;
+    node = parent instanceof ShadowRoot ? parent.host : parent;
+  }
+  return false;
+}
+
+function focusIsNeutral(event: KeyboardEvent): boolean {
+  const doc = document;
+  const active = doc.activeElement;
+  if (active !== null && active !== doc.body && active !== doc.documentElement) return false;
+  const origin = event.composedPath?.()[0] ?? event.target;
+  return origin === doc.body || origin === doc.documentElement || origin === doc || origin === window;
+}
+
+function prefersReducedMotion(): boolean {
+  return typeof window.matchMedia === 'function'
+    && window.matchMedia('(prefers-reduced-motion: reduce)').matches;
+}
+
+function applyScroll(viewport: HTMLElement, action: ScrollKeyAction): void {
+  if (action.kind === 'line') {
+    viewport.scrollBy({ top: action.direction * LINE_STEP_PX, behavior: 'auto' });
+    return;
+  }
+  const behavior: ScrollBehavior = prefersReducedMotion() ? 'auto' : 'smooth';
+  if (action.kind === 'edge') {
+    viewport.scrollTo({ top: action.edge === 'start' ? 0 : viewport.scrollHeight, behavior });
+    return;
+  }
+  const height = viewport.clientHeight;
+  const page = Math.max(1, Math.round(Math.max(height * PAGE_FRACTION, height - PAGE_OVERLAP_PX)));
+  viewport.scrollBy({ top: action.direction * page, behavior });
+}
+
+export interface UseScrollKeyRoutingOptions {
+  /** The element that scrolls the document. Page-scrolling viewports are ignored. */
+  readonly viewport: HTMLElement | null;
+  /** False where another owner has the keys (vim, iframe surfaces). */
+  readonly enabled: boolean;
+}
+
+/** Route unowned scroll keys to the document viewport. See the module comment. */
+export function useScrollKeyRouting({ viewport, enabled }: UseScrollKeyRoutingOptions): void {
+  useEffect(() => {
+    if (!enabled || !viewport || isDocumentScrollViewport(viewport)) return;
+
+    let lastPointerTarget: Node | null = null;
+    const handlePointerDown = (event: PointerEvent) => {
+      const origin = event.composedPath?.()[0] ?? event.target;
+      lastPointerTarget = origin instanceof Node ? origin : null;
+    };
+
+    const handleKeyDown = (event: KeyboardEvent) => {
+      if (event.defaultPrevented) return;
+      const action = scrollKeyAction(event);
+      if (!action) return;
+      if (!viewport.isConnected || !focusIsNeutral(event)) return;
+      if (hasBlockingOverlay() || document.querySelector(OPEN_POPUP_SELECTOR)) return;
+      if (browserHasScrollTarget(lastPointerTarget)) return;
+      event.preventDefault();
+      applyScroll(viewport, action);
+    };
+
+    window.addEventListener('pointerdown', handlePointerDown, true);
+    window.addEventListener('keydown', handleKeyDown);
+    return () => {
+      window.removeEventListener('pointerdown', handlePointerDown, true);
+      window.removeEventListener('keydown', handleKeyDown);
+    };
+  }, [enabled, viewport]);
+}

packages/ui/hooks/useVimDocumentFocus.ts

@@ -38,7 +38,8 @@ function pageFocusIsNeutral(): boolean {
   return document.activeElement === document.body || document.activeElement === null;
 }
 
-function hasBlockingOverlay(): boolean {
+/** True while a dialog, modal overlay, or dismissable popover owns the page. */
+export function hasBlockingOverlay(): boolean {
   return document.querySelector(BLOCKING_OVERLAY_SELECTOR) !== null;
 }