codex-team / editor.js

A block-style editor with clean JSON output
https://editorjs.io
Apache License 2.0
28.39k stars 2.07k forks source link

[Bug] Editing the class of a node always triggers 'didMutated' #1963

Open polyrainbow opened 2 years ago

polyrainbow commented 2 years ago

When changing the class name of an HTML element inside a block, this always triggers the didMutated event.

I've tried to disable this behavior by adding the attribute data-mutation-free="true" to the element, but the code does not respect this because the function shouldFireUpdate only considers added nodes and removed nodes with this attribute, but not the changed node itself.

How about changing this function

const shouldFireUpdate = mutationsOrInputEvent instanceof InputEvent ||
  !mutationsOrInputEvent.some(({
    addedNodes = [],
    removedNodes,
  }) => {
    return [...Array.from(addedNodes), ...Array.from(removedNodes)]
      .some(node => $.isElement(node) && (node as HTMLElement).dataset.mutationFree === 'true');
  });

to this:

const shouldFireUpdate = mutationsOrInputEvent instanceof InputEvent ||
  !mutationsOrInputEvent.some((mutationEvent) => {
    return [
        mutationEvent.target,
        ...Array.from(mutationEvent.addedNodes),
        ...Array.from(mutationEvent.removedNodes)
    ]
    .some(node => $.isElement(node) && (node as HTMLElement).dataset.mutationFree === 'true');
  });

This would include the target of the mutation record in the check for the attribute.

Steps to reproduce:

  1. Change the class name of an HTML element with the attribute data-mutation-free="true"

Expected behavior: The didMutated event should not fire.

Device, Browser, OS: all

Editor.js version: 2.23.0

neSpecc commented 2 years ago

Please explain your use case. Why changing of a classname should not trigger didMutated?

polyrainbow commented 2 years ago

I have a block that loads an image and then displays it. When the loading is finished, the class name of the wrapper element is changed for styling purposes. Basically the same that happens here: https://github.com/editor-js/image/blob/master/src/ui.js#L236

This loading does not mutate the state so IMO it should not trigger didMutated.

polyrainbow commented 2 years ago

I have now gathered some more information about this issue. Sorry if my initial report was confusing. Let me explain the problem in another way again:

The problem arises only under particular circumstances: The block that does the mutation must be the first block in the editor, because then it is automatically selected as current, as soon as the editor instance has finished loading. When a block is selected as current, the MutationObserver is activated and will trigger for each mutation a didMutated event (unless some of the added or removed nodes possess the "data-mutation-free" attribute). Now if this first, selected block must perform an asynchronous operation (e. g. downloading an image) that takes a while, and after that changes its appearance by changing some class names (e. g. removing a loading animation and displaying the image instead), didMutated is triggered, even though there is no state change intended by the user.

So my question is if there is any way to not trigger this didMutated event when changing class attributes of the block elements?

krisnik commented 1 year ago

Hi @SebastianZimmer, running into similar issue. Checking if you did find any solution or a work-around here?

polyrainbow commented 1 year ago

@krisnik No, unfortunately.

cptiwari20 commented 1 year ago

Looking for some solution!!

sKopheK commented 11 months ago

it's interesting that changing style attribute won't trigger the change (please don't add it :D)