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

Unify bookmarks context menu across different views #3138

Merged
merged 37 commits into from
Aug 30, 2024
Merged
Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
784dfbc
NSPopover-based Bookmarks menu system
mallexxx Aug 15, 2024
b6d82b0
fix button spacing constant usage
mallexxx Aug 15, 2024
6f8452a
fix clipped items indicator
mallexxx Aug 15, 2024
2558ca1
fixing Tests
mallexxx Aug 19, 2024
4530a52
Merge remote-tracking branch 'origin/main' into alex/bookmarks-bar/menu
mallexxx Aug 21, 2024
8217dc4
minor PR adjustment
mallexxx Aug 22, 2024
98ec34e
Roll back BookmarkListViewController, add BookmarksBarMenuViewController
mallexxx Aug 22, 2024
a65352c
Unify bookmarks context menu across different views
mallexxx Aug 22, 2024
03580e5
Add more tests, fix search items
mallexxx Aug 23, 2024
7e0b916
fix test
mallexxx Aug 23, 2024
e032961
Merge branch 'alex/bookmarks-context-menu' into alex/bookmarks-bar/menu
mallexxx Aug 23, 2024
152e514
cleanup
mallexxx Aug 23, 2024
a8c0520
Bookmarks bar/menu code adjustments (#3107)
mallexxx Aug 23, 2024
686b2df
Add Open All in new tabs item; Close all bookmarks popovers on select…
mallexxx Aug 23, 2024
376b969
Add Bookmarks menu drag&drop support (#3111)
mallexxx Aug 23, 2024
d54dd22
Bookmarks Menu keyboard controls (#3112)
mallexxx Aug 23, 2024
48e2f66
Update Bookmarks Menu size (#3113)
mallexxx Aug 23, 2024
e162f1a
Merge remote-tracking branch 'origin/main' into alex/bookmarks-contex…
mallexxx Aug 23, 2024
b790e32
Merge branch 'alex/bookmarks-context-menu' into alex/bookmarks-bar/menu
mallexxx Aug 23, 2024
791c3eb
fix missing disclosure indicators
mallexxx Aug 23, 2024
4fd4c67
fix MainActor warnings
mallexxx Aug 28, 2024
cc228c1
Fix Manage button selector; add Manage Bookmarks "…" menu item, Add M…
mallexxx Aug 28, 2024
02a99bc
fix tests
mallexxx Aug 28, 2024
bc2fec3
Merge remote-tracking branch 'origin/main' into alex/bookmarks-bar/menu
mallexxx Aug 28, 2024
e3a1c1c
Merge remote-tracking branch 'origin/main' into alex/bookmarks-contex…
mallexxx Aug 30, 2024
27d9364
Merge branch 'alex/bookmarks-bar/menu' into alex/bookmarks-context-menu
mallexxx Aug 30, 2024
85784d5
Remove Manage Bookmarks item from bookmark manager details context menu
mallexxx Aug 30, 2024
db4b4ab
Fix cells highlighting in different modes
mallexxx Aug 30, 2024
f06c808
Merge branch 'alex/bookmarks-bar/menu' into alex/bookmarks-context-menu
mallexxx Aug 30, 2024
8840635
Fix PR issues
mallexxx Aug 30, 2024
0089d26
fix tests
mallexxx Aug 30, 2024
04f3ee6
fix test
mallexxx Aug 30, 2024
aa46e5f
disable assertion for empty context menu for non-bookmark object
mallexxx Aug 30, 2024
f304159
disable Open All menu items for folders without bookmarks
mallexxx Aug 30, 2024
1730a6f
Fix bookmarks menu width calculation
mallexxx Aug 30, 2024
2120bfe
rollback Submodules/privacy-reference-tests
mallexxx Aug 30, 2024
36a070b
fix tests
mallexxx Aug 30, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
124 changes: 89 additions & 35 deletions DuckDuckGo.xcodeproj/project.pbxproj

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions DuckDuckGo/Application/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate {
return firstLaunchDate >= Date.weekAgo
}

@MainActor
override init() {
// will not add crash handlers and will fire pixel on applicationDidFinishLaunching if didCrashDuringCrashHandlersSetUp == true
let didCrashDuringCrashHandlersSetUp = UserDefaultsWrapper(key: .didCrashDuringCrashHandlersSetUp, defaultValue: false)
Expand Down
1 change: 0 additions & 1 deletion DuckDuckGo/Application/Application.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import Foundation
final class Application: NSApplication {

private let copyHandler = CopyHandler()
// private var _delegate: AppDelegate!
public static var appDelegate: AppDelegate!

override init() {
Expand Down
6 changes: 6 additions & 0 deletions DuckDuckGo/Bookmarks/Extensions/NSOutlineViewExtensions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ extension NSOutlineView {
selectedNodes.compactMap { $0.representedObject as? PseudoFolder }
}

func rowIfValid(forItem item: Any?) -> Int? {
let row = row(forItem: item)
guard row >= 0, row != NSNotFound else { return nil }
return row
}

func revealAndSelect(nodePath: BookmarkNode.Path) {
let totalNodePathComponents = nodePath.components.count
if totalNodePathComponents < 2 {
Expand Down
27 changes: 7 additions & 20 deletions DuckDuckGo/Bookmarks/Model/Bookmark.swift
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,13 @@ internal class BaseBookmarkEntity: Identifiable, Equatable, Hashable {
let id: String
var title: String
let isFolder: Bool
let parentFolderUUID: String?

fileprivate init(id: String,
title: String,
isFolder: Bool) {

fileprivate init(id: String, title: String, isFolder: Bool, parentFolderUUID: String?) {
self.id = id
self.title = title
self.isFolder = isFolder
self.parentFolderUUID = parentFolderUUID
}

static func from(
Expand Down Expand Up @@ -129,7 +128,6 @@ final class BookmarkFolder: BaseBookmarkEntity {
return request
}

let parentFolderUUID: String?
let children: [BaseBookmarkEntity]
let totalChildBookmarks: Int

Expand All @@ -141,11 +139,7 @@ final class BookmarkFolder: BaseBookmarkEntity {
return children.compactMap { $0 as? BookmarkFolder }
}

init(id: String,
title: String,
parentFolderUUID: String? = nil,
children: [BaseBookmarkEntity] = []) {
self.parentFolderUUID = parentFolderUUID
init(id: String, title: String, parentFolderUUID: String? = nil, children: [BaseBookmarkEntity] = []) {
self.children = children

let childFolders = children.compactMap({ $0 as? BookmarkFolder })
Expand All @@ -154,7 +148,7 @@ final class BookmarkFolder: BaseBookmarkEntity {

self.totalChildBookmarks = childBookmarks.count + subfolderBookmarksCount

super.init(id: id, title: title, isFolder: true)
super.init(id: id, title: title, isFolder: true, parentFolderUUID: parentFolderUUID)
}

override func isEqual(to instance: BaseBookmarkEntity) -> Bool {
Expand Down Expand Up @@ -203,7 +197,6 @@ final class Bookmark: BaseBookmarkEntity {

let url: String
var isFavorite: Bool
private(set) var parentFolderUUID: String?

public var urlObject: URL? {
return url.isBookmarklet() ? url.toEncodedBookmarklet() : URL(string: url)
Expand All @@ -223,18 +216,12 @@ final class Bookmark: BaseBookmarkEntity {
}
}

init(id: String,
url: String,
title: String,
isFavorite: Bool,
parentFolderUUID: String? = nil,
faviconManagement: FaviconManagement = FaviconManager.shared) {
init(id: String, url: String, title: String, isFavorite: Bool, parentFolderUUID: String? = nil, faviconManagement: FaviconManagement = FaviconManager.shared) {
self.url = url
self.isFavorite = isFavorite
self.parentFolderUUID = parentFolderUUID
self.faviconManagement = faviconManagement

super.init(id: id, title: title, isFolder: false)
super.init(id: id, title: title, isFolder: false, parentFolderUUID: parentFolderUUID)
}

convenience init(from bookmark: Bookmark, with newUrl: String) {
Expand Down
199 changes: 199 additions & 0 deletions DuckDuckGo/Bookmarks/Model/BookmarkDragDropManager.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,199 @@
//
// BookmarkDragDropManager.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 AppKit
import Common
import Foundation
import os.log

final class BookmarkDragDropManager {

static let shared = BookmarkDragDropManager()

static let draggedTypes: [NSPasteboard.PasteboardType] = [
.string,
.URL,
BookmarkPasteboardWriter.bookmarkUTIInternalType,
FolderPasteboardWriter.folderUTIInternalType
]

private let bookmarkManager: BookmarkManager

init(bookmarkManager: BookmarkManager = LocalBookmarkManager.shared) {
self.bookmarkManager = bookmarkManager
}

func validateDrop(_ info: NSDraggingInfo, to destination: Any) -> NSDragOperation {
let bookmarks = PasteboardBookmark.pasteboardBookmarks(with: info.draggingPasteboard.pasteboardItems)
let folders = PasteboardFolder.pasteboardFolders(with: info.draggingPasteboard.pasteboardItems)

let bookmarksDragOperation = bookmarks.flatMap { validateMove(for: $0, destination: destination) }
let foldersDragOperation = folders.flatMap { validateMove(for: $0, destination: destination) }

switch (bookmarksDragOperation, foldersDragOperation) {
// If the dragged values contain both folders and bookmarks, only validate the move if all objects can be moved.
case (true, true), (true, nil), (nil, true):
return .move
default:
return .none
}
}

private func validateMove(for draggedBookmarks: Set<PasteboardBookmark>, destination: Any) -> Bool? {
guard !draggedBookmarks.isEmpty else { return nil }
guard destination is BookmarkFolder || destination is PseudoFolder else { return false }

return true
}

private func validateMove(for draggedFolders: Set<PasteboardFolder>, destination: Any) -> Bool? {
guard !draggedFolders.isEmpty else { return nil }

guard let destinationFolder = destination as? BookmarkFolder else {
if destination as? PseudoFolder == .bookmarks {
return true
}
return false
}

// Folders cannot be dragged onto themselves or any of their descendants:
return draggedFolders.allSatisfy { folder in
bookmarkManager.canMoveObjectWithUUID(objectUUID: folder.id, to: destinationFolder)
}
}

@discardableResult
@MainActor
func acceptDrop(_ info: NSDraggingInfo, to destination: Any, at index: Int) -> Bool {
defer {
// prevent other drop targets accepting the dragged items twice
info.draggingPasteboard.clearContents()
}
guard let draggedObjectIdentifiers = info.draggingPasteboard.pasteboardItems?.compactMap(\.bookmarkEntityUUID), !draggedObjectIdentifiers.isEmpty else {
return createBookmarks(from: info.draggingPasteboard.pasteboardItems ?? [], in: destination, at: index, window: info.draggingDestinationWindow)
}

switch destination {
case let folder as BookmarkFolder:
if folder.id == PseudoFolder.bookmarks.id { fallthrough }

let index = (index == -1 || index == NSNotFound) ? 0 : index
let parent: ParentFolderType = (folder.id == PseudoFolder.bookmarks.id) ? .root : .parent(uuid: folder.id)
bookmarkManager.move(objectUUIDs: draggedObjectIdentifiers, toIndex: index, withinParentFolder: parent) { error in
if let error = error {
Logger.general.error("Failed to accept existing parent drop via outline view: \(error.localizedDescription)")
}
}

case is PseudoFolder where (destination as? PseudoFolder) == .bookmarks:
if index == -1 || index == NSNotFound {
bookmarkManager.add(objectsWithUUIDs: draggedObjectIdentifiers, to: nil) { error in
if let error = error {
Logger.general.error("Failed to accept nil parent drop via outline view: \(error.localizedDescription)")
}
}
} else {
bookmarkManager.move(objectUUIDs: draggedObjectIdentifiers, toIndex: index, withinParentFolder: .root) { error in
if let error = error {
Logger.general.error("Failed to accept nil parent drop via outline view: \(error.localizedDescription)")
}
}
}

case let pseudoFolder as PseudoFolder where pseudoFolder == .favorites:
if index == -1 || index == NSNotFound {
bookmarkManager.update(objectsWithUUIDs: draggedObjectIdentifiers, update: { entity in
let bookmark = entity as? Bookmark
bookmark?.isFavorite = true
}, completion: { error in
if let error = error {
Logger.general.error("Failed to update entities during drop via outline view: \(error.localizedDescription)")
}
})
} else {
bookmarkManager.moveFavorites(with: draggedObjectIdentifiers, toIndex: index) { error in
if let error = error {
Logger.general.error("Failed to update entities during drop via outline view: \(error.localizedDescription)")
}
}
}

default:
assertionFailure("Unknown destination: \(destination)")
return false
}

return true
}

@MainActor
private func createBookmarks(from pasteboardItems: [NSPasteboardItem], in destination: Any, at index: Int, window: NSWindow?) -> Bool {
var parent: BookmarkFolder?
var isFavorite = false

switch destination {
case let pseudoFolder as PseudoFolder where pseudoFolder == .favorites:
isFavorite = true
case let pseudoFolder as PseudoFolder where pseudoFolder == .bookmarks:
isFavorite = false

case let folder as BookmarkFolder:
parent = folder

default:
assertionFailure("Unknown destination: \(destination)")
return false
}

var currentIndex = index
for item in pasteboardItems {
let url: URL
let title: String
func titleFromUrlDroppingSchemeIfNeeded(_ url: URL) -> String {
let title = url.absoluteString
// drop `http[s]://` from bookmark URL used as its title
if let scheme = url.navigationalScheme, scheme.isHypertextScheme {
return title.dropping(prefix: scheme.separated())
}
return title
}
if let webViewItem = item.draggedWebViewValues() {
url = webViewItem.url
title = webViewItem.title ?? self.title(forTabWith: webViewItem.url, in: window) ?? titleFromUrlDroppingSchemeIfNeeded(url)
} else if let draggedString = item.string(forType: .string),
let draggedURL = URL(string: draggedString) {
url = draggedURL
title = self.title(forTabWith: draggedURL, in: window) ?? titleFromUrlDroppingSchemeIfNeeded(url)
} else {
continue
}

self.bookmarkManager.makeBookmark(for: url, title: title, isFavorite: isFavorite, index: currentIndex, parent: parent)
currentIndex += 1
}

return currentIndex > index
}

@MainActor
private func title(forTabWith url: URL, in window: NSWindow?) -> String? {
guard let mainViewController = window?.contentViewController as? MainViewController else { return nil }
return mainViewController.tabCollectionViewModel.title(forTabWithURL: url)
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ final class BookmarkListTreeControllerSearchDataSource: BookmarkTreeControllerSe
self.bookmarkManager = bookmarkManager
}

func nodes(for searchQuery: String, sortMode: BookmarksSortMode) -> [BookmarkNode] {
func nodes(forSearchQuery searchQuery: String, sortMode: BookmarksSortMode) -> [BookmarkNode] {
let searchResults = bookmarkManager.search(by: searchQuery)

return rebuildChildNodes(for: searchResults.sorted(by: sortMode))
Expand Down
33 changes: 13 additions & 20 deletions DuckDuckGo/Bookmarks/Model/BookmarkNode.swift
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ final class BookmarkNode: Hashable {
var childNodes = [BookmarkNode]()

var isRoot: Bool {
return parent == nil
return representedObject is RootNode
}

var numberOfChildNodes: Int {
Expand Down Expand Up @@ -79,6 +79,17 @@ final class BookmarkNode: Hashable {
self.uniqueID = uniqueId
}

var canBeHighlighted: Bool {
switch representedObject {
case is SpacerNode:
return false
case let menuItem as MenuItemNode:
return menuItem.isEnabled
default:
return true
}
}

/// Creates an instance of a bookmark node.
/// - Parameters:
/// - representedObject: The represented object contained in the node.
Expand Down Expand Up @@ -144,11 +155,7 @@ final class BookmarkNode: Hashable {
}

func childNodeRepresenting(object: AnyObject) -> BookmarkNode? {
return findNodeRepresenting(object: object, recursively: false)
}

func descendantNodeRepresenting(object: AnyObject) -> BookmarkNode? {
return findNodeRepresenting(object: object, recursively: true)
return childNodes.first { $0.representedObjectEquals(object) }
}

func isAncestor(of node: BookmarkNode) -> Bool {
Expand All @@ -169,20 +176,6 @@ final class BookmarkNode: Hashable {
}
}

func findNodeRepresenting(object: AnyObject, recursively: Bool = false) -> BookmarkNode? {
for childNode in childNodes {
if childNode.representedObjectEquals(object) {
return childNode
}

if recursively, let foundNode = childNode.descendantNodeRepresenting(object: object) {
return foundNode
}
}

return nil
}

// MARK: - Hashable

func hash(into hasher: inout Hasher) {
Expand Down
Loading
Loading