Conversation
|
you know i havent had a chance to recheck this but I wonder this is still necessary? at the time it was used to help android tv users login because they couldnt even switch to the password field using the remote control. This was before the frontend login screen was updated to be more modern. I do recall we have a bug from a keyboard only user getting stuck due to this one too. |
There was a problem hiding this comment.
Pull request overview
This PR moves the Android TV DPAD_DOWN → TAB navigation workaround from the legacy WebViewActivity key dispatch override into the shared HAWebView composable, so the behavior is applied at the WebView level whenever HAWebView is used.
Changes:
- Removed
dispatchKeyEventoverride (andKeyEventimport) fromWebViewActivity - Added a
WebView.remapDpadDownToTab()helper using anOnKeyListener - Applied the remapping automatically in
HAWebView’sAndroidView.factoryblock and documented the behavior
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| app/src/main/kotlin/io/homeassistant/companion/android/webview/WebViewActivity.kt | Removes the Activity-level DPAD_DOWN workaround, relying on WebView-level handling instead |
| app/src/main/kotlin/io/homeassistant/companion/android/util/compose/webview/HAWebView.kt | Adds and applies DPAD_DOWN → TAB remapping via OnKeyListener, with updated KDoc |
| private fun WebView.remapDpadDownToTab() { | ||
| setOnKeyListener { view, keyCode, event -> | ||
| if (keyCode == KeyEvent.KEYCODE_DPAD_DOWN && event.action == KeyEvent.ACTION_DOWN) { | ||
| view.dispatchKeyEvent(KeyEvent(KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_TAB)) | ||
| true | ||
| } else { | ||
| false | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This introduces new key-handling behavior (DPAD_DOWN -> TAB) but there is no automated test covering it. Please add a unit/Robolectric or Compose UI test that verifies a DPAD_DOWN key event dispatched to the WebView results in a TAB key event being forwarded, to prevent regressions across Android TV and non-TV devices.
Thanks for jumping in I remember discussing about it but I didn't remember what was the outcome .... So maybe it is not relevant anymore. |
TimoPtr
added
the
WebViewActivity replacement
|
As you clicked the button to request a review: did you end up testing whether tab works/is still necessary with the login screen? |
jpelgrom
changed the title
I already did last time and it was working with both. I honestly didn't test again this time. (Know that I remember) I don't know if we should keep that, but at the same I'm not under the impression that it is going to hurt us. |
|
Maybe it won't hurt but if it is no longer necessary removing it means less code to maintain :) |
|
@dshokouhi Do you still have a TV device with remote and are you willing to test whether onboarding can be completed at all, by any chance? 😇 |
Summary
DPAD_DOWN to TAB remapping for Android TV
Checklist