mjmlio / mjml

MJML: the only framework that makes responsive-email easy
https://mjml.io
MIT License
16.94k stars 954 forks source link

The whole button area is not clickable #2565

Open fernamp opened 1 year ago

fernamp commented 1 year ago

The mj-button does not work as expected in MSO PC desktop because it has specific instructions that produce the problem.

To Reproduce Steps to reproduce the behavior:

  1. use the usual: <mj-button href="www.google.com" border="1px solid red">Unmodified button</mj-button>

  2. The HTML produced is this one:

    <table border="0" cellpadding="0" cellspacing="0" role="presentation" style="border-collapse:separate;line-height:100%;">
    <tr>
        <td align="center" bgcolor="#414141" role="presentation" style="border:1px solid red; border-radius:3px; cursor:auto; background:#414141; mso-padding-alt:10px 25px;" valign="middle">
            <a href="www.google.com" style="display:inline-block; background:#414141; color:#ffffff; font-family:Ubuntu, Helvetica, Arial, sans-serif; font-size:13px; font-weight:normal; line-height:120%; margin:0; text-decoration:none; text-transform:none; padding:10px 25px; mso-padding-alt:0px; border-radius:3px;" target="_blank"> Unmodified button </a>
        </td>
    </tr>
    </table>

Expected behavior This HTML:

<table border="0" cellpadding="0" cellspacing="0" role="presentation" style="border-collapse:separate; line-height:100%;">
    <tr>
        <td align="center" bgcolor="#414141" role="presentation" style="border:1px solid red; border-radius:3px; cursor:auto; background:#414141;" valign="middle">
            <a href="www.google.com" style="display:inline-block; background:#414141; color:#ffffff; font-family:Ubuntu, Helvetica, Arial, sans-serif; font-size:13px; font-weight:normal; line-height:120%; margin:0; text-decoration:none; text-transform:none; padding:10px 25px; border-radius:3px; border: 1px solid #414141;" target="_blank"> Modified button </a>
        </td>
    </tr>
</table>

In MSO, <a> is an inline but UNMODIFIABLE element (it can't become a block, nor an inline-block), unless you trick MSO into think that the <a> is "something" cause it has a border, triggering the use of the display declaration and therefore, the use of the padding (so removing the tool that provides the inner spacing of the button using mso-padding-alt: 0px; is kind of weird (taking into account that adding padding to the <a> in MSO has no real effect).

The previous code make the whole button clickable in MSO.

Screenshots Unmodified MJML button HTML: 198571169-29e2a601-2a90-4c6e-abad-3720978f19af

Modified MJML button HTML: 198571696-79a1ad28-131d-4bc0-9e22-a92169c4cd97

Is this something MJML team can take? Or should I make a branch?

iRyusa commented 1 year ago

Damn that would be such a nice fix to have if it have 0 side effects ! Great finding !

How does it work with transparent buttons ? You put a 1px solid transparent on a tag ? Can we try to have a compatibility (emailonacid/litmus) test with fixed width button, multi lines button, and button with img inside it ?

We need to be sure it won't break anything else in other desktop clients.

fernamp commented 1 year ago

I'll be onto it later this week. I will return to post info as soon as I have some data and tests from EOA and Litmus. Thanks for the testing ideas btw!

robertmylne commented 1 year ago

This issue has been solved here by Maizzle: https://maizzle.com/docs/examples/buttons

<a
  href="https://maizzle.com/"
  class="inline-block py-4 px-6 text-sm leading-none font-semibold rounded [text-decoration:none] text-white bg-indigo-500 hover:bg-indigo-600">
  <!--[if mso]><i style="letter-spacing: 27px; mso-font-width: -100%; mso-text-raise: 26pt;">&nbsp;</i><![endif]-->
    <span style="mso-text-raise: 13pt;">Read more</span>
  <!--[if mso]><i style="letter-spacing: 27px; mso-font-width: -100%;">&nbsp;</i><![endif]-->
</a>
iRyusa commented 1 year ago

We don’t want VML inside mj button because it would require to check if the button is wrapped in an element that can produce VML.But for the general use case it’s a great solution.On 13 Nov 2022, at 01:59, robertmylne @.***> wrote: This issue has been solved here by Maizzle: https://maizzle.com/docs/examples/buttons <a href="https://maizzle.com/" class="inline-block py-4 px-6 text-sm leading-none font-semibold rounded [text-decoration:none] text-white bg-indigo-500 hover:bg-indigo-600">

<span style="mso-text-raise: 13pt;">Read more</span>

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you commented.Message ID: @.***>

fernamp commented 1 year ago

OK, I did some tests and changes to the mjml-button\src\index.js file and that solves the clickable area.

image

The colour of the "border" should be coordinated with the colour of the TD's background.

Here the MJML with the test:

image

and here the resulting HTML:

image

Results in EOA and Litmus show the button working everywhere.

This is from MSO 2019:

image

I have to check with my company if I can share the links.

Or contact me directly to share them (I'll try to do it from my side anyway).

I'll check if the Maizzle implementation is usable/implementable in MJML.

iRyusa commented 1 year ago

Looks good then if you can open a PR about this ? I think it should just handle the case where user wants another border color on this one 👍

iRyusa commented 9 months ago

Sorry to come back very late about this, but can we test multi line button with this technique ?