From f75cc44cf62c517a2d73eb948772e8684ee878ce Mon Sep 17 00:00:00 2001
From: Andy Sellick <andy.sellick@digital.cabinet-office.gov.uk>
Date: Wed, 11 Dec 2024 13:53:08 +0000
Subject: [PATCH 1/3] Add margin_bottom option to component wrapper helper

- add a margin_bottom option, which accepts a number from the govuk-frontend spacing scale to set margin bottom on components. Slightly tricky as it overlaps onto another option - classes - but all we have to do is check the number, generate the right class based on the number, then add it to the end of the classes as they are passed out from all_attributes
- defaults to no margin and no margin class output
- rewrite the component wrapper helper tests a little, firstly to make more specific, secondly to structure and group the tests a bit more logically for readability
- update a trio of components that were broken by these changes, by removing their use of the shared helper for this same functionality and updating tests accordingly
---
 .../component_wrapper_helper_options.rb       |   1 +
 .../components/_details.html.erb              |   5 +-
 .../components/_heading.html.erb              |   1 -
 .../components/_hint.html.erb                 |   3 -
 docs/component-wrapper-helper.md              |   3 +
 .../presenters/component_wrapper_helper.rb    |  13 +
 spec/components/details_spec.rb               |   9 -
 spec/components/heading_spec.rb               |  10 -
 spec/components/hint_spec.rb                  |   6 -
 .../component_wrapper_helper_spec.rb          | 443 ++++++++++--------
 10 files changed, 254 insertions(+), 240 deletions(-)

diff --git a/app/models/govuk_publishing_components/component_wrapper_helper_options.rb b/app/models/govuk_publishing_components/component_wrapper_helper_options.rb
index 6708116518..1222ad4e8b 100644
--- a/app/models/govuk_publishing_components/component_wrapper_helper_options.rb
+++ b/app/models/govuk_publishing_components/component_wrapper_helper_options.rb
@@ -8,6 +8,7 @@ def self.description
 - `data_attributes` - accepts a hash of data attributes
 - `aria` - accepts a hash of aria attributes
 - `classes` - accepts a space separated string of classes, these should not be used for styling and must be prefixed with `js-`
+- `margin_bottom` - accepts a number from `0` to `9` (`0px` to `60px`) using the [GOV.UK Frontend spacing scale](https://design-system.service.gov.uk/styles/spacing/#the-responsive-spacing-scale) (defaults to no margin)
 - `role` - accepts a space separated string of roles
 - `lang` - accepts a language attribute value
 - `open` - accepts an open attribute value (true or false)
diff --git a/app/views/govuk_publishing_components/components/_details.html.erb b/app/views/govuk_publishing_components/components/_details.html.erb
index 10d3e3b37d..c31b5f5489 100644
--- a/app/views/govuk_publishing_components/components/_details.html.erb
+++ b/app/views/govuk_publishing_components/components/_details.html.erb
@@ -1,14 +1,12 @@
 <%
   add_gem_component_stylesheet("details")
 
-  shared_helper = GovukPublishingComponents::Presenters::SharedHelper.new(local_assigns)
   open ||= nil
   disable_ga4 ||= false
   @ga4 ||= OpenStruct.new(index_section: 0) unless disable_ga4
   @ga4[:index_section] += 1 unless disable_ga4
   ga4_attributes ||= {}
-
-  margin_bottom ||= 3
+  local_assigns[:margin_bottom] ||= 3
 
   unless disable_ga4
     ga4_event = {
@@ -25,7 +23,6 @@
 
   component_helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(local_assigns)
   component_helper.add_class("gem-c-details govuk-details")
-  component_helper.add_class(shared_helper.get_margin_bottom)
   component_helper.add_data_attribute({ module: "ga4-event-tracker" }) unless disable_ga4
   component_helper.add_data_attribute({ ga4_event: ga4_event }) unless disable_ga4
   component_helper.set_open(open)
diff --git a/app/views/govuk_publishing_components/components/_heading.html.erb b/app/views/govuk_publishing_components/components/_heading.html.erb
index 7db7b41e4b..6f00dad922 100644
--- a/app/views/govuk_publishing_components/components/_heading.html.erb
+++ b/app/views/govuk_publishing_components/components/_heading.html.erb
@@ -13,7 +13,6 @@
   classes << heading_helper.classes
   classes << brand_helper.brand_class
   classes << brand_helper.border_color_class
-  classes << shared_helper.get_margin_bottom if [*0..9].include?(local_assigns[:margin_bottom])
 
   component_helper.add_class(classes.join(" "))
   component_helper.set_id(heading_helper.id)
diff --git a/app/views/govuk_publishing_components/components/_hint.html.erb b/app/views/govuk_publishing_components/components/_hint.html.erb
index ded1bbff6e..0f9405527a 100644
--- a/app/views/govuk_publishing_components/components/_hint.html.erb
+++ b/app/views/govuk_publishing_components/components/_hint.html.erb
@@ -4,14 +4,11 @@
   local_assigns[:id] ||= "hint-#{SecureRandom.hex(4)}"
   is_radio_label_hint ||= false
   right_to_left ||= false
-  shared_helper = GovukPublishingComponents::Presenters::SharedHelper.new(local_assigns)
 
   component_helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(local_assigns)
   component_helper.add_class("gem-c-hint govuk-hint")
   component_helper.add_class("govuk-radios__hint") if is_radio_label_hint
-  component_helper.add_class(shared_helper.get_margin_bottom) if [*0..9].include?(local_assigns[:margin_bottom])
 %>
-
 <%= tag.div(**component_helper.all_attributes, dir: right_to_left ? "rtl" : nil) do %>
   <%= text %>
 <% end %>
diff --git a/docs/component-wrapper-helper.md b/docs/component-wrapper-helper.md
index 666e1ee5e9..9c22bc4838 100644
--- a/docs/component-wrapper-helper.md
+++ b/docs/component-wrapper-helper.md
@@ -29,6 +29,7 @@ These options can be passed to any component that uses the component wrapper.
 - `data_attributes` - accepts a hash of data attributes
 - `aria` - accepts a hash of aria attributes
 - `classes` - accepts a space separated string of classes, these should not be used for styling and must be prefixed with `js-`
+- `margin_bottom` - accepts a number from `0` to `9` (`0px` to `60px`) using the [GOV.UK Frontend spacing scale](https://design-system.service.gov.uk/styles/spacing/#the-responsive-spacing-scale) (defaults to no margin)
 - `role` - accepts a space separated string of roles
 - `lang` - accepts a language attribute value
 - `open` - accepts an open attribute value (true or false)
@@ -67,6 +68,7 @@ The component wrapper includes several methods to make managing options easier,
   data_attributes ||= {}
   aria_attributes ||= {}
   role ||= nil
+  local_assigns[:margin_bottom] ||= 4
 
   component_helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(local_assigns)
   component_helper.add_class("gem-c-example govuk-example") # combines the given class with any passed classes
@@ -78,6 +80,7 @@ The component wrapper includes several methods to make managing options easier,
   component_helper.set_open(true) # can pass true or false
   component_helper.set_tabindex(1)
   component_helper.set_dir("rtl")
+  component_helper.set_margin_bottom(3) # can pass any number from 1 to 9
 %>
 <%= tag.div(**component_helper.all_attributes) do %>
   component content
diff --git a/lib/govuk_publishing_components/presenters/component_wrapper_helper.rb b/lib/govuk_publishing_components/presenters/component_wrapper_helper.rb
index de44958be7..727c0c7b43 100644
--- a/lib/govuk_publishing_components/presenters/component_wrapper_helper.rb
+++ b/lib/govuk_publishing_components/presenters/component_wrapper_helper.rb
@@ -14,6 +14,7 @@ def initialize(options)
         check_hidden_is_valid(@options[:hidden]) if @options.include?(:hidden)
         check_tabindex_is_valid(@options[:tabindex]) if @options.include?(:tabindex)
         check_dir_is_valid(@options[:dir]) if @options.include?(:dir)
+        check_margin_bottom_is_valid(@options[:margin_bottom]) if @options.include?(:margin_bottom)
       end
 
       def all_attributes
@@ -22,7 +23,10 @@ def all_attributes
         attributes[:id] = @options[:id] unless @options[:id].blank?
         attributes[:data] = @options[:data_attributes] unless @options[:data_attributes].blank?
         attributes[:aria] = @options[:aria] unless @options[:aria].blank?
+
+        ((@options[:classes] ||= "") << " govuk-!-margin-bottom-#{@options[:margin_bottom]}").strip! if @options[:margin_bottom]
         attributes[:class] = @options[:classes] unless @options[:classes].blank?
+
         attributes[:role] = @options[:role] unless @options[:role].blank?
         attributes[:lang] = @options[:lang] unless @options[:lang].blank?
         attributes[:open] = @options[:open] unless @options[:open].blank?
@@ -83,6 +87,11 @@ def set_dir(dir_attribute)
         @options[:dir] = dir_attribute
       end
 
+      def set_margin_bottom(margin_bottom)
+        check_margin_bottom_is_valid(margin_bottom)
+        @options[:margin_bottom] = margin_bottom
+      end
+
     private
 
       def check_id_is_valid(id)
@@ -170,6 +179,10 @@ def check_tabindex_is_valid(tabindex_attribute)
         end
       end
 
+      def check_margin_bottom_is_valid(margin_bottom)
+        raise(ArgumentError, "margin_bottom option (#{margin_bottom}) is not recognised") unless [*0..9].include?(margin_bottom)
+      end
+
       def check_dir_is_valid(dir_attribute)
         return if dir_attribute.nil?
 
diff --git a/spec/components/details_spec.rb b/spec/components/details_spec.rb
index 9fef7f7a5e..650fb68192 100644
--- a/spec/components/details_spec.rb
+++ b/spec/components/details_spec.rb
@@ -56,15 +56,6 @@ def component_name
     assert_select '.govuk-details .govuk-details__summary[aria-label="label"]'
   end
 
-  it "defaults to the initial bottom margin if an incorrect value is passed" do
-    render_component(
-      title: "Some title",
-      margin_bottom: 12,
-    )
-
-    assert_select '.govuk-details.govuk-\!-margin-bottom-3'
-  end
-
   it "increments the GA4 index_section parameter when more than one component instance" do
     render_component(title: "first details")
     assert_select '.govuk-details[data-ga4-event=\'{"event_name":"select_content","type":"detail","text":"first details","section":"first details","index_section":1}\']'
diff --git a/spec/components/heading_spec.rb b/spec/components/heading_spec.rb
index b1caf6e752..c9aeee5d40 100644
--- a/spec/components/heading_spec.rb
+++ b/spec/components/heading_spec.rb
@@ -79,16 +79,6 @@ def component_name
     assert_select '.gem-c-heading.govuk-\!-margin-bottom-7'
   end
 
-  it "defaults to no bottom margin if an incorrect value is passed" do
-    render_component(text: "Margin wat", margin_bottom: 20)
-    assert_select "[class^='govuk-\!-margin-bottom-']", false
-  end
-
-  it "has no margin class added by default" do
-    render_component(text: "No margin")
-    assert_select "[class^='govuk-\!-margin-bottom-']", false
-  end
-
   it "adds border 1" do
     render_component(text: "Border 1", border_top: 1)
     assert_select ".gem-c-heading.gem-c-heading--border-top-1"
diff --git a/spec/components/hint_spec.rb b/spec/components/hint_spec.rb
index f5fd6261df..271f808bae 100644
--- a/spec/components/hint_spec.rb
+++ b/spec/components/hint_spec.rb
@@ -35,12 +35,6 @@ def component_name
     assert_select '.govuk-hint.govuk-\!-margin-bottom-0'
   end
 
-  it "does not default to the initial bottom margin if an incorrect value is passed" do
-    render_component(text: "For example, ‘QQ 12 34 56 C’.", margin_bottom: 12)
-
-    assert_select '.govuk-hint.govuk-\!-margin-bottom-3', false
-  end
-
   it "accepts js classes" do
     render_component(text: "For example, ‘QQ 12 34 56 C’.", classes: "js-class-one js-class-two")
 
diff --git a/spec/lib/govuk_publishing_components/components/component_wrapper_helper_spec.rb b/spec/lib/govuk_publishing_components/components/component_wrapper_helper_spec.rb
index ea8a1c4b7d..183e6da5f8 100644
--- a/spec/lib/govuk_publishing_components/components/component_wrapper_helper_spec.rb
+++ b/spec/lib/govuk_publishing_components/components/component_wrapper_helper_spec.rb
@@ -33,63 +33,6 @@
       expect(component_helper.all_attributes).to eql(expected)
     end
 
-    it "accepts valid class names" do
-      component_helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(classes: "gem-c-component govuk-component app-c-component brand--thing brand__thing direction-rtl")
-      expected = {
-        class: "gem-c-component govuk-component app-c-component brand--thing brand__thing direction-rtl",
-      }
-      expect(component_helper.all_attributes).to eql(expected)
-    end
-
-    it "rejects invalid class names" do
-      classes = "js-okay not-cool-man"
-      expect {
-        GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(classes:)
-      }.to raise_error(ArgumentError, "Classes (#{classes}) must be prefixed with `js-`")
-    end
-
-    it "rejects classes that aren't an exact match of 'direction-rtl'" do
-      classes = "direction-rtll"
-      expect {
-        GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(classes:)
-      }.to raise_error(ArgumentError, "Classes (#{classes}) must be prefixed with `js-`")
-    end
-
-    it "can set an id, overriding a passed value" do
-      helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(id: "original")
-      helper.set_id("override")
-      expect(helper.all_attributes[:id]).to eql("override")
-    end
-
-    it "does not accept invalid ids" do
-      ["1dstartingwithnumber", "id with spaces", "idwith.dot", "id\nwithnewline"].each do |id|
-        expect {
-          GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(id:)
-        }.to raise_error(ArgumentError, / contain/)
-      end
-    end
-
-    it "does not accept invalid ids when passed" do
-      expect {
-        helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(id: "valid")
-        helper.set_id("not. a. valid. id")
-      }.to raise_error(ArgumentError)
-    end
-
-    it "can add a class to already passed classes" do
-      helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(classes: "js-original")
-      helper.add_class("gem-c-extra")
-      expect(helper.all_attributes[:class]).to eql("js-original gem-c-extra")
-    end
-
-    it "will error if trying to add an invalid class to already passed classes" do
-      classes = "gem-c-extra something-invalid"
-      expect {
-        helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(classes: "js-original")
-        helper.add_class(classes)
-      }.to raise_error(ArgumentError, "Classes (#{classes}) must be prefixed with `js-`")
-    end
-
     it "does not error if passed blank values" do
       component_helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(
         id: nil,
@@ -112,201 +55,287 @@
       expect(component_helper.all_attributes).to eql({})
     end
 
-    it "does not accept invalid data attributes" do
-      invalid_data = { module: "ok", xml_something: "notok", "XML_something": "notok", "has space": "notok", "has:colon": "notok", normal: "ok" }
-      error = "Data attributes (xml_something, XML_something, has space, has:colon) cannot contain capitals, spaces or colons, or start with 'xml'"
-      expect {
-        GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(data_attributes: invalid_data)
-      }.to raise_error(ArgumentError, error)
+    describe "classes" do
+      it "accepts valid class names" do
+        component_helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(classes: "gem-c-component govuk-component app-c-component brand--thing brand__thing direction-rtl")
+        expected = {
+          class: "gem-c-component govuk-component app-c-component brand--thing brand__thing direction-rtl",
+        }
+        expect(component_helper.all_attributes).to eql(expected)
+      end
 
-      expect {
-        helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(data_attributes: { module: "something" })
-        helper.add_data_attribute(invalid_data)
-      }.to raise_error(ArgumentError, error)
-    end
+      it "rejects invalid class names" do
+        classes = "js-okay not-cool-man"
+        expect {
+          GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(classes:)
+        }.to raise_error(ArgumentError, "Classes (#{classes}) must be prefixed with `js-`")
+      end
 
-    it "can add data attributes to already passed data attributes" do
-      helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(data_attributes: { module: "original-module", other: "other" })
-      helper.add_data_attribute({ module: "extra-module", another: "another" })
-      expect(helper.all_attributes[:data]).to eql({ module: "original-module extra-module", other: "other", another: "another" })
+      it "rejects classes that aren't an exact match of 'direction-rtl'" do
+        classes = "direction-rtll"
+        expect {
+          GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(classes:)
+        }.to raise_error(ArgumentError, "Classes (#{classes}) must be prefixed with `js-`")
+      end
     end
 
-    it "can add aria attributes to already passed aria attributes" do
-      helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(aria: { label: "original-label", describedby: "other" })
-      helper.add_aria_attribute({ label: "extra-label", controls: "something" })
-      expect(helper.all_attributes[:aria]).to eql({ label: "original-label extra-label", describedby: "other", controls: "something" })
-    end
+    describe "setting an id" do
+      it "can set an id, overriding a passed value" do
+        helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(id: "original")
+        helper.set_id("override")
+        expect(helper.all_attributes[:id]).to eql("override")
+      end
 
-    it "does not accept invalid aria attributes" do
-      invalid_aria = { potato: "salad", label: "something", spoon: "invalid" }
-      error = "Aria attribute (potato, spoon) not recognised"
-      expect {
-        GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(aria: invalid_aria)
-      }.to raise_error(ArgumentError, error)
+      it "does not accept invalid ids" do
+        ["1dstartingwithnumber", "id with spaces", "idwith.dot", "id\nwithnewline"].each do |id|
+          expect {
+            GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(id:)
+          }.to raise_error(ArgumentError, / contain/)
+        end
+      end
 
-      expect {
-        helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(aria: { label: "something" })
-        helper.add_aria_attribute(invalid_aria)
-      }.to raise_error(ArgumentError, error)
+      it "does not accept invalid ids when passed" do
+        expect {
+          helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(id: "valid")
+          helper.set_id("not. a. valid. id")
+        }.to raise_error(ArgumentError)
+      end
     end
 
-    it "does not accept an invalid role" do
-      error = "Role attribute (custarddoughnuts, moose) is not recognised"
-      expect {
-        GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(role: "custarddoughnuts moose")
-      }.to raise_error(ArgumentError, error)
+    describe "classes" do
+      it "can add a class to already passed classes" do
+        helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(classes: "js-original")
+        helper.add_class("gem-c-extra")
+        expect(helper.all_attributes[:class]).to eql("js-original gem-c-extra")
+      end
 
-      expect {
-        GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(role: "navigation custarddoughnuts moose")
-      }.to raise_error(ArgumentError, error)
+      it "will error if trying to add an invalid class to already passed classes" do
+        classes = "gem-c-extra something-invalid"
+        expect {
+          helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(classes: "js-original")
+          helper.add_class(classes)
+        }.to raise_error(ArgumentError, "Classes (#{classes}) must be prefixed with `js-`")
+      end
     end
 
-    it "does not accept an invalid role when passed" do
-      error = "Role attribute (custarddoughnuts, moose) is not recognised"
-      expect {
-        helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(role: "navigation")
-        helper.add_role("custarddoughnuts moose")
-      }.to raise_error(ArgumentError, error)
+    describe "data attributes" do
+      it "does not accept invalid data attributes" do
+        invalid_data = { module: "ok", xml_something: "notok", "XML_something": "notok", "has space": "notok", "has:colon": "notok", normal: "ok" }
+        error = "Data attributes (xml_something, XML_something, has space, has:colon) cannot contain capitals, spaces or colons, or start with 'xml'"
+        expect {
+          GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(data_attributes: invalid_data)
+        }.to raise_error(ArgumentError, error)
 
-      expect {
-        helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(role: "navigation")
-        helper.add_role("alert custarddoughnuts moose")
-      }.to raise_error(ArgumentError, error)
-    end
+        expect {
+          helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(data_attributes: { module: "something" })
+          helper.add_data_attribute(invalid_data)
+        }.to raise_error(ArgumentError, error)
+      end
 
-    it "does not accept an invalid lang" do
-      error = "lang attribute (klingon) is not recognised"
-      expect {
-        GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(lang: "klingon")
-      }.to raise_error(ArgumentError, error)
+      it "can add data attributes to already passed data attributes" do
+        helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(data_attributes: { module: "original-module", other: "other" })
+        helper.add_data_attribute({ module: "extra-module", another: "another" })
+        expect(helper.all_attributes[:data]).to eql({ module: "original-module extra-module", other: "other", another: "another" })
+      end
     end
 
-    it "accepts valid open attribute value" do
-      component_helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(open: true)
-      expected = {
-        open: true,
-      }
-      expect(component_helper.all_attributes).to eql(expected)
-    end
+    describe "aria attributes" do
+      it "can add aria attributes to already passed aria attributes" do
+        helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(aria: { label: "original-label", describedby: "other" })
+        helper.add_aria_attribute({ label: "extra-label", controls: "something" })
+        expect(helper.all_attributes[:aria]).to eql({ label: "original-label extra-label", describedby: "other", controls: "something" })
+      end
 
-    it "can set an open attribute, overriding a passed value" do
-      helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(open: true)
-      helper.set_open(false)
-      expect(helper.all_attributes[:open]).to eql(nil)
-    end
+      it "does not accept invalid aria attributes" do
+        invalid_aria = { potato: "salad", label: "something", spoon: "invalid" }
+        error = "Aria attribute (potato, spoon) not recognised"
+        expect {
+          GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(aria: invalid_aria)
+        }.to raise_error(ArgumentError, error)
 
-    it "does not include an open attribute if the option is false" do
-      component_helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(open: false)
-      expect(component_helper.all_attributes).to eql({})
+        expect {
+          helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(aria: { label: "something" })
+          helper.add_aria_attribute(invalid_aria)
+        }.to raise_error(ArgumentError, error)
+      end
     end
 
-    it "does not accept an invalid open value" do
-      error = "open attribute (false) is not recognised"
-      expect {
-        GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(open: "false")
-      }.to raise_error(ArgumentError, error)
-    end
+    describe "roles" do
+      it "does not accept an invalid role" do
+        error = "Role attribute (custarddoughnuts, moose) is not recognised"
+        expect {
+          GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(role: "custarddoughnuts moose")
+        }.to raise_error(ArgumentError, error)
 
-    it "does not accept an invalid hidden value" do
-      error = "hidden attribute (false) is not recognised"
-      expect {
-        GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(hidden: "false")
-      }.to raise_error(ArgumentError, error)
-    end
+        expect {
+          GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(role: "navigation custarddoughnuts moose")
+        }.to raise_error(ArgumentError, error)
+      end
 
-    it "accepts valid hidden attribute value" do
-      component_helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(hidden: "until-found")
-      expected = {
-        hidden: "until-found",
-      }
-      expect(component_helper.all_attributes).to eql(expected)
-    end
+      it "does not accept an invalid role when passed" do
+        error = "Role attribute (custarddoughnuts, moose) is not recognised"
+        expect {
+          helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(role: "navigation")
+          helper.add_role("custarddoughnuts moose")
+        }.to raise_error(ArgumentError, error)
 
-    it "does not accept an invalid dir value" do
-      error = "dir attribute (false) is not recognised"
-      expect {
-        GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(dir: "false")
-      }.to raise_error(ArgumentError, error)
+        expect {
+          helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(role: "navigation")
+          helper.add_role("alert custarddoughnuts moose")
+        }.to raise_error(ArgumentError, error)
+      end
     end
 
-    it "accepts valid dir attribute value" do
-      component_helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(dir: "rtl")
-      expected = {
-        dir: "rtl",
-      }
-      expect(component_helper.all_attributes).to eql(expected)
-    end
+    describe "lang" do
+      it "does not accept an invalid lang" do
+        error = "lang attribute (klingon) is not recognised"
+        expect {
+          GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(lang: "klingon")
+        }.to raise_error(ArgumentError, error)
+      end
 
-    it "can set an dir attribute, overriding a passed value" do
-      helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(dir: "rtl")
-      helper.set_dir("ltr")
-      expect(helper.all_attributes[:dir]).to eql("ltr")
+      it "accepts valid open attribute value" do
+        component_helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(open: true)
+        expect(component_helper.all_attributes[:open]).to eql(true)
+      end
     end
 
-    it "can set an hidden attribute, overriding a passed value" do
-      helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(hidden: "until-found")
-      helper.set_hidden("hidden")
-      expect(helper.all_attributes[:hidden]).to eql("hidden")
-    end
+    describe "open" do
+      it "can set an open attribute, overriding a passed value" do
+        helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(open: true)
+        helper.set_open(false)
+        expect(helper.all_attributes[:open]).to eql(nil)
+      end
 
-    it "can set an tabindex attribute, overriding a passed value" do
-      helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(tabindex: -1)
-      helper.set_tabindex("1")
-      expect(helper.all_attributes[:tabindex]).to eql("1")
+      it "does not include an open attribute if the option is false" do
+        component_helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(open: false)
+        expect(component_helper.all_attributes[:open]).to eql(nil)
+      end
+
+      it "does not accept an invalid open value" do
+        error = "open attribute (false) is not recognised"
+        expect {
+          GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(open: "false")
+        }.to raise_error(ArgumentError, error)
+      end
     end
 
-    describe "tabindex value regex" do
-      it "accepts string numbers" do
-        component_helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(tabindex: "-984347284732")
-        expected = {
-          tabindex: "-984347284732",
-        }
-        expect(component_helper.all_attributes).to eql(expected)
+    describe "hidden" do
+      it "does not accept an invalid hidden value" do
+        error = "hidden attribute (false) is not recognised"
+        expect {
+          GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(hidden: "false")
+        }.to raise_error(ArgumentError, error)
       end
 
-      it "accepts integer numbers" do
-        component_helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(tabindex: -984_347_284_732)
-        expected = {
-          tabindex: -984_347_284_732,
-        }
-        expect(component_helper.all_attributes).to eql(expected)
+      it "accepts valid hidden attribute value" do
+        component_helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(hidden: "until-found")
+        expect(component_helper.all_attributes[:hidden]).to eql("until-found")
       end
 
-      it "accepts 0" do
-        component_helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(tabindex: "0")
-        expected = {
-          tabindex: "0",
-        }
-        expect(component_helper.all_attributes).to eql(expected)
+      it "can set a hidden attribute, overriding a passed value" do
+        helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(hidden: "until-found")
+        helper.set_hidden("hidden")
+        expect(helper.all_attributes[:hidden]).to eql("hidden")
       end
+    end
 
-      it "does not accept text before a number" do
-        error = "tabindex_attribute attribute (abc1) is not recognised"
+    describe "dir" do
+      it "does not accept an invalid dir value" do
+        error = "dir attribute (false) is not recognised"
         expect {
-          GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(tabindex: "abc1")
+          GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(dir: "false")
         }.to raise_error(ArgumentError, error)
       end
 
-      it "does not accept text after a number" do
-        error = "tabindex_attribute attribute (123abc) is not recognised"
-        expect {
-          GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(tabindex: "123abc")
-        }.to raise_error(ArgumentError, error)
+      it "accepts valid dir attribute value" do
+        component_helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(dir: "rtl")
+        expect(component_helper.all_attributes[:dir]).to eql("rtl")
       end
 
-      it "does not accept extra negative symbols" do
-        error = "tabindex_attribute attribute (--1) is not recognised"
-        expect {
-          GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(tabindex: "--1")
-        }.to raise_error(ArgumentError, error)
+      it "can set an dir attribute, overriding a passed value" do
+        helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(dir: "rtl")
+        helper.set_dir("ltr")
+        expect(helper.all_attributes[:dir]).to eql("ltr")
       end
+    end
 
-      it "does not accept extra symbols" do
-        error = "tabindex_attribute attribute (-1!???) is not recognised"
-        expect {
-          GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(tabindex: "-1!???")
-        }.to raise_error(ArgumentError, error)
+    describe "tabindex" do
+      it "can set an tabindex attribute, overriding a passed value" do
+        helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(tabindex: -1)
+        helper.set_tabindex("1")
+        expect(helper.all_attributes[:tabindex]).to eql("1")
       end
+
+      describe "tabindex value regex" do
+        it "accepts string numbers" do
+          component_helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(tabindex: "-984347284732")
+          expect(component_helper.all_attributes[:tabindex]).to eql("-984347284732")
+        end
+
+        it "accepts integer numbers" do
+          component_helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(tabindex: -984_347_284_732)
+          expect(component_helper.all_attributes[:tabindex]).to eql(-984_347_284_732)
+        end
+
+        it "accepts 0" do
+          component_helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(tabindex: "0")
+          expect(component_helper.all_attributes[:tabindex]).to eql("0")
+        end
+
+        it "does not accept text before a number" do
+          error = "tabindex_attribute attribute (abc1) is not recognised"
+          expect {
+            GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(tabindex: "abc1")
+          }.to raise_error(ArgumentError, error)
+        end
+
+        it "does not accept text after a number" do
+          error = "tabindex_attribute attribute (123abc) is not recognised"
+          expect {
+            GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(tabindex: "123abc")
+          }.to raise_error(ArgumentError, error)
+        end
+
+        it "does not accept extra negative symbols" do
+          error = "tabindex_attribute attribute (--1) is not recognised"
+          expect {
+            GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(tabindex: "--1")
+          }.to raise_error(ArgumentError, error)
+        end
+
+        it "does not accept extra symbols" do
+          error = "tabindex_attribute attribute (-1!???) is not recognised"
+          expect {
+            GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(tabindex: "-1!???")
+          }.to raise_error(ArgumentError, error)
+        end
+      end
+    end
+  end
+
+  describe "margins" do
+    it "complains about an invalid margin" do
+      error = "margin_bottom option (15) is not recognised"
+      expect {
+        GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(margin_bottom: 15)
+      }.to raise_error(ArgumentError, error)
+    end
+
+    it "defaults to no margin" do
+      helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new({})
+      expect(helper.all_attributes[:class]).to eql(nil)
+    end
+
+    it "accepts a passed margin" do
+      helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(margin_bottom: 5)
+      expect(helper.all_attributes[:class]).to eql("govuk-!-margin-bottom-5")
+    end
+
+    it "can set a margin, overriding a passed margin" do
+      helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(margin_bottom: 5)
+      helper.set_margin_bottom(0)
+      expect(helper.all_attributes[:class]).to eql("govuk-!-margin-bottom-0")
     end
   end
 end

From bfad545764eacbc42f638f910a19638d166278b1 Mon Sep 17 00:00:00 2001
From: Andy Sellick <andy.sellick@digital.cabinet-office.gov.uk>
Date: Wed, 11 Dec 2024 17:04:56 +0000
Subject: [PATCH 2/3] Switch components to new system

- switch any components using the shared_helper for margin_bottom, that also use the component wrapper helper, to not use the shared_helper for margin_bottom
- note that some components still use the shared_helper for margin_bottom, but aren't using the component wrapper helper yet, so can't be switched
---
 .../components/_accordion.html.erb                         | 2 --
 .../components/_action_link.html.erb                       | 3 ---
 .../govuk_publishing_components/components/_chart.html.erb | 3 +--
 .../components/_chat_entry.html.erb                        | 2 +-
 .../components/_contents_list.html.erb                     | 2 --
 .../components/_contextual_sidebar.html.erb                | 1 -
 .../components/_document_list.html.erb                     | 2 --
 .../components/_govspeak.html.erb                          | 2 --
 .../components/_inset_text.html.erb                        | 4 +---
 .../components/_lead_paragraph.html.erb                    | 2 --
 .../components/_metadata.html.erb                          | 3 ---
 .../components/_notice.html.erb                            | 2 --
 .../components/_password_input.html.erb                    | 4 +---
 .../govuk_publishing_components/components/_radio.html.erb | 2 --
 .../components/_signup_link.html.erb                       | 1 -
 .../components/_single_page_notification_button.html.erb   | 7 +++----
 16 files changed, 7 insertions(+), 35 deletions(-)

diff --git a/app/views/govuk_publishing_components/components/_accordion.html.erb b/app/views/govuk_publishing_components/components/_accordion.html.erb
index d471e2e25d..7f265a5d21 100644
--- a/app/views/govuk_publishing_components/components/_accordion.html.erb
+++ b/app/views/govuk_publishing_components/components/_accordion.html.erb
@@ -13,8 +13,6 @@
   component_helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(local_assigns)
   component_helper.set_id(id)
   component_helper.add_class("gem-c-accordion govuk-accordion")
-  component_helper.add_class(shared_helper.get_margin_bottom)
-
   component_helper.add_data_attribute({ module: "govuk-accordion gem-accordion" })
   component_helper.add_data_attribute({ module: "ga4-event-tracker" }) unless disable_ga4
   component_helper.add_data_attribute({ ga4_expandable: "" }) unless disable_ga4
diff --git a/app/views/govuk_publishing_components/components/_action_link.html.erb b/app/views/govuk_publishing_components/components/_action_link.html.erb
index cdebf98ef8..db3543af75 100644
--- a/app/views/govuk_publishing_components/components/_action_link.html.erb
+++ b/app/views/govuk_publishing_components/components/_action_link.html.erb
@@ -2,8 +2,6 @@
   add_gem_component_stylesheet("action-link")
 
   local_assigns[:margin_bottom] ||= 0
-  shared_helper = GovukPublishingComponents::Presenters::SharedHelper.new(local_assigns)
-
   text ||= false
   nowrap_text ||= false
   href ||= false
@@ -26,7 +24,6 @@
   css_classes << "gem-c-action-link--simple-light" if simple_light
   css_classes << "gem-c-action-link--with-subtext" if subtext
   css_classes << "gem-c-action-link--mobile-subtext" if mobile_subtext
-  css_classes << shared_helper.get_margin_bottom
 
   link_classes = %w(govuk-link gem-c-action-link__link gem-c-force-print-link-styles)
   link_classes << "govuk-link--inverse" if inverse
diff --git a/app/views/govuk_publishing_components/components/_chart.html.erb b/app/views/govuk_publishing_components/components/_chart.html.erb
index ac76e283bb..fd04667772 100644
--- a/app/views/govuk_publishing_components/components/_chart.html.erb
+++ b/app/views/govuk_publishing_components/components/_chart.html.erb
@@ -14,6 +14,7 @@
   minimal ||= false
   hide_heading ||= minimal
   link ||= false
+  local_assigns[:margin_bottom] ||= 3
 
   chart_id = "chart-id-#{SecureRandom.hex(4)}"
   table_id = "table-id-#{SecureRandom.hex(4)}"
@@ -21,10 +22,8 @@
   @external_script[:loaded] += 1
 
   chart_helper = GovukPublishingComponents::Presenters::ChartHelper.new(local_assigns)
-  shared_helper = GovukPublishingComponents::Presenters::SharedHelper.new(local_assigns)
   component_helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(local_assigns)
   component_helper.add_class("gem-c-chart")
-  component_helper.add_class(shared_helper.get_margin_bottom)
   component_helper.add_class("gem-c-chart--minimal") if minimal
 
   require "chartkick"
diff --git a/app/views/govuk_publishing_components/components/_chat_entry.html.erb b/app/views/govuk_publishing_components/components/_chat_entry.html.erb
index 81055461a4..4d93701f31 100644
--- a/app/views/govuk_publishing_components/components/_chat_entry.html.erb
+++ b/app/views/govuk_publishing_components/components/_chat_entry.html.erb
@@ -9,13 +9,13 @@
   border_bottom ||= false
   disable_ga4 ||= false
   margin_top_until_tablet ||= false
+  local_assigns[:margin_bottom] ||= 6
 
   component_helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(local_assigns)
   component_helper.add_class("gem-c-chat-entry")
   component_helper.add_class("gem-c-chat-entry--border-top") if border_top
   component_helper.add_class("gem-c-chat-entry--border-bottom") if border_bottom
   component_helper.add_class("gem-c-chat-entry--margin-top-until-tablet") if margin_top_until_tablet
-  component_helper.add_class(shared_helper.get_margin_bottom)
   component_helper.add_data_attribute({ module: "ga4-link-tracker" }) unless disable_ga4
 
   unless disable_ga4
diff --git a/app/views/govuk_publishing_components/components/_contents_list.html.erb b/app/views/govuk_publishing_components/components/_contents_list.html.erb
index cd887b4a60..c501475921 100644
--- a/app/views/govuk_publishing_components/components/_contents_list.html.erb
+++ b/app/views/govuk_publishing_components/components/_contents_list.html.erb
@@ -9,7 +9,6 @@
   local_assigns[:aria] ||= {}
   local_assigns[:margin_bottom] ||= 4
 
-  shared_helper = GovukPublishingComponents::Presenters::SharedHelper.new(local_assigns)
   cl_helper = GovukPublishingComponents::Presenters::ContentsListHelper.new(local_assigns)
   brand_helper = GovukPublishingComponents::AppHelpers::BrandHelper.new(brand)
 
@@ -29,7 +28,6 @@
   component_helper.add_class("gem-c-contents-list #{brand_helper.brand_class}")
   component_helper.add_class("gem-c-contents-list--alternative-line-style") if alternative_line_style
   component_helper.add_class("gem-c-contents-list--custom-title") if title
-  component_helper.add_class(shared_helper.get_margin_bottom)
   component_helper.add_data_attribute({ module: "ga4-link-tracker" }) unless disable_ga4
   component_helper.add_aria_attribute({ label: t("components.contents_list.contents") }) unless local_assigns[:aria][:label]
   component_helper.add_role("navigation")
diff --git a/app/views/govuk_publishing_components/components/_contextual_sidebar.html.erb b/app/views/govuk_publishing_components/components/_contextual_sidebar.html.erb
index bd2d194927..4373161694 100644
--- a/app/views/govuk_publishing_components/components/_contextual_sidebar.html.erb
+++ b/app/views/govuk_publishing_components/components/_contextual_sidebar.html.erb
@@ -4,7 +4,6 @@
   disable_ga4 ||= false
   request.query_parameters[:disable_ga4] = disable_ga4
   navigation = GovukPublishingComponents::Presenters::ContextualNavigation.new(content_item, request)
-  shared_helper = GovukPublishingComponents::Presenters::SharedHelper.new(local_assigns)
   show_ukraine_cta = navigation.show_ukraine_cta?
   ga4_tracking_counts = OpenStruct.new(index_section_count: 0)
   ga4_tracking_counts.index_section_count = 1 if show_ukraine_cta
diff --git a/app/views/govuk_publishing_components/components/_document_list.html.erb b/app/views/govuk_publishing_components/components/_document_list.html.erb
index 09581b1b68..5133d0d6b3 100644
--- a/app/views/govuk_publishing_components/components/_document_list.html.erb
+++ b/app/views/govuk_publishing_components/components/_document_list.html.erb
@@ -2,7 +2,6 @@
   add_gem_component_stylesheet("document-list")
 
   local_assigns[:margin_bottom] ||= 5
-  shared_helper = GovukPublishingComponents::Presenters::SharedHelper.new(local_assigns)
   items ||= []
 
   component_helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(local_assigns)
@@ -10,7 +9,6 @@
   component_helper.add_class("gem-c-document-list--no-top-border") if local_assigns[:remove_top_border]
   component_helper.add_class("gem-c-document-list--no-top-border-first-child") if local_assigns[:remove_top_border_from_first_child]
   component_helper.add_class("gem-c-document-list--equal-item-spacing") if local_assigns[:equal_item_spacing]
-  component_helper.add_class(shared_helper.get_margin_bottom)
 
   extra_link_classes = "govuk-link--no-underline" if local_assigns[:remove_underline]
   title_with_context_class = " gem-c-document-list__item-title--context"
diff --git a/app/views/govuk_publishing_components/components/_govspeak.html.erb b/app/views/govuk_publishing_components/components/_govspeak.html.erb
index bc78cb31a8..bb4541beaf 100644
--- a/app/views/govuk_publishing_components/components/_govspeak.html.erb
+++ b/app/views/govuk_publishing_components/components/_govspeak.html.erb
@@ -6,13 +6,11 @@
   direction_class = "gem-c-govspeak--direction-#{direction}" if local_assigns.include?(:direction)
   disable_youtube_expansions = local_assigns.fetch(:disable_youtube_expansions) if local_assigns.include?(:disable_youtube_expansions)
 
-  shared_helper = GovukPublishingComponents::Presenters::SharedHelper.new(local_assigns)
   component_helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(local_assigns)
   component_helper.add_class("gem-c-govspeak govuk-govspeak")
   component_helper.add_class(direction_class) if direction_class
   component_helper.add_class("js-disable-youtube") if disable_youtube_expansions
   component_helper.add_class("gem-c-govspeak--inverse") if inverse
-  component_helper.add_class(shared_helper.get_margin_bottom)
   component_helper.add_data_attribute({ module: "govspeak" })
 %>
 <%= tag.div(**component_helper.all_attributes) do %>
diff --git a/app/views/govuk_publishing_components/components/_inset_text.html.erb b/app/views/govuk_publishing_components/components/_inset_text.html.erb
index 19934bb788..968307a887 100644
--- a/app/views/govuk_publishing_components/components/_inset_text.html.erb
+++ b/app/views/govuk_publishing_components/components/_inset_text.html.erb
@@ -3,17 +3,15 @@
 
   id ||= "inset-text-#{SecureRandom.hex(4)}"
   margin_top ||= 6
-  margin_bottom ||= 6
+  local_assigns[:margin_bottom] ||= 6
 
   shared_helper = GovukPublishingComponents::Presenters::SharedHelper.new({
     margin_top: margin_top,
-    margin_bottom: margin_bottom
   })
 
   component_helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(local_assigns)
   component_helper.add_class("gem-c-inset-text govuk-inset-text gem-c-force-print-link-styles-within")
   component_helper.add_class(shared_helper.get_margin_top)
-  component_helper.add_class(shared_helper.get_margin_bottom)
   component_helper.set_id(id)
 %>
 <%= tag.div(**component_helper.all_attributes) do %>
diff --git a/app/views/govuk_publishing_components/components/_lead_paragraph.html.erb b/app/views/govuk_publishing_components/components/_lead_paragraph.html.erb
index 0a16de3502..9dfdad1f07 100644
--- a/app/views/govuk_publishing_components/components/_lead_paragraph.html.erb
+++ b/app/views/govuk_publishing_components/components/_lead_paragraph.html.erb
@@ -3,12 +3,10 @@
 
   text ||= ""
   inverse ||= false
-  shared_helper = GovukPublishingComponents::Presenters::SharedHelper.new(local_assigns)
 
   component_helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(local_assigns)
   component_helper.add_class("gem-c-lead-paragraph")
   component_helper.add_class("gem-c-lead-paragraph--inverse") if inverse
-  component_helper.add_class(shared_helper.get_margin_bottom) if [*0..9].include?(local_assigns[:margin_bottom])
 %>
 <% if text.present? %>
   <%= tag.p(**component_helper.all_attributes) do %>
diff --git a/app/views/govuk_publishing_components/components/_metadata.html.erb b/app/views/govuk_publishing_components/components/_metadata.html.erb
index f95d8c0183..d4aae19cb4 100644
--- a/app/views/govuk_publishing_components/components/_metadata.html.erb
+++ b/app/views/govuk_publishing_components/components/_metadata.html.erb
@@ -16,8 +16,6 @@
   direction_class = ""
   direction_class = " direction-#{direction}" if local_assigns.include?(:direction)
 
-  shared_helper = GovukPublishingComponents::Presenters::SharedHelper.new(local_assigns)
-
   component_helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(local_assigns)
   component_helper.add_class("gem-c-metadata")
   component_helper.add_class("direction-#{direction}") if local_assigns.include?(:direction)
@@ -27,7 +25,6 @@
     component_helper.add_class("gem-c-metadata--inverse-padded") unless inverse_compress
   end
 
-  component_helper.add_class(shared_helper.get_margin_bottom) if local_assigns[:margin_bottom]
   component_helper.add_data_attribute({ module: "metadata" })
 
   disable_ga4 ||= false
diff --git a/app/views/govuk_publishing_components/components/_notice.html.erb b/app/views/govuk_publishing_components/components/_notice.html.erb
index 20cb6efcb9..47e623ab21 100644
--- a/app/views/govuk_publishing_components/components/_notice.html.erb
+++ b/app/views/govuk_publishing_components/components/_notice.html.erb
@@ -15,10 +15,8 @@
   show_banner_title ||= false
   heading_level = show_banner_title ? "h3" : "h2"
 
-  shared_helper = GovukPublishingComponents::Presenters::SharedHelper.new(local_assigns)
   component_helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(local_assigns)
   component_helper.add_class("govuk-notification-banner gem-c-notice")
-  component_helper.add_class(shared_helper.get_margin_bottom)
 
   component_helper.add_aria_attribute({ label: "Notice" })
   component_helper.add_aria_attribute({ live: "polite" }) if aria_live
diff --git a/app/views/govuk_publishing_components/components/_password_input.html.erb b/app/views/govuk_publishing_components/components/_password_input.html.erb
index e211ebb50d..da633aa0fa 100644
--- a/app/views/govuk_publishing_components/components/_password_input.html.erb
+++ b/app/views/govuk_publishing_components/components/_password_input.html.erb
@@ -2,7 +2,7 @@
   add_gem_component_stylesheet("password-input")
 
   label_text ||= t("components.password_input.label")
-
+  local_assigns[:margin_bottom] ||= 3
   error_text ||= nil
   error_text_prefix ||= t("components.password_input.error_text_prefix")
 
@@ -15,7 +15,6 @@
 
   name ||= "password"
 
-  shared_helper = GovukPublishingComponents::Presenters::SharedHelper.new(local_assigns)
   component_helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(local_assigns)
   component_helper.add_data_attribute({
     module: "govuk-password-input",
@@ -30,7 +29,6 @@
   component_helper.add_class('govuk-form-group--error') if error_text
   component_helper.add_class('govuk-password-input')
   component_helper.add_class('gem-c-password-input')
-  component_helper.add_class(shared_helper.get_margin_bottom)
 
   uid = SecureRandom.hex(4)
 
diff --git a/app/views/govuk_publishing_components/components/_radio.html.erb b/app/views/govuk_publishing_components/components/_radio.html.erb
index ed93fb9a77..516a439f47 100644
--- a/app/views/govuk_publishing_components/components/_radio.html.erb
+++ b/app/views/govuk_publishing_components/components/_radio.html.erb
@@ -31,7 +31,6 @@
   hint ||= nil
   error_message ||= nil
   error_items ||= []
-  margin_bottom = margin_bottom ||= 6
 
   has_error = error_message || error_items.any?
   hint_id = "hint-#{SecureRandom.hex(4)}" if hint
@@ -40,7 +39,6 @@
   component_helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(local_assigns)
   component_helper.add_class("govuk-form-group")
   component_helper.add_class("govuk-form-group--error") if has_error
-  component_helper.add_class(shared_helper.get_margin_bottom)
   component_helper.set_id(id)
 
   radio_classes = %w(govuk-radios)
diff --git a/app/views/govuk_publishing_components/components/_signup_link.html.erb b/app/views/govuk_publishing_components/components/_signup_link.html.erb
index 7f5c59ea5f..283f42c457 100644
--- a/app/views/govuk_publishing_components/components/_signup_link.html.erb
+++ b/app/views/govuk_publishing_components/components/_signup_link.html.erb
@@ -12,7 +12,6 @@
   shared_helper = GovukPublishingComponents::Presenters::SharedHelper.new(local_assigns)
   component_helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(local_assigns)
   component_helper.add_class("gem-c-signup-link govuk-!-display-none-print")
-  component_helper.add_class(shared_helper.get_margin_bottom)
   component_helper.add_class("gem-c-signup-link--link-only") unless heading
   component_helper.add_class("gem-c-signup-link--with-background-and-border") if background
 %>
diff --git a/app/views/govuk_publishing_components/components/_single_page_notification_button.html.erb b/app/views/govuk_publishing_components/components/_single_page_notification_button.html.erb
index fff3d45f58..124d4b0b14 100644
--- a/app/views/govuk_publishing_components/components/_single_page_notification_button.html.erb
+++ b/app/views/govuk_publishing_components/components/_single_page_notification_button.html.erb
@@ -1,18 +1,17 @@
 <%
   add_gem_component_stylesheet("single-page-notification-button")
 
+  local_assigns[:margin_bottom] ||= 3
+
   spnb_helper = GovukPublishingComponents::Presenters::SinglePageNotificationButtonHelper.new(local_assigns)
   shared_helper = GovukPublishingComponents::Presenters::SharedHelper.new(local_assigns)
   component_helper = GovukPublishingComponents::Presenters::ComponentWrapperHelper.new(local_assigns)
 
-  wrapper_classes = %w(govuk-!-display-none-print)
-  wrapper_classes << shared_helper.get_margin_bottom
-
   ga4_data_attributes = ga4_data_attributes ||= nil
   ga4_link_data_attributes = ga4_data_attributes[:ga4_link] if ga4_data_attributes
   ga4_link_data_attributes[:url] = spnb_helper.form_action if ga4_link_data_attributes
 
-  component_helper.add_class(wrapper_classes.join(" "))
+  component_helper.add_class("govuk-!-display-none-print")
   component_helper.add_data_attribute({ module: ga4_data_attributes[:module] }) if ga4_data_attributes
 %>
 <% button_text = capture do %>

From fbeaf6f8936736efacb9587639398e9b5c6d151e Mon Sep 17 00:00:00 2001
From: Andy Sellick <andy.sellick@digital.cabinet-office.gov.uk>
Date: Fri, 13 Dec 2024 10:38:45 +0000
Subject: [PATCH 3/3] Update changelog

---
 CHANGELOG.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 333a52c2b1..c144f4c936 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -9,6 +9,7 @@
 
 ## Unreleased
 
+* Add margin_bottom option to component wrapper helper ([PR #4494](https://github.com/alphagov/govuk_publishing_components/pull/4494))
 * Limit GA4 search term tracking to 500 characters ([PR #4496](https://github.com/alphagov/govuk_publishing_components/pull/4496))
 
 ## 46.3.1