scribe-org / Scribe-iOS

iOS app with keyboards for language learners
https://apps.apple.com/app/scribe-language-keyboards/id1596613886
GNU General Public License v3.0
128 stars 78 forks source link

Alphabetized Autocomplete #188

Closed andrewtavis closed 2 years ago

andrewtavis commented 2 years ago

Terms

Description

This issue is to add an initial auto-suggest feature to Scribe, and is the first part of #3. Work for this will be done in relation to #187 for the Wikimania Hackathon.

This issue comprises:

Contribution

I'll be working on this with @SaurabhJamadagni :)

andrewtavis commented 2 years ago

@SaurabhJamadagni, I added something to the above, as we need to remember that even though the command bar won't be on the screen always, we're still going to need to use it to do noun and preposition annotations. Here's again the picture of eventual coloration in autocomplete:

https://user-images.githubusercontent.com/24387426/182542168-f8e337f4-4709-413c-86e4-e6ded470eed7.png

For annotation we're going to want to remove the lines, and then we can use the command bar with a transparent background to still do annotation as before (none of the command buttons would appear). Remember that the difference here is that annotation happens after space, so we won't be showing suggestions at this point yet (that's autosuggest, rather than autocomplete).

SaurabhJamadagni commented 2 years ago

@andrewtavis, I'm not clear on what you mean by annotations after space. Could you give me an example? Do you mean that the word suggestions will show up alphabetically even if the user just adds a space at the start?

andrewtavis commented 2 years ago

Hi @SaurabhJamadagni :) What I mean by this is the noun genders that are shown to the user after they press space. So as the user is typing a word we want to show them autocomplete suggestions, and after they’ve pressed space we show them annotations as we do now. Rather than being “on the command bar,” they’ll be next to the Scribe key and still will be the bar text, but the command bar will be transparent.

SaurabhJamadagni commented 2 years ago

Ohhh I understand! So basically, we are just going to make the command bar transparent. When the user is typing, the suggestions will be shown with the vertical dividers. At other times, our prompts will be shown like we have right now. Is that correct @andrewtavis?

SaurabhJamadagni commented 2 years ago

Also @andrewtavis, while playing around with the command bar, I came across a bug right now. Can you check if you are experiencing it too? Basically the prompt that says what the command does, doesn't change colour when you switch appearance modes. But if you exit command mode and go back again, then it works fine. The placeholder text does change colour though.

Steps to replicate it:

  1. Select any command.
  2. Change from light to dark mode or vice-versa without typing so that the prompt stays.

Images: Before changing appearance mode: IMG_2957

After changing appearance mode: IMG_2958

After pressing 'x' and choosing the command again: IMG_2959

Seems like a minor bug which we could get to later. Unless it's just a my device glitch 😅

andrewtavis commented 2 years ago

Ohhh I understand! So basically, we are just going to make the command bar transparent. When the user is typing, the suggestions will be shown with the vertical dividers. At other times, our prompts will be shown like we have right now. Is that correct @andrewtavis?

Exactly :) Sorry for being a bit confusing in the explanation! Hard to wrap around all the little things to change.

Per bug that you found, I can reproduce as well. It's likely something simple like making sure that the colors are being changed within checkDarkModeSetColors. Dark and light mode switching was the first major bugs for Scribe - basically didn't work 🤦‍♂️🤣 Not surprised that there's an issue, and we should definitely fix it, but then it's also very unlikely that a user will experience it.

Do you want to make the issue, or should I? If you come across something, feel free to open a bug. If you're seeing it, then it should at least be checked, which is worth an issue 😊

SaurabhJamadagni commented 2 years ago

Dark and light mode switching was the first major bugs for Scribe - basically didn't work 🤦‍♂️🤣 Not surprised that there's an issue, and we should definitely fix it

I see 😂 Let's fix it but definitely not a priority.

Do you want to make the issue, or should I? If you come across something, feel free to open a bug. If you're seeing it, then it should at least be checked, which is worth an issue 😊

I'll create the issue. A little experience with that too. Never raised an issue before haha. Thanks for confirming! 😊

