Address 3580 by using only the items in the DraggingInfo to handle the drag and no view properties associated with a single window.

Remove cached `draggedNodes` optimization and just always use the data on the dragging pasteboard because it works both within the same window and across two windows.

Add support for WebFeedPasteboardWriter to include a parent containerName in the pasteboard dictionary written for a WebFeed object (which doesn't know about it's parent) because we need this for drag and drop to be able to perform a move operation.

Fix issue with dragged feeds not knowing their parent folder and delete deleting the wrong item by telling WebFeedPasteboardWriter what the container of items is.  Remove code that is now unused now that draggedNodes is removed and consolidate code that was expanded to handle both node and pasteboard items since we are just using pasteboard items now.
This commit is contained in:
Tyler
2022-06-03 01:46:19 -07:00
committed by haikusw
parent c65f6b7436
commit 665e97232d
2 changed files with 105 additions and 58 deletions

View File

@@ -15,7 +15,7 @@ typealias PasteboardWebFeedDictionary = [String: String]
struct PasteboardWebFeed: Hashable {
private struct Key {
fileprivate struct Key {
static let url = "URL"
static let homePageURL = "homePageURL"
static let name = "name"
@@ -25,6 +25,7 @@ struct PasteboardWebFeed: Hashable {
static let accountType = "accountType"
static let webFeedID = "webFeedID"
static let editedName = "editedName"
static let containerName = "containerName"
}
let url: String
@@ -34,9 +35,10 @@ struct PasteboardWebFeed: Hashable {
let editedName: String?
let accountID: String?
let accountType: AccountType?
let containerName: String?
let isLocalFeed: Bool
init(url: String, webFeedID: String?, homePageURL: String?, name: String?, editedName: String?, accountID: String?, accountType: AccountType?) {
init(url: String, webFeedID: String?, homePageURL: String?, name: String?, editedName: String?, accountID: String?, accountType: AccountType?, containerName: String? = nil) {
self.url = url.normalizedURL
self.webFeedID = webFeedID
self.homePageURL = homePageURL?.normalizedURL
@@ -44,6 +46,7 @@ struct PasteboardWebFeed: Hashable {
self.editedName = editedName
self.accountID = accountID
self.accountType = accountType
self.containerName = containerName
self.isLocalFeed = accountID != nil
}
@@ -65,7 +68,8 @@ struct PasteboardWebFeed: Hashable {
accountType = AccountType(rawValue: accountTypeInt)
}
self.init(url: url, webFeedID: webFeedID, homePageURL: homePageURL, name: name, editedName: editedName, accountID: accountID, accountType: accountType)
let containerName = dictionary[Key.containerName]
self.init(url: url, webFeedID: webFeedID, homePageURL: homePageURL, name: name, editedName: editedName, accountID: accountID, accountType: accountType, containerName: containerName)
}
init?(pasteboardItem: NSPasteboardItem) {
@@ -142,6 +146,9 @@ struct PasteboardWebFeed: Hashable {
if let accountType = accountType {
d[PasteboardWebFeed.Key.accountType] = String(accountType.rawValue)
}
if let containerName = containerName {
d[PasteboardWebFeed.Key.containerName] = containerName
}
return d
}
}
@@ -161,6 +168,7 @@ extension WebFeed: PasteboardWriterOwner {
static let webFeedUTIInternal = "com.ranchero.NetNewsWire-Evergreen.internal.webFeed"
static let webFeedUTIInternalType = NSPasteboard.PasteboardType(rawValue: webFeedUTIInternal)
var containerID: ContainerIdentifier? = nil
init(webFeed: WebFeed) {
self.webFeed = webFeed
@@ -205,6 +213,12 @@ private extension WebFeedPasteboardWriter {
}
var internalDictionary: PasteboardWebFeedDictionary {
return pasteboardFeed.internalDictionary()
var dictionary = pasteboardFeed.internalDictionary()
if dictionary[PasteboardWebFeed.Key.containerName] == nil,
case let .folder(accountID, folderName) = containerID {
assert(accountID == dictionary[PasteboardWebFeed.Key.accountID], "unexpected: container account doesn't match account of contained item")
dictionary[PasteboardWebFeed.Key.containerName] = folderName
}
return dictionary
}
}

View File

@@ -16,7 +16,6 @@ import Account
let treeController: TreeController
static let dragOperationNone = NSDragOperation(rawValue: 0)
private var draggedNodes: Set<Node>? = nil
init(treeController: TreeController) {
self.treeController = treeController
@@ -44,15 +43,24 @@ import Account
guard nodeRepresentsDraggableItem(node) else {
return nil
}
return (node.representedObject as? PasteboardWriterOwner)?.pasteboardWriter
guard var pasteboardWriter = (node.representedObject as? PasteboardWriterOwner)?.pasteboardWriter else {
return nil
}
// WebFeed objects don't have knowledge of their parent so we inject parent container information
// into WebFeedPasteboardWriter instance and it adds this field to the PasteboardWebFeed objects it writes.
// Add similar to FolderPasteboardWriter if/when we allow sub-folders
if let feedWriter = pasteboardWriter as? WebFeedPasteboardWriter {
if let parentContainerID = (node.parent?.representedObject as? Folder)?.containerID {
feedWriter.containerID = parentContainerID
pasteboardWriter = feedWriter
}
}
return pasteboardWriter
}
// MARK: - Drag and Drop
func outlineView(_ outlineView: NSOutlineView, draggingSession session: NSDraggingSession, willBeginAt screenPoint: NSPoint, forItems draggedItems: [Any]) {
draggedNodes = Set(draggedItems.map { nodeForItem($0) })
}
func outlineView(_ outlineView: NSOutlineView, validateDrop info: NSDraggingInfo, proposedItem item: Any?, proposedChildIndex index: Int) -> NSDragOperation {
let draggedFolders = PasteboardFolder.pasteboardFolders(with: info.draggingPasteboard)
let draggedFeeds = PasteboardWebFeed.pasteboardFeeds(with: info.draggingPasteboard)
@@ -203,13 +211,13 @@ private extension SidebarOutlineDataSource {
return SidebarOutlineDataSource.dragOperationNone
}
if parentNode == dropTargetNode && index == NSOutlineViewDropOnItemIndex {
return localDragOperation(parentNode: parentNode)
return localDragFeedPasteboardOperation(parentNode: parentNode, Set([draggedFeed]))
}
let updatedIndex = indexWhereDraggedFeedWouldAppear(dropTargetNode, draggedFeed)
if parentNode !== dropTargetNode || index != updatedIndex {
outlineView.setDropItem(dropTargetNode, dropChildIndex: updatedIndex)
}
return localDragOperation(parentNode: parentNode)
return localDragFeedPasteboardOperation(parentNode: parentNode, Set([draggedFeed]))
}
func validateLocalFeedsDrop(_ outlineView: NSOutlineView, _ draggedFeeds: Set<PasteboardWebFeed>, _ parentNode: Node, _ index: Int) -> NSDragOperation {
@@ -226,12 +234,12 @@ private extension SidebarOutlineDataSource {
if parentNode !== dropTargetNode || index != NSOutlineViewDropOnItemIndex {
outlineView.setDropItem(dropTargetNode, dropChildIndex: NSOutlineViewDropOnItemIndex)
}
return localDragOperation(parentNode: parentNode)
return localDragFeedPasteboardOperation(parentNode: parentNode, draggedFeeds)
}
func localDragOperation(parentNode: Node) -> NSDragOperation {
guard let firstDraggedNode = draggedNodes?.first else { return .move }
if sameAccount(firstDraggedNode, parentNode) {
func localDragFeedPasteboardOperation(parentNode: Node, _ draggedFeeds: Set<PasteboardWebFeed>)-> NSDragOperation {
guard let firstDraggedFeed = draggedFeeds.first else { return .move }
if sameAccount(firstDraggedFeed, parentNode) {
if NSApplication.shared.currentEvent?.modifierFlags.contains(.option) ?? false {
return .copy
} else {
@@ -256,7 +264,6 @@ private extension SidebarOutlineDataSource {
}
func commonAccountsFor(_ nodes: Set<Node>) -> Set<Account> {
var accounts = Set<Account>()
for node in nodes {
guard let oneAccount = accountForNode(node) else {
@@ -287,7 +294,7 @@ private extension SidebarOutlineDataSource {
if index != updatedIndex {
outlineView.setDropItem(parentNode, dropChildIndex: updatedIndex)
}
return localDragOperation(parentNode: parentNode)
return .copy // different AccountIDs means can only copy
}
func validateLocalFoldersDrop(_ outlineView: NSOutlineView, _ draggedFolders: Set<PasteboardFolder>, _ parentNode: Node, _ index: Int) -> NSDragOperation {
@@ -305,14 +312,10 @@ private extension SidebarOutlineDataSource {
if index != NSOutlineViewDropOnItemIndex {
outlineView.setDropItem(parentNode, dropChildIndex: NSOutlineViewDropOnItemIndex)
}
return localDragOperation(parentNode: parentNode)
return .copy // different AccountIDs means can only copy
}
func copyWebFeedInAccount(node: Node, to parentNode: Node) {
guard let feed = node.representedObject as? WebFeed, let destination = parentNode.representedObject as? Container else {
return
}
func copyWebFeedInAccount(_ feed: WebFeed, _ destination: Container ) {
destination.account?.addWebFeed(feed, to: destination) { result in
switch result {
case .success:
@@ -322,14 +325,8 @@ private extension SidebarOutlineDataSource {
}
}
}
func moveWebFeedInAccount(node: Node, to parentNode: Node) {
guard let feed = node.representedObject as? WebFeed,
let source = node.parent?.representedObject as? Container,
let destination = parentNode.representedObject as? Container else {
return
}
func moveWebFeedInAccount(_ feed: WebFeed, _ source: Container, _ destination: Container) {
BatchUpdate.shared.start()
source.account?.moveWebFeed(feed, from: source, to: destination) { result in
BatchUpdate.shared.end()
@@ -341,11 +338,9 @@ private extension SidebarOutlineDataSource {
}
}
}
func copyWebFeedBetweenAccounts(node: Node, to parentNode: Node) {
guard let feed = node.representedObject as? WebFeed,
let destinationAccount = nodeAccount(parentNode),
let destinationContainer = parentNode.representedObject as? Container else {
func copyWebFeedBetweenAccounts(_ feed: WebFeed, _ destinationContainer: Container) {
guard let destinationAccount = destinationContainer.account else {
return
}
@@ -371,19 +366,37 @@ private extension SidebarOutlineDataSource {
}
func acceptLocalFeedsDrop(_ outlineView: NSOutlineView, _ draggedFeeds: Set<PasteboardWebFeed>, _ parentNode: Node, _ index: Int) -> Bool {
guard let draggedNodes = draggedNodes else {
guard draggedFeeds.isEmpty == false else {
return false
}
draggedNodes.forEach { node in
if sameAccount(node, parentNode) {
draggedFeeds.forEach { pasteboardFeed in
guard let sourceAccountID = pasteboardFeed.accountID,
let sourceAccount = AccountManager.shared.existingAccount(with: sourceAccountID),
let webFeedID = pasteboardFeed.webFeedID,
let feed = sourceAccount.existingWebFeed(withWebFeedID: webFeedID),
let destinationContainer = parentNode.representedObject as? Container
else {
return
}
var sourceContainer: Container = sourceAccount // default to top level,
if let containerName = pasteboardFeed.containerName { // then check if have folder info to use instead.
if let folderContainer = sourceAccount.existingFolder(with: containerName ) {
sourceContainer = folderContainer
} else if let folderContainer = sourceAccount.existingFolder(withDisplayName: containerName) {
sourceContainer = folderContainer
}
}
if sameAccount(pasteboardFeed, parentNode) {
if NSApplication.shared.currentEvent?.modifierFlags.contains(.option) ?? false {
copyWebFeedInAccount(node: node, to: parentNode)
copyWebFeedInAccount(feed, destinationContainer)
} else {
moveWebFeedInAccount(node: node, to: parentNode)
moveWebFeedInAccount(feed, sourceContainer, destinationContainer)
}
} else {
copyWebFeedBetweenAccounts(node: node, to: parentNode)
copyWebFeedBetweenAccounts(feed, destinationContainer)
}
}
@@ -421,13 +434,12 @@ private extension SidebarOutlineDataSource {
}
return ancestorThatCanAcceptNonLocalFeed(parentNode)
}
func copyFolderBetweenAccounts(node: Node, to parentNode: Node) {
guard let folder = node.representedObject as? Folder,
let destinationAccount = nodeAccount(parentNode) else {
return
func copyFolderBetweenAccounts(folder: Folder, to parentNode: Node) {
guard let destinationAccount = nodeAccount(parentNode) else {
return
}
destinationAccount.addFolder(folder.name ?? "") { result in
switch result {
case .success(let destinationFolder):
@@ -456,17 +468,24 @@ private extension SidebarOutlineDataSource {
NSApplication.shared.presentError(error)
}
}
}
func acceptLocalFoldersDrop(_ outlineView: NSOutlineView, _ draggedFolders: Set<PasteboardFolder>, _ parentNode: Node, _ index: Int) -> Bool {
guard let draggedNodes = draggedNodes else {
guard draggedFolders.isEmpty == false else {
return false
}
draggedNodes.forEach { node in
if !sameAccount(node, parentNode) {
copyFolderBetweenAccounts(node: node, to: parentNode)
draggedFolders.forEach { pasteboardFolder in
guard let sourceAccountID = pasteboardFolder.accountID,
let sourceAccount = AccountManager.shared.existingAccount(with: sourceAccountID),
let folderStringID = pasteboardFolder.folderID,
let folderID = Int(folderStringID),
let folder = sourceAccount.existingFolder(withID: folderID)
else {
return
}
if !sameAccount(pasteboardFolder, parentNode) {
copyFolderBetweenAccounts(folder: folder, to: parentNode)
}
}
@@ -506,8 +525,22 @@ private extension SidebarOutlineDataSource {
return false
}
func sameAccount(_ node: Node, _ parentNode: Node) -> Bool {
if let accountID = nodeAccountID(node), let parentAccountID = nodeAccountID(parentNode) {
func sameAccount(_ pasteboardWebFeed: PasteboardWebFeed, _ parentNode: Node) -> Bool {
if let accountID = pasteboardWebFeed.accountID {
return sameAccount(accountID, parentNode)
}
return false
}
func sameAccount(_ pasteboardFolder: PasteboardFolder, _ parentNode: Node) -> Bool {
if let accountID = pasteboardFolder.accountID {
return sameAccount(accountID, parentNode)
}
return false
}
func sameAccount(_ accountID: String, _ parentNode: Node) -> Bool {
if let parentAccountID = nodeAccountID(parentNode) {
if accountID == parentAccountID {
return true
}