[iOS] Keyboard refactor + rotation fix#1347
Conversation
| extension KeymanWebViewController { | ||
| func languageMenuPosition(_ completion: @escaping (CGRect) -> Void) { | ||
| webView.evaluateJavaScript("langMenuPos();") { result, _ in | ||
| webView!.evaluateJavaScript("langMenuPos();") { result, _ in |
There was a problem hiding this comment.
Sometimes you use webView! and others you use webView?. Is this intentional and if so it might be good to clarify why.
There was a problem hiding this comment.
That's a Swift thing - webView! asserts that it's not nil, while webView? has a built-in nil check before continuing with the rest of the line.
There was a problem hiding this comment.
Yep -- it's just not clear to me the reason why sometimes you assert and sometimes you go ahead with a nil check for each of those JS calls.
There was a problem hiding this comment.
OK. There are two cases I see in that file, and they do have a particular reason - they're the setOskWidth/setOskHeight method calls, right? Those two lines actually can be reached before webView is initialized, so they need the nil check. It's also fine if the call doesn't go through, as a state variable is also set with the size information and can be passed along to the webView as part of its initialization, so it's not like the values are being lost.
Without other changes, setting those two checks to use ! instead of ? will actually crash the app.
There was a problem hiding this comment.
Gotcha. Are you happy to add a comment in the source accordingly for the poor future maintenance person? :)
mcdurdin
left a comment
There was a problem hiding this comment.
Rotation looking good. The banner has the wrong aspect ratio on the system keyboard sometimes.
The in-app keyboard rotation is especially smooth. The system keyboard rotation has a little jump at the end to fix its vertical size.
|
Yeah, the system keyboard isn't as consistent about reporting its rotation messages, so I did what I could to smooth it out. |
Fixes #444.
Fixes #1045.
This PR will serve the following purposes:
As #683 already performs a number of tasks related to point 1 above, this PR will be based upon that PR's work until it is merged.
Due to the scale of the work involved, it may be wise to review by commit in some cases. (That said, the line changed count is surprisingly low for this refactor...)
Particularly interesting commits:
034680d
master's current state. It kinda breaks it for the system keyboard, though, but will serve well for further work in constraint-based rotation.9d7da15
Managerfrom theKeymanWebDelegateindirection control flow.KeymanWebViewControllernow directly makes calls to the 'first responder.'KeymanWebDelegateand its usages, though.3f2c59c - a massive refactor that transforms the old system keyboard's base into a unified in-app/system keyboard object.
InputViewControlleras the owner and maintainer of theKeymanWebViewController.InputViewControllercan fulfill that role!KeymanWebDelegate, directly processing all text commands rather than relying on extra supporting code for TextViews and TextFields.InputViewControlleris more generally usable, it can now be used via theinputViewControllerproperty forTextViewandTextField.b9eeab6
e4c6ec6
InputViewControllerwasn't actually making its 'base' view correctly, as it's supposed to setinputViewrather thanviewand use aUIInputView.