andrewtavis commented 2 years ago

Terms

[X] I have searched open and closed feature requests
[X] I agree to follow Scribe-iOS' Code of Conduct

Description

Something that was discovered in the process of doing #188 was that the annotation style needs to be switched with the inclusion of autocomplete and autosuggest. We no longer have the space to display the words with the annotations, so they should now be removed.

The new version of annotations will show them centered over the translate button without the word. This will further open up the possibility for them to be clicked later when we add functionality for explanations of annotations like when there are multiple genders for a noun or multiple cases for a preposition.

Contribution

I’ll be working on this for the v1.5.0 release so that this can be implemented with the new autocomplete and autosuggest functionalities :) Those interested in helping would be welcome 😊

andrewtavis commented 2 years ago

Included in this can also be updating the annotation image for each device so that the new annotation and autosuggest/autocomplete UI is displayed :)

andrewtavis commented 2 years ago

Tasks for this include:

[x] Remove the annotation labels from Keyboard.xib and references to them in KeyboardViewController.swift
[x] Creating classes for annotations from UIButton
[x] Writing a function to determine coordinates of annotations based on how many there are
[x] Allow for noun annotations to be formatted and displayed over the translate button
[x] Allow for preposition annotations to be formatted and displayed over the translate button
[x] Allowing for noun and preposition annotation at the same time (next to each other on the same button)
[x] Sizing the annotations based on the number that are being presented (including texts, and checking iPads)
[x] Creating a function for these annotations being clicked for later, all annotations showing the click animation at the same time when one is clicked
[x] Updating the annotation image for each device on the App Store (should also include "Die" to make it more clear)
- Requires #194 being finished so the suggestions are present with the annotation
- The alpha will need to be removed for the App Store
[x] Updating the dark mode language select image to show autosuggestions
- The alpha will need to be removed for the App Store
[x] Allow for light-dark mode shifting of annotation colors
[x] Making sure that annotation functions are called when needed (after a space, selecting a word and pressing the Scribe key, after auto actions, and after commands)

andrewtavis commented 2 years ago

As discussed, @SaurabhJamadagni, I'll create a branch we can work from and do the first step above so we can work from scratch. Let me know what your feedback on the above steps is! That's just my initial interpretation 😊

SaurabhJamadagni commented 2 years ago

These look good for now @andrewtavis. Maybe we'll figure out something else that needs to be added as we go along these initial steps. 😊

andrewtavis commented 2 years ago

Hi @SaurabhJamadagni 👋 I'm starting to work on this a bit today :) Got Xcode 14 autoinstalled on my computer and was unmotivated to figure out the errors, but it all seems to be working again 😅 Will send along some work later 😊

andrewtavis commented 2 years ago

Alsooooooooooo, @SaurabhJamadagni 🤯🙌🥳 I finally got sick and tired of not being able to debug, and after hours of trying things check this out:

The reason that we've been having issues with the debugger is because we need to be debugging with the target set to the keyboard we're debugging in, not to Scribe (see below):

I'm assuming that you have been trying to debug while running with the target set to Scribe like me? What also makes sense in this is that debugging won't work for a keyboard that's not the current target, so I just switched to the Spanish keyboard and the breakpoint didn't work as I'm running on German atm.

Let me know if this works for you 😊

SaurabhJamadagni commented 2 years ago

I'm starting to work on this a bit today :)

Hey @andrewtavis! I have my exams going on currently. So I'll get started with this in the coming week. I'll check in then :)

SaurabhJamadagni commented 2 years ago

That makes a lot of sense with the debugging. Finally!!!! 🥳

I guess that's why it was very difficult to find an answer online as well as most answers weren't considering multiple targets. I was just using Scribe as the target. I never considered changing the target. This is awesome! This should make so much stuff easier haha 🚀

andrewtavis commented 2 years ago

No stress if you have exams going on, @SaurabhJamadagni :) I'll get done what I can and send along updates as major updates come 😊

