GrapesJS / mjml

Newsletter Builder with MJML components in GrapesJS
http://grapesjs.com/demo-mjml.html
BSD 3-Clause "New" or "Revised" License
637 stars 227 forks source link

Cannot be used with versions greater than 0.12.6 #19

Closed ateshuseyin closed 6 years ago

ateshuseyin commented 6 years ago

I am using this plugin. I stay attached to 0.12.6 We discussed it before in https://github.com/artf/grapesjs/issues/440

artf commented 6 years ago

Does it happen only when there is jquery?

ateshuseyin commented 6 years ago

No.It is not related with jquery. I checkout grapesjs-mjml and upgrade grapesjs to latest, still get same error.

ghost commented 6 years ago

Getting the same error with below sample code from ReadMe.md

<html>
<head>
<link href="https://cdn.jsdelivr.net/npm/grapesjs@0.12.17/dist/css/grapes.min.css" rel="stylesheet"/>
<script src="https://cdn.jsdelivr.net/npm/jquery@3.2.1/dist/jquery.min.js"></script>
<script src="https://cdn.jsdelivr.net/npm/grapesjs@0.12.17/dist/grapes.min.js"></script>
<script src="https://cdn.jsdelivr.net/npm/grapesjs-mjml@0.0.8/dist/grapesjs-mjml.min.js"></script>
</head>
<body>

<div id="gjs">
  <!-- Your MJML body here -->
  <mj-container>
        <mj-section>
          <mj-column>
            <mj-text>My Company</mj-text>
          </mj-column>
        </mj-section>
  <mj-container>
</div>

<script type="text/javascript">
  var editor = grapesjs.init({
      fromElement: 1,
      container : '#gjs',
      plugins: ['gjs-mjml'],
      pluginsOpts: {
        'gjs-mjml': {/* ...options */}
      }
  });
</script>
</body>
</html>

This is the error:

grapesjs-mjml.min.js:107 Uncaught TypeError: Cannot read property 'length' of undefined
    at r.addToCollection (grapesjs-mjml.min.js:107)
    at r.<anonymous> (grapesjs-mjml.min.js:107)
    at grapes.min.js:2
    at Function.C.each.C.forEach (grapes.min.js:2)
    at F.i.each (grapes.min.js:2)
    at render (grapesjs-mjml.min.js:107)
    at F.i.renderChildren (grapesjs-mjml.min.js:2)
    at F.i.render (grapesjs-mjml.min.js:2)
    at F.i.addToCollection (grapes.min.js:2)
    at grapes.min.js:2
ateshuseyin commented 6 years ago

@kavikaran cheers. It is exacly my problem.

artf commented 6 years ago

@ateshuseyin I've updated the current demo (v.0.12.25) and seems to work without issues

StephanHovius commented 6 years ago

I was working in this issue as well and i found a solution for this. Inside the componentsView of grapesjs ia the addToCollection function.

It went wrong at the spot where he tried to get the parentEl and childNodes. when the functions resetChildren() and render() are called, It calls this function with a for each function: this.collection.each(model => this.addToCollection(model));

but in the addTo function: addTo(model) { var i = this.collection.indexOf(model); this.addToCollection(model, null, i);

So, It expects an element but it is an Array / NodeList. I changed the 2 consts parent and children into this: `let parent = this.parentEl; let children = parent.childNodes;

if (typeof this.parentEl.length !== 'undefined' && this.parentEl.length > 0) { parent = this.parentEl[0]; children = this.parentEl[0].childNodes; }`

Maybe you have a better solution already? @artf

Kind regards,

Stephan Hovius

kavi-i commented 6 years ago

Hello @artf,

Thanks for solution. It does not show previous error. But everytime it shows invalid target position .

demo-mjml.html:263 Invalid target position: Target collection not found, 
Target is not droppable, accepts [undefined],
Component not draggable, acceptable by [undefined]
StephanHovius commented 6 years ago

@Kav-i I posted a solution about the issue you are occuring in here: https://github.com/artf/grapesjs-mjml/issues/21

artf commented 6 years ago

Thanks for the suggestion @StephanHovius about addToCollection but actually the problem is inside this repo. The new ComponentsView.rendermethod accepts only HTMLElement as an argument but as you can see here https://github.com/artf/grapesjs-mjml/blob/master/src/components.js#L180 it still uses "$". Currently, I'm working on other stuff so It'd be great if someone could remove all those jquery references and send a PR

mathieuk commented 6 years ago

See https://github.com/artf/grapesjs-mjml/pull/22 ( @StephanHovius is a colleague :) )

StephanHovius commented 6 years ago

good job! @mathieuk

artf commented 6 years ago

I've pushed the new release, thanks guys

kavi-i commented 6 years ago

thank you guys.