From 550aaeb83752d0d9f58a91486a399a54e07cc3cf Mon Sep 17 00:00:00 2001 From: Carlos Scheidegger Date: Mon, 6 Jul 2026 17:04:21 -0500 Subject: [PATCH 1/2] fix(q2-preview): Mod-Enter commit no longer drops selected content in rich editor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the rich-text (tiptap) block editor, committing with `Mod-Enter` while text was selected deleted the selected text. Most visible path: click a block → select-all → bold → commit → the block was written to disk EMPTY (a tight list item `- banana` became `-`; a paragraph was deleted entirely). Root cause: tiptap's HardBreak extension binds `Mod-Enter` → setHardBreak() — the same key RichTextEditor used to commit, but via a DOM keydown listener that ran AFTER ProseMirror's keymap plugins. So `Mod-Enter` (1) fired setHardBreak, replacing the current selection with a hard break (over a full selection: the text is deleted, leaving `paragraph[hardBreak]`), then (2) the DOM handler's preventDefault fired too late and committed the emptied doc. With a collapsed caret it only appended a harmless trailing hard break — which is why plain text edits looked fine but always carried a stray break. Not specific to `Plain`/list items and not introduced by the Plain-gate change (bd-7pxub583): it reproduces identically on a `Para`, in the shared, type-agnostic RichTextEditor path. Fix (A)+(B): - (B) editorConfig.ts: a shared `buildRichTextExtensions()` disables StarterKit's built-in HardBreak and re-adds one bound to `Shift-Enter` ONLY, so `Mod-Enter` never inserts a break. Adds `@tiptap/extension-hard-break` as a direct dep. - (A) RichTextEditor.tsx: Escape / `Mod-Enter` / `Enter` move into a high-priority tiptap keymap (`q2CommitKeymap`) that runs inside ProseMirror's keymap and wins deterministically; the racy DOM keydown listener is removed (the focusout-commit is kept). Preserved: Escape cancels, plain Enter is swallowed (no split), `Shift-Enter` is a hard break, dirty/stale guards. Tests: - richtext/editorConfig.test.ts (fast jsdom): real `Mod-Enter` over a full selection leaves the text intact; no trailing hardBreak at a collapsed caret; `Shift-Enter` still inserts one. Verified red with the fix reverted. - hub-client/e2e/q2-preview-richtext-bold-content-loss.spec.ts (Playwright, real keyboard): bolding a tight-list item and a paragraph now round-trip to `**text**` on disk with no content loss. Was red before the fix. preview-renderer: tsc clean, 487 unit + 517 integration green. hub-client build + 229 unit tests green. Strand: bd-hafs0qho Co-Authored-By: Claude Opus 4.8 (1M context) --- ...chtext-mod-enter-hardbreak-content-loss.md | 171 ++++++++++++++++++ ...preview-richtext-bold-content-loss.spec.ts | 152 ++++++++++++++++ package-lock.json | 1 + ts-packages/preview-renderer/package.json | 1 + .../q2-preview/richtext/RichTextEditor.tsx | 110 ++++++----- .../q2-preview/richtext/editorConfig.test.ts | 107 +++++++++++ .../src/q2-preview/richtext/editorConfig.ts | 59 ++++++ 7 files changed, 559 insertions(+), 42 deletions(-) create mode 100644 claude-notes/plans/2026-07-06-richtext-mod-enter-hardbreak-content-loss.md create mode 100644 hub-client/e2e/q2-preview-richtext-bold-content-loss.spec.ts create mode 100644 ts-packages/preview-renderer/src/q2-preview/richtext/editorConfig.test.ts create mode 100644 ts-packages/preview-renderer/src/q2-preview/richtext/editorConfig.ts diff --git a/claude-notes/plans/2026-07-06-richtext-mod-enter-hardbreak-content-loss.md b/claude-notes/plans/2026-07-06-richtext-mod-enter-hardbreak-content-loss.md new file mode 100644 index 000000000..7bafec20b --- /dev/null +++ b/claude-notes/plans/2026-07-06-richtext-mod-enter-hardbreak-content-loss.md @@ -0,0 +1,171 @@ +# Rich-text editor: `Mod-Enter` commit drops selected content (HardBreak collision) + +**Strand:** bd-hafs0qho (discovered-from bd-7pxub583; related to bd-sjb4pzx8) +**Status:** IN PROGRESS — user approved (A)+(B) on 2026-07-06 + +## Decision locked with user (2026-07-06) + +Do **both (A) and (B)**: +- **(A)** Move the `Mod-Enter` commit into tiptap's keymap so it wins the race + and suppresses `setHardBreak`; retire the DOM `addEventListener` commit path. +- **(B)** Disable HardBreak's `Mod-Enter` binding (keep `Shift-Enter`). + +Rationale (user): a hard break on `Mod-Enter` is never wanted here — the rich +editor is **not** intended for extensive multi-block editing; the preview's +plain-text editor and hub-client's source editor cover those cases. +**Date:** 2026-07-06 + +## Summary + +In the `format: q2-preview` rich-text (tiptap) block editor, committing with +`Mod-Enter` while text is **selected** deletes the selected text. The most +visible path: click a block → select-all (`Mod-a`) → bold (`Mod-b`) → commit +(`Mod-Enter`) → the block is written to disk **empty** (a tight list item +`- banana` becomes `-`; a paragraph is deleted entirely). + +It is **not** specific to `Plain` / list items and **not** introduced by +bd-7pxub583 — it reproduces identically on a `Para`, in the shared, +type-agnostic RichTextEditor path, and predates the Plain-gate change. + +## Reliable reproduction (done) + +`hub-client/e2e/q2-preview-richtext-bold-content-loss.spec.ts` (Playwright, +real CDP keyboard events — the only faithful vehicle; jsdom and MCP-browser +synthetic events were confounded). Two cases: a tight bullet-list item and a +paragraph. Each opens the rich editor, does `Mod-a` / `Mod-b` / `Mod-Enter` +(via Playwright `ControlOrMeta`, which maps to ProseMirror's `Mod`), and asserts +the committed qmd contains the **bolded** text. **Currently RED** — the file is +written with the item emptied: + +``` +## List + +* apple +* ← "banana" gone +* cherry +``` + +> Cross-platform note: use `ControlOrMeta` in the spec. Plain `Control` on macOS +> does NOT trigger ProseMirror's `Mod-b` (which is Meta on mac), so bold never +> fires and the bug is masked — the reason two earlier runs falsely "passed". + +## Root cause (confirmed) + +tiptap's **HardBreak extension binds `Mod-Enter` → `setHardBreak()`** (verified +in `@tiptap/extension-hard-break/dist/index.js`; it binds both `Mod-Enter` and +`Shift-Enter`). RichTextEditor *also* uses `Mod-Enter` for commit — but via a +**DOM `addEventListener('keydown', …)`** on `editor.view.dom` that is registered +*after* ProseMirror's keymap plugin. So on `Mod-Enter`: + +1. ProseMirror/tiptap's keymap runs first → `setHardBreak()` **replaces the + current selection** with a hardBreak node. After select-all + bold the whole + text is selected → the text is **deleted**, leaving `paragraph[hardBreak]`. +2. RichTextEditor's DOM handler runs next → `e.preventDefault()` (too late to + undo step 1) → `commit()` → serializes `paragraph[hardBreak]` → `docToMarkdown` + correctly yields `""` → an empty block is committed. + +Corroborating evidence: +- Instrumented commit (captured earlier) showed the committed doc was exactly + `{paragraph:[{hardBreak}]}`, md `""`. +- The **serializer** and **Rust `apply_node_edit`** are both verified correct in + isolation, and **pure tiptap `selectAll()+toggleBold()+serialize`** (no + `Mod-Enter`) is clean — the damage only appears when `Mod-Enter` is pressed. +- Simple text edits "worked" but the committed doc always carried a **trailing** + hardBreak (`[text, hardBreak]`) — the same `Mod-Enter`→hardBreak insertion, but + harmless at a collapsed cursor because it appends rather than replaces, and a + trailing hardBreak serializes to nothing. + +So the defect is general: **`Mod-Enter` inserts a hardBreak on every commit**; it +is silent at a collapsed caret and destructive over a selection. + +## Fix options (to discuss) + +The goal: `Mod-Enter` must commit **without** inserting a hardBreak, deterministically. + +- **(A) Move commit into tiptap's keymap (recommended).** Add a small tiptap + extension (or `editor`-level `addKeyboardShortcuts`) binding `Mod-Enter` to a + handler that fires the commit and returns `true`. Because it lives in + ProseMirror's keymap with higher priority than HardBreak, it wins the race and + suppresses `setHardBreak`. This also retires the fragile DOM `addEventListener` + commit path (the root of the race). Escape/plain-Enter handling can move the + same way for consistency. +- **(B) Disable HardBreak's `Mod-Enter` binding.** Configure the HardBreak + extension (via StarterKit) so only `Shift-Enter` inserts a hard break; then + `Mod-Enter` falls through to the existing DOM commit handler with nothing to + race. Smaller change, but leaves the DOM-listener commit path in place. +- **(C) Change the commit key.** Least desirable — `Mod-Enter` is the expected + "confirm" gesture; keep it. + +Leaning **(A)**, optionally plus **(B)** as belt-and-suspenders (a hard break on +`Mod-Enter` is never wanted in this single-block editor). Keep `Shift-Enter` as +the intentional hard-break key (the editor already treats `Shift+Enter` as a +hard break; plain `Enter` is swallowed). + +Either way, also confirm the **trailing-hardBreak-on-plain-commit** disappears +(no more `[text, hardBreak]` docs), so commits stop carrying a stray break. + +## Plan (TDD) + +### Phase 1 — Reproduction (DONE) +- [x] Playwright e2e repro (`q2-preview-richtext-bold-content-loss.spec.ts`), + red today; covers list-item **and** paragraph (shared-path proof). + +### Phase 2 — Root cause (DONE) +- [x] Identified the `Mod-Enter` ↔ HardBreak `Mod-Enter` collision + DOM-listener + race; corroborated by the captured `paragraph[hardBreak]` commit doc. + +### Phase 3 — Tests first +- [x] Keep the e2e repro as the end-to-end regression gate (RED before the fix). +- [x] **Fast editor-config unit test** (`richtext/editorConfig.test.ts`, jsdom): + builds the editor via the shared `buildRichTextExtensions()`, select-all, + dispatches a real `Mod-Enter` keydown, asserts the doc is NOT mutated to a + hardBreak (text intact). Also: collapsed-caret `Mod-Enter` leaves no + trailing hardBreak; `Shift-Enter` still inserts one. **Confirmed RED** with + the fix reverted (default HardBreak), green with it. + +### Phase 4 — Implement the fix (A)+(B) +- [x] **(B)** `richtext/editorConfig.ts`: shared static extension config; + disables StarterKit's built-in HardBreak and re-adds one bound to + `Shift-Enter` only. Added `@tiptap/extension-hard-break` as a direct dep. +- [x] **(A)** `RichTextEditor.tsx`: Esc/`Mod-Enter`/`Enter` moved into a + high-priority tiptap keymap extension (`q2CommitKeymap`) via a handlers + ref; the racy DOM `keydown` listener is removed (focusout-commit kept). +- [x] Preserved behavior: Escape cancels; plain Enter swallowed; `Shift-Enter` + is a hard break; dirty guard / stale-target guards unchanged. +- [x] tsc clean; full preview-renderer suites green (487 unit incl. the 3 new, + 517 integration); the editor-mount integration tests (p3-4-inline-breadcrumb, + RichTextEditor.caret) still pass. + +### Phase 5 — Verify +- [x] **e2e repro GREEN** (was red). Both cases now assert the *bolded* text is + present on disk: `**banana**` (list item) and `**Hello world paragraph.**` + (paragraph). This is the faithful end-to-end check — real hub + real + browser + real `Mod`-keyboard, asserting the Automerge/disk content. It + supersedes the earlier MCP-browser manual check (which was confounded by + synthetic-event hardBreak injection). +- [x] hub-client build (VITE_E2E) green; hub-client unit tests green (229). +- [ ] `cargo xtask verify` (hub leg) — run before push. +- [ ] hub-client changelog entry (two-commit workflow) since `hub-client/` is + touched (the e2e spec) and the fix is user-facing (data-loss fix in the + bundled editor). To add as part of the commit. + +## Status: implementation complete + verified; ready to commit + +Working tree = the fix + tests + plan only (build-artifact churn reverted). +Awaiting go-ahead to commit + open a PR (will run `cargo xtask verify` and add +the changelog entry as part of that). + +## Files likely in scope + +- `ts-packages/preview-renderer/src/q2-preview/richtext/RichTextEditor.tsx` — + the `useEditor` extension config + the keydown/commit handling (the fix). +- Possibly a new tiny extension module for the `Mod-Enter` keymap. +- `hub-client/e2e/q2-preview-richtext-bold-content-loss.spec.ts` — the repro + (already written; may add assertions). +- A new fast unit/integration test under `ts-packages/preview-renderer/…/richtext/`. + +## Non-goals + +- Structural list editing (Enter-to-split, Tab-to-nest) — separate (bd-sjb4pzx8 + Phase 1c). +- Any change to the serializer or the Rust commit path — both verified correct. diff --git a/hub-client/e2e/q2-preview-richtext-bold-content-loss.spec.ts b/hub-client/e2e/q2-preview-richtext-bold-content-loss.spec.ts new file mode 100644 index 000000000..115029b1b --- /dev/null +++ b/hub-client/e2e/q2-preview-richtext-bold-content-loss.spec.ts @@ -0,0 +1,152 @@ +/** + * bd-hafs0qho — reproduction: a rich-text BOLD commit on a tight-list item can + * DROP the item's content. + * + * Observed manually in `q2 preview --allow-edit`: clicking a tight bullet-list + * item opens the tiptap rich editor; select-all + bold + commit wrote an EMPTY + * item to disk (`- banana` → `-`). Instrumentation showed the committed + * ProseMirror doc had become `paragraph[hardBreak]` (text gone). The serializer + * and the Rust commit path are both verified correct in isolation, and the pure + * tiptap `selectAll()+toggleBold()+serialize` is ALSO clean — so the fault is in + * the real browser event/commit flow, which only a real-keyboard e2e can drive + * faithfully (synthetic DOM injection in jsdom/MCP-browser was confounded). + * + * This spec drives REAL keyboard events (Control+A, Control+B, Control+Enter) + * through the rich editor and asserts the item KEEPS its content. It FAILS while + * the bug is present (content lost) and passes once fixed. + * + * Harness mirrors q2-preview-inline-edit.spec.ts, with two changes: + * 1. `richText: true` in the seeded preferences (default-on in prod, but the + * e2e preference object otherwise omits it → reads as off), so the rich + * editor opens instead of the textarea. + * 2. Activation waits for the ProseMirror surface (`.ProseMirror`), not a + * textarea, and asserts it is the rich editor. + */ + +import { test, expect, type Page, type FrameLocator } from '@playwright/test'; +import type {} from './helpers/testHooks'; +import { + bootstrapProjectSet, + createProjectOnServer, + seedProjectInBrowser, + getServerUrl, +} from './helpers/projectFactory'; +import { waitForPreviewRender } from './helpers/previewExtraction'; + +async function openFileWithRichText( + page: Page, + serverUrl: string, + docId: string, + filename: string, +): Promise { + // richText ON + nesting cursor ON (the product default): clicking a tight + // list item resolves the INNER Plain (rich editor), not the whole
    + // (which, being a BulletList, would fall back to the textarea). + await page.addInitScript(() => { + localStorage.setItem( + 'quarto-hub:preferences', + JSON.stringify({ + version: 1, + scrollSyncEnabled: true, + errorOverlayCollapsed: true, + colorScheme: 'auto', + unlockNestingCursor: true, + richText: true, + }), + ); + }); + await bootstrapProjectSet(page, serverUrl); + const localId = await seedProjectInBrowser(page, docId, serverUrl); + await page.goto(`/#/p/${localId}/file/${filename}`); + await waitForPreviewRender(page, { kind: 'q2-preview', timeout: 30000 }); + const iframe = page.frameLocator('iframe[src*="q2-preview.html"]'); + await iframe.locator('[data-block-pool-id]').first().waitFor({ timeout: 15_000 }); + return iframe; +} + +async function assertAutomerge( + page: Page, + filename: string, + { contains = [], lacks = [] }: { contains?: string[]; lacks?: string[] }, +): Promise { + await expect(async () => { + const text = await page.evaluate(async f => { + await window.__quartoTestReady; + return window.__quartoTest!.wasmRenderer.getFileContent(f) as string | null; + }, filename); + expect(text).not.toBeNull(); + for (const s of contains) expect(text).toContain(s); + for (const s of lacks) expect(text).not.toContain(s); + }).toPass({ timeout: 10000 }); +} + +test.describe('bd-hafs0qho — rich-text bold commit content loss', () => { + test.setTimeout(120000); + + test.beforeEach(async ({ page }, testInfo) => { + if (testInfo.workerIndex > 0) await page.waitForTimeout(1000); + }); + + test('bolding a tight bullet-list item preserves its content', async ({ page }) => { + const serverUrl = getServerUrl(); + const QMD = + '---\nformat: q2-preview\n---\n\n## List\n\n- apple\n- banana\n- cherry\n'; + const docId = await createProjectOnServer(serverUrl, [ + { path: '_quarto.yml', content: 'project:\n type: default\n', contentType: 'text' }, + { path: 'lists.qmd', content: QMD, contentType: 'text' }, + ]); + + const iframe = await openFileWithRichText(page, serverUrl, docId, 'lists.qmd'); + await expect(iframe.locator('text=banana')).toBeVisible(); + + // The tight-list
  • borrows the leading Plain's pool-id; clicking it + // targets the Plain. Open the rich editor (ProseMirror), not a textarea. + const li = iframe.locator('li[data-block-pool-id]', { hasText: 'banana' }).first(); + await li.click(); + const pm = iframe.locator('.ProseMirror'); + await pm.waitFor({ timeout: 5000 }); + // Guard: this must be the rich editor, not the textarea fallback. + await expect(iframe.locator('textarea')).toHaveCount(0); + await expect(pm).toContainText('banana'); + + // Real keyboard: select all, bold, commit. + await pm.press('ControlOrMeta+a'); + await pm.press('ControlOrMeta+b'); + await pm.press('ControlOrMeta+Enter'); + + // The item must still contain its text, now bolded. The reported bug + // dropped it, yielding an empty bullet. + await assertAutomerge(page, 'lists.qmd', { + contains: ['**banana**', 'apple', 'cherry'], + }); + }); + + test('bolding a paragraph preserves its content (shared-path control)', async ({ page }) => { + // Control case on a Para (rich-editable before bd-7pxub583). If this + // ALSO loses content, it confirms the bug is in the shared, type-agnostic + // RichTextEditor path — not specific to Plain / list items. + const serverUrl = getServerUrl(); + const QMD = + '---\nformat: q2-preview\n---\n\n## Para\n\nHello world paragraph.\n'; + const docId = await createProjectOnServer(serverUrl, [ + { path: '_quarto.yml', content: 'project:\n type: default\n', contentType: 'text' }, + { path: 'para.qmd', content: QMD, contentType: 'text' }, + ]); + + const iframe = await openFileWithRichText(page, serverUrl, docId, 'para.qmd'); + await expect(iframe.locator('text=Hello world paragraph.')).toBeVisible(); + + await iframe.locator('p[data-block-pool-id]', { hasText: 'Hello world' }).first().click(); + const pm = iframe.locator('.ProseMirror'); + await pm.waitFor({ timeout: 5000 }); + await expect(iframe.locator('textarea')).toHaveCount(0); + + await pm.press('ControlOrMeta+a'); + await pm.press('ControlOrMeta+b'); + await pm.press('ControlOrMeta+Enter'); + + await assertAutomerge(page, 'para.qmd', { + contains: ['**Hello world paragraph.**'], + }); + }); +}); diff --git a/package-lock.json b/package-lock.json index 24800a406..56ae88a8f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -13391,6 +13391,7 @@ "@quarto/quarto-automerge-schema": "*", "@revealjs/react": "0.2.0", "@tiptap/core": "^3.27.1", + "@tiptap/extension-hard-break": "^3.27.1", "@tiptap/extension-subscript": "^3.27.1", "@tiptap/extension-superscript": "^3.27.1", "@tiptap/pm": "^3.27.1", diff --git a/ts-packages/preview-renderer/package.json b/ts-packages/preview-renderer/package.json index 1d2b10947..3acfe33df 100644 --- a/ts-packages/preview-renderer/package.json +++ b/ts-packages/preview-renderer/package.json @@ -68,6 +68,7 @@ "@quarto/quarto-automerge-schema": "*", "@revealjs/react": "0.2.0", "@tiptap/core": "^3.27.1", + "@tiptap/extension-hard-break": "^3.27.1", "@tiptap/extension-subscript": "^3.27.1", "@tiptap/extension-superscript": "^3.27.1", "@tiptap/pm": "^3.27.1", diff --git a/ts-packages/preview-renderer/src/q2-preview/richtext/RichTextEditor.tsx b/ts-packages/preview-renderer/src/q2-preview/richtext/RichTextEditor.tsx index 76d62f110..4a40bfd9c 100644 --- a/ts-packages/preview-renderer/src/q2-preview/richtext/RichTextEditor.tsx +++ b/ts-packages/preview-renderer/src/q2-preview/richtext/RichTextEditor.tsx @@ -14,9 +14,7 @@ import { useEffect, useMemo, useRef } from 'react'; import { useEditor, EditorContent } from '@tiptap/react'; -import StarterKit from '@tiptap/starter-kit'; -import Subscript from '@tiptap/extension-subscript'; -import Superscript from '@tiptap/extension-superscript'; +import { Extension } from '@tiptap/core'; import type { Editor } from '@tiptap/core'; import type { Node as PMNode } from '@tiptap/pm/model'; import type { PreviewContextValue, ResolvedSource } from './../PreviewContext'; @@ -24,7 +22,7 @@ import { buildNestingCommitDestination, buildAncestorPath } from './../nestingNa import { BreadcrumbCrumbs } from './../BreadcrumbCrumbs'; import { astToDoc } from './astToProseMirror'; import { docToMarkdown } from './serializer'; -import { Chip } from './chipExtension'; +import { buildRichTextExtensions } from './editorConfig'; import { RichTextToolbar } from './RichTextToolbar'; import type { AstNode, PoolEntry } from './ast'; import { ensureRichTextStyles } from './styles'; @@ -70,27 +68,55 @@ export function RichTextEditor({ // Latch so a commit fires at most once (blur can follow a key-commit). const committedRef = useRef(false); - const editor = useEditor({ - extensions: [ - StarterKit.configure({ - // 1a: paragraphs + inline marks. 1b: + headings. (1c: lists/quotes/code.) - heading: { levels: [1, 2, 3, 4, 5, 6] }, - blockquote: false, - bulletList: false, - orderedList: false, - listItem: false, - codeBlock: false, - horizontalRule: false, - // No phantom trailing paragraph: a single-heading (or any non-paragraph) - // block would otherwise get an empty trailing

    — extra vertical space - // in the editor AND a stray blank block on commit. - trailingNode: false, - link: { openOnClick: false }, + // Keyboard handlers for the commit keymap (bd-hafs0qho). Populated each render + // (below, after `commit`/`cancel` are defined) so the keymap always calls the + // current closures without going stale. + const keymapHandlersRef = useRef<{ + escape: () => void; + modEnter: () => void; + enter: () => void; + } | null>(null); + + // Commit/cancel/plain-Enter live in tiptap's keymap — NOT a DOM keydown + // listener (bd-hafs0qho). A DOM listener runs AFTER ProseMirror's keymap + // plugins, so tiptap's HardBreak `Mod-Enter` (now disabled in editorConfig, + // belt-and-suspenders) or any other binding would win the race and mutate the + // doc before we could `preventDefault`. A high-priority keymap extension runs + // first and returns `true`, so `Mod-Enter` deterministically commits with no + // hard break inserted, `Escape` cancels, and plain `Enter` is swallowed + // (no structural split; `Shift-Enter` still inserts a hard break). + const commitKeymap = useMemo( + () => + Extension.create({ + name: 'q2CommitKeymap', + priority: 1000, + addKeyboardShortcuts() { + return { + Escape: () => { + keymapHandlersRef.current?.escape(); + return true; + }, + 'Mod-Enter': () => { + keymapHandlersRef.current?.modEnter(); + return true; + }, + Enter: () => { + keymapHandlersRef.current?.enter(); + return true; + }, + }; + }, }), - Subscript, - Superscript, - Chip, - ], + [], + ); + + const extensions = useMemo( + () => [...buildRichTextExtensions(), commitKeymap], + [commitKeymap], + ); + + const editor = useEditor({ + extensions, content: seedJSON, // Initial caret placement is owned entirely by the mount effect below (click // position for a mouse-open, else end-of-block) — see bd-q9lyghv2. tiptap's @@ -155,6 +181,21 @@ export function RichTextEditor({ ctx.setEditTarget?.(null); }; + // Keep the commit keymap's handlers pointed at the current closures. Assigned + // during render (idempotent, no external effect) so a keypress after any + // render calls fresh `commit`/`cancel` — no stale-closure window. + keymapHandlersRef.current = { + escape: cancel, + modEnter: () => { + if (!editor) return; + ctx.requestFocusRestore?.(resolved.sourceEntry.r[0]); + commit(editor); + }, + // Swallow plain Enter — no structural split in this single-block editor. + // (Shift-Enter is a hard break, handled by the HardBreak extension.) + enter: () => {}, + }; + // The whole edit box (editor + toolbar + link input) is one focus scope: we // commit only when focus leaves it entirely, so focusing the toolbar's link // input keeps the session open. @@ -188,25 +229,12 @@ export function RichTextEditor({ // eslint-disable-next-line react-hooks/exhaustive-deps }, [editor]); - // Keyboard: Esc cancels; Mod-Enter commits; plain Enter is swallowed (no split). - // Commit: a focusout from the edit box (focus moved outside it) commits. + // Esc/Mod-Enter/plain-Enter are handled by the commit keymap (bd-hafs0qho); + // see `commitKeymap` above. This effect only wires the focusout commit: a + // focus move OUT of the edit box (not into the toolbar/link input) commits. useEffect(() => { if (!editor) return; - const dom = editor.view.dom as HTMLElement; const root = rootRef.current; - const onKeyDown = (e: KeyboardEvent) => { - if (e.key === 'Escape') { - e.preventDefault(); - cancel(); - } else if (e.key === 'Enter' && (e.metaKey || e.ctrlKey)) { - e.preventDefault(); - ctx.requestFocusRestore?.(resolved.sourceEntry.r[0]); - commit(editor); - } else if (e.key === 'Enter' && !e.shiftKey) { - // 1a/1b: no structural split. Swallow plain Enter (Shift+Enter = hard break). - e.preventDefault(); - } - }; const onFocusOut = (e: FocusEvent) => { // A surface swap fires this as the editor unmounts — not a commit. if (ctx.editorModeSwitchRef?.current) return; @@ -216,10 +244,8 @@ export function RichTextEditor({ ctx.requestFocusRestore?.(resolved.sourceEntry.r[0]); commit(editor); }; - dom.addEventListener('keydown', onKeyDown); root?.addEventListener('focusout', onFocusOut); return () => { - dom.removeEventListener('keydown', onKeyDown); root?.removeEventListener('focusout', onFocusOut); }; // eslint-disable-next-line react-hooks/exhaustive-deps diff --git a/ts-packages/preview-renderer/src/q2-preview/richtext/editorConfig.test.ts b/ts-packages/preview-renderer/src/q2-preview/richtext/editorConfig.test.ts new file mode 100644 index 000000000..d06d3f53e --- /dev/null +++ b/ts-packages/preview-renderer/src/q2-preview/richtext/editorConfig.test.ts @@ -0,0 +1,107 @@ +// @vitest-environment jsdom +// +// bd-hafs0qho — `Mod-Enter` (the commit gesture) must NOT insert a hard break. +// +// tiptap's built-in HardBreak binds `Mod-Enter` → setHardBreak(). With a +// selection active (e.g. after select-all + bold), that REPLACES the selected +// text with a hard break; the editor then commits an empty block. This guards +// the fix: the shared config re-binds HardBreak to `Shift-Enter` only, so +// pressing `Mod-Enter` over a full selection leaves the text intact. + +import { describe, it, expect, afterEach } from 'vitest'; +import { Editor } from '@tiptap/core'; +import { buildRichTextExtensions } from './editorConfig'; + +// Mirror prosemirror-keymap's `Mod` resolution: Meta on mac, Ctrl elsewhere. +const IS_MAC = + typeof navigator !== 'undefined' && /Mac|iP(hone|[oa]d)/.test(navigator.platform); + +function makeEditor(text: string): Editor { + const el = document.createElement('div'); + document.body.appendChild(el); + return new Editor({ + element: el, + extensions: buildRichTextExtensions(), + content: { + type: 'doc', + content: [{ type: 'paragraph', content: [{ type: 'text', text }] }], + }, + autofocus: false, + enableInputRules: false, + enablePasteRules: false, + }); +} + +function pressModEnter(editor: Editor): void { + const ev = new KeyboardEvent('keydown', { + key: 'Enter', + code: 'Enter', + bubbles: true, + cancelable: true, + metaKey: IS_MAC, + ctrlKey: !IS_MAC, + }); + editor.view.dom.dispatchEvent(ev); +} + +function pressShiftEnter(editor: Editor): void { + const ev = new KeyboardEvent('keydown', { + key: 'Enter', + code: 'Enter', + bubbles: true, + cancelable: true, + shiftKey: true, + }); + editor.view.dom.dispatchEvent(ev); +} + +let editors: Editor[] = []; +function track(e: Editor): Editor { + editors.push(e); + return e; +} +afterEach(() => { + editors.forEach(e => e.destroy()); + editors = []; + document.body.innerHTML = ''; +}); + +function hasHardBreak(editor: Editor): boolean { + let found = false; + editor.state.doc.descendants(node => { + if (node.type.name === 'hardBreak') found = true; + }); + return found; +} + +describe('bd-hafs0qho — Mod-Enter must not insert a hard break', () => { + it('Mod-Enter over a full selection leaves the text intact (no hard break)', () => { + const editor = track(makeEditor('banana')); + editor.commands.selectAll(); + + pressModEnter(editor); + + expect(editor.state.doc.textContent).toBe('banana'); + expect(hasHardBreak(editor), 'Mod-Enter must not insert a hardBreak').toBe(false); + }); + + it('Mod-Enter at a collapsed caret does not append a trailing hard break', () => { + const editor = track(makeEditor('banana')); + editor.commands.focus('end'); + + pressModEnter(editor); + + expect(editor.state.doc.textContent).toBe('banana'); + expect(hasHardBreak(editor)).toBe(false); + }); + + it('Shift-Enter still inserts a hard break (intentional line break preserved)', () => { + const editor = track(makeEditor('banana')); + editor.commands.focus('end'); + + pressShiftEnter(editor); + + expect(hasHardBreak(editor), 'Shift-Enter must still insert a hardBreak').toBe(true); + expect(editor.state.doc.textContent).toBe('banana'); + }); +}); diff --git a/ts-packages/preview-renderer/src/q2-preview/richtext/editorConfig.ts b/ts-packages/preview-renderer/src/q2-preview/richtext/editorConfig.ts new file mode 100644 index 000000000..f14c45c85 --- /dev/null +++ b/ts-packages/preview-renderer/src/q2-preview/richtext/editorConfig.ts @@ -0,0 +1,59 @@ +// Shared tiptap extension config for the q2-preview rich-text editor. +// +// Extracted so the production editor (RichTextEditor.tsx) and the tests exercise +// the EXACT same extension set — no drift. The component adds one more extension +// on top (the commit keymap, which needs React callbacks); everything here is +// static and component-independent. + +import StarterKit from '@tiptap/starter-kit'; +import Subscript from '@tiptap/extension-subscript'; +import Superscript from '@tiptap/extension-superscript'; +import HardBreak from '@tiptap/extension-hard-break'; +import type { AnyExtension } from '@tiptap/core'; +import { Chip } from './chipExtension'; + +/** + * The static extensions for the rich-text editor. + * + * Scope (1a/1b): paragraphs, headings, inline marks. Lists/quotes/code blocks + * are disabled (structural editing is a later phase); `trailingNode: false` + * avoids a phantom trailing paragraph. + * + * HardBreak (bd-hafs0qho): StarterKit's built-in HardBreak binds BOTH + * `Mod-Enter` and `Shift-Enter` to `setHardBreak()`. `Mod-Enter` is our commit + * gesture, and tiptap's keymap ran before the old DOM commit listener — so a + * commit over a selection replaced the selected text with a hard break and wrote + * an empty block. We disable the built-in HardBreak and re-add one bound to + * `Shift-Enter` ONLY, so `Mod-Enter` never inserts a break. (The commit keymap + * that binds `Mod-Enter` lives in RichTextEditor, since it needs React state.) + */ +export function buildRichTextExtensions(): AnyExtension[] { + return [ + StarterKit.configure({ + heading: { levels: [1, 2, 3, 4, 5, 6] }, + blockquote: false, + bulletList: false, + orderedList: false, + listItem: false, + codeBlock: false, + horizontalRule: false, + trailingNode: false, + link: { openOnClick: false }, + // Disable the built-in HardBreak (binds Mod-Enter + Shift-Enter); we + // re-add a Shift-Enter-only variant below. + hardBreak: false, + }), + HardBreak.extend({ + addKeyboardShortcuts() { + // Shift-Enter inserts a hard break; Mod-Enter deliberately does + // NOT (it is the commit gesture — see RichTextEditor). + return { + 'Shift-Enter': () => this.editor.commands.setHardBreak(), + }; + }, + }), + Subscript, + Superscript, + Chip, + ]; +} From 992f839987973a79b6dbf08947a700a896d3eb8f Mon Sep 17 00:00:00 2001 From: Carlos Scheidegger Date: Mon, 6 Jul 2026 17:04:41 -0500 Subject: [PATCH 2/2] docs(changelog): note the Mod-Enter rich-text content-loss fix (550aaeb8) Co-Authored-By: Claude Opus 4.8 (1M context) --- hub-client/changelog.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/hub-client/changelog.md b/hub-client/changelog.md index c6bbe052c..85d37c59a 100644 --- a/hub-client/changelog.md +++ b/hub-client/changelog.md @@ -15,6 +15,10 @@ be in reverse chronological order (latest first). --> +### 2026-07-06 + +- [`550aaeb8`](https://github.com/quarto-dev/q2/commits/550aaeb8): Fixed a rich-text editor bug where committing with Cmd/Ctrl+Enter while text was selected (for example, select-all then bold) could delete the block's content. + ### 2026-07-01 - [`0b13dbcb`](https://github.com/quarto-dev/q2/commits/0b13dbcb): The preview's code-execution controls are now a single status line — executor status, "showing executed output", and the Run/Re-run and Clear-results buttons share one bar instead of stacking as two.