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

client: add Arabic translations #1898

Merged
merged 5 commits into from Dec 19, 2022
Merged

client: add Arabic translations #1898

merged 5 commits into from Dec 19, 2022

Conversation

chappjc
Copy link
Member

@chappjc chappjc commented Oct 11, 2022

Replaces #1844 with some tweaks

@chappjc chappjc mentioned this pull request Oct 11, 2022
@xaur
Copy link
Contributor

xaur commented Oct 11, 2022

I was wondering if qualifying the language variant is required by this translation system so it would be ar-xx with xx being something from this list. Or just ar is fine for now.

@chappjc
Copy link
Member Author

chappjc commented Oct 11, 2022

I was wondering if qualifying the language variant is required by this translation system so it would be ar-xx with xx being something from this list. Or just ar is fine for now.

Had a similar thought, but just a language with no region is a valid BCP 47 code. The matching and tag parsing code in golang.org/x/text/language accepts "ar" as expected, and fmt.Println(language.Arabic) says "ar", so that's really what matters I think.

@chappjc
Copy link
Member Author

chappjc commented Oct 11, 2022

./dexc --simnet --lang ar:

image

image

image
^ Not sure what those integers are in the details. Looking into it.

image
^ just a few strings missed (master is a moving target), which the build tool does report.

image
^ simnet!

image

image

image

@chappjc
Copy link
Member Author

chappjc commented Oct 11, 2022

client/webserver/locales/ar.go

forms.tmpl
Warning: no ar replacement text for key "disable_wallet", using 'en' value Disable Wallet 
Warning: no ar replacement text for key "enable_wallet", using 'en' value Enable Wallet 
Warning: no ar replacement text for key "disable_wallet_warning", using 'en' value Note: This wallet will not be connected to when you start the DEX client software and cannot be used until it is enabled. 
Warning: no ar replacement text for key "enable_wallet_message", using 'en' value This wallet will resume operation and might take some time to sync. 

markets.tmpl
Warning: no de-DE replacement text for key "add_a_wallet", using 'en' value Add a <span data-tmpl="addWalletSymbol"></span> wallet 
Warning: no ar replacement text for key "add_a_wallet", using 'en' value Add a <span data-tmpl="addWalletSymbol"></span> wallet 
Warning: no ar replacement text for key "Recent Matches", using 'en' value Recent Matches 

order.tmpl
Warning: no ar replacement text for key "Maker Swap", using 'en' value Maker Swap 
Warning: no ar replacement text for key "Pending", using 'en' value Pending 
Warning: no ar replacement text for key "Taker Swap", using 'en' value Taker Swap 
Warning: no ar replacement text for key "Maker Redemption", using 'en' value Maker Redemption 
Warning: no ar replacement text for key "Taker Redemption", using 'en' value Taker Redemption 

wallets.tmpl
Warning: no ar replacement text for key "Disabled", using 'en' value Disabled 
Warning: no ar replacement text for key "disable_wallet", using 'en' value Disable Wallet 
Warning: no ar replacement text for key "enable_wallet", using 'en' value Enable Wallet

@chappjc
Copy link
Member Author

chappjc commented Oct 11, 2022

client/core/locale_ntfn.go

go run ./client/core/localetest/main.go
3 missing notification translations for de-DE
[de-DE] Translation missing for topic RedemptionResubmitted
[de-DE] Translation missing for topic RedemptionConfirmed
[de-DE] Translation missing for topic SwapRefunded
7 missing notification translations for pt-BR
[pt-BR] Translation missing for topic RedemptionResubmitted
[pt-BR] Translation missing for topic WalletCommsWarning
[pt-BR] Translation missing for topic WalletPeersWarning
[pt-BR] Translation missing for topic RedemptionConfirmed
[pt-BR] Translation missing for topic WalletPeersRestored
[pt-BR] Translation missing for topic QueuedCreationFailed
[pt-BR] Translation missing for topic SwapRefunded
7 missing notification translations for zh-CN
[zh-CN] Translation missing for topic SwapRefunded
[zh-CN] Translation missing for topic RedemptionResubmitted
[zh-CN] Translation missing for topic WalletPeersWarning
[zh-CN] Translation missing for topic WalletCommsWarning
[zh-CN] Translation missing for topic RedemptionConfirmed
[zh-CN] Translation missing for topic WalletPeersRestored
[zh-CN] Translation missing for topic QueuedCreationFailed
7 missing notification translations for pl-PL
[pl-PL] Translation missing for topic WalletCommsWarning
[pl-PL] Translation missing for topic WalletPeersWarning
[pl-PL] Translation missing for topic RedemptionConfirmed
[pl-PL] Translation missing for topic WalletPeersRestored
[pl-PL] Translation missing for topic QueuedCreationFailed
[pl-PL] Translation missing for topic SwapRefunded
[pl-PL] Translation missing for topic RedemptionResubmitted
2 stale notification translations for pt-BR
[pt-BR] Translation stale for topic SendSuccess
[pt-BR] Translation stale for topic SendError
2 stale notification translations for zh-CN
[zh-CN] Translation stale for topic SendSuccess
[zh-CN] Translation stale for topic SendError
2 stale notification translations for pl-PL
[pl-PL] Translation stale for topic SendSuccess
[pl-PL] Translation stale for topic SendError
6 stale notification translations for de-DE
[de-DE] Translation stale for topic BuyMatchesMade
[de-DE] Translation stale for topic BuyOrderCanceled
[de-DE] Translation stale for topic BuyOrderPlaced
[de-DE] Translation stale for topic SellOrderPlaced
[de-DE] Translation stale for topic SellMatchesMade
[de-DE] Translation stale for topic SellOrderCanceled

