Skip to content

Conversation

@bhanu-dev82
Copy link
Collaborator

Contributor checklist


Description

Fixes: #398 (broken gender suggestions); also addresses plural and case suggestion issues.

This PR refactors and centralizes the word suggestion logic in KeyHandler.kt to ensure consistent behavior for German noun type, plural, and case annotation suggestions — especially after space, delete, and regular key presses.


✅ Key Changes

🔁 Unified Suggestion Handling

  • Added processWordSuggestions(currentWord) to encapsulate suggestion logic.
  • Called after space, delete, and character key presses (in idle mode).

⌨️ Space & Delete Logic

  • handleKeycodeSpace() now commits a space, then triggers suggestions.
  • handleDeleteKey() updates or clears suggestions after a delete.
  • Skips suggestions in command bar context.

🔤 Character Input

  • handleDefaultKey() disables suggestions during command bar usage.

♻️ Suggestion Clearing

  • Inlined logic to reset noun/plural/case/emoji states in multiple handlers (enter, arrow, etc.).

😊 Emoji Fallback

  • If no noun/case suggestions found, emojis are suggested via updateEmojiSuggestions().

🐛 Bug Fixes

  • Gender Suggestion Missing on Space: Now correctly appears using processWordSuggestions().
  • Incorrect "PL" Suggestion on Empty Field: Fixed with null/empty check.
  • Inconsistent Suggestions: Unified flow ensures reliable updates across keys.

🧪 Testing

  • "buch " → correctly shows "N" (neuter)
  • Deleting characters updates or clears suggestions appropriately
  • Logs confirm expected flow through processWordSuggestions()

🔍 Note and Demo

This issue was approached with a test-driven and results-oriented development process. Community feedback and additional testing are encouraged to ensure comprehensive coverage and robustness across use cases.

Screen_Recording_20250529_042309_Keep.Notes.mp4

Closes: #398

@github-actions
Copy link

Thank you for the pull request! ❤️

The Scribe-Android team will do our best to address your contribution as soon as we can. 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 attending our bi-weekly Saturday dev syncs. It'd be great to meet you 😊

@github-actions
Copy link

Maintainer Checklist

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

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

@andrewtavis
Copy link
Member

Thanks so much for this, @bhanu-dev82! Looking forward to the review as always as the quality of the information is so helpful 😊

@andrewtavis
Copy link
Member

Would you be able to fix the merge conflict with the code from #412, @bhanu-dev82? Maybe your solution also solves the plural annotation after the plural command as well, but if not then that was brought in from that PR 😊

@bhanu-dev82
Copy link
Collaborator Author

Would you be able to fix the merge conflict with the code from #412, @bhanu-dev82? Maybe your solution also solves the plural annotation after the plural command as well, but if not then that was brought in from that PR 😊

I've resolved the merge conflict and confirmed that everything is working as expected. I tested the plural button with "book" and "boy" for English, and "Buch" for German. The gender suggestion feature for the German keyboard is also functioning correctly.

I’d appreciate feedback on the implementation. Due to Detekt constraints (line and function limits), I had to adapt the original logic, which may have slightly altered the intended design—assistance on refining this would be helpful.

Thank you for adding me as a collaborator to this project.

@andrewtavis
Copy link
Member

Thanks for fixing the merge conflict and checking the functionality, @bhanu-dev82! Just checked the changes and all's working great in terms of the work for #398, but it looks like the change from #412 has been lost 🤔 Specifically for this we want the noun annotation to after commands. Here are two situations for the German keyboard that should function:

  • If we translate the English word "book" on the German keyboard and get back "Buch", then we should see the neutral noun annotation (a green N should be in the picture below)
  • Another note on the above is that we're not getting an extra space added in after the translated word
    • Right now we're getting back "Buch" from "book", but we should be getting "Buch " with a space after

Screenshot_20250529_222405

  • If we get the plural for the German word "Buch" on the German keyboard and get back "Bücher", then we should see the plural noun annotation (an orange PL should be in the picture below)
    • This command does correctly return an extra space after the output word :)

Screenshot_20250529_222429

Please let us know if you have questions on the above! I'll let @angrezichatterbox provide feedback on the code specifics 😊

Thank you for being such a great contributor to this project! Your collaborator status was more than earned 😊 Looking forward to continuing to work with you! It really has been a pleasure :)

@bhanu-dev82
Copy link
Collaborator Author

yes i am just completing the fix for it, it will soon be pushed. Thankyou

@andrewtavis
Copy link
Member

Amazing, @bhanu-dev82! Thanks for getting to all of this so quickly! 😊

@bhanu-dev82
Copy link
Collaborator Author

bhanu-dev82 commented May 29, 2025

i have fixed the errors:

  • translate btn now add space after each word
  • plural btn have proper PL suggestions
  • gender suggestions works
  • multiple suggestions (PL/M, PL/F and more) works

I have made 2 new files namely SpaceKeyProcessor.kt and SuggestionHandler.kt just to make detekt forgive me 😜

