35683 block editor 20 improvements and bug fixes v2

[rjvelazco]

Videos

Solution

fix.mov

Issue

issue.mov

This PR fixes: #35683

This PR fixes: #35683

[claude]

Claude finished @rjvelazco's task in 2m 43s —— View job

---

Review

Mostly solid. A handful of real concerns worth a second look.

1. indent becomes a stored attribute on every paragraph / heading / blockquote — JSON shape changes

indent.extension.ts:76-101 adds indent as a global attribute with default: 0. ProseMirror's Node.toJSON() serializes declared attrs whether or not they equal the default, so every paragraph/heading/blockquote in newly emitted JSON will gain "attrs": { "indent": 0, ... } even if the user never indented anything.

The extension's own docstring claims it "round-trips legacy content without losing the value" — but it's also actively rewriting the shape of all content authored in the new editor. Headless renderers / downstream consumers / snapshot tests that diff JSON will see new keys appear on every node. Worth confirming this is acceptable, or otherwise hide the default via a keepOnSplit: false / custom toJSON (or, ideally, omit when 0).

2. Indent on a blockquote indents the blockquote, not the inner paragraph

indent.extension.ts:39-55 — nodesBetween matches the outermost indent type and return false stops descent. With the caret inside <blockquote><p>…</p></blockquote>, pressing Tab indents the blockquote, never the inner paragraph. Users can't indent a paragraph inside a quote. Probably intentional given the goal, but it's a UX corner customers will hit. Mention this in the PR description so QA tests it explicitly.

3. NodeSelection is silently ignored

indent.extension.ts:41 only acts on TextSelection | AllSelection. The block-gutter handle creates a NodeSelection. Selecting a paragraph via the gutter and pressing Tab does nothing — and the toolbar button correctly disables because can().indent() returns false. Inconsistent with the gutter UX (the user explicitly selected the block to act on it). Either widen the guard to handle NodeSelection or leave a TODO.

4. addCommands returns a transaction even in dry-run — minor wastefulness, but check semantics

indent.extension.ts:113-128 always calls updateIndentLevel(state.tr, ...) and checks tr.docChanged. The comment justifies this, and the logic is correct (dispatch is only called when present). The traversal is O(selection) and runs on every transaction via EditorToolbarStore.connect calling can().indent() / can().outdent() — on huge docs with multi-paragraph selections this fires on every keystroke. Acceptable, just noting.

tr.doc?.nodeAt(pos) at line 29 — the nullable check is defensive and unnecessary; tr.doc is always defined.

5. withDocStats skips stamping when charCount() <= 0 — divergence from legacy

editor.component.ts:589-601 — comment says it matches the legacy editor, but the legacy editor stamped 0 values for empty docs. After a user types then deletes everything, the emitted JSON drops attrs.charCount/wordCount/readingTime entirely. Subscribers that read these keys unconditionally will need to handle "undefined" vs "0". If the goal is just "don't ship inflated zeros on a brand-new doc," gate on a "doc was never edited" flag, not on current count.

6. JSP fix is correct, but the surrounding code path is fragile

edit_field.jsp:288-290 — the fix is right and the comment correctly captures why (no valueChange fires until the user touches the editor, so the initial assignment is the only one happening on save). Two follow-ups worth raising:

• The JSON.parse at line 272 followed by JSON.stringify here is a round-trip that normalises whitespace/key order. If the saved JSON shape mattered byte-for-byte (it doesn't, but worth keeping in mind), this would mutate it.
• A regression test (Postman or otherwise) for "open contentlet with Block Editor field, save without touching the editor, reopen — value is still JSON, not [object Object]" would prevent recurrence. This was a data-corruption bug; it deserves a test even though the new-block-editor lib has no spec files today.

7. Toolbar indent() / outdent() have no path for code blocks

toolbar.component.ts:357-379 — when the cursor is inside a codeBlock, neither branch runs (it's not a list, not a table, not a paragraph/heading/blockquote). The button disables via canIndent, which is correct. Just confirming the design: code blocks are intentionally non-indentable. If that ever needs to change, the routing here will need a third branch.

8. No tests

213 lines added, six changed files, three meaningful behaviour changes (indent extension, doc-stats stamping, JSP corruption fix), zero tests. The new-block-editor lib has no .spec.ts files at all, so this isn't a regression — but the JSP fix in particular is a data-loss bug; a Postman regression test is cheap insurance.

---

Nits

• indent.extension.ts:5-12 — declare module '@tiptap/core' uses indent: as the command namespace. Matches name: 'indent', fine.
• editor.component.ts:122 doc-comment edit ("minus doc count attrs" → "") is now stale in the opposite direction: the new code does write doc count attrs. The deletion was correct, but the new behaviour deserves a sentence.
  · Branch