Update devolved nations component to accept type property #2337

jon-kirwan · 2021-10-06T10:55:16Z

What

https://trello.com/c/4SUBn4mb/880-update-devolved-nations-component

Update the component to accept a type property e.g. "Guidance"
Change the default type of content to "Publication"

Why

Currently, all alternative content are labelled as guidance e.g.

Applies to England

Guidance for Wales

Guidance for Scotland

Guidance for Northern Ireland

However, not all alternative content is guidance. We should refer to "Publication" as the default but allow a type property to be passed to the component.

Visual Changes

Before	After

Anything else

Prerequisite for alphagov/government-frontend#2240

maxgds · 2021-10-06T13:21:40Z

app/views/govuk_publishing_components/components/_devolved_nations.html.erb

@@ -15,7 +17,7 @@
    <% if devolved_nations_helper.nations_with_urls.any? %>
      <%= content_tag :ul, class: "govuk-list govuk-!-margin-top-1 govuk-!-margin-bottom-0" do -%>
        <% devolved_nations_helper.nations_with_urls.each do |k, v| %>
-          <%= content_tag(:li, link_to("Guidance for #{t("components.devolved_nations.#{k}")}", v[:alternative_url], class: "govuk-link")) %>
+          <%= content_tag(:li, link_to("#{type} for #{t("components.devolved_nations.#{k}")}", v[:alternative_url], class: "govuk-link")) %>


Can we do anything to clean up the translations here? It seems we only ever have publication or guidance, is that right? Ideally we would be translating the whole phrase Guidance|Publication for |Scotland|Wales|Ireland as you can't guarantee word order in other languages, and also here we're not translating the word "for" at all.

I can see consultations also probably gets passed but doesn't seem to be rendered as they don't seem to have alternatives - but we could have consultations as a third string type, maybe. We could probably have placeholders for the language and so would need only the 3 phrases
Consultation for %{country}
Publication for %{country}
Guidance for %{country}

Can we do anything to clean up the translations here? It seems we only ever have publication or guidance, is that right?

I think so. There's lots of different schema names that could potentially be passed to the component but despite looking at quite a few examples I’ve only found either "Publication" or "Guidance".

I can see consultations also probably gets passed but doesn't seem to be rendered as they don't seem to have alternatives

Similar to above, I can’t seem to find any consultations - do you know of any examples I could take a look at?

We could probably have placeholders for the language and so would need only the 3 phrases Consultation for %{country} Publication for %{country} Guidance for %{country}

The only problem I can see with this approach is that I’m passing in a type that’s already been translated in government_frontend e.g.

https://github.com/alphagov/government-frontend/blob/main/config/locales/en.yml#L118-L120

Maybe I should do the translation in components instead?

All the consultations I found looked like this: https://www.gov.uk/government/consultations/open-access-restriction-at-castle-beeny-farm-2-how-to-comment - basically they're all saying "This applies to England" and offering no alternatives. But I'm by no means certain I'm catching everything.

I think it would make sense to do the translation in the component wholly, it is after all what does the presentation - taking data to make that decision feels cleaner. Do ask around for a second opinion though as I'm not confident that this is definitely the way we should go. We do need to handle the whole string, though, I think, and if the calling app is responsible for the strings you start to question the point of the component, which is why I'm leaning that way.

I'll investigate some more and see if anything comes up. I've been trawling through this list here: https://www.gov.uk/search/policy-papers-and-consultations?page=1 but so far nothing.

I think you're right too - i'd wondered about it myself beforehand but since we already had the translations in government_frontend decided to leave it there.

I'll get another opinion but does seem a better approach to move it all to the component.

There's lots of different schema names that could potentially be passed to the component but despite looking at quite a few examples I’ve only found either "Publication" or "Guidance".

I think it's because the overarching type is Publication for a whole raft of content types - I don't know that getting more granular is necessarily expected as publication is understood internally and relatively clear to the end user in context.

I think it would make sense to do the translation in the component wholly, it is after all what does the presentation - taking data to make that decision feels cleaner. Do ask around for a second opinion though...

Second opinion here: https://gds.slack.com/archives/CARGH33JS/p1633606644023200

I've now created a helper to handle the translation and to keep the view a bit tidier. The only issue I have is that it seems necessary to translate a key twice but this seems to be a common issue in Ruby / Rails https://stackoverflow.com/questions/14306469/use-another-i18n-key-in-an-i18n-interpolation.

Changes include:

Handle different types:

Publication (publication - also the default)

Guidance (guidance and detailed_guide)

Consultation (consultation if any exist)

Update tests to include different types that can be passed the the component including invalid or nil types

Update docs to include:

New use cases and

To replace references to "Guidance" as "Alternative content" (except where "Guidance" is the actual type)

jon-kirwan · 2021-10-12T11:10:37Z

I see Percy is reporting a few errors. As far as I can see the errors are just differences in -

The documentation title for some examples e.g. "Applies to one nation individual guidance available" now reads "Applies to one nation individual publication available"
The links to alternative content in the same examples e.g. "Guidance for Northern Ireland" now reads "Publication for Northern Ireland" (depending on data passed, or not passed, to the component)

maxgds

Nice work.

maxgds · 2021-10-12T13:54:54Z

lib/govuk_publishing_components/presenters/devolved_nations_helper.rb

+      def alternative_content_text(name)
+        nation = I18n.t("components.devolved_nations.#{name}")
+
+        if I18n.exists?("components.devolved_nations.type.#{@type}")


Nice safety check!

maxgds · 2021-10-12T13:55:41Z

spec/components/devolved_nations_spec.rb

    assert_select ".gem-c-devolved-nations > ul > li:nth-child(1) > [href='/guidance-northern-ireland']", text: "Guidance for Northern Ireland"
  end

+  it "renders a devolved nations component, which applies to one nation, with individual pubilcation available, when invalid type provided, correctly" do


jon-kirwan force-pushed the update-devolved-nations-component branch from 539683c to e602f16 Compare October 6, 2021 11:21

jon-kirwan mentioned this pull request Oct 6, 2021

Update devolved nations component uses to pass type property alphagov/government-frontend#2240

Merged

jon-kirwan marked this pull request as ready for review October 6, 2021 12:24

jon-kirwan requested a review from maxgds October 6, 2021 12:25

maxgds reviewed Oct 6, 2021

View reviewed changes

jon-kirwan force-pushed the update-devolved-nations-component branch from e602f16 to cadaa94 Compare October 11, 2021 10:23

jon-kirwan requested a review from maxgds October 11, 2021 11:09

jon-kirwan force-pushed the update-devolved-nations-component branch from cadaa94 to 9b54a82 Compare October 11, 2021 11:26

maxgds approved these changes Oct 12, 2021

View reviewed changes

Update devolved nations component to accept type property

d4d7d1c

jon-kirwan force-pushed the update-devolved-nations-component branch from 9b54a82 to d4d7d1c Compare October 12, 2021 14:47

maxgds merged commit 4273788 into master Oct 12, 2021

maxgds deleted the update-devolved-nations-component branch October 12, 2021 14:53

jon-kirwan mentioned this pull request Oct 13, 2021

Release 27.7.0 #2352

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update devolved nations component to accept type property #2337

Update devolved nations component to accept type property #2337

jon-kirwan commented Oct 6, 2021 •

edited

Loading

maxgds Oct 6, 2021

maxgds Oct 6, 2021

jon-kirwan Oct 6, 2021

jon-kirwan Oct 6, 2021

jon-kirwan Oct 6, 2021 •

edited

Loading

maxgds Oct 6, 2021

jon-kirwan Oct 6, 2021

maxgds Oct 6, 2021

jon-kirwan Oct 7, 2021

jon-kirwan Oct 11, 2021 •

edited

Loading

jon-kirwan commented Oct 12, 2021

maxgds left a comment

maxgds Oct 12, 2021

maxgds Oct 12, 2021

Update devolved nations component to accept type property #2337

Update devolved nations component to accept type property #2337

Conversation

jon-kirwan commented Oct 6, 2021 • edited Loading

What

Why

Visual Changes

Anything else

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jon-kirwan Oct 6, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jon-kirwan Oct 11, 2021 • edited Loading

Choose a reason for hiding this comment

jon-kirwan commented Oct 12, 2021

maxgds left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jon-kirwan commented Oct 6, 2021 •

edited

Loading

jon-kirwan Oct 6, 2021 •

edited

Loading

jon-kirwan Oct 11, 2021 •

edited

Loading