Skip to content

platform: fix annotation-agnostic selection paths for base USJ#463

Open
jolierabideau wants to merge 5 commits intomainfrom
platform-pt-3835-multiple-annotations-paragraph
Open

platform: fix annotation-agnostic selection paths for base USJ#463
jolierabideau wants to merge 5 commits intomainfrom
platform-pt-3835-multiple-annotations-paragraph

Conversation

@jolierabideau
Copy link
Copy Markdown
Contributor

@jolierabideau jolierabideau commented Mar 27, 2026

Fix annotation-agnostic USJ selection paths (onSelectionChange / setAnnotation)

Problem

Editorial’s onSelectionChange (and EditorRef.getSelection()) build SelectionRange from Lexical via $getUsjSelectionFromEditor$getLocationFromNode. For plain text, paths used **$getJsonPathIndexes**, which treats each **TypedMarkNode** (annotation wrapper) as its own content sibling—matching the Lexical tree, not canonical USJ.

So after one annotation (e.g. "neva"), selecting following text (e.g. "sleep") could report paths like $.content[…].content[2] with a small offset. Host code and JSONPath helpers resolve locations against actual USJ (no extra per-annotation content entries), so queries like $.content[20].content[2] fail with “No result found for JSONPath query”. Different annotation providers cannot reason about each other’s marks if coordinates are tied to the annotated Lexical shape.

Expected: jsonPath + offset identify the same scripture position as in base USJ, independent of existing annotations.

Solution

For text nodes and text inside TypedMarkNode, $getLocationFromNode now emits:

  • **jsonPath** from **$getJsonPathIndexesForBaseUsj**: collapses adjacent TextNode + TypedMarkNode into one “content run” and shares one base-USJ content index (indices advance only on non-run nodes such as markers / verses).
  • **offset** from **$getOffsetInContentRun**: character offset within that run (sum of preceding plain text and marked text in the paragraph).

Reverse resolution ($getRangeFromUsjSelection / $getNodeFromLocation) already navigates base-USJ paths and unwraps TypedMarkNode when needed, so forward and backward paths stay aligned.

Changes

Area Change
libs/shared-react/.../selection.utils.ts Base-USJ path + run offset for text; helpers $getJsonPathIndexesForBaseUsj, $getContentIndexWithinParentForBaseUsj, $getOffsetInContentRun.
selection.utils.test.ts Updated expectations for selections involving marks; new test for “sleep” after a mark in the same paragraph (content[0], offsets 12–17).
demos/platform/.../app.tsx Store last SelectionRange in a ref; Insert at selection uses setAnnotation with current selection; replace deprecated MarginalProps<Console> typing with SelectionRange + local OnUsjChangeWithComments.

How to verify

  • Unit tests: nx test shared-react (or targeted selection.utils.test.ts).
  • Manual: annotate a span, select text after it, insert a second annotation or comment; selection should not reference non-existent content[n] on USJ JSONPath resolution.

Scope / follow-ups

  • Focus is selection reporting and consistency with base USJ; other call sites that assumed Lexical-shaped paths may need review if any remain.

Made-with: Cursor


Open with Devin

This change is Reviewable

- Emit jsonPath/offset from base-USJ content runs in selection.utils
- Update selection tests; add multi-annotation paragraph case
- Platform demo: selection ref, Insert at selection, fix callback types

Made-with: Cursor
@codesandbox
Copy link
Copy Markdown

codesandbox bot commented Mar 27, 2026

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

@jolierabideau jolierabideau changed the title fix(usj): annotation-agnostic selection paths for base USJ platform: fix annotation-agnostic selection paths for base USJ Mar 27, 2026
Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 4 additional findings.

Open in Devin Review

…ion-agnostic paths

Update data-driven selection tests to expect base-USJ locations from
$getUsjSelectionFromEditor when paranext document paths differ, including
a marker-mode special case for one content[6] text location.

Made-with: Cursor
devin-ai-integration[bot]

This comment was marked as resolved.

Extend $textRequiresStandardLexicalContentPath to treat VerseNode and
MilestoneNode like CharNode so collapsed base-USJ paths are not emitted
for text that follows those nodes. That keeps jsonPath aligned with
$getContentChildAtIndex and round-trips through $getNodeFromLocation.

Update unit tests for editable para with verse and editable milestone cases.

Made-with: Cursor
devin-ai-integration[bot]

This comment was marked as resolved.

…arkNode

 now adds text lengths of preceding siblings within
a TypedMarkNode before applying the local TextNode offset, fixing USJ
round-trip when formatting splits produce multiple TextNodes under one mark.

Also: resolve shared-react lint (data-driven selection tests use describe.each
and it.each; index-signature style; filter console warn eslint); add round-trip
test for cursor in second TextNode inside a mark.

Made-with: Cursor
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