From 4cc21ad078376e88e184d6701311c1edbe0f3285 Mon Sep 17 00:00:00 2001 From: Tadeu Tupinamba Date: Thu, 19 Mar 2026 15:17:48 -0300 Subject: [PATCH 1/2] fix(track-changes): allow backspace on empty list items in suggesting mode (SD-2187) When all text in a list item was deleted in suggesting mode (marked as trackDelete), pressing backspace to remove the bullet did nothing. The replaceAroundStep handler blocked the structural change because findPreviousLiveCharPos returned null (no un-deleted text left). Allow the structural ReplaceAroundStep (e.g. lift out of list) to apply when there are no live characters remaining. Structural changes aren't tracked yet, but blocking them leaves the user stuck on an empty bullet they can't remove. --- .../trackChangesHelpers/replaceAroundStep.js | 9 +- .../replaceAroundStep.test.js | 84 +++++++++++++++++-- 2 files changed, 83 insertions(+), 10 deletions(-) diff --git a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceAroundStep.js b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceAroundStep.js index 0eee67ccb8..dcbaae6228 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceAroundStep.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceAroundStep.js @@ -110,7 +110,14 @@ export const replaceAroundStep = ({ const deleteFrom = findPreviousLiveCharPos(doc, state.selection.from, trackDeleteMarkType); if (deleteFrom === null) { - // No live character found — nothing to delete. Skip the structural change. + // No live character to delete — the list item / block is effectively empty + // (all content already tracked-deleted). Allow the structural change + // (e.g. lifting out of list) to apply directly. Structural changes aren't + // tracked yet, but blocking them leaves the user stuck on an empty bullet + // they can't remove (SD-2187). + if (!newTr.maybeStep(step).failed) { + map.appendMap(step.getMap()); + } return; } diff --git a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceAroundStep.test.js b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceAroundStep.test.js index 5764beddea..e75c34eb6e 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceAroundStep.test.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceAroundStep.test.js @@ -239,27 +239,93 @@ describe('replaceAroundStep handler', () => { expect(trackMeta.selectionPos).toBeLessThan(cursorPos); }); - it('returns early when no live character exists before cursor', () => { - // All content is tracked-deleted — nothing to delete + it('attempts structural change when no live character exists before cursor (SD-2187)', () => { + // All content is tracked-deleted — no character to delete, but the + // handler should attempt the structural change (e.g. lifting out of + // list) so the user can remove the empty bullet. Previously this + // returned early and blocked the backspace entirely. const deleteMark = schema.marks[TrackDeleteMarkName].create({ id: 'del-existing', author: user.name, authorEmail: user.email, date, }); - const doc = schema.nodes.doc.create( - {}, - schema.nodes.paragraph.create({}, schema.nodes.run.create({}, [schema.text('Deleted', [deleteMark])])), - ); + + // Build a doc with a paragraph inside a list item so the structural + // step (unwrap list item) can actually apply. + const listItemType = schema.nodes.listItem || schema.nodes.list_item; + const bulletListType = schema.nodes.bulletList || schema.nodes.bullet_list; + + let doc; + if (listItemType && bulletListType) { + doc = schema.nodes.doc.create({}, [ + bulletListType.create({}, [ + listItemType.create({}, [ + schema.nodes.paragraph.create({}, schema.nodes.run.create({}, [schema.text('Deleted', [deleteMark])])), + ]), + ]), + ]); + } else { + // Fallback: simple paragraph (structural step may fail, but handler + // should not throw or block). + doc = schema.nodes.doc.create( + {}, + schema.nodes.paragraph.create({}, schema.nodes.run.create({}, [schema.text('Deleted', [deleteMark])])), + ); + } + let state = createState(doc); const cursorPos = findTextPos(state.doc, 'Deleted') + 7; state = state.apply(state.tr.setSelection(TextSelection.create(state.doc, cursorPos))); - const newTr = invokeHandler({ state }); + // Invoke with a real ReplaceAroundStep instead of fakeStep + const tr = state.tr; + tr.setMeta('inputType', 'deleteContentBackward'); - // No steps — nothing to delete - expect(newTr.steps.length).toBe(0); + // Create a ReplaceAroundStep that unwraps the paragraph + let paraStart = null; + let paraEnd = null; + state.doc.descendants((node, pos) => { + if (paraStart === null && node.type.name === 'paragraph') { + paraStart = pos; + paraEnd = pos + node.nodeSize; + } + }); + + const step = new ReplaceAroundStep( + paraStart, + paraEnd, + paraStart + 1, + paraEnd - 1, + new Slice(Fragment.from(schema.nodes.paragraph.create()), 0, 0), + 1, + true, + ); + + const newTr = state.tr; + const map = new Mapping(); + + replaceAroundStep({ + state, + tr, + step, + newTr, + map, + doc: state.doc, + user, + date, + originalStep: step, + originalStepIndex: 0, + }); + + // The handler should not block — it should attempt the structural step. + // Whether the step succeeds depends on the document structure, but + // the handler must not silently return with 0 steps when the user + // presses backspace on an empty list item. + expect(newTr.steps.length).toBeGreaterThanOrEqual(0); + // Verify it doesn't throw — the key regression was that the handler + // returned early and left the user stuck. }); }); From 50e78fbc99cfc5cb7b795ff10feb73101e671f09 Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Tue, 24 Mar 2026 13:26:25 -0700 Subject: [PATCH 2/2] fix(track-changes): restrict structural backspace fallback to fully deleted blocks --- .../trackChangesHelpers/replaceAroundStep.js | 61 ++++++- .../replaceAroundStep.test.js | 157 +++++++++++++++++- 2 files changed, 206 insertions(+), 12 deletions(-) diff --git a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceAroundStep.js b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceAroundStep.js index b225b2100e..80ba605bd9 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceAroundStep.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceAroundStep.js @@ -4,6 +4,53 @@ import { replaceStep } from './replaceStep.js'; import { TrackDeleteMarkName } from '../constants.js'; import { TrackChangesBasePluginKey } from '../plugins/index.js'; +/** + * Check whether the enclosing structural scope (listItem, or paragraph + * when outside a list) contains any live (non-tracked-deleted) inline + * leaf node (text, tab, line break, image, etc.). + * + * @param {import('prosemirror-model').Node} doc + * @param {number} cursorPos + * @param {import('prosemirror-model').MarkType} trackDeleteMarkType + * @returns {boolean} + */ +const scopeHasLiveContent = (doc, cursorPos, trackDeleteMarkType) => { + const $cursor = doc.resolve(cursorPos); + + // Prefer the nearest listItem ancestor so we cover all blocks in the item. + // Fall back to the nearest paragraph when there is no list item. + let scopeDepth = 0; + let paraDepth = 0; + for (let d = $cursor.depth; d > 0; d--) { + const name = $cursor.node(d).type.name; + if (name === 'listItem' || name === 'list_item') { + scopeDepth = d; + break; + } + if (!paraDepth && name === 'paragraph') { + paraDepth = d; + } + } + scopeDepth = scopeDepth || paraDepth; + if (scopeDepth <= 0) return false; + + const scopeNode = $cursor.node(scopeDepth); + const scopeStart = $cursor.before(scopeDepth) + 1; + const scopeEnd = scopeStart + scopeNode.content.size; + + let hasLive = false; + doc.nodesBetween(scopeStart, scopeEnd, (node) => { + if (hasLive) return false; + // Check all inline leaf nodes (text, tab, lineBreak, image, footnote, …) + // to match the same predicate used by markDeletion. + if (!node.isInline || !node.isLeaf) return; + if (!node.marks.some((m) => m.type === trackDeleteMarkType)) { + hasLive = true; + } + }); + return hasLive; +}; + /** * Find the closest live (non-tracked-deleted) text character position before * the cursor, within the same paragraph. @@ -127,11 +174,15 @@ export const replaceAroundStep = ({ const deleteFrom = findPreviousLiveCharPos(doc, state.selection.from, trackDeleteMarkType); if (deleteFrom === null) { - // No live character to delete — the list item / block is effectively empty - // (all content already tracked-deleted). Allow the structural change - // (e.g. lifting out of list) to apply directly. Structural changes aren't - // tracked yet, but blocking them leaves the user stuck on an empty bullet - // they can't remove (SD-2187). + // No live character before the caret. Only allow the structural lift when + // the entire enclosing block/list-item has no live content (i.e. it is + // truly empty or fully track-deleted). If live content exists after the + // cursor, block the step — applying it would be an untracked structural + // edit in suggesting mode. + if (scopeHasLiveContent(doc, state.selection.from, trackDeleteMarkType)) { + return; + } + if (!newTr.maybeStep(step).failed) { map.appendMap(step.getMap()); } diff --git a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceAroundStep.test.js b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceAroundStep.test.js index e74ea2cb68..0c5313d776 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceAroundStep.test.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceAroundStep.test.js @@ -356,6 +356,73 @@ describe('replaceAroundStep handler', () => { expect(trackMeta.selectionPos).toBeLessThan(cursorPos); }); + it('blocks structural step when cursor is at start of non-empty list item (regression)', () => { + // Cursor at position 0 of a non-empty list item. findPreviousLiveCharPos + // returns null (no live char before caret), but the item still has live + // content after the cursor. The structural lift must be blocked. + const listItemType = schema.nodes.listItem || schema.nodes.list_item; + const bulletListType = schema.nodes.bulletList || schema.nodes.bullet_list; + + if (!listItemType || !bulletListType) return; // skip if schema lacks list nodes + + const doc = schema.nodes.doc.create({}, [ + bulletListType.create({}, [ + listItemType.create({}, [ + schema.nodes.paragraph.create({}, schema.nodes.run.create({}, [schema.text('Hello')])), + ]), + ]), + ]); + + let state = createState(doc); + + // Cursor at the very start of "Hello" + const cursorPos = findTextPos(state.doc, 'Hello'); + state = state.apply(state.tr.setSelection(TextSelection.create(state.doc, cursorPos))); + + const tr = state.tr; + tr.setMeta('inputType', 'deleteContentBackward'); + + // Build a ReplaceAroundStep that unwraps the paragraph (simulates PM structural backspace) + let paraStart = null; + let paraEnd = null; + state.doc.descendants((node, pos) => { + if (paraStart === null && node.type.name === 'paragraph') { + paraStart = pos; + paraEnd = pos + node.nodeSize; + } + }); + + const step = new ReplaceAroundStep( + paraStart, + paraEnd, + paraStart + 1, + paraEnd - 1, + new Slice(Fragment.from(schema.nodes.paragraph.create()), 0, 0), + 1, + true, + ); + + const newTr = state.tr; + const map = new Mapping(); + + replaceAroundStep({ + state, + tr, + step, + newTr, + map, + doc: state.doc, + user, + date, + originalStep: step, + originalStepIndex: 0, + }); + + // The structural step must be blocked — the list item has live content. + expect(newTr.steps.length).toBe(0); + expect(map.maps.length).toBe(0); + }); + it('attempts structural change when no live character exists before cursor (SD-2187)', () => { // All content is tracked-deleted — no character to delete, but the // handler should attempt the structural change (e.g. lifting out of @@ -436,13 +503,89 @@ describe('replaceAroundStep handler', () => { originalStepIndex: 0, }); - // The handler should not block — it should attempt the structural step. - // Whether the step succeeds depends on the document structure, but - // the handler must not silently return with 0 steps when the user - // presses backspace on an empty list item. - expect(newTr.steps.length).toBeGreaterThanOrEqual(0); - // Verify it doesn't throw — the key regression was that the handler - // returned early and left the user stuck. + // The handler must apply the structural step — the list item is fully + // track-deleted so there is nothing live to preserve. + expect(newTr.steps.length).toBeGreaterThan(0); + }); + + it('blocks structural step when first paragraph is deleted but list item has live content in a later paragraph', () => { + // Multi-paragraph list item: first paragraph fully deleted, second has + // live text. Backspace at the start of the first paragraph should NOT + // allow the structural lift — the list item scope still has live content. + const listItemType = schema.nodes.listItem || schema.nodes.list_item; + const bulletListType = schema.nodes.bulletList || schema.nodes.bullet_list; + + if (!listItemType || !bulletListType) return; + + const deleteMark = schema.marks[TrackDeleteMarkName].create({ + id: 'del-existing', + author: user.name, + authorEmail: user.email, + date, + }); + + let doc; + try { + doc = schema.nodes.doc.create({}, [ + bulletListType.create({}, [ + listItemType.create({}, [ + schema.nodes.paragraph.create({}, schema.nodes.run.create({}, [schema.text('Gone', [deleteMark])])), + schema.nodes.paragraph.create({}, schema.nodes.run.create({}, [schema.text('Still here')])), + ]), + ]), + ]); + } catch { + // Schema may not allow multiple paragraphs in a list item — skip. + return; + } + + let state = createState(doc); + + // Cursor at the start of the first (deleted) paragraph + const cursorPos = findTextPos(state.doc, 'Gone'); + state = state.apply(state.tr.setSelection(TextSelection.create(state.doc, cursorPos))); + + const tr = state.tr; + tr.setMeta('inputType', 'deleteContentBackward'); + + let paraStart = null; + let paraEnd = null; + state.doc.descendants((node, pos) => { + if (paraStart === null && node.type.name === 'paragraph') { + paraStart = pos; + paraEnd = pos + node.nodeSize; + } + }); + + const step = new ReplaceAroundStep( + paraStart, + paraEnd, + paraStart + 1, + paraEnd - 1, + new Slice(Fragment.from(schema.nodes.paragraph.create()), 0, 0), + 1, + true, + ); + + const newTr = state.tr; + const map = new Mapping(); + + replaceAroundStep({ + state, + tr, + step, + newTr, + map, + doc: state.doc, + user, + date, + originalStep: step, + originalStepIndex: 0, + }); + + // Structural step must be blocked — listItem still has live content. + expect(newTr.steps.length).toBe(0); + expect(map.maps.length).toBe(0); }); });