search

defund12 / defund12.org

defund12.org
https://defund12.org/
MIT License
181 stars 79 forks source link

Single new line vs double new line, renders different on website vs email #1349

Open dionlarson opened 4 years ago

dionlarson commented 4 years ago

Bug report

In rendered markdown email templates (HTML), new lines are handled differently than in email mailto: links (plaintext).

Most submitted templates have a single new line for each line of the signature.

Single new line

Best,
Dion

renders 

<p>Best, Dion</p>

Markdown based solution: enforce one of the styles below

Single new line with two spaces at end of first line (this is "correct")

note: first line below is Best,.

Best,  
Dion

renders

<p>
  Best,<br>
  Dion
</p>

Double new line

Best,

Dion

renders

<p>Best,</p>
<p>Dion</p>

Code based solution

See solution 1 here. Markdown based solution also from this post.

dionlarson commented 4 years ago

Let me know what y'all think. I can write a test to enforce a markdown solution or add the necessary code.

NickCrews commented 4 years ago

I would say that the letter in the .md files should be copy/pastable directly to an email and sent as is. That would mean that requiring 2 spaces for a line break is bad. I see the problem as being the markdownify not wuite doing what we want, so we should fix it there. I'm trying to find a solution now...

NickCrews commented 4 years ago

Here's a solution with some kinda ugly code, but the output HTML is correct semantically. What's more, this also actually may solve a problem we haven't run into yet, where someone inadvertently uses markdown syntax in the body of their letter, and the markdownify filter we are using formats it. This mare-metal solution does a bit less magic, and may be more predictable. It would break people using Siteleaf though I think...

NickCrews commented 4 years ago

Another reason against making the email body have weird rules such as 2 spaces is then those two weird spaces are going to show up everywhere else, such as where we use page.body at https://github.com/defund12/defund12.org/blob/c59fb01054194d5dde25769d283dc35b63252826/_layouts/email.html#L34.

I think we're getting hung up, thinking that the email body is markdown, but it's not markdown, it's just plain text. We shouldn't be running it through the markdownify filter to display it as HTML in the first place. We should instead be phrasing the problem as "how do we convert plaintext to a nice format to display on the web?" Additionally, preferably then if someone selects, copies, and pastes the rendered HTML then it should give the same thing as page.body.

NickCrews commented 4 years ago

OK I made a PR that fixes it I think. It looks nearly identical and functions as we want.

dionlarson commented 4 years ago

This PR (#1370) makes two decisions outside the scope of this issue.

Should email templates allow markdown syntax in body?

The email template is markdown. Remember, emails are being submitted in markdown, and rendered through a markdown processor. It's WYSIWYG in Github. These emails bodies are written in markdown even if people don't use it's features. It's just that defund12.org doesn't have much support for sending HTML (rendered markdown) emails yet.

Emails can be plaintext or HTML. We already are sending HTML emails on Android to support Gmail app (<br> necessary).

Currently, we convert to HTML when displaying, but send text raw as plaintext. These should be the same, but I'm not sure I agree with a "emails are always plaintext" approach.

Another reason against making the email body have weird rules such as 2 spaces is then those two weird spaces are going to show up everywhere else, such as where we use page.body at

This is not an issue if we process the body the same at every copy point (mailto:, copyToClipboard, webpage).

Is Siteleaf causing more problems then helping?

I can't fully answer this because I'm not certain how much time it saves. I do feel like it adds another variable to maintaining the email templates though and we should remove it to simplify things.

NickCrews commented 4 years ago

The email template is markdown. Remember, emails are being submitted in markdown, and rendered through a markdown processor.

I see what you're saying, if people wanted to then they could use the MD features when writing their github issues. But people aren't using those features as far as I can tell, they are just writing plaintext. They either don't know about the markdown feature, assume it won't work in an email, or they have prewritten the letter somewhere else and are copy-pasting. If we did want people to use markdown, we could advertise "Hey you can use markdown, check out those formatting buttons at the top!", but I think we still would have another issue:

This is not an issue if we process the body the same at every copy point (mailto:, copyToClipboard, webpage).

If we use markdown as the email syntax and HTML as the output then we can never have all three methods (mailto:, C2C button, select webpage and copy-paste) give the same results: mailto: links don't work with sending HTML in the body. So you can only get plaintext that way. Also, I don't think we want to give the user HTML when they click the C2C button, because then if you paste that anywhere (gmail, textedit, google doc) you just get HTML tags, not something nicely formatted. I think we need to give plaintext that way. If you select the HTML and copy paste, then that way you do get something nicely formatted, but then it's inconsistent with the other two methods...

I was thinking we should prioritize compatibility, consistency, and simplicity over nice formatting, and that markdown->HTML is awesome for looking nice but fails the other three criteria, whereas plaintext wins those 3.

Am I missing something here, a way to make markdown->HTML compatible, consistent, and simple? Or are there other criteria I'm not thinking of?

dionlarson commented 4 years ago

mailto: HTML/Rich Text support

iOS Mail and Android Gmail (default mail clients on most of these phones) support basic HTML (at least <br>, <i>, <b> I believe). You need <br> tags to get Gmail on Android to have new lines. Further testing would be required.

~If neither support HTML links currently, then I'm fully in support of enforcing plaintext email bodies.~

"Copy to Clipboard" Rich Text support

Getting rich text support on copy might actually be easier than rich text mailto: support on mobile!

Documented test of mailto: links

Bold, italic, link, %0a but no <br>

Bold, italic, link, %0a AND <br>

Bold, italic, link, <br> but no %0a

Bold, italic, link, <ul>, <br> but no %0a

Color & Size

Rich Text links are a huge win!

I'm pleasantly surprised to find both major mobile OS + default email clients work with <a> tags! I think this is a huge win for our templates going forward. We may even be able to find some ways to detect presence of compatible desktop clients.

NickCrews commented 4 years ago

Ok, on a cursory look over of your solutions markdown->HTML looks promising. I'll play around to confirm and then may try to implement something, though I may not get to it for another 24 hours. If someone else wants to hop in and try it please do!

NickCrews commented 4 years ago

I tried the mailto: links above on my Android phone and they worked beautifully! But now I'm trying them on Chrome on OSX, opening them in both gmail the Apple Mail app, and I'm getting Testing,<br>1,<br><b>2</b>,<br><i>3</i>,<br><br><a href="https://defund12.org">Link</a><br><br><ul><li>Item 1</li><li>Item 2</li></ul>. Any ideas on we should do about this? Try to detect the user's OS and give them different links? In which case, what content should I be getting on OSX Chrome?

NickCrews commented 4 years ago

@bingbongle @skullface @minicreative it might be good to get your opinions in here too.

NickCrews commented 4 years ago

In which case, what content should I be getting on OSX Chrome?

I think the best option would just be page.body | escape? Which wouldn't be terrible, but it is not consistent.