From 5e88a2bcedc112c83fe6f7321f7c67fd3008432c Mon Sep 17 00:00:00 2001 From: Steve Mokris Date: Tue, 25 Jan 2022 01:53:40 +0000 Subject: [PATCH] Improve email notification styling Add `enhanced_notify_css` feature flag. When enabled, use the main GitLab UI framework to style email notifications. Changelog: changed --- app/assets/stylesheets/framework/mixins.scss | 6 ++ app/assets/stylesheets/notify.scss | 24 +------ app/assets/stylesheets/notify_base.scss | 25 +++++++ app/assets/stylesheets/notify_enhanced.scss | 68 +++++++++++++++++++ app/views/layouts/notify.html.haml | 5 +- app/views/layouts/service_desk.html.haml | 5 +- app/views/notify/_note_email.html.haml | 4 +- app/views/notify/issue_due_email.html.haml | 4 +- app/views/notify/new_issue_email.html.haml | 4 +- .../notify/new_merge_request_email.html.haml | 2 +- app/views/notify/new_release_email.html.haml | 2 +- .../service_desk_new_note_email.html.haml | 2 +- config/application.rb | 1 + .../development/enhanced_notify_css.yml | 8 +++ ...add_merge_request_approver_email.html.haml | 2 +- ee/app/views/notify/new_epic_email.html.haml | 4 +- 16 files changed, 129 insertions(+), 37 deletions(-) create mode 100644 app/assets/stylesheets/notify_base.scss create mode 100644 app/assets/stylesheets/notify_enhanced.scss create mode 100644 config/feature_flags/development/enhanced_notify_css.yml diff --git a/app/assets/stylesheets/framework/mixins.scss b/app/assets/stylesheets/framework/mixins.scss index 1e51bf3d974e88..1caf02937d55d2 100644 --- a/app/assets/stylesheets/framework/mixins.scss +++ b/app/assets/stylesheets/framework/mixins.scss @@ -439,6 +439,12 @@ .na { color: inherit; } + + // Rouge `Comment` token (quoted text in email body) + .c { + color: $gl-grayish-blue; + font-style: italic; + } } } } diff --git a/app/assets/stylesheets/notify.scss b/app/assets/stylesheets/notify.scss index feb4ea77e588a5..c433d8298809e9 100644 --- a/app/assets/stylesheets/notify.scss +++ b/app/assets/stylesheets/notify.scss @@ -1,24 +1,4 @@ -@import 'framework/mixins'; -@import 'framework/variables'; - -img { - max-width: 100%; - height: auto; -} - -p.details { - font-style: italic; - color: $gray-500; -} - -.footer > p { - font-size: small; - color: $gray-500; -} - -pre.commit-message { - white-space: pre-wrap; -} +@import 'notify_base'; .gl-label-scoped { border: 2px solid currentColor; @@ -47,6 +27,4 @@ pre.commit-message { border: 1px solid $gray-100; border-radius: $border-radius-small; } - - @include email-code-block; } diff --git a/app/assets/stylesheets/notify_base.scss b/app/assets/stylesheets/notify_base.scss new file mode 100644 index 00000000000000..8c6f9a27077ab6 --- /dev/null +++ b/app/assets/stylesheets/notify_base.scss @@ -0,0 +1,25 @@ +@import 'framework/mixins'; +@import 'framework/variables'; + +img { + max-width: 100%; + height: auto; +} + +p.details { + font-style: italic; + color: $gray-500; +} + +.footer > p { + font-size: small; + color: $gray-500; +} + +pre.commit-message { + white-space: pre-wrap; +} + +.content { + @include email-code-block; +} diff --git a/app/assets/stylesheets/notify_enhanced.scss b/app/assets/stylesheets/notify_enhanced.scss new file mode 100644 index 00000000000000..5df5a8592bf186 --- /dev/null +++ b/app/assets/stylesheets/notify_enhanced.scss @@ -0,0 +1,68 @@ +// Import a subset of the GitLab UI framework: +// keep parts that affect elements that can appear in emails; +// omit Bootstrap Reboot since it adds unnecessary styles to every element. +@import 'notify_base'; +@import 'bootstrap/scss/functions'; +@import 'bootstrap/scss/variables'; +@import 'bootstrap/scss/mixins'; +@import 'bootstrap/scss/code'; +@import '@gitlab/ui/src/scss/variables'; +@import '@gitlab/ui/src/scss/utility-mixins/index'; +@import '@gitlab/ui/src/scss/components'; +@import 'bootstrap_migration'; +@import 'framework/common'; +@import 'framework/gfm'; +@import 'framework/kbd'; +@import 'framework/tables'; +@import 'framework/typography'; +@import 'framework/emojis'; + +body { + font-family: $regular-font; + font-size: inherit; +} + +a { + text-decoration: none; +} + +.content { + .md { + padding: 1rem 0; + } + + hr { + border: 1px solid #e1e1e1; + } + + blockquote { + border-top-width: 0; + border-bottom-width: 0; + border-right-width: 0; + + &:dir(rtl) { + border-left-width: 0; + border-right-width: inherit; + } + } + + table { + border-collapse: collapse; + } + + .diff-table.code, + table.code { + width: auto; + + td { + padding: inherit; + + pre { + background-color: inherit; + margin: 0; + padding: 0; + border: inherit; + } + } + } +} diff --git a/app/views/layouts/notify.html.haml b/app/views/layouts/notify.html.haml index e922b505be86a9..3b979f69cac7b6 100644 --- a/app/views/layouts/notify.html.haml +++ b/app/views/layouts/notify.html.haml @@ -3,7 +3,10 @@ %meta{ content: "text/html; charset=utf-8", "http-equiv" => "Content-Type" } %title GitLab - = stylesheet_link_tag 'notify' + - if Feature.enabled?(:enhanced_notify_css) + = stylesheet_link_tag 'notify_enhanced' + - else + = stylesheet_link_tag 'notify' = yield :head %body .content diff --git a/app/views/layouts/service_desk.html.haml b/app/views/layouts/service_desk.html.haml index 26d15a74403218..a838ba91d261a6 100644 --- a/app/views/layouts/service_desk.html.haml +++ b/app/views/layouts/service_desk.html.haml @@ -5,7 +5,10 @@ %title GitLab -# haml-lint:enable NoPlainNodes - = stylesheet_link_tag 'notify' + - if Feature.enabled?(:enhanced_notify_css) + = stylesheet_link_tag 'notify_enhanced' + - else + = stylesheet_link_tag 'notify' = yield :head %body .content diff --git a/app/views/notify/_note_email.html.haml b/app/views/notify/_note_email.html.haml index ad0c873bf56c0e..5598447204720b 100644 --- a/app/views/notify/_note_email.html.haml +++ b/app/views/notify/_note_email.html.haml @@ -25,11 +25,11 @@ = content_for :head do = stylesheet_link_tag 'mailers/highlighted_diff_email' - %table + %table.code = render partial: "projects/diffs/email_line", collection: discussion.truncated_diff_lines(diff_limit: diff_limit), as: :line, locals: { diff_file: discussion.diff_file } -%div{ style: note_style } +.md{ style: note_style } = markdown(note.note, pipeline: :email, author: note.author, current_user: @recipient, issuable_reference_expansion_enabled: true) diff --git a/app/views/notify/issue_due_email.html.haml b/app/views/notify/issue_due_email.html.haml index c9cd9c32b5474f..e512d7732e2c59 100644 --- a/app/views/notify/issue_due_email.html.haml +++ b/app/views/notify/issue_due_email.html.haml @@ -8,5 +8,5 @@ This issue is due on: #{@issue.due_date.to_s(:medium)} - if @issue.description - %div - = markdown(@issue.description, pipeline: :email, author: @issue.author, current_user: @recipient, issuable_reference_expansion_enabled: true) + .md + = markdown(@issue.description, pipeline: :email, author: @issue.author, current_user: @recipient, issuable_reference_expansion_enabled: true) diff --git a/app/views/notify/new_issue_email.html.haml b/app/views/notify/new_issue_email.html.haml index 439604a950a7a3..592b3f453af5ac 100644 --- a/app/views/notify/new_issue_email.html.haml +++ b/app/views/notify/new_issue_email.html.haml @@ -7,5 +7,5 @@ = assignees_label(@issue) - if @issue.description - %div - = markdown(@issue.description, pipeline: :email, author: @issue.author, current_user: @recipient, issuable_reference_expansion_enabled: true) + .md + = markdown(@issue.description, pipeline: :email, author: @issue.author, current_user: @recipient, issuable_reference_expansion_enabled: true) diff --git a/app/views/notify/new_merge_request_email.html.haml b/app/views/notify/new_merge_request_email.html.haml index 54fb6573c26fc2..f67ac5f8fb2a3a 100644 --- a/app/views/notify/new_merge_request_email.html.haml +++ b/app/views/notify/new_merge_request_email.html.haml @@ -15,5 +15,5 @@ = render_if_exists 'notify/merge_request_approvers', presenter: @mr_presenter - if @merge_request.description - %div + .md = markdown(@merge_request.description, pipeline: :email, author: @merge_request.author, current_user: @recipient, issuable_reference_expansion_enabled: true) diff --git a/app/views/notify/new_release_email.html.haml b/app/views/notify/new_release_email.html.haml index 1cd3a2340c6d1f..09c0e7a8abd472 100644 --- a/app/views/notify/new_release_email.html.haml +++ b/app/views/notify/new_release_email.html.haml @@ -1,7 +1,7 @@ - release_link_start = ''.html_safe % { url: @target_url } - description_details = { tag: @release.tag, name: @project.name, release_link_start: release_link_start, release_link_end: ''.html_safe } -%div{ style: "font-family: 'Helvetica Neue',Helvetica,Arial,sans-serif;" } +.md{ style: "font-family: 'Helvetica Neue',Helvetica,Arial,sans-serif;" } %p = _("A new Release %{tag} for %{name} was published. Visit the %{release_link_start}Releases page%{release_link_end} to read more about it.").html_safe % description_details diff --git a/app/views/notify/service_desk_new_note_email.html.haml b/app/views/notify/service_desk_new_note_email.html.haml index 186bdf133e3d47..0c16cf3315fd5b 100644 --- a/app/views/notify/service_desk_new_note_email.html.haml +++ b/app/views/notify/service_desk_new_note_email.html.haml @@ -1,5 +1,5 @@ - if Gitlab::CurrentSettings.email_author_in_body %div = _("%{author_link} wrote:").html_safe % { author_link: link_to(@note.author_name, user_url(@note.author)) } -%div +.md = markdown(@note.note, pipeline: :email, author: @note.author, issuable_reference_expansion_enabled: true) diff --git a/config/application.rb b/config/application.rb index 8d795e6bc4e20d..e66f64ce791046 100644 --- a/config/application.rb +++ b/config/application.rb @@ -247,6 +247,7 @@ class Application < Rails::Application config.assets.precompile << "mailer.css" config.assets.precompile << "mailer_client_specific.css" config.assets.precompile << "notify.css" + config.assets.precompile << "notify_enhanced.css" config.assets.precompile << "mailers/*.css" config.assets.precompile << "page_bundles/_mixins_and_variables_and_functions.css" config.assets.precompile << "page_bundles/admin/application_settings_metrics_and_profiling.css" diff --git a/config/feature_flags/development/enhanced_notify_css.yml b/config/feature_flags/development/enhanced_notify_css.yml new file mode 100644 index 00000000000000..e47db3ba4352b8 --- /dev/null +++ b/config/feature_flags/development/enhanced_notify_css.yml @@ -0,0 +1,8 @@ +--- +name: enhanced_notify_css +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/78604 +rollout_issue_url: +milestone: '14.8' +type: development +group: group::project management +default_enabled: false diff --git a/ee/app/views/notify/add_merge_request_approver_email.html.haml b/ee/app/views/notify/add_merge_request_approver_email.html.haml index 3ac6633b4e4231..be4bd316c4ec37 100644 --- a/ee/app/views/notify/add_merge_request_approver_email.html.haml +++ b/ee/app/views/notify/add_merge_request_approver_email.html.haml @@ -14,5 +14,5 @@ = render 'notify/merge_request_approvers', presenter: @mr_presenter - if @merge_request.description - %div + .md = markdown(@merge_request.description, pipeline: :email, author: @merge_request.author, current_user: @recipient, issuable_reference_expansion_enabled: true) diff --git a/ee/app/views/notify/new_epic_email.html.haml b/ee/app/views/notify/new_epic_email.html.haml index 718bd5233fd49a..6a838754be3292 100644 --- a/ee/app/views/notify/new_epic_email.html.haml +++ b/ee/app/views/notify/new_epic_email.html.haml @@ -7,5 +7,5 @@ = assignees_label(@epic) - if @epic.description - %div - = markdown(@epic.description, pipeline: :email, author: @epic.author, current_user: @recipient, issuable_reference_expansion_enabled: true) + .md + = markdown(@epic.description, pipeline: :email, author: @epic.author, current_user: @recipient, issuable_reference_expansion_enabled: true) -- GitLab