[BUG] Can't add columns in version 0.0.24 about mjml HOT 7 CLOSED

Since grapesjs 0.14.50 drag and drop of mjml components have stopped working. I've debugged it and found the issue with the function updateAttributes inside ComponentView.js, specifically with the following lines:

attrs.forEach(function (attr) {
    return $el.removeAttr(attr);
});

I tried to trace back what the attributes deleted from the element are that cause the sorter not to know what the target element in onMove is, but after three days, I am still not sure.

At the moment I commented out those lines.

I've tested with the MJML project and other projects as well, and it all seems to still work.
What do these lines do? realizing these lines cause the MJML components to stop working - what can we update with the MJML components so they work again?

from mjml.

Confirmed fixed.

from mjml.

I have a fix for this problem, so maybe we can update it soon?

the problem is solved bij changing in components.js row 370:
draggable: '[data-type=mj-container]'
to
draggable: true,

in the domc.addType('mj-section' --> model --> defaults

from mjml.

it's more like a workaround rather than a fix, but as I don't have time to investigate more on it I'll accept a PR

from mjml.

@artf I've looked into this, this seems to happen because for the mjml plugin the actual HTML gets generated at coreMjmlView::getTemplateFromMjml() (https://github.com/artf/grapesjs-mjml/blob/master/src/components.js#L161) and assigned to the sandboxEl. But this sandboxEl is created and then not inserted into the DOM meaning it has no parentNode (https://github.com/artf/grapesjs-mjml/blob/master/src/components.js#L20). This breaks the draggable/droppable filtering in use by this plugin because the Sorter expects a parentNode or falls back to the body of the document that created the element (https://github.com/artf/grapesjs/blob/dev/src/utils/Sorter.js#L178) . Where it won't find it since it's not inserted in the DOM.

So seems to me either the sandboxEl needs to be inserted into the DOM so it can be found by the querySelectorAll() calls done by Sorter::matches() or that same method needs to create a parentNode for the element it's looking for instead of falling back to the body. A sample implementation in Grapesjs' Sorter.js::matches() would be:

  matches(el, selector, useBody, debug=false) {
    var startEl;

    // if we don't have a parentNode, create a node so the 
    // querySelectorAll() call below can find something. 
    if (!el.parentNode) {
        startEl = document.createElement('div');
        startEl.appendChild(el);
    } else {
        startEl = el.parentNode;
    }

    //startEl = useBody ? startEl.ownerDocument.body : startEl;
    var els = startEl.querySelectorAll(selector);
    var i = 0;
    while (els[i] && els[i] !== el)
      ++i;
    return !!els[i];
  }

Would you take that as a PR too ?

from mjml.

Thanks @mathieuk, it was actually an issue with matches in Sorter. It was affecting also other stuff inside the core. Now should be fixed, can you confirm?

from mjml.

Seems like this issue is back.

from mjml.