Skip to content

Commit 8138ce9

Browse files
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 <specc.dev@gmail.com>
1 parent 7ff5faa commit 8138ce9

File tree

3 files changed

+77
-15
lines changed

3 files changed

+77
-15
lines changed

docs/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
# Changelog
22

3+
### 2.30.0
4+
5+
- `Fix``onChange` will be called when removing the entire text within a descendant element of a block.
6+
37
### 2.29.1
48

59
- `Fix` — Toolbox wont be shown when Slash pressed with along with Shift or Alt

src/components/utils/mutations.ts

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,28 +8,28 @@ export function isMutationBelongsToElement(mutationRecord: MutationRecord, eleme
88
const { type, target, addedNodes, removedNodes } = mutationRecord;
99

1010
/**
11-
* In case of removing the whole text in element, mutation type will be 'childList',
12-
* 'removedNodes' will contain text node that is not existed anymore, so we can't check it with 'contains' method
13-
* But Target will be the element itself, so we can detect it.
11+
* Covers all types of mutations happened to the element or it's descendants with the only one exception - removing/adding the element itself;
1412
*/
15-
if (target === element) {
13+
if (element.contains(target)) {
1614
return true;
1715
}
1816

1917
/**
20-
* Check typing and attributes changes
18+
* In case of removing/adding the element itself, mutation type will be 'childList' and 'removedNodes'/'addedNodes' will contain the element.
2119
*/
22-
if (['characterData', 'attributes'].includes(type)) {
23-
const targetElement = target.nodeType === Node.TEXT_NODE ? target.parentNode : target;
20+
if (type === 'childList') {
21+
const elementAddedItself = Array.from(addedNodes).some(node => node === element);
2422

25-
return element.contains(targetElement);
26-
}
23+
if (elementAddedItself) {
24+
return true;
25+
}
2726

28-
/**
29-
* Check new/removed nodes
30-
*/
31-
const addedNodesBelongsToBlock = Array.from(addedNodes).some(node => element.contains(node));
32-
const removedNodesBelongsToBlock = Array.from(removedNodes).some(node => element.contains(node));
27+
const elementRemovedItself = Array.from(removedNodes).some(node => node === element);
28+
29+
if (elementRemovedItself) {
30+
return true;
31+
}
32+
}
3333

34-
return addedNodesBelongsToBlock || removedNodesBelongsToBlock;
34+
return false;
3535
}

test/cypress/tests/onchange.cy.ts

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import Header from '@editorjs/header';
22
import Code from '@editorjs/code';
3+
import ToolMock from '../fixtures/tools/ToolMock';
34
import Delimiter from '@editorjs/delimiter';
45
import { BlockAddedMutationType } from '../../../types/events/block/BlockAdded';
56
import { BlockChangedMutationType } from '../../../types/events/block/BlockChanged';
@@ -787,4 +788,61 @@ describe('onChange callback', () => {
787788
}));
788789
});
789790
});
791+
792+
it('should be fired when the whole text inside some descendant of the block is removed', () => {
793+
/**
794+
* Mock of Tool with nested contenteditable element
795+
*/
796+
class ToolWithContentEditableDescendant extends ToolMock {
797+
/**
798+
* Creates element with nested contenteditable element
799+
*/
800+
public render(): HTMLElement {
801+
const contenteditable = document.createElement('div');
802+
803+
contenteditable.contentEditable = 'true';
804+
contenteditable.innerText = 'a';
805+
contenteditable.setAttribute('data-cy', 'nested-contenteditable');
806+
807+
const wrapper = document.createElement('div');
808+
809+
wrapper.appendChild(contenteditable);
810+
811+
return wrapper;
812+
}
813+
}
814+
815+
const config = {
816+
tools: {
817+
testTool: {
818+
class: ToolWithContentEditableDescendant,
819+
},
820+
},
821+
data: {
822+
blocks: [
823+
{
824+
type: 'testTool',
825+
data: 'a',
826+
},
827+
],
828+
},
829+
onChange: (): void => {
830+
console.log('something changed');
831+
},
832+
};
833+
834+
cy.spy(config, 'onChange').as('onChange');
835+
cy.createEditor(config).as('editorInstance');
836+
837+
cy.get('[data-cy=nested-contenteditable]')
838+
.click()
839+
.clear();
840+
841+
cy.get('@onChange').should('be.calledWithMatch', EditorJSApiMock, Cypress.sinon.match({
842+
type: BlockChangedMutationType,
843+
detail: {
844+
index: 0,
845+
},
846+
}));
847+
});
790848
});

0 commit comments

Comments
 (0)