Merge branch 'slashmanov/migrate_admin_broadcast_messages_form_to_shared_alert' into 'master'

Use GlBroadcastMessage in Broadcast Message admin settings page

See merge request gitlab-org/gitlab!83251

app/assets/javascripts/pages/admin/broadcast_messages/broadcast_message.js

@@ -2,52 +2,41 @@ import $ from 'jquery';
 import { debounce } from 'lodash';
 import createFlash from '~/flash';
 import axios from '~/lib/utils/axios_utils';
-import { textColorForBackground } from '~/lib/utils/color_utils';
 import { DEFAULT_DEBOUNCE_AND_THROTTLE_MS } from '~/lib/utils/constants';
 import { __ } from '~/locale';
 
 export default () => {
-  const $broadcastMessageColor = $('.js-broadcast-message-color');
+  const $broadcastMessageTheme = $('.js-broadcast-message-theme');
   const $broadcastMessageType = $('.js-broadcast-message-type');
-  const $broadcastBannerMessagePreview = $('.js-broadcast-banner-message-preview');
+  const $broadcastBannerMessagePreview = $('.js-broadcast-banner-message-preview [role="alert"]');
   const $broadcastMessage = $('.js-broadcast-message-message');
-  const $jsBroadcastMessagePreview = $('.js-broadcast-message-preview');
+  const $jsBroadcastMessagePreview = $('#broadcast-message-preview');
 
   const reloadPreview = function reloadPreview() {
     const previewPath = $broadcastMessage.data('previewPath');
     const message = $broadcastMessage.val();
     const type = $broadcastMessageType.val();
-
-    if (message === '') {
-      $jsBroadcastMessagePreview.text(__('Your message here'));
-    } else {
-      axios
-        .post(previewPath, {
-          broadcast_message: {
-            message,
-            broadcast_type: type,
-          },
-        })
-        .then(({ data }) => {
-          $jsBroadcastMessagePreview.html(data.message);
-        })
-        .catch(() =>
-          createFlash({
-            message: __('An error occurred while rendering preview broadcast message'),
-          }),
-        );
-    }
+    const theme = $broadcastMessageTheme.val();
+
+    axios
+      .post(previewPath, {
+        broadcast_message: {
+          message,
+          broadcast_type: type,
+          theme,
+        },
+      })
+      .then(({ data }) => {
+        $jsBroadcastMessagePreview.html(data);
+      })
+      .catch(() =>
+        createFlash({
+          message: __('An error occurred while rendering preview broadcast message'),
+        }),
+      );
   };
 
-  $broadcastMessageColor.on('input', function onMessageColorInput() {
-    const previewColor = $(this).val();
-    $broadcastBannerMessagePreview.css('background-color', previewColor);
-  });
-
-  $('input#broadcast_message_font').on('input', function onMessageFontInput() {
-    const previewColor = $(this).val();
-    $broadcastBannerMessagePreview.css('color', previewColor);
-  });
+  $broadcastMessageTheme.on('change', reloadPreview);
 
   $broadcastMessageType.on('change', () => {
     const $broadcastMessageColorFormGroup = $('.js-broadcast-message-background-color-form-group');
@@ -68,37 +57,4 @@ export default () => {
       reloadPreview();
     }, DEFAULT_DEBOUNCE_AND_THROTTLE_MS),
   );
-
-  const updateColorPreview = () => {
-    const selectedBackgroundColor = $broadcastMessageColor.val();
-    const contrastTextColor = textColorForBackground(selectedBackgroundColor);
-
-    // save contrastTextColor to hidden input field
-    $('input.text-font-color').val(contrastTextColor);
-
-    // Updates the preview color with the hex-color input
-    const selectedColorStyle = {
-      backgroundColor: selectedBackgroundColor,
-      color: contrastTextColor,
-    };
-
-    $('.label-color-preview').css(selectedColorStyle);
-
-    return $jsBroadcastMessagePreview.css(selectedColorStyle);
-  };
-
-  const setSuggestedColor = (e) => {
-    const color = $(e.currentTarget).data('color');
-    $broadcastMessageColor
-      .val(color)
-      // Notify the form, that color has changed
-      .trigger('input');
-    // Only banner supports colors
-    if ($broadcastMessageType === 'banner') {
-      updateColorPreview();
-    }
-    return e.preventDefault();
-  };
-
-  $(document).on('click', '.suggest-colors a', setSuggestedColor);
 };

