editor.js icon indicating copy to clipboard operation
editor.js copied to clipboard

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

Open polyrainbow opened this issue 3 years ago • 3 comments

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

polyrainbow avatar Feb 08 '22 09:02 polyrainbow

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

neSpecc avatar Feb 08 '22 11:02 neSpecc

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 avatar Feb 08 '22 11:02 polyrainbow

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?

polyrainbow avatar May 19 '22 18:05 polyrainbow

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

krisnik avatar Jan 06 '23 06:01 krisnik

@krisnik No, unfortunately.

polyrainbow avatar Jan 06 '23 10:01 polyrainbow

Looking for some solution!!

cptiwari20 avatar Jul 10 '23 17:07 cptiwari20

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

sKopheK avatar Oct 10 '23 13:10 sKopheK