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

Jump to bottom

Remove hardcoded strings from controllers, presenters and views #2116

Merged
merged 21 commits into from
May 27, 2021
Merged

Remove hardcoded strings from controllers, presenters and views #2116

merged 21 commits into from
May 27, 2021

Conversation

brucebolt
Copy link
Member

@brucebolt brucebolt commented May 24, 2021 •
edited
Loading

Moves all hardcoded English strings from controllers, presenters and views to the English locale file and adds placeholder keys for these in all other languages.

This will allow us to gain visibility of content that needs to be translated in order to fully support non-English content on GOV.UK.

This does not translate the "histories" pages which contain substantial amounts of hardcoded English content.

Trello card

⚠️ This repo is Continuously Deployed: make sure you follow the guidance ⚠️

@bevanloon bevanloon temporarily deployed to government-f-remove-har-agg3au May 24, 2021 14:13 Inactive
@brucebolt brucebolt force-pushed the remove-hardcoded-strings branch from f1393b6 to 03ff080 Compare May 24, 2021 14:16
@bevanloon bevanloon temporarily deployed to government-f-remove-har-agg3au May 24, 2021 14:16 Inactive
@brucebolt brucebolt force-pushed the remove-hardcoded-strings branch from 03ff080 to e18dbea Compare May 24, 2021 14:23
@bevanloon bevanloon temporarily deployed to government-f-remove-har-agg3au May 24, 2021 14:24 Inactive
@brucebolt brucebolt force-pushed the remove-hardcoded-strings branch from e18dbea to 8b662cc Compare May 24, 2021 14:40
@bevanloon bevanloon temporarily deployed to government-f-remove-har-agg3au May 24, 2021 14:40 Inactive
@brucebolt brucebolt force-pushed the remove-hardcoded-strings branch from 8b662cc to 8a3d7b0 Compare May 24, 2021 14:43
@bevanloon bevanloon temporarily deployed to government-f-remove-har-agg3au May 24, 2021 14:43 Inactive
@brucebolt brucebolt force-pushed the remove-hardcoded-strings branch from 8a3d7b0 to d53b94c Compare May 24, 2021 14:51
@bevanloon bevanloon temporarily deployed to government-f-remove-har-agg3au May 24, 2021 14:52 Inactive
@bevanloon bevanloon temporarily deployed to government-f-remove-har-agg3au May 24, 2021 15:31 Inactive
@brucebolt brucebolt force-pushed the remove-hardcoded-strings branch from 185715a to 6ad033a Compare May 24, 2021 15:53
@bevanloon bevanloon temporarily deployed to government-f-remove-har-agg3au May 24, 2021 15:53 Inactive
@brucebolt brucebolt force-pushed the remove-hardcoded-strings branch from 6ad033a to 94360bf Compare May 25, 2021 09:37
@bevanloon bevanloon temporarily deployed to government-f-remove-har-agg3au May 25, 2021 09:37 Inactive
@brucebolt brucebolt force-pushed the remove-hardcoded-strings branch from 94360bf to 098f900 Compare May 25, 2021 09:51
@bevanloon bevanloon temporarily deployed to government-f-remove-har-agg3au May 25, 2021 09:51 Inactive
@brucebolt brucebolt force-pushed the remove-hardcoded-strings branch from 098f900 to f2a5b8c Compare May 25, 2021 09:54
@bevanloon bevanloon temporarily deployed to government-f-remove-har-agg3au May 25, 2021 09:55 Inactive
@brucebolt brucebolt marked this pull request as ready for review May 25, 2021 10:08
thomasleese
thomasleese reviewed May 26, 2021
View reviewed changes
Copy link
Contributor

@thomasleese thomasleese left a comment

Choose a reason for hiding this comment

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

Looks good, I've left a few comments, although nothing major.

app/views/content_items/contact.html.erb Outdated Show resolved Hide resolved
app/views/content_items/gone.html.erb Outdated Show resolved Hide resolved
app/views/content_items/unpublishing.html.erb Outdated Show resolved Hide resolved
app/views/content_items/html_publication/_print_meta_data.html.erb Outdated Show resolved Hide resolved
@brucebolt brucebolt force-pushed the remove-hardcoded-strings branch from f2a5b8c to f6e6c78 Compare May 26, 2021 10:24
@bevanloon bevanloon temporarily deployed to government-f-remove-har-agg3au May 26, 2021 10:25 Inactive
brucebolt added 15 commits May 26, 2021 11:27
@brucebolt
Remove hardcoded strings from Travel Advice view
f3d6059
@brucebolt
Remove hardcoded strings from Unpublishing view
5ca4459
@brucebolt
Remove hardcoded strings from Working Group view
c6df8f9
@brucebolt
Remove hardcoded strings from print metadata partial
39afcb9
@brucebolt
Remove hardcoded strings from historically political partial
dc12497
@brucebolt
Remove hardcoded strings from webchat partial
8a04650
@brucebolt
Remove hardcoded strings from Fatality Notice presenter
c80bab0
@brucebolt
Remove hardcoded strings from Gone presenter
180ca48
@brucebolt
Remove hardcoded strings from Specialist Document presenter
e3dbdfc
@brucebolt
Remove hardcoded strings from Speech presenter
8f85f9c
@brucebolt
Remove hardcoded strings from Statistics Announcement presenter
61c4e7e
@brucebolt
Remove hardcoded strings from Travel Advice presenter
1bbf66c
@brucebolt
Remove hardcoded strings from Unpublishing presenter
9756a88
@brucebolt
Remove hardcoded strings from Working Group presenter
dd3b76c
@brucebolt
Add placeholder translation keys for all languages
0ec9f6c 
Recent commits have removed all hardcoded English content from the
controllers, presenters and views.

The content is currently only in the English locale file, so adding
placeholders to the other languages gives us visiblity of what needs to
be translated.
@brucebolt brucebolt force-pushed the remove-hardcoded-strings branch from f6e6c78 to 0ec9f6c Compare May 26, 2021 10:29
@bevanloon bevanloon temporarily deployed to government-f-remove-har-agg3au May 26, 2021 10:30 Inactive
@1pretz1
Copy link
Contributor

1pretz1 commented May 26, 2021

Nice work @brucebolt, just have one question. Are we not including specs in these changes? It's quite useful to have the specs using i18n as it means locale changes don't require the tests to be updated.

@brucebolt
Copy link
Member Author

Nice work @brucebolt, just have one question. Are we not including specs in these changes? It's quite useful to have the specs using i18n as it means locale changes don't require the tests to be updated.

That's a good question that I hadn't considered 🙈 . I'll see how much work it would be to update these too.

@brucebolt
Copy link
Member Author

Nice work @brucebolt, just have one question. Are we not including specs in these changes? It's quite useful to have the specs using i18n as it means locale changes don't require the tests to be updated.

Looks like this could be a rather difficult task. We've got strings in the tests that come from government-frontend, but also others from whitehall and other places so there will be a mixture of translations and hardcoded strings. Maybe we should leave the tests as there are for now, then they can be updated when the tests are next updated?

@1pretz1
Copy link
Contributor

1pretz1 commented May 26, 2021 •
edited
Loading

Sure thing, I don't think it's important enough to block this work but is a nice to have. As you say, these can be updated as and when they're changed, hopefully 🤞

I'll probably add them in for collections as I've already done some, and it hasn't got hairy, yet... 😄

1pretz1
1pretz1 approved these changes May 27, 2021
View reviewed changes
Copy link
Contributor

@1pretz1 1pretz1 left a comment

Choose a reason for hiding this comment

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

Looks good to me 🍏

@brucebolt brucebolt merged commit 70981a3 into main May 27, 2021
@brucebolt brucebolt deleted the remove-hardcoded-strings branch May 27, 2021 09:29
@CodeSonia CodeSonia mentioned this pull request Jun 20, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@1pretz1 1pretz1 1pretz1 approved these changes

@thomasleese thomasleese thomasleese approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@brucebolt @1pretz1 @thomasleese @bevanloon