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.

Sign up for GitHub

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

Jump to bottom

Add Hebrew locale #67

Merged
merged 8 commits into from
Feb 15, 2022
Merged

Add Hebrew locale #67

merged 8 commits into from
Feb 15, 2022

Conversation

pantchox
Copy link

  • upadte languages file to include hebrew
  • update i18n file to include hebrew language support and locale

- add hebrew language file
ebd21fa 
- upadte languages file to include hebrew
- update i18n file to include hebrew language support and locale
@netlify
Copy link

netlify bot commented Feb 10, 2022 •
edited
Loading

✔️ Deploy Preview for frosty-lovelace-4cf67f ready!

🔨 Explore the source changes: 75e2945

🔍 Inspect the deploy log: https://app.netlify.com/sites/frosty-lovelace-4cf67f/deploys/620b6456eed3f0000701134b

😎 Browse the preview: https://deploy-preview-67--frosty-lovelace-4cf67f.netlify.app/

tomayac
tomayac reviewed Feb 10, 2022
View reviewed changes
src/js/i18n.js Outdated Show resolved Hide resolved
@tomayac
Copy link
Owner

tomayac commented Feb 10, 2022

Just adding <html dir="rtl"> goes a long way already. I need to fix some stuff as you see. But it's not as bad as I thought it would look. Thank you, CSS and HTML I guess.

Screen Shot 2022-02-10 at 10 36 52

@pantchox
Copy link
Author

Just adding <html dir="rtl"> goes a long way already. I need to fix some stuff as you see. But it's not as bad as I thought it would look. Thank you, CSS and HTML I guess.

Screen Shot 2022-02-10 at 10 36 52

From the screen shot using direction RTL actually messes up the order of the words in strings with mixed languages.
For example Copy SVG becomes SVG copy.
The problem is that if you mix Hebrew words with English in a string, using LTR language settings/encoding actually shows it in the correct order, when switching to real RTL it messes up the order.

There are few options:

  • Don't include the direction attribute and leave it as is - I think it is fine but it won't be a "true" RTL Layout
  • Include the direction attribute and I will modify the translation in the mixed language strings to be in a different order so when using the direction attribute it will be shown properly.
  • Doing some extra research to check if there is a better way (I am certain there might be) - Which I don't mind doing, depending your choice.

@tomayac
Copy link
Owner

tomayac commented Feb 10, 2022

  • Doing some extra research to check if there is a better way (I am certain there might be) - Which I don't mind doing, depending your choice.

Please leave the PR as is. I want to actually do this research myself and learn on my way. I was actually looking for an opportunity to properly learn RTL for a while now. I'd appreciate if, once I have tried my luck with fixes, you could review and see if it "feels" correct.

@pantchox
Copy link
Author

Great! :)

@tomayac
Copy link
Owner

tomayac commented Feb 12, 2022

@pantchox Please take a look at the staging preview. To my eye it looks and feels pretty good. What do you say?

@tomayac
Copy link
Owner

tomayac commented Feb 12, 2022

@pantchox (Be sure to clear storage before, so you don't look at an old cached version. It should look as below.)

localhost_3000_ (1)

@tomayac
Copy link
Owner

tomayac commented Feb 14, 2022

@pantchox Friendly ping regarding a quick check if this PR is good to merge. Staging preview.

@tomayac tomayac changed the title add hebrew language file Add Hebrew locale Feb 14, 2022
@tomayac tomayac mentioned this pull request Feb 14, 2022
Closed
@pantchox
Copy link
Author

Hi, sorry for the late reply,
The top toolbar looks fine, the sidebar translations that includes "SVG" should change order.
Adding a screen shot.

The reason for different word ordering is because in Hebrew adjective comes after a noun.
In the case of "SVG Options" since both nouns in free translation the ordering is "Options (of) SVG".
Hope that makes sense!

image

@tomayac tomayac merged commit bc410f2 into tomayac:main Feb 15, 2022
@pantchox
Copy link
Author

Looking good! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tomayac tomayac tomayac left review comments

Assignees

@pantchox pantchox

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@pantchox @tomayac