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

[NT-551] Crash on logout fix #937

Merged
merged 5 commits into from
Nov 11, 2019
Merged

[NT-551] Crash on logout fix #937

merged 5 commits into from
Nov 11, 2019

Conversation

Scollaco
Copy link
Contributor

@Scollaco Scollaco commented Nov 8, 2019

📲 What

  • Fixes a bug where the app crashes when user tries to logout.
  • The crash started happening after the PR that navigates from Project Page was merged. We were returning a viewController of type T from the function viewControllerAndParam<T, P>(with :param:) -> (T, P)? when we should be returning nil.

🤔 Why

  • Crashes are a no no.

🛠 How

  • Determining on RootTabBarViewController if we are switching to Dashboard.

If yes:

  • We try to cast the viewController[index] to UINavigationController and then cast its first child to type T.

If no:

  • We try to cast the viewController[index] to type T directly.

We return nil otherwise.

👀 See

logout

@@ -184,11 +184,11 @@ public final class RootTabBarViewController: UITabBarController {
}

private func viewControllerAndParam<T, P>(with index: RootViewControllerIndex, param: P) -> (T, P)? {
guard
let vcs = self.viewControllers,
guard let vcs = self.viewControllers,
Copy link
Contributor

@justinswart justinswart Nov 8, 2019

Choose a reason for hiding this comment

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

Can we make this small change for readability:

guard
  let vcs = self.viewControllers,
  let nav = vcs[clamp(0, vcs.count - 1)(index)] as? UINavigationController,
  let vc = nav.children.first as? T
else { return nil }

@Scollaco Scollaco merged commit 17b532a into master Nov 11, 2019
@Scollaco Scollaco deleted the crash-on-logout-fix branch November 11, 2019 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants