-
-
Notifications
You must be signed in to change notification settings - Fork 2
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
Added Unicode autocorrect and font conversion #2
Conversation
Yeah, I'd also consider this a feature for a later version. I also need to think about how to best implement this. Maybe let the user select the language? Or somehow use the browser language? Or can we detect which spellchecking language the user has choosen for an input field? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regarding data-i18n=""
: Ah the localisation is still missing, yeah? Is this intentional?
As far as I can see, this PR generally looks good. My remarks are mostly only small code style things.
I'll see to try that out in practice later.
And again, thanks for implementing this! 😃 IMHO this will be a very handy add-on and certainly useful. 😊
Co-authored-by: rugk <rugk+git@posteo.de>
Yes and yes. I just left that there as I assumed you would want to localize it after you decided on the wording.
I had an issue with the submodules not automatically downloading when I cloned the repository. I actually had to manually download each one: git submodule add https://github.com/TinyWebEx/Localizer/ "src/common/modules/Localizer"
git submodule add https://github.com/TinyWebEx/TestHelper "src/tests/helper"
git submodule add https://github.com/TinyWebEx/AddonSettings "src/common/modules/AddonSettings"
git submodule add https://github.com/TinyWebEx/Logger "src/common/modules/Logger"
git submodule add https://github.com/TinyWebEx/MessageHandler "src/common/modules/MessageHandler"
git submodule add https://github.com/TinyWebEx/BrowserCommunication "src/common/modules/BrowserCommunication"
git submodule add https://github.com/TinyWebEx/RandomTips "src/common/modules/RandomTips"
git submodule add https://github.com/TinyWebEx/AutomaticSettings "src/common/modules/AutomaticSettings" Hopefully this will not happen to you. Also note that the autocorrect feature will not work until you change that line in your BrowserCommunication library.
No problem! I am glad you find it useful. Please let me know if you need me to do anything else. I believe I made all of your requested changes in 7208857. I also backported the applicable changes to rugk/awesome-emoji-picker#93 in rugk/awesome-emoji-picker@8ff9ee3, since there is obviously a lot of code duplication between the two PRs and many of changes applied to both. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah great, only minor changes.
Will do, yes thanks. 😊 And thanks for the backports BTW. 😊 |
Co-authored-by: rugk <rugk+git@posteo.de>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small things only 🎉
Note that if you think it would be useful to make a common JS module (even if it is just for a content script), then we can do so. I'm always in favor of avoiding code duplication. |
Made a PR for Chrome/ium compatibility you've mentioned. Is TinyWebEx/BrowserCommunication#4 enough? |
Co-authored-by: rugk <rugk+git@posteo.de>
Yes, I tested that and it works great! I believe I made all of your requested changes in 12c24cf. I also backported the applicable changes to rugk/awesome-emoji-picker#93 in rugk/awesome-emoji-picker@4f11cf0. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apart from these few comments, ready to merge, IMHO. I may still test the changes in practice through…
Testing some things currently. First question: Sorry, but still don't get the difference between 1/4 → ¼ and the ”fraction” replacement, i.e. the first and the third checkbox in the settings. Is not ¼ also a fraction? Also pushed some changes in your branch. Will do more later. |
No problem. The first checkbox replaces |
Yeah, I had in mind I already asked this, but was confused again. 🙈 |
Note the "it should work without a browser restart" comment is still open. 😃 |
Resolved with 0d91cfb. 🙂 |
…ub.com/rugk/unicodify into Unicode-correction-and-font-conversion
thunderbirdmanifest.json
file tomanifest.json
and moving it tosrc/
.src/
and renaming thechromemanifest.json
file tomanifest.json
and moving it tosrc/
.This line in the BrowserCommunication library will need to be update to justFixed in Make BrowserCommunication Chrome/ium-compatible TinyWebEx/BrowserCommunication#4.return true;
, otherwise the extension will be unable to send responses to messages when using the webextension-polyfill and the autocorrect feature will not work.Note that for the "Use Unicode smart quotes" option, you said you wanted it use the quotes for each language/locale, although I am unsure how you want to implement this, so it still only uses the English smart quotes.
In addition, for rugk/awesome-emoji-picker#106 you said you wanted to include advertisements between both add-ons on the settings page and as a tip. I included a note about the Awesome Emoji Picker on the settings page, although I am unsure how you want to do the tip since it does not have a browser action popup.
Many of my comments from rugk/awesome-emoji-picker#93 also apply to this PR. The autocorrect feature supports Firefox for Android, but the font conversion does not since the Context Menu API is not yet supported (bug 1595822).
The user can press backspace (⌫) to undo any autocorrections. Please let me know if you would like me to make any changes.