Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Switch to tab implementation #3660

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"images" : [
{
"filename" : "History-16.pdf",
"idiom" : "universal"
}
],
"info" : {
"author" : "xcode",
"version" : 1
},
"properties" : {
"template-rendering-intent" : "template"
}
}
Binary file not shown.
35 changes: 26 additions & 9 deletions DuckDuckGo/NavigationBar/View/AddressBarTextField.swift
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,9 @@ final class AddressBarTextField: NSTextField {
PixelKit.fire(autocompletePixel)
}

if NSApp.isCommandPressed {
if case .openTab(let title, url: let url) = suggestion {
switchTo(OpenTab(title: title, url: url))
} else if NSApp.isCommandPressed {
openNew(NSApp.isOptionPressed ? .window : .tab, selected: NSApp.isShiftPressed, suggestion: suggestion)
} else {
hideSuggestionWindow()
Expand Down Expand Up @@ -486,6 +488,21 @@ final class AddressBarTextField: NSTextField {
}
}

private func switchTo(_ tab: OpenTab) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we encapsulate this logic somewhere else? (standalone class maybe?) It is slightly confusing to have logic for switching tabs in AddressBarTextField

if let selectedTabViewModel = tabCollectionViewModel.selectedTabViewModel,
let selectionIndex = tabCollectionViewModel.selectionIndex,
case .newtab = selectedTabViewModel.tab.content {
// close tab with "new tab" page open
tabCollectionViewModel.remove(at: selectionIndex)

// close the window if no more non-pinned tabs are open
if tabCollectionViewModel.tabs.isEmpty, let window, window.isVisible {
window.performClose(self)
}
}
WindowControllersManager.shared.show(url: tab.url, source: .switchToOpenTab, newTab: false)
}

