fix: use targetRange from beforeinput to place text replacements#2595
Draft
christianhg wants to merge 2 commits into
Draft
fix: use targetRange from beforeinput to place text replacements#2595christianhg wants to merge 2 commits into
christianhg wants to merge 2 commits into
Conversation
🦋 Changeset detectedLatest commit: 1e34d62 The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 packages
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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📦 Bundle Stats —
|
| Metric | Value | vs editor-v6.x (ec6940c) |
|---|---|---|
| Internal (raw) | 747.2 KB | -222 B, -0.0% |
| Internal (gzip) | 140.7 KB | -46 B, -0.0% |
| Bundled (raw) | 1.34 MB | -223 B, -0.0% |
| Bundled (gzip) | 301.1 KB | -54 B, -0.0% |
| Import time | 94ms | -1ms, -0.9% |
@portabletext/editor/behaviors
| Metric | Value | vs editor-v6.x (ec6940c) |
|---|---|---|
| Internal (raw) | 467 B | - |
| Internal (gzip) | 207 B | - |
| Bundled (raw) | 424 B | - |
| Bundled (gzip) | 171 B | - |
| Import time | 2ms | -0ms, -1.7% |
@portabletext/editor/plugins
| Metric | Value | vs editor-v6.x (ec6940c) |
|---|---|---|
| Internal (raw) | 2.5 KB | - |
| Internal (gzip) | 910 B | - |
| Bundled (raw) | 2.3 KB | - |
| Bundled (gzip) | 839 B | - |
| Import time | 8ms | -0ms, -1.5% |
@portabletext/editor/selectors
| Metric | Value | vs editor-v6.x (ec6940c) |
|---|---|---|
| Internal (raw) | 60.5 KB | - |
| Internal (gzip) | 9.5 KB | - |
| Bundled (raw) | 56.9 KB | - |
| Bundled (gzip) | 8.7 KB | - |
| Import time | 6ms | -0ms, -0.9% |
@portabletext/editor/utils
| Metric | Value | vs editor-v6.x (ec6940c) |
|---|---|---|
| Internal (raw) | 24.2 KB | - |
| Internal (gzip) | 4.7 KB | - |
| Bundled (raw) | 22.2 KB | - |
| Bundled (gzip) | 4.4 KB | - |
| Import time | 6ms | -0ms, -0.9% |
🗺️ . · ./behaviors · ./plugins · ./selectors · ./utils · Artifacts
Details
- Import time regressions over 10% are flagged with
⚠️ - Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.
christianhg
force-pushed
the
fix/grammarly-replacement-target-range
branch
from
19f9979 to
3ab969a
Compare
christianhg
force-pushed
the
fix/grammarly-replacement-target-range
branch
from
3ab969a to
6f5c797
Compare
christianhg
force-pushed
the
fix/grammarly-replacement-target-range
branch
from
6f5c797 to
fd94ec1
Compare
christianhg
force-pushed
the
fix/grammarly-replacement-target-range
branch
from
fd94ec1 to
f235410
Compare
christianhg
force-pushed
the
fix/grammarly-replacement-target-range
branch
from
f235410 to
23d6c5f
Compare
When an extension or IME uses `insertReplacementText` (Grammarly, browser autocorrect, macOS substitution panel, etc.), the replacement now lands at the range the browser indicates via `getTargetRanges()` instead of at the editor selection, and the caret lands right after the inserted text instead of jumping back to where the user was typing. The previous behavior bailed out when the node-map dirty flag was set from a recent edit, falling back to `editor.selection`. Reported as: typing a character and then accepting a suggestion placed the replacement at the post-typing caret instead of at the underlined word. For cross-block replacements, the caret also stayed in the original block instead of following the replacement. The dirty-flag guard came from upstream Slate to prevent crashes when `NODE_TO_INDEX` and `NODE_TO_PARENT` WeakMaps were stale. PTE's `toSlateRange` walks `data-slate-*` DOM attributes directly via `getDomNodePath` and does not depend on those WeakMaps, so the guard prevents a real fix without guarding against any failure mode in PTE. The selection issue was caused by the targetRange path also stashing the pre-targetRange selection in `userSelection` for later restoration. That restore put the caret back where the user was typing instead of leaving it at the targetRange where the replacement landed. The targetRange is the user's intent for the new selection.
christianhg
force-pushed
the
fix/grammarly-replacement-target-range
branch
from
23d6c5f to
1e34d62
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When an extension or IME (Grammarly, browser autocorrect, macOS substitution panel, etc.) fires
beforeinputwithinputType: 'insertReplacementText', the browser provides a target range viaevent.getTargetRanges()indicating where the replacement should land. The editor was bailing out of that path when its node-map dirty flag was set, falling back toeditor.selection— which is whatever the last keystroke left it at, not the replacement's target. After applying the replacement, the caret also jumped back to the user's pre-replacement position instead of following the inserted text.Reported as: typing a character and then accepting a suggestion places the replacement at the post-typing caret position instead of at the underlined word. For cross-block replacements (caret in one list item, suggestion accepted in another), the replacement landed in the wrong block AND the caret stayed in the original block.
The dirty-flag guard was inherited from upstream Slate, where it prevents crashes when
NODE_TO_INDEXandNODE_TO_PARENTWeakMaps are stale relative to the DOM. PTE doesn't use those WeakMaps;toSlateRangewalksdata-slate-*DOM attributes directly viagetDomNodePath, so the original failure mode the guard was protecting against doesn't exist here. Removing the guard lets the target range take effect; switchingtoSlateRangetosuppressThrow: trueand gating theeditor.select(range)call on a non-null result keeps things safe if a path can't be resolved for any other reason.The caret-jumps-back issue was a separate latent bug in the same path: the targetRange handler also stashed the pre-targetRange selection in
userSelection, which the bottom-of-handler restore would then put back. Dropping that stash leaves the caret at the targetRange (where the replacement landed) — which is what users expect.Includes regression tests for two scenarios: