Add Emoji Autosuggestion Buttons #158

Linfye · 2024-09-26T13:25:22Z

Contributor checklist

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

Add emoji autosuggestion buttons to the keyboard UI #138

github-actions · 2024-09-26T13:25:39Z

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

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.email in their local Scribe-Android repo
The linting and formatting workflows within the PR checks do not indicate new errors in the files changed
The CHANGELOG has been updated with a description of the changes for the upcoming release and the corresponding issue (if necessary)

angrezichatterbox · 2024-09-27T05:36:18Z

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.

angrezichatterbox · 2024-09-27T05:37:17Z

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.

andrewtavis · 2024-09-27T05:54:34Z

Within 24 hours isn't a delayed response, @angrezichatterbox :) School's important and certainly the priority 😊

Thanks for the suggestions!

andrewtavis · 2024-10-07T01:19:03Z

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 😊

Linfye · 2024-10-08T16:19:02Z

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.

andrewtavis · 2024-10-08T21:08:34Z

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 :)

Linfye · 2024-10-09T14:01:11Z

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!

angrezichatterbox · 2024-10-09T16:41:22Z

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 will look into it and get back to you :)

angrezichatterbox · 2024-10-09T17:00:29Z

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!

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.

Linfye · 2024-10-10T15:04:57Z

@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 · 2024-10-10T16:55:25Z

@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

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.

Linfye · 2024-10-11T03:07:02Z

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

angrezichatterbox · 2024-10-11T03:10:14Z

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

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 ?

Linfye · 2024-10-11T03:19:03Z

Yes, it crashed every time. This issue happened after this.

I don't know the exact commit.

angrezichatterbox · 2024-10-11T03:19:54Z

Scribe-Android/app/src/main/java/be/scri/services/EnglishKeyboardIME.kt

Lines 193 to 214 in 6758b8f

    
           MyKeyboard.KEYCODE_DELETE -> { 
        
               if (currentState == ScribeState.IDLE || currentState == ScribeState.SELECT_COMMAND) { 
        
                   handleDelete(false, keyboardBinding) 
        
               } else { 
        
                   handleDelete(true, keyboardBinding) 
        
               } 
        
               keyboardView!!.invalidateAllKeys() 
        
           } 
        
           MyKeyboard.KEYCODE_SHIFT -> { 
        
               super.handleKeyboardLetters(keyboardMode, keyboardView) 
        
               keyboardView!!.invalidateAllKeys() 
        
           } 
        
           KEYCODE_ENTER -> { 
        
               if (currentState == ScribeState.IDLE || currentState == ScribeState.SELECT_COMMAND) { 
        
                   handleKeycodeEnter(keyboardBinding, false) 
        
               } else { 
        
                   handleKeycodeEnter(keyboardBinding, true) 
        
                   currentState = ScribeState.IDLE 
        
                   switchToCommandToolBar() 
        
                   updateUI() 
        
               } 
        
           }

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.

Linfye · 2024-10-11T03:33:11Z

You mean like this? but it still crashed in English keyboard. @angrezichatterbox

angrezichatterbox · 2024-10-11T03:34:28Z

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.

Linfye · 2024-10-11T03:55:14Z

OK, problem solved. Thanks!

andrewtavis · 2024-10-11T08:27:15Z

Are we good for a final check here? Thank you both for the great collaboration!

andrewtavis · 2024-10-11T08:27:42Z

And does this also solve #186?

Linfye · 2024-10-11T09:07:54Z

And does this also solve #186?

Yes, you can close both issues together after this pull request is successfully merged. 😊

angrezichatterbox · 2024-10-11T09:16:27Z

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

Linfye · 2024-10-11T09:42:38Z

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?

angrezichatterbox · 2024-10-11T09:49:50Z

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

Linfye · 2024-10-11T10:09:48Z

👌

andrewtavis · 2024-10-12T14:33:53Z

Could we get the most recent conflicts fixed and then I'll do a final review? :)

Linfye · 2024-10-12T14:59:17Z

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? :)

Linfye · 2024-10-12T16:06:21Z

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. 😁

andrewtavis · 2024-10-13T07:30:30Z

Thanks for getting to this, @Linfye! Quite busy with all the activity in the projects right now, so this really does help 😊

andrewtavis · 2024-10-13T07:30:43Z

Let us know when it's ready :)

Linfye · 2024-10-13T07:46:49Z

I think I've resolved the code conflicts and also implemented the emoji autosuggest button for tablets now. Could you find some time to review it? If there are any issues, please let me know at any time. 😊

andrewtavis · 2024-10-13T14:43:37Z

Could we ask you for one more check here, @angrezichatterbox? :)

angrezichatterbox · 2024-10-13T16:50:57Z

Could we ask you for one more check here, @angrezichatterbox? :)

I'll check it again by tomorrow afternoon. I'm travelling right now :)

angrezichatterbox · 2024-10-14T03:44:51Z

@Linfye, could we make the button visibility based on the application's state? Currently, when users goes to select a command, the Plural Button gets hidden behind the auto-suggestions. Additionally, once the user returns to the idle state, the auto-suggestions button disappears.
A potential solution would be to link the button visibility to the updateUI() function or to integrate it with the toolbar or command view setup or switch functions. This way, we can ensure that the buttons are consistently visible and appropriately reflect the current state of the keyboard.

Linfye · 2024-10-14T05:19:36Z

Thanks, I didn't notice this before. I'll fix this later today. @angrezichatterbox

Linfye · 2024-10-14T13:11:13Z

I've fixed the bug, and now the emoji autosuggest buttons appear only when in the SELECT_COMMAND state. If you there‘s any other issues, please feel free to let me know. :)

angrezichatterbox · 2024-10-14T17:26:34Z

I guess it should be shown only in idle state. Could you make that change as well :) Thanks

andrewtavis · 2024-10-14T17:29:14Z

Hehe sorry I need to read a bit better. Absolutely correct, @angrezichatterbox :) Should only be shown on idle 😊

Linfye · 2024-10-15T03:02:12Z

Now the autosuggest buttons show in idle state. 😊

andrewtavis

Looking really great to me, @Linfye! Thanks so much for the dedication in getting this done 😊 So awesome to have you as such a reliable contributor here :) :)

andrewtavis requested review from angrezichatterbox and andrewtavis September 26, 2024 13:46

Linfye added 9 commits October 13, 2024 15:09

Add emoji buttons

551f77e

Add phone emoji button function

9f59329

Add EmojiButtonsController to SimpleKeyboardIME

0ea7e02

Update binding and English IME

16208d0

Update parameter passing method

bd024d1

Fix format issues

811ba00

Fix typo for scribe-org#186 and binding

93a2aac

Fix binding

78493ad

Add updates to all languages

1f02cbb

Linfye force-pushed the emoji-autosuggestion-buttons branch from eeb03ce to 1f02cbb Compare October 13, 2024 07:18

Add emoji autosuggest buttons for tablet

b07a809

Fix emoji buttons disappearing

9d71676

Emoji autosuggest buttons show in idle state

34645ed

andrewtavis approved these changes Oct 15, 2024

View reviewed changes

andrewtavis merged commit 8e08279 into scribe-org:main Oct 15, 2024
1 check passed

This was referenced Oct 15, 2024

Add emoji autosuggestion buttons to the keyboard UI #138

Closed

Autosuggest Emojis Switch Status Not Saved Across Languages in Language Settings #186

Closed

Linfye deleted the emoji-autosuggestion-buttons branch October 16, 2024 10:00

Add Emoji Autosuggestion Buttons #158

Add Emoji Autosuggestion Buttons #158

Conversation

Linfye commented Sep 26, 2024

Contributor checklist

Description

Related issue

github-actions bot commented Sep 26, 2024 • edited by andrewtavis Loading

Thank you for the pull request!

Maintainer checklist

angrezichatterbox commented Sep 27, 2024

angrezichatterbox commented Sep 27, 2024

andrewtavis commented Sep 27, 2024

andrewtavis commented Oct 7, 2024

Linfye commented Oct 8, 2024

andrewtavis commented Oct 8, 2024

Linfye commented Oct 9, 2024

angrezichatterbox commented Oct 9, 2024

angrezichatterbox commented Oct 9, 2024 • edited Loading

Linfye commented Oct 10, 2024 • edited Loading

angrezichatterbox commented Oct 10, 2024

Linfye commented Oct 11, 2024 • edited Loading

angrezichatterbox commented Oct 11, 2024 • edited Loading

Linfye commented Oct 11, 2024

angrezichatterbox commented Oct 11, 2024 • edited Loading

Linfye commented Oct 11, 2024

angrezichatterbox commented Oct 11, 2024 • edited Loading

Linfye commented Oct 11, 2024

andrewtavis commented Oct 11, 2024

andrewtavis commented Oct 11, 2024

Linfye commented Oct 11, 2024

angrezichatterbox commented Oct 11, 2024

Linfye commented Oct 11, 2024

angrezichatterbox commented Oct 11, 2024

Linfye commented Oct 11, 2024

andrewtavis commented Oct 12, 2024

Linfye commented Oct 12, 2024

Linfye commented Oct 12, 2024 • edited Loading

andrewtavis commented Oct 13, 2024

andrewtavis commented Oct 13, 2024

Linfye commented Oct 13, 2024

andrewtavis commented Oct 13, 2024

angrezichatterbox commented Oct 13, 2024

angrezichatterbox commented Oct 14, 2024 • edited Loading

Linfye commented Oct 14, 2024

Linfye commented Oct 14, 2024

angrezichatterbox commented Oct 14, 2024 • edited Loading

andrewtavis commented Oct 14, 2024

Linfye commented Oct 15, 2024

andrewtavis left a comment

Choose a reason for hiding this comment

github-actions bot commented Sep 26, 2024 •

edited by andrewtavis

Loading

angrezichatterbox commented Oct 9, 2024 •

edited

Loading

Linfye commented Oct 10, 2024 •

edited

Loading

Linfye commented Oct 11, 2024 •

edited

Loading

angrezichatterbox commented Oct 11, 2024 •

edited

Loading

angrezichatterbox commented Oct 11, 2024 •

edited

Loading

angrezichatterbox commented Oct 11, 2024 •

edited

Loading

Linfye commented Oct 12, 2024 •

edited

Loading

angrezichatterbox commented Oct 14, 2024 •

edited

Loading

angrezichatterbox commented Oct 14, 2024 •

edited

Loading