Skip to content

fix: cursor drift during vertical arrow navigation (SD-1689) #1918

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
luccas-harbour wants to merge 21 commits into
base: main
Choose a base branch
from
Open

fix: cursor drift during vertical arrow navigation (SD-1689) #1918

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
4482642
feat: add plugin for handling vertical arrow navigation
Jan 30, 2026
4a19ac6
feat: keep track of X position during vertical navigation
Jan 30, 2026
58239dc
feat: compute next Y position after vertical navigation
Jan 30, 2026
7e92d8e
feat: add helper to convert back to client-space coordinates
Jan 30, 2026
158291c
fix: compute caret position based on closest entry
Feb 2, 2026
754fa3c
feat: expose caret layout computation method
Feb 2, 2026
266cf82
feat: use caret computation to determine current coordinates
Jan 30, 2026
4eb5562
feat: change selection after navigation
Jan 30, 2026
4b4b523
feat: add function for computing page Y offset
Feb 2, 2026
53f9287
fix: adjust coordinates denormalization to account for Y offset
Feb 2, 2026
240c2b1
docs: add documentation to vertical navigation plugin
Feb 2, 2026
529c0cd
test: add tests for the new plugin
Feb 2, 2026
d84a95a
fix: account for zomm during height computation
Feb 3, 2026
9cecef3
refactor: retrieve page element using existing helper
luccas-harbour Feb 4, 2026
f656d53
refactor: simplify getPageOffsetY to call existing helper
luccas-harbour Feb 4, 2026
1b2d158
refactor: use lodash for binary search
luccas-harbour Feb 4, 2026
5bc1189
chore: add lodash as a dependency
luccas-harbour Feb 4, 2026
e8cf464
refactor: reuse variable in adjacent line computation
luccas-harbour Feb 4, 2026
16f2ee2
fix: selection computation
luccas-harbour Feb 4, 2026
d4fd979
fix: always use findEntryClosestToPosition when computing caret position
luccas-harbour Feb 5, 2026
2c62bd0
fix: account for page offset when normalizing point
luccas-harbour Feb 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/super-editor/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@
"eventemitter3": "catalog:",
"he": "catalog:",
"jszip": "catalog:",
"lodash": "^4.17.21",
"marked": "catalog:",
"prosemirror-commands": "catalog:",
"prosemirror-dropcursor": "catalog:",
Expand Down
49 changes: 42 additions & 7 deletions packages/super-editor/src/core/presentation-editor/PresentationEditor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,12 @@ import {
import {
convertPageLocalToOverlayCoords as convertPageLocalToOverlayCoordsFromTransform,
getPageOffsetX as getPageOffsetXFromTransform,
getPageOffsetY as getPageOffsetYFromTransform,
} from './dom/CoordinateTransform.js';
import { normalizeClientPoint as normalizeClientPointFromPointer } from './dom/PointerNormalization.js';
import {
normalizeClientPoint as normalizeClientPointFromPointer,
denormalizeClientPoint as denormalizeClientPointFromPointer,
} from './dom/PointerNormalization.js';
import { getPageElementByIndex } from './dom/PageDom.js';
import { inchesToPx, parseColumns } from './layout/LayoutOptionParsing.js';
import { createLayoutMetrics as createLayoutMetricsFromHelper } from './layout/PresentationLayoutMetrics.js';
Expand Down Expand Up @@ -1701,9 +1705,7 @@ export class PresentationEditor extends EventEmitter {
const isLeftMargin = marginLeft > 0 && x < marginLeft;
const isRightMargin = marginRight > 0 && x > pageWidth - marginRight;

const pageEl = this.#viewportHost.querySelector(
`.superdoc-page[data-page-index="${pageIndex}"]`,
) as HTMLElement | null;
const pageEl = getPageElementByIndex(this.#viewportHost, pageIndex);
if (!pageEl) {
return null;
}
Expand Down Expand Up @@ -2968,9 +2970,7 @@ export class PresentationEditor extends EventEmitter {
previousMeasures,
);
const incrementalLayoutEnd = perfNow();
perfLog(
`[Perf] incrementalLayout: ${(incrementalLayoutEnd - incrementalLayoutStart).toFixed(2)}ms`,
);
perfLog(`[Perf] incrementalLayout: ${(incrementalLayoutEnd - incrementalLayoutStart).toFixed(2)}ms`);

// Type guard: validate incrementalLayout return value
if (!result || typeof result !== 'object') {
Expand Down Expand Up @@ -4447,6 +4447,15 @@ export class PresentationEditor extends EventEmitter {
});
}

#getPageOffsetY(pageIndex: number): number | null {
return getPageOffsetYFromTransform({
painterHost: this.#painterHost,
viewportHost: this.#viewportHost,
zoom: this.#layoutOptions.zoom ?? 1,
pageIndex,
});
}

