Skip to content

Commit

Permalink
fix issue #2523 (#2639)
Browse files Browse the repository at this point in the history
* 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 <[email protected]>
  • Loading branch information
VikhorKonstantin and neSpecc authored Feb 28, 2024
1 parent 7ff5faa commit 8138ce9
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 15 deletions.
4 changes: 4 additions & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
30 changes: 15 additions & 15 deletions src/components/utils/mutations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
* Check new/removed nodes
*/
const addedNodesBelongsToBlock = Array.from(addedNodes).some(node => element.contains(node));
const removedNodesBelongsToBlock = Array.from(removedNodes).some(node => element.contains(node));
const elementRemovedItself = Array.from(removedNodes).some(node => node === element);

if (elementRemovedItself) {
return true;
}
}

return addedNodesBelongsToBlock || removedNodesBelongsToBlock;
return false;
}
58 changes: 58 additions & 0 deletions test/cypress/tests/onchange.cy.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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,
},
}));
});
});

0 comments on commit 8138ce9

Please sign in to comment.