andrewtavis commented 2 years ago

And good luck with the exams as well!!! 🚀

andrewtavis commented 2 years ago

@SaurabhJamadagni, I'm making really good progress on this, as you can see from the tasks above. Am holding off on a PR for now so you can focus on your exams 😊 Let me know when you expect to have a bit of time, and maybe we can do a call then to discuss what all is changing with how annotations work :) I'm back to work this week, but my overall goal is to be mostly done with all this issue this week, and then wrap up all of this next week 🙌

SaurabhJamadagni commented 2 years ago

Am holding off on a PR for now so you can focus on your exams 😊 Let me know when you expect to have a bit of time, and maybe we can do a call then to discuss what all is changing with how annotations work

Hey @andrewtavis! I am done with my exams. We can do a call on Wednesday if you are free :) Is it just as big of a headache to implement everything programmatically as we imagined haha? You really did get a big chunk of it done 🤯

andrewtavis commented 2 years ago

Hey @SaurabhJamadagni! Glad to hear exams are done, and hope they went well 😊 Tomorrow I could do a call at say 17:00 UTC for a bit, as I have a dinner at around 18:00. Let me know if that works :)

Was a bit of a headache. I haven't deleted all of the unnecessary functions in anotate.swift yet, but am hoping to get a bit done for merging noun and preposition annotation right now :)

SaurabhJamadagni commented 2 years ago

Tomorrow I could do a call at say 17:00 UTC for a bit, as I have a dinner at around 18:00. Let me know if that works :)

Yeah @andrewtavis, that sounds good! If it's going to be too much of a rush then we can postpone no problem. Otherwise 17:00 UTC today works! See you then! :)

andrewtavis commented 2 years ago

No rush at all, @SaurabhJamadagni! Just sent you an invite :)

SaurabhJamadagni commented 2 years ago

Hey @andrewtavis I'm sorry but can we reschedule to tomorrow or any other day according to your convenience? A thing has come up and I can't guarantee that I'll be back on time. I would hate it if I kept you waiting again 😅

andrewtavis commented 2 years ago

I didn't see this till now. All good :) If you're back within 30 we can still check in. Big thing is that I'll send along a PR for the work that's been done till now. Would be great if you could:

Check it to familiarize yourself with the changes and make sure things make sense from my end
Give feedback on the new UI for annotations :)
Think about what'd need to happen to get the last coding point above finished (the rest is design issues that I'll do in Figma)

Making sure that annotation functions are called when needed

For this the key is breaking up the massive code block I've put into KeyboardViewController.swift after case spaceBar:
As much of it should go into Annotate.swift as possible
Redundancies should be removed where possible

SaurabhJamadagni commented 2 years ago

f you're back within 30 we can still check in. Big thing is that I'll send along a PR for the work that's been done till now.

I came back a whole 2 hours late @andrewtavis. I just knew with the traffic conditions that I would miss 😅

I'll get to the review and share my feedback or questions in the PR thread itself. Thanks for taking down a huge chunk of this issue! I feel so bad for not helping you out! You can expect more from me in the coming few weeks though haha. Looking forward to our conversation sometime this weekend :)

andrewtavis commented 2 years ago

As I said in the issue, you don't need to worry about me hammering this out :) This is me deleting code that never should have been written in the first place 😅 Happy to have this almost done and with it v2.0.0!

Talk to you Sunday 😊

andrewtavis commented 2 years ago

@SaurabhJamadagni, just FYI, I just signed up to Hacktoberfest 2022 😊 Seems like Scribe's already getting some attention, as two iOS issues have already been finished for it, and I have some interest in Android as well. I'll get us added into the other repos that people can find projects through.

Let's chat about this tomorrow 🚀

andrewtavis commented 2 years ago

Making sure that annotation functions are called when needed (after a space, selecting a word and pressing the Scribe key, after auto actions, and after commands)