#convertPageLocalToOverlayCoords(
pageIndex: number,
pageLocalX: number,
Expand Down Expand Up @@ -4513,12 +4522,34 @@ export class PresentationEditor extends EventEmitter {
visibleHost: this.#visibleHost,
zoom: this.#layoutOptions.zoom ?? 1,
getPageOffsetX: (pageIndex) => this.#getPageOffsetX(pageIndex),
getPageOffsetY: (pageIndex) => this.#getPageOffsetY(pageIndex),
},
clientX,
clientY,
);
}

denormalizeClientPoint(
layoutX: number,
layoutY: number,
pageIndex?: number,
height?: number,
): { x: number; y: number; height?: number } | null {
return denormalizeClientPointFromPointer(
{
viewportHost: this.#viewportHost,
visibleHost: this.#visibleHost,
zoom: this.#layoutOptions.zoom ?? 1,
getPageOffsetX: (pageIndex) => this.#getPageOffsetX(pageIndex),
getPageOffsetY: (pageIndex) => this.#getPageOffsetY(pageIndex),
},
layoutX,
layoutY,
pageIndex,
height,
);
}

/**
* Computes caret layout rectangle using geometry-based calculations.
*
Expand Down Expand Up @@ -4602,6 +4633,10 @@ export class PresentationEditor extends EventEmitter {
return geometry;
}

computeCaretLayoutRect(pos: number): { pageIndex: number; x: number; y: number; height: number } | null {
return this.#computeCaretLayoutRect(pos);
}

#getCurrentPageIndex(): number {
const session = this.#headerFooterSession?.session;
if (session && session.mode !== 'body') {
Expand Down
21 changes: 18 additions & 3 deletions packages/super-editor/src/core/presentation-editor/dom/CoordinateTransform.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { getPageElementByIndex } from './PageDom.js';

/**
* Calculates the offset of a page element within the viewport.
*
Expand Down Expand Up @@ -37,9 +39,7 @@ function getPageOffsets(options: {
return null;
}

const pageEl = options.painterHost.querySelector(
`.superdoc-page[data-page-index="${options.pageIndex}"]`,
) as HTMLElement | null;
const pageEl = getPageElementByIndex(options.painterHost, options.pageIndex);
if (!pageEl) return null;

const pageRect = pageEl.getBoundingClientRect();
Expand All @@ -66,6 +66,21 @@ export function getPageOffsetX(options: {
return getPageOffsets(options)?.x ?? null;
}

/**
* Calculates the vertical offset of a page element within the viewport.
*
* @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: {
Copy link
Contributor

@afn afn Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

@luccas-harbour luccas-harbour Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call! I should have double checked this one

painterHost: HTMLElement | null;
viewportHost: HTMLElement | null;
zoom: number;
pageIndex: number;
}): number | null {
return getPageOffsets(options)?.y ?? null;
}

/**
* Converts page-local coordinates to overlay-absolute coordinates.
*
Expand Down
35 changes: 34 additions & 1 deletion packages/super-editor/src/core/presentation-editor/dom/DomPositionIndex.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { DOM_CLASS_NAMES } from '@superdoc/painter-dom';

import { sortedIndexBy } from 'lodash';
import { debugLog, getSelectionDebugConfig } from '../selection/SelectionDebug.js';

/**
Expand Down Expand Up @@ -224,6 +224,39 @@ export class DomPositionIndex {
return entry;
}

/**
* Finds the index entry that either contains the given position,
* or is the closest entry before or after it.
* @param pos - The ProseMirror position to look up
* @returns The closest entry to this position, or null if the index is empty
* @remarks
* This method first attempts to find an entry that contains the position.
* If none is found, it then finds the closest entry before or after the position.
* If the index is empty, it returns null.
*/
findEntryClosestToPosition(pos: number): DomPositionIndexEntry | null {
if (!Number.isFinite(pos)) return null;
const entries = this.#entries;
if (entries.length === 0) return null;

const entryAtPos = this.findEntryAtPosition(pos);
if (entryAtPos) return entryAtPos;

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

const beforeEntry = idx >= 0 ? entries[idx] : null;
const afterEntry = idx < entries.length - 1 ? entries[idx + 1] : null;

if (beforeEntry && afterEntry) {
const distBefore = pos - beforeEntry.pmEnd;
const distAfter = afterEntry.pmStart - pos;
return distBefore <= distAfter ? beforeEntry : afterEntry;
}
if (beforeEntry) return beforeEntry;
if (afterEntry) return afterEntry;
return null;
}

findElementAtPosition(pos: number): HTMLElement | null {
return this.findEntryAtPosition(pos)?.el ?? null;
}
Expand Down
4 changes: 2 additions & 2 deletions packages/super-editor/src/core/presentation-editor/dom/DomSelectionGeometry.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -559,10 +559,10 @@ export function computeDomCaretPageLocal(
options.rebuildDomPositionIndex();
}

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

Expand Down
172 changes: 172 additions & 0 deletions packages/super-editor/src/core/presentation-editor/dom/PointerNormalization.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,172 @@
import { afterEach, describe, expect, it, vi } from 'vitest';

