Introcude Improv onboarding in FrontendScreen and refactor logic#6865
Open
TimoPtr wants to merge 9 commits into
Open
Introcude Improv onboarding in FrontendScreen and refactor logic#6865TimoPtr wants to merge 9 commits into
TimoPtr wants to merge 9 commits into
Conversation
50 tasks
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors Improv Wi-Fi onboarding into the frontend architecture, moving repository/UI ownership under frontend.improv, adding an orchestrator layer, and wiring the flow into FrontendScreen/FrontendViewModel.
Changes:
- Replaces the legacy Improv repository API with lifecycle-driven Flow operations and Hilt bindings.
- Adds frontend Improv orchestration, UI state, bottom-sheet UI, permission prompting, and scan lifecycle handling.
- Adds unit and screenshot coverage for repository, orchestrator, permissions, ViewModel, and UI states.
Reviewed changes
Copilot reviewed 33 out of 99 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
app/src/test/kotlin/io/homeassistant/companion/android/webview/WebViewPresenterImplTest.kt |
Updates test import to the new Improv repository package. |
app/src/test/kotlin/io/homeassistant/companion/android/frontend/permissions/PermissionManagerTest.kt |
Adds Improv permission flow tests. |
app/src/test/kotlin/io/homeassistant/companion/android/frontend/improv/ImprovRepositoryImplTest.kt |
Adds repository tests for permissions, scanning, and provisioning events. |
app/src/test/kotlin/io/homeassistant/companion/android/frontend/improv/FrontendImprovOrchestratorTest.kt |
Adds orchestrator tests for scan, configure, provision, dismiss, and retry flows. |
app/src/test/kotlin/io/homeassistant/companion/android/frontend/FrontendViewModelTest.kt |
Adds ViewModel coverage for Improv events and state forwarding. |
app/src/test/kotlin/io/homeassistant/companion/android/frontend/FrontendScreenTest.kt |
Updates permission manager construction for new dependency. |
app/src/screenshotTest/kotlin/io/homeassistant/companion/android/frontend/FrontendScreenImprovScreenshotTest.kt |
Adds screenshot coverage for Improv bottom sheets and rationale UI. |
app/src/main/kotlin/io/homeassistant/companion/android/webview/WebViewPresenterImpl.kt |
Updates legacy WebView Improv integration to use the new repository API. |
app/src/main/kotlin/io/homeassistant/companion/android/improv/ui/ImprovSheetView.kt |
Removes the legacy Improv sheet implementation. |
app/src/main/kotlin/io/homeassistant/companion/android/improv/ui/ImprovSheetState.kt |
Removes the legacy sheet state model. |
app/src/main/kotlin/io/homeassistant/companion/android/improv/ui/ImprovSetupDialog.kt |
Reworks legacy dialog host to use the new Improv UI state/repository. |
app/src/main/kotlin/io/homeassistant/companion/android/improv/ui/ImprovPermissionView.kt |
Removes the legacy permission rationale composable. |
app/src/main/kotlin/io/homeassistant/companion/android/improv/ui/ImprovPermissionDialog.kt |
Reuses the new permission rationale UI in the legacy dialog host. |
app/src/main/kotlin/io/homeassistant/companion/android/improv/ImprovRepositoryImpl.kt |
Removes the old repository implementation. |
app/src/main/kotlin/io/homeassistant/companion/android/improv/ImprovRepository.kt |
Removes the old repository interface. |
app/src/main/kotlin/io/homeassistant/companion/android/improv/ImprovModule.kt |
Removes the old Hilt binding. |
app/src/main/kotlin/io/homeassistant/companion/android/frontend/permissions/PermissionRequest.kt |
Adds an Improv-specific permission request type. |
app/src/main/kotlin/io/homeassistant/companion/android/frontend/permissions/PermissionManager.kt |
Adds Improv permission request orchestration and rationale cap handling. |
app/src/main/kotlin/io/homeassistant/companion/android/frontend/permissions/PendingPermissionHandler.kt |
Extracts permission request routing into a reusable composable. |
app/src/main/kotlin/io/homeassistant/companion/android/frontend/permissions/ImprovPermissionPrompt.kt |
Adds frontend Improv permission prompt handling. |
app/src/main/kotlin/io/homeassistant/companion/android/frontend/improv/ui/ImprovSheetView.kt |
Adds new Improv onboarding sheet body using the design system. |
app/src/main/kotlin/io/homeassistant/companion/android/frontend/improv/ui/ImprovPermissionView.kt |
Adds new permission rationale UI. |
app/src/main/kotlin/io/homeassistant/companion/android/frontend/improv/ui/ImprovOverlay.kt |
Adds the frontend bottom-sheet overlay for active Improv sessions. |
app/src/main/kotlin/io/homeassistant/companion/android/frontend/improv/ProvisioningEvent.kt |
Adds provisioning event model. |
app/src/main/kotlin/io/homeassistant/companion/android/frontend/improv/ImprovUIState.kt |
Adds Improv UI state machine model. |
app/src/main/kotlin/io/homeassistant/companion/android/frontend/improv/ImprovRepositoryImpl.kt |
Adds new Flow-based Improv repository implementation. |
app/src/main/kotlin/io/homeassistant/companion/android/frontend/improv/ImprovRepository.kt |
Adds new Improv repository interface. |
app/src/main/kotlin/io/homeassistant/companion/android/frontend/improv/ImprovManagerFactory.kt |
Adds factory abstraction for ImprovManager. |
app/src/main/kotlin/io/homeassistant/companion/android/frontend/improv/FrontendImprovOrchestrator.kt |
Adds frontend orchestration for scan, setup, navigation, and UI state. |
app/src/main/kotlin/io/homeassistant/companion/android/frontend/improv/FrontendImprovModule.kt |
Updates Hilt bindings for the new Improv architecture. |
app/src/main/kotlin/io/homeassistant/companion/android/frontend/FrontendViewState.kt |
Adds Improv UI state to frontend content state. |
app/src/main/kotlin/io/homeassistant/companion/android/frontend/FrontendViewModel.kt |
Wires Improv events, state, and actions through the ViewModel. |
app/src/main/kotlin/io/homeassistant/companion/android/frontend/FrontendScreen.kt |
Renders Improv overlay and lifecycle-bound scan processing. |
Comment on lines
+301
to
+303
| private suspend fun forwardDiscoveredDevices() { | ||
| val sentNames = mutableSetOf<String>() | ||
| improvRepository.scanDevices().collect { devices -> |
Comment on lines
+277
to
+288
| is ProvisioningEvent.Provisioned -> { | ||
| _uiState.update { current -> | ||
| // Guard against a Provisioned event arriving after the user dismissed the | ||
| // sheet — overwriting `null` with `Provisioned` would briefly resurrect it. | ||
| if (current is ImprovUIState.Provisioning) { | ||
| ImprovUIState.Provisioned(domain = event.domain) | ||
| } else { | ||
| current | ||
| } | ||
| } | ||
| externalBusRepository.send(ImprovDeviceSetupDoneMessage) | ||
| } |
Contributor
Test Results 263 files 293 suites 7m 14s ⏱️ Results for commit 3120f52. ♻️ This comment has been updated with latest results. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Rework the Improv UI and logic to share it in the new FrontendScreen. I also introduce a new layer called Orchestrator to be able to use Managers without creating inter deps between them. I re-wrote the ImprovRepository to be able to test it and be more lifecycle aware.
This PR might not pass the CI because it adds too many screenshot tests and without the latest fix from Google from the alpha14 we would need to bump the RAM again (I'll do it if we finish to review this PR before the bump of version from Google).
Checklist
Screenshots