here is a video showing the fix :

Screen_Recording_20250530_024810_Keep.Notes.mp4

@andrewtavis
Copy link
Member

Thanks so much for the quick work here, @bhanu-dev82! We'll try to get to the final review in the coming days 😊

Copy link
Member

@angrezichatterbox angrezichatterbox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for refactoring the codebase while fixing the bug @bhanu-dev82.

There is a slight issue in the implementation that causes the emoji suggestions to disappear when the command bar suggestion shows up. The actual behavior would be that both would appear simultaneously.

@angrezichatterbox
Copy link
Member

@andrewtavis I assume the intended behavior would be both to be shown simultaneously.

@andrewtavis
Copy link
Member

andrewtavis commented May 31, 2025

Assuming you mean noun annotation, @angrezichatterbox? Looking at the most recent video, it looks like we're just getting the annotations like PL after commands, but yes, we should also get the emoji autosuggestios as well 😊

@angrezichatterbox
Copy link
Member

Assuming you mean noun annotation, @angrezichatterbox? Looking at the most recent video, it looks like we're just getting the annotations like PL after commands, but yes, we should also get the emoji autosuggestios as well 😊

Yes, Currently when we type buch only the gender suggestion show up rather than the emoji suggestion also showing up.

@andrewtavis
Copy link
Member

Yes, it'd be important for us to show both :)

Are the colored F, M, N, etc called suggestions in the code? Maybe something I didn't see in review. Would be good if we had some consistency with the iOS app for this, with these being "annotations". We already have autosuggestions and emoji suggestions, so having these be known as something distinct would likely be good :) We could make an issue for the hackathon to rename related variables if this is the case and you're fine with the naming convention, @angrezichatterbox 😊

@bhanu-dev82
Copy link
Collaborator Author

Assuming you mean noun annotation, @angrezichatterbox? Looking at the most recent video, it looks like we're just getting the annotations like PL after commands, but yes, we should also get the emoji autosuggestios as well 😊

Yes, Currently when we type buch only the gender suggestion show up rather than the emoji suggestion also showing up.

Apologies for the delayed response — I've been occupied with exams. The emoji issue will be addressed through the fix for #404, as it includes handling for emoji-related cases. If possible, could we proceed with this approach? if not i will fix it and push in the same PR. i will be working on #404 today.

@andrewtavis
Copy link
Member

I guess if you're fine with it @bhanu-dev82, just send along the changes to this branch and we'll finalize both issues in one PR? That way we can check the functionalities together 😊

@bhanu-dev82 bhanu-dev82 deleted the fix_gender_suggestion branch June 12, 2025 08:12
@bhanu-dev82 bhanu-dev82 restored the fix_gender_suggestion branch June 12, 2025 10:29
@bhanu-dev82 bhanu-dev82 reopened this Jun 12, 2025
@bhanu-dev82
Copy link
Collaborator Author

The recent merge was intended solely to integrate the latest changes. A major update is in progress, which significantly improves performance. Specifically, the complexity of generating suggestions has been optimized from O(n) to O(1), resulting in a much faster keyboard and an overall smoother user experience.

@andrewtavis
Copy link
Member

Let us know when this is ready from your side, @angrezichatterbox :)

@angrezichatterbox
Copy link
Member

angrezichatterbox commented Jun 20, 2025

I am doing some additional chores that I have noticed within the same PR. As I am already reviewing the codebase quite a bit. I hope that would be fine.

@angrezichatterbox
Copy link
Member

@andrewtavis It is ready for review. I have tried seeing how the refactoring would be and refactored some of the components in PR. However for the complete refactoring we could have a separate issue as being discussed in #426.

@angrezichatterbox
Copy link
Member

What do you think about some refactoring issues for the hackathon @andrewtavis

@andrewtavis
Copy link
Member

andrewtavis commented Jun 20, 2025

[Removing the comment that was here - Git was buggy]

@andrewtavis
Copy link
Member

andrewtavis commented Jun 20, 2025

Project isn't building on my end, even after clearing the caches. Are there other steps that I could do to get it working? I also just updated Android Studio to Meercat 🤔

@andrewtavis
Copy link
Member

