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

Support flattened global translations #344

Merged

Conversation

brockfanning
Copy link
Contributor

The sdg-translations project now has "flattened" global translation keys. For example, instead of this:

1:
    title: Foo
2:
    title: Bar

...now has this:

1-title: Foo
2-title: Bar

As a consequence several parts of the templates in Open SDG need to be adjusted. This is an attempt at that, as well as a general cleanup/deprecation of some indicator variables.

Copy link
Contributor

@LucyGwilliamAdmin LucyGwilliamAdmin left a comment

Choose a reason for hiding this comment

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

Tested this here - no errors and all text that comes from global translations looks to be present

@brockfanning brockfanning merged commit faec9dc into open-sdg:master Sep 9, 2019
@LucyGwilliamAdmin
Copy link
Contributor

Sorry, we've just realised we're having this again:
image

Is this something to do with the new translation changes?

@brockfanning
Copy link
Contributor Author

Hmm, let me take a look. More soon.

@brockfanning
Copy link
Contributor Author

Possible fix - try also upgrading to jekyll-open-sdg-plugins 0.0.14 here: https://github.com/ONSdigital/sdg-indicators/blob/flattened-translations/Gemfile#L8

@LucyGwilliamAdmin
Copy link
Contributor

Upgraded but still having issue

@brockfanning
Copy link
Contributor Author

Actually it looks like this is happening on the staging site already, so I don't think it's related to this PR. Do you have any idea when that started? Or did it never get resolved? I remember we were looking into that recently.

@brockfanning
Copy link
Contributor Author

@LucyGwilliamAdmin Think I see the problem. The next Open SDG upgrade (0.9.0) will need to be simultaneous with an SDG Translations upgrade (0.8.0, already released). So try changing this line to version 0.8.0.

@LucyGwilliamAdmin
Copy link
Contributor

So I have upgraded to 0.8.0 and that fixed the indicator title but now the goal title is gone from the ribbon on the goal page:
image

@brockfanning
Copy link
Contributor Author

@LucyGwilliamAdmin Sorry for the confusion - I meant to upgrade to sdg-translations 0.8.0 in your "flattened-translations" branch only - not in "develop". Any site using sdg-translations 0.8.0 will also need to be using open-sdg 0.9.0 (which we haven't released yet) and vice versa.

Hmm, given this issue maybe we should go ahead and release 0.9.0 of open-sdg?

@LucyGwilliamAdmin
Copy link
Contributor

@brockfanning sorry I thought you meant it would fix the issue on the staging/live site (which it did).

I think that might be best.

@LucyGwilliamAdmin
Copy link
Contributor

Should I finish the set chart axis feature first?

@LucyGwilliamAdmin
Copy link
Contributor

@brockfanning the tab title is also missing e.g.
image
but I guess that is also something to do with the upgrade of sdg-translations.

If we don't want to do a new release just to fix UK site, I can downgrade sdg-translations to 0.7.0 - we will then be back with the issue of having duplicate indicator codes in the banner on indicator pages. No idea where that came from - as we haven't made any commits that would have messed with that.

@brockfanning
Copy link
Contributor Author

I think I know what's going on - for now you could try downgrading back to sdg-translations 0.7.0, but also make this change: in Gemfile, change gem "jekyll-open-sdg-plugins", "~> 0.0.13" to gem "jekyll-open-sdg-plugins", "0.0.13". I think that will resolve the double-id problem.

@LucyGwilliamAdmin
Copy link
Contributor

This has fixed it - thanks!

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.

2 participants