Skip to content
This repository has been archived by the owner on Aug 27, 2022. It is now read-only.

Add HTML capability to E-Mail text #231

Merged
merged 3 commits into from
May 4, 2021
Merged

Add HTML capability to E-Mail text #231

merged 3 commits into from
May 4, 2021

Conversation

Raukze
Copy link

@Raukze Raukze commented Apr 6, 2021

Prerequisites checklist

What is the purpose of this pull request? (put an "x" next to an item)

  • Documentation update
  • Bug fix
  • New feature
  • Other, please explain:

What changes did you make? (Give an overview)

I added the capability of the text field of the E-Mail being in HTML format. The configuration option can be found under advanced options in the E-Mail section. It is turned off by default.

Is there anything you'd like reviewers to focus on?

This is my first ever PR so please let me know if there is anything I did wrong. 🙂

@jacques42
Copy link
Collaborator

Hi @Raukze - thanks for submitting this PR, we very much do appreciate your contribution. This enhancement is an excellent idea and we would be very happy to take it into Photobooth. On a first glance all changes look perfectly well. Give me a few days please, in order to find time and try things out in detail.

AltBody replaces the Body if it's an HTML email and the receivers email client does not support HTML
@Raukze
Copy link
Author

Raukze commented Apr 9, 2021

Today I found this article where in the paragraph "Create an HTML email message" they say that you should always set $mail->AltBody when sending an HTML-Mail. This serves as a fallback for when the mail client on the receiving end isn't capable of displaying HTML-Mails. These changes are adressed in f71d724.

There are a few things that need to be thought through further. In the case that someone has enabled the HTML email option but has not filled the alternative text, an empty body would be displayed for email clients that cannot display HTML emails. As I see it, there are at least three options here:

  1. the HTML email option is enabled, but no alternative text is filled, just the body text is used (even if it contains HTML tags).
  2. the function msgHTML() of PHPMailer which automatically converts HTML to plain text
  3. leave it as it is currently: if the user doesn't insert any text, it is assumed that this is what he wanted and the email is empty in this case.

My personal opinion on this is that users who are interested in using HTML email functionality will look through all the configurable options and if they want to set an alternative text they will. The question is also how many cases there really are where the user's email client cannot display HTML emails...

@andi34
Copy link
Owner

andi34 commented Apr 9, 2021

Hey and thanks for your contribution!
I think we should either

  • set msgHTML() if $config['mail']['alt_text']; is empty

Or

  • disable $config['mail']['is_html']; if $config['mail']['alt_text']; is empty

(Sorry for closing the PR by accident, reopened)

@andi34 andi34 closed this Apr 9, 2021
@andi34 andi34 reopened this Apr 9, 2021
@andi34
Copy link
Owner

andi34 commented Apr 9, 2021

A fallback in empty case could be added like here: 3017d15#diff-24a73b9c7159dc18b2595e0c6a5948231af591bdbc9df8e895666d213f039b3f

- only use alt_text if we can assure that it is set
- use msgHTML() if email type is HTML and alt_text is not set
- in all other cases just use the regular text as body
@Raukze
Copy link
Author

Raukze commented Apr 9, 2021

Hey, thanks for the suggestion @andi34
I decided to go the msgHTML() route. I tested it and it looks pretty promising.

@andi34
Copy link
Owner

andi34 commented Apr 25, 2021

Thanks @Raukze , sorry I haven't had time yet to test but will try to test soon!
Could you please run yarn format once and commit the resulting formatting changes? This will help to keep one code style :)

@Raukze
Copy link
Author

Raukze commented Apr 30, 2021

Being honest with you I have never done this before and I tried to figure it out by myself but without any success. I'm willing to help if you could give me a small hint or an instruction on how to do it that'd be great. Thanks :)

@jacques42
Copy link
Collaborator

Run the following command in your root folder of photobooth

yarn format

which will reformat the code to a standard formatting. As this might do changes to the code, then commit those changes into your branch.

@Raukze
Copy link
Author

Raukze commented May 2, 2021

Thanks I was able to run the command. But I'm quite new to git and don't know how I get a new commit with the changes into this PR because I deleted my fork. Any help is appreciated.

@andi34
Copy link
Owner

andi34 commented May 2, 2021

Don't worry, then we'll run it once it's merged.

@andi34 andi34 merged commit e805fe1 into andi34:dev May 4, 2021
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.

3 participants