Skip to content
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

Adding direction to the client- participation #1561

Merged
merged 9 commits into from
Jan 4, 2023
Merged

Adding direction to the client- participation #1561

merged 9 commits into from
Jan 4, 2023

Conversation

xeeg
Copy link
Contributor

@xeeg xeeg commented Nov 26, 2022

Adding option to indicate the direction 'rtl' or 'ltr to client-participation through specifying s.dir in the language-specific strings file. It defaults to 'ltr'.

related issue: #513

@colinmegill
Copy link
Member

Yay! Thanks @xeeg this is a long standing issue and I know will help users around the world. I'll try to review this weekend.

@xeeg
Copy link
Contributor Author

xeeg commented Nov 26, 2022

Thanks for the references Colin! both links have great checklists to go through!

Putting the rtl on the root html was causing some of those issues (mirroring and missaligning the logo and footer) so I put it on the inner div for the comments section. I'll go through the css and check for other issues.

In the meantime, is putting the indicator for rtl in the strings file as opposed to config an acceptable approach?

@colinmegill
Copy link
Member

colinmegill commented Nov 27, 2022

is putting the indicator for rtl in the strings file as opposed to config an acceptable approach?

The strings file is going to be loaded dynamically depending on the browser language. This means that if the browser language is Farsi, it will load the Farsi strings file and add 'rtl'. Is it likely that if the browser language is Farsi (or Hebrew) that the person is showing up to a conversation with a topic and description which is also written in that language? Perhaps it's pretty likely, and relying on that is a good heuristic.

I think the other options are allowing the moderator to set the language in config, as you suggest, which requires database work for this PR, or to dynamically test the language of each line and render it accordingly, which adds third party translation (which we already have integrated!) but large number of network calls related to rendering.

Edge case: Hebrew browser lang visits French conversation

I think this is our best option for this PR, even if we consider it a first step. If someone with a browser language set to Hebrew joins a conversation with a topic and description in French, the French topic and description will appear RTL and that's not ideal, while the instructions and help text will appear in Hebrew, correctly, in RTL.

Edge case: Hebrew translation of French comment

There are important edge cases for bilingual scenarios that should become an issue before merging this PR:

  • create an issue for addressing rtl issues in automatic translations in the voting interface

@colinmegill
Copy link
Member

A potentially useful new browser api: https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/i18n/detectLanguage

i18n.detectLanguage()
Detects the language of the provided text using the [Compact Language Detector](https://github.com/CLD2Owners/cld2) (CLD).

This is an asynchronous function that returns a [Promise](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise).

@xeeg
Copy link
Contributor Author

xeeg commented Nov 29, 2022

Thanks for raising the issue of user inputs and the idea of detecting the language dynamically.

I think the website/admin language needs to support direction independently from the user text inputs (admins and participants). The original commit sets the language for the app and will cover the first case. For the second case most modern apps seem to set the direction at input time. It will be helpful for users to see the text as it will be displayed after it's submitted.

Turns out, by setting the direction to "auto", the browser can detect the correct direction, even for bidirectional text including a mix of rtl and ltr languages.
See: https://www.w3.org/International/questions/qa-bidi-unicode-controls

I still haven't been able to get it to figure out how to do it for the textarea. In worst case scenario we could use this function to detect whether the text is in RTL or LTR from the unicode data for text input: https://stackoverflow.com/questions/12006095/javascript-how-to-check-if-character-is-rtl/14824756#14824756

@xeeg
Copy link
Contributor Author

xeeg commented Nov 29, 2022

(This is not complete yet, there's still a mysterious styling issue with margins on the vote section showing rtl text when browser is in english I need to fix)

@xeeg
Copy link
Contributor Author

xeeg commented Dec 1, 2022

Screenshots of the client participation views in RTL mode:

Website RTL, content LTR

Description & Subscribing to notifications:
Screen Shot 2022-11-30 at 5 52 50 PM
Voting:
Screen Shot 2022-11-30 at 6 46 03 PM

Website RTL, content RTL
Description & Subscribing to notifications:
Screen Shot 2022-11-30 at 3 30 02 PM
Submitting new statements & Voting:
Screen Shot 2022-11-30 at 5 52 59 PM

Website LTR, content RTL
Screen Shot 2022-11-30 at 5 56 35 PM

Website LTR, content LTR
Screen Shot 2022-11-30 at 5 56 06 PM
Screen Shot 2022-11-30 at 5 56 14 PM

@metasoarous
Copy link
Member

Hi @xeeg. Thanks again for this PR!

I wasn't able to push to your fork so I created a site-direction branch on our fork that:

  • resolves conflicts with the edge branch
  • switches from dir to direction in the strings config files for clarity
  • adds this attribute to the Farsi and Hebrew strings files (I don't know of any other RTL langs we support currently)

I gave it a spin to make sure my dir -> direction switch works, and realized that while everything seems fine on Chrome (R), the conversation prompt does not display properly on Firefox (L):

image

I think this still seems like an improvement over what is in production currently, so I'm happy to go ahead and merge this, unless you'd rather look into Firefox support first.

Thanks again!

@xeeg
Copy link
Contributor Author

xeeg commented Dec 17, 2022

Thank you so much for the updates Chris.

What you see in firefox is totally acceptable and is an improvement. But using this same branch I can't reproduce the issue and It looks fine to me. However I noticed a bug in chrome, where the float style of the "comments remaining" is not working. Apparently chrome doesn't support this particular inline-end and inline-start. Using the property for margins seems to be supported. https://caniuse.com/mdn-css_properties_float_flow_relative_values

I fixed it locally but now I don't have permissions to push to the edge/site-direction :-)

@xeeg
Copy link
Contributor Author

xeeg commented Dec 17, 2022

Here's the fix:

whilo@4eb5d1b

@xeeg
Copy link
Contributor Author

xeeg commented Dec 18, 2022

I merged this branch with your changes and tested and it seems fine!

@metasoarous metasoarous merged commit 58906b6 into compdemocracy:edge Jan 4, 2023
ballPointPenguin pushed a commit that referenced this pull request Feb 26, 2023
* edge: (22 commits)
  Configuration unification (#1617)
  housekeeping (#1622)
  Remove intercom (#1547)
  Reduced bundlesize by requiring only victory-core for VictoryAnimation component. (#1618)
  A variety of improvements all around (#1608)
  Update client-participation dependencies, and get off gulp -> webpack (#1607)
  Feature/crkrenn/fixed participants extended sql table (#1586)
  Switch server to debian (#1600)
  More slack removal (#1599)
  Update server node & misc dep versions (#1595)
  switch from npm ci to npm install in client dockerfiles (#1594)
  deleted docker_cloud_notes (#1597)
  Remove slack (#1585)
  specify bullseye debian base for math dockerfile (#1590)
  Feature/crkrenn/improved makefile (#1577)
  Adding direction to the client- participation (#1561)
  update winston package (#1584)
  add scaling roadmap doc (#1582)
  Feature/sirodoht crk/e2e version 11 (#1576)
  German language editing (#1552)
  ...
ballPointPenguin pushed a commit that referenced this pull request Mar 4, 2023
* edge: (22 commits)
  Configuration unification (#1617)
  housekeeping (#1622)
  Remove intercom (#1547)
  Reduced bundlesize by requiring only victory-core for VictoryAnimation component. (#1618)
  A variety of improvements all around (#1608)
  Update client-participation dependencies, and get off gulp -> webpack (#1607)
  Feature/crkrenn/fixed participants extended sql table (#1586)
  Switch server to debian (#1600)
  More slack removal (#1599)
  Update server node & misc dep versions (#1595)
  switch from npm ci to npm install in client dockerfiles (#1594)
  deleted docker_cloud_notes (#1597)
  Remove slack (#1585)
  specify bullseye debian base for math dockerfile (#1590)
  Feature/crkrenn/improved makefile (#1577)
  Adding direction to the client- participation (#1561)
  update winston package (#1584)
  add scaling roadmap doc (#1582)
  Feature/sirodoht crk/e2e version 11 (#1576)
  German language editing (#1552)
  ...
ballPointPenguin pushed a commit that referenced this pull request Mar 5, 2023
* edge: (22 commits)
  Configuration unification (#1617)
  housekeeping (#1622)
  Remove intercom (#1547)
  Reduced bundlesize by requiring only victory-core for VictoryAnimation component. (#1618)
  A variety of improvements all around (#1608)
  Update client-participation dependencies, and get off gulp -> webpack (#1607)
  Feature/crkrenn/fixed participants extended sql table (#1586)
  Switch server to debian (#1600)
  More slack removal (#1599)
  Update server node & misc dep versions (#1595)
  switch from npm ci to npm install in client dockerfiles (#1594)
  deleted docker_cloud_notes (#1597)
  Remove slack (#1585)
  specify bullseye debian base for math dockerfile (#1590)
  Feature/crkrenn/improved makefile (#1577)
  Adding direction to the client- participation (#1561)
  update winston package (#1584)
  add scaling roadmap doc (#1582)
  Feature/sirodoht crk/e2e version 11 (#1576)
  German language editing (#1552)
  ...
ballPointPenguin pushed a commit that referenced this pull request Mar 6, 2023
* edge: (22 commits)
  Configuration unification (#1617)
  housekeeping (#1622)
  Remove intercom (#1547)
  Reduced bundlesize by requiring only victory-core for VictoryAnimation component. (#1618)
  A variety of improvements all around (#1608)
  Update client-participation dependencies, and get off gulp -> webpack (#1607)
  Feature/crkrenn/fixed participants extended sql table (#1586)
  Switch server to debian (#1600)
  More slack removal (#1599)
  Update server node & misc dep versions (#1595)
  switch from npm ci to npm install in client dockerfiles (#1594)
  deleted docker_cloud_notes (#1597)
  Remove slack (#1585)
  specify bullseye debian base for math dockerfile (#1590)
  Feature/crkrenn/improved makefile (#1577)
  Adding direction to the client- participation (#1561)
  update winston package (#1584)
  add scaling roadmap doc (#1582)
  Feature/sirodoht crk/e2e version 11 (#1576)
  German language editing (#1552)
  ...
brendanarnold pushed a commit to DFE-Digital/polis-whitelabel that referenced this pull request Apr 3, 2023
* Adding direction to the participation

* Set client-participation text directions to auto and match css styles to flexibly adapt

* Adjust css styles in css files, modules and admin section

* Fixing the issue in statement margins caused by paragraph direction overriding page direction

* switch from dir to direction for rtl vs ltr specificaiton in strings files

* add direction to Farsi and Hebrew strings files

* Fixing remaining comments floating issue on chrome

Co-authored-by: Christopher Small <metasoarous@gmail.com>
brendanarnold added a commit to DFE-Digital/polis-whitelabel that referenced this pull request Apr 4, 2023
* Adding farsi translation (compdemocracy#1560)

* Adding farsi translation

* Update client-participation/js/strings.js

Co-authored-by: Patrick Connolly <patrick.c.connolly@gmail.com>

Co-authored-by: Colin Megill <colinmegill@gmail.com>
Co-authored-by: Patrick Connolly <patrick.c.connolly@gmail.com>

* Adding direction to the client- participation (compdemocracy#1561)

* Adding direction to the participation

* Set client-participation text directions to auto and match css styles to flexibly adapt

* Adjust css styles in css files, modules and admin section

* Fixing the issue in statement margins caused by paragraph direction overriding page direction

* switch from dir to direction for rtl vs ltr specificaiton in strings files

* add direction to Farsi and Hebrew strings files

* Fixing remaining comments floating issue on chrome

Co-authored-by: Christopher Small <metasoarous@gmail.com>

* German language editing (compdemocracy#1552)

* Bump node from 11.15.0-alpine to 18.9.0-alpine in /client-admin (compdemocracy#1523)

Bumps node from 11.15.0-alpine to 18.9.0-alpine.

---
updated-dependencies:
- dependency-name: node
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump node from 11.15.0-alpine to 18.9.0-alpine in /client-report (compdemocracy#1525)

Bumps node from 11.15.0-alpine to 18.9.0-alpine.

---
updated-dependencies:
- dependency-name: node
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump moment from 2.29.1 to 2.29.4 in /client-report (compdemocracy#1514)

Bumps [moment](https://github.com/moment/moment) from 2.29.1 to 2.29.4.
- [Release notes](https://github.com/moment/moment/releases)
- [Changelog](https://github.com/moment/moment/blob/develop/CHANGELOG.md)
- [Commits](moment/moment@2.29.1...2.29.4)

---
updated-dependencies:
- dependency-name: moment
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump ws from 7.4.2 to 7.5.7 in /client-admin (compdemocracy#1395)

Bumps [ws](https://github.com/websockets/ws) from 7.4.2 to 7.5.7.
- [Release notes](https://github.com/websockets/ws/releases)
- [Commits](websockets/ws@7.4.2...7.5.7)

---
updated-dependencies:
- dependency-name: ws
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump got and nodemon in /server (compdemocracy#1539)

Removes [got](https://github.com/sindresorhus/got). It's no longer used after updating ancestor dependency [nodemon](https://github.com/remy/nodemon). These dependencies need to be updated together.

Removes `got`

Updates `nodemon` from 2.0.7 to 2.0.20
- [Release notes](https://github.com/remy/nodemon/releases)
- [Commits](remy/nodemon@v2.0.7...v2.0.20)

---
updated-dependencies:
- dependency-name: got
  dependency-type: indirect
- dependency-name: nodemon
  dependency-type: direct:development
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump y18n from 3.2.1 to 3.2.2 in /client-report (compdemocracy#932)

Bumps [y18n](https://github.com/yargs/y18n) from 3.2.1 to 3.2.2.
- [Release notes](https://github.com/yargs/y18n/releases)
- [Changelog](https://github.com/yargs/y18n/blob/master/CHANGELOG.md)
- [Commits](https://github.com/yargs/y18n/commits)

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump minimist from 1.2.5 to 1.2.6 in /client-admin (compdemocracy#1384)

Bumps [minimist](https://github.com/substack/minimist) from 1.2.5 to 1.2.6.
- [Release notes](https://github.com/substack/minimist/releases)
- [Commits](https://github.com/substack/minimist/compare/1.2.5...1.2.6)

---
updated-dependencies:
- dependency-name: minimist
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Update de_de.js

Checked german language (native speaker). Corrected some inconsistencies and grammar errors.

* Update de_de.js

* Update de_de.js

* Revert "Bump node from 11.15.0-alpine to 18.9.0-alpine in /client-admin (compdemocracy#1523)"

This reverts commit 0d1242f.

* Revert "Bump node from 11.15.0-alpine to 18.9.0-alpine in /client-report (compdemocracy#1525)"

This reverts commit 54ed279.

* Revert "Bump moment from 2.29.1 to 2.29.4 in /client-report (compdemocracy#1514)"

This reverts commit 4e7dd51.

* Revert "Bump ws from 7.4.2 to 7.5.7 in /client-admin (compdemocracy#1395)"

This reverts commit 0224f8e.

* Revert "Bump got and nodemon in /server (compdemocracy#1539)"

This reverts commit 0c18e1e.

* Revert "Bump minimist from 1.2.5 to 1.2.6 in /client-admin (compdemocracy#1384)"

This reverts commit 0e8263b.

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Christopher Small <metasoarous@gmail.com>

* Update CHANGELOG

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Hadjar Homaei <hadjar@gmail.com>
Co-authored-by: Colin Megill <colinmegill@gmail.com>
Co-authored-by: Patrick Connolly <patrick.c.connolly@gmail.com>
Co-authored-by: Christopher Small <metasoarous@gmail.com>
Co-authored-by: Brendan Arnold <brendan.arnold@policylab.gov.uk>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
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