mjml icon indicating copy to clipboard operation
mjml copied to clipboard

Components overwrite child attributes with null

Open SebastianStehle opened this issue 2 years ago • 1 comments

Describe the bug It is a little bit harder to describe, but I found it when working on a port of mjml to C#:

It is general problem I think, but reproducible in the Social component.

The social element has the following property:

  static allowedAttributes = {
    'text-padding': 'unit(px,%){1,4}'
  }

  static defaultAttributes = {
    'text-padding': '4px 4px 4px 0'
  }

I am going to omit the other attributes because they are not relevant.

So when I render the mj-social-element standalone I have the following (simplified) output for the text:

<td style="vertical-align:middle;padding:4px 4px 4px 0;">
        <a> Facebook </a>
</td>

But the social-element also has the same attribute without a default value, which falls back to null.

So when the social component creates the attributes for the child, it uses this code:

getSocialElementAttributes() {
    const base = {}
    if (this.getAttribute('inner-padding')) {
      base.padding = this.getAttribute('inner-padding')
    }

    return [
      'text-padding',
      MORE PROPERTIES
    ].reduce((res, attr) => {
      res[attr] = this.getAttribute(attr)
      return res
    }, base)
  }

It basically create a object with a lot of null values and these null values seem to override the default values from the child component, which makes no sense to me. Somehow you might have thought about that in the inner-padding, but I think the general component behavior is just wrong here.

This means that when a social-element is rendered inside a social component it has no text-padding anymore.

To Reproduce

<mjml>
  <mj-body>
      <mj-social>
          <mj-social-element name="facebook" href="https://mjml.io/">
            Facebook
          </mj-social-element>
      </mj-social>
  </mj-body>
</mjml>

Expected behavior Components should not override children null.

MJML environment (please complete the following information):

  • OS: -
  • MJML Version: 4.12.0

SebastianStehle avatar Mar 17 '22 16:03 SebastianStehle

Clearly a bad side effect of inheritance that we wanted originally. We should filter null values instead 👍

iRyusa avatar Mar 30 '22 11:03 iRyusa

Should be fixed in recent MJML version.

iRyusa avatar Sep 04 '22 21:09 iRyusa