From 5f35ae510f6fe71dcb2d24c6716e8eb2f12eb78b Mon Sep 17 00:00:00 2001 From: Andy Sellick Date: Thu, 21 Feb 2019 09:06:31 +0000 Subject: [PATCH 1/9] Add shared helper - provides margin bottom for components --- lib/govuk_publishing_components.rb | 1 + .../presenters/shared_helper.rb | 16 ++++++++++++++++ 2 files changed, 17 insertions(+) create mode 100644 lib/govuk_publishing_components/presenters/shared_helper.rb diff --git a/lib/govuk_publishing_components.rb b/lib/govuk_publishing_components.rb index 8152e28457..fae699e7ad 100644 --- a/lib/govuk_publishing_components.rb +++ b/lib/govuk_publishing_components.rb @@ -1,5 +1,6 @@ require "govuk_publishing_components/config" require "govuk_publishing_components/engine" +require "govuk_publishing_components/presenters/shared_helper" require "govuk_publishing_components/presenters/accordion_helper" require "govuk_publishing_components/presenters/breadcrumbs" require "govuk_publishing_components/presenters/button_helper" diff --git a/lib/govuk_publishing_components/presenters/shared_helper.rb b/lib/govuk_publishing_components/presenters/shared_helper.rb new file mode 100644 index 0000000000..677243f292 --- /dev/null +++ b/lib/govuk_publishing_components/presenters/shared_helper.rb @@ -0,0 +1,16 @@ +module GovukPublishingComponents + module Presenters + class SharedHelper + attr_reader :options, :margin_bottom + + def initialize(local_assigns) + @options = local_assigns + @margin_bottom = @options[:margin_bottom] || 3 + end + + def get_margin_bottom + [*0..9].include?(@margin_bottom) ? "govuk-!-margin-bottom-#{margin_bottom}" : "govuk-!-margin-bottom-3" + end + end + end +end From 9cf30de0c2115de441871d64d3c92ba0b1672713 Mon Sep 17 00:00:00 2001 From: Andy Sellick Date: Thu, 21 Feb 2019 09:08:06 +0000 Subject: [PATCH 2/9] Change hint to use shared helper - for margin bottom --- .../govuk_publishing_components/components/_hint.html.erb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/govuk_publishing_components/components/_hint.html.erb b/app/views/govuk_publishing_components/components/_hint.html.erb index fb77495231..5f4dc605ce 100644 --- a/app/views/govuk_publishing_components/components/_hint.html.erb +++ b/app/views/govuk_publishing_components/components/_hint.html.erb @@ -1,9 +1,9 @@ <% id ||= "hint-#{SecureRandom.hex(4)}" - margin_bottom ||= 3 + shared_helper = GovukPublishingComponents::Presenters::SharedHelper.new(local_assigns) css_classes = %w( gem-c-hint govuk-hint ) - css_classes << ([*0..9].include?(margin_bottom) ? "govuk-!-margin-bottom-#{margin_bottom}" : "govuk-!-margin-bottom-3") + css_classes << (shared_helper.get_margin_bottom) %> <%= tag.span id: id, class: css_classes do %> From 0b99233a817aa539771ed08bdf5ea050d093f527 Mon Sep 17 00:00:00 2001 From: Andy Sellick Date: Thu, 21 Feb 2019 10:19:07 +0000 Subject: [PATCH 3/9] Change textarea to use shared helper for margin - restructure textarea slightly to put margin on wrapping element rather than textarea itself, as those two margins were conflicting - set default margin to be 6, overriding default of 3 in shared helper, means this will be a non-breaking change - add margin option to documentation --- .../components/_textarea.scss | 4 ++++ .../components/_textarea.html.erb | 11 +++++++---- .../components/docs/textarea.yml | 7 +++++++ spec/components/textarea_spec.rb | 15 ++++++++++++--- 4 files changed, 30 insertions(+), 7 deletions(-) diff --git a/app/assets/stylesheets/govuk_publishing_components/components/_textarea.scss b/app/assets/stylesheets/govuk_publishing_components/components/_textarea.scss index 29ec47e8bf..7c7c9d99ef 100644 --- a/app/assets/stylesheets/govuk_publishing_components/components/_textarea.scss +++ b/app/assets/stylesheets/govuk_publishing_components/components/_textarea.scss @@ -1 +1,5 @@ @import "govuk-frontend/components/textarea/textarea"; + +.gem-c-textarea .govuk-textarea { + margin-bottom: 0; +} diff --git a/app/views/govuk_publishing_components/components/_textarea.html.erb b/app/views/govuk_publishing_components/components/_textarea.html.erb index dce6252732..129d7ea2b8 100644 --- a/app/views/govuk_publishing_components/components/_textarea.html.erb +++ b/app/views/govuk_publishing_components/components/_textarea.html.erb @@ -7,7 +7,8 @@ label ||= nil hint ||= nil - margin_bottom ||= 3 + local_assigns[:margin_bottom] ||= 6 + local_assigns[:margin_bottom] = 6 if local_assigns[:margin_bottom] > 9 error_message ||= nil error_items ||= nil character_count ||= nil @@ -16,12 +17,14 @@ has_error ||= error_message || error_items&.any? error_id = "error-#{SecureRandom.hex(4)}" - css_classes = %w(gem-c-textarea govuk-textarea) + shared_helper = GovukPublishingComponents::Presenters::SharedHelper.new(local_assigns) + + css_classes = %w(govuk-textarea) css_classes << "js-character-count" if character_count css_classes << "govuk-textarea--error" if has_error - css_classes << ([*0..9].include?(margin_bottom) ? "govuk-!-margin-bottom-#{margin_bottom}" : "govuk-!-margin-bottom-3") - form_group_css_classes = %w(govuk-form-group) + form_group_css_classes = %w(gem-c-textarea govuk-form-group) form_group_css_classes << "govuk-form-group--error" if has_error + form_group_css_classes << (shared_helper.get_margin_bottom) aria_described_by ||= nil if hint || has_error diff --git a/app/views/govuk_publishing_components/components/docs/textarea.yml b/app/views/govuk_publishing_components/components/docs/textarea.yml index 1a6b62931e..648e9c54b6 100644 --- a/app/views/govuk_publishing_components/components/docs/textarea.yml +++ b/app/views/govuk_publishing_components/components/docs/textarea.yml @@ -20,6 +20,13 @@ examples: label: text: "Can you provide more detail?" name: "more-detail" + with_margin_bottom: + description: The component accepts a number for margin bottom from 0 to 9 (0px to 60px) using the [GOV.UK Frontend spacing scale](http://govuk-frontend-review.herokuapp.com/docs/#settings/spacing-variable-govuk-spacing-points). It defaults to a margin bottom of 6 (30px). + data: + margin_bottom: 9 + label: + text: "Can you provide more detail?" + name: "more-detail" specific_rows: description: Textarea with 10 rows data: diff --git a/spec/components/textarea_spec.rb b/spec/components/textarea_spec.rb index 40736bf0ee..8a2b5b37ba 100644 --- a/spec/components/textarea_spec.rb +++ b/spec/components/textarea_spec.rb @@ -76,10 +76,10 @@ def component_name render_component( label: { text: "Can you provide more detail?" }, name: "with-custom-margin-bottom", - margin_bottom: 6 + margin_bottom: 4 ) - assert_select '.govuk-textarea.govuk-\!-margin-bottom-6' + assert_select '.gem-c-textarea.govuk-\!-margin-bottom-4' end it "defaults to the initial bottom margin if an incorrect value is passed" do @@ -89,7 +89,16 @@ def component_name margin_bottom: 12 ) - assert_select '.govuk-textarea.govuk-\!-margin-bottom-3' + assert_select '.gem-c-textarea.govuk-\!-margin-bottom-6' + end + + it "defaults to the initial bottom margin if no value is passed" do + render_component( + label: { text: "Can you provide more detail?" }, + name: "with-no-given-margin-bottom", + ) + + assert_select '.gem-c-textarea.govuk-\!-margin-bottom-6' end context "when a hint is provided" do From ce522a4b283e2696138b175d751042a7392dc17c Mon Sep 17 00:00:00 2001 From: Andy Sellick Date: Thu, 21 Feb 2019 11:56:24 +0000 Subject: [PATCH 4/9] Change subscription links to use shared helper - defaults to no margin as per original behaviour --- .../components/_subscription-links.html.erb | 7 +++++-- spec/components/subscription_links_spec.rb | 10 ++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/app/views/govuk_publishing_components/components/_subscription-links.html.erb b/app/views/govuk_publishing_components/components/_subscription-links.html.erb index 065fb5598f..b7fa654c5f 100644 --- a/app/views/govuk_publishing_components/components/_subscription-links.html.erb +++ b/app/views/govuk_publishing_components/components/_subscription-links.html.erb @@ -1,11 +1,14 @@ <% brand ||= false - margin_bottom ||= 0 brand_helper = GovukPublishingComponents::AppHelpers::BrandHelper.new(brand) sl_helper = GovukPublishingComponents::Presenters::SubscriptionLinksHelper.new(local_assigns) + shared_helper = GovukPublishingComponents::Presenters::SharedHelper.new(local_assigns) + + local_assigns[:margin_bottom] ||= 0 + local_assigns[:margin_bottom] = 0 if local_assigns[:margin_bottom] > 9 css_classes = %w( gem-c-subscription-links ) - css_classes << ([*0..9].include?(margin_bottom) ? "govuk-!-margin-bottom-#{margin_bottom}" : "govuk-!-margin-bottom-0") + css_classes << (shared_helper.get_margin_bottom) unless local_assigns[:margin_bottom] == 0 css_classes << brand_helper.brand_class data = {"module": "gem-toggle"} if sl_helper.feed_link_box_value %> diff --git a/spec/components/subscription_links_spec.rb b/spec/components/subscription_links_spec.rb index 6e4ba305f5..c02649a1f6 100644 --- a/spec/components/subscription_links_spec.rb +++ b/spec/components/subscription_links_spec.rb @@ -33,6 +33,16 @@ def component_name assert_select '.gem-c-subscription-links.govuk-\!-margin-bottom-7' end + it "defaults to the initial bottom margin if an incorrect value is passed" do + render_component(email_signup_link: 'email-signup', feed_link: 'singapore.atom', margin_bottom: 20) + assert_select "[class='^=govuk-\!-margin-bottom-']", false + end + + it "has no margin class added by default" do + render_component(email_signup_link: 'email-signup', feed_link: 'singapore.atom') + assert_select "[class='^=govuk-\!-margin-bottom-']", false + end + it "renders custom texts" do render_component(email_signup_link: 'email-signup', feed_link: 'singapore.atom', email_signup_link_text: 'Get email!', feed_link_text: 'View feed!') assert_select ".gem-c-subscription-links__link--email-alerts[href=\"email-signup\"]", text: "Get email!" From 35c196d1c3b238a5025fd66bd1173bb7f981c6d2 Mon Sep 17 00:00:00 2001 From: Andy Sellick Date: Fri, 22 Feb 2019 10:51:39 +0000 Subject: [PATCH 5/9] Add headings and tests to shared components helper --- .../presenters/shared_helper.rb | 7 +++- spec/lib/components/shared_helper_spec.rb | 39 +++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) create mode 100644 spec/lib/components/shared_helper_spec.rb diff --git a/lib/govuk_publishing_components/presenters/shared_helper.rb b/lib/govuk_publishing_components/presenters/shared_helper.rb index 677243f292..ef12e11054 100644 --- a/lib/govuk_publishing_components/presenters/shared_helper.rb +++ b/lib/govuk_publishing_components/presenters/shared_helper.rb @@ -1,16 +1,21 @@ module GovukPublishingComponents module Presenters class SharedHelper - attr_reader :options, :margin_bottom + attr_reader :options, :margin_bottom, :heading_level def initialize(local_assigns) @options = local_assigns @margin_bottom = @options[:margin_bottom] || 3 + @heading_level = @options[:heading_level] || 2 end def get_margin_bottom [*0..9].include?(@margin_bottom) ? "govuk-!-margin-bottom-#{margin_bottom}" : "govuk-!-margin-bottom-3" end + + def get_heading_level + [*1..6].include?(@heading_level) ? "h#{@heading_level}" : "h2" + end end end end diff --git a/spec/lib/components/shared_helper_spec.rb b/spec/lib/components/shared_helper_spec.rb new file mode 100644 index 0000000000..5b4a905a83 --- /dev/null +++ b/spec/lib/components/shared_helper_spec.rb @@ -0,0 +1,39 @@ +RSpec.describe GovukPublishingComponents::Presenters::SharedHelper do + describe "Shared component helper" do + it "returns a default margin class" do + shared_helper = GovukPublishingComponents::Presenters::SharedHelper.new({}) + margin_class = shared_helper.get_margin_bottom + expect(margin_class).to eql('govuk-!-margin-bottom-3') + end + + it "returns a given margin class" do + shared_helper = GovukPublishingComponents::Presenters::SharedHelper.new(margin_bottom: 6) + margin_class = shared_helper.get_margin_bottom + expect(margin_class).to eql('govuk-!-margin-bottom-6') + end + + it "returns the default margin class if passed value is wrong" do + shared_helper = GovukPublishingComponents::Presenters::SharedHelper.new(margin_bottom: "a") + margin_class = shared_helper.get_margin_bottom + expect(margin_class).to eql('govuk-!-margin-bottom-3') + end + + it "returns a default heading level" do + shared_helper = GovukPublishingComponents::Presenters::SharedHelper.new({}) + heading = shared_helper.get_heading_level + expect(heading).to eql('h2') + end + + it "returns a given heading level" do + shared_helper = GovukPublishingComponents::Presenters::SharedHelper.new(heading_level: 6) + heading = shared_helper.get_heading_level + expect(heading).to eql('h6') + end + + it "returns the default heading level if passed value is wrong" do + shared_helper = GovukPublishingComponents::Presenters::SharedHelper.new(heading_level: 9) + heading = shared_helper.get_heading_level + expect(heading).to eql('h2') + end + end +end From 743f66ffb282862d1065a90176678d688668cf8f Mon Sep 17 00:00:00 2001 From: Andy Sellick Date: Fri, 22 Feb 2019 11:20:37 +0000 Subject: [PATCH 6/9] Change heading to use shared helper for heading level --- .../govuk_publishing_components/components/_heading.html.erb | 3 ++- lib/govuk_publishing_components/presenters/heading_helper.rb | 2 -- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/app/views/govuk_publishing_components/components/_heading.html.erb b/app/views/govuk_publishing_components/components/_heading.html.erb index 387169be82..0029ca559c 100644 --- a/app/views/govuk_publishing_components/components/_heading.html.erb +++ b/app/views/govuk_publishing_components/components/_heading.html.erb @@ -2,8 +2,9 @@ brand ||= false brand_helper = GovukPublishingComponents::AppHelpers::BrandHelper.new(brand) heading_helper = GovukPublishingComponents::Presenters::HeadingHelper.new(local_assigns) + shared_helper = GovukPublishingComponents::Presenters::SharedHelper.new(local_assigns) %> -<%= content_tag(heading_helper.heading_tag, text, +<%= content_tag(shared_helper.get_heading_level, text, class: "gem-c-heading #{heading_helper.classes} #{brand_helper.brand_class} #{brand_helper.border_color_class}", id: heading_helper.id ) %> diff --git a/lib/govuk_publishing_components/presenters/heading_helper.rb b/lib/govuk_publishing_components/presenters/heading_helper.rb index 8e377810d1..bdd3954cad 100644 --- a/lib/govuk_publishing_components/presenters/heading_helper.rb +++ b/lib/govuk_publishing_components/presenters/heading_helper.rb @@ -4,8 +4,6 @@ class HeadingHelper attr_reader :heading_tag, :id, :classes def initialize(options) - @heading_tag = "h2" - @heading_tag = "h#{options[:heading_level]}" if [1, 2, 3, 4, 5, 6].include? options[:heading_level] @id = options[:id] @classes = "" From 0a6e14c5f7fbcbf5a83740f03784676f2a707e58 Mon Sep 17 00:00:00 2001 From: Andy Sellick Date: Fri, 22 Feb 2019 11:25:29 +0000 Subject: [PATCH 7/9] Change accordion to use shared helper for heading --- .../components/_accordion.html.erb | 4 ++-- lib/govuk_publishing_components.rb | 1 - .../presenters/accordion_helper.rb | 12 ------------ 3 files changed, 2 insertions(+), 15 deletions(-) delete mode 100644 lib/govuk_publishing_components/presenters/accordion_helper.rb diff --git a/app/views/govuk_publishing_components/components/_accordion.html.erb b/app/views/govuk_publishing_components/components/_accordion.html.erb index f32b5666d5..8dc551d2a4 100644 --- a/app/views/govuk_publishing_components/components/_accordion.html.erb +++ b/app/views/govuk_publishing_components/components/_accordion.html.erb @@ -1,5 +1,5 @@ <% - heading_helper = GovukPublishingComponents::Presenters::AccordionHelper.new(local_assigns) + shared_helper = GovukPublishingComponents::Presenters::SharedHelper.new(local_assigns) id ||= "default-id-#{SecureRandom.hex(4)}" items ||= [] @@ -31,7 +31,7 @@
<%= content_tag( - heading_helper.heading_tag, + shared_helper.get_heading_level, content_tag('span', item[:heading][:text], class: "govuk-accordion__section-button", id: "#{id}-heading-#{index}", data: item[:data_attributes]), class: 'govuk-accordion__section-heading' ) diff --git a/lib/govuk_publishing_components.rb b/lib/govuk_publishing_components.rb index fae699e7ad..60e17bad24 100644 --- a/lib/govuk_publishing_components.rb +++ b/lib/govuk_publishing_components.rb @@ -1,7 +1,6 @@ require "govuk_publishing_components/config" require "govuk_publishing_components/engine" require "govuk_publishing_components/presenters/shared_helper" -require "govuk_publishing_components/presenters/accordion_helper" require "govuk_publishing_components/presenters/breadcrumbs" require "govuk_publishing_components/presenters/button_helper" require "govuk_publishing_components/presenters/contextual_navigation" diff --git a/lib/govuk_publishing_components/presenters/accordion_helper.rb b/lib/govuk_publishing_components/presenters/accordion_helper.rb deleted file mode 100644 index 4ab7092cb6..0000000000 --- a/lib/govuk_publishing_components/presenters/accordion_helper.rb +++ /dev/null @@ -1,12 +0,0 @@ -module GovukPublishingComponents - module Presenters - class AccordionHelper - attr_reader :heading_tag - - def initialize(options) - @heading_tag = "h2" - @heading_tag = "h#{options[:heading_level]}" if [1, 2, 3, 4, 5, 6].include? options[:heading_level] - end - end - end -end From 969b9a2d54df11da3bac029527bf10d7ad03b2af Mon Sep 17 00:00:00 2001 From: Andy Sellick Date: Fri, 22 Feb 2019 11:32:52 +0000 Subject: [PATCH 8/9] Change image card to use shared helper for heading - update shared helper to return a span if heading level 0 is passed, instead of a heading level - retain the default of h2 if nothing or an invalid value is passed --- .../components/_image_card.html.erb | 3 ++- .../presenters/image_card_helper.rb | 6 ------ lib/govuk_publishing_components/presenters/shared_helper.rb | 4 +++- spec/lib/components/shared_helper_spec.rb | 6 ++++++ 4 files changed, 11 insertions(+), 8 deletions(-) diff --git a/app/views/govuk_publishing_components/components/_image_card.html.erb b/app/views/govuk_publishing_components/components/_image_card.html.erb index e8ecb9b7b7..548eb34ec7 100644 --- a/app/views/govuk_publishing_components/components/_image_card.html.erb +++ b/app/views/govuk_publishing_components/components/_image_card.html.erb @@ -2,6 +2,7 @@ brand ||= false brand_helper = GovukPublishingComponents::AppHelpers::BrandHelper.new(brand) card_helper = GovukPublishingComponents::Presenters::ImageCardHelper.new(local_assigns) + shared_helper = GovukPublishingComponents::Presenters::SharedHelper.new(local_assigns) %> <% if card_helper.href || card_helper.extra_links.any? %>
<%= brand_helper.brand_class %>" @@ -13,7 +14,7 @@ <%= card_helper.context %> <% if card_helper.heading_text %> - <%= content_tag(card_helper.heading_tag, + <%= content_tag(shared_helper.get_heading_level, class: "gem-c-image-card__title") do %> <% if card_helper.href %> <%= link_to card_helper.heading_text, card_helper.href, diff --git a/lib/govuk_publishing_components/presenters/image_card_helper.rb b/lib/govuk_publishing_components/presenters/image_card_helper.rb index 4ca61f49c4..7fc1867ddf 100644 --- a/lib/govuk_publishing_components/presenters/image_card_helper.rb +++ b/lib/govuk_publishing_components/presenters/image_card_helper.rb @@ -16,7 +16,6 @@ def initialize(local_assigns) @description = local_assigns[:description] @large = local_assigns[:large] @heading_text = local_assigns[:heading_text] - @heading_level = local_assigns[:heading_level] || 2 @extra_links_no_indent = local_assigns[:extra_links_no_indent] @metadata = local_assigns[:metadata] end @@ -59,11 +58,6 @@ def context end end - def heading_tag - return "h#{@heading_level}" if [1, 2, 3, 4, 5, 6].include? @heading_level - return "span" if @heading_level.zero? - end - def description content_tag(:div, @description, class: "gem-c-image-card__description") if @description end diff --git a/lib/govuk_publishing_components/presenters/shared_helper.rb b/lib/govuk_publishing_components/presenters/shared_helper.rb index ef12e11054..3e4b02d7c2 100644 --- a/lib/govuk_publishing_components/presenters/shared_helper.rb +++ b/lib/govuk_publishing_components/presenters/shared_helper.rb @@ -14,7 +14,9 @@ def get_margin_bottom end def get_heading_level - [*1..6].include?(@heading_level) ? "h#{@heading_level}" : "h2" + return [*1..6].include?(@heading_level) ? "h#{@heading_level}" : "h2" unless @heading_level.zero? + + "span" end end end diff --git a/spec/lib/components/shared_helper_spec.rb b/spec/lib/components/shared_helper_spec.rb index 5b4a905a83..8e3cda5904 100644 --- a/spec/lib/components/shared_helper_spec.rb +++ b/spec/lib/components/shared_helper_spec.rb @@ -35,5 +35,11 @@ heading = shared_helper.get_heading_level expect(heading).to eql('h2') end + + it "returns a span instead of a heading if heading level is 0" do + shared_helper = GovukPublishingComponents::Presenters::SharedHelper.new(heading_level: 0) + result = shared_helper.get_heading_level + expect(result).to eql('span') + end end end From 78bad64c3b03852f3ed33361fe3290f48ab02bac Mon Sep 17 00:00:00 2001 From: Andy Sellick Date: Fri, 22 Feb 2019 11:48:48 +0000 Subject: [PATCH 9/9] Update CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b90ccfc2fd..cd88555c29 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ ## Unreleased +* Create shared helper for components (PR #759) * Use delegated event handlers for checkbox events (PR #770) ## 16.1.0