andrewtavis commented 2 years ago

@SaurabhJamadagni, here are some directions based on what I've been working on:

I think the function to assign the buttons should be called within conditionallySetAutoActionBtns, which is ran after each key press. The function should find the next three nouns, and that function will assign the words to the buttons that need to be set. I'll look into making case "AutoAction1", etc conditions within executeKeyActions so that we're ready when the function's up and running :) That should be pretty simple 😊

andrewtavis commented 2 years ago

@SaurabhJamadagni, I think that the above is about as much as I should do on this to make sure that you can have a substantial contribution :) 3dc751d allows the keys to be assigned labels in conditionallySetAutoActionBtns, and there are now cases in executeKeyActions to insert the key text, add a space, and check for annotation 😊

If we can't check in before the presentation (I think that what we have so far is presentable btw), then I'm seeing in the schedule that the open hacking space is available right after. We can jump in there and talk things over, or also do a call on Google Meet :) :)

Hope you're having a nice time with your family!

SaurabhJamadagni commented 2 years ago

Thanks @andrewtavis! You were able to get so much done 🤯

I was playing around in Playgrounds to test out different approaches to the function and I think I have something that might work. I'll get to making that addition. Thanks for all the changes you made! I'll check back in once the changes are made. If I am not wrong, I have to checkout the feature branch you have created and push my commits to the same branch right? Or do I create a new branch? Again, sorry for these doubts right now. They are definitely not adding to the productivity 😅

andrewtavis commented 2 years ago

Welcome and thank you, @SaurabhJamadagni! It was nice to really code in here agin 😇

Checking out the feature branch and pushing your commits to the same one is exactly what you should do :) Not sure if you need/want to, but you can also review/accept the changes in the PR if you want to understand the changes I made. Up to you, as we can just test later and figure it out!

Looking forward to the function! I doubt that I'll add that into the presentation given timing, but if we can get it done by the showcase, then that'd be 🔥

Let me know if you have questions or need any help!

SaurabhJamadagni commented 2 years ago

Function to find the next three words

func getAutocompleteWords() {
  let keysOfNouns = Array(nouns!.keys).sorted()
  if let inString = proxy.documentContextBeforeInput {
    let stringOptions = keysOfNouns.filter { item in
         let stringMatch = item.lowercased().range(of: inString.lowercased())
        return stringMatch != nil ? true : false
    }

    if stringOptions.count <= 3 {
        completionWords = [String](stringOptions)
    } else {
        var i = 0
        var threeWords = [String]()
        while i < 3 {
            threeWords.append(stringOptions[i])
            i += 1
        }
        completionWords = threeWords
    }
  }
}
andrewtavis commented 2 years ago

Sorry for what likely are some confusing PR notifications, @SaurabhJamadagni :) It's all figured out now. Next time I'll know to first merge your branch into mine and then go from there.

As I mentioned, before tomorrow I'm going to try to work on the latency a bit by creating a sorted array with words that lack hyphens. I'm thinking that doing that might speed up the assignment a bit, as as of now we're creating and sorting a very large array every time we want to update the keys. It'd also be good if we always assign the "next thing in the list" even if there's nothing that comes after. Would be good to basically just fill in the next word in the list in this case.

If you have suggestions, then I'd be happy to hear them! Thanks again and again 😊

andrewtavis commented 2 years ago

Just played around some more 😊 This is so great! 🥳

andrewtavis commented 2 years ago

And the latency seems to be much faster coming out of a pre-created array and not making/sorting it each time 🙌

SaurabhJamadagni commented 2 years ago

As I mentioned, before tomorrow I'm going to try to work on the latency a bit by creating a sorted array with words that lack hyphens. I'm thinking that doing that might speed up the assignment a bit, as as of now we're creating and sorting a very large array every time we want to update the keys. It'd also be good if we always assign the "next thing in the list" even if there's nothing that comes after. Would be good to basically just fill in the next word in the list in this case.