@SaurabhJamadagni, have merged all the pending PRs, so we're good to get started on this task now 😊

SaurabhJamadagni commented 2 years ago

Hey @andrewtavis, just had one question.

https://github.com/scribe-org/Scribe-iOS/blob/68ddd54b530e7a2e2186b40c1552f93aff703a05/Keyboards/KeyboardsBase/KeyboardViewController.swift#L1566

In this case we are checking for if it's a noun and a preposition. But then we have individual checks for nouns and prepositions as well in else if cases. Is there a particular case for which you had this in mind? What I was thinking of doing is making both isNoun and isPrep checks individual if conditions

if isNoun {
...
...
..
}

if isPrep {
...
...
..
}

This way if the word is both, then both the if cases are triggered, else only the one relevant. As far as I can see the code inside the combined case is just a combination of the individual cases. Is there some particular case that I might be missing?

andrewtavis commented 2 years ago

Hi there, @SaurabhJamadagni :) Sorry the reply's coming later in the day 😊

Separating the checks makes total sense. I honestly was just super tired and at such times tend to write repeated code that I'll combine later 😅 So it'd be:

annotationsToAssign = [String]() // maybe make this a global var?
annotationBtns = [UIButton]()
annotationColors = [UIColor]()
annotationSeparators = [UIView]()

if lastWordTyped == "Scribe" || lastWordTyped == "scribe" {
  annotationState = true
  autoAction1Visible = false
  🥳🥳🥳
} else {
  if isNoun {
    annotationsToAssign.append(annotationForm) // or multiple forms
    annotationColors.append(nounFormToColorDict[annotationsToAssign[i]]!)
  }

  if isPrep {
    activateAnnotationBtn = true
    annotationsToAssign.append(annotationForm) // or multiple forms
    annotationColors.append(UITraitCollection.current.userInterfaceStyle == .light ? .black : .white)
  }

  if annotationsToAssign.count > 0 {
    annotationState = true
    autoAction1Visible = false

    let annotationWidth = annotationFieldWidth / CGFloat(annotationsToAssign.count)
    let numAnnotations = annotationsToAssign.count

    for i in 0..<numAnnotations {
      // Make and assign annotation buttons, etc based on if they're for nouns or prepositions...
    }
  }
}

Let me know if this is around what you were thinking, and thanks for looking into this!! 🙏

SaurabhJamadagni commented 2 years ago

Let me know if this is around what you were thinking

Yes @andrewtavis, something like this. I honestly didn't notice it the first time I saw the code so I am not any better when drowsy either haha. Making the changes. Will have a PR soon 😊

andrewtavis commented 2 years ago

Looking forward, @SaurabhJamadagni!! 😊

andrewtavis commented 2 years ago

Further work for annotations:

[x] Adding screens to the preposition annotation display for pronouns using the 2x2 conjugation view
[x] German noun annotations are firing even if we type the word lowercased

andrewtavis commented 2 years ago

@SaurabhJamadagni, ea54272 fixes the annotation screen that I'd forgotten to add some pronoun conjugations to that need the 2x2 conjugation display 😊 There's a lot of added code in there as well as I set the stage for #218 and #219 :) I decided that I do want to do these for at least German, with the idea being that we can shift between conjugation views as needed to allow a user to find the exact pronoun they need given the gender of the subject, type of pronoun and gender of the object (if applicable). Will be a bit more work, but I want to use this myself, and I also don't like the idea of providing a user with buttons that would insert word/another word seprated by a dash...

I'll setup an enum for the state that the conjugation table's in, which should make all this a lot easier, and then we'll have a few more views at our disposal that we could potentially use for other cool things 😊 Aside from all this it's designs and updating the data (running the script for 10min and being amazed at how much new data has been added over the months we've been working on this 😅).

Closing this one, meaning we're done with all of the main coding tasks for v2.0! Likely will be some fixes going forward, and feel free to test a bit and bring up any issues you pick up. I'll keep you updated as I got for the last issues 🚀 Thanks so much for your efforts!!

SaurabhJamadagni commented 2 years ago

Will be a bit more work, but I want to use this myself, and I also don't like the idea of providing a user with buttons that would insert word/another word seprated by a dash

Yeah @andrewtavis, features we ourselves would use are perfect additions. Could you explain with an example what you mean by providing a user with buttons that would insert word/another word seprated by a dash?

I'll setup an enum for the state that the conjugation table's in, which should make all this a lot easier, and then we'll have a few more views at our disposal that we could potentially use for other cool things 😊 Aside from all this it's designs and updating the data

Sounds good! 🚀

andrewtavis commented 2 years ago

Morning, @SaurabhJamadagni!

Could you explain with an example what you mean by providing a user with buttons that would insert word/another word seprated by a dash?

Lemme give you an example 😊 Also, I meant slash, not dash :)

So within the preposition-case conjugation display we see what pronouns the user should use given the case and what they'd like to talk about. In English again this is the "With whom are you going to the library?" example, which sadly is very much a thing in German (and Russian, but I'm not going to fully implement Russian for v2.0.0 as I need support). Within the conjugation table I don't want to give users more than six options in a 3x2 table, as I think more than that gets too complex, and I think the 1st person singular, first person plural, etc structure is one that people understand. Within that though there are options for formality (du and Sie are "you" formal and informal in German) as well as gender (in German we have er, sie and es for he, she and it, but then we also need to conjugate all kinds pronouns for them too).

Within the table for say the German word "mit", which is "with", we have a couple of fields that are single words, and some words that are separated with slashes if there's more than one option. So hit right once and you see the middle left button is "dir/Ihnen", which is "With you", but for informal and formal. As of now we'd be inserting the string dir/Ihnen into the text proxy, which isn't desirable in my opinion. Rather what's going to happen once #222 and #218 are done is that clicking that will bring up a 1x2 display that will ask the user which formality they want, and then clicking one of the two buttons will insert only that option. Similarly for possessive pronouns that for absolutely no good reason need to be conjugated based on subject AND object (so "my" needs to be conjugated based on what the gender of the thing that's mine AND based on what case you're speaking in 🤦‍♂️🤦‍♂️ 🤦‍♂️ 🤦‍♂️ ), the user will be presented with a display that will ask them what the object is before they make their final selection 🚀

Will need to be worked to perfect it, and maybe we can add some color or symbols into the conjugation display later, but this feature is one that'd save people a lot of headaches 😊

SaurabhJamadagni commented 2 years ago

Will need to be worked to perfect it, and maybe we can add some color or symbols into the conjugation display later, but this feature is one that'd save people a lot of headaches 😊

Ahh, I understand @andrewtavis. Trying to navigate through the cases you explained to me itself was a headache haha. I can definitely see how people would benefit from what you are planning to do. Thanks for the clarification! 😊

I am going to go browse the issues to figure out what to work on next right now. Is there any issue you had in mind for me to work on?

andrewtavis commented 2 years ago

Am open to what you have interest in, @SaurabhJamadagni! I'm hoping to finished up #218 soonish, so maybe wait on starting something till those changes go through, as looking at my local copy there are lots of changes to variable names and other things to make all of this work properly :)

andrewtavis commented 2 years ago

211 is something that I spelled out to the contributor that helped with Russian localizations. Might be helpful for the UI going forward so that someone knows when pressing one of the navigation buttons in the conjugate UI won't do anything (IMO best to just disable the key all together if it's not gonna do anything). To me that should be pretty straightforward for you :) But again, hold off on the upcoming changes, and also feel free to choose what's of interest!

SaurabhJamadagni commented 2 years ago

But again, hold off on the upcoming changes, and also feel free to choose what's of interest!

Cool got it thanks! 😊

scribe-org / Scribe-iOS

Update Annotation Formatting #197

Terms

Description

Contribution