Hr

[brandonbarringer]

Because outlook and office365 doesn't play well with <hr>s, this commit creates a pseudo hr element using table markup

[rafibomb]

Love the PR! My thoughts:
We should avoid using the actual <hr> tag to create something that is not exactly a <hr> when processed. It could be confusing to people who expected an <hr> especially when custom styling. What do you think?

Perhaps we use a different tag?

<line>
<h-line>
<h-rule>

[brandonbarringer]

Excellent thought, and I agree with you on many levels. However, if I can play devils advocate for a moment, If the usual HR tag fails always, is there a reason to allow essentially always broken elements to a template?

[brandonbarringer]

@rafibomb Fixed the test errors by leveraging the code in pull request #37. Did you have additional thoughts on the naming of the component?

[Zettersten]

I agree with both... Is this the first time a decision like this will be made (to use an existing element node)?

If yes - then perhaps we stick with <h-line> due to the need for a larger discussion around the matter.

Good PR!

[brandonbarringer]

You can say that the <button> element would be an override of an existing html element as well

[rafibomb]

Hmm, <button> is a semantic element that is used in HTML5 but not in xhtml 1.0 as I understand it. Also, the <hr> creates a browser styled element where a <button> does not, much like a div.

I love the discussion! Other than user confusion, I'm not seeing an issue with using an <hr>.

[brandonbarringer]

That is a great point I hadn't considered. Also, user confusion is something I always try to avoid. I have been swayed. I'll make the change over to <h-line> if there are no objections.

[rafibomb]

No objections!

Couple last things to get this across the line:

• I think the empty table cell needs an  
• Can we safely omit the <tbody> tag?

[brandonbarringer]

Just to clarify, by empty table cell, are you referring to the <th>?
We can probably remove the <tbody> tag, but I will need to do some additional testing in litmus.
Should I be writing a test case for this as well? Do you have any thoughts around what needs to be tested?

[rafibomb]

Yes, empty <th> could cause issues in some Outlooks.

I added a test case in this branch: ff103b3

I used a <th> but a <td> should work the same or better without <tbody>. You can pull this branch code into yours to get the test case or copy it. And good question - we should have a test for every feature :)

[rafibomb]

So the Inky code here works well, but the SCSS is a bit more challenging to get to work across every email client. Do you have CSS that works well with this?

[brandonbarringer]

Sure. Here is what I used recently on a template

$hr-width: 70px;
$hr-height: 4px;
$hr-margin: 20px;
$hr-color: $medium-gray;

table.hr
    th
        height: 0
        border-bottom: $hr-height solid $hr-color
        width: $hr-width

[rafibomb]

@brandonbarringer Ok - looks great! Can you point this PR to develop so we can merge it in today? We're doing QA for the 2.2 release and would love to get this in!

[rafibomb]

Thanks - that did the trick! Updated the CSS! foundation/foundation-emails@29b923d

As long as we can get this PR into develop we're good for the release tomorrow!

[brandonbarringer]

Sorry it has been so long. I've been swamped with work. I will work on this tonight for tomorrow's release

[jsit]

As far as I can tell, there still remain a number of issues for this approach, all addressed in PR #679:

• Fix margin around <hr>s (#647)
• Make <hr>s 0-height (#648)
• Make <hr>s obey $hr-width (#678)
• Make <hr>s obey $hr-margin in Outlook 2007/10/13 (#682)

Most of these, especially #678, require that the compiled <h-line> markup be a table-within-a-table:

<table class="hr">
  <tr>
    <th>
      <table>
        <tr>
          <td>&nbsp;</td>
        </tr>
      </table>
    </th>
  </tr>
</table>

This is so that the inner <td> can be the element that has the border-bottom on it, with a width narrower than $global-width, and padding above and below it (for Outlook, which ignores margin).

[evanre]

Hi, there is a problem with a self-closing tag. I think it's because of Cheerio

This test will fall

  it('creates a horizontal rule that you can attach classes to', () => {
    var input = `<h-line class="dotted"><h2>Test</h2>`;
    var expected = `
      <table class="h-line dotted">
        <tr>
          <th>&nbsp;</th>
        </tr>
      </table>
      <h2>Test</h2>
    `;
     compare(input, expected);
  });
});