Skip to content
This repository was archived by the owner on Jan 30, 2021. It is now read-only.

Coordinator Cleanup #75

Open
heyltsjay wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Coordinator Cleanup #75

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3805c1c
Coordinator Cleanup
Mar 21, 2018
0e8ea36
Merge branch 'master' into feature/heyltsjay/coordinatorCleanup
Mar 21, 2018
8c57c63
Remove `CoordinatorProtocol`
Mar 21, 2018
51509fb
update readme
Mar 21, 2018
bc90d6d
generate CookieCutter Template
Mar 21, 2018
56dc48b
add missing delegate methods
Mar 21, 2018
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
3 changes: 3 additions & 0 deletions PRODUCTNAME/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ To get started, see [Contributing](#contributing)

- Coordinators should:
- Manage view controller transitions
- Deinit when the views they manage Deinit
- `attach()` to the lifecycle of their children
- **NOT** hold strong references to their children
Copy link

@jvisenti jvisenti Mar 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#nth Add a should **NOT** hold strong references to coordinators somewhere in here. Just to contrast with how we used to always hold strong refs to make sure they didn't go away

- View Controllers should:
- Delegate 'final' actions to a Coordinator
- **Not** access the navigation controller or present view controllers
Expand Down
20 changes: 8 additions & 12 deletions PRODUCTNAME/app/PRODUCTNAME.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,8 @@
2D75C6C71E8EEAE900F2EB1D /* OnboardingCoordinator.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2DFF1CEC1E8944B900B1AD70 /* OnboardingCoordinator.swift */; };
2D75C6C81E8EEB1900F2EB1D /* AuthCoordinator.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2D4FA8F81E8577B5006C38ED /* AuthCoordinator.swift */; };
2DAB59F81E95336100310ABF /* OnboardingSamplePageViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2DAB59F71E95336100310ABF /* OnboardingSamplePageViewModel.swift */; };
2DFF1CEF1E89493300B1AD70 /* SignInCoordinator.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2DFF1CEE1E89493300B1AD70 /* SignInCoordinator.swift */; };
2DFF1CF11E8950F000B1AD70 /* ContentCoordinator.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2DFF1CF01E8950F000B1AD70 /* ContentCoordinator.swift */; };
2DFF1D121E8BE27B00B1AD70 /* OnboardingPageViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2DFF1D111E8BE27B00B1AD70 /* OnboardingPageViewController.swift */; };
2DFF1CF11E8950F000B1AD70 /* HomeCoordinator.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2DFF1CF01E8950F000B1AD70 /* HomeCoordinator.swift */; };
2DFF1D121E8BE27B00B1AD70 /* OnboardingViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2DFF1D111E8BE27B00B1AD70 /* OnboardingViewController.swift */; };
2DFF1D161E8BF38F00B1AD70 /* UIColor+Extensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2DFF1D151E8BF38F00B1AD70 /* UIColor+Extensions.swift */; };
2DFF1D181E8BF82A00B1AD70 /* Colors.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2DFF1D171E8BF82A00B1AD70 /* Colors.swift */; };
2DFF1D1C1E8C424800B1AD70 /* OnboardingSamplePageViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2DFF1D1B1E8C424800B1AD70 /* OnboardingSamplePageViewController.swift */; };
Expand Down Expand Up @@ -135,9 +134,8 @@
2D4FA8F81E8577B5006C38ED /* AuthCoordinator.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = AuthCoordinator.swift; sourceTree = "<group>"; };
2DAB59F71E95336100310ABF /* OnboardingSamplePageViewModel.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = OnboardingSamplePageViewModel.swift; sourceTree = "<group>"; };
2DFF1CEC1E8944B900B1AD70 /* OnboardingCoordinator.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = OnboardingCoordinator.swift; sourceTree = "<group>"; };
2DFF1CEE1E89493300B1AD70 /* SignInCoordinator.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SignInCoordinator.swift; sourceTree = "<group>"; };
2DFF1CF01E8950F000B1AD70 /* ContentCoordinator.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ContentCoordinator.swift; sourceTree = "<group>"; };
2DFF1D111E8BE27B00B1AD70 /* OnboardingPageViewController.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = OnboardingPageViewController.swift; sourceTree = "<group>"; };
2DFF1CF01E8950F000B1AD70 /* HomeCoordinator.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = HomeCoordinator.swift; sourceTree = "<group>"; };
2DFF1D111E8BE27B00B1AD70 /* OnboardingViewController.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = OnboardingViewController.swift; sourceTree = "<group>"; };
2DFF1D151E8BF38F00B1AD70 /* UIColor+Extensions.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "UIColor+Extensions.swift"; sourceTree = "<group>"; };
2DFF1D171E8BF82A00B1AD70 /* Colors.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = Colors.swift; sourceTree = "<group>"; };
2DFF1D1B1E8C424800B1AD70 /* OnboardingSamplePageViewController.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = OnboardingSamplePageViewController.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -272,18 +270,17 @@
children = (
2D4FA8F41E8574F9006C38ED /* AppCoordinator.swift */,
2D4FA8F81E8577B5006C38ED /* AuthCoordinator.swift */,
2DFF1CF01E8950F000B1AD70 /* ContentCoordinator.swift */,
2DFF1CF01E8950F000B1AD70 /* HomeCoordinator.swift */,
2D4FA8F61E85752B006C38ED /* Coordinator.swift */,
2DFF1CEC1E8944B900B1AD70 /* OnboardingCoordinator.swift */,
2DFF1CEE1E89493300B1AD70 /* SignInCoordinator.swift */,
);
path = Coordinators;
sourceTree = "<group>";
};
2DFF1D131E8BE3D400B1AD70 /* Onboarding */ = {
isa = PBXGroup;
children = (
2DFF1D111E8BE27B00B1AD70 /* OnboardingPageViewController.swift */,
2DFF1D111E8BE27B00B1AD70 /* OnboardingViewController.swift */,
2DFF1D1B1E8C424800B1AD70 /* OnboardingSamplePageViewController.swift */,
2DAB59F71E95336100310ABF /* OnboardingSamplePageViewModel.swift */,
);
Expand Down Expand Up @@ -982,15 +979,14 @@
2D75C6C81E8EEB1900F2EB1D /* AuthCoordinator.swift in Sources */,
2D75C6C71E8EEAE900F2EB1D /* OnboardingCoordinator.swift in Sources */,
2DFF1D1C1E8C424800B1AD70 /* OnboardingSamplePageViewController.swift in Sources */,
2DFF1CF11E8950F000B1AD70 /* ContentCoordinator.swift in Sources */,
2DFF1CF11E8950F000B1AD70 /* HomeCoordinator.swift in Sources */,
ABF84F8D1DC99CFD002DB24D /* TextStyle.swift in Sources */,
8662C00A1FA2AA9900ADCBA9 /* SimpleTableCellItem.swift in Sources */,
861732F61FA2984600C14354 /* ViewRepresentable.swift in Sources */,
ABC778EA1DC951B900815FB9 /* LoggingConfiguration.swift in Sources */,
BE4C21EB1E81970A00645143 /* DebugMenuConfiguration.swift in Sources */,
2D4FA8F71E85752B006C38ED /* Coordinator.swift in Sources */,
2D4FA8F51E8574F9006C38ED /* AppCoordinator.swift in Sources */,
2DFF1CEF1E89493300B1AD70 /* SignInCoordinator.swift in Sources */,
2DAB59F81E95336100310ABF /* OnboardingSamplePageViewModel.swift in Sources */,
861732F41FA2984600C14354 /* TableCellItem.swift in Sources */,
861732F81FA2989100C14354 /* TableViewContainerCell.swift in Sources */,
Expand Down Expand Up @@ -1021,7 +1017,7 @@
2DFF1D181E8BF82A00B1AD70 /* Colors.swift in Sources */,
861732D51FA28DB600C14354 /* ModalDismissBehavior.swift in Sources */,
BE28092A1E802893006E50D5 /* Analytics.swift in Sources */,
2DFF1D121E8BE27B00B1AD70 /* OnboardingPageViewController.swift in Sources */,
2DFF1D121E8BE27B00B1AD70 /* OnboardingViewController.swift in Sources */,
);
runOnlyForDeploymentPostprocessing = 0;
};
Expand Down
7 changes: 4 additions & 3 deletions PRODUCTNAME/app/PRODUCTNAME/Application/AppDelegate.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,14 @@ class AppDelegate: UIResponder, UIApplicationDelegate {

let window = UIWindow(frame: UIScreen.main.bounds)
self.window = window

self.coordinator = AppCoordinator(window: window)
coordinator.start(animated: true, completion: {
coordinator.start { rootViewController in
window.rootViewController = rootViewController
window.makeKeyAndVisible()
for config in self.rootViewControllerDependentConfigurations where config.isEnabled {
config.onDidLaunch(application: application, launchOptions: launchOptions)
}
})
}

return true
}
Expand Down
89 changes: 53 additions & 36 deletions PRODUCTNAME/app/PRODUCTNAME/Coordinators/AppCoordinator.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,63 +7,80 @@
//