Yes @andrewtavis, I also noticed that as the bottleneck. I was wondering maybe that I could just create the array in CommandVariables just like we decode the nouns from the JSON there. That way we can just reference it without sorting it again and again like you said. Creating the array just once seems like the way to go.

Thanks for the review! Yes the emails from the PR notifications were quite confusing haha. The first thought that came to mind was that I broke something irreversibly 😂

andrewtavis commented 2 years ago

Committing the minor change to make the array directly in 100951e :) Some current thoughts on this:

We already found a very simple very very German word that wasn't in Wikidata just from me trying it out 😊 Flohmarkt - German for Fleamarket or Saturday Market (they're HUGE here) - has now been added to Wikidata :)

andrewtavis commented 2 years ago

Specifically for the autocomplete stopping, if you just type the word and press space yourself, then it stops. If you select the word, then it tends to keep going. And in regards to the first point above, it seems that after the first autocomplete the next suggestions are all lower case, which specifically for German is problematic as they all need to be capitalized :)

SaurabhJamadagni commented 2 years ago

Specifically for the autocomplete stopping, if you just type the word and press space yourself, then it stops. If you select the word, then it tends to keep going. And in regards to the first point above, it seems that after the first autocomplete the next suggestions are all lower case, which specifically for German is problematic as they all need to be capitalized :)

Ah, I didn't know that about German. Totally my bad. I just formatted it using rules used by English. That was a terrible assumption. As for the second issue, I think maybe adding the boolean variables autoAction1Visible and autoAction2Visible in the case: spaceBar might fix it. I'll try out these changes in the morning if that's ok @andrewtavis?

andrewtavis commented 2 years ago

I should have seen it in the code review myself, so no stress on the German nouns bit :) Has been a long day, so it wasn't my best review. Feel free to send along some changes in the morning! Thanks for all your hard work today! 💪

andrewtavis commented 2 years ago

Big thing is - now looking at Spanish where if you start typing it will start suggesting words that are lower case when you have shift pressed - it should check if the shift key has been pressed, and if so the words in the completions should also be capitalized :)

This is so cool 😊

SaurabhJamadagni commented 2 years ago

Hi @andrewtavis! Just wanted to give an update since I haven't pushed any changes since morning. I fixed the capitalisation issue and the autocomplete on space issue. After that noticed that the auto complete also went away if you delete a word afterwards as the pastStringInTextProxy is never updated. Working on that currently. Trying to find some way of being able to track when to change the variable so auto complete works on delete as well :)

andrewtavis commented 2 years ago

No stress, @SaurabhJamadagni! Just checking on the capitalization, this means that if the user has shift pressed the words will all be capitalized? Also, could we add something similarly where if capslock is on that the words are all caps? Not that I need it, but it is an expected feature :)

Keep up the good work!

SaurabhJamadagni commented 2 years ago

Just checking on the capitalization, this means that if the user has shift pressed the words will all be capitalized? Also, could we add something similarly where if capslock is on that the words are all caps? Not that I need it, but it is an expected feature :)

Yes, the suggestions show a capitalized word if the shift is pressed. I could try it out with the capslock as well no problem. I'll get to it in just a bit.

andrewtavis commented 2 years ago

Wonderful! This is great! It'd be cool to be able to show that off, and that way we can also show autocomplete in other keyboards :)

SaurabhJamadagni commented 2 years ago

Hey @andrewtavis, I fixed the no auto complete on delete and added the capslock word suggestions as well. But when the text proxy is empty, repeatedly pressing the delete button is crashing the keyboard. I'll push my commits and create a PR right now. Could you maybe check why that might be happening. Maybe something you have experienced before. 😅

andrewtavis commented 2 years ago

Hi @SaurabhJamadagni :) I'll give it a look and potentially just bring it in for us to work on later in a different bug issue that needs to be resolved before the release. Will let you know how things are looking in the PR 😊

andrewtavis commented 2 years ago

Thanks again for all this, @SaurabhJamadagni 😊 You've worked a ton on Scribe this weekend! You really have my thanks, and I hope the experience has been an interesting one for you :) I mentioned in the PR that capitalization still needs a bit of work, but it's almost exactly what we need.

