diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index 4d5b1a0e8b..c41c4f9db9 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -2580,6 +2580,7 @@ BB470EBB2C5A66D6002EE91D /* BookmarkManagementDetailViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = BB470EBA2C5A66D6002EE91D /* BookmarkManagementDetailViewModel.swift */; }; BB470EBC2C5A66D6002EE91D /* BookmarkManagementDetailViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = BB470EBA2C5A66D6002EE91D /* BookmarkManagementDetailViewModel.swift */; }; BB5789722B2CA70F0009DFE2 /* DataBrokerProtectionSubscriptionEventHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = BB5789712B2CA70F0009DFE2 /* DataBrokerProtectionSubscriptionEventHandler.swift */; }; + BB5F46A32C8751F6005F72DF /* BookmarkSortTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = BB5F46A22C8751F6005F72DF /* BookmarkSortTests.swift */; }; BB7B5F982C4ED73800BA4AF8 /* BookmarksSearchAndSortMetrics.swift in Sources */ = {isa = PBXBuildFile; fileRef = BB7B5F972C4ED73800BA4AF8 /* BookmarksSearchAndSortMetrics.swift */; }; BB7B5F992C4ED73800BA4AF8 /* BookmarksSearchAndSortMetrics.swift in Sources */ = {isa = PBXBuildFile; fileRef = BB7B5F972C4ED73800BA4AF8 /* BookmarksSearchAndSortMetrics.swift */; }; BBB881882C4029BA001247C6 /* BookmarkListTreeControllerSearchDataSource.swift in Sources */ = {isa = PBXBuildFile; fileRef = BBB881872C4029BA001247C6 /* BookmarkListTreeControllerSearchDataSource.swift */; }; @@ -4331,6 +4332,7 @@ B6FA8940269C425400588ECD /* PrivacyDashboardPopover.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PrivacyDashboardPopover.swift; sourceTree = ""; }; BB470EBA2C5A66D6002EE91D /* BookmarkManagementDetailViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BookmarkManagementDetailViewModel.swift; sourceTree = ""; }; BB5789712B2CA70F0009DFE2 /* DataBrokerProtectionSubscriptionEventHandler.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DataBrokerProtectionSubscriptionEventHandler.swift; sourceTree = ""; }; + BB5F46A22C8751F6005F72DF /* BookmarkSortTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BookmarkSortTests.swift; sourceTree = ""; }; BB7B5F972C4ED73800BA4AF8 /* BookmarksSearchAndSortMetrics.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BookmarksSearchAndSortMetrics.swift; sourceTree = ""; }; BBB881872C4029BA001247C6 /* BookmarkListTreeControllerSearchDataSource.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BookmarkListTreeControllerSearchDataSource.swift; sourceTree = ""; }; BBBB653F2C77BB9400E69AC6 /* BookmarkSearchTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BookmarkSearchTests.swift; sourceTree = ""; }; @@ -6315,6 +6317,7 @@ 7B4CE8E626F02134009134B1 /* TabBarTests.swift */, 56A054522C2592CE007D8FAB /* OnboardingUITests.swift */, BBBB653F2C77BB9400E69AC6 /* BookmarkSearchTests.swift */, + BB5F46A22C8751F6005F72DF /* BookmarkSortTests.swift */, ); path = UITests; sourceTree = ""; @@ -11508,6 +11511,7 @@ buildActionMask = 2147483647; files = ( EEBCE6842BA4643200B9DF00 /* NSSizeExtension.swift in Sources */, + BB5F46A32C8751F6005F72DF /* BookmarkSortTests.swift in Sources */, EE7F74912BB5D76600CD9456 /* BookmarksBarTests.swift in Sources */, EE02D41C2BB460A600DBE6B3 /* BrowsingHistoryTests.swift in Sources */, EE02D41A2BB4609900DBE6B3 /* UITests.swift in Sources */, diff --git a/DuckDuckGo/Bookmarks/View/BookmarkListViewController.swift b/DuckDuckGo/Bookmarks/View/BookmarkListViewController.swift index fb670ee326..d3baebbab8 100644 --- a/DuckDuckGo/Bookmarks/View/BookmarkListViewController.swift +++ b/DuckDuckGo/Bookmarks/View/BookmarkListViewController.swift @@ -45,6 +45,7 @@ final class BookmarkListViewController: NSViewController { private lazy var searchBookmarksButton = MouseOverButton(image: .searchBookmarks, target: self, action: #selector(searchBookmarkButtonClicked)) .withAccessibilityIdentifier("BookmarkListViewController.searchBookmarksButton") private lazy var sortBookmarksButton = MouseOverButton(image: .bookmarkSortAsc, target: self, action: #selector(sortBookmarksButtonClicked)) + .withAccessibilityIdentifier("BookmarkListViewController.sortBookmarksButton") private lazy var buttonsDivider = NSBox() private lazy var manageBookmarksButton = MouseOverButton(title: UserText.bookmarksManage, target: self, action: #selector(openManagementInterface)) diff --git a/DuckDuckGo/Menus/MainMenuActions.swift b/DuckDuckGo/Menus/MainMenuActions.swift index 70371865b5..a2b3583445 100644 --- a/DuckDuckGo/Menus/MainMenuActions.swift +++ b/DuckDuckGo/Menus/MainMenuActions.swift @@ -804,6 +804,7 @@ extension MainViewController { LocalBookmarkManager.shared.resetBookmarks() UserDefaults.standard.set(false, forKey: UserDefaultsWrapper.Key.homePageContinueSetUpImport.rawValue) UserDefaults.standard.set(false, forKey: UserDefaultsWrapper.Key.bookmarksBarPromptShown.rawValue) + LocalBookmarkManager.shared.sortMode = .manual } @objc func resetPinnedTabs(_ sender: Any?) { diff --git a/UITests/BookmarkSearchTests.swift b/UITests/BookmarkSearchTests.swift index 79f2395ddf..e7a7888316 100644 --- a/UITests/BookmarkSearchTests.swift +++ b/UITests/BookmarkSearchTests.swift @@ -66,7 +66,7 @@ class BookmarkSearchTests: XCTestCase { func testFilteredResultsInPanel() { addThreeBookmarks() closeShowBookmarksBarAlert() - openBookmarksPanel() + app.openBookmarksPanel() searchInBookmarksPanel(for: "Bookmark #2") assertOnlyBookmarkExists(on: app.outlines.firstMatch, bookmarkTitle: "Bookmark #2") } @@ -95,7 +95,7 @@ class BookmarkSearchTests: XCTestCase { } func testSearchActionIsHiddenOnBookmarksPanelWhenUserHasNoBookmarks() { - openBookmarksPanel() + app.openBookmarksPanel() let bookmarksPanelPopover = app.popovers.firstMatch XCTAssertFalse(bookmarksPanelPopover.buttons[AccessibilityIdentifiers.searchBookmarksButton].exists) } @@ -110,7 +110,7 @@ class BookmarkSearchTests: XCTestCase { private func addBookmarkAndOpenBookmarksPanel(bookmarkPageTitle: String, in folder: String? = nil) { addBookmark(pageTitle: bookmarkPageTitle, in: folder) closeShowBookmarksBarAlert() - openBookmarksPanel() + app.openBookmarksPanel() } private func addBookmarkAndOpenBookmarksManager(bookmarkPageTitle: String, in folder: String? = nil) { @@ -127,8 +127,7 @@ class BookmarkSearchTests: XCTestCase { private func addBookmark(pageTitle: String, in folder: String? = nil) { let urlForBookmarksBar = UITests.simpleServedPage(titled: pageTitle) - app.openSiteToBookmark(app: app, - url: urlForBookmarksBar, + app.openSiteToBookmark(url: urlForBookmarksBar, pageTitle: pageTitle, bookmarkingViaDialog: true, escapingDialog: true, @@ -181,10 +180,6 @@ class BookmarkSearchTests: XCTestCase { popover.buttons[AccessibilityIdentifiers.searchBookmarksButton].tap() } - private func openBookmarksPanel() { - app.showAndTapBookmarksPanelShortcut() - } - private func openBookmarksManager() { app.openBookmarksManager() } @@ -200,7 +195,7 @@ class BookmarkSearchTests: XCTestCase { closeShowBookmarksBarAlert() if mode == .panel { - openBookmarksPanel() + app.openBookmarksPanel() searchInBookmarksPanel(for: "Bookmark #1") } else { openBookmarksManager() @@ -244,7 +239,7 @@ class BookmarkSearchTests: XCTestCase { addThreeBookmarks() if mode == .panel { closeShowBookmarksBarAlert() - openBookmarksPanel() + app.openBookmarksPanel() } else { openBookmarksManager() } @@ -306,7 +301,7 @@ class BookmarkSearchTests: XCTestCase { } private func createFolderWithSubFolder() { - openBookmarksPanel() + app.openBookmarksPanel() let bookmarksPanel = app.popovers.firstMatch bookmarksPanel.buttons[AccessibilityIdentifiers.newFolderButton].tap() @@ -326,9 +321,3 @@ class BookmarkSearchTests: XCTestCase { app.dismissPopover(buttonIdentifier: "Hide") } } - -// Enum to represent bookmark modes -private enum BookmarkMode { - case panel - case manager -} diff --git a/UITests/BookmarkSortTests.swift b/UITests/BookmarkSortTests.swift new file mode 100644 index 0000000000..70d095058d --- /dev/null +++ b/UITests/BookmarkSortTests.swift @@ -0,0 +1,208 @@ +// +// BookmarkSortTests.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 + +class BookmarkSortTests: XCTestCase { + private var app: XCUIApplication! + + private enum AccessibilityIdentifiers { + static let addressBarTextField = "AddressBarViewController.addressBarTextField" + static let resetBookmarksMenuItem = "MainMenu.resetBookmarks" + static let sortBookmarksButtonPanel = "BookmarkListViewController.sortBookmarksButton" + static let sortBookmarksButtonManager = "BookmarkManagementDetailViewController.sortItemsButton" + } + + override class func setUp() { + UITests.firstRun() + } + + override func setUpWithError() throws { + continueAfterFailure = false + app = XCUIApplication() + app.launchEnvironment["UITEST_MODE"] = "1" + app.launch() + app.resetBookmarks() + app.enforceSingleWindow() + } + + func testWhenChangingSortingInThePanelIsReflectedInTheManager() { + app.openBookmarksPanel() + selectSortByName(mode: .panel) + app.openBookmarksManager() + + app.buttons[AccessibilityIdentifiers.sortBookmarksButtonManager].tap() + + /// If the ascending and descending sort options are enabled, means that the sort in the panel was reflected here. + XCTAssertTrue(app.menuItems["Ascending"].isEnabled) + XCTAssertTrue(app.menuItems["Descending"].isEnabled) + } + + func testWhenChangingSortingInTheManagerIsReflectedInThePanel() { + app.openBookmarksManager() + selectSortByName(mode: .manager) + app.openBookmarksPanel() + + let bookmarksPanelPopover = app.popovers.firstMatch + bookmarksPanelPopover.buttons[AccessibilityIdentifiers.sortBookmarksButtonPanel].tap() + + XCTAssertTrue(bookmarksPanelPopover.menuItems["Ascending"].isEnabled) + XCTAssertTrue(bookmarksPanelPopover.menuItems["Descending"].isEnabled) + } + + func testManualSortWorksAsExpectedOnBookmarksPanel() { + ["Bookmark #2", "Bookmark #3", "Bookmark #1"].forEach { + addBookmark(pageTitle: $0) + app.openNewTab() + } + + closeShowBookmarksBarAlert() + app.openBookmarksPanel() + app.verifyBookmarkOrder(expectedOrder: ["Bookmark #2", "Bookmark #3", "Bookmark #1"], mode: .panel) + } + + func testManualSortWorksAsExpectedOnBookmarksManager() { + ["Bookmark #2", "Bookmark #3", "Bookmark #1"].forEach { + addBookmark(pageTitle: $0) + app.openNewTab() + } + + closeShowBookmarksBarAlert() + app.openBookmarksManager() + app.verifyBookmarkOrder(expectedOrder: ["Bookmark #2", "Bookmark #3", "Bookmark #1"], mode: .manager) + } + + func testNameAscendingSortWorksAsExpectedOnBookmarksPanel() { + ["Bookmark #2", "Bookmark #3", "Bookmark #1"].forEach { + addBookmark(pageTitle: $0) + app.openNewTab() + } + + closeShowBookmarksBarAlert() + app.openBookmarksPanel() + selectSortByName(mode: .panel) + app.verifyBookmarkOrder(expectedOrder: ["Bookmark #1", "Bookmark #2", "Bookmark #3"], mode: .panel) + } + + func testNameAscendingSortWorksAsExpectedOnBookmarksManager() { + ["Bookmark #2", "Bookmark #3", "Bookmark #1"].forEach { + addBookmark(pageTitle: $0) + app.openNewTab() + } + + closeShowBookmarksBarAlert() + app.openBookmarksManager() + selectSortByName(mode: .manager) + app.verifyBookmarkOrder(expectedOrder: ["Bookmark #1", "Bookmark #2", "Bookmark #3"], mode: .manager) + } + + func testNameDescendingSortWorksAsExpectedOnBookmarksPanel() { + ["Bookmark #2", "Bookmark #3", "Bookmark #1"].forEach { + addBookmark(pageTitle: $0) + app.openNewTab() + } + + closeShowBookmarksBarAlert() + app.openBookmarksPanel() + selectSortByName(mode: .panel, descending: true) + app.verifyBookmarkOrder(expectedOrder: ["Bookmark #3", "Bookmark #2", "Bookmark #1"], mode: .panel) + } + + func testNameDescendingSortWorksAsExpectedOnBookmarksManager() { + ["Bookmark #2", "Bookmark #3", "Bookmark #1"].forEach { + addBookmark(pageTitle: $0) + app.openNewTab() + } + + closeShowBookmarksBarAlert() + app.openBookmarksManager() + selectSortByName(mode: .manager, descending: true) + app.verifyBookmarkOrder(expectedOrder: ["Bookmark #3", "Bookmark #2", "Bookmark #1"], mode: .manager) + } + + func testThatSortIsPersistedThroughBrowserRestarts() { + app.openBookmarksPanel() + selectSortByName(mode: .panel) + + app.terminate() + let newApp = XCUIApplication() + newApp.launch() + + app.openBookmarksPanel() + + let sortBookmarksPanelButton = newApp.buttons[AccessibilityIdentifiers.sortBookmarksButtonPanel] + sortBookmarksPanelButton.tap() + + let sortByNameManual = newApp.menuItems["Manual"] + let sortByNameMenuItem = newApp.menuItems["Name"] + let sortByNameAscendingMenuItem = newApp.menuItems["Ascending"] + let sortByNameDescendingMenuItem = newApp.menuItems["Descending"] + + XCTAssertTrue(sortByNameManual.isEnabled) + XCTAssertTrue(sortByNameMenuItem.isEnabled) + XCTAssertTrue(sortByNameAscendingMenuItem.isEnabled) + XCTAssertTrue(sortByNameDescendingMenuItem.isEnabled) + } + + // MARK: - Utilities + + private func tapPanelSortButton() { + let bookmarksPanelPopover = app.popovers.firstMatch + let sortBookmarksButton = bookmarksPanelPopover.buttons[AccessibilityIdentifiers.sortBookmarksButtonPanel] + sortBookmarksButton.tap() + } + + private func selectSortByName(mode: BookmarkMode, descending: Bool = false) { + if mode == .panel { + let bookmarksPanelPopover = app.popovers.firstMatch + let sortBookmarksButton = bookmarksPanelPopover.buttons[AccessibilityIdentifiers.sortBookmarksButtonPanel] + sortBookmarksButton.tap() + bookmarksPanelPopover.menuItems["Name"].tap() + + if descending { + sortBookmarksButton.tap() + bookmarksPanelPopover.menuItems["Descending"].tap() + } + } else { + let sortBookmarksButton = app.buttons[AccessibilityIdentifiers.sortBookmarksButtonManager] + sortBookmarksButton.tap() + app.menuItems["Name"].tap() + + if descending { + sortBookmarksButton.tap() + app.menuItems["Descending"].tap() + /// Here we hover over the sort button, because if we stay where the 'Descending' was selected + /// the label of the bookmark being hovered is different because it shows the URL. + sortBookmarksButton.hover() + } + } + } + + private func addBookmark(pageTitle: String, in folder: String? = nil) { + let urlForBookmarksBar = UITests.simpleServedPage(titled: pageTitle) + app.openSiteToBookmark(url: urlForBookmarksBar, + pageTitle: pageTitle, + bookmarkingViaDialog: true, + escapingDialog: true, + folderName: folder) + } + + private func closeShowBookmarksBarAlert() { + app.dismissPopover(buttonIdentifier: "Hide") + } +} diff --git a/UITests/Common/XCUIApplicationExtension.swift b/UITests/Common/XCUIApplicationExtension.swift index 61ba971620..5044ed490e 100644 --- a/UITests/Common/XCUIApplicationExtension.swift +++ b/UITests/Common/XCUIApplicationExtension.swift @@ -18,6 +18,12 @@ import XCTest +// Enum to represent bookmark modes +enum BookmarkMode { + case panel + case manager +} + extension XCUIApplication { private enum AccessibilityIdentifiers { @@ -35,6 +41,19 @@ extension XCUIApplication { button.tap() } + /// Enforces single a single window by: + /// 1. First, closing all windows + /// 2. Opening a new window + func enforceSingleWindow() { + typeKey("w", modifierFlags: [.command, .option, .shift]) + typeKey("n", modifierFlags: .command) + } + + /// Opens a new tab via keyboard shortcut + func openNewTab() { + typeKey("t", modifierFlags: .command) + } + // MARK: - Bookmarks /// Reset the bookmarks so we can rely on a single bookmark's existence @@ -64,8 +83,7 @@ extension XCUIApplication { /// - Parameter bookmarkingViaDialog: open bookmark dialog, adding bookmark /// - Parameter escapingDialog: `esc` key to leave dialog /// - Parameter folderName: The name of the folder where you want to save the bookmark. If the folder does not exist, it fails. - func openSiteToBookmark(app: XCUIApplication, - url: URL, + func openSiteToBookmark(url: URL, pageTitle: String, bookmarkingViaDialog: Bool, escapingDialog: Bool, @@ -77,27 +95,27 @@ extension XCUIApplication { ) addressBarTextField.typeURL(url) XCTAssertTrue( - app.windows.webViews[pageTitle].waitForExistence(timeout: UITests.Timeouts.elementExistence), + windows.webViews[pageTitle].waitForExistence(timeout: UITests.Timeouts.elementExistence), "Visited site didn't load with the expected title in a reasonable timeframe." ) if bookmarkingViaDialog { - app.typeKey("d", modifierFlags: [.command]) // Add bookmark + typeKey("d", modifierFlags: [.command]) // Add bookmark if let folderName = folderName { - let folderLocationButton = app.popUpButtons["bookmark.add.folder.dropdown"] + let folderLocationButton = popUpButtons["bookmark.add.folder.dropdown"] folderLocationButton.tap() let folderOneLocation = folderLocationButton.menuItems[folderName] folderOneLocation.tap() } if escapingDialog { - app.typeKey(.escape, modifierFlags: []) // Exit dialog + typeKey(.escape, modifierFlags: []) // Exit dialog } } } /// Shows the bookmarks panel shortcut and taps it. If the bookmarks shortcut is visible, it only taps it. - func showAndTapBookmarksPanelShortcut() { + func openBookmarksPanel() { let bookmarksPanelShortcutButton = buttons[AccessibilityIdentifiers.bookmarksPanelShortcutButton] if !bookmarksPanelShortcutButton.exists { typeKey("K", modifierFlags: [.command, .shift]) @@ -105,4 +123,17 @@ extension XCUIApplication { bookmarksPanelShortcutButton.tap() } + + func verifyBookmarkOrder(expectedOrder: [String], mode: BookmarkMode) { + let rowCount = (mode == .panel ? popovers.firstMatch.outlines.firstMatch : tables.firstMatch).cells.count + XCTAssertEqual(rowCount, expectedOrder.count, "Row count does not match expected count.") + + for index in 0..