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

Fix Arabic Selection Option #1236

Open
wants to merge 57 commits into
base: main
Choose a base branch
from
Open

Fix Arabic Selection Option #1236

wants to merge 57 commits into from

Conversation

BasselArt
Copy link

Issue:

@@ -4,8 +4,10 @@
"repository": "https://github.com/getfider/fider",
"private": true,
"dependencies": {
"@lingui/core": "^3.13.3",
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you need to change these dependencies here?

Copy link
Author

Choose a reason for hiding this comment

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

I'm not developer, so I used Cursor to fix Arabic issue

@@ -1,4 +1,4 @@
$font-base: 'Inter', ui-sans-serif, system-ui, -apple-system, BlinkMacSystemFont, "Segoe UI", "Helvetica Neue", Arial, "Noto Sans", sans-serif,
$font-base: 'IBM Plex Sans Arabic', ui-sans-serif, system-ui, -apple-system, BlinkMacSystemFont, "Segoe UI", "Helvetica Neue", Arial, "Noto Sans", sans-serif,
Copy link
Contributor

Choose a reason for hiding this comment

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

We can't change the font, this change would make that font the default for all. We need to work out a solution that will choose Inter for all non-arabic systems, and use your font for all users who those the arabic language?

Copy link
Author

Choose a reason for hiding this comment

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

Agree with you, maybe some condition

@@ -3,6 +3,7 @@
"action.change": "تغيير",
"action.close": "إغلاق",
"action.confirm": "تأكيد",
"action.copylink": "نسخ الرابط",
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, we don't manage changes to the localised strings here.

Please can you make the changes to the arabic strings here: https://crowdin.com/project/fider

Then they can be pulled into the next release

Thanks

Copy link
Author

Choose a reason for hiding this comment

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

Sure I will upload an updated version, but I still have some modification on it along with some missing strings

@@ -0,0 +1,142 @@
package letteravatar
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks interesting. So you use the font to generate the letter avatar, so that it supports arabic letters. That's cool - if we used this we'd just need to make sure we can support both arabic and non-arabic. Do you have any thoughts about how best to manage that?

Copy link
Author

Choose a reason for hiding this comment

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

I give up and convert the Arabic letters to English,Ex: ب = B , from my very basic experience with php and converting text to image, Arabic language is very challenging

Copy link
Contributor

@mattwoberts mattwoberts left a comment

Choose a reason for hiding this comment

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

Hi @BasselArt

Thanks for the PR and the work you're doing to make Fider work in arabic.

I'm not sure however what i can do with this. From a quick scan through the code I see that many of the changes you have added break the existing app for other languages in order to make it work for Arabic. Obviously the only way that we could consider getting these pulled in would be if the changes were fully compatible with all languages.

Thanks

Matt

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.

2 participants