Add OnboardingPreview #594
Conversation
|
@maciek-rdx I think the CombineReducers was neat, needed if one needs to reset state in between runs. Only needed for this PreviewApp I think. I dont think you ckutter the production code with the code you added in the OnboardingFeature itself, fine by me! |
| private func sendDelegateCompleted(state: State) -> EffectTask<Action> { | ||
| .send(.delegate(.completed(state.newAccount))) | ||
| } |
There was a problem hiding this comment.
I think we should avoid special functions like this, they hide what's going on
There was a problem hiding this comment.
Before that, we've had two places where .send(.delegate(.completed))) has been returned. My goal was to keep them together to eliminate the duplication and centralize the provision of state.newAccount. I don't see it as being super wrong, I pretty much followed this best practice, but fine to revert.
Just let me know @kugel3 👍
Sources/Features/CreateAccount/Coordinator/CreateAccountCoordinator+Reducer.swift
Show resolved
Hide resolved
| @@ -18,7 +18,7 @@ public struct OnboardingCoordinator: Sendable, FeatureReducer { | |||
| } | |||
|
|
|||
| public enum DelegateAction: Sendable, Equatable { | |||
| case completed | |||
| case completed(Profile.Network.Account?) | |||
There was a problem hiding this comment.
Is it possible to complete the onboarding without an account?
There was a problem hiding this comment.
Good question! So we send .completed in two paths:
- Via New Wallet
- Via Restore From Backup
Currently, with @CyonAlexRDX we agreed to focus solely on the New Wallet/Device factor source kind, so the Restore From Backup path will return nil
# Conflicts: # App/BabylonWallet.xcodeproj/project.pbxproj
Jira ticket: no ticket, fine without, likely unless we decide that some testing is necessary. Read more in Notes 👇
Description
This PR brings a preview of the Onboarding feature.
Notes
I implemented it according to what we agreed upon with @CyonAlexRDX. It works, but there is a couple of things that I don't like about it and we can potentially do better:
OnboardingCoordinatorin the private scope of the preview app. That's a bit more demanding I think, in a couple of hours I can make some PoC if that's an idea that we consider worth chasing.CombineReducersand additionalReducethat I'm injecting intoFeaturesPreviewer, look like something potentially reusable in other cases. I didn't want to do it now (let me know if I should), but I guess it makes sense to pull in that behavior insideFeaturesPreviewerand make it useful for other previews as well.PreviewResult.Viewwe currently use just.navigationTitle("Feature Result"). Maybe it makes sense to inject some more meaningful string there (like "New account" in the case of this preview)?How to test
Not sure if testing is necessary... I'd say yes (at least smoke testing of the Onboarding completion paths, as I've had to touch the production code), but on the other hand, I'm wondering how you guys prefer to handle such things.
For now, no testing in a QA sense is necessary, as this is our development infra. If you wanna try it just select the
OnboardingPreviewscheme and cmd+R it.Video
Simulator.Screen.Recording.-.iPhone.14.-.2023-06-28.at.12.09.38.mp4
PR submission checklist