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.

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

Add lang attribute to withdrawal notice #1858

Merged
merged 1 commit into from
Sep 15, 2020
Merged

Conversation

maxgds
Copy link
Contributor

@maxgds maxgds commented Sep 15, 2020

https://trello.com/c/k5JCHIoZ/382-add-language-attribute-to-withdrawn-banner

Withdrawal notices are always presented in English but sometimes
appear on a translated page. This passes en as a language value to
the notice component when that happens.

Example page: https://www.gov.uk/government/news/changes-to-uk-visa-application-process-in-germany.de

There are no visual changes. The effects of this won't be seen until the notice component supports the lang attribute which will happen in a forthcoming release (see alphagov/govuk_publishing_components#1686) but there are no ill effects from passing the extra parameter.

⚠️ This application is Continuously Deployed: ⚠️

  • Merged changes are automatically deployed to staging and production.

  • Make sure you follow the guidance for deployments before you merge.

  • Check your branch is being deployed in the Release app, after merging.

@bevanloon bevanloon temporarily deployed to government-f-withdrawn--ztryi1 September 15, 2020 07:31 Inactive
@maxgds maxgds force-pushed the withdrawn-notice-lang branch from 31346a5 to b72e30c Compare September 15, 2020 07:34
@bevanloon bevanloon temporarily deployed to government-f-withdrawn--ztryi1 September 15, 2020 07:34 Inactive
@maxgds maxgds force-pushed the withdrawn-notice-lang branch from b72e30c to ad18506 Compare September 15, 2020 08:07
@bevanloon bevanloon temporarily deployed to government-f-withdrawn--ztryi1 September 15, 2020 08:07 Inactive
@@ -9,11 +9,13 @@ def page_title
end

def withdrawal_notice_component
lang = I18n.locale.to_s == "en" ? false : "en"
Copy link
Contributor

Choose a reason for hiding this comment

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

One little code nitpick. As this variable is only used once, could this ternary operator be moved to the lang attribute below instead of being passed via this variable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good spot, updated.

Withdrawal notices are always presented in English but sometimes
appear on a translated page. This passes en as a language value to
the notice component when that happens.
@maxgds maxgds force-pushed the withdrawn-notice-lang branch from ad18506 to 2aee254 Compare September 15, 2020 09:19
@bevanloon bevanloon temporarily deployed to government-f-withdrawn--ztryi1 September 15, 2020 09:20 Inactive
Copy link
Contributor

@owenatgov owenatgov left a comment

Choose a reason for hiding this comment

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

👌

@maxgds maxgds merged commit f82a1e4 into master Sep 15, 2020
@maxgds maxgds deleted the withdrawn-notice-lang branch September 15, 2020 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants