OrchardCore
OrchardCore copied to clipboard
Support both Text & HTML in the same email
/cc @MikeKry
@Mike4tel could you please try to send via EmailTask
all the 3 cases to ensure it was similar to the old behavior. Also, I might add unit tests too
@hishamco Thanks for looking at this. Code looks good and I have tested all 3 scenarios.
Html - OK Text - OK "All" (may be renamed to multipart/alternative or something) - Works OK (as seen below). Just one more thing. There is need to have two Body fields - text and html (similar as it was before). Because you need to have different formatting in text and html.
"All" (may be renamed to multipart/alternative or something)
All or Both make sense to me
There is need to have two Body fields - text and html (similar as it was before). Because you need to have different formatting in text and HTML.
This will revert the changes and let the body TextBody
& HtmlBody
@sebastienros are we fine to add two bodies into the MailMessage
or shall we let the sender decide if the body is HTML or Text
One option if we use MailMessageFormat.All
is to supply the HTML body and then stripe all the HTML tags to make it fine for the plain-text body. This way we can use a single body instead of two
Just pointing out that only stripping html will not let users format text nicely for plain text.. it is needed for good marketing.
But I can see using stripped html version as another feature just to fight spam filters.
Is it possible to merge this one? :)
We forgot this one :) I will do a final review and then merge
Maybe we should have an interface for the message like INotificationMessage which we already have. Or something similar
I don't think so, the issue was the email can be sent in both formats. In the past we already had IsHtml
but it didn't fit anymore, that's why I introduced something similar to the MimiKit mail body
This pull request has merge conflicts. Please resolve those before requesting a review.
This pull request has merge conflicts. Please resolve those before requesting a review.
@MikeKry could you please do a last test, because we made a massive changes in the OC.Email
module
This pull request has merge conflicts. Please resolve those before requesting a review.
It seems that this pull request didn't really move for quite a while. Is this something you'd like to revisit any time soon or should we close? Please comment if you'd like to pick it up.
I will revise this soon
@MikeKry could you please do one more test if you have time
This pull request has merge conflicts. Please resolve those before requesting a review.
This pull request has merge conflicts. Please resolve those before requesting a review.
for instructions on how to resolve the merge conflicts due to #16572 please follow the step listed in this comment.