Skip to content

fix(selection): table cell and multiline selection rendering (SD-3328)#3688

Open
tupizz wants to merge 4 commits into
mainfrom
tadeu/sd-3328-bug-table-selection-highlight-disappears-over-empty-space
Open

fix(selection): table cell and multiline selection rendering (SD-3328)#3688
tupizz wants to merge 4 commits into
mainfrom
tadeu/sd-3328-bug-table-selection-highlight-disappears-over-empty-space

Conversation

@tupizz

@tupizz tupizz commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

What

Fixes SD-3328. The selection highlight disappeared over empty space inside table cells. While tracking that down I found two more bugs on the same selection rendering path, so this PR groups the three related fixes.

Demo

CleanShot.2026-06-09.at.12.43.07.mp4

Background

Presentation mode draws its own selection overlay. It does not use the browser selection. The overlay rects come from computeSelectionRectsFromDom, which maps the ProseMirror selection to the painted DOM and reads getClientRects().

flowchart LR
  PM["PM selection (from, to)"] --> C[computeSelectionRectsFromDom]
  C --> R["DOM Range + getClientRects()"]
  R --> O[overlay rects]
Loading

1. Dragging into an empty paragraph in a table collapsed the selection (the reported bug)

prosemirror-tables normalizes a text selection whose endpoints resolve to different cells when the head sits at the start of its block (parentOffset === 0). An empty paragraph in a cell only ever has offset 0, so dragging a body selection onto one rewrote [44, 2026] to [44, 49] on dispatch. Text positions in a cell escape because their offset is greater than 0, which is why dragging through cell text worked but an empty paragraph killed it.

sequenceDiagram
  participant Drag
  participant PM as TextSelection.create
  participant Tables as prosemirror-tables
  Drag->>PM: create [44, 2026]
  PM-->>Drag: [44, 2026]
  Drag->>Tables: dispatch
  Tables-->>Drag: normalizes to [44, 49] (collapse)
Loading

Fix: a detector mirrors that exact condition. When a drag frame would collapse, the handler keeps the last good selection and resumes extending as soon as the head moves into cell text. The detector fails open on any unexpected document shape so it can never block a normal selection.

2. Interior lines of a multiline selection rendered blank

A multiline selection built one DOM Range across every line and trusted getClientRects(). Each .superdoc-line is absolutely positioned, and some Chrome builds return incomplete rects for a Range crossing those boxes: the first and last lines render, the interior lines collapse to a few slivers. intersectsNode does not catch it because the entries still report as intersected.

Fix: when a selection spans more than one line, compute rects per line. Interior lines use the line element box, which is reliable across browsers and gives the full width highlight normal selection shows. The first and last lines keep the precise Range so they respect the selection offset and first line indent.

3. Empty cell paragraphs resolved to no hit position

The geometry hit test returned null for an empty cell paragraph (no runs), which aborted the active drag and froze the selection. Now it falls back to the paragraph PM start so every empty cell resolves to a valid position.

Testing

  • New regression tests for each fix. The table detector test pins the behavior against the real prosemirror-tables plugin (it must flag exactly the selections that actually collapse).
  • All presentation editor tests pass (1139).
  • Verified each fix with a real drag against the dev server.

tupizz added 3 commits June 9, 2026 09:40
…sition (SD-3328)

An empty paragraph (blank line / spacer between bullets) in a table cell has no
runs and no measured lines, so the table hit-test fell through to a null result.
A null hit aborts an in-progress drag (EditorInputManager #handleDragSelectionAt
early-return), freezing/collapsing the selection while the pointer is over the
blank line. Derive the paragraph's PM start from its attrs so an empty cell
paragraph always resolves to a valid position, never null.

Also emit a content-width highlight band for blank lines on the selectionToRects
geometry path (fallback parity). Note: the live selection overlay uses the DOM
Range path (DomSelectionGeometry.getClientRects), so the blank-line band and the
interior-line render race on that path are addressed separately.
A multi-line selection built one DOM Range spanning every line and trusted
range.getClientRects(). Each .superdoc-line is absolutely positioned, and some
Chrome builds return incomplete rects for a range crossing those positioned
boxes: interior lines collapse to a few stray slivers while the first and last
lines render fully, leaving the middle of the selection visually unhighlighted.
intersectsNode does not flag it because the entries still report as intersected.

Compute rects per line when a selection spans more than one line. Interior
lines use the line element's own bounding box (reliable across browsers and the
full-width highlight normal selection shows); the first and last lines keep the
precise range so they respect the selection offset and first-line indent.
Single-line selections are unchanged.
Dragging a body text selection into (or through) a table collapsed the whole
selection. prosemirror-tables' normalizeSelection rewrites a TextSelection to the
anchor block's own bounds when its endpoints resolve to different cells and the
head sits at the start of its block (parentOffset 0). An empty paragraph inside a
cell is always at parentOffset 0, so dragging the head onto one rewrote e.g.
[44, 2026] to [44, 49] on dispatch, collapsing the selection to the first run.

Add selectionCollapsesAcrossTableCells to detect that exact frame (mirroring the
upstream condition) and skip dispatching the doomed selection in the drag handler,
preserving the last good selection. The selection resumes extending as soon as the
head moves into cell text. The detector fails open on any unexpected document
shape so it can never block a normal selection.
@linear-code

linear-code Bot commented Jun 9, 2026

Copy link
Copy Markdown

SD-3328

@tupizz tupizz marked this pull request as ready for review June 9, 2026 14:52
@tupizz tupizz requested a review from a team as a code owner June 9, 2026 14:52
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 88450fadc3

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@tupizz tupizz self-assigned this Jun 9, 2026
Dragging a selection from one table cell into another (notably backward, or
starting on an empty cell paragraph) broke: prosemirror-tables will not let a
text selection span cells, so it collapsed the selection, and the empty-para
guard then froze it. Cross-cell drags should be a cell-range selection, the way
Word and Google Docs behave.

Detect cross-cell drags from the resolved PM positions (reliable) rather than the
geometry hit-test (which misses empty cell paragraphs and leaves the cell anchor
unset). When the anchor and head land in different cells of the same table,
dispatch a CellSelection; within a single cell stays a text selection, and a
body-into-table drag keeps the empty-paragraph guard.

Also fix the cell-selection overlay so it renders without a geometry cell anchor:
it now locates the table block by document order instead of guessing an id like
`${tableStart}-table`, which never matched the real sequential ids.

Adds unit tests for the cross-cell resolvers (both directions, the empty-cell
case, and the same-cell / body-to-cell / different-table null cases).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants