Skip to content

Fix the editor context menu and \id-line editing - #14

Open
tjcouch-sil wants to merge 24 commits into
mainfrom
standard-view-pt-4195
Open

tjcouch-sil wants to merge 24 commits into
mainfrom
standard-view-pt-4195

Conversation

@tjcouch-sil

@tjcouch-sil tjcouch-sil commented Sep 15, 2026

Copy link
Copy Markdown
Member

What

Editor-side fixes surfaced while hand-QAing (and then reviewing) an endnote insert item in paranext-core — the host contributes the insert items, and with four of them the right-click menu is eight items long.

Three in ContextMenuPlugin's right-click menu:

  1. The list was clipped with no way to reach the rest. The menu reuses .typeahead-popover ul, whose max-height: 200px is sized for the marker typeahead's long, filterable list. The context menu is a short fixed list at ~36px per item, so it overflows that cap at seven items — the last items sit behind a fold, and the menu has no filter to narrow the list with.
  2. Its own scroll closed it. The close-on-scroll listener is registered on window in capture phase, which fires for a descendant's (non-bubbling) scroll event too — so scrolling the menu's own list to reach the clipped items closed the menu instead.
  3. The fold was invisible, and the menu was unannounceable. Even capped to the viewport, a panel shorter than ~270px still clips it, and the inherited rule pairs overflow-y: scroll with a hidden scrollbar — so nothing on screen said there was more. Separately, the items were role="option" inside a bare ul, which is not a listbox, so the role had no list to mean anything in and nothing told an assistive technology which item the arrow keys had highlighted.

Together, (1) and (2) made the items past the fold unreachable by mouse entirely.

And one unrelated data-rendering bug in the USJ forward adaptor, found by chasing an endnote that a paranext-core E2E spec inserted into the \id line: it saved to USFM correctly but the editor rendered none of it after a chapter re-read.

createBook was the one content container the forward adaptor did not recurse into. It flattened the book's content with getTextContent, which returns "" unless the content is exactly one plain string — so the moment anything else sat in the \id line, the whole content went with it. The line rendered as a bare \id JON, losing both the note and the line's own description text, and the next save wrote that truncation back to the file. The reverse adaptor already recursed the book node's children, so this was the asymmetric half of the pair and broke the round-trip invariant the corpus suite exists to hold.

Changes

  • ContextMenuPlugin: ignore scroll events originating inside the menu; still close on a scroll of anything else, which is what that listener is for (the menu is positioned in fixed viewport coordinates, so the content it was opened over moves out from under it).
  • ContextMenuPlugin: role="listbox" + an accessible name on the option list, and aria-controls/aria-activedescendant on the focused editor root while the menu is open. This completes the pattern the plugin had already started rather than moving to menu/menuitem: focus deliberately stays in the contenteditable the whole time the menu is open — that is what keeps the selection the chosen item acts on — and option-list + aria-activedescendant on the focused element is the pattern for a popup whose owner keeps focus. It is also what Lexical's own typeahead menu does. menu/menuitem would require moving focus into the menu, putting that selection at risk. The item ids move to their own namespace, because Lexical's shared typeahead-* ids are reused by every menu built on its typeahead and an aria-activedescendant IDREF resolving to another menu's item announces the wrong thing.
  • editor.css: cap .typeahead-popover.auto-embed-menu ul to the viewport instead of 200px, and give it overflow-y: auto + scrollbar-width: thin so the fold is visible when the panel is genuinely too short. The .typeahead-popover qualifier is needed to out-specify the shared rule — the plugin puts both classes on the menu's outer element and nests a second .typeahead-popover inside it.
  • usj-editor.adaptor.ts: createBook now recurses its content like every other container, so the two adaptor halves agree. getTextContent had no other caller and goes with it.
  • Adds the plugin's first tests (it had none). They drive real DOM keydown events rather than dispatching Lexical commands, which is what makes the propagation the menu actually depends on observable: its key handling sits on a capture-phase listener on document, ahead of Lexical's bubble-phase listener on the root element. One test is the control — Enter reaches Lexical once the menu has closed — so "Lexical never saw Enter" means the menu claimed it rather than that the harness never routed it. Three more cover the ARIA wiring, including that it is torn down when the menu closes.
  • Adds a corpus fixture for a note in the \id line (round-trip identity in all five view modes, plus the transform fixed point) and a forward anchor on the book node's children — identity alone is blind to a bug mirrored on both sides, which is exactly the shape this one had.

The \id line's content now edits like paragraph content

Fixing createBook made the \id line's content survive a reload; hand QA then showed that everything else about editing it still treated the line as a single run of literal header text. The line is a BookNode at document root, not a ParaNode, but Paratext 9 treats its text as ordinary content — footnotes and character styles included — so each place below now treats the book like a paragraph. The \id GEN prefix stays one immutable decorator the caret cannot enter; only the content after it changed.

This closes PT-4613 — "backslash menu in the \id line lists paragraph markers but committing one does nothing". Its DoD offered a choice between offering only insertable markers and inserting everything offered; both halves landed, because the two triggers need different answers. \ no longer lists paragraph markers in the line (it is character source there now), and the Enter palette — which offers them deliberately — now inserts instead of doing nothing. The rule behind it, a menu offers nothing it cannot insert, is written down in docs/standard-view-invariants.md along with the \id-line rule the defects below all came from; the host-side half is in paranext-core's .context/standards/Standard-View-Invariants.md.

  • Notes in the line can be navigated and expanded (shared-react). ArrowLeft could not get back past a note in the line, and a note at the line's end never expanded for inline editing, because both guards assumed a book held one run of text.
  • \ opens the inline palette there (markerMenuContext.utils.ts). With no ParaNode ancestor the trigger reported paragraph source; even forced to character source it would have listed nothing, because the character list filters on the enclosing paragraph's marker. A caret in the line now reports character source with paraMarker: "id": the character styles valid under id, plus every note style — which is what Paratext 9 lists there.
  • A paragraph pick starts its paragraph after the line (markerMenuApply.utils.ts). The Enter palette's Enter did nothing: there was no paragraph to retag or split. The pick now splits the line at the caret, closing and reopening an open character span across the cut. It lives in $splitParagraphWithMarker, because the Enter palette calls that directly and never goes through the \ palette's apply path.
  • Typed markers settle ($rebuildBook, a fourth settle scope beside paragraphs, note content and chapters). The line had no settle scope, so \nd Lord\nd* typed there stayed literal on screen while the save wrote a real span. Like the note scope, it re-tokenizes only the content and preserves the book node, its code and its prefix. The read-only settle ($settledUsj) mirrors it, the undo re-pend scan descends into the line, and live typing display-maps space runs there the way load and settle already did.
  • A typed \p , \ip or \c ends the line immediately, starting its paragraph or chapter right after the book, with the tail moving into it and the caret following — the same split a paragraph gets. (It previously stayed literal on screen and only turned into a paragraph after a reload.) Bytes that literally begin with \p tokenize to a wrapper indistinguishable from the tokenizer's implied \p, so they are told apart by the bytes and leave the line empty.
  • Backspacing away the marker of the paragraph below the line merges it into the line (markerEditDeletion.utils.ts). The merge only recognized a ParaNode above, so below the \id line it reset the marker to \p with a fresh prefix — and every further Backspace deleted the re-created marker and grew it back again.

Also in this PR: overscroll-behavior: contain on the context menu's list, so a wheel gesture that reaches the end of the list no longer chains to the page and closes the menu.

Since the last update

Code-review fixes (2026-09-18), each with a test that fails without it:

  • The \id line split where the tokenizer does not (tokenizedBookLine). Telling a typed leading \p from the tokenizer's implied one re-read the bytes, and the re-read disagreed with the tokenizer at both edges. It skipped any leading whitespace, but the tokenizer drops a leading run only when it holds a line break, so \p more emptied the line and added a whitespace-only \p paragraph, in the live settle and in the saved USJ alike. It also ended the marker name only at whitespace, where the tokenizer also ends it at \, | and ZWSP, so a \p typed straight before \bd … silently vanished. The pattern now mirrors the tokenizer on both edges, pinned by a table of the shapes where the two could disagree (six rows fail against the old pattern).
  • A paragraph pick in the \id line no longer deletes the selection and then gives up ($splitBookWithMarker). The split can lift its break point only out of char spans, so inside an annotation's mark wrapper it bailed — after it had already removed the selected text. Whether the split can reach the book is now decided before anything is mutated.
  • The context menu leaves keys to whatever has focus once focus leaves the editor (ContextMenuPlugin). Nothing closes the menu on Tab, and its listener hears the whole document, so a focused button's Enter was swallowed (or, with an item highlighted, ran that item). The menu now drives Arrow/Enter only while the editor, or nothing, holds focus; Escape still closes it from anywhere. #2653 depends only on the editor-focused case, which is unchanged.
  • A stale comment in the undo re-pend scan still said the scan never descends into books.

Rebased onto main, past #12 (the font fallback chain). Every source commit is patch-identical to before the rebase. #12's own dist rebuild conflicted with this branch's, so one commit on top regenerates packages/platform/dist from the rebased source.

The context menu owns Enter for as long as it is open and the editor has focus (ContextMenuPlugin). It used to claim Enter only while an item was highlighted, and a right-click highlights nothing. Enter pressed straight after one therefore reached Lexical, which still has DOM focus behind the menu, and split the paragraph; in paranext-core it instead opened a paragraph palette underneath the still-open menu. Enter is now claimed whenever the menu is up. It invokes the highlighted item if that item is enabled, and otherwise is swallowed with the menu left open, so a disabled highlighted option is a deliberate no-op rather than a key that falls through. Two new tests, Enter with nothing highlighted and Enter on a disabled option, both assert that Lexical never sees the press and the menu stays open.

ArrowLeft walks back into a character span in the \id line (ArrowNavigationPlugin). At the start of the text after a span (\id GEN \nd LORD\nd* Berean…), the book-line guard claimed the press and called preventDefault, so ArrowLeft was a dead key there. The guard used "parent is the book node" to mean "nothing to the left", which stopped being true once the line could carry spans. It now checks whether anything other than the line's immutable \id GEN prefix is to the left. The new test asserts defaultPrevented rather than caret position, because jsdom performs no native move and a position check would pass either way. Confirmed red against the old guard.

A racy ScriptureReferencePlugin test now settles before asserting (ScriptureReferencePlugin.test.tsx, test-only). After the rebase, macOS CI failed "stops at the next verse marker for an empty verse with an editable marker" twice in a row, while ubuntu and local runs passed. The placement commits offset 0 of the next verse's marker. The asserted position, the end of this verse's marker, only appears once a selectionchange reads the DOM selection back and Lexical resolves the collapsed offset-0 point to the end of the previous text. jsdom fires that event a macrotask later, and the test asserted right after act(). It had passed only because the setup selection's own queued event happened to land after the placement. The test now flushes the setup event first and the placement's event before asserting, using the file's existing flushQueuedEvents. Forcing the macOS ordering locally reproduced CI's exact keys and offsets 3/3; with the fix it passes 3/3. Nothing in this branch touches that plugin or its test. I didn't pin down which of the branch's changes moved the event ordering on macOS; the race was in the test either way.

Merge order

Merge this before paranext-core#2653, and move platform-yalc past it before #2653 merges. #2653 now stands its Enter handling down whenever this menu is open and relies on the menu claiming the press. Against the current platform-yalc, where the menu claims Enter only with an item highlighted, an Enter pressed right after a right-click would split the paragraph, and #2653's isolated E2E would fail. CI runs only the smoke E2E, so it will not catch the wrong order.

Not fixed: Backspace before a paragraph's marker

Pre-existing and not \id-specific, found while testing the merge above. With the caret before a paragraph's marker glyph, Backspace joins the whole paragraph — glyph included — onto the end of the previous line. The glyph stays visible, but the save drops the marker: \ip more\p s on screen was saved as \ip mores. Normal paragraphs behave the same way.

Not fixed here

A host that claims Enter from a window capture-phase listener still starves this menu's document capture-phase listener, whatever order the two registered in, because capture descends window → document. That is a host-side decision (the host has to stand down while the menu is open) and is fixed in paranext-core rather than here; there is no library-side fix, since nothing this plugin does from document can run before a window listener.

The plugin's built-in Cut / Copy / Paste / Paste as Plain Text items — and the new menu aria-label — are still hardcoded English, and the host can only append items. Pre-existing, and worth its own change.

Repo move

Moved here from eten-tech-foundation/scripture-editors#555, which is closed with a pointer to this PR: paranext-core main now consumes paranext/scripture-editors, so a PR merged on the eten repo would never reach the host. Rebased --onto origin/main from the old base (the two mains have diverged — paranext carries its own PT-4304 commit), every commit patch-identical to the original. One commit is added on top: paranext commits packages/*/dist, which eten does not, so the inherited build output predates this branch.

Testing

  • Code-review fixes (2026-09-18): vitest run across the workspace — 177 files / 4150 passing, 1 skipped. nx run-many -t typecheck lint clean on platform-editor, platform, shared-react and scribe-editor (pre-existing no-console warnings only). pnpm rebuild-committed-output, then verify-committed-dist.mjs reports current after commit; the committed index.d.ts is the 1846-line rollup. paranext-core's isolated E2E was not re-run against this push.
  • This push (2026-09-18), rebased onto main: vitest run across the workspace — 177 files / 4136 passing, 1 skipped. nx run-many -t typecheck lint --skip-nx-cache clean across all 10 projects (warnings only). pnpm rebuild-committed-output, then verify-committed-dist.mjs reports Committed build output is current. ✓; the dist index.d.ts is the 1846-line rollup, not the tsc stub. Verified in the real app through paranext-core's isolated Playwright suite with this commit staged in: the full scripture-editor subset ran 9/9 with no retries, including #2653's new step (right-click, nothing highlighted, Enter then \: no palette, the menu stays up, and the paragraph neither splits nor gains a character).
  • vitest run across the workspace: 176 files / 4096 passing, 1 skipped — re-run on the new base after the move.
  • nx run-many -t typecheck and -t lint clean across the workspace; pnpm rebuild-committed-output then node scripts/verify-committed-dist.mjs reports Committed build output is current. ✓.
  • tsc --noEmit on packages/platform and libs/shared-react; eslint and prettier --check clean on every changed file.
  • The scroll test was verified red-then-green against the scroll fix; the ARIA tests red-then-green against the ARIA change; the \id-line corpus fixture red-then-green against createBook (6 failures before, all passing after).
  • Verified in the real app through paranext-core's isolated Playwright suite (this branch yalc-linked in): inserting an endnote into the \id line and re-reading the chapter rendered 0 notes before and 1 after, with the line's own text intact on both sides of the caller. Full scripture-editor subset 8/8, no retries.
  • \id-line editing (latest push): vitest run on packages/platform96 files / 1721 passing, including new tests that failed before each fix (bookLineSettle.test.tsx, two settleDifferential rows for the read-only mirror, and the \id-line merge in paraWholeDeletion.test.tsx). nx typecheck and nx lint clean on platform-editor (one pre-existing no-loop-func warning in verse-block.utils.ts); pnpm rebuild-committed-output then verify-committed-dist.mjs reports current.
  • PT-4613 pinning pass (latest push): vitest run on packages/platform96 files / 1728 passing. The new \id-line block in markerMenuApply.utils.test.tsx commits every entry the line offers and asserts the document changed, with a pin per offered kind (a real char span in the book; a real note whose key comes back for the host's session). Its falsifiable member is the one comparing the offered list against a live caret's context — checked red by restoring just markerMenuContext.utils.ts and markerMenuApply.utils.ts to their pre-fix revisions; the per-marker commits pass either way, since the old apply path already handled char and note picks. The stale item-source case for the book region, which asserted the earlier "no paraMarker, same list either way" model, was rewritten onto paraMarker: "id". nx lint and nx typecheck clean on platform-editor (same pre-existing no-loop-func warning); verify-committed-dist.mjs reports current — tests and docs only, so no dist rebuild.
  • Verified in the real app through paranext-core with this branch built in: \ip typed at the end of the \id line created the paragraph at once and the next characters went into it; the Enter palette picked \imt and started it after the line; three Backspaces on the marker of the paragraph below merged it into the line with the caret at the junction; and undo reverted the typed \ip and its split together.

🤖 Generated with Claude Code

https://claude.ai/code/session_01TXbNhzh48xgmAWmwxqhMXi


This change is Reviewable

@tjcouch-sil
tjcouch-sil force-pushed the standard-view-pt-4195 branch 2 times, most recently from 849fa4a to 1a64c51 Compare September 15, 2026 22:44
@tjcouch-sil tjcouch-sil changed the title Fix the editor context menu's clipping, scrolling and accessibility — and the \id line's dropped content Fix the editor context menu and \id-line editing Sep 17, 2026
tjcouch-sil and others added 18 commits September 18, 2026 09:23
…ole list

The right-click menu reuses `.typeahead-popover ul`, whose 200px cap is sized for
the marker typeahead's long, filterable list. The context menu is a short fixed
list (~36px per item), so a host contributing a few insert items pushes it past
that cap — and the items behind the fold could not be reached at all, because the
close-on-scroll listener is registered on `window` in capture phase and so fired
for the menu's OWN scroll, closing the menu on any attempt to scroll down to
them.

Cap the menu's list to the viewport instead of 200px, and ignore scroll events
that originate inside the menu.

Also adds the plugin's first tests. They drive real DOM keydowns rather than
dispatching Lexical commands, which is what makes the capture/bubble propagation
the menu depends on observable — its key handling sits on a capture-phase
listener on `document`, ahead of Lexical's bubble-phase listener on the root
element.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CkuPP4eA6PPSgCTadJ1J95
…e does

The same rule is re-stated in paranext-core's `_editor-overrides.scss`, because
the copy of this file vendored there cannot be edited in place. The two comments
were written separately and drifted into saying the same thing two ways; align
them so a reader comparing the two sees only the deliberate difference — the
host's note about when its override can be dropped.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CkuPP4eA6PPSgCTadJ1J95
…apter

The book node was the one content container the forward adaptor did not
recurse into: `createBook` flattened its content with `getTextContent`,
which returns "" unless the content is exactly one plain string. So the
moment anything else sat in the \id line — a note, say — the whole
content went with it, the line rendered as a bare `\id JON`, and the next
save wrote that truncation back to the file.

The reverse adaptor already recursed the book node's children, so this
was the asymmetric half of the pair and broke the round-trip invariant
the corpus suite exists to hold. Recursing makes the two halves agree;
`getTextContent` had no other caller and goes with it.

Pinned by a corpus fixture (round-trip identity in all five view modes,
plus the transform fixed point) and a forward anchor, since identity
alone is blind to a bug mirrored on both sides.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TXbNhzh48xgmAWmwxqhMXi
The menu's items were `role="option"` inside a bare `ul`, which is not a
listbox, so the role had no list to mean anything in and nothing told an
assistive technology which item the arrow keys had highlighted.

Completes the pattern the plugin had already started rather than moving
to menu/menuitem: focus deliberately stays in the contenteditable the
whole time the menu is open — that is what keeps the selection the chosen
item acts on — and the option list plus `aria-activedescendant` on the
focused element is the pattern for a popup whose owner keeps focus. It is
also what Lexical's own typeahead menu does. Moving focus into the menu
would be the menu/menuitem pattern's requirement and would put that
selection at risk.

The item ids move to their own namespace: Lexical's shared `typeahead-*`
ids are reused by every menu built on its typeahead, and an
`aria-activedescendant` IDREF resolving to another menu's item announces
the wrong thing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TXbNhzh48xgmAWmwxqhMXi
Capping the menu to the viewport fixed the fold at ordinary heights, but
the rule it inherits pairs `overflow-y: scroll` with a hidden scrollbar —
right for the marker typeahead, which you narrow by typing rather than by
scrolling. This menu has no filter, so below roughly 270px of panel
height its last items sat behind a fold with nothing on screen saying
they were there.

`auto` rather than `scroll` so the gutter stays out of the way at the
heights where the whole menu already fits.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TXbNhzh48xgmAWmwxqhMXi
…led to the end

A wheel gesture that reaches the end of the clipped list chains to the ancestor
scroller. That ancestor's scroll event does not originate inside the menu, so
the close-on-scroll listener reads it as the page moving out from under a
fixed-positioned menu and closes the menu — in exactly the case the list is long
enough to need scrolling at all. `overscroll-behavior: contain` keeps the
gesture in the list.

The same override was missing entirely from the three vendored copies of this
stylesheet, so the scribe package and both demos still capped the menu at 200px
behind a hidden scrollbar.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TXbNhzh48xgmAWmwxqhMXi
The `\id` line is a content container like any other — Paratext 9 accepts a
footnote or end note in its text content, and now so does the forward adaptor —
but two guards still treated a book node as a single run of text with nothing
before it:

- Backward arrow navigation blocked every move from offset 0 of a book node's
  text, so a caret in the text after a note could not get back past it. It now
  blocks only when nothing the caret can reach sits to the left, and the
  collapsed-note hop applies in the book line too: there is no verse to sit
  before its notes, because the book code is part of the line's own immutable
  marker text.
- The caret-at-end expand tested `$isSomeParaNode`, which excludes `BookNode`,
  so a note at the end of the `\id` line never expanded for inline editing.
  `$isParaLikeNode` is the existing union of exactly those two.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TXbNhzh48xgmAWmwxqhMXi
…id-line fixes

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TXbNhzh48xgmAWmwxqhMXi
…paragraph pick after it

The `\id` line is a BookNode at document root, not a ParaNode, but its text is content like any
paragraph's: Paratext 9 offers the inline marker list there, footnotes included. Two probes still
treated it as the header region:

- The `\` trigger found no ParaNode ancestor and reported paragraph source. Even forced to
  character source it would have listed nothing, because the character list filters on the
  enclosing paragraph's marker. A caret in the line now reports character source with
  `paraMarker: "id"`, which yields the character styles valid under `id` plus every note style.
- A paragraph pick had no paragraph to retag or split, so the Enter palette's Enter did nothing.
  The pick now splits the line at the caret and starts the new paragraph after it, closing and
  reopening an open character span across the cut. It lives in `$splitParagraphWithMarker`
  because the Enter palette calls it directly and never goes through the `\` palette's apply path.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… at a typed paragraph marker

The `\id` line had no settle scope, so the marker-edit engine kept everything typed there
literal: `\nd Lord\nd*` stayed as text on screen while the save wrote a real span, and the next
load showed the span. What the user saw and what the file held disagreed until a reload.

`$rebuildBook` is the fourth settle scope, beside paragraphs, note content and chapters. Like the
note scope it re-tokenizes only the line's content and preserves the book node, its code and its
immutable `\id GEN ` prefix, which are never re-derived from displayed bytes. The read-only settle
mirrors it, the undo re-pend scan now descends into the line, and live typing display-maps space
runs there the way load and settle already did.

A typed block marker ends the line where the file bytes would: `\p`, `\ip` or `\c` starts its
paragraph or chapter directly after the book, with the tail after the marker moving into it and
the caret following its byte — the same split a paragraph gets. Bytes that literally begin with
`\p` tokenize to a wrapper indistinguishable from the tokenizer's implied one, so they are told
apart by the bytes themselves and leave the line empty.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…nto that line

Deleting a paragraph's whole marker merges its content into the previous paragraph, but the
transform only recognized a ParaNode as the previous line. Directly below the `\id` line it fell
through to resetting the marker to `\p` with a fresh visible prefix, so every further Backspace
deleted the re-created marker and grew it back again, forever. The `\id` line takes content like a
paragraph, so a paragraph below it now merges into it the same way, caret at the junction.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ixes

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The offered list and the commit paths are one contract, so the tests now hold both ends of it.
`markerMenuApply.utils.test.tsx` commits EVERY entry the line offers and asserts the document
changed, with a pin per offered kind: a real character span inside the book, and a real note
whose Lexical key comes back for the host's editing session. A further test builds the list from
a caret in a live editor and compares it to the list the sweep iterates, so the two cannot drift
apart. The `\id`-line fixture is hoisted to module scope, since three describes build it.

The item-source case for the book region still asserted the earlier model — no `paraMarker`, and
the same paragraph list whichever source it was asked for — a context shape the editor no longer
produces. It now pins the inline list under `paraMarker: "id"` and that no paragraph entry reaches
it.

`docs/standard-view-invariants.md` records the rule the line's defects all came from ("The `\id`
line is content, not a header") and the general one they are an instance of ("A menu offers
nothing it cannot insert"), plus the book line's outcome beside the other marker-apply outcomes.

PT-4613

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…\id` line

The backward-navigation guard for a book node was relaxed only for notes, but
the `\id` line carries whatever follows the book code — a character span and a
milestone as readily as a note. At offset 0 of the text after one of those the
guard still claimed the press and preventDefault'd it, so ArrowLeft was a dead
key at that position and the caret could not walk back past the span.

The guard now asks the question it means to ask: is there anything but the
line's own immutable `\id GEN ` prefix to the left? Anything else there is a
real position to move to, left to the note handling below it or to the default
move.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…on fix

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…s open

The menu claimed Enter only while `selectedIndex` was set, so a freshly opened
menu — nothing highlighted, which is how every right-click starts — handed the
press back. The editor still has DOM focus behind the menu, so it went to
Lexical and split the paragraph there, and a host that gates its own Enter
behavior on this menu started a second keyboard mode underneath a menu that was
still armed.

Enter is now claimed whenever the menu is up and swallowed when there is
nothing to invoke. A disabled highlighted option is likewise a no-op that
leaves the menu open, rather than the dead key it was: it stopped the press
before testing `isDisabled`, but by then a host keying off `li.selected` had
already stood down.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… fix

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The branch was rebased onto main past #12 (font fallback chain), whose own
dist rebuild conflicted with this branch's. The rebase carried the branch's
dist forward, so the committed output lacked #12's `--usj-font-fallback`
rule; this regenerates it from the rebased source.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
tjcouch-sil and others added 5 commits September 18, 2026 10:33
Placement at an empty verse leaves the boundary element point, which Lexical
commits as offset 0 of the NEXT verse's marker. The asserted position, the end
of this verse's marker, only appears once a `selectionchange` reads the DOM
selection back and Lexical resolves the collapsed offset-0 point to the end of
the previous text. jsdom fires that event a macrotask later, and the test
asserted straight after act() without flushing it. It passed only because the
setup selection's own queued `selectionchange` happened to land after the
placement commit. On the macOS runner it landed before, so nothing re-read the
selection and the assertion saw verse 6's marker (CI: key 282 offset 0 vs 280
offset 5).

Flush the setup event before navigating, so the placement's own event is the
one that settles it, and flush again before asserting. Reproduced locally by
forcing the macOS ordering (3/3 red with the same keys and offsets as CI), and
green 3/3 with the fix.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The `\id` line now holds real character spans and notes, but marker
validation only ran the paragraph-level check on it and never descended
into its content, so an unknown marker there, or an invalid one inside a
note there, was never flagged.

Follow PT9's ValidateUsxStyles: a char checks occursUnder against
ancestor::note[1], else ancestor::para[1], and the `\id` line has no para
ancestor. So the line's own chars and verses are only checked for being
known (unknown is flagged by PT9's CSS everywhere), while a note's content
validates against the note's marker as it does in any paragraph. No
known span directly in the `\id` line can become newly invalid.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
tokenizedBookLine tells a `\p` the author typed at the start of the line's
content from the implied `\p` the tokenizer wraps leading content in by
re-reading the bytes, and that re-read disagreed with the tokenizer at both
edges:

- It skipped any leading whitespace, but the tokenizer drops a leading run
  only when it holds a line break. ` \p more` tokenizes to an implied `\p`
  holding the space plus the author's `\p more`, and was split as if both
  were the author's: the line emptied and a whitespace-only `\p` paragraph
  appeared after it, in both the live settle and the saved USJ.
- It ended the marker name only at whitespace or end of text, but
  scanMarkerName also ends it at `\`, `|` and ZWSP. `\p\bd Genesis\bd*`
  tokenizes to the author's `\p`, was taken for the implied one, and the
  typed `\p` silently disappeared.

The pattern now mirrors the tokenizer on both edges, pinned by a table of
the shapes where the two could disagree (the six bug rows fail against the
old pattern).

Also corrects the re-pend scan's comment, which still said the scan never
descends into books.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…anything

A paragraph pick over a selection in the `\id` line deleted the selection,
then gave up when the break point could not be lifted back to the book --
which happens whenever an annotation's mark wrapper sits between the caret
and the line, since only a char-span stack can be lifted out of. The
selected text was gone and no paragraph was made.

$canSplitBookAt now makes that decision up front for both selection points,
mirroring $liftOutOfCharStack (climb char spans only, then require the
book), so a pick that cannot split leaves the line untouched. The post-lift
bail stays as a backstop for wherever removing a selection left the caret.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…s the editor

The context menu's capture-phase keydown listener hears the whole document,
and nothing closes the menu when focus moves on (Tab). Since Enter is now
claimed whether or not an item is highlighted, a focused button's Enter was
swallowed and the button never activated; with an item highlighted it ran
that item instead.

The menu now drives Arrow/Enter only while the editor (or nothing) holds
focus. Escape still closes it from anywhere.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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