Move cursor for separated verbs #321
Conversation
Moves cursor in between verb + prefix (for verbs with sep. prefixes) or verb + participle (for perf. ind.) after selecting word in conj. menu. To this end, selecting a verb in perf. ind. now writes verb + part. instead of just part. Addressing scribe-org#155
|
Let me get to this early next week, @lucastronova. Sorry for not getting to it sooner :) Has been an crazy busy week. |
| @@ -221,7 +221,7 @@ func returnConjugation(keyPressed: UIButton, requestedForm: String) { | |||
| if wordPressed == invalidCommandMsg { | |||
| proxy.insertText("") | |||
| } else if formsDisplayDimensions == .view3x2 { | |||
| if deConjugationState != .indicativePerfect { | |||
| // if deConjugationState != .indicativePerfect { | |||
There was a problem hiding this comment.
Hey hey! 👋
Just wondering real quick - could the commented-out code be removed if it won't be needed any longer with the new implementation?
There was a problem hiding this comment.
Yes... I forgot to take it out 0_0. Thank you!
| if controllerLanguage == "German" { | ||
| let components = wordToReturn.components(separatedBy: " ") | ||
| if components.count == 2 { | ||
| proxy.adjustTextPosition(byCharacterOffset: (components[1].count + 1) * -1) | ||
| } | ||
| } |
There was a problem hiding this comment.
Also, perhaps a question more for you, @andrewtavis
Was wondering as well - could the functions throughout the code be modified to be more generic/language-agnostic? I'm thinking generally that any functionality that's pretty universal can stay where they are, but for instance, German-specific checks/logic can go under Keyboards/LanguageKeyboards/German/*.swift. Thought of this as I was thinking code may get messy/hard-to-read with language-specific logic sprinkled around, especially as more languages are added over time (and their exceptions and quirks along with them).
Note: This doesn't have to be a part of this PR; can be done later as it might require some refactoring. Just curious on thoughts on the above.
There was a problem hiding this comment.
Met offline with @andrewtavis and @SaurabhJamadagni on the above. There's consensus that refactoring language-specific logic out from the base keyboard should be something Scribe should look to. Created the issue #323 to cover for this. Please anyone feel free to add any thoughts or ideas there! 😄
Also, another good idea could be to keep this refactoring in mind as other work is done throughout the project. If feasible to implement language-specific logic already separated out, do so 🚀
Forgot to remove code commented out for testing..
There was a problem hiding this comment.
@lucastronova, really sorry that this review took so long :( Suffice it to say that a new job, Scribe, another project and on top of it all moving apartments has me bit overburdened atm...
Really appreciate this change 😊 Extremely clean such that I can just see it's going to do exactly what it should, and then testing it locally it just works :) Thanks so much for this contribution. I promise to be a bit more attentive in the future if there are others issues of interest, and really hope that you continue to contribute! The rest of the team and I have been happy to see the initial interest from you :)
Hope you've had a nice weekend!
Contributor checklist
Description
Moves cursor in between verb + prefix (for verbs with sep. prefixes) or verb + participle (for perf. ind.) after selecting word in conj. menu.
To this end, selecting a verb in perf. ind. now writes verb + part. instead of just part.
Tested manually.
Related issue