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

Render translations correctly on publications tagged to a taxon #658

Merged
merged 1 commit into from Jan 3, 2018

Conversation

@fofr
Copy link
Contributor

@fofr fofr commented Jan 3, 2018

Because of the way we use template variants, when we call the shared/translations partial it sometimes renders the shared/translations+taxonomy_navigation variant.

Publications that are tagged to a new taxonomy use this variant to show an alternative breadcrumb, so we can't remove the variant completely for Publications.

The taxonomy_nav version of the translation partial expects to be inlined in content, so doesn’t have a grid class wrapping it. Because of Rails magic loading in this variant the code looks fine, and it only breaks on certain publications which meet the taxonomy nav criteria.

  • Don’t use the shared translations template

This duplicates some code which can be DRY’d up when we no longer need
the taxonomy_nav variants.

Before

screen shot 2018-01-03 at 17 49 54

After

screen shot 2018-01-03 at 17 48 46

Because of the way we use template variants, when we call the
shared/translations partial it renders the
shared/translations+taxonomy_navigation variant. Publications that are
tagged to a new taxonomy use this variant to show an alternative
breadcrumb.

However, the taxonomy_nav version of the partial expects to be inlined
in content, so doesn’t have a grid class wrapping it.

Because of Rails magic loading in this variant the code looks fine, and
it only breaks on certain publications which meat the taxonomy nav
criteria.

* Don’t use the shared translations template

This duplicates some code which can be DRY’d up when we no longer need
the taxonomy_nav variants.
Copy link
Contributor

@steventux steventux left a comment

👍 Good spot @fofr, as mentioned, template variants should be cleaned up as soon as possible.

@fofr fofr merged commit 21efca6 into master Jan 3, 2018
2 checks passed
2 checks passed
continuous-integration/jenkins/branch This commit looks good
Details
continuous-integration/jenkins/publishing-e2e-tests Publishing end-to-end tests succeeded on Jenkins
Details
@fofr fofr deleted the fix-guidance-pub branch Jan 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.