Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor Brexit callout #1958

Merged
merged 2 commits into from
Dec 30, 2020
Merged

Refactor Brexit callout #1958

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1106817
Avoid inaccurate "no deal" term where possible
Dec 29, 2020
a494556
Clarify "header notice" is just "brexit notice"
Dec 29, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 16 additions & 16 deletions ...presenters/content_item/no_deal_notice.rb → app/presenters/content_item/brexit_notice.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,40 +1,40 @@
module ContentItem
module NoDealNotice
def has_no_deal_notice?
module BrexitNotice
def has_brexit_notice?
content_item.dig("details").key?("brexit_no_deal_notice")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mmm yeah, that's a good reminder for future eh?
overly descriptive names for things that might get hijacked in future!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it's an interesting one: going in it may have appeared like a more specific naming was better (the old "avoid premature refactoring" mantra). I expect the product thinking at the time was all around "a brexit notice for a no deal scenario". As developers, the onus was really on us to ask: would we ever have more than one brexit notice? Even if we did, it would still be possible to distinguish them i.e. the future risk of confusion was always low.

Oh well. Easy to say all this with hindsight!

end

def no_deal_notice_component
if has_no_deal_notice?
def brexit_notice_component
if has_brexit_notice?
{
title: no_deal_notice_title,
description: no_deal_notice_description,
link_intro: no_deal_notice_link_intro,
links: no_deal_links,
featured_link: no_deal_landing_page_cta,
title: brexit_notice_title,
description: brexit_notice_description,
link_intro: brexit_notice_link_intro,
links: brexit_links,
featured_link: brexit_landing_page_cta,
}
end
end

private

def no_deal_notice_links
def brexit_notice_links
content_item.dig("details", "brexit_no_deal_notice")
end

def no_deal_notice_title
def brexit_notice_title
"Brexit transition: new rules for 2021"
end

def no_deal_notice_description
def brexit_notice_description
"The UK has agreed a deal with the EU. This page tells you the new rules from 1 January 2021."
end

def no_deal_notice_link_intro
def brexit_notice_link_intro
"For current information, read: "
end

def no_deal_landing_page_cta
def brexit_landing_page_cta
data_attributes = {
"module": "track-click",
"track-category": "no_deal_notice",
Expand All @@ -50,8 +50,8 @@ def no_deal_landing_page_cta
("Use the Brexit checker to " + featured_link + " and sign up for email updates.").html_safe
end

def no_deal_links
no_deal_notice_links.map do |link|
def brexit_links
brexit_notice_links.map do |link|
{
title: link["title"],
href: link["href"],
Expand Down
2 changes: 1 addition & 1 deletion app/presenters/content_item_presenter.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
class ContentItemPresenter
include ContentItem::Withdrawable
include ContentItem::NoDealNotice
include ContentItem::BrexitNotice

attr_reader :content_item,
:requested_path,
Expand Down
2 changes: 1 addition & 1 deletion app/views/content_items/case_study.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

<%= render 'shared/publisher_metadata_with_logo' %>
<%= render 'govuk_publishing_components/components/notice', @content_item.withdrawal_notice_component %>
<%= render 'components/header-notice', @content_item.no_deal_notice_component unless @content_item.withdrawal_notice_component %>
<%= render 'components/header-notice', @content_item.brexit_notice_component unless @content_item.withdrawal_notice_component %>

<div class="govuk-grid-row">
<div class="govuk-grid-column-two-thirds content-bottom-margin">
Expand Down
2 changes: 1 addition & 1 deletion app/views/content_items/detailed_guide.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
<%= render 'shared/publisher_metadata_with_logo' %>
<%= render 'shared/history_notice', content_item: @content_item %>
<%= render 'govuk_publishing_components/components/notice', @content_item.withdrawal_notice_component %>
<%= render 'components/header-notice', @content_item.no_deal_notice_component unless @content_item.withdrawal_notice_component %>
<%= render 'components/header-notice', @content_item.brexit_notice_component unless @content_item.withdrawal_notice_component %>

<div class="govuk-grid-row">
<div class="govuk-grid-column-two-thirds">
Expand Down
2 changes: 1 addition & 1 deletion app/views/content_items/document_collection.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
<div class="govuk-grid-column-two-thirds">
<%= render 'govuk_publishing_components/components/lead_paragraph', text: @content_item.description %>
<%= render 'govuk_publishing_components/components/notice', @content_item.withdrawal_notice_component %>
<%= render 'components/header-notice', @content_item.no_deal_notice_component unless @content_item.withdrawal_notice_component %>
<%= render 'components/header-notice', @content_item.brexit_notice_component unless @content_item.withdrawal_notice_component %>
<%= render 'shared/history_notice', content_item: @content_item %>
</div>
</div>
Expand Down
2 changes: 1 addition & 1 deletion app/views/content_items/html_publication.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
<% end %>

<%= render 'govuk_publishing_components/components/notice', @content_item.withdrawal_notice_component %>
<%= render 'components/header-notice', @content_item.no_deal_notice_component unless @content_item.withdrawal_notice_component %>
<%= render 'components/header-notice', @content_item.brexit_notice_component unless @content_item.withdrawal_notice_component %>

<div
class="govuk-grid-row sidebar-with-body"
Expand Down
2 changes: 1 addition & 1 deletion app/views/content_items/publication.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
<%= render 'shared/history_notice', content_item: @content_item %>

<%= render 'govuk_publishing_components/components/notice', @content_item.withdrawal_notice_component %>
<%= render 'components/header-notice', @content_item.no_deal_notice_component unless @content_item.withdrawal_notice_component %>
<%= render 'components/header-notice', @content_item.brexit_notice_component unless @content_item.withdrawal_notice_component %>

<div class="govuk-grid-row">
<div class="govuk-grid-column-two-thirds responsive-bottom-margin">
Expand Down