Merge pull request #693 from kiwix/692-fix-tab-selection-post-delete

Fix tab selection post deletion

CHANGELOG.md

@@ -13,6 +13,7 @@
 - UPDATE: Improved README file (@kelson42 #533 #683 @BPerlakiH #658)
 - UPDATE: Use latest feed from library.kiwix.org (@BPerlakiH #653)
 - DEL: Old Wikimed related code - is now a proper custom app (@BPerlakiH #636)
+- FIX: iOS tab selection after the selected tab is deleted (@BPerlakiH #692)
 
 ## 3.2
 

Tests/NavigationViewModelTest.swift

@@ -0,0 +1,27 @@
+//
+//  NavigationViewModelTest.swift
+//  UnitTests
+
+import XCTest
+@testable import Kiwix
+
+final class NavigationViewModelTest: XCTestCase {
+
+    func testCloseByNavItem() throws {
+        let noItems: [Int] = []
+        XCTAssertNil(noItems.closeBy { $0 == 1 })
+
+        let onlyItem = [1]
+        XCTAssertNil(onlyItem.closeBy { $0 == 1 })
+        XCTAssertEqual(onlyItem.closeBy { $0 == 9 }, 1)
+
+        let items = [1, 2, 3, 4, 5]
+        XCTAssertEqual(items.closeBy { $0 == 1 }, 2)
+        XCTAssertEqual(items.closeBy { $0 == 2 }, 1)
+        XCTAssertEqual(items.closeBy { $0 == 3 }, 2)
+        XCTAssertEqual(items.closeBy { $0 == 4 }, 3)
+        XCTAssertEqual(items.closeBy { $0 == 5 }, 4)
+        XCTAssertEqual(items.closeBy { $0 == 9 }, 5)
+    }
+
+}

ViewModel/NavigationViewModel.swift

@@ -20,6 +20,8 @@ class NavigationViewModel: ObservableObject {
         let tab = Tab(context: context)
         tab.created = Date()
         tab.lastOpened = Date()
+        try? context.obtainPermanentIDs(for: [tab])
+        try? context.save()
         return tab
     }
     
@@ -28,8 +30,6 @@ class NavigationViewModel: ObservableObject {
         let fetchRequest = Tab.fetchRequest(sortDescriptors: [NSSortDescriptor(key: "lastOpened", ascending: false)])
         fetchRequest.fetchLimit = 1
         let tab = (try? context.fetch(fetchRequest).first) ?? self.makeTab(context: context)
-        try? context.obtainPermanentIDs(for: [tab])
-        try? context.save()
         Task {
             await MainActor.run {
                 currentItem = NavigationItem.tab(objectID: tab.objectID)
@@ -42,8 +42,6 @@ class NavigationViewModel: ObservableObject {
     func createTab() -> NSManagedObjectID {
         let context = Database.viewContext
         let tab = self.makeTab(context: context)
-        try? context.obtainPermanentIDs(for: [tab])
-        try? context.save()
         #if !os(macOS)
         currentItem = NavigationItem.tab(objectID: tab.objectID)
         #endif
@@ -64,25 +62,32 @@ class NavigationViewModel: ObservableObject {
     /// - Parameter tabID: ID of the tab to delete
     func deleteTab(tabID: NSManagedObjectID) {
         Database.performBackgroundTask { context in
-            guard let tab = try? context.existingObject(with: tabID) as? Tab else { return }
+            let sortByCreation = [NSSortDescriptor(key: "created", ascending: false)]
-            
+            guard let tabs: [Tab] = try? context.fetch(Tab.fetchRequest(predicate: nil,
-            // select a new tab if the currently selected tab is being deleted
+                                                                        sortDescriptors: sortByCreation)),
+                  let tab: Tab = tabs.first(where: { $0.objectID == tabID }) else {
+                return
+            }
+            let newlySelectedTab: Tab?
+            // select a closeBy tab if the currently selected tab is to be deleted
             if case let .tab(selectedTabID) = self.currentItem, selectedTabID == tabID {
-                let fetchRequest = Tab.fetchRequest(
+                newlySelectedTab = tabs.closeBy(toWhere: { $0.objectID == tabID }) ?? self.makeTab(context: context)
-                    predicate: NSPredicate(format: "created < %@", tab.created as CVarArg),
+            } else {
-                    sortDescriptors: [NSSortDescriptor(key: "created", ascending: false)]
+                newlySelectedTab = nil // the current selection should remain
-                )
-                fetchRequest.fetchLimit = 1
-                let newTab = (try? context.fetch(fetchRequest).first) ?? self.makeTab(context: context)
-                try? context.obtainPermanentIDs(for: [newTab])
-                DispatchQueue.main.async {
-                    self.currentItem = NavigationItem.tab(objectID: newTab.objectID)
-                }
             }
 
             // delete tab
             context.delete(tab)
             try? context.save()
+
+            // update selection if needed
+            if let newlySelectedTab {
+                Task {
+                    await MainActor.run {
+                        self.currentItem = NavigationItem.tab(objectID: newlySelectedTab.objectID)
+                    }
+                }
+            }
         }
     }
     
@@ -95,12 +100,38 @@ class NavigationViewModel: ObservableObject {
             
             // create new tab
             let newTab = self.makeTab(context: context)
-            try? context.obtainPermanentIDs(for: [newTab])
+            Task {
-            DispatchQueue.main.async {
+                await MainActor.run {
-                self.currentItem = NavigationItem.tab(objectID: newTab.objectID)
+                    self.currentItem = NavigationItem.tab(objectID: newTab.objectID)
+                }
             }
-            
-            try? context.save()
         }
     }
 }
+
+extension Array {
+
+    /// Return an element close to the one defined in the where callback,
+    /// either the one before or if this is the first, the one after
+    /// - Parameter toWhere: similar role as in find(where: ) closure, this element is never returned
+    /// - Returns: the element before or the one after, never the one that matches by toWhere:
+    func closeBy(toWhere whereCallback: @escaping (Element) -> Bool) -> Element? {
+        var previous: Element?
+        var returnNext: Bool = false
+        for element in self {
+            if returnNext {
+                return element
+            }
+            if whereCallback(element) {
+                if let previous {
+                    return previous
+                } else {
+                    returnNext = true
+                }
+            } else {
+                previous = element
+            }
+        }
+        return previous
+    }
+}