Merge pull request #56 from iZettle/split-vc-iscollapsed-fix

Split VC isCollapsed fix (issue 54)

.circleci/config.yml

@@ -1,3 +1,4 @@
+---
 version: 2
 
 common: &common
@@ -11,11 +12,14 @@ env:
 
 jobs:
   swiftlint:
-    docker:
-      - image: dantoml/swiftlint:latest
+    <<: *common
     steps:
       - checkout
-      - run: swiftlint --strict
+      - run:
+          name: Install swiftlint(0.35.0)
+          command: |
+            brew install https://raw.githubusercontent.com/Homebrew/homebrew-core/ab64b13d26a3f8617689e3bc7e73a209d11f1bc8/Formula/swiftlint.rb
+      - run: swiftlint version && swiftlint --strict
 
   test-iOS:
     <<: *common
@@ -71,12 +75,6 @@ workflows:
   build-and-test:
     jobs:
       - swiftlint
-      - test-iOS:
-          requires:
-            - swiftlint
-      - test-iOS12:
-          requires:
-            - swiftlint
-      - examples:
-          requires:
-            - swiftlint
+      - test-iOS
+      - test-iOS12
+      - examples

CHANGELOG.md

@@ -1,3 +1,10 @@
+# 1.9.0
+- [Addition] Add a new `isCollapsedState` signal to DualNavigationControllersSplitDelegate that has a `nil` value until the collapsed state is known. Old `isCollapsedSignal` is deprecated.
+- [Addition] Add a new `init(collapsedState:)` method to DualNavigationControllersSplitDelegate that takes a future to get notified of a known collapsed state. The `init` without parameters is deprecated.
+- [Change] Deprecate MasterDetailSelection's init with `isCollapsed` signal in favour of init that can handle a `nil` collapsed state
+- [Bug fix] DualNavigationControllersSplitDelegate's `isCollapsedSignal` didn't signal `false` when moving from collapsed state to not collapsed (multitasking/rotation)
+- [Bug fix] DualNavigationControllersSplitDelegate's `isCollapsedSignal` didn't signal anything on iOS 13 ([issue #54](https://github.com/iZettle/Presentation/issues/54))
+
 # 1.8.1
 - [Bug fix] Revert a change of the default SplitVC delegate `isCollapsed` value that doesn't work as expected because it's used before the vc is added to the screen and the value is not reliable
 

Examples/Messages/Example/Messages.swift

@@ -52,7 +52,7 @@ extension Messages: Presentable {
         }
 
         let splitDelegate = split.setupSplitDelegate(ownedBy: bag)
-        let selection = MasterDetailSelection(elements: messages, isSame: ==, isCollapsed: splitDelegate.isCollapsedSignal)
+        let selection = MasterDetailSelection(elements: messages, isSame: ==, isCollapsed: splitDelegate.isCollapsed)
 
         bag += selectSignal.onValue { indexPath in
             selection.select(index: indexPath.row)

Presentation/DualNavigationControllersSplitDelegate.swift

@@ -12,8 +12,25 @@ import Flow
 /// A split controller delegate (`UISplitViewControllerDelegate`), that manages navigation controllers for the master and detail view,
 /// as well as moving view controllers between these while expanding or collapsing.
 open class DualNavigationControllersSplitDelegate: NSObject, UISplitViewControllerDelegate {
-    private let isCollapsedProperty = ReadWriteSignal(true)
+
+    @available(*, deprecated, message: "use `init(isCollapsed:)` instead")
+    public convenience override init() {
+        self.init(isCollapsed: Future(true))
+    }
+
+    /// Creates a delegate that will manage a split view controller with the specified initial collapsed state
+    /// - Parameter isCollapsed: Use to report the initial `isCollapsed` state of the split view controller.
+    /// Note that its value can be unreliable until it is rendered on the screen.
+    public init(isCollapsed: Future<Bool>) {
+        super.init()
+        initialCollapsedStateDisposable += isCollapsed.onValue { [weak self] in
+            self?.isCollapsedProperty.value = $0
+        }.disposable
+    }
+
+    private let isCollapsedProperty = ReadWriteSignal<Bool?>(nil)
     private var detailBag: Disposable?
+    private let initialCollapsedStateDisposable = DisposeBag()
 
     /// Customization point for construction of the navigation controllers managed by `self`
     public var makeNavigationController = customNavigationController
@@ -23,8 +40,14 @@ open class DualNavigationControllersSplitDelegate: NSObject, UISplitViewControll
 
     public let presentDetail = Delegate<UISplitViewController, Disposable>()
 
-    /// Returns a signal that will signal when collapsing or expanding
+    /// Returns a signal that will signal when collapsing or expanding with an initial value of true
+    @available(*, deprecated, message: "use `isCollapsed` signal instead")
     public var isCollapsedSignal: ReadSignal<Bool> {
+        return isCollapsedProperty.map { $0 ?? true }
+    }
+
+    /// Returns a signal that will signal when collapsing or expanding. Current value can be nil if the collapsed state cannot be determined reliably yet.
+    public var isCollapsed: ReadSignal<Bool?> {
         return isCollapsedProperty.readOnly()
     }
 
@@ -39,8 +62,6 @@ open class DualNavigationControllersSplitDelegate: NSObject, UISplitViewControll
     }
 
     open func targetDisplayModeForAction(in svc: UISplitViewController) -> UISplitViewController.DisplayMode {
-        isCollapsedProperty.value = svc.isViewLoaded && svc.view.window != nil ? svc.isCollapsed : true
-
         if svc.viewControllers.isEmpty {
             svc.viewControllers.append(makeMasterNavigationController(svc))
         }
@@ -63,6 +84,11 @@ open class DualNavigationControllersSplitDelegate: NSObject, UISplitViewControll
     }
 
     open func splitViewController(_ svc: UISplitViewController, collapseSecondary secondaryViewController: UIViewController, onto primaryViewController: UIViewController) -> Bool {
+        defer {
+            initialCollapsedStateDisposable.dispose()
+            isCollapsedProperty.value = true
+        }
+
         guard let primary = primaryViewController as? UINavigationController, let secondary = secondaryViewController as? UINavigationController else {
             detailBag?.dispose()
             return false
@@ -77,12 +103,14 @@ open class DualNavigationControllersSplitDelegate: NSObject, UISplitViewControll
             primary.transferViewControllers(from: secondary)
         }
 
-        isCollapsedProperty.value = true
-
         return true
     }
 
     open func splitViewController(_ svc: UISplitViewController, separateSecondaryFrom primaryViewController: UIViewController) -> UIViewController? {
+        defer {
+            initialCollapsedStateDisposable.dispose()
+            isCollapsedProperty.value = false
+        }
         guard let primary = primaryViewController as? UINavigationController else { return nil }
         guard svc.viewControllers.count < 2 else { return svc.viewControllers[1] }
 
@@ -103,16 +131,15 @@ open class DualNavigationControllersSplitDelegate: NSObject, UISplitViewControll
             nc.viewControllers = Array(vcs)
         }
 
-        isCollapsedProperty.value = true
-
         return nc
     }
 }
 
 public extension UISplitViewController {
     /// Creates and returns a new split delegate that will be set as `self`'s delegate until `bag` is disposed.
     func setupSplitDelegate(ownedBy bag: DisposeBag) -> DualNavigationControllersSplitDelegate {
-        let splitDelegate = DualNavigationControllersSplitDelegate()
+        let collapsedState = self.view.didLayoutSignal.map { self.isCollapsed }.future
+        let splitDelegate = DualNavigationControllersSplitDelegate(isCollapsed: collapsedState)
         delegate = splitDelegate
         bag += {
             _ = splitDelegate

Presentation/Info.plist

@@ -15,7 +15,7 @@
 	<key>CFBundlePackageType</key>
 	<string>FMWK</string>
 	<key>CFBundleShortVersionString</key>
-	<string>1.8.1</string>
+	<string>1.9.0</string>
 	<key>CFBundleSignature</key>
 	<string>????</string>
 	<key>CFBundleVersion</key>

Presentation/MasterDetailSelection.swift

@@ -21,22 +21,25 @@ public final class MasterDetailSelection<Elements: BidirectionalCollection>: Sig
     private let bag = DisposeBag()
     fileprivate let keepSelection: KeepSelection<Elements>
     private var isSelecting = false
-    fileprivate let isCollapsed: ReadSignal<Bool>
+    fileprivate let isCollapsed: ReadSignal<Bool?>
+
+    @available(*, deprecated, message: "pass isCollapsed as a ReadSignal<Bool?> instead")
+    public convenience init(elements: ReadSignal<Elements>, isSame: @escaping (Element, Element) -> Bool, needsUpdate: @escaping (Element, Element) -> Bool = { _, _ in false }, isCollapsed: ReadSignal<Bool>) {
+        self.init(elements: elements, isSame: isSame, needsUpdate: needsUpdate, isCollapsed: isCollapsed.map { value -> Bool? in return value })
+    }
 
     /// Creates a new instance using changes in `elements` and `isCollapsed` to maintain the selected detail index (provided signal).
     /// - Parameters:
     ///   - isSame: Is it the same row (same identity)
     ///   - needsUpdate: For the same row, does the row have updates that requires presenting new details (refresh details)
     ///   - isCollapsed: Whether or not details are displayed.
-    public init(elements: ReadSignal<Elements>, isSame: @escaping (Element, Element) -> Bool, needsUpdate: @escaping (Element, Element) -> Bool = { _, _ in false }, isCollapsed: ReadSignal<Bool>) {
+    public init(elements: ReadSignal<Elements>, isSame: @escaping (Element, Element) -> Bool, needsUpdate: @escaping (Element, Element) -> Bool = { _, _ in false }, isCollapsed: ReadSignal<Bool?>) {
         keepSelection = KeepSelection(elements: elements, isSame: isSame)
         self.isCollapsed = isCollapsed
 
-        bag += isCollapsed.atOnce().onValueDisposePrevious { [weak self] isCollapsed in
-            guard let `self` = self else { return NilDisposer() }
-            return self.keepSelection.atOnce().enumerate().onValue { [weak self] (eventCount, indexAndElement) in
-                guard let `self` = self else { return }
-
+        bag += isCollapsed.atOnce().with(weak: self).onValueDisposePrevious { isCollapsed, `self` in
+            guard let isCollapsed = isCollapsed else { return NilDisposer() }
+            return self.keepSelection.atOnce().enumerate().with(weak: self).onValue { (eventCount, indexAndElement, `self`) in
                 let indexWasUpdated = eventCount > 0 // if eventCount is 0, it was just the atOnce value
                 let index = indexAndElement?.index
                 let elementDidUpdate = indexAndElement.flatMap { i in self.current.map {
@@ -116,7 +119,7 @@ public final class MasterDetailSelection<Elements: BidirectionalCollection>: Sig
             guard let `self` = self else {
                 return NilDisposer()
             }
-            onSet(self.isCollapsed.value ? nil : self.current)
+            onSet((self.isCollapsed.value ?? true) ? nil: self.current)
             return NilDisposer()
         }
     }()
@@ -145,7 +148,8 @@ public extension MasterDetailSelection {
         var immediate = true
 
         bag += self.presentDetail.set { indexAndElement in
-            guard !self.isCollapsed.value || indexAndElement != nil else {
+            guard let isCollapsed = self.isCollapsed.value else { return }
+            guard !isCollapsed || indexAndElement != nil else {
                 detailBag.dispose()
                 return
             }
@@ -156,7 +160,11 @@ public extension MasterDetailSelection {
 
             immediate = true
             let presentDisposable = vc.present(presentation.onDismiss {
-                if self.isCollapsed.value && !immediate {
+                guard let isCollapsed = self.isCollapsed.value else {
+                    assertionFailure("Should not happen because once the collapsed state is determined it should not turn to nil")
+                    return
+                }
+                if isCollapsed && !immediate {
                     self.deselect()
                 }
             }).disposable

Presentation/UINavigationController+Presenting.swift

@@ -184,7 +184,7 @@ private extension UINavigationController {
         }
 
         let viewControllerToAppear = vcs.last
-        if let navBarHidden = pushPopers.filter ({ $0.vc == viewControllerToAppear }).last?.vcPrefersNavBarHidden {
+        if let navBarHidden = pushPopers.filter({ $0.vc == viewControllerToAppear }).last?.vcPrefersNavBarHidden {
             self.setNavigationBarHidden(navBarHidden, animated: animated)
         }
 

PresentationFramework.podspec

@@ -1,6 +1,6 @@
 Pod::Spec.new do |s|
   s.name         = "PresentationFramework"
-  s.version      = "1.8.1"
+  s.version      = "1.9.0"
   s.module_name  = "Presentation"
   s.summary      = "Driving presentations from model to result"
   s.description  = <<-DESC

PresentationTests/Info.plist

@@ -15,7 +15,7 @@
 	<key>CFBundlePackageType</key>
 	<string>BNDL</string>
 	<key>CFBundleShortVersionString</key>
-	<string>1.8.1</string>
+	<string>1.9.0</string>
 	<key>CFBundleVersion</key>
 	<string>1</string>
 </dict>