fix: cursor drift during vertical arrow navigation (SD-1689) by luccas-harbour · Pull Request #1918 · superdoc-dev/superdoc

luccas-harbour · 2026-02-03T12:23:44Z

Adds a new plugin that handles vertical navigation using arrow keys
The basic logic is:
1. On first vertical move, record goal X from current caret position (in layout space coordinates).
2. Find adjacent line element in the desired direction.
3. Perform hit test to find target ProseMirror position.
4. Move selection to target position, extending if Shift is held.

linear · 2026-02-03T12:23:47Z

SD-1689 Fix cursor drift during vertical arrow navigation

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 09421f5954

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

packages/super-editor/src/extensions/vertical-navigation/vertical-navigation.js

afn

Looks good to the best of my understanding! Had a couple of minor suggestions but nothing blocking.

afn · 2026-02-03T23:59:54Z

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

+  const pageEl = options.painterHost.querySelector(
+    `.superdoc-page[data-page-index="${options.pageIndex}"]`,


Consider making this a helper function (and replace several other places in the codebase that are querying the DOM for data-page-index) to encapsulate the implementation details

It turns out there was already a helper for this here so I am referencing it instead.

afn · 2026-02-04T00:06:02Z

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

+ * @param options - Configuration object containing DOM elements and page information.
+ * @returns The Y offset in layout-space units, or null if calculation fails.
+ */
+export function getPageOffsetY(options: {


This seems to duplicate the contents of getPageOffsets(). Why not use that method (or wrap it like getPageOffsetX) instead?

Good call! I should have double checked this one

afn · 2026-02-04T00:14:41Z

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

+    // Upper-bound search for pmStart <= pos
+    let lo = 0;
+    let hi = entries.length;
+    while (lo < hi) {
+      const mid = (lo + hi) >>> 1;
+      if (entries[mid].pmStart <= pos) {
+        lo = mid + 1;
+      } else {
+        hi = mid;
+      }
+    }
+
+    const idx = lo - 1;


Binary search is notoriously tricky to get right :) Consider using something like lodash's sortedIndexBy (lodash isn't currently a direct dependency of superdoc-editor, but it's pulled in via naive-ui) to simplify the code and avoid potential pitfalls.

Suggested change

// Upper-bound search for pmStart <= pos

let lo = 0;

let hi = entries.length;

while (lo < hi) {

const mid = (lo + hi) >>> 1;

if (entries[mid].pmStart <= pos) {

lo = mid + 1;

} else {

hi = mid;

}

}

const idx = lo - 1;

const idx = sortedIndexBy(entries, { pmStart: pos }, 'pmStart') - 1;

Sounds good, I changed it

afn · 2026-02-04T00:24:46Z

packages/super-editor/src/extensions/vertical-navigation/vertical-navigation.js

+ * @param {boolean} extend
+ * @returns {import('prosemirror-state').Selection | null}
+ */
+function buildSelection(state, pos, extend) {


Consider moving this function to a place where it can be used by other extensions if needed, perhaps src/core/presentation-editor/selection/SelectionHelpers.ts?

I had a look at this one and it looks quite simple and specific to what is being done in this plugin so I think I'd rather keep it as a local function. I could not find anywhere else in the codebase that would benefit from this helper.

I don't think it would really fit in src/core/presentation-editor/selection/SelectionHelpers.ts as it modifies the selection from the ProseMirror side of things, whereas the code in the SelectionHelpers file deals with rendering concerns from the layout-engine's side.

caio-pizzol

added some comments

caio-pizzol · 2026-02-04T19:31:26Z

packages/super-editor/src/extensions/vertical-navigation/vertical-navigation.js

+  const presentationEditor = editor.presentationEditor;
+  const layoutSpaceCoords = presentationEditor.computeCaretLayoutRect(selection.head);
+  if (!layoutSpaceCoords) return null;
+  const clientCoords = presentationEditor.denormalizeClientPoint(layoutSpaceCoords.x, layoutSpaceCoords.y, layoutSpaceCoords.pageIndex, layoutSpaceCoords.height);


denormalizeClientPoint can return null but we're accessing .x without a guard

this could throw if hosts are missing or coords invalid

That function will only return null if the values of x and y are null. Those values come from the call to presentationEditor.computeCaretLayoutRect(selection.head) which will never return null for these values.

caio-pizzol · 2026-02-04T19:32:30Z

packages/super-editor/src/extensions/vertical-navigation/vertical-navigation.js

+  const pageEl = adjacentLine.closest?.(`.${DOM_CLASS_NAMES.PAGE}`);
+  const pageIndex = pageEl ? Number(pageEl.dataset.pageIndex ?? 'NaN') : null;
+  const rect = adjacentLine.getBoundingClientRect();
+  const clientY = rect.top + rect.height / 2;


clientY is computed here then recomputed in the return below

maybe use a variable?

oops, absolutely
fixed

caio-pizzol · 2026-02-04T19:45:44Z

packages/super-editor/src/extensions/vertical-navigation/vertical-navigation.js

+  const clamped = Math.max(0, Math.min(pos, doc.content.size));
+  if (extend) {
+    const anchor = selection.anchor ?? selection.from;
+    return TextSelection.create(doc, clamped, anchor);


args appear reversed - TextSelection.create(doc, anchor, head) expects anchor first.

ref: https://prosemirror.net/docs/ref/#state.TextSelection.constructor

great catch! fixed

caio-pizzol · 2026-02-04T19:53:01Z

packages/super-editor/src/extensions/vertical-navigation/vertical-navigation.js

+ */
+function getHitFromLayoutCoords(editor, goalX, clientY, coords, pageIndex) {
+  const presentationEditor = editor.presentationEditor;
+  const clientPoint = presentationEditor.denormalizeClientPoint(goalX, coords.y, pageIndex);


does crossing page boundaries affects Y and the X calculation here?

it does and that's taken into account via the pageIndex parameter (right here)

Luccas Correa added 10 commits February 3, 2026 09:20

feat: add plugin for handling vertical arrow navigation

a698105

feat: keep track of X position during vertical navigation

226c06b

feat: compute next Y position after vertical navigation

69b6e17

feat: add helper to convert back to client-space coordinates

b75a913

fix: compute caret position based on closest entry

279d297

feat: expose caret layout computation method

d6f4c16

feat: use caret computation to determine current coordinates

662c0f4

feat: change selection after navigation

41f11dd

feat: add function for computing page Y offset

a732682

fix: adjust coordinates denormalization to account for Y offset

23bb02e

luccas-harbour force-pushed the luccas/sd-1689-fix-cursor-drift-during-vertical-arrow-navigation branch from af96315 to 09421f5 Compare February 3, 2026 12:38

luccas-harbour marked this pull request as ready for review February 3, 2026 13:44

luccas-harbour requested a review from caio-pizzol February 3, 2026 13:44

chatgpt-codex-connector bot reviewed Feb 3, 2026

View reviewed changes

packages/super-editor/src/extensions/vertical-navigation/vertical-navigation.js Outdated Show resolved Hide resolved

Luccas Correa added 3 commits February 3, 2026 13:32

docs: add documentation to vertical navigation plugin

910cfa5

test: add tests for the new plugin

3bf9216

fix: account for zomm during height computation

94c3fcf

luccas-harbour force-pushed the luccas/sd-1689-fix-cursor-drift-during-vertical-arrow-navigation branch from 3ded6e3 to 94c3fcf Compare February 3, 2026 16:32

luccas-harbour requested a review from tupizz February 3, 2026 18:43

harbournick requested a review from afn February 3, 2026 21:42

afn reviewed Feb 4, 2026

View reviewed changes

luccas-harbour added 3 commits February 4, 2026 15:00

refactor: retrieve page element using existing helper

8a1c7a8

refactor: simplify getPageOffsetY to call existing helper

6f490d2

refactor: use lodash for binary search

e710bcc

luccas-harbour force-pushed the luccas/sd-1689-fix-cursor-drift-during-vertical-arrow-navigation branch from 4d9c694 to e710bcc Compare February 4, 2026 18:35

luccas-harbour requested a review from afn February 4, 2026 18:40

chore: add lodash as a dependency

1d9e6b7

caio-pizzol reviewed Feb 4, 2026

View reviewed changes

luccas-harbour added 2 commits February 4, 2026 17:06

refactor: reuse variable in adjacent line computation

837f6ab

fix: selection computation

bdaa189

luccas-harbour requested a review from caio-pizzol February 4, 2026 20:12

		const pageEl = options.painterHost.querySelector(
		`.superdoc-page[data-page-index="${options.pageIndex}"]`,

Conversation

luccas-harbour commented Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

linear bot commented Feb 3, 2026

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

afn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

luccas-harbour Feb 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

caio-pizzol left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

luccas-harbour Feb 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

luccas-harbour commented Feb 3, 2026 •

edited

Loading

luccas-harbour Feb 4, 2026 •

edited

Loading

luccas-harbour Feb 4, 2026 •

edited

Loading