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

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

caio-pizzol · 2026-02-05T11:10:36Z

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

+  let entry = options.domPositionIndex.findEntryClosestToPosition(pos);
  if (entry && !entry.el.isConnected) {
    options.rebuildDomPositionIndex();
    entry = options.domPositionIndex.findEntryAtPosition(pos);


is it on purpose to use findEntryAtPosition here instead of findEntryClosestToPosition?

I hadn't modified it because during my tests execution never entered the if statement but now after checking the code, I see it makes sense to change it there too. Done.

caio-pizzol · 2026-02-05T11:12:09Z

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

+ * @param height - Optional layout height to scale into screen pixels.
+ * @returns The client-space point (and optional height), or null if inputs are not finite.
+ */
+export function denormalizeClientPoint(


q: denormalizeClientPoint adds pageOffsetY but normalizeClientPoint doesn't subtract it - is this intentional?

I hadn't touched normalizeClientPoint as I saw it was being used in other places but now I investigated it a bit more deeply and indeed that looks like a mistake. I fixed it.

caio-pizzol · 2026-02-05T11:14:29Z

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

+      baseX = layoutX + pageOffsetX;
+    }
+
+    const pageOffsetY = options.getPageOffsetY(pageIndex);


nit: X uses let baseX = layoutX then modifies baseX, but Y mutates the param directly

luccas-harbour · 2026-02-05T15:07:38Z

@caio-pizzol this PR will likely conflict with this one: #1944

let's wait until that other one is merged and then I can handle the conflicts and we can merge this one

harbournick · 2026-02-06T02:29:52Z

@luccas-harbour fyi #1944 is now merged

luccas-harbour · 2026-02-06T10:10:44Z

@harbournick @caio-pizzol this has been rebased on top of main and is ready for re-review/merge

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-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 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

caio-pizzol reviewed Feb 4, 2026

View reviewed changes

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

caio-pizzol reviewed Feb 5, 2026

View reviewed changes

caio-pizzol requested review from caio-pizzol and removed request for caio-pizzol February 5, 2026 11:15

luccas-harbour force-pushed the luccas/sd-1689-fix-cursor-drift-during-vertical-arrow-navigation branch 2 times, most recently from 6c2f9f3 to 6d68731 Compare February 5, 2026 18:26

harbournick assigned luccas-harbour Feb 6, 2026

Luccas Correa added 3 commits February 6, 2026 07:00

feat: add plugin for handling vertical arrow navigation

4482642

feat: keep track of X position during vertical navigation

4a19ac6

feat: compute next Y position after vertical navigation

58239dc

Luccas Correa and others added 18 commits February 6, 2026 07:00

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

7e92d8e

fix: compute caret position based on closest entry

158291c

feat: expose caret layout computation method

754fa3c

feat: use caret computation to determine current coordinates

266cf82

feat: change selection after navigation

4eb5562

feat: add function for computing page Y offset

4b4b523

fix: adjust coordinates denormalization to account for Y offset

53f9287

docs: add documentation to vertical navigation plugin

240c2b1

test: add tests for the new plugin

529c0cd

fix: account for zomm during height computation

d84a95a

refactor: retrieve page element using existing helper

9cecef3

refactor: simplify getPageOffsetY to call existing helper

f656d53

refactor: use lodash for binary search

1b2d158

chore: add lodash as a dependency

5bc1189

refactor: reuse variable in adjacent line computation

e8cf464

fix: selection computation

16f2ee2

fix: always use findEntryClosestToPosition when computing caret position

d4fd979

fix: account for page offset when normalizing point

2c62bd0

luccas-harbour force-pushed the luccas/sd-1689-fix-cursor-drift-during-vertical-arrow-navigation branch from 6d68731 to 2c62bd0 Compare February 6, 2026 10:06

luccas-harbour requested a review from harbournick February 6, 2026 10:06

		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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

caio-pizzol Feb 5, 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!

Choose a reason for hiding this comment

Uh oh!

luccas-harbour commented Feb 5, 2026

Uh oh!

harbournick commented Feb 6, 2026

Uh oh!

luccas-harbour commented Feb 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

luccas-harbour commented Feb 3, 2026 •

edited

Loading

luccas-harbour Feb 4, 2026 •

edited

Loading

caio-pizzol Feb 5, 2026 •

edited

Loading