Skip to content

fix: resolve drop and click positions in viewport coordinates - #3225

Closed
christianhg wants to merge 1 commit into
mainfrom
event-position-client-y
Closed

fix: resolve drop and click positions in viewport coordinates#3225
christianhg wants to merge 1 commit into
mainfrom
event-position-client-y

Conversation

@christianhg

@christianhg christianhg commented Sep 4, 2026

Copy link
Copy Markdown
Member

Take an editor on a page where the whole window scrolls, like the playground or a plain embed (Studio is unaffected: its panes scroll an inner container). Before scrolling, drag and drop works. Scroll down and drag a block: the drop line now always sits at the bottom edge of whatever block you hover, and dropping always puts the dragged block after it. Dropping before a block becomes impossible.

The cause is a units mix-up. To decide whether the pointer is in the upper or lower half of a block, the code compared the pointer position measured from the top of the page (pageY, which includes the scroll distance) with the block position measured from the top of the visible window (getBoundingClientRect). Once the page is scrolled, the pointer number is bigger than every block number by the scroll distance, so the pointer always looks like it is below everything and every check answers end. The fix measures both from the window: clientY.

A small perf rider: the "above the first block / below the last block" checks now run only when the pointer is over the editor's own padding. Over a block they can never be true, and skipping them removes two layout reads per drag event.

The pinning test sets pageY on the event by hand, the way a real scrolled page would; synthetic events always have pageY equal to clientY, so a real scroll cannot trigger the bug from a test. It fails on the old code. Full editor suite green (2041 passed, 3 expected fail).

@changeset-bot

changeset-bot Bot commented Sep 4, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: acf9123

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 14 packages
Name Type
@portabletext/editor Patch
@portabletext/plugin-character-pair-decorator Patch
@portabletext/plugin-dnd Patch
@portabletext/plugin-emoji-picker Patch
@portabletext/plugin-input-rule Patch
@portabletext/plugin-list-index Patch
@portabletext/plugin-markdown-shortcuts Patch
@portabletext/plugin-one-line Patch
@portabletext/plugin-paste-link Patch
@portabletext/plugin-sdk-value Patch
@portabletext/plugin-table Patch
@portabletext/plugin-typeahead-picker Patch
@portabletext/plugin-typography Patch
@portabletext/toolbar Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel

vercel Bot commented Sep 4, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
portable-text-editor-documentation Ready Ready Preview Sep 4, 2026 6:39am UTC
portable-text-example-basic Ready Ready Preview Sep 4, 2026 6:39am UTC
portable-text-playground Ready Ready Preview Sep 4, 2026 6:39am UTC

Request Review

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Bundle Stats

✅ No significant changes.

All scenario measurements (7)

🗺️ @portabletext/editor / @portabletext/editor · @portabletext/editor / @portabletext/editor/behaviors · @portabletext/editor / @portabletext/editor/plugins · @portabletext/editor / @portabletext/editor/selectors · @portabletext/editor / @portabletext/editor/traversal · @portabletext/editor / @portabletext/editor/utils · @portabletext/markdown / @portabletext/markdown · Artifacts

Scenario Kind Bundle (raw / gzip) Gzip change Import time Import change
⚪ @portabletext/editor / @portabletext/editor export 1.09 MB / 253.7 KB -1 B, -0.0% 70 ms +3 ms, +4.7%
⚪ @portabletext/editor / @portabletext/editor/behaviors export 4.0 KB / 1.4 KB None 2 ms +0 ms, +4.3%
⚪ @portabletext/editor / @portabletext/editor/plugins export 5.1 KB / 1.8 KB None 7 ms +0 ms, +6.0%
⚪ @portabletext/editor / @portabletext/editor/selectors export 93.7 KB / 21.3 KB None 8 ms +0 ms, +2.0%
⚪ @portabletext/editor / @portabletext/editor/traversal export 41.7 KB / 10.8 KB None 6 ms +0 ms, +3.8%
⚪ @portabletext/editor / @portabletext/editor/utils export 33.1 KB / 8.7 KB None 6 ms +0 ms, +2.9%
⚪ @portabletext/markdown / @portabletext/markdown export 272.2 KB / 79.6 KB None 38 ms +2 ms, +4.2%

Significant means at least 1.0 KB and 1% gzip, or at least 5 ms and 10% import time.

@christianhg
christianhg marked this pull request as ready for review September 4, 2026 06:37
`getEventPositionBlock` compared `event.pageY` (page coordinates,
scroll offset included) against `getBoundingClientRect` values
(viewport coordinates) in all three checks. On a window-scrolled page
the inflated `pageY` made the below-last-block check fire for any
pointer position and pushed the half detection past `height / 2`, so
every drop position resolved to 'end': the indicator stuck to bottom
edges and drops landed after the hovered block. An inner scroll
container keeps `pageY` and `clientY` equal, which is why Studio
never showed it.

Compare `clientY` throughout. The first/last block boundary checks
also move behind a `nodePath.length === 0` gate: they can only decide
anything when the pointer is over the editor's own surface (its
padding), because over a block the pointer sits inside that block's
band, between the first block's top and the last block's bottom. That
removes two rect reads per drag event on the hot path.

Pinned by a test dispatching a `dragover` through the real pipeline
with the trusted-event coordinate relationship simulated on the
instance (`pageY = clientY + 2000`; synthetic events always report
`pageY === clientY`, so a real window scroll cannot exercise the old
code from a test): red on `pageY`, hovering a block's upper half must
resolve 'start'.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant