|
| 1 | +# Rich-text editor support for `Plain` blocks (tight list items) |
| 2 | + |
| 3 | +**Strand:** bd-7pxub583 (related to bd-sjb4pzx8 — the tiptap rich-text block editor) |
| 4 | +**Status:** IN PROGRESS — user approved 2026-07-06 |
| 5 | +**Date:** 2026-07-06 |
| 6 | + |
| 7 | +## Decisions locked with user (2026-07-06) |
| 8 | + |
| 9 | +- **Scope: BROAD.** Add `'Plain'` to `RICHTEXT_SUPPORTED_TYPES` so the rich-text |
| 10 | + editor activates for *every* reachable `Plain` edit target, not just |
| 11 | + list-item `Plain`s. Rationale (user): "the more settings where the rich text |
| 12 | + editor works well, the better." Risk #2's narrow option is dropped. |
| 13 | +- **Characterize tight/loose empirically with the `pampa` binary.** Q2 uses a |
| 14 | + **custom** markdown parser whose tight-vs-loose rules are *not identical to |
| 15 | + Pandoc's*. Do not rely on memory of Pandoc list semantics — generate each |
| 16 | + list variant as qmd and run it through `cargo run -p pampa -- -t json` (or |
| 17 | + `-t native`) to see the actual `Plain`/`Para` shape Q2 produces, and derive |
| 18 | + fixtures from that ground truth. |
| 19 | + |
| 20 | +## Overview |
| 21 | + |
| 22 | +In `format: q2-preview`, the tiptap rich-text block editor (bd-sjb4pzx8) |
| 23 | +activates only for a subset of Pandoc block types. That subset omits `Plain`. |
| 24 | +Because **tight** bullet/ordered lists store each item's content as a `Plain` |
| 25 | +block (loose lists use `Para`), clicking to edit a tight-list item's content |
| 26 | +resolves a `sourceNode` of type `Plain`, the availability gate returns `false`, |
| 27 | +and the editor falls back to the monospaced textarea instead of the rich-text |
| 28 | +surface. |
| 29 | + |
| 30 | +Goal: make the content of tight-list items editable as rich text, matching the |
| 31 | +experience already available for paragraphs and headers. |
| 32 | + |
| 33 | +## Root cause (confirmed by reading the code) |
| 34 | + |
| 35 | +The single gate is `RICHTEXT_SUPPORTED_TYPES` in |
| 36 | +`ts-packages/preview-renderer/src/q2-preview/richTextSupport.ts:17`: |
| 37 | + |
| 38 | +```ts |
| 39 | +export const RICHTEXT_SUPPORTED_TYPES = new Set<string>(['Para', 'Header']); |
| 40 | +``` |
| 41 | + |
| 42 | +`richTextAvailable(ctx, sourceNodeType)` (same file) is `ctx.richText && |
| 43 | +RICHTEXT_SUPPORTED_TYPES.has(sourceNodeType)`. The dispatcher |
| 44 | +(`dispatchers.tsx:517-526`, `renderBlockEditSurface`) uses exactly this |
| 45 | +predicate to choose `<RichTextEditor>` vs. the textarea. A tight-list item's |
| 46 | +leading block is a `Plain` (see `nestingNav.ts:649-653`, `outerBlocks.ts:333`), |
| 47 | +so it never matches. |
| 48 | + |
| 49 | +### Why the rest of the pipeline is (probably) already ready |
| 50 | + |
| 51 | +- **AST → ProseMirror** already maps `Plain` to a paragraph node: |
| 52 | + `astToProseMirror.ts:131-135` handles `case 'Para': case 'Plain':` |
| 53 | + identically. Tightness is inferred separately by `isTight()` |
| 54 | + (`astToProseMirror.ts:122-126`), which checks whether any item block is a |
| 55 | + `Para`. So seeding the rich editor from a `Plain` source node already works. |
| 56 | +- **Commit path is shared with the textarea.** Both `<RichTextEditor>` and |
| 57 | + `<EditTextarea>` commit through the identical `commitTextEdit(destination, |
| 58 | + newText)` byte-range-replacement channel (`usePreviewEdit.ts`, |
| 59 | + `PreviewContext.tsx`). The textarea **already** lets a user edit a tight-list |
| 60 | + `Plain` item today (it just isn't rich). So the byte-range resolution and |
| 61 | + the Rust-side re-parse/`apply_node_edit` machinery are already exercised for |
| 62 | + `Plain`. The rich editor changes only *what text* is written into that same |
| 63 | + range, not *how* it is written. |
| 64 | + |
| 65 | +**Net:** the visible bug is a one-line omission, but "add `'Plain'` to the set" |
| 66 | +is necessary, not obviously sufficient. The real work is verifying round-trip |
| 67 | +fidelity and deciding the scope of "any `Plain`" vs. "only list-item `Plain`". |
| 68 | + |
| 69 | +## Key risks / open questions to resolve before/while implementing |
| 70 | + |
| 71 | +1. **Tight-stays-tight round-trip — largely already solved by the backend.** |
| 72 | + When the rich editor serializes a single-`Plain`-seeded doc back to markdown |
| 73 | + (`docToMarkdown`, `serializer.ts`), the PM node is a `paragraph` — |
| 74 | + indistinguishable from a `Para` seed. The edit target is only the item's |
| 75 | + *inner* byte range (no `- ` marker), and `commitTextEdit` splices new text |
| 76 | + into that range, where it re-parses as a bare `Paragraph`. **But the Rust |
| 77 | + commit path already anticipates exactly this:** `preserve_leaf_variant` in |
| 78 | + `crates/pampa/src/apply_node_edit.rs:253-265` coerces a *single-block* |
| 79 | + `Paragraph` replacement back to `Plain` when the original block was `Plain`, |
| 80 | + precisely so an inline-text edit of a tight list item does not loosen the |
| 81 | + list. The rich editor commits through the **same text channel** the textarea |
| 82 | + uses, so it inherits this guard for free. The `len() == 1` condition is |
| 83 | + load-bearing: a legitimately multi-block edit still loosens. Since a single |
| 84 | + tight-item content edit stays one paragraph (the serializer's `_`-italic and |
| 85 | + mark transforms don't add blocks), the guard should fire. `nestingNav.ts:184` |
| 86 | + flags the same hazard for divs, and the textarea path already relies on this |
| 87 | + guard today. → **Test:** interactive edit of a tight-list item, assert the |
| 88 | + on-disk qmd stays tight and the AST item block stays `Plain` — this is a |
| 89 | + *regression guard* on `preserve_leaf_variant` being exercised by the rich |
| 90 | + path, not new machinery we must build. |
| 91 | + |
| 92 | +2. **Scope of "`Plain`".** `Plain` appears in more than tight list items: |
| 93 | + table cells, definition-list terms/definitions, and (historically) some |
| 94 | + figure/caption contexts. Adding `'Plain'` to `RICHTEXT_SUPPORTED_TYPES` |
| 95 | + makes *every* reachable `Plain` edit target rich. We must decide: |
| 96 | + - **(a) Broad:** allow rich text for any `Plain`. Simplest; but only correct |
| 97 | + if every `Plain` edit target round-trips safely and the measured-edit box |
| 98 | + renders acceptably in those contexts (table cells especially). |
| 99 | + - **(b) Narrow:** allow rich text for `Plain` only when the resolved edit |
| 100 | + target sits inside a list item. Requires threading list-context into the |
| 101 | + availability predicate (the resolver / breadcrumb already knows the |
| 102 | + ancestor chain — see `nestingNav.ts`, `outerBlocks.ts`). |
| 103 | + → Determine which `Plain` contexts are even *reachable* as edit targets in |
| 104 | + q2-preview today. If only list items are reachable, (a) and (b) converge and |
| 105 | + we take (a). |
| 106 | + |
| 107 | +3. **Multi-block tight items.** A tight list item can be `Plain` + nested |
| 108 | + `BulletList` (a tight item with a sublist). The edit target for the leading |
| 109 | + `Plain` should cover only that `Plain`'s range, leaving the sublist intact. |
| 110 | + Confirm the leading-`Plain` edit target does not swallow the sublist on |
| 111 | + commit. → **Test:** nested tight list, edit the parent item text, assert the |
| 112 | + sublist survives unchanged. |
| 113 | + |
| 114 | +4. **Breadcrumb / measured-edit parity.** The breadcrumb and the |
| 115 | + measure-and-set edit box already special-case `Plain` (`outerBlocks.ts:333`, |
| 116 | + `403`, the `Pl` glyph in `nestingNav.ts`). Adding rich-text for `Plain` |
| 117 | + should reuse the existing `leaf-text` category (`categoryForSourceNode` |
| 118 | + already groups `Plain` with `Para`/`Header`), so the breadcrumb should need |
| 119 | + no change — verify, don't assume. |
| 120 | + |
| 121 | +## Plan (TDD — tests first, per project policy) |
| 122 | + |
| 123 | +### Phase 0 — Reproduce & characterize (no code change yet) |
| 124 | +- [x] **Characterize Q2's tight/loose list AST with `pampa`.** Ran qmd variants |
| 125 | + through `cargo run -p pampa --bin pampa -- -t json`. Findings: |
| 126 | + - Tight bullet/ordered → items are `Plain`; loose (blank line between |
| 127 | + items) → items are `Para`. Matches the standard model. |
| 128 | + - Nested tight: first item = `[Plain, BulletList]`; the leading `Plain` |
| 129 | + has its own source range distinct from the sublist, so an edit target |
| 130 | + scopes to just that `Plain` (won't swallow the sublist). |
| 131 | + - **Table cells → `Plain`.** Under broad scope, table-cell content becomes |
| 132 | + rich-editable too — include in corpus + verify. |
| 133 | + - Def lists: `Term\n: def` did **not** parse as a `DefinitionList` in Q2 |
| 134 | + (came out as two `Para`s), so def-list `Plain` is not a live edit-target |
| 135 | + context here. Not a concern. |
| 136 | +- [x] Reachable `Plain` contexts: **tight list items (bullet/ordered, nested)** |
| 137 | + and **table cells**. Broad scope covers both. |
| 138 | +- [x] **Backend tightness guarantee already exists AND is tested.** |
| 139 | + `preserve_leaf_variant` (`apply_node_edit.rs:253`) coerces a single |
| 140 | + re-parsed `Paragraph` back to `Plain` on the text channel. Covered by |
| 141 | + `text_edit_preserves_bullet_list_tightness` (node_edit_tests.rs:1308) and |
| 142 | + `text_edit_preserves_ordered_list_tightness` (1347), which drive |
| 143 | + `edit_nested_block` — the *exact* text-channel path `commitTextEdit` uses. |
| 144 | + **Conclusion:** no Rust changes needed; risk #1 is already closed. The |
| 145 | + fix is TS-gate-only. |
| 146 | +- [ ] Live browser "before" is superseded by the failing gate test (Phase 1) as |
| 147 | + the bug evidence; live confirmation folded into Phase 3 end-to-end. |
| 148 | + |
| 149 | +### Phase 1 — Tests first (TDD red → green) |
| 150 | +- [x] **Gate unit test** — `richTextSupport.test.ts` (new; the module had no |
| 151 | + direct test). Locks the membership set + flag/mode interactions. The |
| 152 | + `Plain` assertions **failed before the fix** (3 red) and pass after. This |
| 153 | + is the direct encoding of the bug fix. |
| 154 | +- [x] **Dispatcher integration test** — `plain-list-item-richtext.integration.test.tsx` |
| 155 | + (new). Drives the real `PreviewRoot`; clicking a tight bullet-list item |
| 156 | + opens the **rich** editor (`.q2-rt-toolbar` present, no textarea) with the |
| 157 | + flag on, and the **textarea** with the flag off. **Confirmed it fails with |
| 158 | + the fix reverted** (toolbar null) → true regression guard. |
| 159 | +- [x] **Converter seed test** — `richtext/plainSeed.test.ts` (new, pure/no |
| 160 | + pampa). Guards the exact `astToDoc([Plain], …)` seed the RichTextEditor |
| 161 | + uses: a lone `Plain` → one paragraph → serializes to the bare inline text |
| 162 | + (marks incl. `**bold**`/`_italic_`), no dropped nodes, no list marker. |
| 163 | +- [~] Tightness-on-commit is **already** covered by the Rust suite |
| 164 | + (`text_edit_preserves_{bullet,ordered}_list_tightness`, Phase 0). The rich |
| 165 | + editor commits through that same text channel, so no new Rust test is |
| 166 | + needed; Phase 3 confirms it live. |
| 167 | + |
| 168 | +### Phase 2 — Implement the gate change (BROAD) |
| 169 | +- [x] Added `'Plain'` to `RICHTEXT_SUPPORTED_TYPES` in `richTextSupport.ts` and |
| 170 | + documented why (tight-list items + table cells; backend preserves |
| 171 | + tightness). No dispatcher change needed — the existing predicate + the |
| 172 | + already-Plain-aware `astToProseMirror` do the rest. |
| 173 | +- [x] Phase 1 tests green with the fix in place. |
| 174 | + |
| 175 | +### Phase 3 — End-to-end verification (mandatory, per CLAUDE.md) |
| 176 | +Built the real chain (`cargo xtask build-q2-preview-spa` + `cargo build --bin q2`; |
| 177 | +no WASM/Rust change so no WASM rebuild needed) and drove `q2 preview --allow-edit` |
| 178 | +in a browser on a tight-list fixture. |
| 179 | + |
| 180 | +- [x] **Rich editor opens for a tight-list `Plain` item.** Clicking "banana" |
| 181 | + opened the tiptap editor — ProseMirror `<p>banana</p>`, the formatting |
| 182 | + toolbar (B/I/S/sub/sup/link), the "Editing… rich text / plain text" |
| 183 | + toggle, and the `Pl` breadcrumb glyph. Before the fix this opened the |
| 184 | + monospaced textarea. **This is the core deliverable and it works.** |
| 185 | +- [x] **Simple text edit round-trips and preserves tightness.** Edited |
| 186 | + "banana" → "banana XX" and committed; on-disk file kept a **tight** list |
| 187 | + (`pampa -t json` → all three items `Plain`). Instrumented commit confirmed |
| 188 | + `sourceNodeType: "Plain"`, committed markdown `"banana XX"`. |
| 189 | +- [x] Suites green after the change (unit 484, integration 517, tsc clean; the |
| 190 | + pampa-oracle round-trip 15/15). No regressions in the existing Para/Header |
| 191 | + rich-text tests. |
| 192 | +- [x] **`cargo xtask verify` (Rust+hub) still owed** before push (TS-only change, |
| 193 | + but run it per policy). |
| 194 | + |
| 195 | +#### ⚠️ Bug found during E2E — filed as **bd-hafs0qho** (not caused by this change) |
| 196 | +Committing a **bold** edit (select-all + bold + commit) could **drop the item's |
| 197 | +content** — e.g. `- banana` → `-` (empty). Instrumented capture showed the |
| 198 | +committed ProseMirror doc was `paragraph[hardBreak]` (text gone), so the |
| 199 | +serializer correctly produced an empty string. |
| 200 | + |
| 201 | +Root cause is **upstream** of the serializer and the Rust commit path, both of |
| 202 | +which I verified correct: |
| 203 | +- serializer: `paragraph[strong "x"]` → `**x**`, `paragraph[strong "x", hardBreak]` |
| 204 | + → `**x**` (deterministic tests); |
| 205 | +- pampa `apply_node_edit`: replacing a tight-list `Plain` with `**BOLD**` → |
| 206 | + `* **BOLD**`, tight preserved (node_edit_tests.rs repro). |
| 207 | + |
| 208 | +It is **not `Plain`-specific**: the RichTextEditor, `astToProseMirror`, and the |
| 209 | +serializer are type-agnostic (Para and Plain both map to a `paragraph` node and |
| 210 | +share seed/edit/serialize/commit). A synthetic repro on a **Para** also injected |
| 211 | +a spurious `hardBreak`, so the behavior is shared with the pre-existing Para path |
| 212 | +(bd-sjb4pzx8) and predates this change. My browser repros were partly confounded |
| 213 | +(couldn't drive tiptap's internal selection from outside), so bd-hafs0qho needs a |
| 214 | +clean real-user reproduction + root-cause of the spurious hardBreak/text-drop. |
| 215 | + |
| 216 | +**Decision point for the user:** the Plain-gate change is safe and delivers the |
| 217 | +feature for text editing, and the bold anomaly is pre-existing (affects Para |
| 218 | +equally). Options: (a) merge this change now and fix bd-hafs0qho separately, or |
| 219 | +(b) hold this change until bd-hafs0qho is root-caused. Awaiting direction. |
| 220 | + |
| 221 | +### Phase 4 — Wrap up |
| 222 | +- [ ] Record the exact invocation + inspected output in this plan and the |
| 223 | + strand, per the end-to-end-verification policy. |
| 224 | +- [ ] Update `hub-client/changelog.md` if any `hub-client/` file changed |
| 225 | + (the two-commit workflow). |
| 226 | +- [ ] Close bd-7pxub583 with a reason once all tests pass and the feature is |
| 227 | + verified end-to-end. |
| 228 | + |
| 229 | +## Files in scope |
| 230 | + |
| 231 | +- `ts-packages/preview-renderer/src/q2-preview/richTextSupport.ts` — the gate |
| 232 | + (primary change). |
| 233 | +- `ts-packages/preview-renderer/src/q2-preview/richtext/roundtrip.test.ts` — |
| 234 | + new fixtures/assertions. |
| 235 | +- `ts-packages/preview-renderer/src/q2-preview/richtext/astToProseMirror.ts` / |
| 236 | + `serializer.ts` — likely **no change** (already handle `Plain`), but the |
| 237 | + serializer is where a tightness-preservation fix would land if risk #1 bites. |
| 238 | +- Possibly `dispatchers.tsx` — only if the narrow (context-aware) option is |
| 239 | + chosen and the predicate needs more than the node type. |
| 240 | + |
| 241 | +## Non-goals |
| 242 | + |
| 243 | +- Structural list editing (Enter to split items, Tab to nest) — that is |
| 244 | + bd-sjb4pzx8's Phase 1c and is out of scope here. This strand is specifically |
| 245 | + about making the *content* of an existing tight-list item rich-text editable. |
| 246 | +- Table-cell / definition-list rich editing beyond whatever falls out of the |
| 247 | + broad-vs-narrow decision. |
0 commit comments