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

feat: Switch to mjml to handle email templates #1527

Open
wants to merge 24 commits into
base: master
Choose a base branch
from

Conversation

arsforza
Copy link
Contributor

@arsforza arsforza commented Jul 19, 2024

closes #1515

Describe your changes

  • Install mjml
  • Migrate to object structure
  • Test for multiple disasters

Checklist before requesting a review

  • I have performed a self-review of my code
  • I have added tests wherever relevant
  • I have made sure that all automated checks pass before requesting a review

@gulfaraz gulfaraz mentioned this pull request Jul 24, 2024
3 tasks
@arsforza arsforza force-pushed the feat.email-mjml branch 2 times, most recently from aa3edd0 to e623b39 Compare August 1, 2024 10:01
@arsforza arsforza requested a review from gulfaraz August 23, 2024 14:23
Copy link
Member

@gulfaraz gulfaraz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed over a call with @arsforza

Change Requests

  • Breakpoint - email should take full width in mobile
  • Font - use the same font as in the emails before this change
  • Button border-radius - button-radius does not work in outlook
  • Add space between button and text in mobile where the button and text is stacked instead of next to each other
  • Add space after Advisory in the event text
  • In event text it reads "from nows from now"
  • Button labels are different (maybe this gets fixed after merge to master)
    • Open IBF
    • Join WhatsApp
    • About Trigger
  • Mailchimp footer does not match the old emails
  • Do a manual comparison of the copy from the emails generated in the master branch

Relevant Links

@gulfaraz
Copy link
Member

@arsforza I missed one change in our review call. Let's use date-fns instead of moment/luxon. Refer to this comment for context.

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.

[Task] Switch to mjml to handle email templates
2 participants