private func makeUrl(suggestion: Suggestion?, stringValueWithoutSuffix: String, completion: @escaping (URL?, String, Bool) -> Void) {
let finalUrl: URL?
let userEnteredValue: String
Expand Down Expand Up @@ -1037,14 +1054,14 @@ extension AddressBarTextField: NSTextFieldDelegate {
return true

case #selector(NSResponder.deleteBackward(_:)),
#selector(NSResponder.deleteForward(_:)),
#selector(NSResponder.deleteToMark(_:)),
#selector(NSResponder.deleteWordForward(_:)),
#selector(NSResponder.deleteWordBackward(_:)),
#selector(NSResponder.deleteToEndOfLine(_:)),
#selector(NSResponder.deleteToEndOfParagraph(_:)),
#selector(NSResponder.deleteToBeginningOfLine(_:)),
#selector(NSResponder.deleteBackwardByDecomposingPreviousCharacter(_:)):
#selector(NSResponder.deleteForward(_:)),
#selector(NSResponder.deleteToMark(_:)),
#selector(NSResponder.deleteWordForward(_:)),
#selector(NSResponder.deleteWordBackward(_:)),
#selector(NSResponder.deleteToEndOfLine(_:)),
#selector(NSResponder.deleteToEndOfParagraph(_:)),
#selector(NSResponder.deleteToBeginningOfLine(_:)),
#selector(NSResponder.deleteBackwardByDecomposingPreviousCharacter(_:)):

suggestionContainerViewModel?.clearSelection()
return false
Expand Down
34 changes: 26 additions & 8 deletions DuckDuckGo/Suggestions/Model/SuggestionContainer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,22 @@
// limitations under the License.
//

import Foundation
import Suggestions
import Combine
import Common
import Foundation
import History
import PixelKit
import os.log
import PixelKit
import Suggestions

final class SuggestionContainer {

static let maximumNumberOfSuggestions = 9

@PublishedAfter var result: SuggestionResult?

typealias OpenTabsProvider = @MainActor () -> [any Suggestions.BrowserTab]
private let openTabsProvider: OpenTabsProvider
private let historyCoordinating: HistoryCoordinating
private let bookmarkManager: BookmarkManager
private let startupPreferences: StartupPreferences
Expand All @@ -41,7 +44,8 @@ final class SuggestionContainer {

fileprivate let suggestionsURLSession = URLSession(configuration: .ephemeral)

init(suggestionLoading: SuggestionLoading, historyCoordinating: HistoryCoordinating, bookmarkManager: BookmarkManager, startupPreferences: StartupPreferences = .shared) {
init(openTabsProvider: @escaping OpenTabsProvider, suggestionLoading: SuggestionLoading, historyCoordinating: HistoryCoordinating, bookmarkManager: BookmarkManager, startupPreferences: StartupPreferences = .shared) {
self.openTabsProvider = openTabsProvider
self.bookmarkManager = bookmarkManager
self.historyCoordinating = historyCoordinating
self.startupPreferences = startupPreferences
Expand All @@ -52,8 +56,16 @@ final class SuggestionContainer {
let urlFactory = { urlString in
return URL.makeURL(fromSuggestionPhrase: urlString)
}

self.init(suggestionLoading: SuggestionLoader(urlFactory: urlFactory),
let openTabsProvider: OpenTabsProvider = { @MainActor in
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would using WindowControllersManagerProtocol make more sense here since other data are passed this way too?

let selectedTab = WindowControllersManager.shared.selectedTab
return WindowControllersManager.shared.allTabViewModels.compactMap { model in
guard model.tab !== selectedTab, model.tab.content.isUrl else { return nil }
return model.tab.content.userEditableUrl.map { url in
OpenTab(title: model.title, url: url)
}
}
}
self.init(openTabsProvider: openTabsProvider, suggestionLoading: SuggestionLoader(urlFactory: urlFactory),
historyCoordinating: HistoryCoordinator.shared,
bookmarkManager: LocalBookmarkManager.shared)
}
Expand Down Expand Up @@ -92,6 +104,13 @@ final class SuggestionContainer {

}

struct OpenTab: BrowserTab {

let title: String
let url: URL

}

extension SuggestionContainer: SuggestionLoadingDataSource {

var platform: Platform {
Expand Down Expand Up @@ -123,8 +142,7 @@ extension SuggestionContainer: SuggestionLoadingDataSource {
}

@MainActor func openTabs(for suggestionLoading: any Suggestions.SuggestionLoading) -> [any Suggestions.BrowserTab] {
// Support for this on macOS will come later.
[]
openTabsProvider()
}

func suggestionLoading(_ suggestionLoading: SuggestionLoading,
Expand Down
9 changes: 5 additions & 4 deletions DuckDuckGo/Suggestions/ViewModel/SuggestionViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,8 @@ struct SuggestionViewModel: Equatable {
return title ?? url.toString(forUserInput: userStringValue)
}
case .bookmark(title: let title, url: _, isFavorite: _, allowedInTopHits: _),
.internalPage(title: let title, url: _), .openTab(title: let title, url: _):
.internalPage(title: let title, url: _),
.openTab(title: let title, url: _):
return title
case .unknown(value: let value):
return value
Expand All @@ -115,7 +116,8 @@ struct SuggestionViewModel: Equatable {
return title
}
case .bookmark(title: let title, url: _, isFavorite: _, allowedInTopHits: _),
.internalPage(title: let title, url: _), .openTab(title: let title, url: _):
.internalPage(title: let title, url: _),
.openTab(title: let title, url: _):
return title
}
}
Expand Down Expand Up @@ -187,8 +189,7 @@ struct SuggestionViewModel: Equatable {
guard url == URL(string: StartupPreferences.shared.formattedCustomHomePageURL) else { return nil }
return .home16
case .openTab:
assertionFailure("specify an icon")
return nil
return .openTabSuggestion
}
}

Expand Down
7 changes: 4 additions & 3 deletions DuckDuckGo/Tab/Model/TabContent.swift
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ extension TabContent {
case link
case appOpenUrl
case reload
case switchToOpenTab

case webViewUpdated

Expand All @@ -71,7 +72,7 @@ extension TabContent {
switch self {
case .userEntered(_, downloadRequested: true):
.custom(.userRequestedPageDownload)
case .userEntered:
case .userEntered, .switchToOpenTab /* fallback */:
.custom(.userEnteredUrl)
case .pendingStateRestoration:
.sessionRestoration
Expand Down Expand Up @@ -100,7 +101,7 @@ extension TabContent {
.returnCacheDataElseLoad
case .reload, .loadedByStateRestoration:
.reloadIgnoringCacheData
case .userEntered, .bookmark, .ui, .link, .appOpenUrl, .webViewUpdated:
case .userEntered, .bookmark, .ui, .link, .appOpenUrl, .webViewUpdated, .switchToOpenTab:
.useProtocolCachePolicy
}
}
Expand Down Expand Up @@ -262,7 +263,7 @@ extension TabContent {
case .url(_, _, source: let source):
return source
case .newtab, .settings, .bookmarks, .onboardingDeprecated, .onboarding, .releaseNotes, .dataBrokerProtection,
.subscription, .identityTheftRestoration, .none:
.subscription, .identityTheftRestoration, .none:
return .ui
}
}
Expand Down
1 change: 1 addition & 0 deletions DuckDuckGo/Tab/View/BrowserTabViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -667,6 +667,7 @@ final class BrowserTabViewController: NSViewController {
.url(_, _, source: .ui),
.url(_, _, source: .link),
.url(_, _, source: .appOpenUrl),
.url(_, _, source: .switchToOpenTab),
.url(_, _, source: .reload):
return true

Expand Down
1 change: 1 addition & 0 deletions DuckDuckGo/Tab/ViewModel/TabViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ final class TabViewModel {
.url(_, _, source: .bookmark),
.url(_, _, source: .ui),
.url(_, _, source: .appOpenUrl),
.url(_, _, source: .switchToOpenTab),
.url(_, _, source: .reload),
.newtab,
.settings,
Expand Down
10 changes: 10 additions & 0 deletions DuckDuckGo/TabBar/ViewModel/TabCollectionViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -787,6 +787,16 @@ extension TabCollectionViewModel {
return nil
}

func indexInAllTabs(where condition: (Tab) -> Bool) -> TabIndex? {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice 👍 Very useful

if let index = pinnedTabsCollection?.tabs.firstIndex(where: condition) {
return .pinned(index)
}
if let index = tabCollection.tabs.firstIndex(where: condition) {
return .unpinned(index)
}
return nil
}

private func tab(at tabIndex: TabIndex) -> Tab? {
switch tabIndex {
case .pinned(let index):
Expand Down
21 changes: 21 additions & 0 deletions DuckDuckGo/Windows/View/WindowControllersManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,12 @@ extension WindowControllersManager {
// If there is any non-popup window available, open the URL in it
?? nonPopupMainWindowControllers.first {

// Switch to already open tab if present
if [.appOpenUrl, .switchToOpenTab].contains(source),
let url, switchToOpenTab(with: url, preferring: windowController) == true {
return
}

show(url: url, in: windowController, source: source, newTab: newTab)
return
}
Expand All @@ -204,6 +210,21 @@ extension WindowControllersManager {
}
}

private func switchToOpenTab(with url: URL, preferring mainWindowController: MainWindowController) -> Bool {
for (windowIdx, windowController) in ([mainWindowController] + mainWindowControllers).enumerated() {
// prefer current main window
guard windowIdx == 0 || windowController !== mainWindowController else { continue }
let tabCollectionViewModel = windowController.mainViewController.tabCollectionViewModel
guard let index = tabCollectionViewModel.indexInAllTabs(where: { $0.content.urlForWebView == url }) else { continue }

windowController.window?.makeKeyAndOrderFront(self)
tabCollectionViewModel.select(at: index)

return true
}
return false
}

private func show(url: URL?, in windowController: MainWindowController, source: Tab.TabContent.URLSource, newTab: Bool) {
let viewController = windowController.mainViewController
windowController.window?.makeKeyAndOrderFront(self)
Expand Down
Loading