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
11 stars 13 forks source link

Improve icons support #82

Closed hugolpz closed 3 weeks ago

hugolpz commented 3 weeks ago

Problem

I researched upon it , and found out that popup scripts has more access to extension's directories/files... whereas content scripts are directly injected into the DOM and hence have less access. I'll raise a PR for the same for a potential fix , kindly review and test it when possible. -- @kabir-afk

Distinct references

[We may] use Wikimedia icons in modal(content-scripts) and referenced icons in popup. Simply because it violates a part of CSP when using API icons in popup and doesn't in modal. Although it would feel inconsistent.

Centralized references

[We may] use the same icons (referenced onces ) in both popup and modal ,but it requires additional css through jQuery in SignItVideosGallery.js , and unlike OOjs-UI library buttons , the opacity won't be controlled automatically of those buttons.

Unicode characters

We may use unicode characters 🢐 🢒 🢑 🢓 , 🢔 🢖 🢕 🢗 , ⌃ ⌄ , ⮹

kabir-afk commented 3 weeks ago

Distinct References

There is one thing I forgot to mention. The wikimedia API icons only worked on Wikipedia and similar sites(other wikimedia tools most likely) which also relied on these icons. Since they were a part of website/DOM(Website Devtools-> Applications-> CSS), they were being rendered by default. This didn't happen in other sites. So I guess this shouldn't even have been the choice to begin with.

Centralized References

While they can certainly work, they need additional code for change in opacity when surfing through the carousel of videos.

Unicode References

This was a new approach to me and it is something that works across both content and popup scripts, hence consistent. It also doesn't need any opacity change as it adapts to the OOjs UI library's CSS, unlike the icons in centralized references. I'm going forward with this approach. Kindly review and test the PR before merging it.

hugolpz commented 3 weeks ago

Check https://doc.wikimedia.org/codex/latest/icons/all-icons.html > search : cdxIconPrevious and cdxIconNext > Observe svg.

kabir-afk commented 3 weeks ago

Do we need to change icons again ? If yes , then correct me if I am wrong.

Can't think of any other alternative as of now.

hugolpz commented 3 weeks ago

@kabir-afk hello, I gather all resources on this topic here. It's food for though and discussion. Still, current unicode approach you implemented is fine as of today.