Add check against site language to navItem language to determine href… #175
+42
−3
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jean-gui
requested changes
Jul 29, 2025
| @@ -1,6 +1,7 @@ | |||
| <nav id="global-nav" aria-label="Main"> | |||
| <div class="global-nav__inner l-center"> | |||
| <a class="logo-link" href="{% block homepage_link %}/{% endblock %}" hreflang="{% block homepage_hreflang %}en{% endblock %}"> | |||
| {% set siteLang %} {% block homepage_hreflang %}en{% endblock %} {% endset %} | |||
There was a problem hiding this comment.
I don't think the block is useful here.
Suggested change
| {% set siteLang %} {% block homepage_hreflang %}en{% endblock %} {% endset %} | |
| {% set siteLang = site.locale|default('en') %} |
| @@ -11,7 +12,9 @@ | |||
| <ul data-component="nav-double-intro"> | |||
| {%~ for navItem in navigation %} | |||
| <li class="top-nav-item has-children"> | |||
| <a href="{{ navItem.titleLink }}" class="nav-link">{{ navItem.title }}</a> | |||
| {% set langDiff = siteLang is same as navItem.targetLinkLanguage ? false : true %} | |||
There was a problem hiding this comment.
This can be simplified to:
Suggested change
| {% set langDiff = siteLang is same as navItem.targetLinkLanguage ? false : true %} | |
| {% set langDiff = siteLang != navItem.targetLinkLanguage %} |
| <a href="{{ navItem.titleLink }}" class="nav-link">{{ navItem.title }}</a> | ||
| {% set langDiff = siteLang is same as navItem.targetLinkLanguage ? false : true %} | ||
| {% if navItem.language is defined %}{{ navItem.language }}{% endif %} | ||
| <a href="{{ navItem.titleLink }}" class="nav-link"{% if langDiff %}hreflang="{{ navItem.targetLinkLanguage }}"{% endif %}>{{ navItem.title }}</a> |
There was a problem hiding this comment.
Space mising:
Suggested change
| <a href="{{ navItem.titleLink }}" class="nav-link"{% if langDiff %}hreflang="{{ navItem.targetLinkLanguage }}"{% endif %}>{{ navItem.title }}</a> | |
| <a href="{{ navItem.titleLink }}" class="nav-link"{% if langDiff %} hreflang="{{ navItem.targetLinkLanguage }}"{% endif %}>{{ navItem.title }}</a> |
| @@ -28,8 +31,9 @@ | |||
| {% if navItem.children is defined and navItem.children|length > 0 -%} | |||
| <ul> | |||
| {%~ for child in navItem.children %} | |||
| {% set childLangDiff = siteLang is same as child.targetLinkLanguage ? false : true %} | |||
There was a problem hiding this comment.
Can be simplified:
Suggested change
| {% set childLangDiff = siteLang is same as child.targetLinkLanguage ? false : true %} | |
| {% set childLangDiff = siteLang != child.targetLinkLanguage %} |
| <li{{ child.startNewColumn is defined and child.startNewColumn ? ' class="break-after"' : '' }}>{# -#} | ||
| <a href="{{ child.url }}">{{ child.title }}</a>{# -#} | ||
| <a href="{{ child.url }}{% if childLangDiff %}hreflang="{{ child.targetLinkLanguage }}"{% endif %}">{{ child.title }}</a>{# -#} |
There was a problem hiding this comment.
Missing space and wrong quoting:
Suggested change
| <a href="{{ child.url }}{% if childLangDiff %}hreflang="{{ child.targetLinkLanguage }}"{% endif %}">{{ child.title }}</a>{# -#} | |
| <a href="{{ child.url }}"{% if childLangDiff %} hreflang="{{ child.targetLinkLanguage }}"{% endif %}>{{ child.title }}</a>{# -#} |
| @@ -11,7 +12,9 @@ | |||
| <ul data-component="nav-double-intro"> | |||
| {%~ for navItem in navigation %} | |||
| <li class="top-nav-item has-children"> | |||
| <a href="{{ navItem.titleLink }}" class="nav-link">{{ navItem.title }}</a> | |||
| {% set langDiff = siteLang is same as navItem.targetLinkLanguage ? false : true %} | |||
| {% if navItem.language is defined %}{{ navItem.language }}{% endif %} | |||
There was a problem hiding this comment.
Looks like some debug information
Suggested change
| {% if navItem.language is defined %}{{ navItem.language }}{% endif %} |
|
This ticket related to task w3c/w3c-website#517 It is currently being tested internall. We'll update the task & convert this PR to "Ready for review" once this is done. Sorry for any confusion on the status of this one. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request adds support for the "targetLinkLanguage" field that is added by a branch of the same name in the w3c-website-craft repo and the w3c-website-frontend repo
When it detects that the language of the current site (
siteLang) is different from the value of thetargetLinkLanguagefield (if it's populated), it adds ahreflangattribute to the relevant<a>tag as well as some indicative text following the current text inside the<a>tag.This has been tested locally and seems to be working as intended
How to test
ddev composer updates on the frontend andddev craft project-config/applyon the craft repoIf you would like to test this on another language, you can do the following:
Expected behaviour
If a navigation item has a Target Link Language that is different from the site language, it should have a
hreflangin the markup for the anchor tag for that link, and there should be an indicator (in the Roman Alphabet / Romaji) of what language the target page is in. [e.g - Working Groups (Chinese)] or something similar.