From 4a243956ce4b09c4c9432bfff4392e5fbc5d1b2a Mon Sep 17 00:00:00 2001 From: Peter Yates Date: Tue, 5 Oct 2021 12:19:33 +0100 Subject: [PATCH 1/5] Allow crown's fallback image to have custom path The previously-hardcoded value makes a lot of assumptions on how the application is bundling its assets, this should offer a little more flexibility. --- .../govuk_component/header_component.html.erb | 3 +- .../govuk_component/header_component.rb | 13 ++++++++ .../govuk_component/header_component_spec.rb | 33 +++++++++++++++---- 3 files changed, 42 insertions(+), 7 deletions(-) diff --git a/app/components/govuk_component/header_component.html.erb b/app/components/govuk_component/header_component.html.erb index fa9b83e4..19b947be 100644 --- a/app/components/govuk_component/header_component.html.erb +++ b/app/components/govuk_component/header_component.html.erb @@ -9,7 +9,8 @@ <% if crown %> <% end %> <%= tag.span(logotype, class: "govuk-header__logotype-text") %> diff --git a/app/components/govuk_component/header_component.rb b/app/components/govuk_component/header_component.rb index 3a853560..acd1fd17 100644 --- a/app/components/govuk_component/header_component.rb +++ b/app/components/govuk_component/header_component.rb @@ -5,6 +5,7 @@ class GovukComponent::HeaderComponent < GovukComponent::Base attr_reader :logotype, :crown, + :crown_fallback_image, :homepage_url, :service_name, :service_url, @@ -17,6 +18,7 @@ def initialize(classes: [], html_attributes: {}, logotype: 'GOV.UK', crown: true, + crown_fallback_image: '/assets/images/govuk-logotype-crown.png', homepage_url: '/', menu_button_label: 'Show or hide navigation menu', navigation_classes: [], @@ -29,6 +31,7 @@ def initialize(classes: [], @logotype = logotype @crown = crown + @crown_fallback_image = crown_fallback_image @homepage_url = homepage_url @service_name = service_name @service_url = service_url @@ -52,6 +55,16 @@ def container_classes combine_classes(%w(govuk-header__container govuk-width-container), custom_container_classes) end + def crown_fallback_image_attributes + { + src: crown_fallback_image, + class: "govuk-header__logotype-crown-fallback-image", + width: "36", + height: "32", + "xlink:href" => "", + } + end + class NavigationItem < GovukComponent::Base attr_reader :text, :href, :options, :active diff --git a/spec/components/govuk_component/header_component_spec.rb b/spec/components/govuk_component/header_component_spec.rb index 0bd588c7..c451e954 100644 --- a/spec/components/govuk_component/header_component_spec.rb +++ b/spec/components/govuk_component/header_component_spec.rb @@ -41,15 +41,36 @@ end end - context 'when the crown is disabled' do - let(:kwargs) { { crown: false } } + describe 'the crown' do + context 'when the crown is not disabled' do + specify 'the crown SVG is rendered along with a fallback image' do + expect(rendered_component).to have_tag('.govuk-header__logotype') do + with_tag('svg', with: { class: 'govuk-header__logotype-crown' }) do + with_tag('image', with: { class: 'govuk-header__logotype-crown-fallback-image', src: '/assets/images/govuk-logotype-crown.png' }) + end + end + end - specify "doesn't render the crown" do - expect(rendered_component).not_to have_tag("svg") + context 'when the crown path is overridden' do + let(:custom_path) { '/an-alternative-crown-file.jpg' } + let(:kwargs) { { crown_fallback_image: custom_path } } + + specify 'renders the fallback image with the custom path' do + expect(rendered_component).to have_tag('image', with: { src: custom_path, class: 'govuk-header__logotype-crown-fallback-image' }) + end + end end - specify "renders the default logotype" do - expect(rendered_component).to have_tag("span", text: /GOV.UK/) + context 'when the crown is disabled' do + let(:kwargs) { { crown: false } } + + specify "doesn't render the crown" do + expect(rendered_component).not_to have_tag("svg") + end + + specify "renders the default logotype" do + expect(rendered_component).to have_tag("span", text: /GOV.UK/) + end end end From e7a46f6ad59da519b7b1a5e41a2c1967309e4e5a Mon Sep 17 00:00:00 2001 From: Peter Yates Date: Tue, 5 Oct 2021 14:11:48 +0100 Subject: [PATCH 2/5] Remove the default crown fallback image path Now there is no default, no fallback will be rendered unless an image path is provided --- .../govuk_component/header_component.html.erb | 4 +++- app/components/govuk_component/header_component.rb | 2 +- .../govuk_component/header_component_spec.rb | 14 ++++++++------ 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/app/components/govuk_component/header_component.html.erb b/app/components/govuk_component/header_component.html.erb index 19b947be..8e230ff5 100644 --- a/app/components/govuk_component/header_component.html.erb +++ b/app/components/govuk_component/header_component.html.erb @@ -10,7 +10,9 @@ <% end %> <%= tag.span(logotype, class: "govuk-header__logotype-text") %> diff --git a/app/components/govuk_component/header_component.rb b/app/components/govuk_component/header_component.rb index acd1fd17..9f62b28b 100644 --- a/app/components/govuk_component/header_component.rb +++ b/app/components/govuk_component/header_component.rb @@ -18,7 +18,7 @@ def initialize(classes: [], html_attributes: {}, logotype: 'GOV.UK', crown: true, - crown_fallback_image: '/assets/images/govuk-logotype-crown.png', + crown_fallback_image: nil, homepage_url: '/', menu_button_label: 'Show or hide navigation menu', navigation_classes: [], diff --git a/spec/components/govuk_component/header_component_spec.rb b/spec/components/govuk_component/header_component_spec.rb index c451e954..b0306dfb 100644 --- a/spec/components/govuk_component/header_component_spec.rb +++ b/spec/components/govuk_component/header_component_spec.rb @@ -43,20 +43,22 @@ describe 'the crown' do context 'when the crown is not disabled' do - specify 'the crown SVG is rendered along with a fallback image' do + specify 'the crown SVG is rendered along with no fallback image' do expect(rendered_component).to have_tag('.govuk-header__logotype') do - with_tag('svg', with: { class: 'govuk-header__logotype-crown' }) do - with_tag('image', with: { class: 'govuk-header__logotype-crown-fallback-image', src: '/assets/images/govuk-logotype-crown.png' }) - end + with_tag('svg', with: { class: 'govuk-header__logotype-crown' }) end end - context 'when the crown path is overridden' do + context 'when a fallback image path is provided' do let(:custom_path) { '/an-alternative-crown-file.jpg' } let(:kwargs) { { crown_fallback_image: custom_path } } specify 'renders the fallback image with the custom path' do - expect(rendered_component).to have_tag('image', with: { src: custom_path, class: 'govuk-header__logotype-crown-fallback-image' }) + expect(rendered_component).to have_tag('.govuk-header__logotype') do + with_tag('svg', with: { class: 'govuk-header__logotype-crown' }) do + with_tag('image', with: { src: custom_path, class: 'govuk-header__logotype-crown-fallback-image' }) + end + end end end end From 282833257e2367e199b8a68f68008b918efedb01 Mon Sep 17 00:00:00 2001 From: Peter Yates Date: Tue, 5 Oct 2021 15:28:54 +0100 Subject: [PATCH 3/5] Add IE8 conditionals in line with latest version --- .../govuk_component/header_component.html.erb | 13 +++++++++---- app/components/govuk_component/header_component.rb | 1 - .../govuk_component/header_component_spec.rb | 8 ++++---- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/app/components/govuk_component/header_component.html.erb b/app/components/govuk_component/header_component.html.erb index 8e230ff5..199a69ec 100644 --- a/app/components/govuk_component/header_component.html.erb +++ b/app/components/govuk_component/header_component.html.erb @@ -7,14 +7,19 @@ <%= custom_logo %> <% else %> <% if crown %> + + + <% end %> + + <% if crown_fallback_image.present? %> + <% end %> + <%= tag.span(logotype, class: "govuk-header__logotype-text") %> <% end %> diff --git a/app/components/govuk_component/header_component.rb b/app/components/govuk_component/header_component.rb index 9f62b28b..647102f7 100644 --- a/app/components/govuk_component/header_component.rb +++ b/app/components/govuk_component/header_component.rb @@ -57,7 +57,6 @@ def container_classes def crown_fallback_image_attributes { - src: crown_fallback_image, class: "govuk-header__logotype-crown-fallback-image", width: "36", height: "32", diff --git a/spec/components/govuk_component/header_component_spec.rb b/spec/components/govuk_component/header_component_spec.rb index b0306dfb..6d59cef2 100644 --- a/spec/components/govuk_component/header_component_spec.rb +++ b/spec/components/govuk_component/header_component_spec.rb @@ -54,10 +54,10 @@ let(:kwargs) { { crown_fallback_image: custom_path } } specify 'renders the fallback image with the custom path' do - expect(rendered_component).to have_tag('.govuk-header__logotype') do - with_tag('svg', with: { class: 'govuk-header__logotype-crown' }) do - with_tag('image', with: { src: custom_path, class: 'govuk-header__logotype-crown-fallback-image' }) - end + expect(rendered_component).to have_tag('.govuk-header__logotype') do |logotype| + # NOTE: it's rendered inside a IE8 conditional comment so we can't + # assert its presence normally, just ensure the path's included + expect(logotype.current_scope.inner_html).to include(custom_path) end end end From 1347978623fe114f46f43bd5fc37c5c6e75cb50b Mon Sep 17 00:00:00 2001 From: Peter Yates Date: Wed, 6 Oct 2021 10:08:19 +0100 Subject: [PATCH 4/5] Drop unnecessary xlink:href attribute Co-authored-by: Frankie Roberto --- app/components/govuk_component/header_component.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/components/govuk_component/header_component.rb b/app/components/govuk_component/header_component.rb index 647102f7..39b36330 100644 --- a/app/components/govuk_component/header_component.rb +++ b/app/components/govuk_component/header_component.rb @@ -60,7 +60,6 @@ def crown_fallback_image_attributes class: "govuk-header__logotype-crown-fallback-image", width: "36", height: "32", - "xlink:href" => "", } end From 5794081f4f3d4a47435b697cccb3249a77bb1f0d Mon Sep 17 00:00:00 2001 From: Peter Yates Date: Wed, 6 Oct 2021 10:13:05 +0100 Subject: [PATCH 5/5] Add path suffix to crown_fallback_image This makes it clear we're expecting an image path rather than name or file object --- app/components/govuk_component/header_component.html.erb | 4 ++-- app/components/govuk_component/header_component.rb | 6 +++--- spec/components/govuk_component/header_component_spec.rb | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/components/govuk_component/header_component.html.erb b/app/components/govuk_component/header_component.html.erb index 199a69ec..7189c667 100644 --- a/app/components/govuk_component/header_component.html.erb +++ b/app/components/govuk_component/header_component.html.erb @@ -14,9 +14,9 @@ <% end %> - <% if crown_fallback_image.present? %> + <% if crown_fallback_image_path.present? %> <% end %> diff --git a/app/components/govuk_component/header_component.rb b/app/components/govuk_component/header_component.rb index 39b36330..ebbdfe5d 100644 --- a/app/components/govuk_component/header_component.rb +++ b/app/components/govuk_component/header_component.rb @@ -5,7 +5,7 @@ class GovukComponent::HeaderComponent < GovukComponent::Base attr_reader :logotype, :crown, - :crown_fallback_image, + :crown_fallback_image_path, :homepage_url, :service_name, :service_url, @@ -18,7 +18,7 @@ def initialize(classes: [], html_attributes: {}, logotype: 'GOV.UK', crown: true, - crown_fallback_image: nil, + crown_fallback_image_path: nil, homepage_url: '/', menu_button_label: 'Show or hide navigation menu', navigation_classes: [], @@ -31,7 +31,7 @@ def initialize(classes: [], @logotype = logotype @crown = crown - @crown_fallback_image = crown_fallback_image + @crown_fallback_image_path = crown_fallback_image_path @homepage_url = homepage_url @service_name = service_name @service_url = service_url diff --git a/spec/components/govuk_component/header_component_spec.rb b/spec/components/govuk_component/header_component_spec.rb index 6d59cef2..6b6a9d53 100644 --- a/spec/components/govuk_component/header_component_spec.rb +++ b/spec/components/govuk_component/header_component_spec.rb @@ -51,7 +51,7 @@ context 'when a fallback image path is provided' do let(:custom_path) { '/an-alternative-crown-file.jpg' } - let(:kwargs) { { crown_fallback_image: custom_path } } + let(:kwargs) { { crown_fallback_image_path: custom_path } } specify 'renders the fallback image with the custom path' do expect(rendered_component).to have_tag('.govuk-header__logotype') do |logotype|