scribe-org/Scribe-iOS

User pronoun selection paths in the case conjugation display

andrewtavis opened this issue · 10 comments

andrewtavis commented

Terms

Description

With #210 being finished, one final issue is that if there is more than one pronoun in the field, that all options are inserted with the slashes, which is a problematic user flow. One way to solve this is that when a user presses something with more than one possible pronoun, they'll be taken to the 2x2 or a 3x1 conjugation display (to allow for spacing for the words) where they can select the annotation that fits the situation.

Contribution

This would be a good added feature for a future release after v2.0.0, but could be included to make sure that this feature is in a usable state from the start :)

andrewtavis commented

Included in this would also be removing the pronoun ending marker on possessive pronoun displays (the letters in parentheses after the title). Instead the user will just be prompted with which ending to choose in the 2x2 conjugation menu after.

  • Check when done
andrewtavis commented
  • Note that this is blocked by #222 as we need the different views :)
andrewtavis commented

MVP for this is just going to do the journey for German and not Russian, which can be added at a later time :)

andrewtavis commented

For this issue conjugateAlternateView needs to be made into an enum with the available layout options :)

  • Check when done
andrewtavis commented

Closed in c2bbf4a 🚀🙌

andrewtavis commented

@SaurabhJamadagni, let me give you a run through of what happened in c2bbf4a. Sorry that I didn't do a PR on it, but then again I was just kind of changing things at will, and LOTS of things would have been needed to be looked at. Everything that was happening was within conjugation, which is still working fine, so I just wanted to save a potentially major headache for the review :)

What c2bbf4a did:

  • Renamed the pronoun case variables using the word "declension", which I remembered is what the conjugation of a pronoun is called
  • I renamed the variables that are used for conjugation and declension to "form", so now it's formFPS instead of conjFPS for what should be assigned to the top left of the 3x2 view
  • Removed individual label variables for the conjugation view and put them all in a dictionary formLabelsDict
    • What this allows us to do is clear them all by just iterating over the keys and setting them to "", which is necessary now that we have declension views switching to other declension views that have different dimensions (the labels of the old one would still show up)
  • returnDeclension is a massive function to cycle through declension views and trigger another loadKeys as well as return a desired pronoun if there's only one word in the field
    • This function is called from within returnConjugation if commandState == .selectCaseDeclension
  • formsDisplayDimensions is an enum that determines how many cells are in the conjugation/declension view, and replaces a boolean that said that the "alternate view" (2x2) for conjugation should be shown
  • There's also some cleanup and comments 😊

This was the last coding task for v2.0.0! I'm gonna chill and work on something else for the rest of the day, but before next weekend I'll be sure to make the new videos, do the data update, and then send it off!! 🚀 I'll also do some testing in the coming days just to make sure that everything's ok, which you're also welcome to do 😊

Hope you're having a nice weekend!

  • 🚀1
andrewtavis commented

And @SaurabhJamadagni, just something that I'm realizing, it seems that getDefaultAutosuggestions is being called in cases when it shouldn't. I'm seeing them a lot more than I expect to, and specifically after I type a word with an annotation it's very regular that the first two defaults will be the ones that are shown after the annotation.

Example:

  • Type "Mit" and press space and you'll see the correct autosuggestions for it "dem" and "der" after the annotation
  • Delete once
  • Press space again and rather than "dem" and "der" you see "ich" and "die", the first two default autosuggestions for German

Should we open an issue for this? You're welcome to look into it if you'd like, as I expect it's just that some states are being crossed still :)

andrewtavis commented

@SaurabhJamadagni, committing 1ff0c2a to try to fix the issue with getDefaultAutosuggestions. It seems to work at this point 😊

What that changed:

  • I got rid of secondaryPastStringOnDelete as to me it seemed like we could more easily get the word we want by selecting the second to last word in the proxy when separating by spaces
  • The function clearCommandBar seemed to be a function that was from the past UI, so I removed it (had been considering this for a bit)
  • Added some lines to remove the annotation even if the user presses space (had been buggy as the keyboard wasn't loading again, so it'd just stay)
  • Made it so that deleting back to a space will autosuggest (if a user had multiple spaces and deleted one, then it'd try to complete and just present the first three words alphabetically)
  • Other minor changes :)

Coding wise it all seems ok to me at this point! Let me know what you think 😊

SaurabhJamadagni commented

Sorry that I didn't do a PR on it, but then again I was just kind of changing things at will, and LOTS of things would have been needed to be looked at.

No worries @andrewtavis!

This was the last coding task for v2.0.0! I'm gonna chill and work on something else for the rest of the day, but before next weekend I'll be sure to make the new videos, do the data update, and then send it off!! 🚀 I'll also do some testing in the coming days just to make sure that everything's ok, which you're also welcome to do 😊

Sounds awesome! Yup I'll test a little bit too! Have a chill rest of the weekend 😊

  • 😄1
SaurabhJamadagni commented

I got rid of secondaryPastStringOnDelete as to me it seemed like we could more easily get the word we want by selecting the second to last word in the proxy when separating by spaces

Yes that's perfect. I planned to do that myself anyway. The seperating words by the space seperator into an array is a much better method.

Coding wise it all seems ok to me at this point! Let me know what you think 😊

All looks good @andrewtavis! Testing will be the key now I think. Thanks for fixing this one! Looking forward to v2.0.0!! 🚀 😊

  • 🚀1