Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch the feedback emails to HTML. #2675

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

drgrice1
Copy link
Member

@drgrice1 drgrice1 commented Feb 16, 2025

The email content is generated by the templates/ContentGenerator/Feedback/feedback_email.html.ep template. All of the same content is there, just arranged nicely into tables and such. The link to the page the user sent the email from is of course an a href. For now it reads "To visit the page from which (user name) sent feedback, click here." where "click here" is the link. Note that it used to read "To visit the page from which the user sent feedback, long link." We have the user name so why not use it here?

Also, when the sent message is displayed for the user in the browser, the message is formated much the same as it now is in the email. That is it is not wrapped and placed in a pre tag. Instead it is in a bordered div formatted with essentially the same style as in the email.

Here is a screen shot of the email with the default verbosity of 1. At least what I could fit in. Below what is show are the tables for "Data about the problem" and "Data about the homework set". They look much the same as the tables shown.
feedback-email

I would like to reorder the tables. I would like to have "Data about the problem" first (assuming the email is sent from a problem page) since that is the information that is most useful in the case that a user is asking about a problem. Then "Data about the homework set" next. Then "Data about the user". The user data is almost all shown above in the other content or the subject anyway, so it doesn't need to be so high. Then the "Data about the problem processor" last. I generally consider that information mostly useless. It definitely should not be at the top, and I would even vote for not even including it in the default verbosity.

@somiaj
Copy link
Contributor

somiaj commented Feb 17, 2025

Isn't "click here" considered bad practice for an accessible link? To me I don't see needing two links, one to the webwork server and one to the page the email was sent from, could that be combined into a single link to the page the email was sent from?

@drgrice1
Copy link
Member Author

As I said, the "click here" is for now. Yes, the "click here" is bad practice, and I was hoping someone would offer a better suggestion.

The webwork server is not technically a link. It is just a URL. However, most email clients present any URL as a link even if they are not a hrefed. That was in the email before. I didn't change that.

@Alex-Jordan
Copy link
Contributor

Alex-Jordan commented Feb 17, 2025 via email

@somiaj
Copy link
Contributor

somiaj commented Feb 17, 2025

Thanks, I am trying to think of suggestions but can't think of anything I'm sold on. But I am thinking something along the lines of combining the two lines into a single line something like:

Message from $first_name $last_name ($username) via <this WeBWorK page>.

I was also considering we could put the institution there, but still can't think of any phrasing I'm sold on.

@drgrice1
Copy link
Member Author

At this point this pull request does not change the content of the message (at least not in any significant way). It just reformats the content from the previous text email to html. So at this point, consider the pull request's reformatting only. Further changes to wording, reorganization, etc., can be done in a later pull request as @Alex-Jordan said.

@Alex-Jordan
Copy link
Contributor

Do we need to care about faculty who use an email client that won't process HTML in email? I notice that now what is sent is not very human readable. In addition to the HTML you get a bunch of =09 in the message for the tabs that are used to indent the HTML tags.

@drgrice1
Copy link
Member Author

@Alex-Jordan: Thanks for reminding me about a text fallback. I meant to do that, but forgot. I have added that now.

The text message is generated by the templates/ContentGenerator/Feedback/feedback_email.txt.ep template. The text message is essentially the same as the prior text email. If your email client is set to show text, then you will see this message. If your client is set to show HTML, then you will see the new HTML message. If your client does not support HTML, then you will see the text message.

@drgrice1 drgrice1 force-pushed the feedback-email-html branch 2 times, most recently from eb360ea to bf557ac Compare February 18, 2025 01:53
@pstaabp
Copy link
Member

pstaabp commented Feb 18, 2025

First @drgrice1, love the feedback message sent from your fake student. Pure honesty.

Overall, this looks great. How about something along the lines of: STUDENT NAME sent feedback about THIS WeBWorK PAGE

where the end is the link. I was thinking "this WeBWorK problem", but there are non-problem related pages that students can email about.

@drgrice1
Copy link
Member Author

I changed the line with the "click here" link to

<%= $user->first_name %> sent feedback from <%= link_to 'this page' => $emailableURL %>.

Lets go with that for now. The previous line is still the same. This can be tweaked in a later pull request if desired, but I think for this pull request focus on the HTML/text functionality aspects.

The email content is generated by the `templates/ContentGenerator/Feedback/feedback_email.html.ep`
 template. All of the same content is there, just arranged nicely into
tables and such. The link to the page the user sent the email from is of
course an `a href`.  For now it reads "To visit the page from which
(user name) sent feedback, click here." where "click here" is the link.
Note that it used to read "To visit the page from which the user sent
feedback, long link."  We have the user name so why not use it here?

Also, when the sent message is displayed for the user in the browser,
the message is formated much the same as it now is in the email.  That
is it is not wrapped and placed in a `pre` tag.  Instead it is in a
bordered `div` formatted with essentially the same style as in the
email.
The text message is generated by the `templates/ContentGenerator/Feedback/feedback_email.txt.ep` template.
The text message is essentially the same as the prior text email. If
your email client is set to show text, then you will see this message.
If your client is set to show HTML, then you will see the new HTML
message. If your client does not support HTML, then you will see the
text message.
It is now

```
<%= $user->first_name %> sent feedback from <%= link_to 'this page' => $emailableURL %>.
```
@drgrice1 drgrice1 force-pushed the feedback-email-html branch from 703d4e9 to fc8515b Compare February 18, 2025 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants