From c82227f31ad2d8a89de26e5c68dbf9117c72b9b7 Mon Sep 17 00:00:00 2001 From: Juan Manuel Pereira Date: Wed, 24 Jul 2024 09:25:21 -0300 Subject: [PATCH] Sort bookmarks in bookmarks panel (#3001) --- DuckDuckGo.xcodeproj/project.pbxproj | 12 ++ .../xcshareddata/swiftpm/Package.resolved | 4 +- .../BookmarkSortAsc.imageset/Contents.json | 15 ++ .../BookmarkSortAsc.imageset/sort-asc.svg | 7 + .../BookmarkSortDesc.imageset/Contents.json | 15 ++ .../BookmarkSortDesc.imageset/sort-desc.svg | 7 + DuckDuckGo/Bookmarks/Model/Bookmark.swift | 13 ++ ...BookmarkListTreeControllerDataSource.swift | 64 ++++---- ...rkListTreeControllerSearchDataSource.swift | 4 +- .../Model/BookmarkOutlineViewDataSource.swift | 13 +- .../Model/BookmarkSidebarTreeController.swift | 2 +- .../Model/BookmarkTreeController.swift | 26 ++-- .../Services/MenuItemSelectors.swift | 7 + .../View/BookmarkListViewController.swift | 67 ++++++++- ...kmarkManagementSidebarViewController.swift | 10 +- .../ViewModel/SortBookmarksViewModel.swift | 142 ++++++++++++++++++ DuckDuckGo/Common/Localizables/UserText.swift | 5 + DuckDuckGo/Localizable.xcstrings | 60 ++++++++ .../Model/BaseBookmarkEntityTests.swift | 45 ++++++ .../BookmarkOutlineViewDataSourceTests.swift | 32 ++-- .../BookmarkSidebarTreeControllerTests.swift | 8 +- .../Bookmarks/Model/TreeControllerTests.swift | 19 ++- .../ViewModels/BookmarksSortModeTests.swift | 103 +++++++++++++ 23 files changed, 589 insertions(+), 91 deletions(-) create mode 100644 DuckDuckGo/Assets.xcassets/Images/BookmarkSortAsc.imageset/Contents.json create mode 100644 DuckDuckGo/Assets.xcassets/Images/BookmarkSortAsc.imageset/sort-asc.svg create mode 100644 DuckDuckGo/Assets.xcassets/Images/BookmarkSortDesc.imageset/Contents.json create mode 100644 DuckDuckGo/Assets.xcassets/Images/BookmarkSortDesc.imageset/sort-desc.svg create mode 100644 DuckDuckGo/Bookmarks/ViewModel/SortBookmarksViewModel.swift create mode 100644 UnitTests/Bookmarks/ViewModels/BookmarksSortModeTests.swift diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index 5faf4e0802..5d2a9ebfab 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -2537,6 +2537,10 @@ BBB881892C4029BA001247C6 /* BookmarkListTreeControllerSearchDataSource.swift in Sources */ = {isa = PBXBuildFile; fileRef = BBB881872C4029BA001247C6 /* BookmarkListTreeControllerSearchDataSource.swift */; }; BBDFDC5A2B2B8A0900F62D90 /* DataBrokerProtectionExternalWaitlistPixels.swift in Sources */ = {isa = PBXBuildFile; fileRef = BBDFDC592B2B8A0900F62D90 /* DataBrokerProtectionExternalWaitlistPixels.swift */; }; BBDFDC5D2B2B8E2100F62D90 /* DataBrokerProtectionExternalWaitlistPixels.swift in Sources */ = {isa = PBXBuildFile; fileRef = BBDFDC592B2B8A0900F62D90 /* DataBrokerProtectionExternalWaitlistPixels.swift */; }; + BBFB727F2C48047C0088884C /* SortBookmarksViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = BBFB727E2C48047C0088884C /* SortBookmarksViewModel.swift */; }; + BBFB72802C48047C0088884C /* SortBookmarksViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = BBFB727E2C48047C0088884C /* SortBookmarksViewModel.swift */; }; + BBFF355D2C4AF26200DA3289 /* BookmarksSortModeTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = BBFF355C2C4AF26200DA3289 /* BookmarksSortModeTests.swift */; }; + BBFF355E2C4AF26200DA3289 /* BookmarksSortModeTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = BBFF355C2C4AF26200DA3289 /* BookmarksSortModeTests.swift */; }; BD384AC92BBC821A00EF3735 /* vpn-dark-mode.json in Resources */ = {isa = PBXBuildFile; fileRef = BD384AC82BBC821100EF3735 /* vpn-dark-mode.json */; }; BD384ACA2BBC821A00EF3735 /* vpn-light-mode.json in Resources */ = {isa = PBXBuildFile; fileRef = BD384AC72BBC821100EF3735 /* vpn-light-mode.json */; }; BD384ACB2BBC821B00EF3735 /* vpn-dark-mode.json in Resources */ = {isa = PBXBuildFile; fileRef = BD384AC82BBC821100EF3735 /* vpn-dark-mode.json */; }; @@ -4213,6 +4217,8 @@ BB5789712B2CA70F0009DFE2 /* DataBrokerProtectionSubscriptionEventHandler.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DataBrokerProtectionSubscriptionEventHandler.swift; sourceTree = ""; }; BBB881872C4029BA001247C6 /* BookmarkListTreeControllerSearchDataSource.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BookmarkListTreeControllerSearchDataSource.swift; sourceTree = ""; }; BBDFDC592B2B8A0900F62D90 /* DataBrokerProtectionExternalWaitlistPixels.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DataBrokerProtectionExternalWaitlistPixels.swift; sourceTree = ""; }; + BBFB727E2C48047C0088884C /* SortBookmarksViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SortBookmarksViewModel.swift; sourceTree = ""; }; + BBFF355C2C4AF26200DA3289 /* BookmarksSortModeTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BookmarksSortModeTests.swift; sourceTree = ""; }; BD384AC72BBC821100EF3735 /* vpn-light-mode.json */ = {isa = PBXFileReference; lastKnownFileType = text.json; path = "vpn-light-mode.json"; sourceTree = ""; }; BD384AC82BBC821100EF3735 /* vpn-dark-mode.json */ = {isa = PBXFileReference; lastKnownFileType = text.json; path = "vpn-dark-mode.json"; sourceTree = ""; }; BDA7647B2BC497BE00D0400C /* DefaultVPNLocationFormatter.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = DefaultVPNLocationFormatter.swift; sourceTree = ""; }; @@ -6654,6 +6660,7 @@ 9F26060D2B85E17D00819292 /* AddEditBookmarkDialogCoordinatorViewModelTests.swift */, 9F0FFFB32BCCAE37007C87DD /* BookmarkAllTabsDialogCoordinatorViewModelTests.swift */, 9FA5A0A82BC900FC00153786 /* BookmarkAllTabsDialogViewModelTests.swift */, + BBFF355C2C4AF26200DA3289 /* BookmarksSortModeTests.swift */, ); path = ViewModels; sourceTree = ""; @@ -7479,6 +7486,7 @@ 9FEE986C2B85BA17002E44E8 /* AddEditBookmarkDialogCoordinatorViewModel.swift */, 9F9C4A002BC7F36D0099738D /* BookmarkAllTabsDialogCoordinatorViewModel.swift */, 9F9C49FC2BC7E9820099738D /* BookmarkAllTabsDialogViewModel.swift */, + BBFB727E2C48047C0088884C /* SortBookmarksViewModel.swift */, ); path = ViewModel; sourceTree = ""; @@ -10008,6 +10016,7 @@ 4BF0E5062AD2551A00FFEC9E /* NetworkProtectionPixelEvent.swift in Sources */, 3706FAF8293F65D500E42796 /* URLEventHandler.swift in Sources */, 9FBD84742BB3E15D00220859 /* InstallationAttributionPixelHandler.swift in Sources */, + BBFB72802C48047C0088884C /* SortBookmarksViewModel.swift in Sources */, 37197EA72942443D00394917 /* AuthenticationAlert.swift in Sources */, 3706FEC3293F6F0600E42796 /* BWCommunicator.swift in Sources */, 3706FAFA293F65D500E42796 /* CleanThisHistoryMenuItem.swift in Sources */, @@ -10850,6 +10859,7 @@ 3706FE49293F661700E42796 /* BookmarkNodePathTests.swift in Sources */, 1DE03425298BC7F000CAB3D7 /* InternalUserDeciderStoreMock.swift in Sources */, 3706FE4A293F661700E42796 /* BookmarkManagedObjectTests.swift in Sources */, + BBFF355E2C4AF26200DA3289 /* BookmarksSortModeTests.swift in Sources */, EEC8EB402982CD550065AA39 /* JSAlertViewModelTests.swift in Sources */, BDA764922BC4E57200D0400C /* MockVPNLocationFormatter.swift in Sources */, 3706FE4B293F661700E42796 /* BookmarksHTMLImporterTests.swift in Sources */, @@ -11711,6 +11721,7 @@ B6E6B9E32BA1F5F1008AA7E1 /* FilePresenter.swift in Sources */, B6CC266C2BAD9CD800F53F8D /* FileProgressPresenter.swift in Sources */, 3158B14D2B0BF74D00AF130C /* DataBrokerProtectionManager.swift in Sources */, + BBFB727F2C48047C0088884C /* SortBookmarksViewModel.swift in Sources */, 4BA1A6A5258B07DF00F6F690 /* EncryptedValueTransformer.swift in Sources */, B634DBE1293C8FD500C3C99E /* Tab+Dialogs.swift in Sources */, 4B92929F26670D2A00AD2C21 /* PasteboardBookmark.swift in Sources */, @@ -12230,6 +12241,7 @@ 37CD54BB27F25A4000F1F7B9 /* DownloadsPreferencesTests.swift in Sources */, 4BE344EE2B2376DF003FC223 /* VPNFeedbackFormViewModelTests.swift in Sources */, 9F872D9D2B9058D000138637 /* Bookmarks+TabTests.swift in Sources */, + BBFF355D2C4AF26200DA3289 /* BookmarksSortModeTests.swift in Sources */, 9F3910622B68C35600CB5112 /* DownloadsTabExtensionTests.swift in Sources */, 4B9DB0562A983B55000927DB /* MockNotificationService.swift in Sources */, 4B02199C25E063DE00ED7DEA /* FireproofDomainsTests.swift in Sources */, diff --git a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index e463d65deb..6ab63bc872 100644 --- a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -131,8 +131,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/apple/swift-argument-parser.git", "state" : { - "revision" : "0fbc8848e389af3bb55c182bc19ca9d5dc2f255b", - "version" : "1.4.0" + "revision" : "41982a3656a71c768319979febd796c6fd111d5c", + "version" : "1.5.0" } }, { diff --git a/DuckDuckGo/Assets.xcassets/Images/BookmarkSortAsc.imageset/Contents.json b/DuckDuckGo/Assets.xcassets/Images/BookmarkSortAsc.imageset/Contents.json new file mode 100644 index 0000000000..a5e151c20f --- /dev/null +++ b/DuckDuckGo/Assets.xcassets/Images/BookmarkSortAsc.imageset/Contents.json @@ -0,0 +1,15 @@ +{ + "images" : [ + { + "filename" : "sort-asc.svg", + "idiom" : "universal" + } + ], + "info" : { + "author" : "xcode", + "version" : 1 + }, + "properties" : { + "template-rendering-intent" : "template" + } +} diff --git a/DuckDuckGo/Assets.xcassets/Images/BookmarkSortAsc.imageset/sort-asc.svg b/DuckDuckGo/Assets.xcassets/Images/BookmarkSortAsc.imageset/sort-asc.svg new file mode 100644 index 0000000000..89b922f2a2 --- /dev/null +++ b/DuckDuckGo/Assets.xcassets/Images/BookmarkSortAsc.imageset/sort-asc.svg @@ -0,0 +1,7 @@ + + + + + + + diff --git a/DuckDuckGo/Assets.xcassets/Images/BookmarkSortDesc.imageset/Contents.json b/DuckDuckGo/Assets.xcassets/Images/BookmarkSortDesc.imageset/Contents.json new file mode 100644 index 0000000000..a338c75ce9 --- /dev/null +++ b/DuckDuckGo/Assets.xcassets/Images/BookmarkSortDesc.imageset/Contents.json @@ -0,0 +1,15 @@ +{ + "images" : [ + { + "filename" : "sort-desc.svg", + "idiom" : "universal" + } + ], + "info" : { + "author" : "xcode", + "version" : 1 + }, + "properties" : { + "template-rendering-intent" : "template" + } +} diff --git a/DuckDuckGo/Assets.xcassets/Images/BookmarkSortDesc.imageset/sort-desc.svg b/DuckDuckGo/Assets.xcassets/Images/BookmarkSortDesc.imageset/sort-desc.svg new file mode 100644 index 0000000000..0c40784402 --- /dev/null +++ b/DuckDuckGo/Assets.xcassets/Images/BookmarkSortDesc.imageset/sort-desc.svg @@ -0,0 +1,7 @@ + + + + + + + diff --git a/DuckDuckGo/Bookmarks/Model/Bookmark.swift b/DuckDuckGo/Bookmarks/Model/Bookmark.swift index 4d3cf399d8..f4cd289182 100644 --- a/DuckDuckGo/Bookmarks/Model/Bookmark.swift +++ b/DuckDuckGo/Bookmarks/Model/Bookmark.swift @@ -275,3 +275,16 @@ final class Bookmark: BaseBookmarkEntity { } } + +extension Array where Element == BaseBookmarkEntity { + func sorted(by sortMode: BookmarksSortMode) -> [BaseBookmarkEntity] { + switch sortMode { + case .manual: + return self + case .nameAscending: + return self.sorted { $0.title.localizedCaseInsensitiveCompare($1.title) == .orderedAscending } + case .nameDescending: + return self.sorted { $0.title.localizedCaseInsensitiveCompare($1.title) == .orderedDescending } + } + } +} diff --git a/DuckDuckGo/Bookmarks/Model/BookmarkListTreeControllerDataSource.swift b/DuckDuckGo/Bookmarks/Model/BookmarkListTreeControllerDataSource.swift index e600584ebb..acef6ed832 100644 --- a/DuckDuckGo/Bookmarks/Model/BookmarkListTreeControllerDataSource.swift +++ b/DuckDuckGo/Bookmarks/Model/BookmarkListTreeControllerDataSource.swift @@ -26,35 +26,37 @@ final class BookmarkListTreeControllerDataSource: BookmarkTreeControllerDataSour self.bookmarkManager = bookmarkManager } - func treeController(childNodesFor node: BookmarkNode) -> [BookmarkNode] { - return node.isRoot ? childNodesForRootNode(node) : childNodes(node) + func treeController(childNodesFor node: BookmarkNode, sortMode: BookmarksSortMode) -> [BookmarkNode] { + return node.isRoot ? childNodesForRootNode(node, for: sortMode) : childNodes(node, for: sortMode) } // MARK: - Private - private func childNodesForRootNode(_ node: BookmarkNode) -> [BookmarkNode] { - let topLevelNodes = bookmarkManager.list?.topLevelEntities.compactMap { (item) -> BookmarkNode? in - if let folder = item as? BookmarkFolder { - let itemNode = node.createChildNode(item) - itemNode.canHaveChildNodes = !folder.children.isEmpty - - return itemNode - } else if item is Bookmark { - let itemNode = node.findOrCreateChildNode(with: item) - itemNode.canHaveChildNodes = false - return itemNode - } else { - assertionFailure("\(#file): Tried to display non-bookmark type in bookmark list") - return nil - } - } ?? [] + private func childNodesForRootNode(_ node: BookmarkNode, for sortMode: BookmarksSortMode) -> [BookmarkNode] { + let topLevelNodes = bookmarkManager.list?.topLevelEntities + .sorted(by: sortMode) + .compactMap { (item) -> BookmarkNode? in + if let folder = item as? BookmarkFolder { + let itemNode = node.createChildNode(item) + itemNode.canHaveChildNodes = !folder.children.isEmpty + + return itemNode + } else if item is Bookmark { + let itemNode = node.findOrCreateChildNode(with: item) + itemNode.canHaveChildNodes = false + return itemNode + } else { + assertionFailure("\(#file): Tried to display non-bookmark type in bookmark list") + return nil + } + } ?? [] return topLevelNodes } - private func childNodes(_ node: BookmarkNode) -> [BookmarkNode] { + private func childNodes(_ node: BookmarkNode, for sortMode: BookmarksSortMode) -> [BookmarkNode] { if let folder = node.representedObject as? BookmarkFolder { - return childNodes(for: folder, parentNode: node) + return childNodes(for: folder, parentNode: node, sortMode: sortMode) } return [] @@ -72,20 +74,22 @@ final class BookmarkListTreeControllerDataSource: BookmarkTreeControllerDataSour return node } - private func childNodes(for folder: BookmarkFolder, parentNode: BookmarkNode) -> [BookmarkNode] { + private func childNodes(for folder: BookmarkFolder, parentNode: BookmarkNode, sortMode: BookmarksSortMode) -> [BookmarkNode] { var updatedChildNodes = [BookmarkNode]() - folder.children.forEach { representedObject in - if let existingNode = parentNode.childNodeRepresenting(object: representedObject) { - if !updatedChildNodes.contains(existingNode) { - updatedChildNodes += [existingNode] - return + folder.children + .sorted(by: sortMode) + .forEach { representedObject in + if let existingNode = parentNode.childNodeRepresenting(object: representedObject) { + if !updatedChildNodes.contains(existingNode) { + updatedChildNodes += [existingNode] + return + } } - } - let newNode = self.createNode(representedObject, parent: parentNode) - updatedChildNodes += [newNode] - } + let newNode = self.createNode(representedObject, parent: parentNode) + updatedChildNodes += [newNode] + } return updatedChildNodes } diff --git a/DuckDuckGo/Bookmarks/Model/BookmarkListTreeControllerSearchDataSource.swift b/DuckDuckGo/Bookmarks/Model/BookmarkListTreeControllerSearchDataSource.swift index 5b629281bc..c7f9afbcd1 100644 --- a/DuckDuckGo/Bookmarks/Model/BookmarkListTreeControllerSearchDataSource.swift +++ b/DuckDuckGo/Bookmarks/Model/BookmarkListTreeControllerSearchDataSource.swift @@ -25,10 +25,10 @@ final class BookmarkListTreeControllerSearchDataSource: BookmarkTreeControllerSe self.bookmarkManager = bookmarkManager } - func nodes(for searchQuery: String) -> [BookmarkNode] { + func nodes(for searchQuery: String, sortMode: BookmarksSortMode) -> [BookmarkNode] { let searchResults = bookmarkManager.search(by: searchQuery) - return rebuildChildNodes(for: searchResults) + return rebuildChildNodes(for: searchResults.sorted(by: sortMode)) } private func rebuildChildNodes(for results: [BaseBookmarkEntity]) -> [BookmarkNode] { diff --git a/DuckDuckGo/Bookmarks/Model/BookmarkOutlineViewDataSource.swift b/DuckDuckGo/Bookmarks/Model/BookmarkOutlineViewDataSource.swift index 076d181f9e..20adffa41e 100644 --- a/DuckDuckGo/Bookmarks/Model/BookmarkOutlineViewDataSource.swift +++ b/DuckDuckGo/Bookmarks/Model/BookmarkOutlineViewDataSource.swift @@ -51,6 +51,7 @@ final class BookmarkOutlineViewDataSource: NSObject, NSOutlineViewDataSource, NS contentMode: ContentMode, bookmarkManager: BookmarkManager, treeController: BookmarkTreeController, + sortMode: BookmarksSortMode, showMenuButtonOnHover: Bool = true, onMenuRequestedAction: ((BookmarkOutlineCellView) -> Void)? = nil, presentFaviconsFetcherOnboarding: (() -> Void)? = nil @@ -64,20 +65,20 @@ final class BookmarkOutlineViewDataSource: NSObject, NSOutlineViewDataSource, NS super.init() - reloadData() + reloadData(with: sortMode) } - func reloadData() { + func reloadData(with sortMode: BookmarksSortMode) { isSearching = false dragDestinationFolderInSearchMode = nil setFolderCount() - treeController.rebuild() + treeController.rebuild(for: sortMode) } - func reloadData(for searchQuery: String) { + func reloadData(for searchQuery: String, and sortMode: BookmarksSortMode) { isSearching = true setFolderCount() - treeController.rebuild(for: searchQuery) + treeController.rebuild(for: searchQuery, sortMode: sortMode) } private func setFolderCount() { @@ -270,7 +271,7 @@ final class BookmarkOutlineViewDataSource: NSObject, NSOutlineViewDataSource, NS let containsDescendantOfDestination = draggedFolders.contains { draggedFolder in let folder = BookmarkFolder(id: draggedFolder.id, title: draggedFolder.name, parentFolderUUID: draggedFolder.parentFolderUUID, children: draggedFolder.children) - guard let draggedNode = treeController.node(representing: folder) else { + guard let draggedNode = treeController.findNodeWithId(representing: folder) else { return false } diff --git a/DuckDuckGo/Bookmarks/Model/BookmarkSidebarTreeController.swift b/DuckDuckGo/Bookmarks/Model/BookmarkSidebarTreeController.swift index 8de2705759..7c168ff5f9 100644 --- a/DuckDuckGo/Bookmarks/Model/BookmarkSidebarTreeController.swift +++ b/DuckDuckGo/Bookmarks/Model/BookmarkSidebarTreeController.swift @@ -20,7 +20,7 @@ import Foundation final class BookmarkSidebarTreeController: BookmarkTreeControllerDataSource { - func treeController(childNodesFor node: BookmarkNode) -> [BookmarkNode] { + func treeController(childNodesFor node: BookmarkNode, sortMode: BookmarksSortMode) -> [BookmarkNode] { return node.isRoot ? childNodesForRootNode(node) : childNodes(for: node) } diff --git a/DuckDuckGo/Bookmarks/Model/BookmarkTreeController.swift b/DuckDuckGo/Bookmarks/Model/BookmarkTreeController.swift index e8306c7939..ecf145ba66 100644 --- a/DuckDuckGo/Bookmarks/Model/BookmarkTreeController.swift +++ b/DuckDuckGo/Bookmarks/Model/BookmarkTreeController.swift @@ -20,12 +20,12 @@ import Foundation protocol BookmarkTreeControllerDataSource: AnyObject { - func treeController(childNodesFor: BookmarkNode) -> [BookmarkNode] + func treeController(childNodesFor: BookmarkNode, sortMode: BookmarksSortMode) -> [BookmarkNode] } protocol BookmarkTreeControllerSearchDataSource: AnyObject { - func nodes(for searchQuery: String) -> [BookmarkNode] + func nodes(for searchQuery: String, sortMode: BookmarksSortMode) -> [BookmarkNode] } final class BookmarkTreeController { @@ -36,28 +36,30 @@ final class BookmarkTreeController { private weak var searchDataSource: BookmarkTreeControllerSearchDataSource? init(dataSource: BookmarkTreeControllerDataSource, + sortMode: BookmarksSortMode, searchDataSource: BookmarkTreeControllerSearchDataSource? = nil, rootNode: BookmarkNode) { self.dataSource = dataSource self.searchDataSource = searchDataSource self.rootNode = rootNode - rebuild() + rebuild(for: sortMode) } convenience init(dataSource: BookmarkTreeControllerDataSource, + sortMode: BookmarksSortMode, searchDataSource: BookmarkTreeControllerSearchDataSource? = nil) { - self.init(dataSource: dataSource, searchDataSource: searchDataSource, rootNode: BookmarkNode.genericRootNode()) + self.init(dataSource: dataSource, sortMode: sortMode, searchDataSource: searchDataSource, rootNode: BookmarkNode.genericRootNode()) } // MARK: - Public - func rebuild(for searchQuery: String) { - rootNode.childNodes = searchDataSource?.nodes(for: searchQuery) ?? [] + func rebuild(for searchQuery: String, sortMode: BookmarksSortMode) { + rootNode.childNodes = searchDataSource?.nodes(for: searchQuery, sortMode: sortMode) ?? [] } - func rebuild() { - rebuildChildNodes(node: rootNode) + func rebuild(for sortMode: BookmarksSortMode) { + rebuildChildNodes(node: rootNode, sortMode: sortMode) } func visitNodes(with visitBlock: (BookmarkNode) -> Void) { @@ -68,7 +70,7 @@ final class BookmarkTreeController { return nodeInArrayRepresentingObject(nodes: [rootNode]) { $0.representedObjectEquals(object) } } - func findNodeInSearchMode(representing object: AnyObject) -> BookmarkNode? { + func findNodeWithId(representing object: AnyObject) -> BookmarkNode? { return nodeInArrayRepresentingObject(nodes: [rootNode]) { $0.representedObjectHasSameId(object) } } @@ -98,12 +100,12 @@ final class BookmarkTreeController { } @discardableResult - private func rebuildChildNodes(node: BookmarkNode) -> Bool { + private func rebuildChildNodes(node: BookmarkNode, sortMode: BookmarksSortMode = .manual) -> Bool { guard node.canHaveChildNodes else { return false } - let childNodes: [BookmarkNode] = dataSource?.treeController(childNodesFor: node) ?? [] + let childNodes: [BookmarkNode] = dataSource?.treeController(childNodesFor: node, sortMode: sortMode) ?? [] var childNodesDidChange = childNodes != node.childNodes if childNodesDidChange { @@ -111,7 +113,7 @@ final class BookmarkTreeController { } childNodes.forEach { childNode in - if rebuildChildNodes(node: childNode) { + if rebuildChildNodes(node: childNode, sortMode: sortMode) { childNodesDidChange = true } } diff --git a/DuckDuckGo/Bookmarks/Services/MenuItemSelectors.swift b/DuckDuckGo/Bookmarks/Services/MenuItemSelectors.swift index 3e3c399d1f..9f54209b45 100644 --- a/DuckDuckGo/Bookmarks/Services/MenuItemSelectors.swift +++ b/DuckDuckGo/Bookmarks/Services/MenuItemSelectors.swift @@ -49,3 +49,10 @@ import AppKit func showInFolder(_ sender: NSMenuItem) } + +@objc protocol BookmarkSortMenuItemSelectors { + + func manualSort(_ sender: NSMenuItem) + func sortByNameAscending(_ sender: NSMenuItem) + func sortByNameDescending(_ sender: NSMenuItem) +} diff --git a/DuckDuckGo/Bookmarks/View/BookmarkListViewController.swift b/DuckDuckGo/Bookmarks/View/BookmarkListViewController.swift index dd62b0e685..dd977760f3 100644 --- a/DuckDuckGo/Bookmarks/View/BookmarkListViewController.swift +++ b/DuckDuckGo/Bookmarks/View/BookmarkListViewController.swift @@ -71,6 +71,7 @@ final class BookmarkListViewController: NSViewController { private lazy var newBookmarkButton = MouseOverButton(image: .addBookmark, target: self, action: #selector(newBookmarkButtonClicked)) private lazy var newFolderButton = MouseOverButton(image: .addFolder, target: self, action: #selector(newFolderButtonClicked)) private lazy var searchBookmarksButton = MouseOverButton(image: .searchBookmarks, target: self, action: #selector(searchBookmarkButtonClicked)) + private lazy var sortBookmarksButton = MouseOverButton(image: .bookmarkSortAsc, target: self, action: #selector(sortBookmarksButtonClicked)) private var isSearchVisible = false private lazy var buttonsDivider = NSBox() @@ -92,8 +93,10 @@ final class BookmarkListViewController: NSViewController { private let bookmarkManager: BookmarkManager private let treeControllerDataSource: BookmarkListTreeControllerDataSource private let treeControllerSearchDataSource: BookmarkListTreeControllerSearchDataSource + private let sortBookmarksViewModel = SortBookmarksViewModel() private lazy var treeController = BookmarkTreeController(dataSource: treeControllerDataSource, + sortMode: sortBookmarksViewModel.selectedSortMode, searchDataSource: treeControllerSearchDataSource) private lazy var dataSource: BookmarkOutlineViewDataSource = { @@ -101,6 +104,7 @@ final class BookmarkListViewController: NSViewController { contentMode: .bookmarksAndFolders, bookmarkManager: bookmarkManager, treeController: treeController, + sortMode: sortBookmarksViewModel.selectedSortMode, onMenuRequestedAction: { [weak self] cell in self?.showContextMenu(for: cell) }, @@ -168,6 +172,7 @@ final class BookmarkListViewController: NSViewController { stackView.translatesAutoresizingMaskIntoConstraints = false stackView.addArrangedSubview(newBookmarkButton) stackView.addArrangedSubview(newFolderButton) + stackView.addArrangedSubview(sortBookmarksButton) stackView.addArrangedSubview(searchBookmarksButton) stackView.addArrangedSubview(buttonsDivider) stackView.addArrangedSubview(manageBookmarksButton) @@ -196,6 +201,14 @@ final class BookmarkListViewController: NSViewController { searchBookmarksButton.translatesAutoresizingMaskIntoConstraints = false searchBookmarksButton.toolTip = UserText.bookmarksSearch + sortBookmarksButton.bezelStyle = .shadowlessSquare + sortBookmarksButton.cornerRadius = 4 + sortBookmarksButton.normalTintColor = .button + sortBookmarksButton.mouseDownColor = .buttonMouseDown + sortBookmarksButton.mouseOverColor = .buttonMouseOver + sortBookmarksButton.translatesAutoresizingMaskIntoConstraints = false + sortBookmarksButton.toolTip = UserText.bookmarksSort + searchBar.translatesAutoresizingMaskIntoConstraints = false searchBar.delegate = self @@ -308,6 +321,9 @@ final class BookmarkListViewController: NSViewController { searchBookmarksButton.heightAnchor.constraint(equalToConstant: 28).isActive = true searchBookmarksButton.widthAnchor.constraint(equalToConstant: 28).isActive = true + sortBookmarksButton.heightAnchor.constraint(equalToConstant: 28).isActive = true + sortBookmarksButton.widthAnchor.constraint(equalToConstant: 28).isActive = true + buttonsDivider.widthAnchor.constraint(equalToConstant: 13).isActive = true buttonsDivider.heightAnchor.constraint(equalToConstant: 18).isActive = true @@ -383,6 +399,19 @@ final class BookmarkListViewController: NSViewController { self.outlineView.isHidden = false } }.store(in: &cancellables) + + sortBookmarksViewModel.$selectedSortMode.sink { [weak self] newSortMode in + guard let self else { return } + + switch newSortMode { + case .nameDescending: + self.sortBookmarksButton.image = .bookmarkSortDesc + default: + self.sortBookmarksButton.image = .bookmarkSortAsc + } + + self.setupSort(mode: newSortMode) + }.store(in: &cancellables) } override func viewWillAppear() { @@ -410,13 +439,13 @@ final class BookmarkListViewController: NSViewController { hideSearchBar() updateSearchAndExpand(destinationFolder) } else { - dataSource.reloadData(for: searchBar.stringValue) + dataSource.reloadData(for: searchBar.stringValue, and: sortBookmarksViewModel.selectedSortMode) outlineView.reloadData() } } else { let selectedNodes = self.selectedNodes - dataSource.reloadData() + dataSource.reloadData(with: sortBookmarksViewModel.selectedSortMode) outlineView.reloadData() expandAndRestore(selectedNodes: selectedNodes) @@ -451,6 +480,11 @@ final class BookmarkListViewController: NSViewController { } } + @objc func sortBookmarksButtonClicked(_ sender: NSButton) { + let menu = sortBookmarksViewModel.selectedSortMode.menu + menu.popUpAtMouseLocation(in: sortBookmarksButton) + } + private func showSearchBar() { view.addSubview(searchBar) @@ -473,6 +507,14 @@ final class BookmarkListViewController: NSViewController { searchBookmarksButton.mouseOverColor = .buttonMouseOver } + private func setupSort(mode: BookmarksSortMode) { + hideSearchBar() + dataSource.reloadData(with: mode) + outlineView.reloadData() + sortBookmarksButton.backgroundColor = mode.shouldHighlightButton ? .buttonMouseDown : .clear + sortBookmarksButton.mouseOverColor = mode.shouldHighlightButton ? .buttonMouseDown : .buttonMouseOver + } + @objc func openManagementInterface(_ sender: NSButton) { showManageBookmarks() } @@ -506,7 +548,7 @@ final class BookmarkListViewController: NSViewController { } private func expandFoldersAndScrollUntil(_ folder: BookmarkFolder) { - guard let folderNode = treeController.findNodeInSearchMode(representing: folder) else { + guard let folderNode = treeController.findNodeWithId(representing: folder) else { return } @@ -534,7 +576,7 @@ final class BookmarkListViewController: NSViewController { private func showTreeView() { emptyState.isHidden = true outlineView.isHidden = false - dataSource.reloadData() + dataSource.reloadData(with: sortBookmarksViewModel.selectedSortMode) outlineView.reloadData() } @@ -830,6 +872,21 @@ extension BookmarkListViewController: BookmarkSearchMenuItemSelectors { } } +extension BookmarkListViewController: BookmarkSortMenuItemSelectors { + + func manualSort(_ sender: NSMenuItem) { + sortBookmarksViewModel.selectedSortMode = .manual + } + + func sortByNameAscending(_ sender: NSMenuItem) { + sortBookmarksViewModel.selectedSortMode = .nameAscending + } + + func sortByNameDescending(_ sender: NSMenuItem) { + sortBookmarksViewModel.selectedSortMode = .nameDescending + } +} + // MARK: - Search field delegate extension BookmarkListViewController: NSSearchFieldDelegate { @@ -847,7 +904,7 @@ extension BookmarkListViewController: NSSearchFieldDelegate { } private func showSearch(for searchQuery: String) { - dataSource.reloadData(for: searchQuery) + dataSource.reloadData(for: searchQuery, and: sortBookmarksViewModel.selectedSortMode) if dataSource.treeController.rootNode.childNodes.isEmpty { showEmptyStateView(for: .noSearchResults) diff --git a/DuckDuckGo/Bookmarks/View/BookmarkManagementSidebarViewController.swift b/DuckDuckGo/Bookmarks/View/BookmarkManagementSidebarViewController.swift index 6621046849..5b8b3e01d0 100644 --- a/DuckDuckGo/Bookmarks/View/BookmarkManagementSidebarViewController.swift +++ b/DuckDuckGo/Bookmarks/View/BookmarkManagementSidebarViewController.swift @@ -50,8 +50,12 @@ final class BookmarkManagementSidebarViewController: NSViewController { private lazy var scrollView = NSScrollView(frame: NSRect(x: 0, y: 0, width: 232, height: 410)) private lazy var outlineView = BookmarksOutlineView(frame: scrollView.frame) - private lazy var treeController = BookmarkTreeController(dataSource: treeControllerDataSource) - private lazy var dataSource = BookmarkOutlineViewDataSource(contentMode: .foldersOnly, bookmarkManager: bookmarkManager, treeController: treeController, showMenuButtonOnHover: false) + private lazy var treeController = BookmarkTreeController(dataSource: treeControllerDataSource, sortMode: .manual) + private lazy var dataSource = BookmarkOutlineViewDataSource(contentMode: .foldersOnly, + bookmarkManager: bookmarkManager, + treeController: treeController, + sortMode: .manual, + showMenuButtonOnHover: false) private var cancellables = Set() @@ -184,7 +188,7 @@ final class BookmarkManagementSidebarViewController: NSViewController { private func reloadData() { let selectedNodes = self.selectedNodes - dataSource.reloadData() + dataSource.reloadData(with: .manual) outlineView.reloadData() expandAndRestore(selectedNodes: selectedNodes) diff --git a/DuckDuckGo/Bookmarks/ViewModel/SortBookmarksViewModel.swift b/DuckDuckGo/Bookmarks/ViewModel/SortBookmarksViewModel.swift new file mode 100644 index 0000000000..0884c46f15 --- /dev/null +++ b/DuckDuckGo/Bookmarks/ViewModel/SortBookmarksViewModel.swift @@ -0,0 +1,142 @@ +// +// SortBookmarksViewModel.swift +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Foundation +import AppKit +import Combine + +enum BookmarksSortMode: Codable { + case manual + case nameAscending + case nameDescending + + var title: String { + switch self { + case .manual: + return UserText.bookmarksSortManual + case .nameAscending: + return UserText.bookmarksSortByNameAscending + case .nameDescending: + return UserText.bookmarksSortByNameDescending + } + } + + var action: Selector { + switch self { + case .manual: + return #selector(BookmarkSortMenuItemSelectors.manualSort(_:)) + case .nameAscending: + return #selector(BookmarkSortMenuItemSelectors.sortByNameAscending(_:)) + case .nameDescending: + return #selector(BookmarkSortMenuItemSelectors.sortByNameDescending(_:)) + } + } + + var shouldHighlightButton: Bool { + return self != .manual + } + + var menu: NSMenu { + switch self { + case .manual: + return NSMenu(items: [ + menuItem(for: .manual, state: .on), + sortByName(state: .off), + NSMenuItem.separator(), + menuItem(for: .nameAscending, state: .off, disabled: true), + menuItem(for: .nameDescending, state: .off, disabled: true) + ]) + case .nameAscending: + return NSMenu(items: [ + menuItem(for: .manual, state: .off), + sortByName(state: .on), + NSMenuItem.separator(), + menuItem(for: .nameAscending, state: .on), + menuItem(for: .nameDescending, state: .off) + ]) + case .nameDescending: + return NSMenu(items: [ + menuItem(for: .manual, state: .off), + sortByName(state: .on), + NSMenuItem.separator(), + menuItem(for: .nameAscending, state: .off), + menuItem(for: .nameDescending, state: .on) + ]) + } + } + + private func menuItem(for mode: BookmarksSortMode, state: NSControl.StateValue, disabled: Bool = false) -> NSMenuItem { + return NSMenuItem(title: mode.title, action: disabled ? nil : mode.action, state: state) + } + + private func sortByName(state: NSControl.StateValue) -> NSMenuItem { + return NSMenuItem(title: UserText.bookmarksSortByName, action: BookmarksSortMode.nameAscending.action, state: state) + } +} + +protocol SortBookmarksRepository { + + var storedSortMode: BookmarksSortMode { get set } +} + +final class SortBookmarksUserDefaults: SortBookmarksRepository { + + private enum Keys { + static let sortMode = "com.duckduckgo.bookmarks.sort.mode" + } + + private let userDefaults: UserDefaults + + init(userDefaults: UserDefaults = .standard) { + self.userDefaults = userDefaults + } + + var storedSortMode: BookmarksSortMode { + get { + if let data = userDefaults.data(forKey: Keys.sortMode), + let mode = try? JSONDecoder().decode(BookmarksSortMode.self, from: data) { + return mode + } + // Default value if not set + return .manual + } + set { + if let data = try? JSONEncoder().encode(newValue) { + userDefaults.set(data, forKey: Keys.sortMode) + } + } + } +} + +final class SortBookmarksViewModel { + + private var repository: SortBookmarksRepository + + @Published + var selectedSortMode: BookmarksSortMode = .manual { + didSet { + repository.storedSortMode = selectedSortMode + } + } + + init(repository: SortBookmarksRepository = SortBookmarksUserDefaults()) { + self.repository = repository + + selectedSortMode = repository.storedSortMode + } +} diff --git a/DuckDuckGo/Common/Localizables/UserText.swift b/DuckDuckGo/Common/Localizables/UserText.swift index cad4398d67..455f229d06 100644 --- a/DuckDuckGo/Common/Localizables/UserText.swift +++ b/DuckDuckGo/Common/Localizables/UserText.swift @@ -998,6 +998,11 @@ struct UserText { static let manageBookmarksTooltip = NSLocalizedString("tooltip.bookmarks.manage-bookmarks", value: "Manage bookmarks", comment: "Tooltip for the Manage Bookmarks button") static let bookmarksManage = NSLocalizedString("bookmarks.manage", value: "Manage", comment: "Button for opening the bookmarks management interface") static let bookmarksSearch = NSLocalizedString("tooltip.bookmarks.search", value: "Search bookmarks", comment: "Tooltip to activate the bookmark search") + static let bookmarksSort = NSLocalizedString("tooltip.bookmarks.sort", value: "Sort bookmarks", comment: "Tooltip to activate the bookmark sort") + static let bookmarksSortManual = NSLocalizedString("bookmarks.sort.manual", value: "Manual", comment: "Button to sort bookmarks by manual") + static let bookmarksSortByName = NSLocalizedString("bookmarks.sort.name", value: "Name", comment: "Button to sort bookmarks by name ascending") + static let bookmarksSortByNameAscending = NSLocalizedString("bookmarks.sort.name.asc", value: "Ascending", comment: "Button to sort bookmarks by name ascending") + static let bookmarksSortByNameDescending = NSLocalizedString("bookmarks.sort.name.desc", value: "Descending", comment: "Button to sort bookmarks by name descending") static let bookmarksEmptyStateTitle = NSLocalizedString("bookmarks.empty.state.title", value: "No bookmarks yet", comment: "Title displayed in Bookmark Manager when there is no bookmarks yet") static let bookmarksEmptyStateMessage = NSLocalizedString("bookmarks.empty.state.message", value: "If your bookmarks are saved in another browser, you can import them into DuckDuckGo.", comment: "Text displayed in Bookmark Manager when there is no bookmarks yet") diff --git a/DuckDuckGo/Localizable.xcstrings b/DuckDuckGo/Localizable.xcstrings index 4a8c0a517a..7732284f49 100644 --- a/DuckDuckGo/Localizable.xcstrings +++ b/DuckDuckGo/Localizable.xcstrings @@ -11342,6 +11342,54 @@ } } }, + "bookmarks.sort.manual" : { + "comment" : "Button to sort bookmarks by manual", + "extractionState" : "extracted_with_value", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "new", + "value" : "Manual" + } + } + } + }, + "bookmarks.sort.name" : { + "comment" : "Button to sort bookmarks by name ascending", + "extractionState" : "extracted_with_value", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "new", + "value" : "Name" + } + } + } + }, + "bookmarks.sort.name.asc" : { + "comment" : "Button to sort bookmarks by name ascending", + "extractionState" : "extracted_with_value", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "new", + "value" : "Ascending" + } + } + } + }, + "bookmarks.sort.name.desc" : { + "comment" : "Button to sort bookmarks by name descending", + "extractionState" : "extracted_with_value", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "new", + "value" : "Descending" + } + } + } + }, "Bring All to Front" : { "comment" : "Main Menu Window item", "localizations" : { @@ -55017,6 +55065,18 @@ } } }, + "tooltip.bookmarks.sort" : { + "comment" : "Tooltip to activate the bookmark sort", + "extractionState" : "extracted_with_value", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "new", + "value" : "Sort bookmarks" + } + } + } + }, "tooltip.clearHistory" : { "comment" : "Tooltip for burn button where %@ is the domain", "extractionState" : "extracted_with_value", diff --git a/UnitTests/Bookmarks/Model/BaseBookmarkEntityTests.swift b/UnitTests/Bookmarks/Model/BaseBookmarkEntityTests.swift index 11ffefab31..5951f89c92 100644 --- a/UnitTests/Bookmarks/Model/BaseBookmarkEntityTests.swift +++ b/UnitTests/Bookmarks/Model/BaseBookmarkEntityTests.swift @@ -244,4 +244,49 @@ final class BaseBookmarkEntityTests: XCTestCase { XCTAssertFalse(result) } + func testWhenSortingByManualModeThenBookmarksAreReturnedInOriginalOrder() { + // GIVEN + let bookmark = Bookmark(id: "1", url: URL.duckDuckGo.absoluteString, title: "Test 3", isFavorite: true) + let folder = BookmarkFolder(id: "2", title: "Test 1") + let bookmarkTwo = Bookmark(id: "3", url: URL.duckDuckGo.absoluteString, title: "Test 2", isFavorite: false) + + // WHEN + let sut = [bookmark, folder, bookmarkTwo].sorted(by: .manual) + + // THEN + XCTAssertEqual(sut[0], bookmark) + XCTAssertEqual(sut[1], folder) + XCTAssertEqual(sut[2], bookmarkTwo) + } + + func testWhenSortingByNameAscThenBookmarksAreReturnedByAscendingTitle() { + // GIVEN + let bookmark = Bookmark(id: "1", url: URL.duckDuckGo.absoluteString, title: "Test 3", isFavorite: true) + let folder = BookmarkFolder(id: "2", title: "Test 1") + let bookmarkTwo = Bookmark(id: "3", url: URL.duckDuckGo.absoluteString, title: "Test 2", isFavorite: false) + + // WHEN + let sut = [bookmark, folder, bookmarkTwo].sorted(by: .nameAscending) + + // THEN + XCTAssertEqual(sut[0], folder) + XCTAssertEqual(sut[1], bookmarkTwo) + XCTAssertEqual(sut[2], bookmark) + } + + func testWhenSortingByNameDescThenBookmarksAreReturnedByDescendingTitle() { + // GIVEN + let bookmark = Bookmark(id: "1", url: URL.duckDuckGo.absoluteString, title: "Test 3", isFavorite: true) + let folder = BookmarkFolder(id: "2", title: "Test 1") + let bookmarkTwo = Bookmark(id: "3", url: URL.duckDuckGo.absoluteString, title: "Test 2", isFavorite: false) + + // WHEN + let sut = [bookmark, folder, bookmarkTwo].sorted(by: .nameDescending) + + // THEN + XCTAssertEqual(sut[0], bookmark) + XCTAssertEqual(sut[1], bookmarkTwo) + XCTAssertEqual(sut[2], folder) + } + } diff --git a/UnitTests/Bookmarks/Model/BookmarkOutlineViewDataSourceTests.swift b/UnitTests/Bookmarks/Model/BookmarkOutlineViewDataSourceTests.swift index d1cc5fa4d8..148c98f870 100644 --- a/UnitTests/Bookmarks/Model/BookmarkOutlineViewDataSourceTests.swift +++ b/UnitTests/Bookmarks/Model/BookmarkOutlineViewDataSourceTests.swift @@ -26,7 +26,7 @@ class BookmarkOutlineViewDataSourceTests: XCTestCase { let mockFolder = BookmarkFolder.mock let treeController = createTreeController(with: [mockFolder]) let mockFolderNode = treeController.node(representing: mockFolder)! - let dataSource = BookmarkOutlineViewDataSource(contentMode: .foldersOnly, bookmarkManager: LocalBookmarkManager(), treeController: treeController) + let dataSource = BookmarkOutlineViewDataSource(contentMode: .foldersOnly, bookmarkManager: LocalBookmarkManager(), treeController: treeController, sortMode: .manual) let notification = Notification(name: NSOutlineView.itemDidExpandNotification, object: nil, userInfo: ["NSObject": mockFolderNode]) dataSource.outlineViewItemDidExpand(notification) @@ -38,7 +38,7 @@ class BookmarkOutlineViewDataSourceTests: XCTestCase { let mockFolder = BookmarkFolder.mock let treeController = createTreeController(with: [mockFolder]) let mockFolderNode = treeController.node(representing: mockFolder)! - let dataSource = BookmarkOutlineViewDataSource(contentMode: .foldersOnly, bookmarkManager: LocalBookmarkManager(), treeController: treeController) + let dataSource = BookmarkOutlineViewDataSource(contentMode: .foldersOnly, bookmarkManager: LocalBookmarkManager(), treeController: treeController, sortMode: .manual) let expandNotification = Notification(name: NSOutlineView.itemDidExpandNotification, object: nil, userInfo: ["NSObject": mockFolderNode]) dataSource.outlineViewItemDidExpand(expandNotification) @@ -56,7 +56,7 @@ class BookmarkOutlineViewDataSourceTests: XCTestCase { let mockOutlineView = NSOutlineView(frame: .zero) let treeController = createTreeController(with: [mockFolder]) let mockFolderNode = treeController.node(representing: mockFolder)! - let dataSource = BookmarkOutlineViewDataSource(contentMode: .foldersOnly, bookmarkManager: LocalBookmarkManager(), treeController: treeController) + let dataSource = BookmarkOutlineViewDataSource(contentMode: .foldersOnly, bookmarkManager: LocalBookmarkManager(), treeController: treeController, sortMode: .manual) let writer = dataSource.outlineView(mockOutlineView, pasteboardWriterForItem: mockFolderNode) as? FolderPasteboardWriter XCTAssertNotNil(writer) @@ -69,7 +69,7 @@ class BookmarkOutlineViewDataSourceTests: XCTestCase { let mockFolder = BookmarkFolder.mock let mockOutlineView = NSOutlineView(frame: .zero) let treeController = createTreeController(with: [mockFolder]) - let dataSource = BookmarkOutlineViewDataSource(contentMode: .foldersOnly, bookmarkManager: LocalBookmarkManager(), treeController: treeController) + let dataSource = BookmarkOutlineViewDataSource(contentMode: .foldersOnly, bookmarkManager: LocalBookmarkManager(), treeController: treeController, sortMode: .manual) let spacerNode = BookmarkNode(representedObject: SpacerNode.blank, parent: nil) let writer = dataSource.outlineView(mockOutlineView, pasteboardWriterForItem: spacerNode) as? FolderPasteboardWriter @@ -86,9 +86,9 @@ class BookmarkOutlineViewDataSourceTests: XCTestCase { bookmarkManager.loadBookmarks() let treeDataSource = BookmarkSidebarTreeController(bookmarkManager: bookmarkManager) - let treeController = BookmarkTreeController(dataSource: treeDataSource) + let treeController = BookmarkTreeController(dataSource: treeDataSource, sortMode: .manual) let mockDestinationNode = treeController.node(representing: mockDestinationFolder)! - let dataSource = BookmarkOutlineViewDataSource(contentMode: .foldersOnly, bookmarkManager: bookmarkManager, treeController: treeController) + let dataSource = BookmarkOutlineViewDataSource(contentMode: .foldersOnly, bookmarkManager: LocalBookmarkManager(), treeController: treeController, sortMode: .manual) let pasteboardBookmark = PasteboardBookmark(id: UUID().uuidString, url: "https://example.com", title: "Pasteboard Bookmark") let result = dataSource.validateDrop(for: [pasteboardBookmark], destination: mockDestinationNode) @@ -106,9 +106,9 @@ class BookmarkOutlineViewDataSourceTests: XCTestCase { bookmarkManager.loadBookmarks() let treeDataSource = BookmarkSidebarTreeController(bookmarkManager: bookmarkManager) - let treeController = BookmarkTreeController(dataSource: treeDataSource) + let treeController = BookmarkTreeController(dataSource: treeDataSource, sortMode: .manual) let mockDestinationNode = treeController.node(representing: mockDestinationFolder)! - let dataSource = BookmarkOutlineViewDataSource(contentMode: .foldersOnly, bookmarkManager: bookmarkManager, treeController: treeController) + let dataSource = BookmarkOutlineViewDataSource(contentMode: .foldersOnly, bookmarkManager: LocalBookmarkManager(), treeController: treeController, sortMode: .manual) let pasteboardFolder = PasteboardFolder(folder: .init(id: UUID().uuidString, title: "Pasteboard Folder")) let result = dataSource.validateDrop(for: [pasteboardFolder], destination: mockDestinationNode) @@ -126,8 +126,8 @@ class BookmarkOutlineViewDataSourceTests: XCTestCase { bookmarkManager.loadBookmarks() let treeDataSource = BookmarkSidebarTreeController(bookmarkManager: bookmarkManager) - let treeController = BookmarkTreeController(dataSource: treeDataSource) - let dataSource = BookmarkOutlineViewDataSource(contentMode: .foldersOnly, bookmarkManager: bookmarkManager, treeController: treeController) + let treeController = BookmarkTreeController(dataSource: treeDataSource, sortMode: .manual) + let dataSource = BookmarkOutlineViewDataSource(contentMode: .foldersOnly, bookmarkManager: LocalBookmarkManager(), treeController: treeController, sortMode: .manual) let mockDestinationNode = treeController.node(representing: mockDestinationFolder)! let pasteboardFolder = PasteboardFolder(folder: mockDestinationFolder) @@ -148,8 +148,8 @@ class BookmarkOutlineViewDataSourceTests: XCTestCase { bookmarkManager.loadBookmarks() let treeDataSource = BookmarkSidebarTreeController(bookmarkManager: bookmarkManager) - let treeController = BookmarkTreeController(dataSource: treeDataSource) - let dataSource = BookmarkOutlineViewDataSource(contentMode: .foldersOnly, bookmarkManager: bookmarkManager, treeController: treeController) + let treeController = BookmarkTreeController(dataSource: treeDataSource, sortMode: .manual) + let dataSource = BookmarkOutlineViewDataSource(contentMode: .foldersOnly, bookmarkManager: LocalBookmarkManager(), treeController: treeController, sortMode: .manual) let mockDestinationNode = treeController.node(representing: childFolder)! // Simulate dragging the root folder onto the child folder: @@ -167,7 +167,7 @@ class BookmarkOutlineViewDataSourceTests: XCTestCase { let mockFolderNode = treeController.node(representing: mockFolder)! var didFireClosure = false var capturedCell: BookmarkOutlineCellView? - let dataSource = BookmarkOutlineViewDataSource(contentMode: .foldersOnly, bookmarkManager: LocalBookmarkManager(), treeController: treeController) { cell in + let dataSource = BookmarkOutlineViewDataSource(contentMode: .foldersOnly, bookmarkManager: LocalBookmarkManager(), treeController: treeController, sortMode: .manual) { cell in didFireClosure = true capturedCell = cell } @@ -187,7 +187,7 @@ class BookmarkOutlineViewDataSourceTests: XCTestCase { let mockOutlineView = NSOutlineView(frame: .zero) let treeController = createTreeController(with: [mockFolder]) let mockFolderNode = treeController.node(representing: mockFolder)! - let dataSource = BookmarkOutlineViewDataSource(contentMode: .bookmarksAndFolders, bookmarkManager: LocalBookmarkManager(), treeController: treeController, showMenuButtonOnHover: true) + let dataSource = BookmarkOutlineViewDataSource(contentMode: .bookmarksAndFolders, bookmarkManager: LocalBookmarkManager(), treeController: treeController, sortMode: .manual, showMenuButtonOnHover: true) // WHEN let cell = try XCTUnwrap(dataSource.outlineView(mockOutlineView, viewFor: nil, item: mockFolderNode) as? BookmarkOutlineCellView) @@ -202,7 +202,7 @@ class BookmarkOutlineViewDataSourceTests: XCTestCase { let mockOutlineView = NSOutlineView(frame: .zero) let treeController = createTreeController(with: [mockFolder]) let mockFolderNode = treeController.node(representing: mockFolder)! - let dataSource = BookmarkOutlineViewDataSource(contentMode: .foldersOnly, bookmarkManager: LocalBookmarkManager(), treeController: treeController, showMenuButtonOnHover: false) + let dataSource = BookmarkOutlineViewDataSource(contentMode: .foldersOnly, bookmarkManager: LocalBookmarkManager(), treeController: treeController, sortMode: .manual, showMenuButtonOnHover: false) // WHEN let cell = try XCTUnwrap(dataSource.outlineView(mockOutlineView, viewFor: nil, item: mockFolderNode) as? BookmarkOutlineCellView) @@ -222,7 +222,7 @@ class BookmarkOutlineViewDataSourceTests: XCTestCase { bookmarkManager.loadBookmarks() let treeDataSource = BookmarkSidebarTreeController(bookmarkManager: bookmarkManager) - return BookmarkTreeController(dataSource: treeDataSource) + return BookmarkTreeController(dataSource: treeDataSource, sortMode: .manual) } } diff --git a/UnitTests/Bookmarks/Model/BookmarkSidebarTreeControllerTests.swift b/UnitTests/Bookmarks/Model/BookmarkSidebarTreeControllerTests.swift index 907ae7a08d..0638af297e 100644 --- a/UnitTests/Bookmarks/Model/BookmarkSidebarTreeControllerTests.swift +++ b/UnitTests/Bookmarks/Model/BookmarkSidebarTreeControllerTests.swift @@ -24,7 +24,7 @@ class BookmarkSidebarTreeControllerTests: XCTestCase { func testWhenBookmarkStoreHasNoFolders_ThenOnlyDefaultNodesAreReturned() { let dataSource = BookmarkSidebarTreeController(bookmarkManager: LocalBookmarkManager()) - let treeController = BookmarkTreeController(dataSource: dataSource) + let treeController = BookmarkTreeController(dataSource: dataSource, sortMode: .manual) let defaultNodes = treeController.rootNode.childNodes let representedObjects = defaultNodes.representedObjects() @@ -48,7 +48,7 @@ class BookmarkSidebarTreeControllerTests: XCTestCase { bookmarkManager.loadBookmarks() let dataSource = BookmarkSidebarTreeController(bookmarkManager: bookmarkManager) - let treeController = BookmarkTreeController(dataSource: dataSource) + let treeController = BookmarkTreeController(dataSource: dataSource, sortMode: .manual) let defaultNodes = treeController.rootNode.childNodes XCTAssertEqual(defaultNodes.count, 1) @@ -69,7 +69,7 @@ class BookmarkSidebarTreeControllerTests: XCTestCase { bookmarkManager.loadBookmarks() let dataSource = BookmarkSidebarTreeController(bookmarkManager: bookmarkManager) - let treeController = BookmarkTreeController(dataSource: dataSource) + let treeController = BookmarkTreeController(dataSource: dataSource, sortMode: .manual) let defaultNodes = treeController.rootNode.childNodes XCTAssertEqual(defaultNodes.count, 1) @@ -92,7 +92,7 @@ class BookmarkSidebarTreeControllerTests: XCTestCase { bookmarkManager.loadBookmarks() let dataSource = BookmarkSidebarTreeController(bookmarkManager: bookmarkManager) - let treeController = BookmarkTreeController(dataSource: dataSource) + let treeController = BookmarkTreeController(dataSource: dataSource, sortMode: .manual) let defaultNodes = treeController.rootNode.childNodes XCTAssertEqual(defaultNodes.count, 1) diff --git a/UnitTests/Bookmarks/Model/TreeControllerTests.swift b/UnitTests/Bookmarks/Model/TreeControllerTests.swift index 82b98960a8..95012536d7 100644 --- a/UnitTests/Bookmarks/Model/TreeControllerTests.swift +++ b/UnitTests/Bookmarks/Model/TreeControllerTests.swift @@ -21,16 +21,15 @@ import XCTest private class MockTreeControllerDataSource: BookmarkTreeControllerDataSource { - func treeController(childNodesFor node: BookmarkNode) -> [BookmarkNode] { + func treeController(childNodesFor node: BookmarkNode, sortMode: BookmarksSortMode) -> [BookmarkNode] { return node.childNodes } } private final class MockTreeControllerSearchDataSource: BookmarkTreeControllerSearchDataSource { - var returnNodes: [BookmarkNode] = [] - func nodes(for searchQuery: String) -> [BookmarkNode] { + func nodes(for searchQuery: String, sortMode: BookmarksSortMode) -> [BookmarkNode] { return returnNodes } } @@ -42,14 +41,14 @@ class TreeControllerTests: XCTestCase { func testWhenInitializingTreeControllerWithRootNode_ThenRootNodeIsSet() { let dataSource = MockTreeControllerDataSource() let node = BookmarkNode(representedObject: TestObject(), parent: nil) - let treeController = BookmarkTreeController(dataSource: dataSource, rootNode: node) + let treeController = BookmarkTreeController(dataSource: dataSource, sortMode: .manual, rootNode: node) XCTAssertEqual(treeController.rootNode, node) } func testWhenInitializingTreeControllerWithoutRootNode_ThenGenericRootNodeIsCreated() { let dataSource = MockTreeControllerDataSource() - let treeController = BookmarkTreeController(dataSource: dataSource) + let treeController = BookmarkTreeController(dataSource: dataSource, sortMode: .manual) XCTAssertTrue(treeController.rootNode.canHaveChildNodes) } @@ -65,7 +64,7 @@ class TreeControllerTests: XCTestCase { rootNode.childNodes = [firstChildNode, secondChildNode] let dataSource = MockTreeControllerDataSource() - let treeController = BookmarkTreeController(dataSource: dataSource, rootNode: rootNode) + let treeController = BookmarkTreeController(dataSource: dataSource, sortMode: .manual, rootNode: rootNode) let foundNode = treeController.node(representing: desiredObject) XCTAssertEqual(foundNode, secondChildNode) @@ -80,7 +79,7 @@ class TreeControllerTests: XCTestCase { rootNode.childNodes = [firstChildNode, secondChildNode] let dataSource = MockTreeControllerDataSource() - let treeController = BookmarkTreeController(dataSource: dataSource, rootNode: rootNode) + let treeController = BookmarkTreeController(dataSource: dataSource, sortMode: .manual, rootNode: rootNode) let foundNode = treeController.node(representing: TestObject()) XCTAssertNil(foundNode) @@ -95,7 +94,7 @@ class TreeControllerTests: XCTestCase { rootNode.childNodes = [firstChildNode, secondChildNode] let dataSource = MockTreeControllerDataSource() - let treeController = BookmarkTreeController(dataSource: dataSource, rootNode: rootNode) + let treeController = BookmarkTreeController(dataSource: dataSource, sortMode: .manual, rootNode: rootNode) var visitedNodes = Set() @@ -113,9 +112,9 @@ class TreeControllerTests: XCTestCase { let dataSource = MockTreeControllerDataSource() let searchDataSource = MockTreeControllerSearchDataSource() searchDataSource.returnNodes = [firstNode, secondNode, thirdNode] - let sut = BookmarkTreeController(dataSource: dataSource, searchDataSource: searchDataSource) + let sut = BookmarkTreeController(dataSource: dataSource, sortMode: .manual, searchDataSource: searchDataSource) - sut.rebuild(for: "some search query") + sut.rebuild(for: "some search query", sortMode: .manual) XCTAssertEqual(sut.rootNode.childNodes.count, 3) } diff --git a/UnitTests/Bookmarks/ViewModels/BookmarksSortModeTests.swift b/UnitTests/Bookmarks/ViewModels/BookmarksSortModeTests.swift new file mode 100644 index 0000000000..08f31bf0e6 --- /dev/null +++ b/UnitTests/Bookmarks/ViewModels/BookmarksSortModeTests.swift @@ -0,0 +1,103 @@ +// +// BookmarksSortModeTests.swift +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import XCTest +@testable import DuckDuckGo_Privacy_Browser + +class BookmarksSortModeTests: XCTestCase { + + func testTitle() { + XCTAssertEqual(BookmarksSortMode.manual.title, UserText.bookmarksSortManual) + XCTAssertEqual(BookmarksSortMode.nameAscending.title, UserText.bookmarksSortByNameAscending) + XCTAssertEqual(BookmarksSortMode.nameDescending.title, UserText.bookmarksSortByNameDescending) + } + + func testAction() { + XCTAssertEqual(BookmarksSortMode.manual.action, #selector(BookmarkSortMenuItemSelectors.manualSort(_:))) + XCTAssertEqual(BookmarksSortMode.nameAscending.action, #selector(BookmarkSortMenuItemSelectors.sortByNameAscending(_:))) + XCTAssertEqual(BookmarksSortMode.nameDescending.action, #selector(BookmarkSortMenuItemSelectors.sortByNameDescending(_:))) + } + + func testShouldHighlightButton() { + XCTAssertFalse(BookmarksSortMode.manual.shouldHighlightButton) + XCTAssertTrue(BookmarksSortMode.nameAscending.shouldHighlightButton) + XCTAssertTrue(BookmarksSortMode.nameDescending.shouldHighlightButton) + } + + func testMenuForManual() { + let manualMenu = BookmarksSortMode.manual.menu + + XCTAssertEqual(manualMenu.items.count, 5) + + XCTAssertEqual(manualMenu.items[0].title, UserText.bookmarksSortManual) + XCTAssertEqual(manualMenu.items[0].state, .on) + + XCTAssertEqual(manualMenu.items[1].title, UserText.bookmarksSortByName) + XCTAssertEqual(manualMenu.items[1].state, .off) + + XCTAssert(manualMenu.items[2].isSeparatorItem) + + XCTAssertEqual(manualMenu.items[3].title, UserText.bookmarksSortByNameAscending) + XCTAssertEqual(manualMenu.items[3].state, .off) + XCTAssertNil(manualMenu.items[3].action) + + XCTAssertEqual(manualMenu.items[4].title, UserText.bookmarksSortByNameDescending) + XCTAssertEqual(manualMenu.items[4].state, .off) + XCTAssertNil(manualMenu.items[4].action) + } + + func testMenuForNameAscending() { + let ascendingMenu = BookmarksSortMode.nameAscending.menu + + XCTAssertEqual(ascendingMenu.items.count, 5) + + XCTAssertEqual(ascendingMenu.items[0].title, UserText.bookmarksSortManual) + XCTAssertEqual(ascendingMenu.items[0].state, .off) + + XCTAssertEqual(ascendingMenu.items[1].title, UserText.bookmarksSortByName) + XCTAssertEqual(ascendingMenu.items[1].state, .on) + + XCTAssert(ascendingMenu.items[2].isSeparatorItem) + + XCTAssertEqual(ascendingMenu.items[3].title, UserText.bookmarksSortByNameAscending) + XCTAssertEqual(ascendingMenu.items[3].state, .on) + + XCTAssertEqual(ascendingMenu.items[4].title, UserText.bookmarksSortByNameDescending) + XCTAssertEqual(ascendingMenu.items[4].state, .off) + } + + func testMenuForNameDescending() { + let descendingMenu = BookmarksSortMode.nameDescending.menu + + XCTAssertEqual(descendingMenu.items.count, 5) + + XCTAssertEqual(descendingMenu.items[0].title, UserText.bookmarksSortManual) + XCTAssertEqual(descendingMenu.items[0].state, .off) + + XCTAssertEqual(descendingMenu.items[1].title, UserText.bookmarksSortByName) + XCTAssertEqual(descendingMenu.items[1].state, .on) + + XCTAssert(descendingMenu.items[2].isSeparatorItem) + + XCTAssertEqual(descendingMenu.items[3].title, UserText.bookmarksSortByNameAscending) + XCTAssertEqual(descendingMenu.items[3].state, .off) + + XCTAssertEqual(descendingMenu.items[4].title, UserText.bookmarksSortByNameDescending) + XCTAssertEqual(descendingMenu.items[4].state, .on) + } +}