From 7e88127529181b929b536d63b9daa8a921290f02 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Thu, 15 Oct 2026 12:50:17 +0200 Subject: [PATCH] Prevent text blocks with empty value Deleting a text block while all of its paragraphs were selected removed every paragraph but kept the content element, persisting an empty value that later caused Slate errors. Delete the whole content element in that case instead. Render existing text blocks with empty values as an empty paragraph so affected entries can be edited and commented on again. --- .../contentElements/textBlock/editor-spec.js | 28 +++++++++ .../frontend/commenting/EditableText-spec.js | 11 ++++ .../EditableText/features/placeholder-spec.js | 10 ++++ .../features/selectionTransientState-spec.js | 60 +++++++++++++++++++ .../spec/support/pageObjects/inlineEditing.js | 22 ++++++- .../src/contentElements/textBlock/editor.js | 2 +- .../src/frontend/commenting/EditableText.js | 2 +- .../inlineEditing/EditableText/Selection.js | 3 +- .../inlineEditing/EditableText/index.js | 11 ++-- 9 files changed, 140 insertions(+), 9 deletions(-) create mode 100644 entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/selectionTransientState-spec.js diff --git a/entry_types/scrolled/package/spec/contentElements/textBlock/editor-spec.js b/entry_types/scrolled/package/spec/contentElements/textBlock/editor-spec.js index 7956218d44..f2c132666b 100644 --- a/entry_types/scrolled/package/spec/contentElements/textBlock/editor-spec.js +++ b/entry_types/scrolled/package/spec/contentElements/textBlock/editor-spec.js @@ -1,5 +1,6 @@ import 'contentElements/textBlock/editor'; import {editor} from 'editor'; +import {ContentElement} from 'editor/models/ContentElement'; describe('contentElements/textBlock/editor', () => { const type = editor.contentElementTypes.findByTypeName('textBlock'); @@ -129,4 +130,31 @@ describe('contentElements/textBlock/editor', () => { }); }); + describe('#handleDestroy', () => { + it('removes selected blocks if selection does not cover all blocks', () => { + const contentElement = new ContentElement({ + typeName: 'textBlock', + transientState: {editableTextSelectionCoversAllBlocks: false} + }); + jest.spyOn(contentElement, 'postCommand'); + + const result = type.handleDestroy(contentElement); + + expect(contentElement.postCommand).toHaveBeenCalledWith({type: 'REMOVE'}); + expect(result).toBe(false); + }); + + it('lets content element be deleted if selection covers all blocks', () => { + const contentElement = new ContentElement({ + typeName: 'textBlock', + transientState: {editableTextSelectionCoversAllBlocks: true} + }); + jest.spyOn(contentElement, 'postCommand'); + + const result = type.handleDestroy(contentElement); + + expect(contentElement.postCommand).not.toHaveBeenCalled(); + expect(result).not.toBe(false); + }); + }); }); diff --git a/entry_types/scrolled/package/spec/frontend/commenting/EditableText-spec.js b/entry_types/scrolled/package/spec/frontend/commenting/EditableText-spec.js index 2ad10a6584..4aec9b572c 100644 --- a/entry_types/scrolled/package/spec/frontend/commenting/EditableText-spec.js +++ b/entry_types/scrolled/package/spec/frontend/commenting/EditableText-spec.js @@ -33,6 +33,17 @@ describe('commenting EditableText', () => { expect(entry.queryByText('Select text to comment')).not.toBeInTheDocument(); }); + it('renders empty paragraph if value is empty array', () => { + const entry = renderEntry({ + contentElement: { + ui: , + typeOptions: {inlineComments: true} + } + }); + + expect(entry.container.querySelector('[data-slate-node="element"]')).toBeInTheDocument(); + }); + it('shows hint in add comment mode', async () => { const user = userEvent.setup(); const entry = renderEntry({ diff --git a/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/placeholder-spec.js b/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/placeholder-spec.js index 4241478e8e..58573c27fd 100644 --- a/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/placeholder-spec.js +++ b/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/placeholder-spec.js @@ -36,6 +36,16 @@ describe('inline editing EditableText placeholder', () => { expect(entry.queryByText('Some placeholder')).toBeInTheDocument(); }); + it('renders placeholder if value is empty array', () => { + const entry = renderEntry({ + contentElement: { + ui: + } + }); + + expect(entry.queryByText('Some placeholder')).toBeInTheDocument(); + }); + it('does not render placeholder if value is present', () => { const entry = renderEntry({ contentElement: { diff --git a/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/selectionTransientState-spec.js b/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/selectionTransientState-spec.js new file mode 100644 index 0000000000..3829ebf45d --- /dev/null +++ b/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/selectionTransientState-spec.js @@ -0,0 +1,60 @@ +import React from 'react'; + +import {EditableText} from 'frontend'; +import {renderEntry, useInlineEditingPageObjects} from 'support/pageObjects/inlineEditing'; + +import {waitFor} from '@testing-library/react'; +import '@testing-library/jest-dom/extend-expect'; + +describe('inline editing EditableText selection transient state', () => { + useInlineEditingPageObjects(); + + beforeEach(() => { + Range.prototype.getBoundingClientRect = () => document.body.getBoundingClientRect(); + Range.prototype.getClientRects = () => []; + }); + + afterEach(() => { + delete Range.prototype.getBoundingClientRect; + delete Range.prototype.getClientRects; + }); + + const value = [ + {type: 'paragraph', children: [{text: 'First paragraph'}]}, + {type: 'paragraph', children: [{text: 'Second paragraph'}]} + ]; + + it('reports selection covering all blocks', async () => { + const entry = renderEntry({ + contentElement: { + ui: , + typeOptions: {customSelectionRect: true} + } + }); + + entry.simulateSelectMessage({type: 'contentElement', id: 1, range: [0, 2]}); + + await waitFor(() => + expect(entry.getTransientContentElementState(1)).toMatchObject({ + editableTextSelectionCoversAllBlocks: true + }) + ); + }); + + it('reports selection not covering all blocks', async () => { + const entry = renderEntry({ + contentElement: { + ui: , + typeOptions: {customSelectionRect: true} + } + }); + + entry.simulateSelectMessage({type: 'contentElement', id: 1, range: [1, 2]}); + + await waitFor(() => + expect(entry.getTransientContentElementState(1)).toMatchObject({ + editableTextSelectionCoversAllBlocks: false + }) + ); + }); +}); diff --git a/entry_types/scrolled/package/spec/support/pageObjects/inlineEditing.js b/entry_types/scrolled/package/spec/support/pageObjects/inlineEditing.js index 4bfef69af8..936592a0d4 100644 --- a/entry_types/scrolled/package/spec/support/pageObjects/inlineEditing.js +++ b/entry_types/scrolled/package/spec/support/pageObjects/inlineEditing.js @@ -24,7 +24,27 @@ export function renderEntry({commenting, ...options} = {}) { return { ...result, queryAllCommentBadges: () => - result.queryAllByRole('status').map(createCommentBadgePageObject) + result.queryAllByRole('status').map(createCommentBadgePageObject), + + simulateSelectMessage(payload) { + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: {type: 'SELECT', payload}, + origin: window.location.origin + })); + }); + }, + + getTransientContentElementState(id) { + return window.parent.postMessage.mock.calls + .map(([message]) => message) + .filter(message => + message.type === 'UPDATE_TRANSIENT_CONTENT_ELEMENT_STATE' && + message.payload.id === id + ) + .map(message => message.payload.state) + .pop(); + } }; } diff --git a/entry_types/scrolled/package/src/contentElements/textBlock/editor.js b/entry_types/scrolled/package/src/contentElements/textBlock/editor.js index 3e3237d240..12df429482 100644 --- a/entry_types/scrolled/package/src/contentElements/textBlock/editor.js +++ b/entry_types/scrolled/package/src/contentElements/textBlock/editor.js @@ -161,7 +161,7 @@ editor.contentElementTypes.register('textBlock', { handleDestroy(contentElement) { const transientState = contentElement.get('transientState') || {}; - if (!transientState.editableTextIsSingleBlock) { + if (!transientState.editableTextSelectionCoversAllBlocks) { contentElement.postCommand({type: 'REMOVE'}); return false; } diff --git a/entry_types/scrolled/package/src/frontend/commenting/EditableText.js b/entry_types/scrolled/package/src/frontend/commenting/EditableText.js index 28302cdf9d..95b786e119 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/EditableText.js +++ b/entry_types/scrolled/package/src/frontend/commenting/EditableText.js @@ -91,7 +91,7 @@ function CommentingEditableText({ typographyVariant={typographyVariant} typographySize={typographySize}> {}}> [{ + type: 'paragraph', + children: [{text: ''}] + }], []); + + const [cachedValue, setCachedValue] = useCachedValue(value?.length ? value : emptyValue, { onDebouncedChange: nextValue => { if (onChange) { onChange(nextValue, {commentThreadSubjectRanges: getTrackedSubjectRanges()});