fix: escape a container at the document edge like block objects#2910
Merged
Conversation
🦋 Changeset detectedLatest commit: 99db65f 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📦 Bundle Stats —
|
| Metric | Value | vs main (e94e676) |
|---|---|---|
| Internal (raw) | 802.6 KB | +1.9 KB, +0.2% |
| Internal (gzip) | 153.5 KB | +326 B, +0.2% |
| Bundled (raw) | 1.41 MB | +1.9 KB, +0.1% |
| Bundled (gzip) | 317.7 KB | +323 B, +0.1% |
| Import time | 96ms | +1ms, +1.5% |
@portabletext/editor/behaviors
| Metric | Value | vs main (e94e676) |
|---|---|---|
| Internal (raw) | 467 B | - |
| Internal (gzip) | 207 B | - |
| Bundled (raw) | 424 B | - |
| Bundled (gzip) | 171 B | - |
| Import time | 2ms | +0ms, +0.5% |
@portabletext/editor/plugins
| Metric | Value | vs main (e94e676) |
|---|---|---|
| Internal (raw) | 2.7 KB | - |
| Internal (gzip) | 894 B | - |
| Bundled (raw) | 2.5 KB | - |
| Bundled (gzip) | 827 B | - |
| Import time | 7ms | +0ms, +1.1% |
@portabletext/editor/selectors
| Metric | Value | vs main (e94e676) |
|---|---|---|
| Internal (raw) | 81.2 KB | -22 B, -0.0% |
| Internal (gzip) | 14.9 KB | +45 B, +0.3% |
| Bundled (raw) | 76.7 KB | - |
| Bundled (gzip) | 13.8 KB | +4 B, +0.0% |
| Import time | 8ms | +0ms, +0.1% |
@portabletext/editor/traversal
| Metric | Value | vs main (e94e676) |
|---|---|---|
| Internal (raw) | 28.1 KB | +30 B, +0.1% |
| Internal (gzip) | 5.6 KB | -8 B, -0.1% |
| Bundled (raw) | 27.9 KB | - |
| Bundled (gzip) | 5.5 KB | - |
| Import time | 6ms | -0ms, -2.0% |
@portabletext/editor/utils
| Metric | Value | vs main (e94e676) |
|---|---|---|
| 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.0% |
🗺️ . · ./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 (e94e6766)
| Metric | Value | vs main (e94e676) |
|---|---|---|
| Internal (raw) | 53.8 KB | - |
| Internal (gzip) | 9.8 KB | - |
| Bundled (raw) | 348.9 KB | - |
| Bundled (gzip) | 96.3 KB | - |
| Import time | 36ms | +0ms, +0.9% |
🗺️ 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.
dd69683 to
b07543a
Compare
b07543a to
c970a0a
Compare
c970a0a to
65a1e9b
Compare
65a1e9b to
18febf7
Compare
18febf7 to
a41d9a1
Compare
a41d9a1 to
fbf51f9
Compare
fbf51f9 to
4869d43
Compare
4869d43 to
3abf8dc
Compare
3abf8dc to
3dd5335
Compare
3dd5335 to
64ea121
Compare
When a container is the document's first or last block, the caret could get trapped inside it. Arrow navigation at the container's document-edge block was deliberately suppressed (the dead-end guard predates the escape machinery in the same file, and suppression beat letting the caret escape into unmappable DOM), and the lonely-block click behaviors never matched containers because their guard resolved the click position through `getFocusBlockObject`, which containers never satisfy, the position resolves to a leaf inside them. Block objects already had both escapes, so this is a parity fix at the shared level. The arrow dead-end behaviors now escape on a bare arrow, inserting a placeholder block beyond the deepest escapable container (reusing `getEscapeTarget` and `createPlaceholderBlock` from the double-Enter escape); modified arrows and dead ends without an escape target keep the protective suppression. The click behaviors resolve the innermost block object first, preserving the lonely-image-inside- a-cell contract, and fall back to the root-level block when it is a container, gated on `isEditor`: clicks inside a container's content resolve `isContainer` and must not insert, only clicks on the editable's own surface are beyond the container. `tests/container-edge-escape.test.tsx` pins all four escapes plus the sibling fall-through (native navigation owns that case).
…ies beyond The engine's container escape only fires at the document-edge block, which for a table means the corner cells. Table semantics are wider: ArrowDown from any bottom-row cell (and ArrowUp from any top-row cell) should exit the table. The nav behaviors now escape when there is no row to move to and the table has no sibling on that side, inserting an empty text block beyond the table and moving the caret into it. When a sibling exists the behaviors keep falling through, native navigation owns that case and preserves the caret's goal column. The tests also stop focusing editors with real pointer clicks: a click on the editable's surface near a lonely table now legitimately inserts an escape block, which made click-for-focus geometry-sensitive. They focus via the `focus` event instead, and the two arrow pins at the table's edges flip from "does not move" to the escape contract.
64ea121 to
99db65f
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
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 against the playground table: with a table and no other text in the document, input focus cannot leave the table, neither arrow navigation nor clicking moves the caret outside. Both halves turned out to be container/block-object parity gaps: block objects already escape both ways, and containers were either deliberately trapped or unrecognized.
The keyboard half was a suppression predating its own solution. The container dead-end behaviors (
arrowDownOutOfContainer/arrowUpOutOfContainer) detect exactly this situation and consumed the arrow with empty actions, built to keep the caret from escaping into DOM that doesn't map back to the model. The escape machinery arrived later in the same file for the double-Enter case (getEscapeTarget,createPlaceholderBlock). The dead-end behaviors now use it: a bare arrow inserts a placeholder block beyond the deepest escapable container and moves the caret there, while modified arrows (selection extension) and dead ends without an escape target keep the protective suppression.The pointer half was two gaps in one. First,
clickingBelow/AboveLonelyBlockObjectalready checkevent.position.isContainer, but their guard resolves the click throughgetFocusBlockObject, which containers never satisfy. Second, and deeper: for a click on a container's surface,EventPosition.selectionresolved to whatever leafcaretPositionFromPointlanded on inside the container, so the position said that a container was clicked without its selection saying which one, making the nested case (a lonely container inside another container) unresolvable in principle. The fix corrects the selection itself: the drag pipeline already pointed the selection at the container for exactly this reason, scoped to drags 'because clicks don't need the override', and clicks need it now. The position's selection uniformly names what was hit, a leaf for text, a void for voids, the container itself for its own surface, and the guards reduce to one rule: insert beside the non-text edge child of the clicked surface (the root block for editor-surface clicks, the clicked container's first/last child otherwise). That single rule covers the root-level container, the pinned lonely-image-inside-a-cell contract (its tests updated to the position shape the engine now emits), and the nested container, each with its own test, while clicks on a block's content never insert, a distinction the red tests flushed out when the plugin suite's own focus-clicks started inserting escape blocks. Those tests now focus via thefocusevent instead of synthetic pointer geometry.The second commit adds the table-domain half: the engine's escape fires only at the document-edge block, which for a table means the corner cells, but ArrowDown from any bottom-row cell should exit. The plugin's nav behaviors now own the whole edge: when a sibling block lies beyond the table the caret moves into it, entering at its current horizontal position (the same hit-testing idiom the cell-to-cell moves use), and when nothing lies beyond, an empty text block is inserted and receives the caret. The original design let native navigation own the sibling case, but a field report showed chromium's native ArrowDown at a table's bottom row walks forward through the cells instead of exiting (and ArrowUp at the top row walks backwards), so falling through navigated to the next cell rather than the block below; both directions are pinned with the caret's landing block asserted.
A field report against the rebased branch caught the guard the escape inherited being wrong all along:
isAtContainerDeadEndresolved the container as the nearest editable-container ancestor and used the container-scopedgetFirstBlock/getLastBlock, so inside a table it declared every row's first and last cell a dead end, not just the table's corners. While the dead-end behaviors only suppressed, the over-match swallowed an occasional arrow invisibly; once they insert, it manufactured phantom blocks beside the table, intermittently, because the model selection lags chromium's native sideways caret walk along a table's edge row, so the misfire depended on sync timing. The guard now measures the dead end where the DOM caret actually escapes, the outermost container's edge (its edge leaf block viagetLeaf, its sibling at root level). Pinned twice: a synthetic core test drives arrows at a nested row's first cell mid-container (red under the old guard, deterministically, no browser timing involved), and a plugin round-trip test arrows through a 3×3 table with a block above for three full passes, asserting the value never grows.Two more field reports against the rebased branch hardened the same seams. Clicking the table's own chrome band (the surface right above or below the grid, DOM-inside the table block) inserted the placeholder inside
rows, a text block among rows, because the container-surface rule inserts beside the clicked container's edge child without asking whether that position accepts a text block. The rule now checks the clicked container'sofand inserts nothing when text blocks aren't valid there; the editor-surface click beyond a lonely table keeps escaping, and both are pinned in one test that drives the contrast against the grid fixture. And arrows on a block object inside a cell (an image at a cell's top) were stolen by the plugin's cell navigation before the engine's lonely-block-object escape could insert a text line beside it inside the cell; the plugin's arrow guards now yield whenever a block object is focused, pinned in both directions.Tab/Shift+Tabget the same treatment for list items: cell navigation yields to the engine's list indent/unindent when the caret sits in a list item, pinned with a level round-trip that asserts the caret never leaves the cell.tests/container-edge-escape.test.tsxpins all four engine escapes plus the sibling fall-through; the plugin's two edge-arrow pins flip from "does not move" to the escape contract. Rebased onto thedefineTablerefactor: the nav behaviors' escape logic moved into the config-bound factory, and the newerdefine-table.test.tsxsuite adopted the same focus-via-event idiom (its surface clicks near a lonely table hit exactly the geometry sensitivity this PR removes). The plugin gains aminorchangeset now that it's published. Editor container/block-object suites and the full plugin suite (121 chromium) pass locally.