Execution failed for task ':app:processCoreDebugResources'.
> Could not resolve all files for configuration ':app:coreDebugRuntimeClasspath'.
   > Failed to transform biometric-1.2.0-alpha05.aar (androidx.biometric:biometric:1.2.0-alpha05) to match attributes {artifactType=android-compiled-dependencies-resources, org.gradle.category=library, org.gradle.dependency.bundling=external, org.gradle.libraryelements=aar, org.gradle.status=release, org.gradle.usage=java-runtime}.
      > Execution failed for AarResourcesCompilerTransform: PATH/.gradle/caches/8.11.1/transforms/7dd7686cc8712ff5bf13738323e503b0/transformed/biometric-1.2.0-alpha05.
         > AAPT2 aapt2-8.10.1-12782657-osx Daemon #0: Daemon startup failed
           This should not happen under normal circumstances, please file an issue if it does.
   > Failed to transform appcompat-1.7.1.aar (androidx.appcompat:appcompat:1.7.1) to match attributes {artifactType=android-compiled-dependencies-resources, org.gradle.category=library, org.gradle.dependency.bundling=external, org.gradle.libraryelements=aar, org.gradle.status=release, org.gradle.usage=java-runtime}.
      > Execution failed for AarResourcesCompilerTransform: PATH/.gradle/caches/8.11.1/transforms/d7fd5fcd804e9647c3a8b284a003996a/transformed/appcompat-1.7.1.
         > AAPT2 aapt2-8.10.1-12782657-osx Daemon #1: Daemon startup failed
           This should not happen under normal circumstances, please file an issue if it does.
   > Failed to transform core-1.16.0.aar (androidx.core:core:1.16.0) to match attributes {artifactType=android-compiled-dependencies-resources, org.gradle.category=library, org.gradle.dependency.bundling=external, org.gradle.libraryelements=aar, org.gradle.status=release, org.gradle.usage=java-runtime}.
      > Execution failed for AarResourcesCompilerTransform: PATH/.gradle/caches/8.11.1/transforms/58c640cf083e1536287e06acc04912f9/transformed/core-1.16.0.
         > AAPT2 aapt2-8.10.1-12782657-osx Daemon #2: Daemon startup failed
           This should not happen under normal circumstances, please file an issue if it does.

* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Run with --scan to get full insights.
> Get more help at https://help.gradle.org.
BUILD FAILED in 21s
30 actionable tasks: 29 executed, 1 up-to-date

@angrezichatterbox
Copy link
Member

Could you try running ./gradlew clean and ./gradlew build It build fine for me. The errors seem to be from cache. I could revert back the build.gradle.kts

@angrezichatterbox
Copy link
Member

Could you check now. Or else I would revert back the entire build.gradle.kts. I have downgraded the ksp to support kotlin 2.0.0

@andrewtavis
Copy link
Member

./gradlew build failed with the following:

> Task :app:packageCoreRelease FAILED
[Incubating] Problems report is available at: file:///Users/andrewtavis/coding/scribe-org/Scribe-Android/build/reports/problems/problems-report.html

FAILURE: Build failed with an exception.

The info that's in the problem report is:

    - [warn]  The Configuration.fileCollection(Spec) method has been deprecated. (10)
            - [warn]  The Configuration.fileCollection(Spec) method has been deprecated.
            - [warn]  The Configuration.fileCollection(Spec) method has been deprecated.
            - [warn]  The Configuration.fileCollection(Spec) method has been deprecated.
            - [warn]  The Configuration.fileCollection(Spec) method has been deprecated.
            - [warn]  The Configuration.fileCollection(Spec) method has been deprecated.
            - [warn]  The Configuration.fileCollection(Spec) method has been deprecated.
            - [warn]  The Configuration.fileCollection(Spec) method has been deprecated.
            - [warn]  The Configuration.fileCollection(Spec) method has been deprecated.
            - [warn]  The Configuration.fileCollection(Spec) method has been deprecated.
            - [warn]  The Configuration.fileCollection(Spec) method has been deprecated.

The build works now within Android Studio, then just the app loads, I can install a keyboard within the settings, but I can't use the keyboard 🤔

@angrezichatterbox
Copy link
Member

angrezichatterbox commented Jun 21, 2025

The build works now within Android Studio, then just the app loads, I can install a keyboard within the settings, but I can't use the keyboard 🤔

Are you using a Pixel Pro 9 API 35 itself ? ./gradlew build is linked with other tasks now that's why its failing. The update makes the default emulator API 36

@angrezichatterbox
Copy link
Member

For API 36 emulator none of the external downloaded keyboard works. Like I tried the fossify keyboard we can't switch to it as well.

@andrewtavis
Copy link
Member

andrewtavis commented Jun 21, 2025

I'll try with a Pixel 9 and API 36 35 when I'm home :)

Copy link
Member

@andrewtavis andrewtavis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All's working with Pixel 9 and API 35 ✨🎉 Amazing work here, @bhanu-dev82! Really commendable how much we've progressed in this single PR 😊 Thanks also for the review and the refactoring/updates, @angrezichatterbox! 🙏

So glad to have this one in :) Let's focus on bringing in #395 and #420 now. You'd be welcome to start reviewing PRs too, @bhanu-dev82! Please let us know if this would be something you'd have interest in. As it suits you 😊

@andrewtavis
Copy link
Member

There's enough work in here that I'm not going to squash so it's a bit more reflective of the effort that went into this. Let's also discuss what's up with API 36... Likely is just something that will come in a forthcoming update.

@andrewtavis andrewtavis merged commit 0d6e781 into scribe-org:main Jun 21, 2025
6 checks passed
@DeleMike
Copy link
Collaborator

Great work here! @bhanu-dev82

Really inspiring ⭐🙌🏾✨

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The Gender Suggestion on basis of input text is broken

4 participants