-
-
Notifications
You must be signed in to change notification settings - Fork 166
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
base: develop
Are you sure you want to change the base?
Conversation
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? |
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 |
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:
I was also considering we could put the institution there, but still can't think of any phrasing I'm sold on. |
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. |
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 |
@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 |
eb360ea
to
bf557ac
Compare
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. |
I changed the line with the "click here" link to
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 %>. ```
703d4e9
to
fc8515b
Compare
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 ana 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 bordereddiv
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.

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.