Skip to content
This repository has been archived by the owner on Oct 14, 2024. It is now read-only.

[BUG] .dismissModally(.soft) does not behave as documented. #96

Open
TheInkedEngineer opened this issue Oct 18, 2020 · 0 comments
Open

Comments

@TheInkedEngineer
Copy link
Contributor

TheInkedEngineer commented Oct 18, 2020

Below is a copy of Tempura's ModalDismissBehaviour documentation.

  /// Define one of the two possible behaviours when dismissing a modal ViewController:
  ///
  /// `.soft`: dismiss the ViewController but keep all the presented ViewControllers
  ///
  /// `.hard`: the usual UIKit behaviour, dismiss the ViewController and all the ViewControllers that is presenting
  public enum ModalDismissBehaviour {
    /// If the targeted modal is presenting other modals, keep them alive.
    case soft
    /// While removing the targeted modal, remove also all the modals that it is presenting.
    case hard
  }

In other words, the expected behavior when using the .soft modal dismiss is to hide only the intended view controller without hiding the stack on top of it.

However, as shown in the attached project, that is not the actual behavior of Tempura.

Regardless if .soft or .hard is passed to the .dismissModally function, the latter is applied.

My best guess this is due to the following code:

  func hide(_ elementToHide: RouteElementIdentifier, animated: Bool, context: Any?, atomic: Bool = false) -> Promise<Void> {
    let promise = Promise<Void>(in: .background) { resolve, reject, _ in
      let oldRoutables = UIApplication.shared.currentRoutables
      let oldRoute = oldRoutables.map { $0.routeIdentifier }
      
      guard let start = oldRoute.indices.reversed().first(where: { oldRoute[$0] == elementToHide }) else {
        resolve(())
        return
      }
      
      let newRoute = Array(oldRoute[0..<start])
      
      let routeChanges = Navigator.routingChanges(from: oldRoutables, new: newRoute, atomic: atomic)
      
      self.routeDidChange(changes: routeChanges, isAnimated: animated, context: context) {
        resolve(())
      }
    }
    return promise
  }

By trimming the array of routes to generate the new route and using that latter to generate the diff between old and new routes, before checking the modal dismiss behavior, the .soft keyword becomes obsolete.

So in a scenario where:

A -> B -> C, where -> is the equivalent of a modal presentation,
by wanting to hide B, C is hidden as well regardless of the behavior.

More so, if no view controller manages the .hide of C, the app will crash regardless if the modal dismiss behavior is .hard or .soft.

  • Xcode version 12.0
  • Swift Version 5.3
  • Tempura, version 4.4.0

Snippet project that reproduces the bug.
It consists of a navigation controller that presents a view controller which in its turn presents another view controller.
A button is attached to the third view controller that tries to hide the second one, which should dismiss softly, however it hides the topmost view controller as well.

TempuraBug.zip

┆Issue is synchronized with this Asana task by Unito

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant