-
Notifications
You must be signed in to change notification settings - Fork 95
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
The vocabulary list on the frontpage shows the vocabulary names in th… #1259
base: skosmos-2
Are you sure you want to change the base?
Conversation
…e default language of the vocabulary
Codecov Report
@@ Coverage Diff @@
## master #1259 +/- ##
=========================================
Coverage 69.26% 69.26%
Complexity 1646 1646
=========================================
Files 32 32
Lines 4041 4041
=========================================
Hits 2799 2799
Misses 1242 1242 Continue to review full report at Codecov.
|
view/vocabularylist.twig
Outdated
{% set useDefaultLanguage = null %} | ||
{% if request.contentLang not in vocab.config.languages and request.contentLang != '' %} | ||
{% set useDefaultLanguage = true %} | ||
{% endif %} | ||
<li><a class="navigation-font" href="{{ vocab.id }}/{{ request.lang }}/{% if request.contentLang not in vocab.config.languages and request.contentLang != '' %}?clang={{ vocab.config.defaultLanguage }}{% endif %}"> | ||
{% if useDefaultLanguage is not null %} {{ vocab.title(vocab.config.defaultLanguage) }} | ||
{% else %} {{ vocab.title(request.contentLang) }} | ||
{% endif %} </a></li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't we use something like (not tested) this?
{% set useDefaultLanguage = null %} | |
{% if request.contentLang not in vocab.config.languages and request.contentLang != '' %} | |
{% set useDefaultLanguage = true %} | |
{% endif %} | |
<li><a class="navigation-font" href="{{ vocab.id }}/{{ request.lang }}/{% if request.contentLang not in vocab.config.languages and request.contentLang != '' %}?clang={{ vocab.config.defaultLanguage }}{% endif %}"> | |
{% if useDefaultLanguage is not null %} {{ vocab.title(vocab.config.defaultLanguage) }} | |
{% else %} {{ vocab.title(request.contentLang) }} | |
{% endif %} </a></li> | |
{% set useDefaultLanguage = request.contentLang not in vocab.config.languages and request.contentLang != '' %} | |
<li><a class="navigation-font" href="{{ vocab.id }}/{{ request.lang }}/{% if useDefaultLanguage %}?clang={{ vocab.config.defaultLanguage }}{% endif %}"> | |
{% if useDefaultLanguage %} {{ vocab.title(vocab.config.defaultLanguage) }} | |
{% else %} {{ vocab.title(request.contentLang) }} | |
{% endif %} </a></li> |
I think we have the same condition in two places, so maybe storing the true
or false
value of that condition in that variable would simplify it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The way you proposed was clearly more elegant, I changed the code. Thanks a lot @kinow
… the vocabulary listing
Kudos, SonarCloud Quality Gate passed! |
Another question is where the business logic is located. For instance, I think it is also possible to modify a request according to the needs mentioned in the original issue. The configuration and therefore the default language of the vocabulary could be checked before accessing the data depending on the request. |
@@ -24,3 +30,8 @@ | |||
</div> | |||
{% endif %} | |||
</div> | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess the newlines are not needed?
…e default language of the vocabulary
Thanks @henriyli, @osma and @kouralex for your contribution :-)
Reasons for creating this PR
To follow wishes mentioned in the PR #747
Link to relevant issue(s), if any
#731
Description of the changes in this PR
The vocabulary list on the front page shows the vocabulary names in the default language of the vocabulary
Known problems or uncertainties in this PR
Checklist
When it comes to the phpUnit tests, at the moment I am investigating ways to run PHPUnit tests for Symphony Twig templates. Tests are coming a bit later.