import UIKit
import Services

class AppCoordinator: Coordinator {

private let window: UIWindow
fileprivate let rootController: UIViewController
var childCoordinator: Coordinator?

init(window: UIWindow) {
self.window = window
let rootController = UIViewController()
rootController.view.backgroundColor = .white
self.rootController = rootController
}

func start(animated: Bool, completion: VoidClosure?) {
// Configure window/root view controller
window.setRootViewController(rootController, animated: false, completion: {
self.window.makeKeyAndVisible()
func start(with presentation: (UIViewController) -> Void) {
guard OnboardingCoordinator.hasOnboarded else {
Copy link

@jvisenti jvisenti Mar 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#pp Instead of guarding with early returns, use if/else. This bit me on a project where I wanted to do something after the correct coordinator was chosen, and had to do it with a defer.

let onboardingCoordinator = OnboardingCoordinator()
onboardingCoordinator.delegate = self
attach(to: onboardingCoordinator)
onboardingCoordinator.start(with: presentation)
return
}

// Spin off auth coordinator
let authCoordinator = AuthCoordinator(self.rootController)
let authCoordinator = AuthCoordinator()
guard authCoordinator.isAuthenticated else {
authCoordinator.delegate = self
self.childCoordinator = authCoordinator
authCoordinator.start(animated: animated, completion: completion)
})
}
attach(to: authCoordinator)
authCoordinator.startSignIn(with: presentation)
return
}

func cleanup(animated: Bool, completion: VoidClosure?) {
completion?()
let homeCoordinator = HomeCoordinator()
attach(to: homeCoordinator)
homeCoordinator.start(with: presentation)
}

}

extension AppCoordinator: AuthCoordinatorDelegate {
extension AppCoordinator: OnboardingCoordinator.Delegate {

func authCoordinator(_ coordinator: AuthCoordinator, didNotify action: AuthCoordinator.Action) {
func onboardingCoordinator(_ coordinator: OnboardingCoordinator, didNotify action: OnboardingCoordinator.Action) {
switch action {
case .didSignIn:
guard let authCoordinator = childCoordinator as? AuthCoordinator else {
preconditionFailure("Upon signing in, AppCoordinator should have an AuthCoordinator as a child.")
case .skip:
let homeCoordinator = HomeCoordinator()
attach(to: homeCoordinator)
homeCoordinator.start {
window.setRootViewController($0, animated: true)
}
childCoordinator = nil
authCoordinator.cleanup(animated: true) {
let contentCoordinator = ContentCoordinator(self.rootController)
self.childCoordinator = contentCoordinator
contentCoordinator.start(animated: true, completion: nil)

case .join:
let authCoordinator = AuthCoordinator()
authCoordinator.delegate = self
attach(to: authCoordinator)
authCoordinator.startSignUp {
window.rootViewController?.present($0, animated: true, completion: nil)
}
case .didSkipAuth:
guard let authCoordinator = childCoordinator as? AuthCoordinator else {
preconditionFailure("Upon signing in, AppCoordinator should have an AuthCoordinator as a child.")

case .signIn:
let authCoordinator = AuthCoordinator()
authCoordinator.delegate = self
attach(to: authCoordinator)
authCoordinator.startSignIn {
window.rootViewController?.present($0, animated: true, completion: nil)
}
childCoordinator = nil
authCoordinator.cleanup(animated: false) {
let contentCoordinator = ContentCoordinator(self.rootController)
self.childCoordinator = contentCoordinator
contentCoordinator.start(animated: true, completion: nil)

}
}

}

extension AppCoordinator: AuthCoordinator.Delegate {

func authCoordinator(_ coordinator: AuthCoordinator, didNotify action: AuthCoordinator.Action) {
switch action {
case .signedIn:
let homeCoordinator = HomeCoordinator()
attach(to: homeCoordinator)
homeCoordinator.start {
window.setRootViewController($0, animated: true)
}
}
}
Expand Down
110 changes: 21 additions & 89 deletions PRODUCTNAME/app/PRODUCTNAME/Coordinators/AuthCoordinator.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,112 +7,44 @@
//

import UIKit
import Services

private enum State {

case authenticated
case onboarded
case needsOnboarding

}

class AuthCoordinator: Coordinator {

var childCoordinator: Coordinator?
let baseController: UIViewController
weak var delegate: Delegate?

private let client = APIClient.shared
private var state: State {
if client.oauthClient.isAuthenticated {
return .authenticated
}
else if UserDefaults.hasOnboarded {
return .onboarded
}
else {
return .needsOnboarding
}
var isAuthenticated: Bool {
// TODO: inject and check from services
return false
}

init(_ baseController: UIViewController) {
self.baseController = baseController
}
func startSignIn(with presentation: (UIViewController) -> Void) {
let nav = UINavigationController()
attach(to: nav)

func start(animated: Bool, completion: VoidClosure?) {
switch state {
case .authenticated:
notify(.didSignIn)
case .onboarded:
let signInCoordinator = SignInCoordinator(baseController)
signInCoordinator.delegate = self
signInCoordinator.start(animated: animated, completion: completion)
childCoordinator = signInCoordinator
case .needsOnboarding:
let onboardCoordinator = OnboardingCoordinator(baseController)
onboardCoordinator.delegate = self
onboardCoordinator.start(animated: animated, completion: completion)
childCoordinator = onboardCoordinator
}
let signInViewController = UIViewController()
signInViewController.view.backgroundColor = .yellow
signInViewController.title = L10n.Signin.title
nav.viewControllers = [signInViewController]
presentation(nav)
}

func cleanup(animated: Bool, completion: VoidClosure?) {
childCoordinator?.cleanup(animated: animated, completion: completion)
func startSignUp(with presentation: (UIViewController) -> Void) {
let nav = UINavigationController()
attach(to: nav)

let signUpViewController = UIViewController()
signUpViewController.view.backgroundColor = .orange
signUpViewController.title = L10n.Signup.title
nav.viewControllers = [signUpViewController]
presentation(nav)
}

}

extension AuthCoordinator: Actionable {

enum Action {
case didSignIn
case didSkipAuth
}

}

extension AuthCoordinator: SignInCoordinatorDelegate {

func signInCoordinator(_ coordinator: SignInCoordinator, didNotify action: SignInCoordinator.Action) {
switch action {
case .didSignIn:
notify(.didSignIn)
}
}

}

extension AuthCoordinator: OnboardingCoordinatorDelegate {

func onboardingCoordinator(_ coordinator: OnboardingCoordinator, didNotify action: OnboardingCoordinator.Action) {
switch action {
case .didSkipAuth:
notify(.didSkipAuth)
case .didRequestJoin:
guard let onboardCoordinator = childCoordinator as? OnboardingCoordinator else {
preconditionFailure("Upon signing in, AppCoordinator should have an AuthCoordinator as a child.")
}
childCoordinator = nil
onboardCoordinator.cleanup(animated: true) {
let signInCoordinator = SignInCoordinator(self.baseController)
signInCoordinator.delegate = self
self.childCoordinator = signInCoordinator
// TODO - signInCoordinator move from signIn to register here
signInCoordinator.start(animated: true, completion: nil)
}
case .didRequestSignIn:
guard let onboardCoordinator = childCoordinator as? OnboardingCoordinator else {
preconditionFailure("Upon signing in, AppCoordinator should have an AuthCoordinator as a child.")
}
childCoordinator = nil
onboardCoordinator.cleanup(animated: true) {
let signInCoordinator = SignInCoordinator(self.baseController)
signInCoordinator.delegate = self
self.childCoordinator = signInCoordinator
signInCoordinator.start(animated: true, completion: nil)
}
}
case signedIn
}

}
31 changes: 0 additions & 31 deletions PRODUCTNAME/app/PRODUCTNAME/Coordinators/ContentCoordinator.swift
View file
Open in desktop

This file was deleted.

Loading