From 400c3a6fa0bbf730a49d2ad95a83549a4833a86a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 10 Aug 2020 15:59:02 +0200 Subject: [PATCH 1/4] Move header margin styles to CSS Using HTML classes to apply styles to certain elements makes views harder to customize. --- app/assets/stylesheets/participation.scss | 3 +++ app/views/budgets/index.html.erb | 2 +- app/views/shared/_section_header.html.erb | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/app/assets/stylesheets/participation.scss b/app/assets/stylesheets/participation.scss index 8485b5468..2a8ed17c1 100644 --- a/app/assets/stylesheets/participation.scss +++ b/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, diff --git a/app/views/budgets/index.html.erb b/app/views/budgets/index.html.erb index b24d67e16..1beeb1179 100644 --- a/app/views/budgets/index.html.erb +++ b/app/views/budgets/index.html.erb @@ -7,7 +7,7 @@ <% end %> <% if current_budget.present? %> -
+
diff --git a/app/views/shared/_section_header.html.erb b/app/views/shared/_section_header.html.erb index 6ae7580ce..fbda3c408 100644 --- a/app/views/shared/_section_header.html.erb +++ b/app/views/shared/_section_header.html.erb @@ -1,4 +1,4 @@ -
+
<%= image_tag "help/help_icon_#{image}.png", alt: t("#{i18n_namespace}.icon_alt"), class: "align-top" %> From 14a5d823773f808325e095c08cf5d061d1919ab6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 10 Aug 2020 16:02:00 +0200 Subject: [PATCH 2/4] Fix banner overlapping with other content In some sections we had negative top margins to compensate the header bottom margin. However, when adding a banner between the header and those sections, the negative margin caused the content of those sections to overlap with the content of the banner. Removing the negative margins when a banner is present solves the issue. --- app/assets/stylesheets/layout.scss | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/app/assets/stylesheets/layout.scss b/app/assets/stylesheets/layout.scss index 6dbf15537..0ba2c3e3e 100644 --- a/app/assets/stylesheets/layout.scss +++ b/app/assets/stylesheets/layout.scss @@ -2287,6 +2287,13 @@ table { // 18. Banners // ----------- +.banner { + + + .budget.expanded, + + .jumbo { + margin-top: 0; + } +} // 19. Recommendations // ------------------- From 361b7ee09d75bbaaebf3ef0b39bee6419167eb3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 10 Aug 2020 16:31:52 +0200 Subject: [PATCH 3/4] Fix banner text alignment Unlike the rest of the page, it had no left margin nor padding whatsoever. --- app/assets/stylesheets/layout.scss | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/assets/stylesheets/layout.scss b/app/assets/stylesheets/layout.scss index 0ba2c3e3e..05e8189e0 100644 --- a/app/assets/stylesheets/layout.scss +++ b/app/assets/stylesheets/layout.scss @@ -2289,6 +2289,11 @@ table { .banner { + a > * { + @include grid-row; + padding: 0 rem-calc(16); + } + + .budget.expanded, + .jumbo { margin-top: 0; From 0b83be68377c8aaced7c9d9fbae51de17fae5035 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 12 Aug 2020 14:45:45 +0200 Subject: [PATCH 4/4] Apply banner default colors to dev seeds Banners created through the admin form were getting the default color. However, banners created by other means (like the `db:dev_seed` rake task) were not getting these default values. This feature was originally implemented when we were using Rails 4. With Rails 5, we can provide default values to all new banners and simplify the code at the same time thanks to its `attribute` method. Now, when creating a new banner, instead of getting a blank space, we get an empty line with the banner's default background color, which most users won't know what it's about until they fill in the banner's title. So we're not displaying the content of the banner when it's empty, thanks to the `:empty` CSS pseudoclass. --- app/assets/stylesheets/layout.scss | 4 ++++ app/helpers/banners_helper.rb | 16 ---------------- app/models/banner.rb | 3 +++ app/views/admin/banners/_form.html.erb | 6 ++---- spec/models/banner_spec.rb | 7 +++++++ 5 files changed, 16 insertions(+), 20 deletions(-) diff --git a/app/assets/stylesheets/layout.scss b/app/assets/stylesheets/layout.scss index 05e8189e0..dabfb2330 100644 --- a/app/assets/stylesheets/layout.scss +++ b/app/assets/stylesheets/layout.scss @@ -2292,6 +2292,10 @@ table { a > * { @include grid-row; padding: 0 rem-calc(16); + + &:empty { + display: none; + } } + .budget.expanded, diff --git a/app/helpers/banners_helper.rb b/app/helpers/banners_helper.rb index c43bc7e13..42cdb240d 100644 --- a/app/helpers/banners_helper.rb +++ b/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}") + diff --git a/app/models/banner.rb b/app/models/banner.rb index f4b9b1d09..b5484e397 100644 --- a/app/models/banner.rb +++ b/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 diff --git a/app/views/admin/banners/_form.html.erb b/app/views/admin/banners/_form.html.erb index bf8fc89b7..e6b607650 100644 --- a/app/views/admin/banners/_form.html.erb +++ b/app/views/admin/banners/_form.html.erb @@ -56,8 +56,7 @@

<%= t("admin.shared.color_help") %>

- <%= f.text_field :background_color, label: false, type: :color, - value: banner_bg_color_or_default %> + <%= f.text_field :background_color, label: false, type: :color %>
<%= f.text_field :background_color, label: false, id: "background_color_input" %> @@ -70,8 +69,7 @@

<%= t("admin.shared.color_help") %>

- <%= f.text_field :font_color, label: false, type: :color, - value: banner_font_color_or_default %> + <%= f.text_field :font_color, label: false, type: :color %>
<%= f.text_field :font_color, label: false, id: "font_color_input" %> diff --git a/spec/models/banner_spec.rb b/spec/models/banner_spec.rb index 92e3ef407..75c05d918 100644 --- a/spec/models/banner_spec.rb +++ b/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