fix(vim): keep the j/k cursor clear of the HUD bands when scrolling

[rNoz]

Summary

Fixes #1153.

Document Vim navigation now reveals the cursor target with a margin instead of pinning it flush against the viewport edge, so j/k motion no longer parks the caret behind the sticky action bar (top) or the key HUD / status pill (bottom). A target already inside the safe band does not scroll, and a target with no scroll viewport falls back to the previous behavior unchanged.

The scroll math lives in a small pure helper (computeVimScrollDelta); the piece that makes it actually engage is where the viewport comes from. The document scroll host is a native-scroll <div> with no OverlayScrollbars attribute to rediscover, so the helper takes the scrolling element from ScrollViewportContext — the same node the reticle already measures against — rather than by selector. Resolving it by the (absent) [data-overlayscrollbars-viewport] attribute would silently match nothing and fall back to the old edge-pinning behavior, which is exactly the trap this avoids.

Proofs using Plannotator in my local integration branch, including this feature:

Screen.Recording.2026-07-30.at.08.12.03.mov

As you see, working flawlessly and w/o requiring the mouse to scroll and view items under the HUD overlays.

Why

Every cursor/target move used Element.scrollIntoView({ block: 'nearest' }), which pins the target flush against the nearest viewport edge — exactly where the fixed HUD overlays float. Keyboard motion to the top or bottom of a document then hid the caret behind an overlay, while a mouse wheel (which the browser lets overshoot) kept the same line nearer the centre. This reproduces the mouse feel for keyboard motion.

Changes

• Add packages/ui/utils/vimScroll.ts:
  • computeVimScrollDelta(viewport, target, band) — a pure function returning the signed scrollTop delta needed to clear the HUD bands, or 0 when the target already sits inside the safe band. A target taller than the band is aligned to its top edge (reading order wins — the start of the block is never pushed above the top margin to chase its bottom).
  • resolveVimScrollMargin(viewportHeight) — the band size, clamp(20% of height, 24px..160px), so short viewports keep a usable margin and tall ones do not reserve most of the screen.
  • scrollVimTargetIntoView(element) — resolves the owning scroll viewport the same way the reticle geometry does ([data-overlayscrollbars-viewport]), widens the top band to also clear a sticky action bar when present, and applies the delta. Falls back to scrollIntoView({ block: 'nearest' }) when there is no such viewport, and ignores zero-size (unrendered) targets.
• Take the scrolling element from ScrollViewportContext (useScrollViewport() in Viewer, the same node the reticle measures against) and pass it into scrollVimTargetIntoView. The document scroll host is a native-scroll <div> (OverlayScrollArea) that carries no [data-overlayscrollbars-viewport] attribute, so resolving the viewport by that selector silently matched nothing and fell back to the old edge-pinning scrollIntoView; taking the element from context is what makes the band actually engage. The selector is kept as a secondary fallback for hosts that mount the real OverlayScrollbars library.
• Route all cursor/target movement sites in packages/ui/hooks/useVimSelection.ts through scrollVimTargetIntoView (threading the context viewport), instead of scrollIntoView({ block: 'nearest' }).

Semantics and limits

• The scroll is applied as a relative viewport.scrollTop += delta; the browser clamps to [0, scrollHeight - clientHeight], so a delta larger than the available room simply reveals as far as the document allows.
• A target already inside the safe band produces a zero delta and does not scroll, preserving the prior "do nothing when already visible" feel.
• The band is a fraction of viewport height clamped to [24px, 160px]; on ordinary viewports this reads as a Vim scrolloff-style margin rather than a constant recentre.
• The move is instant, not smooth, so per-keystroke j/k tracks the caret without animation lag; the reticle's existing rAF-coalesced scroll listener repaints without a feedback loop.
• Scope: this covers the Markdown Vim surface. The --render-html annotate surface has its own injected-script navigation with the same edge-parking behavior; it is not addressed here.

Validation

• packages/ui/utils/vimScroll.test.ts: 16 passed (7 pure-math cases covering the safe band, the j-to-bottom and k-to-top bugs, tall-target top-alignment, a too-tall target straddling both edges, and a viewport offset from the page top; 3 margin clamp cases; 6 DOM cases under DOM_TESTS=1 covering the explicit-viewport scroll, the attribute-fallback path, an already-safe no-op, sticky-bar widening, the no-viewport fallback, and a zero-size target).
• npx tsc --noEmit -p packages/ui/tsconfig.json: clean.
• npx tsc --noEmit -p packages/ui/tsconfig.strict-consumer.json: clean (@plannotator/core/ui stays browser-safe and zero-dep — no node: imports).

Compatibility

The helper is additive. Any target without a resolvable OverlayScrollbars viewport takes the identical scrollIntoView({ block: 'nearest' }) path as before, so non-Plannotator hosts and any surface without the reticle are unchanged. No public API, prop, or payload changes.

Related work

