Scribe-Android
Scribe-Android copied to clipboard
scribe-org
Add Emoji Autosuggestion Buttons
Contributor checklist
- [x] This pull request is on a separate branch and not the main branch
Description
This PR adds buttons for emoji suggestions for mobile view and the logic to enable these buttons; the tablet view has not been added yet.
The current code implementation has some issues. I've added log statements as follows:
2024-09-26 20:59:18.321 3031-3031 EmojiButtonController be.scri.debug D pluralBtn initialized: null
2024-09-26 20:59:18.322 3031-3031 EmojiButtonController be.scri.debug D pluralBtn visibility: null
However, this code does not work properly. It seems that EmojiButtonController.kt has not successfully obtained the button elements from keyboard_view_command_options.xml and made modifications. I've tried many methods but to no avail. Can you help me with this?
Related issue
- #138
Thank you for the pull request!
The Scribe team will do our best to address your contribution as soon as we can. The following is a checklist for maintainers to make sure this process goes as well as possible. Feel free to address the points below yourself in further commits if you realize that actions are needed :)
If you're not already a member of our public Matrix community, please consider joining! We'd suggest using Element as your Matrix client, and definitely join the General and Android rooms once you're in. Also consider joining our bi-weekly Saturday dev syncs. It'd be great to have you!
Maintainer checklist
-
[x] The commit messages for the remote branch should be checked to make sure the contributor's email is set up correctly so that they receive credit for their contribution
- The contributor's name and icon in remote commits should be the same as what appears in the PR
- If there's a mismatch, the contributor needs to make sure that the email they use for GitHub matches what they have for
git config user.emailin their local Scribe-Android repo
-
[ ] The linting and formatting workflows within the PR checks do not indicate new errors in the files changed
-
[x] The CHANGELOG has been updated with a description of the changes for the upcoming release and the corresponding issue (if necessary)
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
Sorry for the delay in my response. I had some college work yesterday. So you have two choices to prevent those from going null. You could initialize the controller in the individual keyboard IME class and accordingly use them. The binding is inflated only in the individual keyboard IME. Alternatively, you could move the controller code to the simple keyboard IME abstract class that consists of the keyboard's current functions and then call them from the individual keyboard IME.
Sorry for the delay in my response. I had some college work yesterday. So you have two choices to prevent those from going null. You could initialize the controller in the individual keyboard IME class and accordingly use them. The binding is inflated only in the individual keyboard IME. Alternatively, you could move the controller code to the simple keyboard IME abstract class that consists of the keyboard's current functions and then call them from the individual keyboard IME.
The binding has to be passed as a parameter to the functions if the second thought is considered. I think it is the same for the first way but not so sure.
Within 24 hours isn't a delayed response, @angrezichatterbox :) School's important and certainly the priority 😊
Thanks for the suggestions!
Hey @Linfye 👋 Checking in to see what your thoughts are on @angrezichatterbox's feedback :) Let's us know if you need any support here!
Hope you're well 😊
I apologize for not informing you earlier. I had to return to my hometown recently to deal with some urgent matters, and I didn’t have the time or proper environment to work, which is why I haven’t been able to respond. I’ll be returning to school tomorrow and should be able to resume work within the next couple of days.
I'm very sorry for making you wait so long.
No need to apologize whatsoever, @Linfye! The assumption on my part was that something was up, so wanted to check in. Hope that you're doing ok, and take your time jumping back in here :)
I put the controller code in the SimpleKeyboardIME. But it seemed that initializeEmojiButtons was never called while running the App according to logcat. Can you help me with this? @angrezichatterbox Thanks!
I put the controller code in the SimpleKeyboardIME. But it seemed that
initializeEmojiButtonswas never called while running the App according to logcat. Can you help me with this? @angrezichatterbox Thanks!
I will look into it and get back to you :)
I put the controller code in the SimpleKeyboardIME. But it seemed that
initializeEmojiButtonswas never called while running the App according to logcat. Can you help me with this? @angrezichatterbox Thanks!
The Simple Keyboard IME is an abstract class that consists of any functions that are being used by all the languages. The individual languages have their codebase for the setup of the keyboard. You could call the function from the individual keyboard IME. Also, could you use binding instead of calling using findViewById? Thanks :)
Overall everything looks nice @Linfye :) These changes would make it functional hopefully.
You could initialize the function in onCreateInputView and onStartupInputView for the individual language IME.
@angrezichatterbox Now the initializeEmojiButtons and updateButtonVisibility can be called in the app. But updateButtonVisibility returned null buttons Can you give me some instructions? Thanks a lot!
And in my development branch, whenever I press the backspace key, the built app crashes. Do you have this issue? If so, we can file a bug report.
cc: @andrewtavis
@angrezichatterbox Now the
initializeEmojiButtonsandupdateButtonVisibilitycan be called in the app. ButupdateButtonVisibilityreturned null buttons Can you give me some instructions? Thanks a lot!And in my development branch, whenever I press the backspace key, the built app crashes. Do you have this issue? If so, we can file a bug report.
cc: @andrewtavis
I will reply for the first thing later on as I'm travelling. As for the back space crash. Could u try pull the new main and rebasing your branch with new main. Or merge main into your branch. It was fixed recently.
I've changed to another way of passing parameters, allowing EnglishKeyboardIME to access the status of the Switch in LanguageSettingsFragment, which can solve the previous issue.
However, as shown in #186, this problem also exists in the latest main branch. So to test the current feature, you should return to the phone's Home screen without going back to the Settings page after modifying the Switch, and then open the keyboard to test. I don't know the cause of #186; the Autosuggest Emojis Switch was also added by me referring to the Double Space Peroids code, but this problem occurred.
I've rebased the code from the main branch, but the Backspace crash issue that I mentioned earlier still exists. Have you tested it on your machine? If you have, it might just be my problem, and I will try to resolve it later.
The current code implements two Emoji versions for the phone view. If this works fine for you, I would like to merge the current PR first. For the three Emoji versions of the tablet view, I think a new issue can be created for other contributors to refer to and work on for hacktoberfest. Of course, if no one takes it on, I can handle it.
Thank you for your help on this issue. 😊 @angrezichatterbox @andrewtavis
I've changed to another way of passing parameters, allowing
EnglishKeyboardIMEto access the status of the Switch inLanguageSettingsFragment, which can solve the previous issue.However, as shown in #186, this problem also exists in the latest main branch. So to test the current feature, you should return to the phone's Home screen without going back to the Settings page after modifying the Switch, and then open the keyboard to test. I don't know the cause of #186; the Autosuggest Emojis Switch was also added by me referring to the Double Space Peroids code, but this problem occurred.
I've rebased the code from the main branch, but the Backspace crash issue that I mentioned earlier still exists. Have you tested it on your machine? If you have, it might just be my problem, and I will try to resolve it later.
The current code implements two Emoji versions for the phone view. If this works fine for you, I would like to merge the current PR first. For the three Emoji versions of the tablet view, I think a new issue can be created for other contributors to refer to and work on for hacktoberfest. Of course, if no one takes it on, I can handle it.
Thank you for your help on this issue. 😊 @angrezichatterbox @andrewtavis
For #186 the value gets changed however there is a typo in the value that is checked is retrieving isChecked = sharedPref.getBoolean("autosuggest_emojis_$language", true), Could you change this to emoji_suggestions_$language. Could u add the corrected naming for it as well the functionality for auto suggest emojis in all the languages IME. Thanks :) . I will look into the backspace crash.
Is the app crashing on everytime the back space is used ?
Yes, it crashed every time. This issue happened after this.
I don't know the exact commit.
https://github.com/scribe-org/Scribe-Android/blob/6758b8faa659047028927e72f8a17b0175c3b0a2/app/src/main/java/be/scri/services/EnglishKeyboardIME.kt#L193-L214
Could you replace for every time the Scribe State is idle for binding to be null? The problem was not fixed for the English Keyboard. It does not happen for the other keyboard right.
You mean like this? but it still crashed in English keyboard. @angrezichatterbox
You mean like this? but it still crashed in English keyboard. @angrezichatterbox
Pass it like binding = null. Similar to how it is done for the German and other keyboards.
Are we good for a final check here? Thank you both for the great collaboration!
And does this also solve #186?
Yes, you can close both issues together after this pull request is successfully merged. 😊
And does this also solve #186?
Yes, you can close both issues together after this pull request is successfully merged. 😊
Could u also add the initialization and update to the other Keyboard IME
And does this also solve #186?
Yes, you can close both issues together after this pull request is successfully merged. 😊
Could u also add the initialization and update to the other Keyboard IME
Are you referring to languages other than English and German?
And does this also solve #186?
Yes, you can close both issues together after this pull request is successfully merged. 😊
Could u also add the initialization and update to the other Keyboard IME
Are you referring to languages other than English and German?
Yes
Could we get the most recent conflicts fixed and then I'll do a final review? :)
Okay, could you resolve the conflicting code to keep these features? I don't have the permission to do this. 😊
Could we get the most recent conflicts fixed and then I'll do a final review? :)
You should be able to pull the main branch into your branch and then resolve the conflicts, @Linfye :) We can do it for you if you'd like us to though 😊
You should be able to pull the main branch into your branch and then resolve the conflicts, @Linfye :) We can do it for you if you'd like us to though 😊
Oh, I know now. But it's midnight here. 💤 If you have time, could you do this? Or wait until I can do it tomorrow. 😁