Alpha application #4
Conversation
| if (Session.getInstance().url.isNullOrBlank()) { | ||
| startActivity(OnboardingActivity.newInstance(this)) | ||
| } else { | ||
| startActivity(WebViewActivity.newInstance(this)) |
There was a problem hiding this comment.
For future, maybe a good idea to fire off token valid check here? That way we don't have to wait until the frontend requests a token but can make sure it's valid.
There was a problem hiding this comment.
Yes agreed. Could we enable Github Project to list all the task ?
like listen connection-status and display authentication screen again in case of "auth-invalid" | "disconnected"
app/src/main/java/io/homeassistant/android/api/AuthenticationService.kt
Outdated
Show resolved
Hide resolved
| webView.post { | ||
| webView.evaluateJavascript( | ||
| "${JSONObject(callback).get("callback")}(true, {\n" + | ||
| " \"access_token\": \"${token.accessToken}\",\n" + |
There was a problem hiding this comment.
This seems like it is hardcoding the tokens from the first launch. However, if the token is expired in the frontend, this code should refresh the token before giving it to the app.
There was a problem hiding this comment.
I noticed token is refreshed if necessary when we open the webview. This will work ok if the user is not planning to use the app longer than that the token is currently valid. Better if we refresh it in this function, if necessary.
Ideally, it would work like this:
- the moment the app starts up and it has tokens, start a refresh request if they are no longer valid
- in the JavaScript interface, if a refresh request is in progress (because app got opened), wait for that request to finish, otherwise start own request for refresh.
There was a problem hiding this comment.
To make it quicker I refresh the token (if needed) each time getExternalAuth is call. Is that ok for you ?
app/src/main/java/io/homeassistant/android/webview/WebViewActivity.kt
Outdated
Show resolved
Hide resolved
|
Noticed you are using findViewById in a few places. I know there isn't going to be a lot of native UI in the app to start with, but might want to look at using Kotlin Synthetic properties. basically would let you replace, for example with no need to implicitly provide the view type. Just a thought. |
|
@w1ll1am23 As you say we won't have a lot of native UI component. |
|
@balloob I will fix these comment |
Check token each time the frontend hit getExternalAuth Use Android client id
|
@CedrickFlocon it was just a thought, no reason to actually implement it if there are no plans to add additional native UI. I could see the settings menu getting pretty full, but once that happens we can revisit. Thanks for all the hard work here! Looks great. |
|
@CedrickFlocon - I'm no android dev, but when there will be possibility to join alpha/beta test, please provide way for people interested in. :-) |
|
@andriej Sure, I am working on a CI / CD stuff. |
No description provided.