From 1c07c4c537188a530aba7cb9284c9ac4d60c247b Mon Sep 17 00:00:00 2001 From: Tyler Longwell <109685178+tlongwell-block@users.noreply.github.com> Date: Tue, 19 May 2026 15:12:24 -0400 Subject: [PATCH 1/3] fix(composer): use native ProseMirror inserts for autocomplete MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mention/channel/emoji autocomplete used to round-trip through the full markdown parser on every selection: build a new `content` string by splicing the suggestion in at text-offset space, then call `setContent(markdown)`. That reparse: - dropped Shift+Enter hard breaks (newlines collapsed when you picked a mention/emoji — sprout-bugs report #2), - blew away any in-progress marks / list context at the cursor (the "mention ruins formatting" report — sprout-bugs report #3), - is exquisitely sensitive to subtle markdown parser/serializer asymmetries that can shift as either side ticks. It also exposed a latent off-by-one: `getTextAndCursor` used `state.doc.textContent`, which does *not* honour Tiptap's hardBreak `renderText` — so each Shift+Enter shifted the cursor offset by 1 relative to what mention/channel detection saw. The bug was invisible until you tried to round-trip the offset back to a PM position, which `setContent` happened to do indirectly. This change replaces the round-trip with a single native ProseMirror transaction: - New module `plainTextProjection.ts` builds a one-shot projection of the doc into plain text (hardBreak + inter-block boundary both contribute `\n`, matching `doc.textBetween(0, end, "\n", "\n")`) and exposes two helpers that share the *same* walk: `mapPMToTextOffset(pm)` `mapTextOffsetToPM(offset)` One source of truth → forward + reverse mappings can't drift. - `useRichTextEditor` gains: `getPlainTextAndCursor()` replaces `getTextAndCursor` `replacePlainTextRange(from, to, text)` single `tr.insertText` call; cursor lands at `tr.mapping.map(fromPM + text.length)` so the helper remains correct if anything else shifts positions. - Hooks (`useMentions`, `useChannelLinks`, `useEmojiAutocomplete`) now return an `AutocompleteEdit` descriptor `{ replaceFromOffset, replaceToOffset, insertText }` instead of a pre-spliced `{ nextContent, nextCursor }`. They stay editor-agnostic — they just describe the edit in plain-text-offset space. - Both composers (`MessageComposer` + `ForumComposer`) swap `setContentWithTrailingSpace(nextContent)` for `replacePlainTextRange(from, to, insertText)` via a small shared `applyAutocompleteEdit` helper. Removed: - `setContentWithTrailingSpace` (no callers). - The trailing-space ProseMirror transaction hack (`insertText` already ends in a space; no need to evade markdown's whitespace trimming if we never re-parse). - The old node-walk in `getTextAndCursor` (replaced by the projection helper, which is correct for hardBreak). Preserved: - `breaks: true` on tiptap-markdown (already set). - `\\\n` → `\n` strip in `getMarkdownFromEditor` — wire-format behaviour; render uses remark-breaks; no longer load-bearing for cursor edits, safe either way. - Emoji-picker button + toolbar @ button paths (already native `chain().insertContent()` — control cases, unchanged). Tests: - New `plainTextProjection.test.mjs` (20 tests) covering single paragraph, hardBreak before/after/across, multi-paragraph, nested list items, paragraph + bullet-list mix, edge cases (empty doc, out-of-range clamping), and a property-test round-trip that every text offset maps to a PM position that maps back. - Removed `useRichTextEditor.test.mjs` — it tested a copy of the old algorithm against a hand-built node list, which no longer exists. The new test exercises the real Tiptap schema directly. - All existing node tests still pass (176/176). - typecheck + biome + production build clean. - File sizes within limits. Net diff in existing files: +155 / -273 (-118 LOC). New files: ~200 LOC of pure projection + thorough tests. Co-authored-by: Quinn (sprout agent) <96f056ad5f2305c8ddf637dc65d048aa4c12d7daeb8867690e34fca46b0ef64c@sprout-oss.stage.blox.sqprod.co> Signed-off-by: Tyler Longwell <109685178+tlongwell-block@users.noreply.github.com> --- .../src/features/forum/ui/ForumComposer.tsx | 45 ++-- .../messages/lib/plainTextProjection.test.mjs | 217 ++++++++++++++++++ .../messages/lib/plainTextProjection.ts | 203 ++++++++++++++++ .../features/messages/lib/useChannelLinks.ts | 19 +- .../messages/lib/useEmojiAutocomplete.ts | 20 +- .../src/features/messages/lib/useMentions.ts | 19 +- .../messages/lib/useRichTextEditor.test.mjs | 126 ---------- .../messages/lib/useRichTextEditor.ts | 130 +++++------ .../features/messages/ui/MessageComposer.tsx | 69 +++--- 9 files changed, 575 insertions(+), 273 deletions(-) create mode 100644 desktop/src/features/messages/lib/plainTextProjection.test.mjs create mode 100644 desktop/src/features/messages/lib/plainTextProjection.ts delete mode 100644 desktop/src/features/messages/lib/useRichTextEditor.test.mjs diff --git a/desktop/src/features/forum/ui/ForumComposer.tsx b/desktop/src/features/forum/ui/ForumComposer.tsx index 012799b37..94432712f 100644 --- a/desktop/src/features/forum/ui/ForumComposer.tsx +++ b/desktop/src/features/forum/ui/ForumComposer.tsx @@ -96,40 +96,47 @@ export function ForumComposer({ setContent(markdown); contentRef.current = markdown; - const { cursor } = richText.getTextAndCursor(); + const { cursor } = richText.getPlainTextAndCursor(); mentions.updateMentionQuery(text, cursor); channelLinks.updateChannelQuery(text, cursor); }, }); // ── Mention / channel autocomplete insertion ──────────────────────── + // Native ProseMirror transactions — no markdown round-trip. const applyMentionInsert = React.useCallback( (suggestion: MentionSuggestion) => { - const { text, cursor } = richText.getTextAndCursor(); - const result = mentions.insertMention(suggestion, text, cursor); - richText.setContentWithTrailingSpace(result.nextContent); - setContent(result.nextContent); - contentRef.current = result.nextContent; + const { cursor } = richText.getPlainTextAndCursor(); + const { replaceFromOffset, replaceToOffset, insertText } = + mentions.insertMention(suggestion, cursor); + richText.replacePlainTextRange( + replaceFromOffset, + replaceToOffset, + insertText, + ); }, [ mentions.insertMention, - richText.getTextAndCursor, - richText.setContentWithTrailingSpace, + richText.getPlainTextAndCursor, + richText.replacePlainTextRange, ], ); const applyChannelInsert = React.useCallback( (suggestion: ChannelSuggestion) => { - const { text, cursor } = richText.getTextAndCursor(); - const result = channelLinks.insertChannel(suggestion, text, cursor); - richText.setContentWithTrailingSpace(result.nextContent); - setContent(result.nextContent); - contentRef.current = result.nextContent; + const { cursor } = richText.getPlainTextAndCursor(); + const { replaceFromOffset, replaceToOffset, insertText } = + channelLinks.insertChannel(suggestion, cursor); + richText.replacePlainTextRange( + replaceFromOffset, + replaceToOffset, + insertText, + ); }, [ channelLinks.insertChannel, - richText.getTextAndCursor, - richText.setContentWithTrailingSpace, + richText.getPlainTextAndCursor, + richText.replacePlainTextRange, ], ); @@ -147,7 +154,7 @@ export function ForumComposer({ // ── @ mention picker (toolbar button) ─────────────────────────────── const openMentionPicker = React.useCallback(() => { if (!richText.editor) return; - const { text, cursor } = richText.getTextAndCursor(); + const { text, cursor } = richText.getPlainTextAndCursor(); const beforeCursor = text.slice(0, cursor); if (/(?:^|[\s])@[^\s]*$/.test(beforeCursor)) { @@ -162,12 +169,12 @@ export function ForumComposer({ richText.editor.chain().focus().insertContent(prefix).run(); setIsEmojiPickerOpen(false); - const updatedText = richText.editor.state.doc.textContent; - const { cursor: updatedCursor } = richText.getTextAndCursor(); + const { text: updatedText, cursor: updatedCursor } = + richText.getPlainTextAndCursor(); mentions.updateMentionQuery(updatedText, updatedCursor); }, [ richText.editor, - richText.getTextAndCursor, + richText.getPlainTextAndCursor, richText.focus, mentions.updateMentionQuery, ]); diff --git a/desktop/src/features/messages/lib/plainTextProjection.test.mjs b/desktop/src/features/messages/lib/plainTextProjection.test.mjs new file mode 100644 index 000000000..4c7c89dd8 --- /dev/null +++ b/desktop/src/features/messages/lib/plainTextProjection.test.mjs @@ -0,0 +1,217 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { getSchema } from "@tiptap/core"; +import StarterKit from "@tiptap/starter-kit"; + +import { buildPlainTextProjection } from "./plainTextProjection.ts"; + +// ── Build the actual Tiptap schema we use in the composer ───────────── +// Matching useRichTextEditor's StarterKit configuration (minus things +// that don't affect the schema shape). This guarantees the projection +// is tested against the *real* node names and types. + +const schema = getSchema([ + StarterKit.configure({ + hardBreak: { keepMarks: true }, + heading: false, + trailingNode: false, + link: false, + }), +]); + +const para = (...c) => schema.nodes.paragraph.create(null, c); +const t = (s) => schema.text(s); +const br = () => schema.nodes.hardBreak.create(); +const li = (...c) => schema.nodes.listItem.create(null, c); +const ul = (...c) => schema.nodes.bulletList.create(null, c); + +function doc(...content) { + return schema.nodes.doc.create(null, content); +} + +// ── Helper: assert text equals textBetween(blockSep, leafText="\n") ─── + +function assertMatchesTextBetween(d, p) { + const expected = d.textBetween(0, d.content.size, "\n", "\n"); + assert.equal( + p.text, + expected, + `projection.text should equal doc.textBetween(0..size, "\\n", "\\n")`, + ); +} + +// ── Single-paragraph ────────────────────────────────────────────────── + +test("single paragraph: text is the paragraph's content", () => { + const d = doc(para(t("hello"))); + const p = buildPlainTextProjection(d); + assert.equal(p.text, "hello"); + assertMatchesTextBetween(d, p); +}); + +test("single paragraph: PM↔text mapping is identity within text node", () => { + const d = doc(para(t("hello"))); + const p = buildPlainTextProjection(d); + // PM: para=0, "hello"=1..6 + assert.equal(p.mapPMToTextOffset(1), 0); + assert.equal(p.mapPMToTextOffset(3), 2); + assert.equal(p.mapPMToTextOffset(6), 5); + assert.equal(p.mapTextOffsetToPM(0), 1); + assert.equal(p.mapTextOffsetToPM(2), 3); + assert.equal(p.mapTextOffsetToPM(5), 6); +}); + +// ── HardBreak ───────────────────────────────────────────────────────── + +test("hardBreak contributes a single \\n", () => { + const d = doc(para(t("hello"), br(), t("world"))); + const p = buildPlainTextProjection(d); + assert.equal(p.text, "hello\nworld"); + assertMatchesTextBetween(d, p); +}); + +test("cursor before
maps to text offset just before the \\n", () => { + // PM: para=0, "hello"=1..5,
=6, "world"=7..11 + const d = doc(para(t("hello"), br(), t("world"))); + const p = buildPlainTextProjection(d); + assert.equal(p.mapPMToTextOffset(6), 5); +}); + +test("cursor after
maps to text offset just after the \\n", () => { + const d = doc(para(t("hello"), br(), t("world"))); + const p = buildPlainTextProjection(d); + assert.equal(p.mapPMToTextOffset(7), 6); +}); + +test("text offset right after \\n maps to PM after the break", () => { + const d = doc(para(t("hello"), br(), t("world"))); + const p = buildPlainTextProjection(d); + assert.equal(p.mapTextOffsetToPM(6), 7); +}); + +test("text offset just before \\n maps to PM before the break", () => { + const d = doc(para(t("hello"), br(), t("world"))); + const p = buildPlainTextProjection(d); + assert.equal(p.mapTextOffsetToPM(5), 6); +}); + +// ── Multi-paragraph ─────────────────────────────────────────────────── + +test("two paragraphs: block boundary contributes a single \\n", () => { + const d = doc(para(t("aaa")), para(t("bbb"))); + const p = buildPlainTextProjection(d); + assert.equal(p.text, "aaa\nbbb"); + assertMatchesTextBetween(d, p); +}); + +test("two paragraphs: cursor in second paragraph maps past the boundary \\n", () => { + // PM: p1 nodeSize=5 (token + 3 chars + token), p2 opens at PM=5 + // "aaa" text at 1..4 (size 3), p1 closes at PM=4..5, p2 opens at PM=5, + // "bbb" text at 6..9 + const d = doc(para(t("aaa")), para(t("bbb"))); + const p = buildPlainTextProjection(d); + assert.equal(p.mapPMToTextOffset(6), 4); + assert.equal(p.mapTextOffsetToPM(4), 6); +}); + +test("three paragraphs: cumulative block boundaries", () => { + const d = doc(para(t("aaa")), para(t("bbb")), para(t("ccc"))); + const p = buildPlainTextProjection(d); + assert.equal(p.text, "aaa\nbbb\nccc"); + assertMatchesTextBetween(d, p); + // "ccc" text starts at PM=11 → text offset 8 + assert.equal(p.mapPMToTextOffset(11), 8); + assert.equal(p.mapTextOffsetToPM(8), 11); +}); + +// ── HardBreak + multi-paragraph ────────────────────────────────────── + +test("paragraph with
then second paragraph", () => { + const d = doc(para(t("line1"), br(), t("line2")), para(t("para2"))); + const p = buildPlainTextProjection(d); + assert.equal(p.text, "line1\nline2\npara2"); + assertMatchesTextBetween(d, p); +}); + +test("range crossing a
", () => { + // PM: para=0, "line1"=1..5,
=6, "line2"=7..11 + // textOffset 2..8 = "ne1\nli" → PM 3..9 + const d = doc(para(t("line1"), br(), t("line2"))); + const p = buildPlainTextProjection(d); + assert.equal(p.mapTextOffsetToPM(2), 3); + assert.equal(p.mapTextOffsetToPM(8), 9); +}); + +// ── List items (nested blocks under bulletList) ────────────────────── + +test("bullet list: items separated by \\n (matches textBetween)", () => { + const d = doc(ul(li(para(t("first"))), li(para(t("second"))))); + const p = buildPlainTextProjection(d); + assert.equal(p.text, "first\nsecond"); + assertMatchesTextBetween(d, p); +}); + +test("paragraph + bullet list", () => { + const d = doc(para(t("intro")), ul(li(para(t("a"))), li(para(t("b"))))); + const p = buildPlainTextProjection(d); + assert.equal(p.text, "intro\na\nb"); + assertMatchesTextBetween(d, p); +}); + +test("list item: PM↔text round-trip lands in the right item", () => { + const d = doc(ul(li(para(t("first"))), li(para(t("second"))))); + const p = buildPlainTextProjection(d); + const pm = p.mapTextOffsetToPM(6); // start of "second" + assert.equal(p.mapPMToTextOffset(pm), 6); +}); + +// ── Edge cases ─────────────────────────────────────────────────────── + +test("offset 0 maps inside the first text node", () => { + const d = doc(para(t("hello"))); + const p = buildPlainTextProjection(d); + assert.equal(p.mapTextOffsetToPM(0), 1); +}); + +test("offset past end clamps to end-of-doc content position", () => { + const d = doc(para(t("hello"))); + const p = buildPlainTextProjection(d); + assert.equal(p.mapTextOffsetToPM(999), 6); +}); + +test("PM position past doc clamps to text.length", () => { + const d = doc(para(t("hello"))); + const p = buildPlainTextProjection(d); + assert.equal(p.mapPMToTextOffset(999), 5); +}); + +test("empty paragraph: empty text, mappings clamp safely", () => { + const d = doc(para()); + const p = buildPlainTextProjection(d); + assert.equal(p.text, ""); + assert.equal(p.mapPMToTextOffset(0), 0); + assert.equal(p.mapPMToTextOffset(1), 0); + // Inside the empty paragraph → PM=1 + assert.equal(p.mapTextOffsetToPM(0), 1); +}); + +// ── Property: round-trip ───────────────────────────────────────────── + +test("round-trip: text offset → PM → text offset is identity", () => { + const d = doc( + para(t("line1"), br(), t("line2")), + para(t("para2")), + ul(li(para(t("item-a"))), li(para(t("item-b")))), + ); + const p = buildPlainTextProjection(d); + for (let offset = 0; offset <= p.text.length; offset++) { + const pm = p.mapTextOffsetToPM(offset); + const back = p.mapPMToTextOffset(pm); + assert.equal( + back, + offset, + `offset ${offset} → pm ${pm} → offset ${back} (text=${JSON.stringify(p.text)})`, + ); + } +}); diff --git a/desktop/src/features/messages/lib/plainTextProjection.ts b/desktop/src/features/messages/lib/plainTextProjection.ts new file mode 100644 index 000000000..9f8c16bcc --- /dev/null +++ b/desktop/src/features/messages/lib/plainTextProjection.ts @@ -0,0 +1,203 @@ +import type { Node as ProseMirrorNode } from "@tiptap/pm/model"; + +/** + * Plain-text projection of a ProseMirror document. + * + * Plain-text is what a textarea-shaped consumer (mention/channel/emoji + * autocomplete) reads: hard breaks render as `\n`, and a single `\n` + * separates content in different leaf blocks (paragraphs, list items, + * etc.) — the same convention as `doc.textBetween(from, to, "\n", "\n")`. + * + * The same walk is used to map *both* directions: + * - PM position → plain-text offset (`mapPMToTextOffset`) + * - text offset → PM position (`mapTextOffsetToPM`) + * + * Keeping a single source of truth means the two mappings can't drift, + * which is the historic source of off-by-one bugs around `hardBreak` and + * multi-block docs. + * + * Pure function — no editor / view / React dependency. + */ +export type PlainTextProjection = { + /** The plain-text projection of the document. */ + text: string; + /** Map a ProseMirror position to a plain-text offset. Clamps to [0, text.length]. */ + mapPMToTextOffset: (pm: number) => number; + /** Map a plain-text offset to a ProseMirror position. Clamps to a valid in-doc position. */ + mapTextOffsetToPM: (offset: number) => number; +}; + +type Segment = + // A text node: pm range and text range have equal length. + | { + kind: "text"; + pmFrom: number; + pmTo: number; + textFrom: number; + textTo: number; + } + // A hardBreak: 1 PM position wide, contributes one `\n`. + | { + kind: "hardBreak"; + pmFrom: number; + pmTo: number; + textFrom: number; + textTo: number; + } + // A boundary between two leaf-block siblings (paragraphs, list items, + // headings, etc.) — zero PM positions wide. Both sides resolve to + // `pmAt`, which is the boundary point between the two blocks (= start + // of the next leaf-block's content, minus its opening token). + | { + kind: "blockBoundary"; + pmAt: number; + textFrom: number; + textTo: number; + }; + +/** + * Build a `PlainTextProjection` for the given doc. + * + * Walks the doc once and records each text node, hard break, and the + * boundary between consecutive leaf-blocks. A "leaf block" is any block + * node that does not itself contain blocks — `doc.textBetween` treats + * exactly those boundaries as inserting the blockSeparator, and we do + * the same so our text projection equals `doc.textBetween(0, end, "\n", "\n")`. + */ +export function buildPlainTextProjection( + doc: ProseMirrorNode, +): PlainTextProjection { + const segments: Segment[] = []; + const textParts: string[] = []; + let cursorText = 0; + /** + * True once we've entered at least one leaf-block. Subsequent leaf-blocks + * emit a boundary `\n` before their content — matching `textBetween`. + */ + let sawLeafBlock = false; + + doc.descendants((node, pos) => { + // ── Leaf inline: text ────────────────────────────────────────── + if (node.isText) { + const t = node.text ?? ""; + segments.push({ + kind: "text", + pmFrom: pos, + pmTo: pos + t.length, + textFrom: cursorText, + textTo: cursorText + t.length, + }); + textParts.push(t); + cursorText += t.length; + return false; // text nodes have no children + } + + // ── Leaf inline: hard break ──────────────────────────────────── + if (node.type.name === "hardBreak") { + segments.push({ + kind: "hardBreak", + pmFrom: pos, + pmTo: pos + 1, + textFrom: cursorText, + textTo: cursorText + 1, + }); + textParts.push("\n"); + cursorText += 1; + return false; + } + + // ── Block ────────────────────────────────────────────────────── + if (node.isBlock) { + // Only "leaf blocks" (those with inline content — paragraphs, + // headings, code blocks) produce text in `textBetween`'s sense. + // Mixed-content blocks (lists, blockquotes) just contain other + // blocks, so their *inner* leaf blocks record boundaries instead. + const isLeafBlock = !!node.type.inlineContent; + + if (isLeafBlock) { + if (sawLeafBlock) { + // Boundary between the previous leaf-block and this one. The + // PM position of the boundary is `pos` — the opening token of + // the new leaf-block. textBetween emits the separator here. + segments.push({ + kind: "blockBoundary", + pmAt: pos, + textFrom: cursorText, + textTo: cursorText + 1, + }); + textParts.push("\n"); + cursorText += 1; + } + sawLeafBlock = true; + } + return true; // descend into block children + } + + // Other inline leaf nodes (none today) — skip silently. + return true; + }); + + const text = textParts.join(""); + + function mapPMToTextOffset(pm: number): number { + if (pm <= 0) return 0; + for (const seg of segments) { + if (seg.kind === "text") { + if (pm <= seg.pmTo) { + if (pm <= seg.pmFrom) return seg.textFrom; + return seg.textFrom + (pm - seg.pmFrom); + } + } else if (seg.kind === "hardBreak") { + if (pm <= seg.pmFrom) return seg.textFrom; + if (pm <= seg.pmTo) return seg.textTo; + } else { + // blockBoundary at pmAt: zero PM-width. + if (pm <= seg.pmAt) return seg.textFrom; + } + } + return text.length; + } + + function mapTextOffsetToPM(offset: number): number { + if (offset <= 0) { + const first = segments.find( + (s) => s.kind === "text" || s.kind === "hardBreak", + ); + if (first) return first.pmFrom; + // No content nodes: position 1 (inside the first block) if any, else 0. + return doc.content.size > 0 ? 1 : 0; + } + // Iterate segments; an offset that falls *exactly* at the right edge + // of a separator segment (hardBreak / blockBoundary) is interpreted + // as "start of the next content node" and so falls through to be + // claimed by the next segment. + for (const seg of segments) { + if (seg.kind === "text") { + if (offset <= seg.textTo) { + return seg.pmFrom + (offset - seg.textFrom); + } + } else if (seg.kind === "hardBreak") { + if (offset < seg.textTo) { + // Anywhere before the right edge of the `\n` → before the break. + return seg.pmFrom; + } + // offset === seg.textTo → falls through; the next text segment + // (whose textFrom == this textTo) will claim it and return pmTo. + } else { + // blockBoundary — zero PM-width. + // offset < textTo → "end of previous block" → pmAt + // offset === textTo → "start of next block" → falls through; the + // next content segment returns its pmFrom + // (= pmAt + 1). + if (offset < seg.textTo) return seg.pmAt; + } + } + // Beyond all content → end-of-doc text position. + const last = segments[segments.length - 1]; + if (!last) return doc.content.size > 0 ? 1 : 0; + if (last.kind === "text" || last.kind === "hardBreak") return last.pmTo; + return last.pmAt; + } + + return { text, mapPMToTextOffset, mapTextOffsetToPM }; +} diff --git a/desktop/src/features/messages/lib/useChannelLinks.ts b/desktop/src/features/messages/lib/useChannelLinks.ts index 7ecb43f07..c33888863 100644 --- a/desktop/src/features/messages/lib/useChannelLinks.ts +++ b/desktop/src/features/messages/lib/useChannelLinks.ts @@ -2,6 +2,7 @@ import * as React from "react"; import { useChannelNavigation } from "@/shared/context/ChannelNavigationContext"; import { detectPrefixQuery } from "@/shared/lib/detectPrefixQuery"; +import type { AutocompleteEdit } from "./useRichTextEditor"; export type ChannelSuggestion = { id: string; @@ -74,27 +75,23 @@ export function useChannelLinks() { const isChannelOpen = channelQuery !== null && channelSuggestions.length > 0; const insertChannel = React.useCallback( - ( - suggestion: ChannelSuggestion, - content: string, - selectionEnd: number, - ): { nextContent: string; nextCursor: number } => { + (suggestion: ChannelSuggestion, selectionEnd: number): AutocompleteEdit => { // Cancel any pending debounced detection — user already selected if (debounceTimerRef.current !== null) { clearTimeout(debounceTimerRef.current); debounceTimerRef.current = null; } - const before = content.slice(0, channelStartIndex); - const after = content.slice(selectionEnd); - const inserted = `#${suggestion.name} `; - const nextContent = `${before}${inserted}${after}`; - const nextCursor = before.length + inserted.length; + const insertText = `#${suggestion.name} `; setChannelQuery(null); setChannelSelectedIndex(0); - return { nextContent, nextCursor }; + return { + replaceFromOffset: channelStartIndex, + replaceToOffset: selectionEnd, + insertText, + }; }, [channelStartIndex], ); diff --git a/desktop/src/features/messages/lib/useEmojiAutocomplete.ts b/desktop/src/features/messages/lib/useEmojiAutocomplete.ts index d655911d2..beedd1f34 100644 --- a/desktop/src/features/messages/lib/useEmojiAutocomplete.ts +++ b/desktop/src/features/messages/lib/useEmojiAutocomplete.ts @@ -3,6 +3,8 @@ import * as React from "react"; import { init, SearchIndex } from "emoji-mart"; import data from "@emoji-mart/data"; +import type { AutocompleteEdit } from "./useRichTextEditor"; + export type EmojiSuggestion = { id: string; name: string; @@ -101,26 +103,22 @@ export function useEmojiAutocomplete() { const isEmojiAutocompleteOpen = emojiQuery !== null && suggestions.length > 0; const insertEmoji = React.useCallback( - ( - suggestion: EmojiSuggestion, - content: string, - selectionEnd: number, - ): { nextContent: string; nextCursor: number } => { + (suggestion: EmojiSuggestion, selectionEnd: number): AutocompleteEdit => { if (debounceTimerRef.current !== null) { clearTimeout(debounceTimerRef.current); debounceTimerRef.current = null; } - const before = content.slice(0, emojiStartIndex); - const after = content.slice(selectionEnd); - const inserted = `${suggestion.native} `; - const nextContent = `${before}${inserted}${after}`; - const nextCursor = before.length + inserted.length; + const insertText = `${suggestion.native} `; setEmojiQuery(null); setEmojiSelectedIndex(0); - return { nextContent, nextCursor }; + return { + replaceFromOffset: emojiStartIndex, + replaceToOffset: selectionEnd, + insertText, + }; }, [emojiStartIndex], ); diff --git a/desktop/src/features/messages/lib/useMentions.ts b/desktop/src/features/messages/lib/useMentions.ts index 819a8dee3..d09eec76c 100644 --- a/desktop/src/features/messages/lib/useMentions.ts +++ b/desktop/src/features/messages/lib/useMentions.ts @@ -6,6 +6,7 @@ import { } from "@/features/agents/hooks"; import { useChannelMembersQuery } from "@/features/channels/hooks"; import type { MentionSuggestion } from "@/features/messages/ui/MentionAutocomplete"; +import type { AutocompleteEdit } from "./useRichTextEditor"; import type { ChannelMember } from "@/shared/api/types"; import type { UserProfileLookup } from "@/features/profile/lib/identity"; import { detectPrefixQuery } from "@/shared/lib/detectPrefixQuery"; @@ -169,11 +170,7 @@ export function useMentions( const isMentionOpen = mentionQuery !== null && suggestions.length > 0; const insertMention = React.useCallback( - ( - suggestion: MentionSuggestion, - content: string, - selectionEnd: number, - ): { nextContent: string; nextCursor: number } => { + (suggestion: MentionSuggestion, selectionEnd: number): AutocompleteEdit => { // Cancel any pending debounced detection — user already selected if (debounceTimerRef.current !== null) { clearTimeout(debounceTimerRef.current); @@ -181,11 +178,7 @@ export function useMentions( } const displayName = suggestion.displayName; - const before = content.slice(0, mentionStartIndex); - const after = content.slice(selectionEnd); - const inserted = `@${displayName} `; - const nextContent = `${before}${inserted}${after}`; - const nextCursor = before.length + inserted.length; + const insertText = `@${displayName} `; const mentions = mentionMapRef.current; mentions.set(displayName, suggestion.pubkey); @@ -193,7 +186,11 @@ export function useMentions( setMentionQuery(null); setMentionSelectedIndex(0); - return { nextContent, nextCursor }; + return { + replaceFromOffset: mentionStartIndex, + replaceToOffset: selectionEnd, + insertText, + }; }, [mentionStartIndex], ); diff --git a/desktop/src/features/messages/lib/useRichTextEditor.test.mjs b/desktop/src/features/messages/lib/useRichTextEditor.test.mjs deleted file mode 100644 index ce53969e1..000000000 --- a/desktop/src/features/messages/lib/useRichTextEditor.test.mjs +++ /dev/null @@ -1,126 +0,0 @@ -import assert from "node:assert/strict"; -import test from "node:test"; - -/** - * Pure extraction of the ProseMirror → plain-text cursor mapping logic - * from getTextAndCursor in useRichTextEditor.ts. - * - * Takes a list of "visited nodes" (as the descendants callback would see them) - * and a ProseMirror anchor position, returns the plain-text offset. - */ -function mapAnchorToPlainText(nodes, anchor) { - let offset = 0; - let found = false; - for (const { isText, isBlock, pos, nodeSize } of nodes) { - if (found) break; - if (isText) { - const nodeEnd = pos + nodeSize; - if (anchor <= nodeEnd) { - offset += anchor - pos; - found = true; - break; - } - offset += nodeSize; - } else if (isBlock && pos > 0) { - offset += 1; - } - } - return found ? offset : -1; // -1 means "fell through" -} - -// ── Single paragraph ────────────────────────────────────────────────── - -test("cursor at start of single paragraph", () => { - // doc > paragraph(pos=0) > text "hello"(pos=1, size=5) - const nodes = [ - { isText: false, isBlock: true, pos: 0, nodeSize: 7 }, - { isText: true, isBlock: false, pos: 1, nodeSize: 5 }, - ]; - // Anchor at pos=1 → plain-text offset 0 - assert.equal(mapAnchorToPlainText(nodes, 1), 0); -}); - -test("cursor at end of single paragraph", () => { - const nodes = [ - { isText: false, isBlock: true, pos: 0, nodeSize: 7 }, - { isText: true, isBlock: false, pos: 1, nodeSize: 5 }, - ]; - // Anchor at pos=6 → plain-text offset 5 - assert.equal(mapAnchorToPlainText(nodes, 6), 5); -}); - -test("cursor mid-word in single paragraph", () => { - const nodes = [ - { isText: false, isBlock: true, pos: 0, nodeSize: 7 }, - { isText: true, isBlock: false, pos: 1, nodeSize: 5 }, - ]; - // Anchor at pos=3 → plain-text offset 2 (after "he") - assert.equal(mapAnchorToPlainText(nodes, 3), 2); -}); - -// ── Two paragraphs (the bug scenario) ───────────────────────────────── -// doc structure: doc > p1("hello") > p2("world") -// ProseMirror positions: doc=0, p1=0, "hello"=1..5, /p1=6, p2=7, "world"=8..12, /p2=13 -// textContent = "hello\nworld" (11 chars) - -test("cursor in second paragraph accounts for block boundary newline", () => { - const nodes = [ - { isText: false, isBlock: true, pos: 0, nodeSize: 7 }, // p1 - { isText: true, isBlock: false, pos: 1, nodeSize: 5 }, // "hello" - { isText: false, isBlock: true, pos: 7, nodeSize: 7 }, // p2 (pos > 0 → newline) - { isText: true, isBlock: false, pos: 8, nodeSize: 5 }, // "world" - ]; - // Anchor at pos=8 → start of "world" → plain-text offset 6 ("hello\n" = 6 chars) - assert.equal(mapAnchorToPlainText(nodes, 8), 6); -}); - -test("cursor mid-word in second paragraph", () => { - const nodes = [ - { isText: false, isBlock: true, pos: 0, nodeSize: 7 }, - { isText: true, isBlock: false, pos: 1, nodeSize: 5 }, - { isText: false, isBlock: true, pos: 7, nodeSize: 7 }, - { isText: true, isBlock: false, pos: 8, nodeSize: 5 }, - ]; - // Anchor at pos=10 → "wo|rld" → plain-text offset 8 ("hello\nwo" = 8 chars) - assert.equal(mapAnchorToPlainText(nodes, 10), 8); -}); - -// ── Three paragraphs (cumulative drift) ─────────────────────────────── -// "aaa\nbbb\nccc" — without the fix, offset would drift by 1 per boundary - -test("cursor in third paragraph accounts for two block boundaries", () => { - const nodes = [ - { isText: false, isBlock: true, pos: 0, nodeSize: 5 }, // p1 - { isText: true, isBlock: false, pos: 1, nodeSize: 3 }, // "aaa" - { isText: false, isBlock: true, pos: 5, nodeSize: 5 }, // p2 - { isText: true, isBlock: false, pos: 6, nodeSize: 3 }, // "bbb" - { isText: false, isBlock: true, pos: 10, nodeSize: 5 }, // p3 - { isText: true, isBlock: false, pos: 11, nodeSize: 3 }, // "ccc" - ]; - // Anchor at pos=11 → start of "ccc" → plain-text offset 8 ("aaa\nbbb\n" = 8 chars) - assert.equal(mapAnchorToPlainText(nodes, 11), 8); -}); - -test("cursor at end of third paragraph", () => { - const nodes = [ - { isText: false, isBlock: true, pos: 0, nodeSize: 5 }, - { isText: true, isBlock: false, pos: 1, nodeSize: 3 }, - { isText: false, isBlock: true, pos: 5, nodeSize: 5 }, - { isText: true, isBlock: false, pos: 6, nodeSize: 3 }, - { isText: false, isBlock: true, pos: 10, nodeSize: 5 }, - { isText: true, isBlock: false, pos: 11, nodeSize: 3 }, - ]; - // Anchor at pos=14 → end of "ccc" → plain-text offset 11 ("aaa\nbbb\nccc" = 11 chars) - assert.equal(mapAnchorToPlainText(nodes, 14), 11); -}); - -// ── First paragraph is unaffected (pos === 0, no newline) ───────────── - -test("first block boundary at pos 0 does not add newline", () => { - const nodes = [ - { isText: false, isBlock: true, pos: 0, nodeSize: 7 }, - { isText: true, isBlock: false, pos: 1, nodeSize: 5 }, - ]; - // Anchor at pos=4 → plain-text offset 3 (no extra newline for first block) - assert.equal(mapAnchorToPlainText(nodes, 4), 3); -}); diff --git a/desktop/src/features/messages/lib/useRichTextEditor.ts b/desktop/src/features/messages/lib/useRichTextEditor.ts index d04720e30..ed6c4d215 100644 --- a/desktop/src/features/messages/lib/useRichTextEditor.ts +++ b/desktop/src/features/messages/lib/useRichTextEditor.ts @@ -14,6 +14,18 @@ import { MentionHighlightExtension, mentionHighlightKey, } from "./mentionHighlightExtension"; +import { buildPlainTextProjection } from "./plainTextProjection"; + +/** + * Plain-text edit descriptor returned by autocomplete hooks + * (mentions / channel links / emoji). Offsets are in plain-text space — + * see `buildPlainTextProjection`. + */ +export type AutocompleteEdit = { + replaceFromOffset: number; + replaceToOffset: number; + insertText: string; +}; export type RichTextEditorOptions = { placeholder?: string; @@ -262,7 +274,11 @@ export function useRichTextEditor({ }, onUpdate: ({ editor: ed }) => { const markdown = getMarkdownFromEditor(ed); - const text = ed.state.doc.textContent; + // Use the same plain-text projection that `getPlainTextAndCursor` + // uses, so autocomplete detection sees the *same* string the + // cursor offset is mapped against. `state.doc.textContent` would + // diverge by 1 per hard-break / block boundary. + const text = buildPlainTextProjection(ed.state.doc).text; onUpdateRef.current?.({ markdown, text }); }, }, @@ -332,86 +348,72 @@ export function useRichTextEditor({ }, [editor]); /** - * Replace editor content and append a trailing space that survives parsing. + * Plain-text view of the document plus the cursor position in + * plain-text offset space. Used by autocomplete detection (mentions, + * channel links, emoji) which is shaped like a textarea. + * + * The plain-text projection treats both `hardBreak` and inter-block + * boundaries as `\n` — matching `doc.textBetween(0, end, "\n", "\n")`. + * See `plainTextProjection.ts`. + */ + const getPlainTextAndCursor = React.useCallback((): { + text: string; + cursor: number; + } => { + if (!editor) return { text: "", cursor: 0 }; + const projection = buildPlainTextProjection(editor.state.doc); + const anchor = editor.state.selection.anchor; + return { + text: projection.text, + cursor: projection.mapPMToTextOffset(anchor), + }; + }, [editor]); + + /** + * Replace a plain-text range with literal text, in a single native + * ProseMirror transaction. + * + * `fromOffset` and `toOffset` are in plain-text-offset space (the + * same space as `getPlainTextAndCursor`). `text` is inserted verbatim + * — including any trailing space — without a markdown re-parse. * - * `setContent(markdown)` roundtrips through TipTap's markdown parser which - * strips trailing whitespace from text nodes. TipTap's `insertContent(" ")` - * also normalises it away. This method bypasses both by creating a raw - * ProseMirror text node and inserting it via a direct transaction — the - * only path that reliably preserves a literal trailing space. + * This replaces the old `setContentWithTrailingSpace` + full-doc + * markdown round-trip used by autocomplete: by going through + * `tr.insertText` we preserve active marks, hard breaks, list + * structure, undo history continuity, and any whitespace. * - * Used by mention and channel-link autocomplete insertion. + * Returns the new cursor PM position, mapped through `tr.mapping` so + * callers get a position that's valid after the transaction is + * applied. */ - const setContentWithTrailingSpace = React.useCallback( - (markdown: string) => { + const replacePlainTextRange = React.useCallback( + (fromOffset: number, toOffset: number, text: string) => { if (!editor) return; - editor.commands.setContent(markdown); - // Insert a literal space via a raw ProseMirror transaction so it - // bypasses TipTap's content parser which strips trailing whitespace. - const { tr, schema, doc } = editor.state; - const endPos = doc.content.size - 1; // before the closing node token - const spaceNode = schema.text(" "); - tr.insert(endPos, spaceNode); - // Place cursor after the inserted space. - const cursorPos = endPos + spaceNode.nodeSize; - tr.setSelection(TextSelection.create(tr.doc, cursorPos)); + const projection = buildPlainTextProjection(editor.state.doc); + const fromPM = projection.mapTextOffsetToPM(fromOffset); + const toPM = projection.mapTextOffsetToPM(toOffset); + + const tr = editor.state.tr.insertText(text, fromPM, toPM); + // Place cursor at the end of the inserted text, mapped through the + // transaction in case anything else shifted positions (e.g. mark + // normalisation). Robust even if `text` becomes non-pure in future. + const cursorPM = tr.mapping.map(fromPM + text.length); + tr.setSelection(TextSelection.create(tr.doc, cursorPM)); editor.view.dispatch(tr); editor.view.focus(); }, [editor], ); - /** - * Returns the plain-text content and an approximate cursor offset. - * Used to bridge the existing useMentions / useChannelLinks hooks which - * were designed for a plain