mjml icon indicating copy to clipboard operation
mjml copied to clipboard

This code in mj-hero makes no sense.

Open SebastianStehle opened this issue 2 years ago • 1 comments

I could be really wrong of course but I do not understand this code:

From: https://github.com/mjmlio/mjml/blob/master/packages/mjml-hero/src/index.js#L49

I added my comments to the code.

getChildContext() {
    // Refactor -- removePaddingFor(width, ['padding', 'inner-padding'])
    const { containerWidth } = this.context
    const paddingSize =
      this.getShorthandAttrValue('padding', 'left') +
      this.getShorthandAttrValue('padding', 'right')

    // First we format the value to add "px" suffix.
    let currentContainerWidth = `${parseFloat(containerWidth)}px`

    // Now we parse the code again, but why?
    const { unit, parsedWidth } = widthParser(currentContainerWidth, {
      parseFloatToInt: false,
    })

   // We added the "px" suffix a few lines before, how can it ever be "%" unit?
    if (unit === '%') {
      currentContainerWidth = `${
        (parseFloat(containerWidth) * parsedWidth) / 100 - paddingSize
      }px`
    } else {
      currentContainerWidth = `${parsedWidth - paddingSize}px`
    }

    return {
      ...this.context,
      containerWidth: currentContainerWidth,
    }
  }

SebastianStehle avatar Mar 14 '22 16:03 SebastianStehle

Yup some of the code are non-necessary today. It looks like we tried at somepoint to have a width attribute on mj-hero so that's why we had to parse it back. This could be simplified easily now. Feel free to open a PR on this

iRyusa avatar Mar 16 '22 10:03 iRyusa