Add file chooser capability to FrontendScreen#6755
Conversation
There was a problem hiding this comment.
Pull request overview
Adds WebView file upload (file chooser) support to the Compose-based FrontendScreen, aligning it with the legacy WebViewActivity behavior by plumbing onShowFileChooser through a custom WebChromeClient into Compose via a pending request flow.
Changes:
- Extend
HAWebChromeClientwith anonShowFileChoosercallback hook. - Add
pendingFileChooserstate inFrontendViewModeland handle it inFrontendScreenvia a newFileChooserEffect. - Add unit tests for
HAWebChromeClientandFrontendViewModelfile chooser behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| app/src/main/kotlin/io/homeassistant/companion/android/util/HAWebChromeClient.kt | Adds a hook to intercept and delegate WebView file chooser requests |
| app/src/main/kotlin/io/homeassistant/companion/android/frontend/FrontendViewModel.kt | Stores pending file chooser requests and exposes them via StateFlow |
| app/src/main/kotlin/io/homeassistant/companion/android/frontend/filechooser/FileChooserEffect.kt | Implements Compose-side activity result handling for the file chooser |
| app/src/main/kotlin/io/homeassistant/companion/android/frontend/FrontendScreen.kt | Wires the pending file chooser flow into the screen and triggers the effect |
| app/src/test/kotlin/io/homeassistant/companion/android/util/HAWebChromeClientTest.kt | Adds unit coverage for HAWebChromeClient.onShowFileChooser delegation behavior |
| app/src/test/kotlin/io/homeassistant/companion/android/frontend/FrontendViewModelTest.kt | Adds unit coverage for pendingFileChooser set/clear behavior |
86b145c to
c7f1ab2
Compare
There was a problem hiding this comment.
Pull request overview
Adds WebView file upload handling to the Compose-based FrontendScreen (Frontend V2) by wiring WebChromeClient.onShowFileChooser into a queued, suspendable request/response flow, reusing the existing single-slot queue pattern already used for dialogs/permissions.
Changes:
- Added
SingleSlotQueue.awaitResult()to support request/response-style “emit then await callback result” interactions, and refactoredFrontendDialogManagerto use it. - Implemented file chooser request handling (
FileChooserManager+FileChooserEffect) and integrated it intoFrontendViewModel/FrontendScreen. - Added/updated unit tests covering the new queue behavior, WebChromeClient callback plumbing, and file chooser manager/viewmodel behavior.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| common/src/main/kotlin/io/homeassistant/companion/android/common/util/SingleSlotQueue.kt | Adds awaitResult() helper for request/response queue interactions |
| common/src/test/kotlin/io/homeassistant/companion/android/common/util/SingleSlotQueueTest.kt | Adds unit tests validating awaitResult() behavior (resolve, cancel, FIFO) |
| app/src/main/kotlin/io/homeassistant/companion/android/util/HAWebChromeClient.kt | Adds onShowFileChooser callback plumbing |
| app/src/test/kotlin/io/homeassistant/companion/android/util/HAWebChromeClientTest.kt | Adds unit tests for file chooser callback behavior |
| app/src/main/kotlin/io/homeassistant/companion/android/frontend/filechooser/FileChooserManager.kt | Introduces a ViewModel-scoped manager exposing a pending file chooser request via StateFlow |
| app/src/main/kotlin/io/homeassistant/companion/android/frontend/filechooser/FileChooserEffect.kt | Adds Compose effect that launches the system picker and returns results to the pending request |
| app/src/test/kotlin/io/homeassistant/companion/android/frontend/filechooser/FileChooserManagerTest.kt | Adds unit tests for manager queuing, completion, and cancellation |
| app/src/main/kotlin/io/homeassistant/companion/android/frontend/dialog/FrontendDialogManager.kt | Refactors confirm dialog to use SingleSlotQueue.awaitResult() |
| app/src/main/kotlin/io/homeassistant/companion/android/frontend/FrontendViewModel.kt | Wires WebChromeClient file chooser requests into the file chooser manager and exposes pending state |
| app/src/test/kotlin/io/homeassistant/companion/android/frontend/FrontendViewModelTest.kt | Adds tests verifying pending file chooser exposure and callback delivery |
| app/src/main/kotlin/io/homeassistant/companion/android/frontend/FrontendScreen.kt | Collects pending file chooser state and installs FileChooserEffect in the screen content |
| if (pendingRequest != null) { | ||
| LaunchedEffect(pendingRequest) { | ||
| currentRequest = pendingRequest | ||
| launcher.launch(pendingRequest.fileChooserParams) | ||
| } |
There was a problem hiding this comment.
If launcher.launch(...) throws (e.g., ActivityNotFoundException / registry not ready), the pending request will never receive onResult and the awaiting coroutine will stay suspended, effectively wedging future file-chooser requests. Consider wrapping the launch in a narrow try/catch that (1) delivers a null result via pendingRequest.onResult(null) and (2) clears currentRequest, and optionally logs the failure.
There was a problem hiding this comment.
If it throws today it crashes.
…d/filechooser/FileChooserManager.kt Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
It isn't a state for the view as you pointed out, and fits in nicely with the other pending things from the webview triggering actions/effects outside the view 👍 |
jpelgrom
left a comment
There was a problem hiding this comment.
Tested and confirmed, approach seems good also combined with the other PR stacked on top.
One step closer to completion...
Summary
Add file chooser capability to the
FrontendScreen, I added a newStateFlowexposing the Pending request.Checklist
Screenshots
Screen_recording_20260422_165637.mp4
Any other notes
I struggle between creating a new StateFlow and adding this to the ViewState. @jpelgrom any opinion here? Using the ViewState would avoid exposing yet another StateFlow but at the same time it is not really relevant to the state.