From fb2c5250dcf07909b245b27bdce74c7b5639ae03 Mon Sep 17 00:00:00 2001 From: Balazs Perlaki-Horvath Date: Sat, 16 Mar 2024 10:01:17 +0100 Subject: [PATCH 1/3] Fix tab selection post deletion --- Tests/NavigationViewModelTest.swift | 27 ++++++++++ ViewModel/NavigationViewModel.swift | 77 ++++++++++++++++++++--------- 2 files changed, 80 insertions(+), 24 deletions(-) create mode 100644 Tests/NavigationViewModelTest.swift diff --git a/Tests/NavigationViewModelTest.swift b/Tests/NavigationViewModelTest.swift new file mode 100644 index 00000000..6db2815c --- /dev/null +++ b/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) + } + +} diff --git a/ViewModel/NavigationViewModel.swift b/ViewModel/NavigationViewModel.swift index 04e1ec92..3701668b 100644 --- a/ViewModel/NavigationViewModel.swift +++ b/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,30 @@ 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 } - - // select a new tab if the currently selected tab is being deleted - if case let .tab(selectedTabID) = self.currentItem, selectedTabID == tabID { - let fetchRequest = Tab.fetchRequest( - predicate: NSPredicate(format: "created < %@", tab.created as CVarArg), - sortDescriptors: [NSSortDescriptor(key: "created", ascending: false)] - ) - 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) - } + guard let tabs: [Tab] = try? context.fetch(Tab.fetchRequest()), + 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 { + newlySelectedTab = tabs.closeBy(toWhere: { $0.objectID == tabID }) ?? self.makeTab(context: context) + } else { + newlySelectedTab = nil // the current selection should remain + } + // 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 +98,38 @@ class NavigationViewModel: ObservableObject { // create new tab let newTab = self.makeTab(context: context) - try? context.obtainPermanentIDs(for: [newTab]) - DispatchQueue.main.async { - self.currentItem = NavigationItem.tab(objectID: newTab.objectID) + Task { + await MainActor.run { + 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 + } +} From c5c4507d6ad07d0ad33d84df495e613199ca53cb Mon Sep 17 00:00:00 2001 From: Balazs Perlaki-Horvath Date: Sun, 17 Mar 2024 09:52:59 +0100 Subject: [PATCH 2/3] Add sort to fetching just in case --- ViewModel/NavigationViewModel.swift | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ViewModel/NavigationViewModel.swift b/ViewModel/NavigationViewModel.swift index 3701668b..53abcfe4 100644 --- a/ViewModel/NavigationViewModel.swift +++ b/ViewModel/NavigationViewModel.swift @@ -62,7 +62,9 @@ class NavigationViewModel: ObservableObject { /// - Parameter tabID: ID of the tab to delete func deleteTab(tabID: NSManagedObjectID) { Database.performBackgroundTask { context in - guard let tabs: [Tab] = try? context.fetch(Tab.fetchRequest()), + let sortByCreation = [NSSortDescriptor(key: "created", ascending: false)] + guard let tabs: [Tab] = try? context.fetch(Tab.fetchRequest(predicate: nil, + sortDescriptors: sortByCreation)), let tab: Tab = tabs.first(where: { $0.objectID == tabID }) else { return } From 94acb3dbae6d3d5c8683ff4719f28a3c1bafd878 Mon Sep 17 00:00:00 2001 From: Balazs Perlaki-Horvath Date: Sun, 17 Mar 2024 12:02:31 +0100 Subject: [PATCH 3/3] Update CHAGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a8003d09..dc7b1969 100644 --- a/CHANGELOG.md +++ b/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