Support for Right-to-left languages #4430

Closed
opened 2020-06-24 09:46:29 +02:00 by marko-lorentz · 10 comments
marko-lorentz commented 2020-06-24 09:46:29 +02:00 (Migrated from github.com)

Right-to-left languages are not properly supported by the application. Neither direction switch nor CSS adaptions are implemented yet.

Languages in question

  • Arabic (> 310 M speakers)
  • Hebrew (> 9 M speakers)
  • Persian (> 70 M speakers)
  • Urdu (> 60 M speakers)
  • Sindhi (> 25 M speakers)

Additions/Changes required

  • Must: HTML/CSS needs to allow switching the direction
  • Must: Current language must be taken into account when considering direction
  • Must: Language switcher might need to kick direction switching
  • Must: Some CSS rules are broken from the RTL point of view as they force margins to be on the left or right of an element. This no longer works after changing the direction. Margins must follow element flow direction instead.
    Example for wrong usage of margin in the main menu:
    .menu__list { margin-left: calc(var(--spacing-m) * -1); ...}
    image
  • Nice to have: Encapsulation of English terms that appear in RTL text to avoid that words are moved to the beginning or end of the line by the rendering engine
  • Nice to have: auto-detection of direction for <span>s with varying content

Further reading

[Right-to-left languages](https://en.wikipedia.org/wiki/Right-to-left) are not properly supported by the application. Neither direction switch nor CSS adaptions are implemented yet. **Languages in question** * Arabic (> 310 M speakers) * Hebrew (> 9 M speakers) * Persian (> 70 M speakers) * Urdu (> 60 M speakers) * Sindhi (> 25 M speakers) **Additions/Changes required** * Must: HTML/CSS needs to allow switching the direction * Must: Current language must be taken into account when considering direction * Must: Language switcher might need to kick direction switching * Must: Some CSS rules are broken from the RTL point of view as they force margins to be on the left or right of an element. This no longer works after changing the direction. Margins must follow element flow direction instead. Example for wrong usage of margin in the main menu: `.menu__list { margin-left: calc(var(--spacing-m) * -1); ...}` ![image](https://user-images.githubusercontent.com/66749449/85516456-8079ad80-b5fe-11ea-969b-c038656512b0.png) * Nice to have: Encapsulation of English terms that appear in RTL text to avoid that words are moved to the beginning or end of the line by the rendering engine * Nice to have: auto-detection of direction for `<span>`s with varying content **Further reading** * [Mozilla RTL Guidelines](https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/RTL_Guidelines)
marko-lorentz commented 2020-06-24 10:08:50 +02:00 (Migrated from github.com)

Proposal for solution
My employee @sanabhass is about to prepare a PR but might not be able to fully complete it without some discussion with the core team.

(Maybe it would be worth considering to have a branch in the main repo instead of a fork to ease collaboration.)

**Proposal for solution** My employee @sanabhass is about to prepare a PR but might not be able to fully complete it without some discussion with the core team. (Maybe it would be worth considering to have a branch in the main repo instead of a fork to ease collaboration.)
sanabhass commented 2020-06-29 17:03:07 +02:00 (Migrated from github.com)

Done

  • Direction for frame in <html>.
  • Style sheet files of the lbry-desktop and components modified in a way that support switching the direction correctly.
  • Auto-detection of direction for content with varying direction (rtl and ltr content).

Tests

Not yet supported :

  • Automatic switching based an the language setting
**Done** - [x] Direction for frame in <html>. - [x] Style sheet files of the lbry-desktop and components modified in a way that support switching the direction correctly. - [ ] Auto-detection of direction for content with varying direction (rtl and ltr content). **Tests** - [x] Create [channel ](lbry://@سناء#9) with an arabic name : OK - [ ] Upload [Markdown content](lbry://@سناء#9/إرشادات_RTL#c) in arabic in imbeding html for direction setting : The direction is broken. - [x] Upload [HTML content](lbry://@سناء#9/rtl_Guidelines#d) in arabic : OK **Not yet supported :** - [ ] Automatic switching based an the language setting
marko-lorentz commented 2020-06-30 11:58:43 +02:00 (Migrated from github.com)

@seanyesmunt
Could we get some support from a team member in order to solve the last open issue (switch the direction depending on the language setting)?

(We can already provide a PR for components for review if you like. The PR for lbry-desktop will need some input as mentioned above.)

@seanyesmunt Could we get some support from a team member in order to solve the last open issue (switch the direction depending on the language setting)? (We can already provide a PR for `components` for review if you like. The PR for `lbry-desktop` will need some input as mentioned above.)
neb-b commented 2020-06-30 16:24:26 +02:00 (Migrated from github.com)

Hey @marko-lorentz, sorry I missed this. I just got back from vacation yesterday.

I support this idea. I am not sure how the markdown content will work. We use this for our editor
https://github.com/RIP21/react-simplemde-editor (it might be simple and I am just missing something).

For automatic switching based on language setting:

I would put the direction "setting" here https://github.com/lbryio/lbry-desktop/blob/master/ui/redux/reducers/settings.js#L37

Then right here you can check if the dispatched action is for updating the language. If it is, you can change the direction to the appropriate setting.

If you open a PR I would be happy to review/test it.

Hey @marko-lorentz, sorry I missed this. I just got back from vacation yesterday. I support this idea. I am not sure how the markdown content will work. We use this for our editor https://github.com/RIP21/react-simplemde-editor (it might be simple and I am just missing something). For automatic switching based on language setting: I would put the direction "setting" here https://github.com/lbryio/lbry-desktop/blob/master/ui/redux/reducers/settings.js#L37 Then right here you can check if the dispatched action is for updating the language. If it is, you can change the direction to the appropriate setting. If you open a PR I would be happy to review/test it.
sanabhass commented 2020-07-01 19:13:59 +02:00 (Migrated from github.com)

@seanyesmunt
We followed what you did in the theme to apply the direction changes. We used CSS Custom Properties (Variables) so that we need only to add the tag direction = "rtl" to the html frame to switch the direction to RTL. I am wondering were can we add the direction tag to the html frame. e.g:
<html theme="light" direction="rtl">
We provided a PR for the lbryio/components and a second PR for the lbry-desktop is on the way so that you can have a look and understand better what we did till now to get some support.

@seanyesmunt We followed what you did in the theme to apply the direction changes. We used CSS Custom Properties (Variables) so that we need only to add the tag direction = "rtl" to the html frame to switch the direction to RTL. I am wondering were can we add the direction tag to the html frame. e.g: `<html theme="light" direction="rtl">` We provided a [PR](https://github.com/lbryio/components/pull/19) for the lbryio/components and a second PR for the lbry-desktop is on the way so that you can have a look and understand better what we did till now to get some support.
neb-b commented 2020-07-01 23:16:20 +02:00 (Migrated from github.com)

@sanabhass after spending more time thinking about this, I think something like this would be more useful
https://github.com/nicolashemonic/rtl-css-transform-webpack-plugin

It appears twitter uses something similar. I don't think always writing css like this will be very sustainable, and not very easy to start using for developers new the project (I didn't even know a lot of these properties existed before your PR).

For these few languages, we could load a separate css file that just has most properties flipped.

Sorry I didn't recognize this earlier but dealing with RTL languages is very new to me!

@sanabhass after spending more time thinking about this, I think something like this would be more useful https://github.com/nicolashemonic/rtl-css-transform-webpack-plugin It appears twitter uses something similar. I don't think always writing css like this will be very sustainable, and not very easy to start using for developers new the project (I didn't even know a lot of these properties existed before your PR). For these few languages, we could load a separate css file that just has most properties flipped. Sorry I didn't recognize this earlier but dealing with RTL languages is very new to me!
sanabhass commented 2020-07-08 15:03:35 +02:00 (Migrated from github.com)

@seanyesmunt
after spending some time looking through your proposal, we think that this https://github.com/vkalinichev/postcss-rtl plugin would be more useful. It's basically doing the same work as the https://github.com/nicolashemonic/rtl-css-transform-webpack-plugin but it is better maintained. We integrated it in the project and with a few lines of code everything seems to work perfectly.
( But, still need to handle the auto-detection of direction for content with varying direction (rtl and ltr content) e.g channel name, video title, comments, etc. Basically there is two options to detect the direction at run time, add dir=auto to input elements or handle bidirectional content by aligning each content depending on the dominant directionality of the content characters. To figure out which chars were RTL, we can use this regex: /[\u0600-\u06FF]|[\u0750-\u077F]|[\u0590-\u05FF]|[\uFE70-\uFEFF]/m. Also, still need support to be able to add the direction tag to the html frame. e.g: <html theme="light" dir="rtl"> )
We provided a PR for the lbryio/lbry-desktop containing changes to integrate the PostCSS-RTL plugin .

@seanyesmunt after spending some time looking through your proposal, we think that this [https://github.com/vkalinichev/postcss-rtl](https://github.com/vkalinichev/postcss-rtl) plugin would be more useful. It's basically doing the same work as the [https://github.com/nicolashemonic/rtl-css-transform-webpack-plugin](https://github.com/nicolashemonic/rtl-css-transform-webpack-plugin) but it is better maintained. We integrated it in the project and with a few lines of code everything seems to work perfectly. ( But, still need to handle the auto-detection of direction for content with varying direction (rtl and ltr content) e.g channel name, video title, comments, etc. Basically there is two options to detect the direction at run time, add `dir=auto` to input elements or handle bidirectional content by aligning each content depending on the dominant directionality of the content characters. To figure out which chars were RTL, we can use this regex: /[\u0600-\u06FF]|[\u0750-\u077F]|[\u0590-\u05FF]|[\uFE70-\uFEFF]/m. Also, still need support to be able to add the direction tag to the html frame. e.g: `<html theme="light" dir="rtl">` ) We provided a [PR](https://github.com/lbryio/lbry-desktop/pull/4496) for the lbryio/lbry-desktop containing changes to integrate the [PostCSS-RTL](https://github.com/vkalinichev/postcss-rtl) plugin .
marko-lorentz commented 2021-06-18 16:20:09 +02:00 (Migrated from github.com)

(Up ⬆️)

We forgot to finish and close this issue last year. Mea culpa.
Few missing lines of code coming in PR soon.

(Up ⬆️) We forgot to finish and close this issue last year. Mea culpa. Few missing lines of code coming in PR soon.
marko-lorentz commented 2021-07-20 16:56:20 +02:00 (Migrated from github.com)

Looks (technically) ok after the merge.
Tests with some native speakers ongoing.

Looks (technically) ok after the merge. Tests with some native speakers ongoing.
marko-lorentz commented 2021-07-22 12:03:03 +02:00 (Migrated from github.com)

No bigger problems found. Closing.

No bigger problems found. Closing.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: LBRYCommunity/lbry-desktop#4430
No description provided.