import { denormalizeClientPoint, normalizeClientPoint } from './PointerNormalization.js';

describe('PointerNormalization', () => {
const makeHosts = () => {
const viewportHost = document.createElement('div');
const visibleHost = document.createElement('div');
viewportHost.appendChild(visibleHost);

vi.spyOn(viewportHost, 'getBoundingClientRect').mockReturnValue({
left: 20,
top: 10,
right: 0,
bottom: 0,
width: 0,
height: 0,
x: 0,
y: 0,
toJSON: () => ({}),
} as DOMRect);

(visibleHost as HTMLElement & { scrollLeft: number; scrollTop: number }).scrollLeft = 30;
(visibleHost as HTMLElement & { scrollLeft: number; scrollTop: number }).scrollTop = 40;

return { viewportHost, visibleHost };
};

afterEach(() => {
vi.restoreAllMocks();
delete (document as Document & { elementsFromPoint?: Document['elementsFromPoint'] }).elementsFromPoint;
});

describe('normalizeClientPoint', () => {
it('returns null for non-finite client coordinates', () => {
const { viewportHost, visibleHost } = makeHosts();

const options = {
viewportHost,
visibleHost,
zoom: 1,
getPageOffsetX: () => 0,
getPageOffsetY: () => 0,
};

expect(normalizeClientPoint(options, NaN, 0)).toBe(null);
expect(normalizeClientPoint(options, 0, Infinity)).toBe(null);
expect(normalizeClientPoint(options, -Infinity, 0)).toBe(null);
});

it('normalizes client coordinates to layout coordinates with zoom and scroll', () => {
const { viewportHost, visibleHost } = makeHosts();

const options = {
viewportHost,
visibleHost,
zoom: 2,
getPageOffsetX: () => 0,
getPageOffsetY: () => 0,
};

const result = normalizeClientPoint(options, 200, 150);
expect(result).toEqual({ x: 105, y: 90 });
});

it('adjusts X when the pointer is over a page with a known offset', () => {
const { viewportHost, visibleHost } = makeHosts();
const pageEl = document.createElement('div');
pageEl.className = 'superdoc-page';
pageEl.dataset.pageIndex = '2';

(document as Document & { elementsFromPoint: Document['elementsFromPoint'] }).elementsFromPoint = vi
.fn()
.mockReturnValue([pageEl]);

const options = {
viewportHost,
visibleHost,
zoom: 2,
getPageOffsetX: (pageIndex: number) => (pageIndex === 2 ? 12 : null),
getPageOffsetY: (pageIndex: number) => (pageIndex === 2 ? 8 : null),
};

const result = normalizeClientPoint(options, 200, 150);
expect(result).toEqual({ x: 93, y: 82 });
});

it('does not adjust X when page offset is unavailable', () => {
const { viewportHost, visibleHost } = makeHosts();
const pageEl = document.createElement('div');
pageEl.className = 'superdoc-page';
pageEl.dataset.pageIndex = '3';

(document as Document & { elementsFromPoint: Document['elementsFromPoint'] }).elementsFromPoint = vi
.fn()
.mockReturnValue([pageEl]);

const options = {
viewportHost,
visibleHost,
zoom: 2,
getPageOffsetX: () => null,
getPageOffsetY: () => null,
};

const result = normalizeClientPoint(options, 200, 150);
expect(result).toEqual({ x: 105, y: 90 });
});
});

describe('denormalizeClientPoint', () => {
it('returns null for non-finite layout coordinates', () => {
const { viewportHost, visibleHost } = makeHosts();

const options = {
viewportHost,
visibleHost,
zoom: 1,
getPageOffsetX: () => 0,
getPageOffsetY: () => 0,
};

expect(denormalizeClientPoint(options, NaN, 0)).toBe(null);
expect(denormalizeClientPoint(options, 0, Infinity)).toBe(null);
});

it('denormalizes layout coordinates to client coordinates', () => {
const { viewportHost, visibleHost } = makeHosts();

const options = {
viewportHost,
visibleHost,
zoom: 2,
getPageOffsetX: () => 0,
getPageOffsetY: () => 0,
};

const result = denormalizeClientPoint(options, 50, 60);
expect(result).toEqual({ x: 90, y: 90 });
});

it('applies page offsets when a page index is provided', () => {
const { viewportHost, visibleHost } = makeHosts();

const options = {
viewportHost,
visibleHost,
zoom: 2,
getPageOffsetX: () => 15,
getPageOffsetY: () => 25,
};

const result = denormalizeClientPoint(options, 50, 60, 3);
expect(result).toEqual({ x: 120, y: 140 });
});

it('scales height based on the zoom level when provided', () => {
const { viewportHost, visibleHost } = makeHosts();

const options = {
viewportHost,
visibleHost,
zoom: 1.5,
getPageOffsetX: () => 0,
getPageOffsetY: () => 0,
};

const result = denormalizeClientPoint(options, 10, 12, undefined, 8);
expect(result).toEqual({ x: 5, y: -12, height: 12 });
});
});
});
Loading