Skip to content

Conversation

JamieC24
Copy link
Contributor

@JamieC24 JamieC24 commented Jul 25, 2025

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 the targetLinkLanguage field (if it's populated), it adds a hreflang attribute 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

  1. Ensure you have checked out the branch feature/#517-menu-target-lang in both the front-end and craft repos, and have run the necessary ddev composer updates on the frontend and ddev craft project-config/apply on the craft repo
  2. Go to one of the Main Navigation items in Craft and choose a "Target Link Language" from the dropdown that differs from the language of the site the navigation item belongs to
  3. Save your changes and check the front-end

If you would like to test this on another language, you can do the following:

  1. Change the language of the Craft dashboard to whichever language you wish to test (e.g Chinese)
  2. Repeat steps 2 and 3

Expected behaviour

If a navigation item has a Target Link Language that is different from the site language, it should have a hreflang in 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.

<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 %}
Copy link
Contributor

Choose a reason for hiding this comment

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

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') %}

{%~ 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 %}
Copy link
Contributor

Choose a reason for hiding this comment

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

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>
Copy link
Contributor

Choose a reason for hiding this comment

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

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>

{% 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 %}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can be simplified:

Suggested change
{% set childLangDiff = siteLang is same as child.targetLinkLanguage ? false : true %}
{% set childLangDiff = siteLang != child.targetLinkLanguage %}

{% set childLangDiff = siteLang is same as child.targetLinkLanguage ? false : true %}
<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>{# -#}
Copy link
Contributor

Choose a reason for hiding this comment

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

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>{# -#}

<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 %}
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like some debug information

Suggested change
{% if navItem.language is defined %}{{ navItem.language }}{% endif %}

@simonrjones simonrjones marked this pull request as draft July 29, 2025 14:32
@simonrjones
Copy link
Member

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.

@JamieC24 JamieC24 marked this pull request as ready for review August 5, 2025 13:40
@JamieC24 JamieC24 requested a review from jean-gui August 5, 2025 13:40
Comment on lines 58 to 62
{% if child.targetLinkLanguage == 'zh-Hans' %}
{% set targetLangLabel = "Chinese" %}
{% else %}
{% set targetLangLabel = "English" %}
{% endif %}
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't work for languages other than Chinese or English. The following code should make it generic, thanks to twig's language_name filter.

Suggested change
{% if child.targetLinkLanguage == 'zh-Hans' %}
{% set targetLangLabel = "Chinese" %}
{% else %}
{% set targetLangLabel = "English" %}
{% endif %}
{% set targetLangLabel = child.targetLinkLanguage|replace('-', '_')|language_name(siteLang) %}

Note that I replace - with _ because language_name seems to expect 'zh_Hans' instead of 'zh-Hans'

Comment on lines 22 to 26
{% if navItem.targetLinkLanguage == 'zh-Hans' %}
{% set targetLangLabel = "Chinese" %}
{% else %}
{% set targetLangLabel = "English" %}
{% endif %}
Copy link
Contributor

@jean-gui jean-gui Sep 3, 2025

Choose a reason for hiding this comment

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

See my previous comment on how to make this more generic

<a href="{{ navItem.titleLink }}" class="nav-link"{% if langDiff is defined and langDiff %} hreflang="{{ navItem.targetLinkLanguage }}"{% endif %}>
{{ navItem.title }}
{% if navItem.targetLinkLanguage is not empty %}
{{ (langDiff is defined and langDiff) ? '(' ~ targetLangLabel ~ ')' : '' }}
Copy link
Contributor

Choose a reason for hiding this comment

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

When the target language is different from the current one, you chose to display it as "(Language)". Do you think it should be styled to be less prominent?

https://www.w3.org/International/questions/qa-link-lang suggests some CSS, that, despite not being accessible (at least because of the contrast ratio), makes that less "in your face". It also suggests adding the language through CSS content rather than in the markup, but I don't know what are the implications of one or the other.

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.

3 participants