I'm thinking also that maybe adding in verbs and prepositions might be best, as that way there's always suggestions happening :) We can work on this going forward and shoot for a release with #194 when we're feeling comfortable with how it all's working. Major major strides on the project this weekend, and it's totally amazing how much we've gotten done in a short time!

SaurabhJamadagni commented 2 years ago

Hey @andrewtavis! How are you doing? 😊

I will get to the requested review on PR #195 sometime today or tomorrow. I'm also interested in working on the AutoSuggest issue so I'll comment on that page shortly. Looking forward to the next release! It's super exciting 🚀

andrewtavis commented 2 years ago

Hi @SaurabhJamadagni :) I'm well! How's all with you? Am talking with someone from the hackathon about whether they'd like to work on the Android version :)

No stress on the PR for #195 - get to it when you can. And yes, let's definitely talk #194 so that we can get both of these features released! I'm really looking forward to it as well, and think that people are going to be really impressed 😊

andrewtavis commented 2 years ago

Hi @SaurabhJamadagni, hope you're well :)

Let me know if you need any support on the PR. I was feeling a bit tired post hackathon and had lots to do, but have a bit more bandwidth now if you want to discuss the needed changes 😊

SaurabhJamadagni commented 2 years ago

Hi @andrewtavis! Sorry I haven't been updating lately. My summer break ended today. So I have was a bit busy prepping for classes. Anyway, I have tomorrow off and a long free weekend ahead. I'll give it a shot tomorrow and let you know if I need help. Thanks for asking. Sorry for the delay! 😊

andrewtavis commented 2 years ago

No worries on a delay whatsoever, @SaurabhJamadagni! Just be in touch and we'll figure it out :)

andrewtavis commented 2 years ago

Also, a final thing we should consider is expanding the word list a bit. Linking in the potential of using wordfreq or other Python based analytics in #194, we could also potentially subset a few popular verbs and include their conjugations in the list, and maybe including prepositions would also make sense. Removing words that are only one or two characters might also make sense 🤔

andrewtavis commented 2 years ago

Also also, I'm wondering if my logic on annotation canceling the plural autocomplete field was misplaced? Maybe the best thing is to keep it all in the first section where the translate button would be, and then we can just do ... if the word is long. The user knows what word they just typed, so they don't need to see the whole thing. See designs below:

Autocomplete

Seems better to me 🤔

SaurabhJamadagni commented 2 years ago

Maybe the best thing is to keep it all in the first section where the translate button would be, and then we can just do ... if the word is long.

Yeah @andrewtavis I was wondering the same thing during the hackathon. But because of the time constraint it completely slipped my mind to discuss it. It also seems like unnecessary computation when it has to figure out if the word is short enough to fit in one button or not. We should definitely look into this appraoch where we keep it in just the translate button.

SaurabhJamadagni commented 2 years ago

Also, the wordfreq library that you have linked seems super cool! We could definitely utilise it in autosuggest, rather definitely should 😅

we could also potentially subset a few popular verbs and include their conjugations in the list, and maybe including prepositions would also make sense.

I didn't understand what you mean by this. I am guessing you are referring to the autocompleteWords array that we use to filter words from.

andrewtavis commented 2 years ago

I didn't understand what you mean by this. I am guessing you are referring to the autocompleteWords array that we use to filter words from.

Yes exactly, adding words to autocompleteWords to make sure it's covering most of the use cases :)

andrewtavis commented 2 years ago

We should definitely look into this appraoch where we keep it in just the translate button.

I'll change it when you're done with #195 :) I think that this also adds the potential to allow the user to click on the annotations and have the translate button fire for other features. Specifically what I'm thinking is adding ways to figure out what's the difference if there are two genders on a noun or two cases for a preposition - the user could press that button and then have information displayed to them to help them figure out the context.

Basic example of this is the word "Schild" in German, which is masculine or neutral, and has the meaning shield and sign respectively.

SaurabhJamadagni commented 2 years ago

