From 8138ce95b2fb482e8618c5b260b48f5095815beb Mon Sep 17 00:00:00 2001 From: VikhorKonstantin Date: Wed, 28 Feb 2024 21:08:08 +0400 Subject: [PATCH] fix issue #2523 (#2639) * fix isMutationBelongsToElement function: make it return true if the whole text node is deleted inside of some descendant of the passed element * isMutationBelongsToElement function shouldn't return true if some of the ancestors of the passed element were added or deleted, only if the element itself * add test case verifying that 'onChange' is fired when the whole text inside some nested descendant of the block is removed * replace introduced dependency with ToolMock * add comment explaining isMutationBelongsToElement behaviour in case of adding/removing the passed element itself * fix formatting * added some more explanation * added record to the changelog --------- Co-authored-by: Peter Savchenko --- docs/CHANGELOG.md | 4 +++ src/components/utils/mutations.ts | 30 ++++++++-------- test/cypress/tests/onchange.cy.ts | 58 +++++++++++++++++++++++++++++++ 3 files changed, 77 insertions(+), 15 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 0a1108fc..6d63e3c4 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +### 2.30.0 + +- `Fix` — `onChange` will be called when removing the entire text within a descendant element of a block. + ### 2.29.1 - `Fix` — Toolbox wont be shown when Slash pressed with along with Shift or Alt diff --git a/src/components/utils/mutations.ts b/src/components/utils/mutations.ts index ce8bcff8..6e98456d 100644 --- a/src/components/utils/mutations.ts +++ b/src/components/utils/mutations.ts @@ -8,28 +8,28 @@ export function isMutationBelongsToElement(mutationRecord: MutationRecord, eleme const { type, target, addedNodes, removedNodes } = mutationRecord; /** - * In case of removing the whole text in element, mutation type will be 'childList', - * 'removedNodes' will contain text node that is not existed anymore, so we can't check it with 'contains' method - * But Target will be the element itself, so we can detect it. + * Covers all types of mutations happened to the element or it's descendants with the only one exception - removing/adding the element itself; */ - if (target === element) { + if (element.contains(target)) { return true; } /** - * Check typing and attributes changes + * In case of removing/adding the element itself, mutation type will be 'childList' and 'removedNodes'/'addedNodes' will contain the element. */ - if (['characterData', 'attributes'].includes(type)) { - const targetElement = target.nodeType === Node.TEXT_NODE ? target.parentNode : target; + if (type === 'childList') { + const elementAddedItself = Array.from(addedNodes).some(node => node === element); - return element.contains(targetElement); + if (elementAddedItself) { + return true; + } + + const elementRemovedItself = Array.from(removedNodes).some(node => node === element); + + if (elementRemovedItself) { + return true; + } } - /** - * Check new/removed nodes - */ - const addedNodesBelongsToBlock = Array.from(addedNodes).some(node => element.contains(node)); - const removedNodesBelongsToBlock = Array.from(removedNodes).some(node => element.contains(node)); - - return addedNodesBelongsToBlock || removedNodesBelongsToBlock; + return false; } diff --git a/test/cypress/tests/onchange.cy.ts b/test/cypress/tests/onchange.cy.ts index d4c90835..ef26d1bd 100644 --- a/test/cypress/tests/onchange.cy.ts +++ b/test/cypress/tests/onchange.cy.ts @@ -1,5 +1,6 @@ import Header from '@editorjs/header'; import Code from '@editorjs/code'; +import ToolMock from '../fixtures/tools/ToolMock'; import Delimiter from '@editorjs/delimiter'; import { BlockAddedMutationType } from '../../../types/events/block/BlockAdded'; import { BlockChangedMutationType } from '../../../types/events/block/BlockChanged'; @@ -787,4 +788,61 @@ describe('onChange callback', () => { })); }); }); + + it('should be fired when the whole text inside some descendant of the block is removed', () => { + /** + * Mock of Tool with nested contenteditable element + */ + class ToolWithContentEditableDescendant extends ToolMock { + /** + * Creates element with nested contenteditable element + */ + public render(): HTMLElement { + const contenteditable = document.createElement('div'); + + contenteditable.contentEditable = 'true'; + contenteditable.innerText = 'a'; + contenteditable.setAttribute('data-cy', 'nested-contenteditable'); + + const wrapper = document.createElement('div'); + + wrapper.appendChild(contenteditable); + + return wrapper; + } + } + + const config = { + tools: { + testTool: { + class: ToolWithContentEditableDescendant, + }, + }, + data: { + blocks: [ + { + type: 'testTool', + data: 'a', + }, + ], + }, + onChange: (): void => { + console.log('something changed'); + }, + }; + + cy.spy(config, 'onChange').as('onChange'); + cy.createEditor(config).as('editorInstance'); + + cy.get('[data-cy=nested-contenteditable]') + .click() + .clear(); + + cy.get('@onChange').should('be.calledWithMatch', EditorJSApiMock, Cypress.sinon.match({ + type: BlockChangedMutationType, + detail: { + index: 0, + }, + })); + }); });