GrapesJS / mjml

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

Cannot add element to empty body in Firefox #260

Closed Cha195 closed 1 year ago

Cha195 commented 2 years ago

Hello, I've found an error with Firefox while using the Grapes-Editor with this MJML-Plugin. The Mjml-body takes up the height of elements within it rather than the entire height. The issue arises when there are no elements, making the height of the body zero. When this happens, I'm not able to add any element to the body. This issue isn't present in Chrome or Edge. Here attached is an image from the Demo.

To reproduce: Open the demo editor in Firefox and remove all the elements. Try adding an element to it.

Thanks for any advice.

image

thewrath commented 2 years ago

Hello, I have the same problem on Firefox. The problem seems to come from the CSS property "min-height" of Body component which, in this context, doesn't behave like on chromium. A quick and easy fix would be to set this value in pixel for example but it's not necessarily the right solution (it's a temporary patch). I'll see how to make this temporary patch while waiting to find better.

thewrath commented 2 years ago

Looking in detail at the DOM, I see that there is a tag "mjml" I think that browsers do not work in the same way with this non-standard tag. Shouldn't it be encapsulated in a div like "mjml-body"?

image

ronaldohoch commented 2 years ago

more easy to test, run the commands:

editor.DomComponents.clear() editor.setComponents("<mjml><mj-head></mj-head><mj-body></mj-body></mjml>")

Also, the solution proposed by @thewrath works for me.

thewrath commented 2 years ago

I had some time today to work on that. I'm not necessarily good at HTML/CSS but with some research it seems that Firefox can't calculate the height of an element if the height of the parent element isn't set (min-height isn't enough). So the elements data-gjs-type="wrapper" and data-gjs-type="mjml" (or directly <mjml> need to have their properties height=100%.

I'm not sure but it seems to be directly a bug in GrapesJS since the wrapper component belongs to GrapesJS.

thewrath commented 2 years ago

If you want to patch this quickly you can define a small plugin that adds the missing CSS rule :

const wrapperHeightPatch = (e) => {
  e.on('load', () => {
    e.DomComponents.getWrapper().set('style', {'height': '100%'});
  })
}
artf commented 1 year ago

This should be fixed in the latest release.