gakimball / react-inky

🦑 React components for Inky
https://gakimball.github.io/react-inky
MIT License
21 stars 10 forks source link

Spacer not working - outlook does not recognize px on td #16

Closed mir3z closed 6 years ago

mir3z commented 6 years ago

See this issue: https://foundation.zurb.com/forum/posts/47983-outlook-2010-spacer-issue

When rendering you add height={${size}px} on td. According to above mentioned issue Outlook does not recognize px and <Spacer /> does not work - which I can confirm.

The best solution seems to be adding height twice - with px and without it. What do you think?

Edit: I know your React port works exactly link Inky does... Which is strange to me that it hasn't been fixed for so long time. I've created similar issue in Inky: https://github.com/zurb/inky/issues/104, but then I've found another: https://github.com/zurb/foundation-emails/issues/725

gakimball commented 6 years ago

I could see adding this fix, but I'd probably put it behind a feature flag, because it would deviate from how Inky works.

However, I'd prefer to only make this change if we can verify that changing to a unitless height value won't break spacers in any other email clients. I don't have my own Litmus account, so I can't do any tests myself unfortunately.

mir3z commented 6 years ago

I did quick test yesterday as I was working on rewriting our mailing system using your library and I can confirm that change didn't break anything at least in email clients we support. I will do additional testing later today and let you know.

I hope guys from inky and email foundation add their two cents to the discussion. It would be so much easier...

mir3z commented 6 years ago

I've done testing again, using zurb foundation styles and your library with the following markup:

    <Wrapper>
        <Row>
            <Column><h1>TOP</h1></Column>
        </Row>
        <Spacer size={ 30 } />
        <Row>
            <Column><h1>BOTTOM</h1></Column>
        </Row>
    </Wrapper>

Outlook 2007, 2010, 2013, 2013 120 DPI, 2019, Windows 10 Mail are broken. After adding height without units that clients looks good and I can't see any regression in the remaining clients.

mir3z commented 6 years ago

Great, thanks again for the fix ;)