Skip to content
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 4 commits into from
Sep 12, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions Bitwarden/Application/SceneDelegateTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,14 @@ import XCTest
class SceneDelegateTests: BitwardenTestCase {
// MARK: Properties

var appCoordinator: MockCoordinator<AppRoute>!
var appModule: MockAppModule!
var subject: SceneDelegate!

// MARK: Setup & Teardown

override func setUp() {
super.setUp()
appCoordinator = MockCoordinator<AppRoute>()
appModule = MockAppModule()
appModule.appCoordinator = appCoordinator.asAnyCoordinator()
subject = SceneDelegate()
subject.appModule = appModule
}
Expand All @@ -42,7 +39,7 @@ class SceneDelegateTests: BitwardenTestCase {

XCTAssertNotNil(subject.appCoordinator)
XCTAssertNotNil(subject.window)
XCTAssertTrue(appCoordinator.isStarted)
XCTAssertTrue(appModule.appCoordinator.isStarted)
}

/// `scene(_:willConnectTo:options:)` without a `UIWindowScene` fails to create the app's UI.
Expand All @@ -56,6 +53,6 @@ class SceneDelegateTests: BitwardenTestCase {

XCTAssertNil(subject.appCoordinator)
XCTAssertNil(subject.window)
XCTAssertFalse(appCoordinator.isStarted)
XCTAssertFalse(appModule.appCoordinator.isStarted)
}
}
82 changes: 82 additions & 0 deletions BitwardenShared/UI/Auth/AuthCoordinator.swift
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.
private weak var rootNavigator: (any RootNavigator)?

/// The stack navigator that is managed by this coordinator.
private 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)
}
}
71 changes: 71 additions & 0 deletions BitwardenShared/UI/Auth/AuthCoordinatorTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
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
)
}
Comment on lines +17 to +25
Copy link
Member

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 being nil? Not sure which would be the real case where the AuthCoordinator receives a nil root value but if I'm not mistaken it's currently feasible.

Copy link
Contributor Author

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.

Copy link
Contributor Author

@nathan-livefront nathan-livefront Sep 7, 2023

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 change rootNavigator to not be optional. I can't think of a real-world use case for it being nil, 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 is weak, I've added a test that validates that AuthCoordinator does not retain a strong reference to rootNavigator!

Copy link
Member

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!

Copy link
Member

@fedemkr fedemkr Sep 7, 2023

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 execute func navigate(to route: AuthRoute, context: AnyObject?) and if so, would you foresee any problems? I guess the stackNavigator would be still in memory so the action could be executed but nothing will be seen by the user given that I assume the rootNavigator won't be in screen if the reference has been lost.
Does it makes sense to add tests for when rootNavigator is nil for start and navigate?

Copy link
Contributor Author

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 and start methods on an instance of AuthCoordinator while rootNavigator is nil, I don't think it's necessarily worth testing in this case.

rootNavigator is weak because the rootNavigator is a property of the parent that is displaying AuthCoordinator. We want this property to be weak so that AuthCoordinator does not strongly hold a reference to its parent (and thus create a circular reference/memory leak).

The only time that rootNavigator would be nil is if AuthCoordinator's parent was no longer being displayed, but a reference to AuthCoordinator was still retained somewhere. In that case, you are totally correct. Both the start and navigate methods could technically be called on AuthCoordinator, but they would have no visible effect if AuthCoordinator's stackNavigator is not being displayed.


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)
}

/// `start()` presents the stack navigator within the root navigator.
func test_start() {
subject.start()
XCTAssertIdentical(rootNavigator.navigatorShown, stackNavigator)
}
}
31 changes: 31 additions & 0 deletions BitwardenShared/UI/Auth/AuthModule.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
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.
///
/// - Parameter navigator: The object 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()
}
}
16 changes: 16 additions & 0 deletions BitwardenShared/UI/Auth/AuthRoute.swift
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
}
19 changes: 19 additions & 0 deletions BitwardenShared/UI/Auth/Landing/LandingAction.swift
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)
}
42 changes: 42 additions & 0 deletions BitwardenShared/UI/Auth/Landing/LandingProcessor.swift
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 BitwardenShared/UI/Auth/Landing/LandingProcessorTests.swift
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)
}
}
29 changes: 29 additions & 0 deletions BitwardenShared/UI/Auth/Landing/LandingState.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// MARK: - LandingState

/// An object that defines the current state of a `LandingView`.
///
struct LandingState: Equatable {
// MARK: Properties

/// The email address provided by the user.
var email: String

/// A flag indicating if the "Remember Me" toggle is on.
var isRememberMeOn: Bool

// MARK: Initialization

/// Creates a new `LandingState`.
///
/// - Parameters:
/// - email: The email address provided by the user.
/// - isRememberMeOn: A flag indicating if the "Remember Me" toggle is on.
///
init(
email: String = "",
isRememberMeOn: Bool = false
) {
self.email = email
self.isRememberMeOn = isRememberMeOn
}
}
Loading