From 585742d06a4a579167b47a897c6cfaf9408fdaf9 Mon Sep 17 00:00:00 2001 From: Brad Slayter Date: Mon, 22 Apr 2024 09:58:36 -0500 Subject: [PATCH] Support close tabs to left and move close actions to submenu (#2664) Task/Issue URL: https://app.asana.com/0/1199230911884351/1207117125840765/f Tech Design URL: CC: @ayoy **Description**: This PR adds the option to close all tabs to the left when right clicking a tab. It also moves this action as well as "close to right" and "close other tabs" to a submenu in the tab context menu. **Steps to test this PR**: 1. Open some tabs 2. Right click a tab, ensure "Close Other Tabs" opens a sub menu 3. Clicking "Close Tabs to the Left" should close all tabs to the left of the clicked tab except pinned tabs 4. "Close to the right" and "Close all other tabs" should work as expected. --- ###### Internal references: [Pull Request Review Checklist](https://app.asana.com/0/1202500774821704/1203764234894239/f) [Software Engineering Expectations](https://app.asana.com/0/59792373528535/199064865822552) [Technical Design Template](https://app.asana.com/0/59792373528535/184709971311943) [Pull Request Documentation](https://app.asana.com/0/1202500774821704/1204012835277482/f) --------- Co-authored-by: Dominik Kapusta --- DuckDuckGo/Common/Localizables/UserText.swift | 2 + DuckDuckGo/Localizable.xcstrings | 24 +++++++ DuckDuckGo/TabBar/Model/TabCollection.swift | 5 ++ .../TabBar/View/TabBarViewController.swift | 9 +++ DuckDuckGo/TabBar/View/TabBarViewItem.swift | 32 ++++++++- .../ViewModel/TabCollectionViewModel.swift | 16 +++++ .../TabBar/View/MockTabViewItemDelegate.swift | 7 +- .../TabBar/View/TabBarViewItemTests.swift | 41 ++++++++++-- .../TabCollectionViewModelTests.swift | 65 +++++++++++++++++++ 9 files changed, 191 insertions(+), 10 deletions(-) diff --git a/DuckDuckGo/Common/Localizables/UserText.swift b/DuckDuckGo/Common/Localizables/UserText.swift index 9cb9493ebf..cc208ef9ab 100644 --- a/DuckDuckGo/Common/Localizables/UserText.swift +++ b/DuckDuckGo/Common/Localizables/UserText.swift @@ -202,6 +202,8 @@ struct UserText { static let muteTab = NSLocalizedString("mute.tab", value: "Mute Tab", comment: "Menu item. Mute tab") static let unmuteTab = NSLocalizedString("unmute.tab", value: "Unmute Tab", comment: "Menu item. Unmute tab") static let closeOtherTabs = NSLocalizedString("close.other.tabs", value: "Close Other Tabs", comment: "Menu item") + static let closeAllOtherTabs = NSLocalizedString("close.all.other.tabs", value: "Close All Other Tabs", comment: "Menu item") + static let closeTabsToTheLeft = NSLocalizedString("close.tabs.to.the.left", value: "Close Tabs to the Left", comment: "Menu item") static let closeTabsToTheRight = NSLocalizedString("close.tabs.to.the.right", value: "Close Tabs to the Right", comment: "Menu item") static let openInNewTab = NSLocalizedString("open.in.new.tab", value: "Open in New Tab", comment: "Menu item that opens the link in a new tab") static let openInNewWindow = NSLocalizedString("open.in.new.window", value: "Open in New Window", comment: "Menu item that opens the link in a new window") diff --git a/DuckDuckGo/Localizable.xcstrings b/DuckDuckGo/Localizable.xcstrings index ef1f610c2e..c0b1404759 100644 --- a/DuckDuckGo/Localizable.xcstrings +++ b/DuckDuckGo/Localizable.xcstrings @@ -10739,6 +10739,18 @@ } } }, + "close.all.other.tabs" : { + "comment" : "Menu item", + "extractionState" : "extracted_with_value", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "new", + "value" : "Close All Other Tabs" + } + } + } + }, "close.other.tabs" : { "comment" : "Menu item", "extractionState" : "extracted_with_value", @@ -10979,6 +10991,18 @@ } } }, + "close.tabs.to.the.left" : { + "comment" : "Menu item", + "extractionState" : "extracted_with_value", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "new", + "value" : "Close Tabs to the Left" + } + } + } + }, "close.tabs.to.the.right" : { "comment" : "Menu item", "extractionState" : "extracted_with_value", diff --git a/DuckDuckGo/TabBar/Model/TabCollection.swift b/DuckDuckGo/TabBar/Model/TabCollection.swift index 94b632fa86..87c15fe1a6 100644 --- a/DuckDuckGo/TabBar/Model/TabCollection.swift +++ b/DuckDuckGo/TabBar/Model/TabCollection.swift @@ -78,6 +78,11 @@ final class TabCollection: NSObject { tabs = tab.map { [$0] } ?? [] } + func removeTabs(before index: Int) { + tabsWillClose(range: 0.. DuckDuckGo_Privacy_Browser.OtherTabBarViewItemsState { - OtherTabBarViewItemsState(hasItemsToTheLeft: false, hasItemsToTheRight: hasItemsToTheRight) + OtherTabBarViewItemsState(hasItemsToTheLeft: hasItemsToTheLeft, hasItemsToTheRight: hasItemsToTheRight) } func clear() { diff --git a/UnitTests/TabBar/View/TabBarViewItemTests.swift b/UnitTests/TabBar/View/TabBarViewItemTests.swift index 208d73a29e..f1fd24399a 100644 --- a/UnitTests/TabBar/View/TabBarViewItemTests.swift +++ b/UnitTests/TabBar/View/TabBarViewItemTests.swift @@ -50,8 +50,16 @@ final class TabBarViewItemTests: XCTestCase { XCTAssertTrue(menu.item(at: 5)?.isSeparatorItem ?? false) XCTAssertEqual(menu.item(at: 6)?.title, UserText.closeTab) XCTAssertEqual(menu.item(at: 7)?.title, UserText.closeOtherTabs) - XCTAssertEqual(menu.item(at: 8)?.title, UserText.closeTabsToTheRight) - XCTAssertEqual(menu.item(at: 9)?.title, UserText.moveTabToNewWindow) + XCTAssertEqual(menu.item(at: 8)?.title, UserText.moveTabToNewWindow) + + // Check "Close Other Tabs" submenu + guard let submenu = menu.item(at: 7)?.submenu else { + XCTFail("\"Close Other Tabs\" menu item should have a submenu") + return + } + XCTAssertEqual(submenu.item(at: 0)?.title, UserText.closeTabsToTheLeft) + XCTAssertEqual(submenu.item(at: 1)?.title, UserText.closeTabsToTheRight) + XCTAssertEqual(submenu.item(at: 2)?.title, UserText.closeAllOtherTabs) } func testThatMuteIsShownWhenCurrentAudioStateIsUnmuted() { @@ -75,7 +83,8 @@ final class TabBarViewItemTests: XCTestCase { func testWhenOneTabCloseThenOtherTabsItemIsDisabled() { tabBarViewItem.menuNeedsUpdate(menu) - let item = menu.items .first { $0.title == UserText.closeOtherTabs } + let submenu = menu.items .first { $0.title == UserText.closeOtherTabs } + let item = submenu?.submenu?.items .first { $0.title == UserText.closeAllOtherTabs } XCTAssertFalse(item?.isEnabled ?? true) } @@ -83,7 +92,8 @@ final class TabBarViewItemTests: XCTestCase { delegate.hasItemsToTheRight = true tabBarViewItem.menuNeedsUpdate(menu) - let item = menu.items .first { $0.title == UserText.closeOtherTabs } + let submenu = menu.items .first { $0.title == UserText.closeOtherTabs } + let item = submenu?.submenu?.items .first { $0.title == UserText.closeAllOtherTabs } XCTAssertTrue(item?.isEnabled ?? false) } @@ -102,10 +112,28 @@ final class TabBarViewItemTests: XCTestCase { XCTAssertTrue(item?.isEnabled ?? false) } + func testWhenNoTabsToTheLeftThenCloseTabsToTheLeftIsDisabled() { + tabBarViewItem.menuNeedsUpdate(menu) + + let submenu = menu.items .first { $0.title == UserText.closeOtherTabs } + let item = submenu?.submenu?.items .first { $0.title == UserText.closeTabsToTheLeft } + XCTAssertFalse(item?.isEnabled ?? true) + } + + func testWhenTabsToTheLeftThenCloseTabsToTheLeftIsEnabled() { + delegate.hasItemsToTheLeft = true + tabBarViewItem.menuNeedsUpdate(menu) + + let submenu = menu.items .first { $0.title == UserText.closeOtherTabs } + let item = submenu?.submenu?.items .first { $0.title == UserText.closeTabsToTheLeft } + XCTAssertTrue(item?.isEnabled ?? false) + } + func testWhenNoTabsToTheRightThenCloseTabsToTheRightIsDisabled() { tabBarViewItem.menuNeedsUpdate(menu) - let item = menu.items .first { $0.title == UserText.closeTabsToTheRight } + let submenu = menu.items .first { $0.title == UserText.closeOtherTabs } + let item = submenu?.submenu?.items .first { $0.title == UserText.closeTabsToTheRight } XCTAssertFalse(item?.isEnabled ?? true) } @@ -113,7 +141,8 @@ final class TabBarViewItemTests: XCTestCase { delegate.hasItemsToTheRight = true tabBarViewItem.menuNeedsUpdate(menu) - let item = menu.items .first { $0.title == UserText.closeTabsToTheRight } + let submenu = menu.items .first { $0.title == UserText.closeOtherTabs } + let item = submenu?.submenu?.items .first { $0.title == UserText.closeTabsToTheRight } XCTAssertTrue(item?.isEnabled ?? false) } diff --git a/UnitTests/TabBar/ViewModel/TabCollectionViewModelTests.swift b/UnitTests/TabBar/ViewModel/TabCollectionViewModelTests.swift index efc39feac0..ffdd3912e4 100644 --- a/UnitTests/TabBar/ViewModel/TabCollectionViewModelTests.swift +++ b/UnitTests/TabBar/ViewModel/TabCollectionViewModelTests.swift @@ -318,6 +318,71 @@ final class TabCollectionViewModelTests: XCTestCase { XCTAssertEqual(firstTab, tabCollectionViewModel.selectedTabViewModel?.tab) } + func testWhenTabsToTheLeftAreRemovedAndSelectionIsRemoved_ThenSelectionIsCorrectlyUpdated() { + let tabCollectionViewModel = TabCollectionViewModel.aTabCollectionViewModel() + + tabCollectionViewModel.appendNewTab() + tabCollectionViewModel.appendNewTab() + tabCollectionViewModel.appendNewTab() + + tabCollectionViewModel.select(at: .unpinned(1)) + tabCollectionViewModel.removeTabs(before: 2) + + XCTAssertEqual(tabCollectionViewModel.selectionIndex?.item, 0) + } + + func testWhenTabsToTheLeftAreRemovedAndSelectionRemains_ThenSelectionIsCorrectlyUpdated() { + let tabCollectionViewModel = TabCollectionViewModel.aTabCollectionViewModel() + + tabCollectionViewModel.appendNewTab() + tabCollectionViewModel.appendNewTab() + tabCollectionViewModel.appendNewTab() + tabCollectionViewModel.appendNewTab() + tabCollectionViewModel.appendNewTab() + tabCollectionViewModel.select(at: .unpinned(3)) + tabCollectionViewModel.removeTabs(before: 3) + + XCTAssertEqual(tabCollectionViewModel.selectionIndex?.item, 0) + } + + func testWhenTabsToTheLeftAreRemovedAndSelectionRemainsAndIsToTheRight_ThenSelectionIsCorrectlyUpdated() { + let tabCollectionViewModel = TabCollectionViewModel.aTabCollectionViewModel() + + tabCollectionViewModel.appendNewTab() + tabCollectionViewModel.appendNewTab() + tabCollectionViewModel.appendNewTab() + tabCollectionViewModel.appendNewTab() + tabCollectionViewModel.appendNewTab() + tabCollectionViewModel.select(at: .unpinned(4)) + tabCollectionViewModel.removeTabs(before: 2) + + XCTAssertEqual(tabCollectionViewModel.selectionIndex?.item, 2) + } + + func testWhenTabsToTheRightAreRemovedAndSelectionIsRemoved_ThenSelectionIsCorrectlyUpdated() { + let tabCollectionViewModel = TabCollectionViewModel.aTabCollectionViewModel() + + tabCollectionViewModel.appendNewTab() + tabCollectionViewModel.appendNewTab() + tabCollectionViewModel.appendNewTab() + + tabCollectionViewModel.select(at: .unpinned(1)) + tabCollectionViewModel.removeTabs(after: 0) + + XCTAssertEqual(tabCollectionViewModel.selectionIndex?.item, 0) + } + + func testWhenTabsToTheRightAreRemovedAndSelectionRemains_ThenSelectionIsCorrectlyUpdated() { + let tabCollectionViewModel = TabCollectionViewModel.aTabCollectionViewModel() + + tabCollectionViewModel.appendNewTab() + tabCollectionViewModel.appendNewTab() + tabCollectionViewModel.select(at: .unpinned(1)) + tabCollectionViewModel.removeTabs(after: 1) + + XCTAssertEqual(tabCollectionViewModel.selectionIndex?.item, 1) + } + func testWhenLastTabIsRemoved_ThenSelectionIsNil() { let tabCollectionViewModel = TabCollectionViewModel.aTabCollectionViewModel()