Merge pull request #300 from mattrubin/menu-component-refactor

Refactor the Menu component

Author: mattrubin

Authenticator/Source/Menu.swift

@@ -23,11 +23,13 @@
 //  SOFTWARE.
 //
 
-struct Menu {
-    let infoList: InfoList
-    private(set) var child: Child
+import Foundation
 
-    enum Child {
+struct Menu: Component {
+    private let infoList: InfoList
+    private var child: Child
+
+    private enum Child {
         case none
         case info(Info)
         case displayOptions(DisplayOptions)
@@ -54,6 +56,8 @@ struct Menu {
         child = .info(info)
     }
 
+    // MARK: View
+
     func viewModel(digitGroupSize: Int) -> ViewModel {
         return ViewModel(infoList: infoList.viewModel, child: child.viewModel(digitGroupSize: digitGroupSize))
     }
@@ -69,34 +73,123 @@ struct Menu {
         }
     }
 
+    // MARK: Update
+
+    enum Action {
+        case dismissInfo
+        case dismissDisplayOptions
+
+        case infoListEffect(InfoList.Effect)
+        case infoEffect(Info.Effect)
+        case displayOptionsEffect(DisplayOptions.Effect)
+    }
+
+    enum Effect {
+        case dismissMenu
+        case showErrorMessage(String)
+        case showSuccessMessage(String)
+        case openURL(URL)
+        case setDigitGroupSize(Int)
+    }
+
+    mutating func update(with action: Action) throws -> Effect? {
+        switch action {
+        case .dismissInfo:
+            try dismissInfo()
+            return nil
+
+        case .dismissDisplayOptions:
+            try dismissDisplayOptions()
+            return nil
+
+        case .infoListEffect(let effect):
+            return try handleInfoListEffect(effect)
+
+        case .infoEffect(let effect):
+            return handleInfoEffect(effect)
+
+        case .displayOptionsEffect(let effect):
+            return handleDisplayOptionsEffect(effect)
+        }
+    }
+
+    private mutating func handleInfoListEffect(_ effect: InfoList.Effect) throws -> Effect? {
+        switch effect {
+        case .showDisplayOptions:
+            try showDisplayOptions()
+            return nil
+
+        case .showBackupInfo:
+            let backupInfo: Info
+            do {
+                backupInfo = try Info.backupInfo()
+            } catch {
+                return .showErrorMessage("Failed to load backup info.")
+            }
+            try showInfo(backupInfo)
+            return nil
+
+        case .showLicenseInfo:
+            let licenseInfo: Info
+            do {
+                licenseInfo = try Info.licenseInfo()
+            } catch {
+                return .showErrorMessage("Failed to load acknowledgements.")
+            }
+            try showInfo(licenseInfo)
+            return nil
+
+        case .done:
+            return .dismissMenu
+        }
+    }
+
+    private mutating func handleInfoEffect(_ effect: Info.Effect) -> Effect? {
+        switch effect {
+        case .done:
+            return .dismissMenu
+        case let .openURL(url):
+            return .openURL(url)
+        }
+    }
+
+    private mutating func handleDisplayOptionsEffect(_ effect: DisplayOptions.Effect) -> Effect? {
+        switch effect {
+        case .done:
+            return .dismissMenu
+        case let .setDigitGroupSize(digitGroupSize):
+            return .setDigitGroupSize(digitGroupSize)
+        }
+    }
+
     // MARK: -
 
-    enum Error: Swift.Error {
+    private enum Error: Swift.Error {
         case badChildState
     }
 
-    mutating func showInfo(_ info: Info) throws {
+    private mutating func showInfo(_ info: Info) throws {
         guard case .none = child else {
             throw Error.badChildState
         }
         child = .info(info)
     }
 
-    mutating func dismissInfo() throws {
+    private mutating func dismissInfo() throws {
         guard case .info = child else {
             throw Error.badChildState
         }
         child = .none
     }
 
-    mutating func showDisplayOptions() throws {
+    private mutating func showDisplayOptions() throws {
         guard case .none = child else {
             throw Error.badChildState
         }
         child = .displayOptions(DisplayOptions())
     }
 
-    mutating func dismissDisplayOptions() throws {
+    private mutating func dismissDisplayOptions() throws {
         guard case .displayOptions = child else {
             throw Error.badChildState
         }

Authenticator/Source/Root.swift

@@ -88,12 +88,7 @@ extension Root {
         case tokenEntryFormAction(TokenEntryForm.Action)
         case tokenEditFormAction(TokenEditForm.Action)
         case tokenScannerAction(TokenScanner.Action)
-
-        case infoListEffect(InfoList.Effect)
-        case infoEffect(Info.Effect)
-        case displayOptionsEffect(DisplayOptions.Effect)
-        case dismissInfo
-        case dismissDisplayOptions
+        case menuAction(Menu.Action)
 
         case addTokenFromURL(Token)
     }
@@ -161,26 +156,11 @@ extension Root {
                     handleTokenScannerEffect(effect)
                 }
 
-            case .infoListEffect(let effect):
-                return try handleInfoListEffect(effect)
-
-            case .infoEffect(let effect):
-                return handleInfoEffect(effect)
-
-            case .displayOptionsEffect(let effect):
-                return handleDisplayOptionsEffect(effect)
-
-            case .dismissInfo:
-                try modal.withMenu { menu in
-                    try menu.dismissInfo()
-                }
-                return nil
-
-            case .dismissDisplayOptions:
-                try modal.withMenu { menu in
-                    try menu.dismissDisplayOptions()
+            case .menuAction(let action):
+                let effect = try modal.withMenu({ menu in try menu.update(with: action) })
+                return effect.flatMap { effect in
+                    handleMenuEffect(effect)
                 }
-                return nil
 
             case .addTokenFromURL(let token):
                 return .addToken(token,
@@ -323,60 +303,21 @@ extension Root {
         }
     }
 
-    private mutating func handleInfoListEffect(_ effect: InfoList.Effect) throws -> Effect? {
+    private mutating func handleMenuEffect(_ effect: Menu.Effect) -> Effect? {
         switch effect {
-        case .showDisplayOptions:
-            try modal.withMenu { menu in
-                try menu.showDisplayOptions()
-            }
-            return nil
-
-        case .showBackupInfo:
-            let backupInfo: Info
-            do {
-                backupInfo = try Info.backupInfo()
-            } catch {
-                return .showErrorMessage("Failed to load backup info.")
-            }
-            try modal.withMenu { menu in
-                try menu.showInfo(backupInfo)
-            }
-            return nil
-
-        case .showLicenseInfo:
-            let licenseInfo: Info
-            do {
-                licenseInfo = try Info.licenseInfo()
-            } catch {
-                return .showErrorMessage("Failed to load acknowledgements.")
-            }
-            try modal.withMenu { menu in
-                try menu.showInfo(licenseInfo)
-            }
-            return nil
-
-        case .done:
+        case .dismissMenu:
             modal = .none
             return nil
 
-        }
-    }
+        case let .showErrorMessage(message):
+            return .showErrorMessage(message)
+
+        case let .showSuccessMessage(message):
+            return .showSuccessMessage(message)
 
-    private mutating func handleInfoEffect(_ effect: Info.Effect) -> Effect? {
-        switch effect {
-        case .done:
-            modal = .none
-            return nil
         case let .openURL(url):
             return .openURL(url)
-        }
-    }
 
-    private mutating func handleDisplayOptionsEffect(_ effect: DisplayOptions.Effect) -> Effect? {
-        switch effect {
-        case .done:
-            modal = .none
-            return nil
         case let .setDigitGroupSize(digitGroupSize):
             return .setDigitGroupSize(digitGroupSize)
         }

Authenticator/Source/RootViewController.swift

@@ -155,23 +155,23 @@ extension RootViewController {
             case .info(let infoViewModel):
                 presentViewModels(menuViewModel.infoList,
                                   using: InfoListViewController.self,
-                                  actionTransform: Root.Action.infoListEffect,
+                                  actionTransform: compose(Menu.Action.infoListEffect, Root.Action.menuAction),
                                   and: infoViewModel,
                                   using: InfoViewController.self,
-                                  actionTransform: Root.Action.infoEffect)
+                                  actionTransform: compose(Menu.Action.infoEffect, Root.Action.menuAction))
 
             case .displayOptions(let displayOptionsViewModel):
                 presentViewModels(menuViewModel.infoList,
                                   using: InfoListViewController.self,
-                                  actionTransform: Root.Action.infoListEffect,
+                                  actionTransform: compose(Menu.Action.infoListEffect, Root.Action.menuAction),
                                   and: displayOptionsViewModel,
                                   using: DisplayOptionsViewController.self,
-                                  actionTransform: Root.Action.displayOptionsEffect)
+                                  actionTransform: compose(Menu.Action.displayOptionsEffect, Root.Action.menuAction))
 
             case .none:
                 presentViewModel(menuViewModel.infoList,
                                  using: InfoListViewController.self,
-                                 actionTransform: Root.Action.infoListEffect)
+                                 actionTransform: compose(Menu.Action.infoListEffect, Root.Action.menuAction))
             }
         }
         currentViewModel = viewModel
@@ -216,11 +216,11 @@ extension RootViewController: UINavigationControllerDelegate {
             case .info:
                 // If the current modal state is the menu with an Info child, and the just-shown view controller is
                 // an InfoList, then the user has popped the Info view controller.
-                dispatchAction(.dismissInfo)
+                dispatchAction(.menuAction(.dismissInfo))
             case .displayOptions:
                 // If the current modal state is the menu with a DisplayOptions child, and the just-shown view
                 // controller is an InfoList, then the user has popped the DisplayOptions view controller.
-                dispatchAction(.dismissDisplayOptions)
+                dispatchAction(.menuAction(.dismissDisplayOptions))
             default:
                 break
             }

AuthenticatorTests/RootTests.swift

@@ -66,7 +66,7 @@ class RootTests: XCTestCase {
         }
 
         // Hide the backup info.
-        let hideAction: Root.Action = .infoEffect(.done)
+        let hideAction: Root.Action = .menuAction(.infoEffect(.done))
         let hideEffect: Root.Effect?
         do {
             hideEffect = try root.update(with: hideAction)
@@ -113,7 +113,7 @@ class RootTests: XCTestCase {
         }
 
         // Show the license info.
-        let showAction: Root.Action = .infoListEffect(.showLicenseInfo)
+        let showAction: Root.Action = .menuAction(.infoListEffect(.showLicenseInfo))
         let showEffect: Root.Effect?
         do {
             showEffect = try root.update(with: showAction)
@@ -138,7 +138,7 @@ class RootTests: XCTestCase {
         }
 
         // Hide the license info.
-        let hideAction: Root.Action = .infoEffect(.done)
+        let hideAction: Root.Action = .menuAction(.infoEffect(.done))
         let hideEffect: Root.Effect?
         do {
             hideEffect = try root.update(with: hideAction)
@@ -164,9 +164,10 @@ class RootTests: XCTestCase {
             return
         }
 
-        let action: Root.Action = .infoEffect(.openURL(url))
+        let action: Root.Action = .menuAction(.infoEffect(.openURL(url)))
         let effect: Root.Effect?
         do {
+            XCTAssertNil(try root.update(with: .tokenListAction(.showBackupInfo)))
             effect = try root.update(with: action)
         } catch {
             XCTFail("Unexpected error: \(error)")