Custom brand header logo in emails
What does this MR do?
This adds the custom brand image of a gitlab instance as the header logo in the pipeline emails (pipeline_success_email
and pipeline_failed_email
).
The gitlab logo in the email footer remains static.
I also extracted a layout from the pipeline emails. This allows for a unification of all other emails with the same email layout.
For now I only applied this to the 2 pipeline emails, but IMO it would make sense to use this new layout for all other emails as well, and therefore allow the use of the custom brand image for all emails.
Possible further work I could put into this MR
-
Change the admin section title from "Navigation bar:" to something more general as "Custom branding:"
Possible further work I could put into a separate MR
-
Apply the same email layout to all emails. Currently there are at least 3 different email styles: - Pipeline emails (which now use
app/views/layouts/mailer{html.haml,text.haml}
) - All notify emails except the 2 pipeline ones use
app/views/layouts/notify.html.haml
- All devise emails use
app/views/layouts/devise_mailer.html.haml
- Pipeline emails (which now use
-
Extract the inline styles from app/views/layouts/mailer{html.haml,text.haml}
and the pipeline emails and solve the premailer problems mentioned in https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6019 and make them available to all other emails.
To illustrate the update of all mails I updated the app/views/devise/mailer/password_change.html.haml
template with the extracted new mailer layout. This shows how minimal the updates could be as a first step towards updating and unifying all emails. (not in this MR included)
%tr
%td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;color:#333333;font-size:15px;font-weight:400;line-height:1.4;padding:15px 5px;text-align:center;" }
Hello, #{@resource.name}!
%p
The password for your GitLab account on
#{link_to(Gitlab.config.gitlab.url, Gitlab.config.gitlab.url)}
has successfully been changed.
%p
If you did not initiate this change, please contact your administrator
immediately.
The development of this MR is sponsored by @siemens (/cc @bufferoverflow).
Why was this MR needed?
The brand logo helps the users to identify the emails as they now use the same header logo as the gitlab instance.
Screenshots
Does this MR meet the acceptance criteria?
-
Changelog entry added -
Documentation created/updated -
API support added - Tests
-
Added for this feature/bug -
All builds are passing
-
-
Conform by the merge request performance guides -
Conform by the style guides -
Branch has no merge conflicts with master
(if it does - rebase it please) -
Squashed related commits together
What are the relevant issue numbers?
Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/24969
May partially fix https://gitlab.com/gitlab-org/gitlab-ce/issues/25570
Relates to (regarding unifying of mail templates) https://gitlab.com/gitlab-org/gitlab-ce/issues/25572
Merge request reports
Activity
marked the task Changelog entry added as completed
@dimitrieh Do you want to chime in? since it affects email layout.
I think this is great!
because (if i am correct):
- Custom branding to improve UX
- Standardise all email layouts
@mikegreiling as this is your expertise!
assigned to @mikegreiling
@jamedjo @mikegreiling please let us know where we can improve or extend this MR so @koffeinfrei can continue working on this.
@koffeinfrei In https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6019 there was some discussion about ensuring old images continue to display correctly. Will we need to copy
app/assets/images/mailers/ci_pipeline_notif_v1/gitlab-logo.gif
instead of moving/renaming it for that to happen?- Resolved by James Edwards-Jones
@koffeinfrei If someone uploads a particularly large image how will this display in the email? Is it possible that some users have use an image for the header which will be too wide for some email clients?
I'd like to second @jamedjo's comment about renaming the email image path. We definitely need to keep these at their current location in order to prevent old emails from having missing images. Please make a copy of them if you need to reference them in a different location or simply use their current asset path.
In https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6019 there was some discussion about ensuring old images continue to display correctly. Will we need to copy
app/assets/images/mailers/ci_pipeline_notif_v1/gitlab-logo.gif
instead of moving/renaming it for that to happen?
I'd like to second @jamedjo's comment about renaming the email image path. We definitely need to keep these at their current location in order to prevent old emails from having missing images. Please make a copy of them if you need to reference them in a different location or simply use their current asset path.
I see. I'll copy them for now. I believe this is not a sustainable solution though, as it makes maintenance very hard. I'd suggest considering one of the following:
- Break old references, as the gitlab emails are usually one-time emails that aren't kept.
- Don't use the rails asset pipeline and place them in /public instead. When renaming / moving images we could use http redirects.
If someone uploads a particularly large image how will this display in the email? Is it possible that some users have use an image for the header which will be too wide for some email clients?
Possibly yes. Should I restrict the image size to be of 50px height? This is the same height as the default email header image uses.
If someone uploads a particularly large image how will this display in the email? Is it possible that some users have use an image for the header which will be too wide for some email clients?
Possibly yes. Should I restrict the image size to be of 50px height? This is the same height as the default email header image uses.
@koffeinfrei That could work. I don't have much of an opinion either way other than to test it and see what it look like in different mail clients. I saw that @mikegreiling used a Litmus trial, screenshot in https://gitlab.com/gitlab-com/marketing/issues/495, which could be helpful.
added 1151 commits
-
8c0783a2...b08be165 - 1146 commits from branch
gitlab-org:master
- 11377207 - extract pipeline mails layout
- c1c98ebd - use custom brand logo in pipeline mails
- 214c095f - add documentation for custom brand logo in email
- 264c47cf - add changelog entry
- 029f383d - restrict height of the custom brand logo in emails
Toggle commit list-
8c0783a2...b08be165 - 1146 commits from branch
marked as a Work In Progress from siemens/gitlab-ce@f034f6b3
That could work. I don't have much of an opinion either way other than to test it and see what it look like in different mail clients. I saw that @mikegreiling used a Litmus trial, screenshot in https://gitlab.com/gitlab-com/marketing/issues/495, which could be helpful.
I don't have access to that issue. I added a
max-height
to the image now. I added an inline style for now, as the whole new email layout is done with inline styles too.added 1 commit
- 50b11757 - restrict height of the custom brand logo in emails
@koffeinfrei Yes, I agree that is unexpected and annoying. There is an issue open to address this bug here: #27971 (moved)
- Resolved by Alexis Reigel
assigned to @koffeinfrei
added 1 commit
- eb08378f - restrict height of the custom brand logo in emails
added 190 commits
-
eb08378f...5f3f6ee6 - 185 commits from branch
gitlab-org:master
- c607c774 - extract pipeline mails layout
- 4e248445 - use custom brand logo in pipeline mails
- c51c312b - add documentation for custom brand logo in email
- 997ef554 - add changelog entry
- f7930061 - restrict height of the custom brand logo in emails
Toggle commit list-
eb08378f...5f3f6ee6 - 185 commits from branch
assigned to @smcgivern
- Resolved by Alexis Reigel
@koffeinfrei thanks, I just have one comment! Nice change!
@mikegreiling what do we need to do for the EE approvals mail here? That should probably be done by someone at GitLab.
assigned to @koffeinfrei
@smcgivern once this is merged I can open a separate MR to address the EE changes. It's not going to break EE without it, it's merely a place where we can reduce code duplication.
changed milestone to %9.0
@mikegreiling done!
Thanks @koffeinfrei!
assigned to @smcgivern
mentioned in merge request !10483 (merged)
mentioned in issue #15661 (closed)
added devopsplan label
mentioned in issue #30760 (closed)
mentioned in merge request gitlab!17699 (merged)