fix: sync the selection in read-only editors#2918
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: cd1ba48 The changes in this PR will be included in the next version bump. This PR includes changesets to release 14 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 |
Contributor
📦 Bundle Stats —
|
| Metric | Value | vs main (cc7c1f0) |
|---|---|---|
| Internal (raw) | 801.1 KB | -3 B, -0.0% |
| Internal (gzip) | 153.2 KB | +3 B, +0.0% |
| Bundled (raw) | 1.41 MB | -3 B, -0.0% |
| Bundled (gzip) | 317.4 KB | +2 B, +0.0% |
| Import time | 96ms | +2ms, +2.1% |
@portabletext/editor/behaviors
| Metric | Value | vs main (cc7c1f0) |
|---|---|---|
| Internal (raw) | 467 B | - |
| Internal (gzip) | 207 B | - |
| Bundled (raw) | 424 B | - |
| Bundled (gzip) | 171 B | - |
| Import time | 2ms | -0ms, -0.4% |
@portabletext/editor/plugins
| Metric | Value | vs main (cc7c1f0) |
|---|---|---|
| Internal (raw) | 2.7 KB | - |
| Internal (gzip) | 894 B | - |
| Bundled (raw) | 2.5 KB | - |
| Bundled (gzip) | 827 B | - |
| Import time | 7ms | -0ms, -1.6% |
@portabletext/editor/selectors
| Metric | Value | vs main (cc7c1f0) |
|---|---|---|
| Internal (raw) | 81.2 KB | - |
| Internal (gzip) | 14.9 KB | - |
| Bundled (raw) | 76.7 KB | - |
| Bundled (gzip) | 13.8 KB | - |
| Import time | 8ms | -0ms, -0.2% |
@portabletext/editor/traversal
| Metric | Value | vs main (cc7c1f0) |
|---|---|---|
| Internal (raw) | 28.0 KB | - |
| Internal (gzip) | 5.6 KB | - |
| Bundled (raw) | 27.9 KB | - |
| Bundled (gzip) | 5.5 KB | - |
| Import time | 6ms | +0ms, +0.7% |
@portabletext/editor/utils
| Metric | Value | vs main (cc7c1f0) |
|---|---|---|
| Internal (raw) | 29.7 KB | - |
| Internal (gzip) | 6.2 KB | - |
| Bundled (raw) | 27.1 KB | - |
| Bundled (gzip) | 5.8 KB | - |
| Import time | 6ms | -0ms, -0.6% |
🗺️ . · ./behaviors · ./plugins · ./selectors · ./traversal · ./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.
📦 Bundle Stats — @portabletext/markdown
Compared against main (cc7c1f0f)
| Metric | Value | vs main (cc7c1f0) |
|---|---|---|
| Internal (raw) | 53.8 KB | - |
| Internal (gzip) | 9.8 KB | - |
| Bundled (raw) | 348.9 KB | - |
| Bundled (gzip) | 96.3 KB | - |
| Import time | 37ms | -1ms, -3.8% |
🗺️ View treemap · 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.
The `selectionchange` handler bailed unless the editable was the document's active element, and a read-only editable (`contenteditable="false"`, no tab stop) can never be the active element, so DOM-to-model selection sync was structurally dead in read-only editors. The failure surfaced two ways: a selection made before switching to read-only froze in the model (stale selection-derived UI stuck on screen), and selections made while read-only never reached the model, so `clipboard.copy` serialized an outdated selection onto a prevented default and produced an empty clipboard. This regressed in 2a3e039, which added the early return so a toolbar click's blur could not clobber the editor's selection with the browser's collapsed one. That purpose is editable-mode-specific, and everything around the gate expects read-only selection to work: the editor machine whitelists `select`, `clipboard.copy`, and `serialize` in read-only mode, the handler carries a read-only-specific deselect guard, and `hasDOMNode`'s editable check explicitly accommodates targets inside a `contenteditable="false"` editor root. The gate now yields in read-only editors only. Editable editors keep the early return exactly as 2a3e039 intended; the selectable and in-editor checks below still confine the sync to selections inside the editor, and the deselect guard still clears the model when the selection leaves. Red-verified: both pinned scenarios fail without the gate change.
christianhg
force-pushed
the
fix-readonly-selection-sync
branch
from
99fdf25 to
cd1ba48
Compare
Merged
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.
A field report from the table work: switch an editor to read-only while a multi-cell (selection-derived) highlight is active and the highlight is stuck forever; make a new selection in the read-only editor and no selection-derived UI appears, and copying puts nothing on the clipboard.
Both symptoms share one root. The
selectionchangesync bails unless the editable is the document's active element, and a read-only editable (contenteditable="false", no tab stop) can never be the active element, so DOM-to-model selection sync is structurally dead in read-only editors. The model selection freezes at whatever it held when read-only began: stale selection-derived rendering paints forever, new selections never reachgetTableSelection-style consumers, andclipboard.copyserializes the outdated selection onto a prevented default, hence the empty clipboard.The intent of the surrounding code is clearly the opposite, three separate pieces already accommodate read-only selection: the editor machine whitelists
select,clipboard.copy, andserializein read-only mode, theselectionchangehandler carries a read-only-specific deselect guard just below the gate, andhasDOMNode's editable check has an explicit clause accepting targets whose closestcontenteditable="false"ancestor is the editor root, commented as existing precisely for the read-only case. The focus gate above them was the one piece that never yielded.This is a regression from 2a3e039 (April 2026), which added a one-line early return so a toolbar click's blur could not clobber the editor's selection with the browser's collapsed one. That purpose is editable-mode-specific; read-only editors, which are never the active element, lost selection sync as collateral.
The fix makes the gate yield in read-only editors only, preserving 2a3e039's behavior for editable editors exactly. The selectable/in-editor checks below still confine the sync to selections inside the editor, and the existing deselect guard still clears the model when the selection leaves it, so the blast radius is the intended one: read-only editors gain working selection tracking (and therefore selection-derived UI and copy), while editable editors keep the gate exactly as before, all 1701 existing browser tests pass unmodified.
selection-readonly-sync.test.tsxpins both reported shapes: a selection made while read-only reaches the model with full literal points, and a selection made before switching to read-only keeps following the DOM instead of freezing. Both scenarios are red without the gate change.