PIA-1058: App navigation setup #51
Conversation
There was a problem hiding this comment.
Great job @kp-laura-sempere! Just a couple of comments to see what you think.
Thank you!
|
|
||
| import Foundation | ||
| import SwiftUI | ||
| import PIALibrary |
There was a problem hiding this comment.
I think since this is presentation, we could avoid leaking framework details like SwiftUI. I'd also try to avoid importing PIALibrary on presentation and domain layers. It's true that we will be removing PIALibrary but the AccountProvider protocol exposes many different methods that this ViewModel does not care about. I'd suggest to have something in between which will only expose the methods the ViewModel will be using.
What do you think?
| // TODO: Update this value from the Vpn OnBoarding installation profile screen | ||
| @AppStorage(.kOnboardingVpnProfileInstalled) var onBoardingVpnProfileInstalled = true | ||
|
|
||
| let accountProvider: AccountProvider |
There was a problem hiding this comment.
I'd suggest the same as the comment above. To have something that hides all the AccountProvider methods and it exposes only what the ViewModel needs.
There was a problem hiding this comment.
Yes, I was thinking the same. I think we can create a AccountProviderType protocol that only contains the methods that the presentation layer needs.
Let me try to add it 👍
RootContainerViewthat is responsible to show the correct flow based on the authentication state:AppRouterthat will be used inside each main flow (Not authenticated and Authenticated flows) inside aNavigationStack:https://developer.apple.com/documentation/swiftui/navigationstack
https://developer.apple.com/documentation/swiftui/migrating-to-new-navigation-types
NOTE: This PR depends on this other PR from PIALibrary:
pia-foss/mobile-ios-library#24