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

Enhance Update Email Layout #199

Open
VaarunSinha opened this issue Jun 20, 2024 · 16 comments
Open

Enhance Update Email Layout #199

VaarunSinha opened this issue Jun 20, 2024 · 16 comments
Assignees

Comments

@VaarunSinha
Copy link

Currently all update emails look like this:

image

I want to make the emails have a HTML Table, with a fallback to this text version.

I can work on this and submit a PR, if this is approved.

Please let me know what you all think.

@sabderemane
Copy link
Member

Hey @VaarunSinha 👋🏽

I think you can't have an HTML table really, the template used by Trac is a text file, you can customise it but that's all (ref. the documentation)

IMO, best would be to change the table in the text file to have a something to fit in the email even if it's a phone. Currently the table from the text file seems to be defined for desktop only.

@VaarunSinha
Copy link
Author

https://trac-hacks.org/wiki/TracHtmlNotificationPlugin -- We can use this plugin

I will first try optimizing the texts more to look like a complete table. If that works then we can do the same.

@VaarunSinha
Copy link
Author

I can't find the email template in codebase please guide so that I can try that change out.

And let me know your thoughts on this Plugin.

@bmispelon
Copy link
Member

Hi,

I do agree that this email is not the best to look at and I would like to see improvements on it too.
As far as I can tell, this is not something that we configured, so the content and layout of the email comes straight from Trac.

About the plugin, I haven't tested it but it does seem a bit maintained at least (last commit was 2 years ago which is pretty recent by Trac's standards). Still, I'm wary about adding any dependencies to Trac because of the risk that they could block a future upgrade.

I've done a quick search and found that Trac has a configuration options for the format of the email, with a default of text/plain: https://trac.edgewall.org/wiki/TracNotification#ConfigurationOptions. I'm going to try and see how it looks like when setting it to text/html. If that works, I would be in favor of that rather than a plugin.

Thanks for flagging this up 🌞

@bmispelon
Copy link
Member

I've spent some time experimenting with the TracHtmlNotificationPlugin on my local version and it does seem to work but I've found two major issues (one of which being a blocker for me):

  1. The styling of the email uses Trac's default color (yellow background, red text, lots of drop shadows) and is not easy to customize.
  2. The notification email includes the CC list of the ticket in the clear, instead of the usual example@... that Trac normally shows. I've submitted a bug report to the project for this: https://trac-hacks.org/ticket/14346

Issue 2) is a blocker for me (because it leaks confidential information about our users without their consent), and I would not install this plugin until the bug is fixed. Issue 1) is not a deal-breaker but it's a big negative for me. I don't mind the email not looking exactly like the site, but the colors should at least match.

@VaarunSinha
Copy link
Author

I will try to customise the email and also see to the user information issue if there's an extra configuration which is the reason for this behaviour.

Also let me see if the "text/html" option works.

Will let you know after experimenting.

@bmispelon
Copy link
Member

A quick update on my end:

  • the issue with cc emails being leaked was fixed, so that's not a blocker anymore
  • I've done some experiments and the text/html option only works if you have the plugin installed (otherwise notifications just aren't sent, with no error message)

Just so you're prepared, this level of experimenting with Trac can be challenging. To test things out I've installed a local version of mailhog (https://github.com/mailhog/MailHog) and changed the smtp_port configuration option in trac-env/conf/trac.ini (pointint it to mailhog's defaullt of 1025). I've also had to replace configuration options pointing to code.djangoproject.com and use my localhost:9000 running version.

If you have questions, you can always tag me in the #contributor-discussion in the Django discord. Good luck 😄

@VaarunSinha
Copy link
Author

So what are the further steps? Do I integrate the plugin and make a PR or have a discussion on whether or not it's worth adding a new plugin?

@bmispelon
Copy link
Member

My point 2 from my previous comment still stands: if we were to include the plugin, I would like the emails to use the custom Django colors (and styles as much as possible, but colors is a minimum).

Based on my quick experiment with the plugin, this won't be an easy task, especially if we want to do it in a sustainable manner where we don't duplicate the styles we already have in scss/*scss (which themselves were already duplicated from a previous version of the djangoproject.com styles).

The next steps would be for you (or any other contributor) to get the plugin installed locally and find a way to change the styles in the email. Then open a PR, ideally with some screenshots. Does that make sense?

@VaarunSinha
Copy link
Author

Got it that makes sense! Please assign the issue to me and I will take care of setting up the plugin and coding the HTML Notification Email.

@bmispelon
Copy link
Member

Please assign the issue to me

Done ✅

For the record, you're always free to work on any tickets in this project, even if they're not assigned to you. You can post a comment when you start on something, just to let us know and so that there's no conflicts.

@VaarunSinha
Copy link
Author

I am confused on how to setup and run the trac instance. The docker runs fine but I think it only runs postgres? Please guide me.

Till then I am writing HTML and Basic CSS for the email. Once that's complete I will setup Trac and customise the plugin with the HTML and the styles.

@VaarunSinha
Copy link
Author

I am done working on the HTML Emails here are my proposed designs:

Design 1 : https://app.postdrop.io/share/K11L2Qs6bukI2b87uAHW2vnp-YWUr9wzdn7O (uses 4 column approach)

Looks amazing on Desktop, but on mobile it overflows gracefully.

Design 2 : https://app.postdrop.io/share/XYIwZZVMoPdOiNZN1VTX-H2IYndtsBjqXwXg (uses 2 column approach)

Looks good on Desktop and better on mobile as nothing overflows. But makes the Desktop mail longer due to only 2 columns being there.

I could have used media queries but they aren't that widely supported on all mail clients yet.

Let me know if you like my HTML Emails and which one we should go with.

@VaarunSinha
Copy link
Author

https://forum.djangoproject.com/t/sending-html-emails-in-trac-notification/32520

I have opened a forum discussion for the designs as well.

@bmispelon
Copy link
Member

Hi again,

I am confused on how to setup and run the trac instance. The docker runs fine but I think it only runs postgres? Please guide me.

Are you following the instructions in the README? Which steps have you done, and which one are you stuck on exactly?

Thanks for the email designs, they seem promising. One thing you should watch out for is that there might be limitations in Trac in terms of what HTML structure we can work with. From what I've seen in my experiment, the plugin you linked to reuses the same template for the email as the one used for the ticket page on the site, so you can't customize the HTML that much. Before you put in too much work on the design, it would maybe be good to make sure that it's technically feasible to use it within Trac.

@VaarunSinha
Copy link
Author

I have setup trac installed the plugin and also setup the mailhog. Even after setting the preference to text/html I am receiving text notifications inside of mailhog.

@bmispelon

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

No branches or pull requests

3 participants