Skip to content
This repository has been archived by the owner on Mar 2, 2020. It is now read-only.

Remove nil from format options #61

Closed
wants to merge 2 commits into from

Conversation

brandonhilkert
Copy link

When the format is not specified, the formats array includes nil [nil, "text/html", "text/plain"]. find_part with a nil format will bring back the plain text version. So if a format isn't specified, we remove before we find the preferred format.

@brandonhilkert
Copy link
Author

Fixes #60

@thinkswan
Copy link

+1

1 similar comment
@patrickberkeley
Copy link

👍

@jeremy jeremy mentioned this pull request Dec 11, 2013
@jeremy
Copy link
Member

jeremy commented Dec 11, 2013

Sounds good. Test case?

@brandonhilkert
Copy link
Author

Added.

@jeremy
Copy link
Member

jeremy commented Dec 11, 2013

Hm, I misunderstood. A nil format is meaningful. That asks for the multipart/alternative's default part, rather than saying 'text/html' please.

This way, you're seeing the part ordering that you set up in the message. If you're seeing text default but want html, reverse the order of the parts.

@jeremy jeremy closed this Dec 11, 2013
@brandonhilkert
Copy link
Author

It might be outside the context of this PR, but is there a way to change the order of the parts in ActionMailer?

On Wednesday, December 11, 2013, Jeremy Kemper wrote:

Hm, I misunderstood. A nil format is meaningful. That asks for the
multipart/alternative's default part, rather than saying 'text/html' please.

This way, you're seeing the part ordering that you set up in the message.
If you're seeing text default but want html, reverse the order of the parts.


Reply to this email directly or view it on GitHubhttps://github.com//pull/61#issuecomment-30358919
.


http://brandonhilkert.com

@brandonhilkert
Copy link
Author

Disregard my previous question, I found it in the Rails docs. I understand the need for nil now. I'm personally using mail_vew to view the HTML part of my emails. I'm using premailer to automatically compose the text portion, and while it's not perfect, I trust it to get close. So my reasoning for this change was to show the HTML part by÷default. Given that the default order of ActionMailer doesn't put the HTML portion first, I personally think it'd be nice to designate, maybe in a config, the HTML portion be the default. So when I click the email name link, it takes me to straight to the ".html" view.

Is that something you would be interested in supporting? If so, I can probably find some time to make the change.

@thinkswan
Copy link

Is the HTML view not already the default view? When I click the links at /mail_view, I see the HTML email first.

@brandonhilkert
Copy link
Author

It's not for me, but now it makes me wonder if it's something premailer is
doing becaUse it's the source of my plain text view.

On Wednesday, December 11, 2013, Graham Swan wrote:

Is the HTML view not already the default view? When I click the links at
/mail_view, I see the HTML email first.


Reply to this email directly or view it on GitHubhttps://github.com//pull/61#issuecomment-30363148
.


http://brandonhilkert.com

@patrickberkeley
Copy link

@brandonhilkert I'm using premailer as well and HTML is not the default view.

@brandonhilkert
Copy link
Author

Ok. Thanks for the feedback. I'll go poke around premailer then and try to
fix it there.

On Wednesday, December 11, 2013, Patrick Berkeley wrote:

@brandonhilkert https://github.com/brandonhilkert I'm using premailer
as well and HTML is not the default view.


Reply to this email directly or view it on GitHubhttps://github.com//pull/61#issuecomment-30373082
.


http://brandonhilkert.com

@brandonhilkert
Copy link
Author

Just as follow up, I've confirmed it is premailer that is prepending the text portion, thus making it the default format. I have submitted a PR: fphilipe/premailer-rails#97

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants