Improve layout of contacts content

app/assets/stylesheets/views/_contact.scss

@@ -2,4 +2,13 @@
   .add-title-margin {
     @include responsive-top-margin;
   }
+
+  // FIXME: Overrides specific govspeak heading styles while h3s & h4s look similar
+  // Govspeak component renders h3s and h4s at the same font size with different margins
+  // Reduce the visual importance of specific h4s by making them a normal font weight.
+  // This gives content correct visual hierarchy and semantics
+  // `normal-weight` is not a class used by the govspeak component
+  .govuk-govspeak .normal-weight {
+    font-weight: normal;
+  }
 }

app/presenters/contact_presenter.rb

@@ -72,7 +72,7 @@ def phone
   end
 
   def phone_body
-    content_item["details"]["more_info_phone_number"].html_safe
+    content_item.dig("details", "more_info_phone_number").try(:html_safe)
   end
 
   def post
@@ -95,7 +95,7 @@ def post
   end
 
   def post_body
-    content_item["details"]["more_info_post_address"].html_safe
+    content_item.dig("details", "more_info_post_address").try(:html_safe)
   end
 
   def email
@@ -112,7 +112,7 @@ def email
   end
 
   def email_body
-    content_item["details"]["more_info_email_address"].html_safe
+    content_item.dig("details", "more_info_email_address").try(:html_safe)
   end
 
   def show_webchat?

app/views/content_items/contact.html.erb

@@ -44,27 +44,26 @@
       <% if @content_item.phone.any? %>
         <h2 id="phone-title">Phone</h2>
         <% @content_item.phone.each do |phone_group| %>
-          <h3><%= phone_group[:title] %></h3>
-
+          <% if @content_item.phone.size > 1 %>
+            <h3><%= phone_group[:title] %></h3>
+          <% end %>
+          <%= phone_group[:description] %>
           <% if phone_group[:numbers].any? %>
             <div class="contact">
               <div class="content">
                 <% phone_group[:numbers].each do |number| %>
-                  <p><%= number[:label] %>:<br /><%= number[:number] %></p>
+                  <p><%= number[:label] %>:<br /><strong><%= number[:number] %></strong></p>
                 <% end%>
               </div>
             </div>
           <% end %>
-
-          <%= phone_group[:description] %>
-
           <% if phone_group[:opening_times].present? %>
-            <h4>Opening times</h4>
+            <h4 class="normal-weight">Opening times:</h4>
             <%= phone_group[:opening_times] %>
           <% end %>
 
           <% if phone_group[:best_time_to_call].present? %>
-            <h4>Best time to call</h4>
+            <h4 class="normal-weight">Best time to call:</h4>
             <%= phone_group[:best_time_to_call] %>
           <% end %>
         <% end %>

test/integration/contact_test.rb

@@ -31,16 +31,27 @@ class ContactTest < ActionDispatch::IntegrationTest
   test "phones are rendered" do
     setup_and_visit_content_item('contact')
     within_component_govspeak do |component_args|
-      content = component_args.fetch("content")
-      first_phone = @content_item["details"]["phone_numbers"].first["number"]
+      first_phone = @content_item["details"]["phone_numbers"].first
+      html = Nokogiri::HTML.parse(component_args.fetch("content"))
 
-      html = Nokogiri::HTML.parse(content)
       assert_not_nil html.at_css("h2#phone-title")
-      assert_not_nil html.at("p:contains(\"#{first_phone}\")")
+      assert_not_nil html.at("h3:contains(\"#{first_phone['title']}\")")
+      assert_not_nil html.at("p:contains(\"#{first_phone['number']}\")")
       assert_not_nil html.at("p:contains(\"24 hours a day, 7 days a week\")")
     end
   end
 
+  test "phone number heading is not rendered when only one number" do
+    setup_and_visit_content_item('contact_with_welsh')
+    within_component_govspeak do |component_args|
+      first_phone = @content_item["details"]["phone_numbers"].first
+      html = Nokogiri::HTML.parse(component_args.fetch("content"))
+
+      assert_equal 1, @content_item["details"]["phone_numbers"].size
+      refute html.at("h3:contains(\"#{first_phone['title']}\")")
+    end
+  end
+
   test "posts are rendered" do
     setup_and_visit_content_item('contact')
     within_component_govspeak do |component_args|

test/presenters/contact_presenter_test.rb

@@ -77,5 +77,27 @@ def format_name
     test 'email_body' do
       assert_equal schema_item['details']['more_info_email_address'], presented_item.email_body
     end
+
+    test 'handles more info when set to nil' do
+      example = schema_item
+      example['details']['more_info_phone_number'] = nil
+      example['details']['more_info_email_address'] = nil
+      example['details']['more_info_post_address'] = nil
+
+      assert_equal nil, present_example(example).phone_body
+      assert_equal nil, present_example(example).email_body
+      assert_equal nil, present_example(example).post_body
+    end
+
+    test 'handles more info when not set' do
+      example = schema_item
+      example['details'].delete('more_info_phone_number')
+      example['details'].delete('more_info_email_address')
+      example['details'].delete('more_info_post_address')
+
+      assert_equal nil, present_example(example).phone_body
+      assert_equal nil, present_example(example).email_body
+      assert_equal nil, present_example(example).post_body
+    end
   end
 end