search

lingua-libre / SignIt

🌻 Lingua Libre SignIt web-browser extension translates selected word in French Sign Language via an elegant pop up so you learn sign language while reading online.
https://addons.mozilla.org/en-US/firefox/addon/lingua-libre-signit/
MIT License
15 stars 15 forks source link

Harmonize i18n system #84

Closed hugolpz closed 5 months ago

hugolpz commented 5 months ago

Reminder : The user can submit a iso code such as hi using the Popup > Settings > UI languages' drop down select button.

Safari

Safari lags behind with few less native i18n API functions working and only 4% Desktop marketshare. We can ignore it. image

kabir-afk commented 5 months ago

That is true in case of SignItCoreContent and SignItVideosGallery , but in case of popup, it is kind of not true. Since, when we fetch background page and then banana for FF , it successfully fetches the banana and it's functionality as well , while in chrome it doesn't. Due to which we have to mimic it manually. Do I centralize the i18n as per chrome and then change it in all the files OR change it in 2 files and let popup remain the same ?

Btw , it also made me wonder that earlier the reason you were not able to fetch banana in other content scripts was maybe because you had to fetch background page first by running await browser.runtime.getBackgroundPage, and then fetch banana by using _backgroundPage.banana. Just maybe , not completely sure.

hugolpz commented 5 months ago

We should keep things simple and with one single system for i18n.

Having two i18n systems working in parallel, then we add complexity/confusion. In the context of open sources where human ressources turn over is high and onboarding is critical, we need to code build low complexity, preparing easy dive into the code and straightforward maintenance.

This add to the argument to eventually migrate to browser.i18n . 🤔 (We discuss it Monday)

kabir-afk commented 5 months ago

I can go with signIti18n function but it makes the popup UI buggy in FF as well. Using local storage shouldn't cause this issue but it is what is is. Like you said it adds to the argument to eventually migrate to browser.i18n

hugolpz commented 5 months ago

@kabir-afk hello, This ticket's description have been fully rewritten. Discussion en between are therefore obsolete. This issue may cause side effects and requires new solutions and revamps, you have enough time to consider it Tuesday + Wednesday.

kabir-afk commented 5 months ago

Resolved in above PR !! 🎉