generated from bitwarden/template
-
Notifications
You must be signed in to change notification settings - Fork 34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโll occasionally send you account related emails.
Already on GitHub? Sign in to your account
BIT-155: Adds the landing screen and Auth module #20
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
b9e7d47
BIT-155 Adds the landing screen and auth module
nathan-livefront 1ffdde3
BIT-155 Fixes a build error
nathan-livefront ddf5480
BIT-155 Adds a test for AuthCoordinatorโs rootNavigator
nathan-livefront c40bafe
BIT-155 Makes rootNavigator non-optional in AuthCoordinator.init
nathan-livefront File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,82 @@ | ||
import SwiftUI | ||
import UIKit | ||
|
||
// MARK: - AuthCoordinator | ||
|
||
/// A coordinator that manages navigation in the authentication flow. | ||
/// | ||
internal final class AuthCoordinator: Coordinator { | ||
// MARK: Properties | ||
|
||
/// The root navigator used to display this coordinator's interface. | ||
weak var rootNavigator: (any RootNavigator)? | ||
|
||
/// The stack navigator that is managed by this coordinator. | ||
var stackNavigator: StackNavigator | ||
|
||
// MARK: Initialization | ||
|
||
/// Creates a new `AuthCoordinator`. | ||
/// | ||
/// - Parameters: | ||
/// - rootNavigator: The root navigator used to display this coordinator's interface. | ||
/// - stackNavigator: The stack navigator that is managed by this coordinator. | ||
/// | ||
init( | ||
rootNavigator: RootNavigator, | ||
stackNavigator: StackNavigator | ||
) { | ||
self.rootNavigator = rootNavigator | ||
self.stackNavigator = stackNavigator | ||
} | ||
|
||
// MARK: Methods | ||
|
||
func navigate(to route: AuthRoute, context: AnyObject?) { | ||
switch route { | ||
case .createAccount: | ||
showCreateAccount() | ||
case .landing: | ||
showLanding() | ||
case .login: | ||
showLogin() | ||
case .regionSelection: | ||
showRegionSelection() | ||
} | ||
} | ||
|
||
func start() { | ||
rootNavigator?.show(child: stackNavigator) | ||
} | ||
|
||
// MARK: Private Methods | ||
|
||
/// Shows the create account screen. | ||
private func showCreateAccount() { | ||
let view = Text("Create Account") | ||
stackNavigator.push(view, animated: UI.animated) | ||
} | ||
|
||
/// Shows the landing screen. | ||
private func showLanding() { | ||
let processor = LandingProcessor( | ||
coordinator: asAnyCoordinator(), | ||
state: LandingState() | ||
) | ||
let store = Store(processor: processor) | ||
let view = LandingView(store: store) | ||
stackNavigator.push(view, animated: UI.animated) | ||
} | ||
|
||
/// Shows the login screen. | ||
private func showLogin() { | ||
let view = Text("Login") | ||
stackNavigator.push(view, animated: UI.animated) | ||
} | ||
|
||
/// Shows the region selection screen. | ||
private func showRegionSelection() { | ||
let view = Text("Region") | ||
stackNavigator.push(view, animated: UI.animated) | ||
} | ||
} |
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,84 @@ | ||
import SwiftUI | ||
import XCTest | ||
|
||
@testable import BitwardenShared | ||
|
||
// MARK: - AuthCoordinatorTests | ||
|
||
class AuthCoordinatorTests: BitwardenTestCase { | ||
// MARK: Properties | ||
|
||
var rootNavigator: MockRootNavigator! | ||
var stackNavigator: MockStackNavigator! | ||
var subject: AuthCoordinator! | ||
|
||
// MARK: Setup & Teardown | ||
|
||
override func setUp() { | ||
super.setUp() | ||
rootNavigator = MockRootNavigator() | ||
stackNavigator = MockStackNavigator() | ||
subject = AuthCoordinator( | ||
rootNavigator: rootNavigator, | ||
stackNavigator: stackNavigator | ||
) | ||
} | ||
|
||
override func tearDown() { | ||
super.tearDown() | ||
rootNavigator = nil | ||
stackNavigator = nil | ||
subject = nil | ||
} | ||
|
||
// MARK: Tests | ||
|
||
/// `navigate(to:)` with `.createAccount` pushes the create account view onto the stack navigator. | ||
func test_navigate_createAccount() { | ||
subject.navigate(to: .createAccount) | ||
|
||
// Placeholder assertion until the create account screen is added: BIT-157 | ||
XCTAssertTrue(stackNavigator.actions.last?.view is Text) | ||
} | ||
|
||
/// `navigate(to:)` with `.landing` pushes the landing view onto the stack navigator. | ||
func test_navigate_landing() { | ||
subject.navigate(to: .landing) | ||
XCTAssertTrue(stackNavigator.actions.last?.view is LandingView) | ||
} | ||
|
||
/// `navigate(to:)` with `.login` pushes the login view onto the stack navigator. | ||
func test_navigate_login() { | ||
subject.navigate(to: .login) | ||
|
||
// Placeholder assertion until the login screen is added: BIT-83 | ||
XCTAssertTrue(stackNavigator.actions.last?.view is Text) | ||
} | ||
|
||
/// `navigate(to:)` with `.regionSelection` pushes the region selection view onto the stack navigator. | ||
func test_navigate_regionSelection() { | ||
subject.navigate(to: .regionSelection) | ||
|
||
// Placeholder assertion until the region selection screen is added: BIT-268 | ||
XCTAssertTrue(stackNavigator.actions.last?.view is Text) | ||
} | ||
|
||
/// `rootNavigator` uses a weak reference and does not retain a value once the root navigator has been erased. | ||
func test_rootNavigator_resetWeakReference() { | ||
var rootNavigator: MockRootNavigator? = MockRootNavigator() | ||
subject = AuthCoordinator( | ||
rootNavigator: rootNavigator!, | ||
stackNavigator: stackNavigator | ||
) | ||
XCTAssertNotNil(subject.rootNavigator) | ||
|
||
rootNavigator = nil | ||
XCTAssertNil(subject.rootNavigator) | ||
} | ||
|
||
/// `start()` presents the stack navigator within the root navigator. | ||
func test_start() { | ||
subject.start() | ||
XCTAssertIdentical(rootNavigator.navigatorShown, stackNavigator) | ||
} | ||
} |
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
import UIKit | ||
|
||
// MARK: - AuthModule | ||
|
||
/// An object that builds coordinators for the auth flow. | ||
@MainActor | ||
public protocol AuthModule { | ||
/// Initializes a coordinator for navigating between `AuthRoute`s. | ||
/// | ||
/// - rootNavigator: The root navigator used to display this coordinator's interface. | ||
/// - stackNavigator: The stack navigator that will be used to navigate between routes. | ||
/// - Returns: A coordinator that can navigate to `AuthRoute`s. | ||
/// | ||
func makeAuthCoordinator( | ||
rootNavigator: RootNavigator, | ||
stackNavigator: StackNavigator | ||
) -> AnyCoordinator<AuthRoute> | ||
} | ||
|
||
// MARK: - DefaultAppModule | ||
|
||
extension DefaultAppModule: AuthModule { | ||
public func makeAuthCoordinator( | ||
rootNavigator: RootNavigator, | ||
stackNavigator: StackNavigator | ||
) -> AnyCoordinator<AuthRoute> { | ||
AuthCoordinator( | ||
rootNavigator: rootNavigator, | ||
stackNavigator: stackNavigator | ||
).asAnyCoordinator() | ||
} | ||
} |
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
// MARK: - AuthRoute | ||
|
||
/// A route to a specific screen in the authentication flow. | ||
public enum AuthRoute: Equatable { | ||
/// A route to the create account screen. | ||
case createAccount | ||
|
||
/// A route to the landing screen. | ||
case landing | ||
|
||
/// A route to the login screen. | ||
case login | ||
|
||
/// A route to the region selection screen. | ||
case regionSelection | ||
} |
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
// MARK: - LandingAction | ||
|
||
/// Actions that can be processed by a `LandingProcessor`. | ||
enum LandingAction { | ||
/// The continue button was pressed. | ||
case continuePressed | ||
|
||
/// The create account button was pressed. | ||
case createAccountPressed | ||
|
||
/// The value for the email was changed. | ||
case emailChanged(String) | ||
|
||
/// The region button was pressed. | ||
case regionPressed | ||
|
||
/// The value for the remember me toggle was changed. | ||
case rememberMeChanged(Bool) | ||
} |
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
import Combine | ||
|
||
// MARK: - LandingProcessor | ||
|
||
/// The processor used to manage state and handle actions for the landing screen. | ||
/// | ||
class LandingProcessor: StateProcessor<LandingState, LandingAction, Void> { | ||
// MARK: Private Properties | ||
|
||
/// The coordinator that handles navigation. | ||
private let coordinator: AnyCoordinator<AuthRoute> | ||
|
||
// MARK: Initialization | ||
|
||
/// Creates a new `LandingProcessor`. | ||
/// | ||
/// - Parameters: | ||
/// - coordinator: The coordinator that handles navigation. | ||
/// - state: The initial state of the processor. | ||
/// | ||
init(coordinator: AnyCoordinator<AuthRoute>, state: LandingState) { | ||
self.coordinator = coordinator | ||
super.init(state: state) | ||
} | ||
|
||
// MARK: Methods | ||
|
||
override func receive(_ action: LandingAction) { | ||
switch action { | ||
case .continuePressed: | ||
coordinator.navigate(to: .login) | ||
case .createAccountPressed: | ||
coordinator.navigate(to: .createAccount) | ||
case let .emailChanged(newValue): | ||
state.email = newValue | ||
case .regionPressed: | ||
coordinator.navigate(to: .regionSelection) | ||
case let .rememberMeChanged(newValue): | ||
state.isRememberMeOn = newValue | ||
} | ||
} | ||
} |
67 changes: 67 additions & 0 deletions
67
BitwardenShared/UI/Auth/Landing/LandingProcessorTests.swift
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
import XCTest | ||
|
||
@testable import BitwardenShared | ||
|
||
// MARK: - LandingProcessorTests | ||
|
||
class LandingProcessorTests: BitwardenTestCase { | ||
// MARK: Properties | ||
|
||
var coordinator: MockCoordinator<AuthRoute>! | ||
var subject: LandingProcessor! | ||
|
||
// MARK: Setup & Teardown | ||
|
||
override func setUp() { | ||
super.setUp() | ||
coordinator = MockCoordinator<AuthRoute>() | ||
|
||
let state = LandingState() | ||
subject = LandingProcessor( | ||
coordinator: coordinator.asAnyCoordinator(), | ||
state: state | ||
) | ||
} | ||
|
||
override func tearDown() { | ||
super.tearDown() | ||
coordinator = nil | ||
subject = nil | ||
} | ||
|
||
// MARK: Tests | ||
|
||
/// `receive(_:)` with `.continuePressed` navigates to the login screen. | ||
func test_receive_continuePressed() { | ||
subject.receive(.continuePressed) | ||
XCTAssertEqual(coordinator.routes.last, .login) | ||
} | ||
|
||
/// `receive(_:)` with `.createAccountPressed` navigates to the create account screen. | ||
func test_receive_createAccountPressed() { | ||
subject.receive(.createAccountPressed) | ||
XCTAssertEqual(coordinator.routes.last, .createAccount) | ||
} | ||
|
||
/// `receive(_:)` with `.emailChanged` updates the state to reflect the changes. | ||
func test_receive_emailChanged() { | ||
XCTAssertEqual(subject.state.email, "") | ||
|
||
subject.receive(.emailChanged("[email protected]")) | ||
XCTAssertEqual(subject.state.email, "[email protected]") | ||
} | ||
|
||
/// `receive(_:)` with `.regionPressed` navigates to the region selection screen. | ||
func test_receive_regionPressed() { | ||
subject.receive(.regionPressed) | ||
XCTAssertEqual(coordinator.routes.last, .regionSelection) | ||
} | ||
|
||
/// `receive(_:)` with `.emailChanged` updates the state to reflect the changes. | ||
func test_receive_rememberMeChanged() { | ||
XCTAssertFalse(subject.state.isRememberMeOn) | ||
|
||
subject.receive(.rememberMeChanged(true)) | ||
XCTAssertTrue(subject.state.isRememberMeOn) | ||
} | ||
} |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
๐ค Should tests be added with
rootNavigator
beingnil
? Not sure which would be the real case where theAuthCoordinator
receives anil
root value but if I'm not mistaken it's currently feasible.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch! That is something that is currently feasible, so I'll add tests for that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On second thought, I'm going to changerootNavigator
to not be optional. I can't think of a real-world use case for it beingnil
, since the auth flow will always need to be displayed within another context. So I'll update this property and not add any tests!Scratch all this ๐คฆโโ๏ธ
rootNavigator
is optional because the variable isweak
, I've added a test that validates thatAuthCoordinator
does not retain a strong reference torootNavigator
!There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh that makes perfect sense, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one final question @nathan-livefront, is it possible that if the
weak
reference is lost, the caller can executefunc navigate(to route: AuthRoute, context: AnyObject?)
and if so, would you foresee any problems? I guess thestackNavigator
would be still in memory so the action could be executed but nothing will be seen by the user given that I assume therootNavigator
won't be in screen if the reference has been lost.Does it makes sense to add tests for when
rootNavigator
isnil
forstart
andnavigate
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While it is possible for the caller to execute the
navigate
andstart
methods on an instance ofAuthCoordinator
whilerootNavigator
isnil
, I don't think it's necessarily worth testing in this case.rootNavigator
isweak
because therootNavigator
is a property of the parent that is displayingAuthCoordinator
. We want this property to beweak
so thatAuthCoordinator
does not strongly hold a reference to its parent (and thus create a circular reference/memory leak).The only time that
rootNavigator
would benil
is ifAuthCoordinator
's parent was no longer being displayed, but a reference toAuthCoordinator
was still retained somewhere. In that case, you are totally correct. Both thestart
andnavigate
methods could technically be called onAuthCoordinator
, but they would have no visible effect ifAuthCoordinator
'sstackNavigator
is not being displayed.