Skip to content
Snippets Groups Projects

Custom brand header logo in emails

Merged Alexis Reigel requested to merge siemens/gitlab-ce:feature/brand-logo-in-emails into master
All threads resolved!

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
  • 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.

PNG_Image__747___469_pixels

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

custom_brand_email

Does this MR meet the acceptance criteria?

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

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • @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.

  • Author Contributor

    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.
  • Author Contributor

    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.

  • Alexis Reigel added 1151 commits

    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

    Compare with previous version

  • Alexis Reigel marked as a Work In Progress from siemens/gitlab-ce@f034f6b3

    marked as a Work In Progress from siemens/gitlab-ce@f034f6b3

  • Author Contributor

    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.

  • Alexis Reigel unmarked as a Work In Progress

    unmarked as a Work In Progress

  • Author Contributor

    (That's interesting, my MR was marked as "Work In Progress" - i.e. the title was changed to "WIP: ..." because I rebased to master which contained new commits marked as "WIP")

  • Alexis Reigel added 1 commit

    added 1 commit

    • 50b11757 - restrict height of the custom brand logo in emails

    Compare with previous version

  • @koffeinfrei Yes, I agree that is unexpected and annoying. There is an issue open to address this bug here: #27971 (moved)

  • This LGTM :ok_hand:

    If we're extracting the layout of the pipeline emails into layouts/mailer.html.haml we ought to use this for the merge request approval notification in EE as well since it uses pretty much the same header and footer.

  • Alexis Reigel added 1 commit

    added 1 commit

    • eb08378f - restrict height of the custom brand logo in emails

    Compare with previous version

  • Alexis Reigel resolved all discussions

    resolved all discussions

  • Alexis Reigel added 190 commits

    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

    Compare with previous version

  • Sean McGivern
  • @koffeinfrei thanks, I just have one comment! Nice change! :tada:

    @mikegreiling what do we need to do for the EE approvals mail here? That should probably be done by someone at GitLab.

  • Alexis Reigel resolved all discussions

    resolved all discussions

  • Alexis Reigel added 5 commits

    added 5 commits

    • f0766fde - extract pipeline mails layout
    • 0df104a3 - use custom brand logo in pipeline mails
    • 84420c89 - add documentation for custom brand logo in email
    • 26da2c45 - add changelog entry
    • c1e94479 - restrict height of the custom brand logo in emails

    Compare with previous version

  • @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.

  • Sean McGivern changed milestone to %9.0

    changed milestone to %9.0

  • merged

  • Alexis Reigel mentioned in merge request !10483 (merged)

    mentioned in merge request !10483 (merged)

  • mentioned in issue #15661 (closed)

  • mentioned in issue #30760 (closed)

  • Diego Louzán mentioned in merge request gitlab!17699 (merged)

    mentioned in merge request gitlab!17699 (merged)

  • Please register or sign in to reply
    Loading