update bookmark nav item state between viewDidLoad and Issue Load (#1791)

Author: BrianLitwin

Classes/Bookmark/BookmarkNavigationController.swift

@@ -22,12 +22,19 @@ final class BookmarkNavigationController {
     }
 
     // MARK: Public API
-
+    
     var navigationItem: UIBarButtonItem {
+        let item = UIBarButtonItem()
+        configureNavigationItem(item)
+        return item
+    }
+    
+    func configureNavigationItem(_ item: UIBarButtonItem) {
+        
         let accessibilityLabel: String
         let imageName: String
         let selector: Selector
-
+        
         if store.contains(model) {
             imageName = "nav-bookmark-selected"
             accessibilityLabel = Constants.Strings.removeBookmark
@@ -38,24 +45,34 @@ final class BookmarkNavigationController {
             selector = #selector(BookmarkNavigationController.add(sender:))
         }
 
-        let item = UIBarButtonItem(image: UIImage(named: imageName), target: self, action: selector)
         item.accessibilityLabel = accessibilityLabel
+        item.image = UIImage(named: imageName)?.withRenderingMode(.alwaysTemplate)
+        item.target = self
+        item.action = selector
+        item.isEnabled = true
+    }
+    
+    //for timeframe between viewDidLoad() and bookmark info is loaded 
+    static var disabledNavigationItem: UIBarButtonItem {
+        let item = UIBarButtonItem()
+        item.image = UIImage(named: "nav-bookmark")?.withRenderingMode(.alwaysTemplate)
+        item.isEnabled = false
         return item
     }
-
+    
     // MARK: Private API
-
-    @objc func add(sender: UIButton) {
+    
+    @objc func add(sender: UIBarButtonItem) {
         Haptic.triggerSelection()
-        sender.addTarget(self, action: #selector(BookmarkNavigationController.remove(sender:)), for: .touchUpInside)
-        sender.setImage(UIImage(named: "nav-bookmark-selected")?.withRenderingMode(.alwaysTemplate), for: .normal)
+        sender.action = #selector(BookmarkNavigationController.remove(sender:))
+        sender.image = UIImage(named: "nav-bookmark-selected")?.withRenderingMode(.alwaysTemplate)
         store.add(model)
     }
-
-    @objc func remove(sender: UIButton) {
-        sender.addTarget(self, action: #selector(BookmarkNavigationController.add(sender:)), for: .touchUpInside)
-        sender.setImage(UIImage(named: "nav-bookmark")?.withRenderingMode(.alwaysTemplate), for: .normal)
+    
+    @objc func remove(sender: UIBarButtonItem) {
+        sender.action = #selector(BookmarkNavigationController.add(sender:))
+        sender.image = UIImage(named: "nav-bookmark")?.withRenderingMode(.alwaysTemplate)
         store.remove(model)
     }
-
+    
 }

Classes/Issues/IssuesViewController.swift

@@ -164,7 +164,9 @@ final class IssuesViewController:
         actions.frame = CGRect(x: 0, y: 0, width: 0, height: 40)
         messageView.add(contentView: actions)
 
-        navigationItem.rightBarButtonItem = moreOptionsItem
+        
+        //show disabled bookmark button until issue has finished loading
+        navigationItem.rightBarButtonItems = [ moreOptionsItem, BookmarkNavigationController.disabledNavigationItem ]
     }
 
     override func viewDidAppear(_ animated: Bool) {
@@ -224,11 +226,9 @@ final class IssuesViewController:
     }
 
     func configureNavigationItems() {
-        var items = [moreOptionsItem]
-        if let bookmarkItem = bookmarkNavController?.navigationItem {
-            items.append(bookmarkItem)
-        }
-        navigationItem.rightBarButtonItems = items
+        guard let rightbarButtonItems = navigationItem.rightBarButtonItems else { return }
+        guard let bookmarkItem = rightbarButtonItems.last else { return }
+        bookmarkNavController?.configureNavigationItem(bookmarkItem)
     }
 
     func viewOwnerAction() -> UIAlertAction? {

Freetime.xcodeproj/project.pbxproj

@@ -418,6 +418,7 @@
 		98F9F4001F9CCFFE005A0266 /* ImageUploadTableViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 98F9F3FD1F9CCFFE005A0266 /* ImageUploadTableViewController.swift */; };
 		98F9F4011F9CCFFE005A0266 /* ImgurClient.swift in Sources */ = {isa = PBXBuildFile; fileRef = 98F9F3FE1F9CCFFE005A0266 /* ImgurClient.swift */; };
 		98F9F4031F9CD006005A0266 /* Image+Base64.swift in Sources */ = {isa = PBXBuildFile; fileRef = 98F9F4021F9CD006005A0266 /* Image+Base64.swift */; };
+		BD3761B0209E032500401DFB /* BookmarkNavigationItemTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = BD3761AF209E032500401DFB /* BookmarkNavigationItemTests.swift */; };
 		D8BAD0601FDA0A1A00C41071 /* LabelListCell.swift in Sources */ = {isa = PBXBuildFile; fileRef = D8BAD05F1FDA0A1A00C41071 /* LabelListCell.swift */; };
 		D8BAD0641FDF221900C41071 /* LabelListView.swift in Sources */ = {isa = PBXBuildFile; fileRef = D8BAD0631FDF221900C41071 /* LabelListView.swift */; };
 		D8BAD0661FDF224600C41071 /* WrappingStaticSpacingFlowLayout.swift in Sources */ = {isa = PBXBuildFile; fileRef = D8BAD0651FDF224600C41071 /* WrappingStaticSpacingFlowLayout.swift */; };
@@ -929,6 +930,7 @@
 		A4D6EAEE85A1F4878338D48C /* Pods-FreetimeWatch Extension.debug.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-FreetimeWatch Extension.debug.xcconfig"; path = "Pods/Target Support Files/Pods-FreetimeWatch Extension/Pods-FreetimeWatch Extension.debug.xcconfig"; sourceTree = "<group>"; };
 		ACAE4A11E9671879046F0CE7 /* Pods-FreetimeWatch.testflight.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-FreetimeWatch.testflight.xcconfig"; path = "Pods/Target Support Files/Pods-FreetimeWatch/Pods-FreetimeWatch.testflight.xcconfig"; sourceTree = "<group>"; };
 		B3C439BE890EECD7C0C692C5 /* Pods-Freetime.testflight.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-Freetime.testflight.xcconfig"; path = "Pods/Target Support Files/Pods-Freetime/Pods-Freetime.testflight.xcconfig"; sourceTree = "<group>"; };
+		BD3761AF209E032500401DFB /* BookmarkNavigationItemTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BookmarkNavigationItemTests.swift; sourceTree = "<group>"; };
 		D396E0DA66FED629384A84BC /* Pods_FreetimeWatch.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Pods_FreetimeWatch.framework; sourceTree = BUILT_PRODUCTS_DIR; };
 		D8BAD05F1FDA0A1A00C41071 /* LabelListCell.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LabelListCell.swift; sourceTree = "<group>"; };
 		D8BAD0631FDF221900C41071 /* LabelListView.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = LabelListView.swift; sourceTree = "<group>"; };
@@ -2050,6 +2052,7 @@
 			isa = PBXGroup;
 			children = (
 				DCA5ED181FAEF32F0072F074 /* BookmarkViewModelTests.swift */,
+				BD3761AF209E032500401DFB /* BookmarkNavigationItemTests.swift */,
 			);
 			path = "Bookmark Tests";
 			sourceTree = "<group>";
@@ -3050,6 +3053,7 @@
 				DC60C6CE1F98346400241271 /* MockSearchEmptyViewDelegate.swift in Sources */,
 				DC60C6CB1F98341900241271 /* SearchEmptyViewTests.swift in Sources */,
 				293A45771F296B7E00DD1006 /* ListKitTestCase.swift in Sources */,
+				BD3761B0209E032500401DFB /* BookmarkNavigationItemTests.swift in Sources */,
 				DC5C02C51F9C6E3500E80B9F /* SearchQueryTests.swift in Sources */,
 				293A457E1F296BD500DD1006 /* API.swift in Sources */,
 				49AF91B1204B416500DFF325 /* MergeTests.swift in Sources */,

FreetimeTests/Bookmark Tests/BookmarkNavigationItemTests.swift

@@ -0,0 +1,48 @@
+//
+//  BookmarkNavigationItemTests.swift
+//  FreetimeTests
+//
+//  Created by B_Litwin on 5/5/18.
+//  Copyright © 2018 Ryan Nystrom. All rights reserved.
+//
+
+import XCTest
+@testable import Freetime
+
+class BookmarkNavigationControllerTests: XCTestCase {
+    
+    func test_bookmarkNavItemActionsWork() {
+        
+        //setup
+        
+        let issue = Bookmark(type: .issue, name: "issue 1", owner: "brian", title: "issue 1 tests")
+        let other = Bookmark(type: .commit, name: "issue 2", owner: "litwin")
+        let bookmarkStore = BookmarkStore(token: "")
+        bookmarkStore.values = [other]
+        
+        guard let bookmarkNavigationController = BookmarkNavigationController(store: bookmarkStore, model: issue) else {
+            XCTFail("bookmarkNavigationController could not be initialized")
+            return
+        }
+        
+        let navigationItem = BookmarkNavigationController.disabledNavigationItem
+        bookmarkNavigationController.configureNavigationItem(navigationItem)
+        
+        //test initial button and model state
+        XCTAssertEqual(bookmarkStore.values, [other]) //models state
+        XCTAssertEqual(navigationItem.action, #selector(bookmarkNavigationController.add(sender:))) //item's initial action
+        
+        //test button tap updates model and button action
+        bookmarkNavigationController.add(sender: navigationItem)
+        XCTAssertEqual(navigationItem.action, #selector(bookmarkNavigationController.remove(sender:))) //item's action
+        XCTAssertEqual(bookmarkStore.values, [issue, other]) //model updated
+        
+        //test button tapped again updates model and button action
+        bookmarkNavigationController.remove(sender: navigationItem)
+        XCTAssertEqual(navigationItem.action, #selector(bookmarkNavigationController.add(sender:))) //item's action
+        XCTAssertEqual(bookmarkStore.values, [other]) //model updated
+        
+    }
+    
+}
+