(nothing missing for ar!)

@chappjc chappjc added the i18n Internationalization and translation label Oct 12, 2022
@xaur
Copy link
Contributor

xaur commented Oct 13, 2022

Arabic is right-to-left and that brings a few new challenges not seen before:

  • The easiest I think is paragraph alignment - paragraph lines need to be right-aligned. They look left-aligned in notifications and settings images above. In tables, text columns with ar should be right-aligned too, like on this image.

  • A mix of LTR and RTL text in same sentence ("bidi" for bidirectional text). On a high structural level, paragraphs and sentences should flow right to left, but on a lower level, non-ar elements embedded in sentences (English words, numbers) should flow left to right. Maybe DEX UI doesn't have a lot of these issues, but we've encountered a ton of software that cannot get it right (including GitHub, Atom, VSCode, etc.) so it's something to keep an eye on.

  • The hardest I think is when the language affects widget layout. If there is a checkbox or a radio button with a label in Arabic, should the checkbox go to the right? I think looks more natural for RTL readers, but I don't know if the world of Arabic software has any guidelines like that.

@chappjc
Copy link
Member Author

chappjc commented Oct 13, 2022

Arabic is right-to-left and that brings a few new challenges not seen before:

The easiest I think is paragraph alignment - paragraph lines need to be right-aligned. They look left-aligned in notifications and settings images above. In tables, text columns with ar should be right-aligned too, like on this image.

Realizing this too. To do this properly in html, we'd need to include dir="rtl" or dir="ltr" attributes in various elements. We're not equipped to do that programmatically at present, but I think that's what we will need to do, eventually. e.g. setting "ltr" on a token: 3dd6b5b And like you said, we'd also need to specify appropriate right justification in places.

A mix of LTR and RTL text in same sentence ("bidi" for bidirectional text). On a high structural level, paragraphs and sentences should flow right to left, but on a lower level, non-ar elements embedded in sentences (English words, numbers) should flow left to right. Maybe DEX UI doesn't have a lot of these issues, but we've encountered a ton of software that cannot get it right (including GitHub, Atom, VSCode, etc.) so it's something to keep an eye on.

This one became very clear yesterday. We do have a lot of mixed directionality sentences/phrases, and UTF-8 rendering of these can scramble up the non-ar elements, such as tokens or hexadecimal hashes. To address that I've had to introduce control code sequences just before the placeholders for the non-ar elements to switch from the RTL formatting of preceding Arabic text to LTR formatting required for the tokens and other inserted values. That's what is going on in db0f1c7 with all those \u200e inserted

The hardest I think is when the language affects widget layout. If there is a checkbox or a radio button with a label in Arabic, should the checkbox go to the right? I think looks more natural for RTL readers, but I don't know if the world of Arabic software has any guidelines like that.

Yes, this is trouble. Lots of things fall apart in our layout, and it's not easy to address.

@chappjc
Copy link
Member Author

chappjc commented Dec 12, 2022

The most egregious display bugs relating to bidirectional text that were visible in #1898 (comment) were resolved. I don't know what else we can do with this PR at present.
I think it's better than no AR translation, even if it would hurt the viewer's eyes for the reasons described in the last few comments.
Unless there are other smaller suggestions, or just corrections on the translations, we'll probably merge this this week..

@xaur
Copy link
Contributor

xaur commented Dec 12, 2022

Agreed. If it helps Arabic users to navigate and understand what is going on (more than the English version), it is an improvement. Smaller issues can be addressed in the future.

Copy link
Member

@buck54321 buck54321 left a comment

Choose a reason for hiding this comment

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

Looks good tome.

client/webserver/site/.eslintrc.js Show resolved Hide resolved
@chappjc chappjc merged commit f637495 into decred:master Dec 19, 2022
@chappjc chappjc deleted the ar branch December 19, 2022 21:54
@chappjc chappjc added this to the 0.6 milestone Dec 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
i18n Internationalization and translation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants