Merge pull request #4080 from consul/banner_margin

Fix banner overlapping with other content

app/assets/stylesheets/layout.scss

@@ -2287,6 +2287,22 @@ table {
 // 18. Banners
 // -----------
 
+.banner {
+
+  a > * {
+    @include grid-row;
+    padding: 0 rem-calc(16);
+
+    &:empty {
+      display: none;
+    }
+  }
+
+  + .budget.expanded,
+  + .jumbo {
+    margin-top: 0;
+  }
+}
 
 // 19. Recommendations
 // -------------------

app/assets/stylesheets/participation.scss

@@ -909,6 +909,8 @@
 .help-header {
   background: #fafafa;
   border-bottom: 1px solid #eee;
+  margin-top: -$line-height;
+  margin-bottom: $line-height;
   padding-bottom: $line-height / 2;
   padding-top: $line-height;
 
@@ -1142,6 +1144,7 @@
 
   &.budget {
     background: $budget;
+    margin-top: -$line-height;
 
     h1,
     h2,

app/helpers/banners_helper.rb

@@ -3,22 +3,6 @@ module BannersHelper
     @banners.present? && @banners.count > 0
   end
 
-  def banner_default_bg_color
-    "#e7f2fc"
-  end
-
-  def banner_default_font_color
-    "#222222"
-  end
-
-  def banner_bg_color_or_default
-    @banner.background_color.presence || banner_default_bg_color
-  end
-
-  def banner_font_color_or_default
-    @banner.font_color.presence || banner_default_font_color
-  end
-
   def banner_target_link(banner)
     link_to banner.target_url do
       tag.h2(banner.title, style: "color:#{banner.font_color}") +

app/models/banner.rb

@@ -2,6 +2,9 @@ class Banner < ApplicationRecord
   acts_as_paranoid column: :hidden_at
   include ActsAsParanoidAliases
 
+  attribute :background_color, default: "#e7f2fc"
+  attribute :font_color, default: "#222222"
+
   translates :title,       touch: true
   translates :description, touch: true
   include Globalizable

app/views/admin/banners/_form.html.erb

@@ -56,8 +56,7 @@
       <p class="help-text"><%= t("admin.shared.color_help") %></p>
       <div class="row collapse">
         <div class="small-12 medium-6 column">
-          <%= f.text_field :background_color, label: false, type: :color,
-                                              value: banner_bg_color_or_default %>
+          <%= f.text_field :background_color, label: false, type: :color %>
         </div>
         <div class="small-12 medium-6 column">
           <%= f.text_field :background_color, label: false, id: "background_color_input" %>
@@ -70,8 +69,7 @@
       <p class="help-text"><%= t("admin.shared.color_help") %></p>
       <div class="row collapse">
         <div class="small-12 medium-6 column">
-          <%= f.text_field :font_color, label: false, type: :color,
-                                        value: banner_font_color_or_default %>
+          <%= f.text_field :font_color, label: false, type: :color %>
         </div>
         <div class="small-12 medium-6 column">
           <%= f.text_field :font_color, label: false, id: "font_color_input" %>

app/views/budgets/index.html.erb

@@ -7,7 +7,7 @@
 <% end %>
 
 <% if current_budget.present? %>
-  <div id="budget_heading" class="expanded budget no-margin-top">
+  <div id="budget_heading" class="expanded budget">
     <div class="row" data-equalizer data-equalizer-on="medium">
       <div class="small-12 medium-9 column padding" data-equalizer-watch>
 

app/views/shared/_section_header.html.erb

@@ -1,4 +1,4 @@
-<div class="help-header no-margin-top margin-bottom">
+<div class="help-header">
   <div class="row">
     <div class="small-12 column" data-magellan>
       <%= image_tag "help/help_icon_#{image}.png", alt: t("#{i18n_namespace}.icon_alt"), class: "align-top" %>

spec/models/banner_spec.rb

@@ -11,4 +11,11 @@ describe Banner do
   it "is valid" do
     expect(banner).to be_valid
   end
+
+  it "assigns default values to new banners" do
+    banner = Banner.new
+
+    expect(banner.background_color).to be_present
+    expect(banner.font_color).to be_present
+  end
 end