Skip to content

Fix: Set lang and dir attributes for RTL languages in non-multilingual sites (#6851) #6855

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

superAman07
Copy link

Description
This pull request addresses issue #6851, which highlights a problem where the tag's lang attribute remains in English (en) and the dir attribute is missing when a page is set to a Right-to-Left (RTL) language in a non-multilingual site.

Changes Made
Dynamic lang Attribute:

The tag now dynamically sets the lang attribute based on the page's language.
If the page's language is not explicitly set, it falls back to the default locale.
Dynamic dir Attribute:

Added a dir attribute to the tag to handle text direction.
The direction is determined based on a predefined list of RTL languages (RTL_LANGUAGES).

Code Updates:
Modified the App.jsx file to include logic for determining the lang and dir attributes.
Used the Helmet component to inject these attributes into the tag.

Expected Behavior
When a page is set to an RTL language (e.g., Arabic, Hebrew), the tag will have:
lang="ar" (or the appropriate language code).
dir="rtl" for correct text alignment.
For Left-to-Right (LTR) languages, the dir attribute will default to ltr.

Testing
Since the full setup (including Docker) was not available, the changes were implemented based on the issue description and expected behavior. The logic for dynamically setting the lang and dir attributes has been added and should work as intended. However, the UI for changing the language was not tested directly.

Linked Issue
Fixes #6851.

Screenshots
N/A

Checklist
Code changes are implemented based on the issue description.
Changes are tested locally (UI testing was not performed due to setup limitations).
The issue is resolved as described.
No breaking changes introduced

Copy link

boring-cyborg bot commented Mar 23, 2025

Caution

The Volto Team has suspended its review of new pull requests from first-time contributors until the release of Plone 7, which is preliminarily scheduled for the second quarter of 2026.
Read details.

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, read
Plone's Code of Conduct,
Contributing to Plone,
First-time contributors, and
Contributing to Volto,
as this will greatly help the review process.

Welcome to the Plone community! 🎉

@mister-roboto
Copy link

@superAman07 you need to sign the Plone Contributor Agreement to merge this pull request.

Learn about the Plone Contributor Agreement: https://plone.org/foundation/contributors-agreement

If you have already signed the agreement, please allow a week for your agreement to be processed.
Once it is processed, you will receive an email invitation to join the plone GitHub organization as a Contributor.

If after a week you have not received an invitation, then please contact [email protected].

Copy link

netlify bot commented Mar 23, 2025

Deploy Preview for plone-components canceled.

Name Link
🔨 Latest commit 82e6838
🔍 Latest deploy log https://app.netlify.com/sites/plone-components/deploys/67dfc11b09132900082fd54e

@erral
Copy link
Member

erral commented Apr 8, 2025

Some comments:

  • In Classic UI, we have a closed list of languages that are RTL, in Classic case we just have 4 languages there, check it here: https://github.com/plone/plone.app.layout/blob/master/plone/app/layout/globals/portal.py#L21. So as a first step we need to check which languages are RTL, and update that list accordingly. I would also ask @terapyon whether Japanese is a RTL language and whether they had any issues with this.

  • As a second step, the list of languages can't live in App.jsx it should be somewhere in constants, perhaps adding a new file there and importing the list from there.

@terapyon
Copy link
Member

terapyon commented Apr 9, 2025

@erral
Japanese and almost East and SouthEast Asian languages are LTR, it same as European languages.
I think RTL languages are Arabic and Middle Asian and North African.

BTW, about 80 years ago, Japanese horizontal shown was RTL, and Japanese has vertical shown without Internet. newspapers and books.

@erral
Copy link
Member

erral commented Apr 9, 2025

Thanks @terapyon

@erral erral self-requested a review April 9, 2025 08:54
@@ -51,6 +51,31 @@ import RouteAnnouncer from '@plone/volto/components/theme/RouteAnnouncer/RouteAn
* @class App
* @extends {Component}
*/
//added more languages
const RTL_LANGUAGES = [
Copy link
Member

Choose a reason for hiding this comment

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

This should not be here but in constants

'zh',
'ja',
'ko',
];
Copy link
Member

Choose a reason for hiding this comment

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

The list of languages should be reviewed, not all listed here are RTL (for instance ja, see @terapyon's comment). As a reference this is the list of RTL languages in Classic: https://github.com/plone/plone.app.layout/blob/master/plone/app/layout/globals/portal.py#L21

@JeffersonBledsoe
Copy link
Member

i18next has the following language codes listed as RTL:

['ar', 'shu', 'sqr', 'ssh', 'xaa', 'yhd', 'yud', 'aao', 'abh', 'abv', 'acm', 'acq', 'acw', 'acx', 'acy', 'adf', 'ads', 'aeb', 'aec', 'afb', 'ajp', 'apc', 'apd', 'arb', 'arq', 'ars', 'ary', 'arz', 'auz', 'avl', 'ayh', 'ayl', 'ayn', 'ayp', 'bbz', 'pga', 'he', 'iw', 'ps', 'pbt', 'pbu', 'pst', 'prp', 'prd', 'ug', 'ur', 'ydd', 'yds', 'yih', 'ji', 'yi', 'hbo', 'men', 'xmn', 'fa', 'jpr', 'peo', 'pes', 'prs', 'dv', 'sam', 'ckb']
https://github.com/i18next/i18next/blob/59498aede43b28d2c29806da0fb947c8c62795b1/i18next.js#L2142

@erral Any thoughts on if it's worth fetching the list of languages from the backend so it's consistent across all of Plone?
It's probably more convenient as a frontend developer to have them all listed here I suppose

@erral
Copy link
Member

erral commented Apr 11, 2025

i18next has the following language codes listed as RTL:

['ar', 'shu', 'sqr', 'ssh', 'xaa', 'yhd', 'yud', 'aao', 'abh', 'abv', 'acm', 'acq', 'acw', 'acx', 'acy', 'adf', 'ads', 'aeb', 'aec', 'afb', 'ajp', 'apc', 'apd', 'arb', 'arq', 'ars', 'ary', 'arz', 'auz', 'avl', 'ayh', 'ayl', 'ayn', 'ayp', 'bbz', 'pga', 'he', 'iw', 'ps', 'pbt', 'pbu', 'pst', 'prp', 'prd', 'ug', 'ur', 'ydd', 'yds', 'yih', 'ji', 'yi', 'hbo', 'men', 'xmn', 'fa', 'jpr', 'peo', 'pes', 'prs', 'dv', 'sam', 'ckb'] https://github.com/i18next/i18next/blob/59498aede43b28d2c29806da0fb947c8c62795b1/i18next.js#L2142

@erral Any thoughts on if it's worth fetching the list of languages from the backend so it's consistent across all of Plone? It's probably more convenient as a frontend developer to have them all listed here I suppose

The list is fixed, so it will never change, so I see no problem on duplicating them on the frontend and the backend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Needs discussion
Development

Successfully merging this pull request may close these issues.

5 participants