-
Notifications
You must be signed in to change notification settings - Fork 93
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
SEO functionality backport for skosmos 2 #1666
Conversation
Quality Gate passedIssues Measures |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## skosmos-2 #1666 +/- ##
===============================================
+ Coverage 70.33% 70.39% +0.06%
- Complexity 1669 1674 +5
===============================================
Files 32 32
Lines 4305 4314 +9
===============================================
+ Hits 3028 3037 +9
Misses 1277 1277 ☔ View full report in Codecov by Sentry. |
{% block title %}: {{ vocab.shortName }}{% if search_results|length == 1 %}: {{ concept_label }}{% endif %} | ||
{% endblock %} | ||
{% block title %}{{ concept_label }} - {{ vocab.shortName }} - {{ GlobalConfig.serviceName }}{% endblock %} | ||
{% block description %}{% set term = concept_label %}{% set vocabObj = vocab %}{% set vocab = vocab.title(request.contentLang) %}{% trans %}Concept {{ term }} in vocabulary {{ vocab }}{% endtrans %}{% set vocab = vocabObj %}{% endblock %} |
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 minor quip is that we have only the translation strings for "Concept X" in different languages, whereas it look a bit weird in cases where the data is instances or classification:
- Skosmos2/ykl/en/page/2
- "Concept RELIGION in vocabulary PLC - Finnish Public Libraries Classification System"
- Could be currently generated as "YKL Class RELIGION ..."
- Skosmos2/finaf/en/page/000179638
- "Concept A. Aallon Rytmiorkesteri in vocabulary KANTO - Kansalliset toimijatiedot"
- Could be currently merely generated as "Yhteisö A. Aallon Rytmiorkesteri ..." as there's no translation for the concept type in english in the data
One way would be to omit the word "Concept" but it's a minor thing.
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.
Good point! This was part of the speccing done earlier. I guess we didn't consider classifications back then.
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.
Works as expected. I encountered some minor awkwardness in the wordings of the descriptions in concept page that I hadn't spotted before. Could be a subject for further improvement.
Reasons for creating this PR
Some new SEO / social media sharing features were implemented for Skosmos 3 in several PRs related to issue #1533. This PR is a backport of these features to Skosmos 2.
Link to relevant issue(s), if any
Description of the changes in this PR
Contains commits cherry-picked and/or adapted from the following PRs that were made for Skosmos 3:
The Twig template code changes had to be rewritten because Skosmos 2 has a very different template structure than Skosmos 3. While doing this, I decided to merge
meta.twig
intolight.twig
. I also created a separatelanding.twig
template; previouslylight.twig
was used both as a base template and as the template for the landing page, but this wasn't flexible enough for implementing the meta descriptions.Known problems or uncertainties in this PR
None so far 🤞
Checklist
.sr-only
class, color contrast)