From ead12171a2e058fc6a49fa31a3f3f8e364629002 Mon Sep 17 00:00:00 2001 From: Max Lehmann Date: Thu, 5 Sep 2019 13:50:19 +0100 Subject: [PATCH] Adjust focus states This adds the govuk-link class to multiple links throughout the codebase in order to inherit the correct styles, and updates related tests. Where the class is overridden by govuk-template the styles have been updated to use a temporary mixin to enforce the correct colour state and the class is locally overridden where white links are needed. --- .../stylesheets/components/_back-to-top.scss | 3 --- .../components/_important-metadata.scss | 12 ++++++++---- .../stylesheets/components/_published-dates.scss | 11 ++++++----- .../components/_publisher-metadata.scss | 2 +- app/assets/stylesheets/helpers/_parts.scss | 2 +- .../_govuk-template-link-focus-override.scss | 9 +++++++++ app/presenters/content_item/linkable.rb | 6 +++--- app/presenters/content_item/parts.rb | 2 +- app/presenters/fatality_notice_presenter.rb | 2 +- app/presenters/specialist_document_presenter.rb | 2 +- app/views/components/_back-to-top.html.erb | 2 +- app/views/components/_print-link.html.erb | 2 +- app/views/components/_published-dates.html.erb | 4 ++-- test/presenters/case_study_presenter_test.rb | 12 ++++++------ test/presenters/content_item/linkable_test.rb | 4 ++-- test/presenters/content_item/parts_test.rb | 8 ++++---- test/presenters/publication_presenter_test.rb | 6 +++--- .../specialist_document_presenter_test.rb | 3 ++- test/presenters/speech_presenter_test.rb | 6 +++--- .../statistics_announcement_presenter_test.rb | 2 +- test/presenters/travel_advice_presenter_test.rb | 16 ++++++++-------- 21 files changed, 64 insertions(+), 52 deletions(-) create mode 100644 app/assets/stylesheets/mixins/_govuk-template-link-focus-override.scss diff --git a/app/assets/stylesheets/components/_back-to-top.scss b/app/assets/stylesheets/components/_back-to-top.scss index 6d786c4f0..baf181e0b 100644 --- a/app/assets/stylesheets/components/_back-to-top.scss +++ b/app/assets/stylesheets/components/_back-to-top.scss @@ -4,9 +4,6 @@ margin-left: govuk-spacing(3); margin-right: govuk-spacing(3); - &:focus { - @include govuk-focused-text; - } } .app-c-back-to-top__icon { diff --git a/app/assets/stylesheets/components/_important-metadata.scss b/app/assets/stylesheets/components/_important-metadata.scss index 4175d88c6..d90184bf1 100644 --- a/app/assets/stylesheets/components/_important-metadata.scss +++ b/app/assets/stylesheets/components/_important-metadata.scss @@ -1,7 +1,6 @@ .app-c-important-metadata { - @include white-links; background: $govuk-blue; - color: $white; + color: govuk-colour("white"); padding: govuk-spacing(3); @include media(tablet) { @@ -30,7 +29,12 @@ .app-c-important-metadata__definition { font-weight: bold; - a:focus { - @include govuk-focused-text; + .app-link { + color: govuk-colour("white"); + + &:focus { + @include govuk-template-link-focus-override; + } } + } diff --git a/app/assets/stylesheets/components/_published-dates.scss b/app/assets/stylesheets/components/_published-dates.scss index 922b6cebe..10b124218 100644 --- a/app/assets/stylesheets/components/_published-dates.scss +++ b/app/assets/stylesheets/components/_published-dates.scss @@ -6,18 +6,19 @@ .app-c-published-dates__toggle { display: none; - &:focus { - @include govuk-focused-text; - } - .js-enabled & { display: inline-block; } + + &:focus { + @include govuk-template-link-focus-override; + } + } .app-c-published-dates__history-link { &:focus { - @include govuk-focused-text; + @include govuk-template-link-focus-override; } } diff --git a/app/assets/stylesheets/components/_publisher-metadata.scss b/app/assets/stylesheets/components/_publisher-metadata.scss index 2ff229647..c82bcec9c 100644 --- a/app/assets/stylesheets/components/_publisher-metadata.scss +++ b/app/assets/stylesheets/components/_publisher-metadata.scss @@ -8,7 +8,7 @@ font-weight: bold; &:focus { - @include govuk-focused-text; + @include govuk-template-link-focus-override; } } diff --git a/app/assets/stylesheets/helpers/_parts.scss b/app/assets/stylesheets/helpers/_parts.scss index 56a2eec8c..97810d96e 100644 --- a/app/assets/stylesheets/helpers/_parts.scss +++ b/app/assets/stylesheets/helpers/_parts.scss @@ -29,7 +29,7 @@ display: block; &:focus { - @include govuk-focused-text; + @include govuk-template-link-focus-override; } } } diff --git a/app/assets/stylesheets/mixins/_govuk-template-link-focus-override.scss b/app/assets/stylesheets/mixins/_govuk-template-link-focus-override.scss new file mode 100644 index 000000000..a32091473 --- /dev/null +++ b/app/assets/stylesheets/mixins/_govuk-template-link-focus-override.scss @@ -0,0 +1,9 @@ +// TODO: Remove when appropriate +// govuk_template overrides the styles set by +// govuk-frontend 3.0.0. This mixin is intended as a temporary fix +// to ensure focus styles are as expected in apps using govuk_template + +@mixin govuk-template-link-focus-override { + @include govuk-focused-text; + color: govuk-colour("black") !important; +} diff --git a/app/presenters/content_item/linkable.rb b/app/presenters/content_item/linkable.rb index 945d1e829..45a5f553c 100644 --- a/app/presenters/content_item/linkable.rb +++ b/app/presenters/content_item/linkable.rb @@ -37,7 +37,7 @@ def links_group(types) def organisations_ordered_by_importance organisations_with_emphasised_first.map do |link| - link_to(link["title"], link["base_path"]) + link_to(link["title"], link["base_path"], class: "govuk-link") end end @@ -54,12 +54,12 @@ def emphasised_organisations def link_for_type(type, link) return link_for_world_location(link) if type == "world_locations" - link_to(link["title"], link["base_path"]) + link_to(link["title"], link["base_path"], class: "govuk-link") end def link_for_world_location(link) base_path = WorldLocationBasePath.for(link) - link_to(link["title"], base_path) + link_to(link["title"], base_path, class: "govuk-link") end end end diff --git a/app/presenters/content_item/parts.rb b/app/presenters/content_item/parts.rb index 94fe0bea8..02dfcf40f 100644 --- a/app/presenters/content_item/parts.rb +++ b/app/presenters/content_item/parts.rb @@ -86,7 +86,7 @@ def current_part def part_links parts.map.with_index(1) do |part, position| if part['slug'] != current_part['slug'] - link_to part['title'], part['full_path'], + link_to part['title'], part['full_path'], class: "govuk-link", data: { track_category: 'contentsClicked', track_action: "content_item #{position}", diff --git a/app/presenters/fatality_notice_presenter.rb b/app/presenters/fatality_notice_presenter.rb index 3b2c99865..eb0418269 100644 --- a/app/presenters/fatality_notice_presenter.rb +++ b/app/presenters/fatality_notice_presenter.rb @@ -18,7 +18,7 @@ def image def important_metadata super.tap do |m| if field_of_operation - m.merge!('Field of operation' => link_to(field_of_operation.title, field_of_operation.path)) + m.merge!('Field of operation' => link_to(field_of_operation.title, field_of_operation.path, class: "govuk-link app-link")) end end end diff --git a/app/presenters/specialist_document_presenter.rb b/app/presenters/specialist_document_presenter.rb index 09d43425c..f2dfcba8b 100644 --- a/app/presenters/specialist_document_presenter.rb +++ b/app/presenters/specialist_document_presenter.rb @@ -168,7 +168,7 @@ def facet_block(facet, allowed_value) def facet_link(label, value, key) finder_base_path = finder['base_path'] - link_to(label, "#{finder_base_path}?#{key}%5B%5D=#{value}") + link_to(label, "#{finder_base_path}?#{key}%5B%5D=#{value}", class: "govuk-link app-link") end def first_published_at_facet_key diff --git a/app/views/components/_back-to-top.html.erb b/app/views/components/_back-to-top.html.erb index 34b70141d..3f5c70898 100644 --- a/app/views/components/_back-to-top.html.erb +++ b/app/views/components/_back-to-top.html.erb @@ -1,7 +1,7 @@ <% text ||= t('content_item.contents', default: "Contents") %> - diff --git a/app/views/components/_print-link.html.erb b/app/views/components/_print-link.html.erb index 4afa3dbc3..ed21070d0 100644 --- a/app/views/components/_print-link.html.erb +++ b/app/views/components/_print-link.html.erb @@ -5,7 +5,7 @@
<%= link_text %> diff --git a/app/views/components/_published-dates.html.erb b/app/views/components/_published-dates.html.erb index ab7f85fcf..e1aee3b90 100644 --- a/app/views/components/_published-dates.html.erb +++ b/app/views/components/_published-dates.html.erb @@ -14,10 +14,10 @@ <% if last_updated %> <% if published %>
<% end %>Last updated <%= last_updated %> <% if link_to_history && history.empty? %> - — <%= t('components.published_dates.see_all_updates', locale: :en) %> + — <%= t('components.published_dates.see_all_updates', locale: :en) %> <% elsif history.any? %> + <%= t('components.published_dates.show_all_updates', locale: :en) %> diff --git a/test/presenters/case_study_presenter_test.rb b/test/presenters/case_study_presenter_test.rb index ab1db0b14..f57d64fda 100644 --- a/test/presenters/case_study_presenter_test.rb +++ b/test/presenters/case_study_presenter_test.rb @@ -42,9 +42,9 @@ def schema_name } expected_from_links = [ - link_to('Lead org', '/orgs/lead'), - link_to('Supporting org', '/orgs/supporting'), - link_to('DFID Pakistan', '/government/world/organisations/dfid-pakistan'), + link_to('Lead org', '/orgs/lead', class: "govuk-link"), + link_to('Supporting org', '/orgs/supporting', class: "govuk-link"), + link_to('DFID Pakistan', '/government/world/organisations/dfid-pakistan', class: "govuk-link"), ] assert_equal expected_from_links, presented_item(schema_name, with_organisations).from @@ -60,9 +60,9 @@ def schema_name ] expected_part_of_links = [ - link_to('Work Programme real life stories', '/government/collections/work-programme-real-life-stories'), - link_to('Cheese', '/policy/cheese'), - link_to('Pakistan', '/world/pakistan/news'), + link_to('Work Programme real life stories', '/government/collections/work-programme-real-life-stories', class: "govuk-link"), + link_to('Cheese', '/policy/cheese', class: "govuk-link"), + link_to('Pakistan', '/world/pakistan/news', class: "govuk-link"), ] assert_equal expected_part_of_links, presented_item(schema_name, with_extras).part_of end diff --git a/test/presenters/content_item/linkable_test.rb b/test/presenters/content_item/linkable_test.rb index 156166ff1..4eba604d6 100644 --- a/test/presenters/content_item/linkable_test.rb +++ b/test/presenters/content_item/linkable_test.rb @@ -24,7 +24,7 @@ def initialize ] expected_from_links = [ - %{Winston Churchill} + %{Winston Churchill} ] assert_equal expected_from_links, item.from end @@ -55,7 +55,7 @@ def initialize ] expected_from_links = [ - %{Germany} + %{Germany} ] assert_equal expected_from_links, item.part_of end diff --git a/test/presenters/content_item/parts_test.rb b/test/presenters/content_item/parts_test.rb index f75b48135..487cff474 100644 --- a/test/presenters/content_item/parts_test.rb +++ b/test/presenters/content_item/parts_test.rb @@ -154,7 +154,7 @@ def requested_content_item_path assert_equal @parts.parts_navigation, [[ "first-title", - "second-title" @@ -212,17 +212,17 @@ def content_item assert_equal @parts.parts_navigation, [ [ - "first-title", "second-title" ], [ - "third-title", - "fourth-title" diff --git a/test/presenters/publication_presenter_test.rb b/test/presenters/publication_presenter_test.rb index df24aee5e..be4715b29 100644 --- a/test/presenters/publication_presenter_test.rb +++ b/test/presenters/publication_presenter_test.rb @@ -37,17 +37,17 @@ def schema_name test '#from presents people' do minister = schema_item["links"]["people"][0] - assert presented_item.from.include?("#{minister['title']}") + assert presented_item.from.include?("#{minister['title']}") end test '#part_of presents topical events' do event = schema_item["links"]["topical_events"][0] - assert presented_item.part_of.include?("#{event['title']}") + assert presented_item.part_of.include?("#{event['title']}") end test '#part_of presents related statistical data sets' do data_set = schema_item("statistics_publication")["links"]["related_statistical_data_sets"][0] - assert presented_item("statistics_publication").part_of.include?("#{data_set['title']}") + assert presented_item("statistics_publication").part_of.include?("#{data_set['title']}") end test '#national_statistics? matches national statistics documents' do diff --git a/test/presenters/specialist_document_presenter_test.rb b/test/presenters/specialist_document_presenter_test.rb index fd04755cf..5cbc28e27 100644 --- a/test/presenters/specialist_document_presenter_test.rb +++ b/test/presenters/specialist_document_presenter_test.rb @@ -131,6 +131,7 @@ class PresentedSpecialistDocumentWithFinderFacets < SpecialistDocumentTestCase def example_with_finder_facets(facets = [], values = {}) example = schema_item('aaib-reports') example_finder = { + "class" => "govuk-link", "base_path" => "/finder-base-path", "title" => "Finder title", "details" => { @@ -267,7 +268,7 @@ def example_facet(overrides = {}) example = example_with_finder_facets([example_facet(overrides)], values) presented = present_example(example) - expected_link = "Something" + expected_link = "Something" assert_equal expected_link, presented.important_metadata["Facet name"] end diff --git a/test/presenters/speech_presenter_test.rb b/test/presenters/speech_presenter_test.rb index 2a36b3f83..d94ef1604 100644 --- a/test/presenters/speech_presenter_test.rb +++ b/test/presenters/speech_presenter_test.rb @@ -39,7 +39,7 @@ class PresentedSpeech < SpeechTestCase end test 'from includes speaker' do - assert presented_item.from.include?("The Rt Hon Andrea Leadsom MP") + assert presented_item.from.include?("The Rt Hon Andrea Leadsom MP") end end @@ -88,8 +88,8 @@ def example_schema_name test 'includes speaker without profile in from_with_speaker' do assert_equal [ - "Prime Minister's Office, 10 Downing Street", - "Cabinet Office", + "Prime Minister's Office, 10 Downing Street", + "Cabinet Office", "Her Majesty the Queen" ], presented_item(example_schema_name).from end diff --git a/test/presenters/statistics_announcement_presenter_test.rb b/test/presenters/statistics_announcement_presenter_test.rb index c2b542bc0..9a66496c3 100644 --- a/test/presenters/statistics_announcement_presenter_test.rb +++ b/test/presenters/statistics_announcement_presenter_test.rb @@ -9,7 +9,7 @@ def schema_name test 'presents from as links to organisations' do links = [ - link_to('NHS England', '/government/organisations/nhs-commissioning-board') + link_to('NHS England', '/government/organisations/nhs-commissioning-board', class: "govuk-link") ] assert_equal links, statistics_announcement.from end diff --git a/test/presenters/travel_advice_presenter_test.rb b/test/presenters/travel_advice_presenter_test.rb index 4b940076e..ea69c51ed 100644 --- a/test/presenters/travel_advice_presenter_test.rb +++ b/test/presenters/travel_advice_presenter_test.rb @@ -93,16 +93,16 @@ def schema_name another_part_nav_item = navigation_items[0][2] assert_equal summary_nav_item, - "Summary" + "Summary" assert_equal current_part_nav_item, current_part['title'] assert_equal another_part_nav_item, - "Terrorism" + "Terrorism" end test "navigation items link to all parts" do