Implement new layout for barong emails #187
Conversation
amir-budaychiev
changed the title
amir-budaychiev
changed the title
app/helpers/mailer_helper.rb
Outdated
| websites.each do |website| | ||
| logo_url = website.logo if host.include?(website.domain) | ||
| end | ||
| domain_logo_tag(logo_url) |
There was a problem hiding this comment.
domain_logo_tag loads white logo. So we either should load black logo or change background color to darker.
app/views/layouts/mailer.html.erb
Outdated
| </td> | ||
| <td class="container"> | ||
| <div class="content"> | ||
| <%= yield %> |
There was a problem hiding this comment.
Also we should change texts in emails a bit
|
@gfedorenko finished |
app/views/layouts/mailer.html.erb
Outdated
| </div> | ||
| </td> | ||
| </tr> | ||
| </tbody></table> |
app/views/layouts/mailer.html.erb
Outdated
| </td> | ||
| </tr> | ||
| </tbody></table> | ||
| </body> |
There was a problem hiding this comment.
Missing closing html tag
| <p>Hello <%= @resource.email %>!</p> | ||
| <p>Dear <%= @resource.email %>!</p> | ||
|
|
||
| <p>Thank you for using with CoinField.</p> |
There was a problem hiding this comment.
Replace all "CoinField" with ENV.fetch('APP_NAME', 'Barong')
There was a problem hiding this comment.
you can't fetch things everywhere.
There was a problem hiding this comment.
We can have a helper method
|
@gfedorenko recheck again please |
There was a problem hiding this comment.
almost there maybe there is a lighter way to express the style.
also we need to determine the application domain name I wonder if we can pass this context to the mailer
@dkkoval @amir-budaychiev
app/mailers/application_mailer.rb
Outdated
| @@ -1,6 +1,7 @@ | |||
| # frozen_string_literal: true | |||
|
|
|||
| class ApplicationMailer < ActionMailer::Base | |||
| default from: 'from@example.com' | |||
| # default from: 'from@example.com' | |||
There was a problem hiding this comment.
@mod Please, go to /config/initializers/devise.rb line 22
app/helpers/mailer_helper.rb
Outdated
|
|
||
| def determine_logo_url | ||
| host = confirmation_url(Account.first) # TODO: need way to determine a host correctly, just tip to me, but it works! | ||
| websites = Website.all |
There was a problem hiding this comment.
we should compare current application domain name and find it in website
we may want to make a fallback variable like APP_DOMAIN
app/views/layouts/mailer.html.erb
Outdated
| @@ -1,13 +1,58 @@ | |||
| <!DOCTYPE html> | |||
| <html> | |||
| <head> | |||
| <meta http-equiv="Content-Type" content="text/html; charset=utf-8" /> | |||
| <title>Barong Mailer</title> | |||
There was a problem hiding this comment.
We cannot write barong anywhere
and normally we need to use information from website like application name
| </style> | ||
| </head> | ||
|
|
||
| <body> | ||
| <%= yield %> | ||
| <table class="body-wrap"> |
There was a problem hiding this comment.
are you sure table is the best way to acheive this template ?
I was thinking just a few div.
There was a problem hiding this comment.
@mod It's best practice to working with tables when we building email templates, allows us to center elements and etc. You can find a lot of interesting information on this topic, for example:
https://litmus.com/blog/the-tyranny-of-tables-why-web-and-email-design-are-so-different
https://mailbakery.com/blog/how-email-html-coding-differs-from-web-html-coding/
There was a problem hiding this comment.
can we pass parameter in this way to devise emails ?
http://guides.rubyonrails.org/action_mailer_basics.html#sending-emails-with-dynamic-delivery-options
There was a problem hiding this comment.
@amir-budaychiev we need to pass the current hostname when the email sending was originated I doubt you understand how websites table works.
we match domain name please read our code regarding custom logo in layout.
…title, changed development settings
|
@mod I found an easier way for determine hostname. Variable URL_HOST already used in production settings, so we don't have to create other variable like APP_DOMAIN. We use this in the configuration settings the mailer config.action_mailer.default_url_options. |
app/helpers/mailer_helper.rb
Outdated
| end | ||
|
|
||
| def determine_logo_url | ||
| websites = Website.all |
| # Mailer proxy to send devise emails in the background | ||
| class DeviseBackgroundMailer | ||
|
|
||
| def self.confirmation_instructions(record, token, opts = {}) |
There was a problem hiding this comment.
This and below methods could be DRY-ed by
%i[confirmation_instructions reset_password_instructions ..].each { |method_name| define_singleton_method(method_name) do ..
|
@mod Now it works good and get request.domain value and pass it via Rails cache. So, it works without background job. At the moment this is the best solution that I could found by studying the devise documentation and source code of gem devise. |
Related with issue #174