Repository navigation
Add a cancel button to the onboarding device name step - #6067
Conversation
Since the onboarding rewrite pushed this screen with its navigation bar hidden, it had no way out: a save that keeps failing (for example when the server is unreachable) left the user stuck. Show the bar with a Cancel button that ends the auth flow and leaves the screen right away instead of waiting for the token revocation.
The permissions steps are not on a navigation stack, so a choice could not be revised: after picking the most secure local access, the home network step could only be left by entering a network name. Show a back button in the navigation bar from the second step on. Where the flow is opened from the app with a close button, that button moves to the trailing edge.
There was a problem hiding this comment.
🟡 Changes recommended
Delayed callbacks can disrupt a subsequent login or leave home-network navigation unable to advance.
2 open findings
What changed in this PR
Restores exits from the companion app’s onboarding screens after the SwiftUI navigation rewrite.
Changes:
- Adds device-name cancellation and immediate return to the servers list.
- Adds back navigation within permissions and moves the container close button.
- Adds navigation and cancellation tests plus a localized Back label.
| File | Description |
|---|---|
| Tests/App/Onboarding/OnboardingPermissionsNavigationViewModelTests.swift | Tests back navigation and state reset. |
| Tests/App/Auth/OnboardingAuthStepDeviceNaming.test.swift | Tests cancellation guards. |
| Sources/Shared/Resources/Swiftgen/Strings.swift | Adds generated Back accessor. |
| Sources/App/Resources/en.lproj/Localizable.strings | Adds English Back label. |
| Sources/App/Onboarding/Steps/Permissions/OnboardingPermissionsNavigationViewModel.swift | Adds previous-step navigation. |
| Sources/App/Onboarding/Steps/Permissions/OnboardingPermissionsNavigationView.swift | Adds permissions back button. |
| Sources/App/Onboarding/Steps/DeviceName/DeviceNameView.swift | Adds Cancel button. |
| Sources/App/Onboarding/Container/OnboardingNavigationView.swift | Keeps navigation bars visible. |
| Sources/App/Onboarding/API/Steps/OnboardingAuthStepDeviceNaming.swift | Pops authentication immediately on cancellation. |
| Sources/App/Onboarding/API/OnboardingDeviceNameRequest.swift | Renames and documents cancellation API. |
| Sources/App/Container/ContainerView.swift | Moves close button to trailing edge. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
The onboarding was designed to be one direction only to simplify edge cases. I can review this change but it will require testing all possible scenarios, with the conditional screens that we show or hide depending on the user’s setup. On top of that, since this was done in coordination with Android, you will need to open a PR in there as well to keep the feature parity |
A cancelled auth flow settles only after its cleanup, so its result could pop the pages of a flow the user started in the meantime; only the latest flow's result is handled now. A home network save that lands after going back no longer marks the step as done.
|
Makes sense. I opened this because I got stuck myself: I was on VPN to a remote site I wanted to add, picked the location-based option because I thought it meant geofencing, and the next step then had my home Wi-Fi filled in. There was no way back to change my answer, so I had to kill the app in the middle of onboarding. What's your take: should I prepare the same for Android, or do you see this as working as designed? |
|
It's not nice to be locked into a situation like you were so at least for the naming device I would be Ok with, the permissions flow I truly have to spend time testing it to see if there are not gaps, if you could help me out, asking AI to map the possible flows and test if your implementation introduces a regression is also helpful. Perhaps you can split this PR in 2? Then the same for Android |
|
Please also test the secondary server onboard (when you add a second server to the app) |
This pull request keeps only the device name cancel.
|
Split: this PR now only has the device name cancel. The permissions back button is in #6071. |
Cancelling the device name step removed any server stored under the onboarded instance's identifier, though this flow had not saved anything yet.
…ection Webhooks sent during onboarding cache an API for the unsaved server; a failure before saving now evicts only that one. A cancelled flow that settles while a newer selection still waits for discovery no longer resets the newer flow's loading state.
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
…is gone Cancel pops the auth pages long before the cancelled flow settles, so the invitation buttons and the manual URL spinner stayed disabled until token revocation returned. A result arriving after the servers list was released no longer pops a flow started from a new list.
With nothing saved there is nothing to roll back, so revoking the token runs in the background and the flow ends right away. That makes the early pop, the attempt tracking and the loader callback unnecessary, so they are removed again.
|
Tested both, cancel returns right away, retry works, existing server untouched. Videos are in the description. |
bgoncal
left a comment
There was a problem hiding this comment.
Looks good from my side, have you checked android?
|
Android already has a back button |
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (63.15%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #6067 +/- ##
==========================================
+ Coverage 54.81% 55.99% +1.18%
==========================================
Files 1248 1254 +6
Lines 82776 83262 +486
==========================================
+ Hits 45370 46622 +1252
+ Misses 37406 36640 -766
☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
<!-- Thank you for submitting a Pull Request and helping to improve Home Assistant. Please complete the following sections to help the processing and review of your changes. Please do not delete anything from this template. --> ## AI Policy <!-- Please read our AI policy before contributing: https://contributors.home-assistant.io/start/ai-policy/ --> - [x] I have read the [Open Home Foundation AI Policy](https://contributors.home-assistant.io/start/ai-policy/). Select exactly one option that describes AI usage in this contribution: - [ ] I have not used AI for this contribution. - [ ] AI assistance was used for this contribution. - [x] AI fully generated the code for this contribution, but I've reviewed and understood it before submitting and will respond without AI during review. ## Summary <!-- Provide a brief summary of the changes you have made and most importantly what they aim to achieve --> After "Most secure", the home network step had no way back. It now has a Back button in the navigation bar, on this step only. Going back restores the previous security level; a location grant or Wi-Fi save arriving afterwards no longer moves the flow on. The back transition needed a fix: a removed view animates with its last render, so the page being left is redrawn with the back direction first. The direction now lives in the view. Android counterpart: home-assistant/android#7642. Android keeps "Most secure" selected instead of restoring the level. ### Flow map ```mermaid flowchart TD A([Onboarding]) -->|no remote access set up| DIS[Local-only disclaimer] A -->|remote access set up| LOC DIS --> LOC["Location (first server)<br/>Privacy choices (added server)"] LOC -->|server has an HTTPS URL| DONE([Done]) LOC -->|HTTP URLs only| LA[Local access] W([Frontend, HTTP-only server]) -->|location not decided| LOC2[Location] W -->|security level not decided| LA LOC2 --> LA LA -->|Less secure| DONE LA -->|Most secure, location denied| LA LA -->|Most secure, location allowed| HN[Home network] HN -->|Wi-Fi name saved| DONE HN ==>|"Back (this PR)"| LA linkStyle 12 stroke:#03a9f4,stroke-width:3px ``` ### Tested | # | Start | Path | Back shown | Result | |---|---|---|---|---| | 1 | First server, location allowed | Home network → Back → Less secure | Home network only | Less secure | | 2 | First server, location allowed | Home network → Back → Most secure → Wi-Fi name | Home network only | Most secure, SSID saved, no second location prompt | | 3 | First server, location denied | Most secure stays on Local access → Less secure | never | Less secure | | 4 | Second server | Privacy → Local access → Home network → Back → Less secure | Home network only | Second: Less secure, first server unchanged | | 5 | Frontend, location reset | Location → Local access → Home network → Back → Most secure → Wi-Fi name | Home network only, Close still there | Less → Most secure, other server unchanged | Simulator, result checked in the server settings. The new unit tests fail without their change. The E2E test now goes back once, green in light and dark. ## Screenshots <!-- If this is a user-facing change not in the frontend, please include screenshots in light and dark mode. --> | | Light | Dark | |---|---|---| | Onboarding, home network | <img width="270" alt="Home network, light" src="https://github.com/user-attachments/assets/5268aaf1-9cb8-47f5-a147-8699ab8bb2e7" /> | <img width="270" alt="Home network, dark" src="https://github.com/user-attachments/assets/e61fbbcb-e605-4046-98d3-9741f978b2c9" /> | | Frontend entry (Close and Back) | <img width="300" alt="Frontend entry, Close and Back" src="https://github.com/user-attachments/assets/4f9a5015-2389-42cc-9a1b-04d3f8f51b23" /> | | Back and forward again, light: https://github.com/user-attachments/assets/6e36274f-dcfb-4ab4-9d54-551d34ddb6f3 Dark: https://github.com/user-attachments/assets/fb3a53c5-d605-4848-b893-81eaea9cf6cf ## Link to pull request in Documentation repository <!-- Pull requests that add, change or remove functionality must have a corresponding pull request in the Companion App Documentation repository (https://github.com/home-assistant/companion.home-assistant). Please add the number of this pull request after the "#" --> Documentation: home-assistant/companion.home-assistant# ## Any other notes <!-- If there is any other information of note, like if this Pull Request is part of a bigger change, please include it here. --> Split from #6067. --------- Co-authored-by: Bruno Pantaleão Gonçalves <5808343+bgoncal@users.noreply.github.com>


AI Policy
Select exactly one option that describes AI usage in this contribution:
Summary
The device name screen has had no way out since #5178. If saving keeps failing, the app has to be killed.
Tested in the simulator, including the invitation flow.
Screenshots
Device name
device-name-cancel.mp4
Invitation
invitation-cancel-frozen.mp4
Second server
second-server-cancel.mp4
Link to pull request in Documentation repository
Documentation: home-assistant/companion.home-assistant#
Any other notes
Permissions back button: #6071. Android already has a back button here.