From cd2408ff3a94405f2a16d95514e4176ada208d78 Mon Sep 17 00:00:00 2001 From: rNoz Date: Wed, 29 Jul 2026 23:55:27 +0200 Subject: [PATCH] fix(vim): keep j/k cursor clear of HUD bands when scrolling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Document Vim navigation moved the cursor with `Element.scrollIntoView({ block: 'nearest' })`, which parks the target flush against the nearest viewport edge — exactly where the sticky action bar (top) and the key HUD / status pill (bottom) float. 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 centre. Add vimScroll.ts: a pure computeVimScrollDelta returning the signed scrollTop delta needed to clear a HUD band at each edge (0 when the target is already inside the safe band; a target taller than the band aligns to its top edge so reading order wins), resolveVimScrollMargin sizing the fallback band as clamp(20% of viewport height, 24px..160px), and a scrollVimTargetIntoView wrapper. Both bands are measured from live geometry instead of guessed constants: the top band widens past the ratio margin to clear the sticky action bar, and the bottom band derives from the portaled key HUD / mode pill rects ([data-vim-key-hud] / [data-vim-mode-badge]). The opt-in key HUD (fixed bottom: 150, height: 88 — a ~238px band, past the 160px clamp) is actually cleared, while the default pill reserves ~49px instead of over-reserving 160px. An expanded key HUD is a deliberate modal state that can outgrow the viewport, so it keeps the ratio margin. The scrolling element is the native-scroll host fed through ScrollViewportContext, so the wrapper takes that element from the caller (useScrollViewport() in Viewer, the same node the reticle measures against) and falls back to the historical scrollIntoView when it is absent, so behaviour never regresses. Route every cursor/target move in useVimSelection through it, and add vimScroll.test.ts to the DOM allowlist in test.yml so its integration tests run in CI. --- .github/workflows/test.yml | 1 + packages/ui/components/Viewer.tsx | 25 +-- packages/ui/hooks/useVimSelection.ts | 23 ++- packages/ui/utils/vimScroll.test.ts | 261 +++++++++++++++++++++++++++ packages/ui/utils/vimScroll.ts | 162 +++++++++++++++++ 5 files changed, 454 insertions(+), 18 deletions(-) create mode 100644 packages/ui/utils/vimScroll.test.ts create mode 100644 packages/ui/utils/vimScroll.ts diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index fd9ffe32d..54c77dd23 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -49,6 +49,7 @@ jobs: packages/ui/codeAnnotationDraftPersistence.test.tsx packages/ui/components/html-viewer/srcdoc.test.ts packages/ui/utils/clipboard.test.ts + packages/ui/utils/vimScroll.test.ts packages/ui/components/InlineMarkdown.resolveLinkedDoc.test.tsx packages/ui/components/MarkdownDiff.frozen.test.tsx packages/ui/components/MarkdownEditor.extensions.test.tsx diff --git a/packages/ui/components/Viewer.tsx b/packages/ui/components/Viewer.tsx index 7e90e30b4..a50cd436f 100644 --- a/packages/ui/components/Viewer.tsx +++ b/packages/ui/components/Viewer.tsx @@ -248,6 +248,9 @@ export const Viewer = forwardRef(({ } }; const containerRef = useRef(null); + // The element that actually scrolls; shared by the Vim scroll math, the + // sticky-header observer, and the reticle geometry. + const scrollViewport = useScrollViewport(); // The badge cluster (repo chips / diff badge) is absolutely positioned in the // card's top padding. One row fits; a second row (diff badge) or mobile // wrapping outgrows the padding and lands on the document's first heading. @@ -461,6 +464,7 @@ export const Viewer = forwardRef(({ }, []); const vim = useVimSelection({ containerRef, + scrollViewport, enabled: vimModeActive, hudEnabled: vimHudEnabled, blocked: vimBlocked, @@ -535,16 +539,15 @@ export const Viewer = forwardRef(({ // Detect when sticky action bar is "stuck" to show card background. // The IntersectionObserver root must be the actual scroll element — the // OverlayScrollArea viewport — not the
host, which doesn't scroll. - const stickyScrollViewport = useScrollViewport(); useEffect(() => { - if (!stickyActions || !stickySentinelRef.current || !stickyScrollViewport) return; + if (!stickyActions || !stickySentinelRef.current || !scrollViewport) return; const observer = new IntersectionObserver( ([entry]) => setIsStuck(!entry.isIntersecting), - { root: stickyScrollViewport, threshold: 0 } + { root: scrollViewport, threshold: 0 } ); observer.observe(stickySentinelRef.current); return () => observer.disconnect(); - }, [stickyActions, stickyScrollViewport]); + }, [stickyActions, scrollViewport]); useEffect(() => { const handleHashChange = () => { @@ -561,7 +564,7 @@ export const Viewer = forwardRef(({ if (!anchor) return false; const container = containerRef.current; - if (!container || !stickyScrollViewport) return false; + if (!container || !scrollViewport) return false; const target = document.getElementById(anchor); if (!target || !container.contains(target)) return false; @@ -573,27 +576,27 @@ export const Viewer = forwardRef(({ const headerOffset = stickyActionsEl ? stickyActionsEl.getBoundingClientRect().height + stickyTop : 0; - const containerRect = stickyScrollViewport.getBoundingClientRect(); + const containerRect = scrollViewport.getBoundingClientRect(); const targetRect = target.getBoundingClientRect(); const relativeTop = targetRect.top - containerRect.top; - const offsetPosition = stickyScrollViewport.scrollTop + relativeTop - headerOffset; + const offsetPosition = scrollViewport.scrollTop + relativeTop - headerOffset; - stickyScrollViewport.scrollTo({ + scrollViewport.scrollTo({ top: Math.max(0, offsetPosition), behavior: 'smooth', }); return true; - }, [stickyScrollViewport]); + }, [scrollViewport]); useEffect(() => { - if (!stickyScrollViewport || !locationHash || lastAutoScrolledHashRef.current === locationHash) return; + if (!scrollViewport || !locationHash || lastAutoScrolledHashRef.current === locationHash) return; const timer = window.setTimeout(() => { if (scrollToAnchor(locationHash)) { lastAutoScrolledHashRef.current = locationHash; } }, 0); return () => window.clearTimeout(timer); - }, [blocks, locationHash, scrollToAnchor, stickyScrollViewport]); + }, [blocks, locationHash, scrollToAnchor, scrollViewport]); // Use the native copy event so clipboard writes are synchronous (Safari // rejects the async navigator.clipboard API outside the user-gesture window). diff --git a/packages/ui/hooks/useVimSelection.ts b/packages/ui/hooks/useVimSelection.ts index 5af5ef0d8..ee76f4f6f 100644 --- a/packages/ui/hooks/useVimSelection.ts +++ b/packages/ui/hooks/useVimSelection.ts @@ -47,11 +47,14 @@ import { type VimVisualBlockState, type VimVisualState, } from '../utils/vimNavigation'; +import { scrollVimTargetIntoView } from '../utils/vimScroll'; import { useVimDocumentFocus } from './useVimDocumentFocus'; /** Inputs required by the Markdown semantic Vim controller. */ export interface UseVimSelectionOptions { readonly containerRef: RefObject; + /** The element that actually scrolls (ScrollViewportContext value). */ + readonly scrollViewport?: HTMLElement | null; readonly enabled: boolean; readonly hudEnabled: boolean; readonly blocked: boolean; @@ -234,6 +237,7 @@ function applyVisualBlockSelection( */ export function useVimSelection({ containerRef, + scrollViewport, enabled, hudEnabled, blocked, @@ -257,6 +261,11 @@ export function useVimSelection({ const pointerFocusRef = useRef(false); const restoringFocusRef = useRef(false); + // Read the live scroll viewport without adding a dependency to every + // navigation callback below. + const scrollViewportRef = useRef(scrollViewport); + scrollViewportRef.current = scrollViewport; + const setState = useCallback((next: VimSelectionState) => { stateRef.current = next; setStateValue(next); @@ -283,7 +292,7 @@ export function useVimSelection({ const next: VimBlockState = { phase: 'block', targetKey: initial.key }; setState(next); window.getSelection()?.removeAllRanges(); - initial.element.scrollIntoView({ block: 'nearest' }); + scrollVimTargetIntoView(initial.element, scrollViewportRef.current); return next; }, [containerRef, setState]); @@ -358,7 +367,7 @@ export function useVimSelection({ const setSemanticTarget = useCallback((target: SemanticTarget) => { setState(semanticStateForTarget(target)); window.getSelection()?.removeAllRanges(); - target.element.scrollIntoView({ block: 'nearest' }); + scrollVimTargetIntoView(target.element, scrollViewportRef.current); }, [setState]); const updateTextState = useCallback(( @@ -373,9 +382,9 @@ export function useVimSelection({ normalized.cursor, normalized.phase === 'visual' ? normalized.anchor : null, ); - resolveTextPosition(graph.container, normalized.cursor) - ?.node.parentElement - ?.scrollIntoView({ block: 'nearest' }); + const cursorParent = resolveTextPosition(graph.container, normalized.cursor) + ?.node.parentElement; + if (cursorParent) scrollVimTargetIntoView(cursorParent, scrollViewportRef.current); }, [setState]); const enterTextAtTarget = useCallback(( @@ -420,7 +429,7 @@ export function useVimSelection({ if (!getTextElementBounds(graph.container, block.element)) return false; setState(next); applyVisualBlockSelection(graph, next); - block.element.scrollIntoView({ block: 'nearest' }); + scrollVimTargetIntoView(block.element, scrollViewportRef.current); return true; }, [setState]); @@ -730,7 +739,7 @@ export function useVimSelection({ }; setState(nextState); applyVisualBlockSelection(graph, nextState); - next.element.scrollIntoView({ block: 'nearest' }); + scrollVimTargetIntoView(next.element, scrollViewportRef.current); return true; } if (key === 'o') { diff --git a/packages/ui/utils/vimScroll.test.ts b/packages/ui/utils/vimScroll.test.ts new file mode 100644 index 000000000..564931aad --- /dev/null +++ b/packages/ui/utils/vimScroll.test.ts @@ -0,0 +1,261 @@ +import { beforeEach, describe, expect, test } from 'bun:test'; +import { + VIM_SCROLL_MARGIN_MAX, + VIM_SCROLL_MARGIN_MIN, + computeVimScrollDelta, + resolveVimScrollMargin, + scrollVimTargetIntoView, +} from './vimScroll'; + +const hasDom = typeof document !== 'undefined'; + +// A 1000px-tall viewport whose top edge sits at page y=0, with a 200px HUD band +// reserved at each edge — the safe band is [200, 800]. +const viewport = { top: 0, height: 1000 }; +const band = { topMargin: 200, bottomMargin: 200 }; + +describe('computeVimScrollDelta', () => { + test('leaves a target already inside the safe band untouched', () => { + expect(computeVimScrollDelta(viewport, { top: 400, bottom: 460 }, band)).toBe(0); + // Flush against the inner edges of the band still counts as safe. + expect(computeVimScrollDelta(viewport, { top: 200, bottom: 800 }, band)).toBe(0); + }); + + test('reveals a target parked behind the bottom HUD (the j-to-bottom bug)', () => { + // scrollIntoView({ block: 'nearest' }) would pin this line at y≈980, behind + // the bottom HUD. We instead scroll it down to the bottom margin at y=800. + const delta = computeVimScrollDelta(viewport, { top: 960, bottom: 980 }, band); + expect(delta).toBe(180); // 980 - (1000 - 200) + }); + + test('reveals a target parked behind the top HUD (the k-to-top bug)', () => { + // A line at y≈20 sits under the sticky action bar; scroll up to the margin. + const delta = computeVimScrollDelta(viewport, { top: 20, bottom: 40 }, band); + expect(delta).toBe(-180); // 20 - 200 + }); + + test('aligns a target taller than the band to its top edge, not its bottom', () => { + // A 700px block (taller than the 600px safe band) sitting low: honouring the + // bottom margin alone would push its top above the top margin and hide the + // start of the block. Reading order wins — clamp to the top margin instead. + const bottomOnly = 940 - 800; // 140 if we only chased the bottom + const topRoom = 240 - 200; // 40 before the top slips under the margin + const delta = computeVimScrollDelta(viewport, { top: 240, bottom: 940 }, band); + expect(delta).toBe(Math.min(bottomOnly, topRoom)); + expect(delta).toBe(40); + }); + + test('clears the top HUD for a too-tall target whose top is occluded', () => { + // A block spanning 100..900 is taller than the band and straddles both + // edges. Its top sits behind the top HUD (100 < 200), so reveal the start + // of the block by scrolling up to the top margin — reading order wins. + expect(computeVimScrollDelta(viewport, { top: 100, bottom: 900 }, band)).toBe(-100); + }); + + test('leaves a too-tall target straddling the band untouched once its top is clear', () => { + // Top already at the margin, bottom past it: moving either way would hide an + // edge, so hold position. + expect(computeVimScrollDelta(viewport, { top: 200, bottom: 900 }, band)).toBe(0); + }); + + test('accounts for a viewport offset from the page top', () => { + const offset = { top: 300, height: 400 }; // safe band = page [400, 500] + const smallBand = { topMargin: 100, bottomMargin: 200 }; + // Target at page y=650..680 is below the safe bottom (500) → scroll down 180. + expect(computeVimScrollDelta(offset, { top: 650, bottom: 680 }, smallBand)).toBe(180); + }); +}); + +describe('resolveVimScrollMargin', () => { + test('uses the 20% ratio in the ordinary range', () => { + expect(resolveVimScrollMargin(600)).toBe(120); + }); + + test('clamps short viewports up to the minimum', () => { + expect(resolveVimScrollMargin(50)).toBe(VIM_SCROLL_MARGIN_MIN); + }); + + test('clamps tall viewports down to the maximum', () => { + expect(resolveVimScrollMargin(4000)).toBe(VIM_SCROLL_MARGIN_MAX); + }); +}); + +describe.if(hasDom)('scrollVimTargetIntoView', () => { + // Clean even when an assertion fires before a test's own teardown, so a + // failure never leaks stub HUDs into the next test. + beforeEach(() => { + document.body.replaceChildren(); + }); + + function stubRect(element: HTMLElement, rect: Partial): void { + const full = { top: 0, bottom: 0, left: 0, right: 0, width: 0, height: 0, x: 0, y: 0 }; + const merged = { ...full, ...rect }; + // Keep the rect internally consistent like a real getBoundingClientRect: + // deriving bottom/right from top/left + height/width when only those are + // stubbed. + if (rect.bottom === undefined && rect.height !== undefined) { + merged.bottom = merged.top + merged.height; + } + if (rect.right === undefined && rect.width !== undefined) { + merged.right = merged.left + merged.width; + } + element.getBoundingClientRect = () => merged as DOMRect; + } + + function buildViewport(clientHeight: number): { + viewport: HTMLElement; + target: HTMLElement; + } { + const viewportEl = document.createElement('div'); + // The real app scrolls a native
the caller passes in explicitly via + // ScrollViewportContext; no rediscovery attribute exists on it. + Object.defineProperty(viewportEl, 'clientHeight', { + configurable: true, + value: clientHeight, + }); + viewportEl.scrollTop = 0; + const target = document.createElement('p'); + viewportEl.appendChild(target); + document.body.appendChild(viewportEl); + return { viewport: viewportEl, target }; + } + + // The HUD widgets are portaled to document.body, so tests mount them there. + function addHud( + attribute: string, + rect: Partial, + expanded = false, + ): void { + const hud = document.createElement('div'); + hud.setAttribute(attribute, ''); + if (attribute === 'data-vim-key-hud') { + hud.setAttribute('data-expanded', expanded ? 'true' : 'false'); + } + stubRect(hud, rect); + document.body.appendChild(hud); + } + + test('falls back to the ratio band when no HUD is mounted yet', () => { + const { viewport: viewportEl, target } = buildViewport(1000); + stubRect(viewportEl, { top: 0, height: 1000 }); + // Margin = clamp(1000 * 0.2) = 160. Target at 950..970 is behind it. + stubRect(target, { top: 950, bottom: 970, height: 20, width: 100 }); + + // The native-scroll host carries no attribute; the caller passes it in. + scrollVimTargetIntoView(target, viewportEl); + + // 970 - (1000 - 160) = 130. + expect(viewportEl.scrollTop).toBe(130); + }); + + test('derives the bottom band from the live key HUD rect', () => { + const { viewport: viewportEl, target } = buildViewport(1000); + stubRect(viewportEl, { top: 0, height: 1000 }); + // The real key HUD is fixed at bottom: 150 with height 88, so its top sits + // at 762 on a 1000px viewport — a 246px band, past the 160 ratio clamp. + addHud('data-vim-key-hud', { top: 762, bottom: 850, height: 88 }); + stubRect(target, { top: 950, bottom: 970, height: 20, width: 100 }); + + scrollVimTargetIntoView(target, viewportEl); + + // safeBottom = 1000 - 246 = 754; delta = 970 - 754 = 216. + expect(viewportEl.scrollTop).toBe(216); + }); + + test('shrinks the bottom band to the mode pill instead of over-reserving', () => { + const { viewport: viewportEl, target } = buildViewport(1000); + stubRect(viewportEl, { top: 0, height: 1000 }); + // The default pill floats at bottom-4 and is ~25px tall: top at 959, so + // the derived band is 49px — not the 160px ratio reserve. + addHud('data-vim-mode-badge', { top: 959, bottom: 984, height: 25 }); + // Target at 900..920 was "behind the HUD" under the ratio band but is + // genuinely clear of the pill, so it must not scroll. + stubRect(target, { top: 900, bottom: 920, height: 20, width: 100 }); + + scrollVimTargetIntoView(target, viewportEl); + + expect(viewportEl.scrollTop).toBe(0); + }); + + test('still reveals a target that is genuinely behind the mode pill', () => { + const { viewport: viewportEl, target } = buildViewport(1000); + stubRect(viewportEl, { top: 0, height: 1000 }); + addHud('data-vim-mode-badge', { top: 959, bottom: 984, height: 25 }); + stubRect(target, { top: 960, bottom: 980, height: 20, width: 100 }); + + scrollVimTargetIntoView(target, viewportEl); + + // safeBottom = 1000 - 49 = 951; delta = 980 - 951 = 29. + expect(viewportEl.scrollTop).toBe(29); + }); + + test('skips the widened band while the key HUD is expanded (modal state)', () => { + const { viewport: viewportEl, target } = buildViewport(1000); + stubRect(viewportEl, { top: 0, height: 1000 }); + // Expanded, the HUD can stand taller than the viewport; the band stays at + // the ratio margin until the user collapses it. + addHud('data-vim-key-hud', { top: 200, bottom: 760, height: 560 }, true); + stubRect(target, { top: 950, bottom: 970, height: 20, width: 100 }); + + scrollVimTargetIntoView(target, viewportEl); + + // Ratio band: 970 - (1000 - 160) = 130. + expect(viewportEl.scrollTop).toBe(130); + }); + + test('leaves scrollTop untouched when the target is already safe', () => { + const { viewport: viewportEl, target } = buildViewport(1000); + stubRect(viewportEl, { top: 0, height: 1000 }); + stubRect(target, { top: 480, bottom: 500, height: 20, width: 100 }); + + scrollVimTargetIntoView(target, viewportEl); + + expect(viewportEl.scrollTop).toBe(0); + }); + + test('widens the top band to clear a sticky action bar', () => { + // Realistic geometry: the sticky cluster is 44px tall, and the widening + // only matters on a short viewport where the ratio band (here 40px) is + // smaller than the cluster. + const { viewport: viewportEl, target } = buildViewport(200); + stubRect(viewportEl, { top: 0, height: 200 }); + const sticky = document.createElement('div'); + sticky.setAttribute('data-sticky-actions', ''); + viewportEl.appendChild(sticky); + stubRect(sticky, { top: 0, bottom: 44, height: 44 }); + // Target at 46..66 sits under the sticky-derived margin (44 + 8 = 52) but + // below the 40px ratio band. Start scrolled down so the upward delta stays + // inside the range a real browser allows (scrollTop clamps at 0). + stubRect(target, { top: 46, bottom: 66, height: 20, width: 100 }); + viewportEl.scrollTop = 20; + + scrollVimTargetIntoView(target, viewportEl); + + // 20 + (46 - 52) = 14. + expect(viewportEl.scrollTop).toBe(14); + }); + + test('falls back to scrollIntoView when there is no scroll viewport', () => { + const orphan = document.createElement('p'); + document.body.appendChild(orphan); + let called = false; + orphan.scrollIntoView = () => { + called = true; + }; + + scrollVimTargetIntoView(orphan); + + expect(called).toBe(true); + }); + + test('ignores a zero-size target (unrendered / collapsed)', () => { + const { viewport: viewportEl, target } = buildViewport(1000); + stubRect(viewportEl, { top: 0, height: 1000 }); + stubRect(target, { top: 0, bottom: 0, height: 0, width: 0 }); + viewportEl.scrollTop = 42; + + scrollVimTargetIntoView(target, viewportEl); + + expect(viewportEl.scrollTop).toBe(42); + }); +}); diff --git a/packages/ui/utils/vimScroll.ts b/packages/ui/utils/vimScroll.ts new file mode 100644 index 000000000..6c4d3622d --- /dev/null +++ b/packages/ui/utils/vimScroll.ts @@ -0,0 +1,162 @@ +/** + * Keep the Vim cursor clear of the HUD bands that hug the viewport edges. + * + * `Element.scrollIntoView({ block: 'nearest' })` parks a target flush against + * the nearest viewport edge — exactly where the sticky action bar (top) and the + * key HUD / status pill (bottom) float. Keyboard motion then lands the caret + * behind an overlay, while a mouse wheel (which the browser lets overshoot) + * keeps the same text nearer the centre. These helpers reproduce that mouse + * feel: a target inside the safe band never scrolls, and one that strays into a + * HUD band is revealed with a margin instead of being pinned to the edge. + */ + +/** A viewport's vertical geometry, relative to the page. */ +export interface VimScrollViewportRect { + readonly top: number; + readonly height: number; +} + +/** A target's vertical extent, relative to the page. */ +export interface VimScrollTargetRect { + readonly top: number; + readonly bottom: number; +} + +/** The occluded strips to keep the caret out of, measured from each edge. */ +export interface VimScrollBand { + readonly topMargin: number; + readonly bottomMargin: number; +} + +/** Fraction of the viewport height reserved as a HUD band at each edge. */ +export const VIM_SCROLL_MARGIN_RATIO = 0.2; +/** Lower clamp so short viewports still leave a usable margin. */ +export const VIM_SCROLL_MARGIN_MIN = 24; +/** Upper clamp so tall viewports do not reserve most of the screen. */ +export const VIM_SCROLL_MARGIN_MAX = 160; + +/** + * How far to move `scrollTop` so `target` clears the HUD bands. + * + * Returns a signed delta (negative scrolls up, positive scrolls down) 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, so the start of the + * block is never pushed above the top margin to chase its bottom. + */ +export function computeVimScrollDelta( + viewport: VimScrollViewportRect, + target: VimScrollTargetRect, + band: VimScrollBand, +): number { + const relativeTop = target.top - viewport.top; + const relativeBottom = target.bottom - viewport.top; + const safeTop = band.topMargin; + const safeBottom = viewport.height - band.bottomMargin; + + // Behind the top HUD → scroll up just enough to reach the top margin. + if (relativeTop < safeTop) return relativeTop - safeTop; + + // Behind the bottom HUD → scroll down, but never past the point where the + // target's top would slip under the top margin. + if (relativeBottom > safeBottom) { + const bottomDelta = relativeBottom - safeBottom; + const topRoom = relativeTop - safeTop; + return Math.min(bottomDelta, Math.max(0, topRoom)); + } + + return 0; +} + +/** Resolve the ratio-based HUD margin, clamped for very short or tall viewports. */ +export function resolveVimScrollMargin(viewportHeight: number): number { + return Math.min( + Math.max(viewportHeight * VIM_SCROLL_MARGIN_RATIO, VIM_SCROLL_MARGIN_MIN), + VIM_SCROLL_MARGIN_MAX, + ); +} + +/** + * Top edge of the lowest floating Vim HUD, or `undefined` when none is shown. + * + * The key HUD and the mode badge are portaled to `document.body`, outside the + * scroll viewport's subtree, so the query is necessarily document-wide. It is + * still scoped to `element.ownerDocument` (never the global `document`) so a + * host mounted inside another document measures its own HUD, and because both + * widgets are fixed-position singletons, a host mounting two viewers in one + * document gets the same band geometry from either instance. + * + * The expanded key HUD is deliberately skipped: it is a modal state that can + * stand taller than the viewport, so no band could clear it — scrolling keeps + * the ratio margin until the user collapses it. + */ +function vimHudBandTop(element: HTMLElement): number | undefined { + const doc = element.ownerDocument; + const keyHud = doc.querySelector('[data-vim-key-hud]'); + const badge = doc.querySelector('[data-vim-mode-badge]'); + const tops: number[] = []; + if (keyHud && keyHud.getAttribute('data-expanded') !== 'true') { + const rect = keyHud.getBoundingClientRect(); + if (rect.height > 0) tops.push(rect.top); + } + if (badge) { + const rect = badge.getBoundingClientRect(); + if (rect.height > 0) tops.push(rect.top); + } + return tops.length > 0 ? Math.min(...tops) : undefined; +} + +/** + * Scroll `element` into view while keeping it clear of the Vim HUD bands. + * + * `scrollViewport` is the element that actually scrolls — the caller passes the + * value it already holds from ScrollViewportContext (the same node the reticle + * measures against), because the native-scroll host carries no attribute that + * would rediscover it. When it is absent the helper falls back to the + * historical `scrollIntoView({ block: 'nearest' })`, so behaviour never + * regresses. + * + * Both bands are measured from live geometry rather than guessed constants: + * the top band clears the sticky action bar when present, and the bottom band + * clears the floating key HUD / mode pill, so the caret is never parked behind + * either overlay — and the default pill configuration no longer reserves the + * full ratio band for a 25px widget. + */ +export function scrollVimTargetIntoView( + element: HTMLElement, + scrollViewport?: HTMLElement | null, +): void { + const viewport = scrollViewport ?? null; + if (!viewport) { + element.scrollIntoView({ block: 'nearest' }); + return; + } + + const viewportRect = viewport.getBoundingClientRect(); + const targetRect = element.getBoundingClientRect(); + if (targetRect.height === 0 && targetRect.width === 0) return; + + const margin = resolveVimScrollMargin(viewport.clientHeight); + const stickyBottom = viewport + .querySelector('[data-sticky-actions]') + ?.getBoundingClientRect().bottom; + const topMargin = stickyBottom !== undefined + ? Math.max(margin, stickyBottom - viewportRect.top + 8) + : margin; + + // Mirror of the top band: keep the caret above the floating HUD by the same + // 8px gap. The HUD rect wins over the ratio band whenever it is larger (the + // key HUD needs ~238px, well past the 160px clamp) and is allowed to shrink + // past it when only the small mode pill floats (floor: the minimum margin). + const hudTop = vimHudBandTop(element); + const bottomMargin = hudTop !== undefined + ? Math.max(VIM_SCROLL_MARGIN_MIN, viewportRect.bottom - hudTop + 8) + : margin; + + const delta = computeVimScrollDelta( + { top: viewportRect.top, height: viewport.clientHeight }, + { top: targetRect.top, bottom: targetRect.bottom }, + { topMargin, bottomMargin }, + ); + if (delta === 0) return; + viewport.scrollTop += delta; +}