None. This is a self-contained ergonomics fix to Vim-mode document navigation.

[rNoz]

@backnotprop you are gonna like this one :) Proofs both at the issue and PR, just cooked during the last 9h of work.

[backnotprop]

Review (at ac4f1337)

TLDR: needs one focused change. Your diagnosis, the pure-function factoring, and the ScrollViewportContext insight are all correct, and the fix fully repairs the default configuration. But the bottom band is a guessed constant that never measures the HUD, so in the opt-in key-HUD configuration the new math moves the cursor into the HUD instead of clear of it. Derive the bottom margin from the live HUD rect the way your top margin already derives from the sticky bar, and this merges. Apologies for the long review wait; the PR aged perfectly (clean merge onto tonight's main, zero conflicts with the recent Pierre and edit-mode work).

Detail (AI review findings, measured live over CDP against built plan-review servers, PR vs main baseline; screenshots retained):

The regression: resolveVimScrollMargin returns clamp(0.2h, 24, 160). VimKeyHud is position: fixed; bottom: 150; height: 88, occupying viewport-bottom offsets that need a band of at least ~238px to clear. On main, block:'nearest' accidentally landed targets below the HUD (clipped at the viewport edge, but visible); with the PR, mid-document j places the target fully inside the HUD with ~300px of horizontal overlap. Corroboration that the band is HUD-blind: measured scrollTop is identical with the key HUD on and off. Meanwhile in the shipped default (pill only), the PR fixes all three reproductions of #1153 cleanly: mid-document occlusion, the never-reaches-bottom case, and the top clip under the sticky actions.

The fix shape, mirroring what you already did for the top band: both elements are queryable today ([data-vim-key-hud], [data-vim-mode-badge]); take the max of the current margin and viewportBottom - hudRect.top + 8. That also removes the 6x over-reserve in the default config (160px reserved for a 25px pill, which reads as an aggressive recentre). The expanded key map can never be fully cleared on a short window; skipping widening while expanded is a reasonable documented choice since it is a deliberate modal state.

Also needed: add vimScroll.test.ts to the DOM list in .github/workflows/test.yml. It is DOM-gated and not on the allowlist, so its six integration tests (the exact code path that caused the original bug) never run in CI; verified 10 pass / 6 skip plain, 16 pass with the flag.

Low/nits, none blocking: the -58 scrollTop assertion tests arithmetic a real browser clamps away; the 300px sticky fixture is unrealistic (real cluster is 44px); the sticky widening only engages below ~300px viewport height until the band is geometry-derived, at which point it becomes meaningful; the document-level querySelector diverges from the Viewer-scoped precedent it cites (theoretical today, but the package is published for hosts that could mount two viewers); the OverlayScrollbars fallback targets an attribute that no longer exists in the repo, as your PR body candidly notes.

Checked and clear: merges onto current main with no conflicts; no interaction with tonight's edit-mode HUD (different surface, no z-index or selector overlap); typecheck including the strict consumer config, full suite 2860 pass / 0 fail, and all three builds green on the merged tree; no dist or lockfile changes.

[backnotprop]

Saw the rebase onto current main, thanks, that resolves the staleness cleanly. The one blocking item from the review (deriving the bottom band from the live HUD rect instead of the fixed constant, plus the test.yml DOM registration) is still open whenever you get to it, no rush.

[rNoz]

@backnotprop review addressed at cd2408f, CI green.

Bottom band is now geometry-derived, mirroring the top band. vimHudBandTop measures the live [data-vim-key-hud] / [data-vim-mode-badge] rects and the bottom margin becomes max(VIM_SCROLL_MARGIN_MIN, viewportBottom - hudTop + 8), falling back to the ratio band only when no HUD is mounted. The key HUD (bottom: 150 + height: 88 → ~246px band) is actually cleared now, and the default pill reserves ~49px instead of the 160px over-reserve. The expanded key HUD skips widening — deliberate modal state, can outgrow the viewport — documented in the helper. Both widgets are portaled to document.body, so the query is necessarily document-wide; it is scoped to element.ownerDocument (never the global), and since both are fixed-position singletons, a two-viewer host measures the same band from either instance.

vimScroll.test.ts added to the DOM list in test.yml — it runs in the seam-contract + DOM step now.

Nits folded in: the sticky test uses the realistic 44px cluster on a short viewport (where the widening genuinely engages) and starts scrolled down so the asserted scrollTop stays in browser-legal range; the dead OverlayScrollbars closest() fallback is removed (the attribute left the repo with #509) along with its test; the rect stub now derives bottom/right from top/left + size like a real getBoundingClientRect, and a beforeEach cleanup keeps a failing test from leaking stub HUDs into the next one.

Verified locally: 19/19 with DOM_TESTS=1 (10 pass / 9 skip plain), full suite green except one review-workspace integration test that fails identically on a clean main checkout here (environmental, macOS), typecheck including the strict consumer config, and build:hook + review build green. No dist or lockfile changes.