I think that this also adds the potential to allow the user to click on the annotations and have the translate button fire for other features. Specifically what I'm thinking is adding ways to figure out what's the difference if there are two genders on a noun or two cases for a preposition - the user could press that button and then have information displayed to them to help them figure out the context.

That is a nice idea! This could be a feature that works when the user is online though. Adding this amount of size could surely add up, increasing the size of the app I think. What do you think? 😅

SaurabhJamadagni commented 2 years ago

@andrewtavis, I also noticed a bug showing similar nature as when the keyboard crashes when delete is pressed. It isn't allowing to highlight text and then delete it. To replicate it, try and select a bunch of text and hit the delete on the scribe keyboard. The keyboard just bobs up and down, similar to when the keys would be reloaded. I think the problem which is causing the crash is the same problem that is causing this. It's just a hunch. Didn't create a new issue around it because it is working fine in the app. Just a problem with our current code state.

andrewtavis commented 2 years ago

That is a nice idea! This could be a feature that works when the user is online though. Adding this amount of size could surely add up, increasing the size of the app I think. What do you think? 😅

Yes certainly, but then the goal for later is that the user will be downloading language packs, so this could still work offline potentially :)

andrewtavis commented 2 years ago

To replicate it, try and select a bunch of text and hit the delete on the scribe keyboard.

Thanks for pointing this out, @SaurabhJamadagni! Will have a look when I'm check #195 :)

andrewtavis commented 2 years ago

Also @SaurabhJamadagni, when you have a moment let me know what you think of the current ideas within #194 and if you have questions :) I was thinking that you could do the Swift side implementation, and I can generate the words it'll use based on what we want to build. Obviously just random words would be easiest, but if we can create a commonWordSuggestions array easily enough with Python, then it could definitely be worth it to have this feature be close to optimal from the start 😊

SaurabhJamadagni commented 2 years ago

Sorry @andrewtavis! I didn't have my email notifications on for issue #194. So didn't have an idea you had commented there. I'll check it out soon. I was also wondering if I could tag along for some parts when you do the python side of things. It could be a nice learning opportunity through observing the process and working with different technologies. I have a good grasp of python so I might be able to help out a bit too hopefully 😅

Do let me know whenever and if it's possible :)

andrewtavis commented 2 years ago

100% yes, @SaurabhJamadagni :) We can figure out another time for a call and then I can walk you through the process. You’d of course also be welcome to contribute, so I’ll make some tasks like in here 😊

Big question is, do we do the simple one with a series of most common words that we assign randomly, or do we think that the other method with Wikipedia generated arrays of words that follow common words would be a better option? I don’t think it’ll be crazy amounts of effort to set up, as again I have a lot of the code in other projects 🐍 There definitely will be some waiting involved though for all the calculations 😅

SaurabhJamadagni commented 2 years ago

We can figure out another time for a call and then I can walk you through the process. You’d of course also be welcome to contribute, so I’ll make some tasks like in here 😊

Thank you so much @andrewtavis! I would love that :)

Big question is, do we do the simple one with a series of most common words that we assign randomly, or do we think that the other method with Wikipedia generated arrays of words that follow common words would be a better option?

I commented on issue #194 with my thoughts on this just now. I think the Wikipedia system would give us better results and would be more practical. I still don't quite get how we will implement it as I said in the other comment. But this seems like a good direction to go in for now. We can go back to a frequency based implementation if we face problems of course.

andrewtavis commented 2 years ago

I’ll comment more in the other issue, @SaurabhJamadagni :) Looking forward to talking some Python with you!

andrewtavis commented 2 years ago

Thanks again for all the efforts for this, @SaurabhJamadagni! We're now done with the initial tasks, and I'll take on these:

Once we're done with these we'll be good to go! 🚀

SaurabhJamadagni commented 2 years ago

Awesome! This was a great feature to work on! I am so happy about the progress! 🚀

Fix the bug where delete now crashes the keyboard

Also, let me know if you would like me to try to figure this one out too @andrewtavis. I would be eager to jump on a meeting to try to solve this problem, if needed. Maybe the bouncing of ideas might work like last time. Thanks for reviewing the PR 😊