app/assets/stylesheets/framework/broadcast_messages.scss

@@ -15,6 +15,10 @@
 
 .broadcast-banner-message {
   text-align: center;
+
+  p {
+    margin-bottom: 0;
+  }
 }
 
 .broadcast-notification-message {

app/controllers/admin/broadcast_messages_controller.rb

@@ -45,8 +45,8 @@ class Admin::BroadcastMessagesController < Admin::ApplicationController
   end
 
   def preview
-    broadcast_message = BroadcastMessage.new(broadcast_message_params)
-    render json: { message: render_broadcast_message(broadcast_message) }
+    @broadcast_message = BroadcastMessage.new(broadcast_message_params)
+    render partial: 'admin/broadcast_messages/preview'
   end
 
   protected
@@ -58,8 +58,8 @@ class Admin::BroadcastMessagesController < Admin::ApplicationController
   def broadcast_message_params
     params.require(:broadcast_message).permit(%i(
       color
+      theme
       ends_at
-      font
       message
       starts_at
       target_path

app/helpers/broadcast_messages_helper.rb

@@ -25,23 +25,7 @@ module BroadcastMessagesHelper
   def broadcast_message(message, opts = {})
     return unless message.present?
 
-    render "shared/broadcast_message", { message: message, opts: opts }
-  end
-
-  def broadcast_message_style(broadcast_message)
-    return '' if broadcast_message.notification?
-
-    style = []
-
-    if broadcast_message.color.present?
-      style << "background-color: #{broadcast_message.color}"
-    end
-
-    if broadcast_message.font.present?
-      style << "color: #{broadcast_message.font}"
-    end
-
-    style.join('; ')
+    render "shared/broadcast_message", { message: message, **opts }
   end
 
   def broadcast_message_status(broadcast_message)
@@ -70,6 +54,10 @@ module BroadcastMessagesHelper
     BroadcastMessage.broadcast_types.keys.map { |w| [w.humanize, w] }
   end
 
+  def broadcast_theme_options
+    BroadcastMessage.themes.keys
+  end
+
   def target_access_level_options
     BroadcastMessage::ALLOWED_TARGET_ACCESS_LEVELS.map do |access_level|
       [Gitlab::Access.human_access(access_level), access_level]

app/models/broadcast_message.rb

@@ -32,6 +32,19 @@ class BroadcastMessage < ApplicationRecord
 
   after_commit :flush_redis_cache
 
+  enum theme: {
+    indigo: 0,
+    'light-indigo': 1,
+    blue: 2,
+    'light-blue': 3,
+    green: 4,
+    'light-green': 5,
+    red: 6,
+    'light-red': 7,
+    dark: 8,
+    light: 9
+  }, _default: 0, _prefix: true
+
   enum broadcast_type: {
     banner: 1,
     notification: 2

app/views/admin/broadcast_messages/_form.html.haml

-.broadcast-message.broadcast-banner-message.gl-alert-warning.js-broadcast-banner-message-preview.gl-mt-3{ style: broadcast_message_style(@broadcast_message), class: ('gl-display-none' unless @broadcast_message.banner? ) }
-  .gl-alert-container
-    = sprite_icon('bullhorn', css_class: 'vertical-align-text-top')
-    .js-broadcast-message-preview
-      .gl-alert-content
-        - if @broadcast_message.message.present?
-          = render_broadcast_message(@broadcast_message)
-        - else
-          = _('Your message here')
-.d-flex.justify-content-center
-  .broadcast-message.broadcast-notification-message.preview.js-broadcast-notification-message-preview.mt-2{ class: ('hidden' unless @broadcast_message.notification? ) }
-    = sprite_icon('bullhorn', css_class: 'vertical-align-text-top')
-    .js-broadcast-message-preview
-      - if @broadcast_message.message.present?
-        = render_broadcast_message(@broadcast_message)
-      - else
-        = _('Your message here')
+#broadcast-message-preview
+  = render 'preview'
 
 = gitlab_ui_form_for [:admin, @broadcast_message], html: { class: 'broadcast-message-form js-quick-submit js-requires-input'} do |f|
   = form_errors(@broadcast_message)
@@ -34,19 +19,10 @@
       = f.select :broadcast_type, broadcast_type_options, {}, class: 'form-control js-broadcast-message-type'
   .form-group.row.js-broadcast-message-background-color-form-group{ class: ('hidden' unless @broadcast_message.banner? ) }
     .col-sm-2.col-form-label
-      = f.label :color, _("Background color")
+      = f.label :theme, _("Theme")
     .col-sm-10
       .input-group
-        .input-group-prepend
-          .input-group-text.label-color-preview{ :style => 'background-color: ' + @broadcast_message.color + '; color: ' + @broadcast_message.font }
-            = ' '.html_safe
-        = f.text_field :color, class: "form-control gl-form-input js-broadcast-message-color"
-      .form-text.text-muted
-        = _('Choose any color.')
-        %br
-        = _("Or you can choose one of the suggested colors below")
-
-      = render_suggested_colors
+        = f.select :theme, broadcast_theme_options, {}, class: 'form-control js-broadcast-message-theme'
 
   .form-group.row.js-broadcast-message-dismissable-form-group{ class: ('hidden' unless @broadcast_message.banner? ) }
     .col-sm-2.col-form-label.pt-0

app/views/admin/broadcast_messages/_preview.html.haml

+.js-broadcast-banner-message-preview
+  = render "shared/broadcast_message", { message: @broadcast_message, preview: true } do
+    = _('Your message here')

app/views/shared/_broadcast_message.html.haml

-- is_banner = message.broadcast_type == 'banner'
+- icon_name = 'bullhorn'
+- dismissable = message.dismissable?
+- preview = local_assigns.fetch(:preview, false)
 
-%div{ class: "broadcast-message #{'alert-warning' if is_banner} broadcast-#{message.broadcast_type}-message #{opts[:preview] && 'preview'} js-broadcast-notification-#{message.id} gl-display-flex",
-  style: broadcast_message_style(message), dir: 'auto' }
-  .gl-flex-grow-1.gl-text-right.gl-pr-3
-    = sprite_icon('bullhorn', css_class: 'vertical-align-text-top')
-  %div{ class: !fluid_layout && 'container-limited' }
-    = render_broadcast_message(message)
-  .gl-flex-grow-1.gl-flex-basis-0.gl-text-right
-    - if (message.notification? || message.dismissable?) && opts[:preview].blank?
+- unless message.notification?
+  .gl-broadcast-message.broadcast-banner-message{ role: "alert", class: "js-broadcast-notification-#{message.id} #{message.theme}" }
+    .gl-broadcast-message-content
+      .gl-broadcast-message-icon
+        = sprite_icon(icon_name)
+      .gl-broadcast-message-text.js-broadcast-message-preview
+        - if message.message.present?
+          = render_broadcast_message(message)
+        - else
+          = yield
+    - if dismissable && !preview
+      %button.btn.gl-close-btn-color-inherit.gl-broadcast-message-dismiss.btn-default.btn-sm.gl-button.btn-default-tertiary.btn-icon.js-dismiss-current-broadcast-notification{ 'aria-label' => _('Close'), :type => 'button', data: { id: message.id, expire_date: message.ends_at.iso8601 } }
+        = sprite_icon('close', size: 16, css_class: "gl-icon gl-mx-3! gl-text-white")
+- else
+  - notification_class = "js-broadcast-notification-#{message.id}"
+  - notification_class << ' preview' if preview
+  .broadcast-message.broadcast-notification-message.mt-2{ role: "alert", class: notification_class }
+    = sprite_icon(icon_name, css_class: 'vertical-align-text-top')
+    - if message.message.present?
+      = render_broadcast_message(message)
+    - else
+      = yield
+    - if !preview
       %button.js-dismiss-current-broadcast-notification.btn.btn-link.gl-button{ 'aria-label' => _('Close'), :type => 'button', data: { id: message.id, expire_date: message.ends_at.iso8601 } }
-        = sprite_icon('close', size: 16, css_class: "gl-icon gl-mx-3! #{is_banner ? 'gl-text-white' : 'gl-text-gray-700'}")
+        = sprite_icon('close', size: 16, css_class: "gl-icon gl-mx-3! gl-text-gray-700")

db/migrate/20220329095632_add_theme_to_broadcast_message.rb

+# frozen_string_literal: true
+
+class AddThemeToBroadcastMessage < Gitlab::Database::Migration[1.0]
+  def change
+    add_column :broadcast_messages, :theme, :smallint, null: false, default: 0
+  end
+end

db/schema_migrations/20220329095632

+19ab6c5663ec07c7f4bdd92bf6d38e36766b27e6d0f0460f929a71b5e20fa396
\ No newline at end of file

db/structure.sql

@@ -12004,7 +12004,8 @@ CREATE TABLE broadcast_messages (
     target_path character varying(255),
     broadcast_type smallint DEFAULT 1 NOT NULL,
     dismissable boolean,
-    target_access_levels integer[] DEFAULT '{}'::integer[] NOT NULL
+    target_access_levels integer[] DEFAULT '{}'::integer[] NOT NULL,
+    theme smallint DEFAULT 0 NOT NULL
 );
 
 CREATE SEQUENCE broadcast_messages_id_seq

locale/gitlab.pot

@@ -37962,6 +37962,9 @@ msgstr ""
 msgid "The vulnerability is no longer detected. Verify the vulnerability has been remediated before changing its status."
 msgstr ""
 
+msgid "Theme"
+msgstr ""
+
 msgid "There are Advanced Search migrations pending that require indexing to pause. Indexing must remain paused until GitLab completes the migrations."
 msgstr ""
 

spec/features/admin/admin_broadcast_messages_spec.rb

@@ -22,9 +22,8 @@ RSpec.describe 'Admin Broadcast Messages' do
 
   it 'creates a customized broadcast banner message' do
     fill_in 'broadcast_message_message', with: 'Application update from **4:00 CST to 5:00 CST**'
-    fill_in 'broadcast_message_color', with: '#f2dede'
     fill_in 'broadcast_message_target_path', with: '*/user_onboarded'
-    fill_in 'broadcast_message_font', with: '#b94a48'
+    select 'light-indigo', from: 'broadcast_message_theme'
     select Date.today.next_year.year, from: 'broadcast_message_ends_at_1i'
     check 'Guest'
     check 'Owner'
@@ -35,7 +34,7 @@ RSpec.describe 'Admin Broadcast Messages' do
     expect(page).to have_content 'Guest, Owner'
     expect(page).to have_content '*/user_onboarded'
     expect(page).to have_selector 'strong', text: '4:00 CST to 5:00 CST'
-    expect(page).to have_selector %(div[style="background-color: #f2dede; color: #b94a48"])
+    expect(page).to have_selector %(.light-indigo[role=alert])
   end
 
   it 'creates a customized broadcast notification message' do
@@ -90,7 +89,7 @@ RSpec.describe 'Admin Broadcast Messages' do
     fill_in 'broadcast_message_message', with: "Live **Markdown** previews. :tada:"
     select 'Notification', from: 'broadcast_message_broadcast_type'
 
-    page.within('.js-broadcast-notification-message-preview') do
+    page.within('#broadcast-message-preview') do
       expect(page).to have_selector('strong', text: 'Markdown')
       expect(page).to have_emoji('tada')
     end

spec/helpers/broadcast_messages_helper_spec.rb

@@ -115,37 +115,8 @@ RSpec.describe BroadcastMessagesHelper do
     end
 
     it 'includes the current message' do
-      allow(helper).to receive(:broadcast_message_style).and_return(nil)
-
       expect(helper.broadcast_message(current_broadcast_message)).to include 'Current Message'
     end
-
-    it 'includes custom style' do
-      allow(helper).to receive(:broadcast_message_style).and_return('foo')
-
-      expect(helper.broadcast_message(current_broadcast_message)).to include 'style="foo"'
-    end
-  end
-
-  describe 'broadcast_message_style' do
-    it 'defaults to no style' do
-      broadcast_message = spy
-
-      expect(helper.broadcast_message_style(broadcast_message)).to eq ''
-    end
-
-    it 'allows custom style for banner messages' do
-      broadcast_message = BroadcastMessage.new(color: '#f2dede', font: '#b94a48', broadcast_type: "banner")
-
-      expect(helper.broadcast_message_style(broadcast_message))
-        .to match('background-color: #f2dede; color: #b94a48')
-    end
-
-    it 'does not add style for notification messages' do
-      broadcast_message = BroadcastMessage.new(color: '#f2dede', broadcast_type: "notification")
-
-      expect(helper.broadcast_message_style(broadcast_message)).to eq ''
-    end
   end
 
   describe 'broadcast_message_status' do