Improve drag and drop items (#1145)

mvasilak · web-flow · commit 6f5891bfcba2 · 2025-07-07T13:25:30.000+03:00
* Improve RItem based drag item data representation

Allow drag and drop of multiple valid items
Fix TagFilterViewController drop logic

* Add text data representation for RItem based drag item

* Improve DragDropController
diff --git a/Zotero/Controllers/Citation/CitationController.swift b/Zotero/Controllers/Citation/CitationController.swift
@@ -24,6 +24,7 @@ private struct StyleData {
     }
 }
 
+@preconcurrency
 class CitationController: NSObject {
     struct Session: Hashable {
         let id = UUID()
diff --git a/Zotero/Controllers/DragDropController.swift b/Zotero/Controllers/DragDropController.swift
@@ -7,48 +7,74 @@
 //
 
 import UIKit
+import UniformTypeIdentifiers
 
-final class DragDropController {
-    func dragItem(from item: RItem) -> UIDragItem {
-        let provider = NSItemProvider(object: item.key as NSString)
-        let dragItem = UIDragItem(itemProvider: provider)
-        dragItem.localObject = item
-        return dragItem
-    }
+@preconcurrency
+import RxSwift
 
-    func item(from dragItem: UIDragItem) -> RItem? {
-        return dragItem.localObject as? RItem
-    }
+struct DragSessionItemsLocalContext {
+    let libraryIdentifier: LibraryIdentifier
+    let keys: Set<String>
 
-    func dragItem(from tag: RTag) -> UIDragItem {
-        let provider = NSItemProvider(object: tag.name as NSString)
-        let dragItem = UIDragItem(itemProvider: provider)
-        dragItem.localObject = tag
-        return dragItem
-    }
-
-    func tag(from dragItem: UIDragItem) -> RTag? {
-        return dragItem.localObject as? RTag
+    func createNewContext(with item: RItem) -> Self? {
+        guard libraryIdentifier == item.libraryIdentifier, !keys.contains(item.key) else { return nil }
+        return Self(libraryIdentifier: libraryIdentifier, keys: keys.union([item.key]))
     }
+}
 
-    func keys(from dragItems: [UIDragItem], completed: @escaping (Set<String>) -> Void) {
-        var keys: Set<String> = []
-
-        let group = DispatchGroup()
-
-        for dragItem in dragItems {
-            group.enter()
+final class DragDropController {
+    func dragItem(from item: RItem, citationController: CitationController?, disposeBag: DisposeBag) -> UIDragItem {
+        let itemProvider = NSItemProvider()
+        if let citationController {
+            registerDataRepresentation(for: itemProvider, contentType: .html, item: item, citationController: citationController, format: .html, disposeBag: disposeBag)
+            registerDataRepresentation(for: itemProvider, contentType: .plainText, item: item, citationController: citationController, format: .text, disposeBag: disposeBag)
+        }
+        let dragItem = UIDragItem(itemProvider: itemProvider)
+        dragItem.localObject = item
+        return dragItem
 
-            dragItem.itemProvider.loadObject(ofClass: NSString.self) { nsString, _ in
-                if let key = nsString as? String {
-                    keys.insert(key)
+        func registerDataRepresentation(
+            for itemProvider: NSItemProvider,
+            contentType: UTType,
+            item: RItem,
+            citationController: CitationController,
+            format: CitationController.Format,
+            disposeBag: DisposeBag
+        ) {
+            let key = item.key
+            let libraryId = item.libraryIdentifier
+            itemProvider.registerDataRepresentation(for: contentType, visibility: .all) { completion in
+                let progress = Progress(totalUnitCount: 2)
+                DispatchQueue.main.async {
+                    var session: CitationController.Session?
+                    citationController.startSession(
+                        for: Set(arrayLiteral: key),
+                        libraryId: libraryId,
+                        styleId: Defaults.shared.quickCopyStyleId,
+                        localeId: Defaults.shared.quickCopyLocaleId
+                    )
+                    .flatMap({ startedSession -> Single<String> in
+                        session = startedSession
+                        progress.completedUnitCount = 1
+                        return citationController.bibliography(for: startedSession, format: format)
+                    })
+                    .subscribe { bibliography in
+                        progress.completedUnitCount = 2
+                        completion(bibliography.data(using: .utf8), nil)
+                        if let session {
+                            citationController.endSession(session)
+                        }
+                    } onFailure: { error in
+                        progress.completedUnitCount = 2
+                        completion(nil, error)
+                        if let session {
+                            citationController.endSession(session)
+                        }
+                    }
+                    .disposed(by: disposeBag)
                 }
-                group.leave()
+                return progress
             }
         }
-
-        group.notify(queue: .main) {
-            completed(keys)
-        }
     }
 }
diff --git a/Zotero/Scenes/Detail/Items/Views/ItemsTableViewHandler.swift b/Zotero/Scenes/Detail/Items/Views/ItemsTableViewHandler.swift
@@ -54,6 +54,7 @@ final class ItemsTableViewHandler: NSObject {
     private unowned let delegate: ItemsTableViewHandlerDelegate
     private unowned let dataSource: ItemsTableViewDataSource
     private unowned let dragDropController: DragDropController
+    private unowned let citationController: CitationController?
     private let disposeBag: DisposeBag
 
     private var reloadAnimationsDisabled: Bool
@@ -62,12 +63,14 @@ final class ItemsTableViewHandler: NSObject {
         tableView: UITableView,
         delegate: ItemsTableViewHandlerDelegate,
         dataSource: ItemsTableViewDataSource,
-        dragDropController: DragDropController
+        dragDropController: DragDropController,
+        citationController: CitationController?
     ) {
         self.tableView = tableView
         self.delegate = delegate
         self.dataSource = dataSource
         self.dragDropController = dragDropController
+        self.citationController = citationController
         reloadAnimationsDisabled = false
         disposeBag = DisposeBag()
 
@@ -308,56 +311,47 @@ extension ItemsTableViewHandler: UITableViewDelegate {
 extension ItemsTableViewHandler: UITableViewDragDelegate {
     func tableView(_ tableView: UITableView, itemsForBeginning session: UIDragSession, at indexPath: IndexPath) -> [UIDragItem] {
         guard let item = dataSource.object(at: indexPath.row) as? RItem else { return [] }
-        return [self.dragDropController.dragItem(from: item)]
+        session.localContext = DragSessionItemsLocalContext(libraryIdentifier: item.libraryIdentifier, keys: Set([item.key]))
+        return [dragDropController.dragItem(from: item, citationController: citationController, disposeBag: disposeBag)]
+    }
+
+    func tableView(_ tableView: UITableView, itemsForAddingTo session: any UIDragSession, at indexPath: IndexPath, point: CGPoint) -> [UIDragItem] {
+        guard let item = dataSource.object(at: indexPath.row) as? RItem,
+              let localContext = session.localContext as? DragSessionItemsLocalContext,
+              let newLocalContext = localContext.createNewContext(with: item)
+        else { return [] }
+        session.localContext = newLocalContext
+        return [dragDropController.dragItem(from: item, citationController: citationController, disposeBag: disposeBag)]
     }
 }
 
 extension ItemsTableViewHandler: UITableViewDropDelegate {
     func tableView(_ tableView: UITableView, performDropWith coordinator: UITableViewDropCoordinator) {
-        guard let object = coordinator.destinationIndexPath.flatMap({ dataSource.object(at: $0.row) }) else { return }
-
+        guard let indexPath = coordinator.destinationIndexPath, let object = dataSource.object(at: indexPath.row) else { return }
         switch coordinator.proposal.operation {
         case .copy:
             let key = object.key
-            let localObject = coordinator.items.first?.dragItem.localObject
-            self.dragDropController.keys(from: coordinator.items.map({ $0.dragItem })) { [weak self] keys in
-                guard let self else { return }
-                if localObject is RItem {
-                    delegate.process(dragAndDropAction: .moveItems(keys: keys, toKey: key))
-                } else if localObject is RTag {
-                    delegate.process(dragAndDropAction: .tagItem(key: key, libraryId: object.libraryIdentifier, tags: keys))
-                }
-            }
-        default: break
-        }
-    }
+            guard let localContext = coordinator.session.localDragSession?.localContext as? DragSessionItemsLocalContext, !localContext.keys.isEmpty else { break }
+            delegate.process(dragAndDropAction: .moveItems(keys: localContext.keys, toKey: key))
 
-    func tableView(_ tableView: UITableView, dropSessionDidUpdate session: UIDropSession, withDestinationIndexPath destinationIndexPath: IndexPath?) -> UITableViewDropProposal {
-        guard
-            delegate.library.metadataEditable,                // allow only when library is editable
-            session.localDragSession != nil,                  // allow only local drag session
-            let destinationIndexPath = destinationIndexPath,
-            destinationIndexPath.row < dataSource.count,
-            session.items.first?.localObject is RItem
-        else {
-            return UITableViewDropProposal(operation: .forbidden)
+        default:
+            break
         }
-        return self.itemDropSessionDidUpdate(session: session, withDestinationIndexPath: destinationIndexPath)
     }
 
-    private func itemDropSessionDidUpdate(session: UIDropSession, withDestinationIndexPath destinationIndexPath: IndexPath) -> UITableViewDropProposal {
-        guard let object = dataSource.object(at: destinationIndexPath.row) else {
-            return UITableViewDropProposal(operation: .forbidden)
-        }
-        let dragItemsLibraryId = session.items.compactMap({ $0.localObject as? RItem }).compactMap({ $0.libraryId }).first
-
-        if dragItemsLibraryId != object.libraryIdentifier ||                                // allow dropping only to the same library
-           object.isNote || object.isAttachment ||                                          // allow dropping only to non-standalone items
-           session.items.compactMap({ self.dragDropController.item(from: $0) })             // allow drops of only standalone items
-                        .contains(where: { $0.rawType != ItemTypes.attachment && $0.rawType != ItemTypes.note }) {
-           return UITableViewDropProposal(operation: .forbidden)
-        }
-
+    func tableView(_ tableView: UITableView, dropSessionDidUpdate session: UIDropSession, withDestinationIndexPath destinationIndexPath: IndexPath?) -> UITableViewDropProposal {
+        let library = delegate.library
+        guard library.metadataEditable,
+              let localContext = session.localDragSession?.localContext as? DragSessionItemsLocalContext,
+              localContext.libraryIdentifier == library.identifier,
+              !localContext.keys.isEmpty,
+              let destinationIndexPath,
+              destinationIndexPath.row < dataSource.count,
+              let object = dataSource.object(at: destinationIndexPath.row),
+              !object.isNote,
+              !object.isAttachment,
+              !session.items.compactMap({ $0.localObject as? RItem }).contains(where: { $0.rawType != ItemTypes.attachment && $0.rawType != ItemTypes.note })
+        else { return UITableViewDropProposal(operation: .forbidden) }
         return UITableViewDropProposal(operation: .copy, intent: .insertIntoDestinationIndexPath)
     }
 }
diff --git a/Zotero/Scenes/Detail/Items/Views/ItemsViewController.swift b/Zotero/Scenes/Detail/Items/Views/ItemsViewController.swift
@@ -50,7 +50,13 @@ final class ItemsViewController: BaseItemsViewController {
             recognizerController: controllers.userControllers?.recognizerController,
             schemaController: controllers.schemaController
         )
-        handler = ItemsTableViewHandler(tableView: tableView, delegate: self, dataSource: dataSource, dragDropController: controllers.dragDropController)
+        handler = ItemsTableViewHandler(
+            tableView: tableView,
+            delegate: self,
+            dataSource: dataSource,
+            dragDropController: controllers.dragDropController,
+            citationController: controllers.userControllers?.citationController
+        )
         toolbarController = ItemsToolbarController(viewController: self, data: toolbarData, collection: collection, library: library, delegate: self)
         setupRightBarButtonItems(expectedItems: rightBarButtonItemTypes(for: viewModel.state))
         setupFileObservers()
diff --git a/Zotero/Scenes/Detail/Trash/Views/TrashViewController.swift b/Zotero/Scenes/Detail/Trash/Views/TrashViewController.swift
@@ -38,7 +38,13 @@ final class TrashViewController: BaseItemsViewController {
         super.viewDidLoad()
 
         dataSource = TrashTableViewDataSource(viewModel: viewModel, schemaController: controllers.schemaController, fileDownloader: controllers.userControllers?.fileDownloader)
-        handler = ItemsTableViewHandler(tableView: tableView, delegate: self, dataSource: dataSource, dragDropController: controllers.dragDropController)
+        handler = ItemsTableViewHandler(
+            tableView: tableView,
+            delegate: self,
+            dataSource: dataSource,
+            dragDropController: controllers.dragDropController,
+            citationController: controllers.userControllers?.citationController
+        )
         toolbarController = ItemsToolbarController(viewController: self, data: toolbarData, collection: collection, library: library, delegate: self)
         setupRightBarButtonItems(expectedItems: rightBarButtonItemTypes(for: viewModel.state))
         setupDownloadObserver()
diff --git a/Zotero/Scenes/Master/Collections/Views/ExpandableCollectionsCollectionViewHandler.swift b/Zotero/Scenes/Master/Collections/Views/ExpandableCollectionsCollectionViewHandler.swift
@@ -260,25 +260,24 @@ extension ExpandableCollectionsCollectionViewHandler: UICollectionViewDropDelega
         guard let indexPath = coordinator.destinationIndexPath, let key = dataSource.itemIdentifier(for: indexPath)?.identifier.key else { return }
         switch coordinator.proposal.operation {
         case .copy:
-            dragDropController.keys(from: coordinator.items.map({ $0.dragItem })) { [weak self] keys in
-                self?.viewModel.process(action: .assignKeysToCollection(itemKeys: keys, collectionKey: key))
-            }
+            guard let localContext = coordinator.session.localDragSession?.localContext as? DragSessionItemsLocalContext, !localContext.keys.isEmpty else { break }
+            viewModel.process(action: .assignKeysToCollection(itemKeys: localContext.keys, collectionKey: key))
 
         default:
             break
         }
     }
 
     func collectionView(_ collectionView: UICollectionView, dropSessionDidUpdate session: UIDropSession, withDestinationIndexPath destinationIndexPath: IndexPath?) -> UICollectionViewDropProposal {
-        guard
-            viewModel.state.library.metadataEditable &&                                                                                     // allow only when library is editable
-            session.localDragSession != nil &&                                                                                              // allow only local drag session
-            session.items.compactMap({ $0.localObject as? RItem }).compactMap({ $0.libraryId }).first == viewModel.state.library.identifier // allow drag from the same library
+        let library = viewModel.state.library
+        guard library.metadataEditable,
+              let localContext = session.localDragSession?.localContext as? DragSessionItemsLocalContext,
+              localContext.libraryIdentifier == library.identifier,
+              !localContext.keys.isEmpty,
+              let destinationIndexPath,
+              let collection = dataSource.itemIdentifier(for: destinationIndexPath),
+              collection.identifier.isCollection
         else { return UICollectionViewDropProposal(operation: .forbidden) }
-        // Allow only dropping to user collections, not custom collections, such as "All Items" or "My Publications"
-        if let destination = destinationIndexPath, let collection = dataSource.itemIdentifier(for: destination), collection.identifier.isCollection {
-            return UICollectionViewDropProposal(operation: .copy, intent: .insertIntoDestinationIndexPath)
-        }
-        return UICollectionViewDropProposal(operation: .forbidden)
+        return UICollectionViewDropProposal(operation: .copy, intent: .insertIntoDestinationIndexPath)
     }
 }
diff --git a/Zotero/Scenes/Master/TagFiltering/Views/TagFilterViewController.swift b/Zotero/Scenes/Master/TagFiltering/Views/TagFilterViewController.swift
@@ -103,9 +103,8 @@ class TagFilterViewController: UIViewController {
             self.collectionView = collectionView
             view.insertSubview(collectionView, belowSubview: searchContainer)
 
-            if traitCollection.horizontalSizeClass == .regular && UIDevice.current.userInterfaceIdiom == .pad {
-                collectionView.dropDelegate = self
-            } else {
+            collectionView.dropDelegate = self
+            if traitCollection.horizontalSizeClass != .regular || UIDevice.current.userInterfaceIdiom != .pad {
                 collectionView.keyboardDismissMode = .onDrag
             }
 
@@ -274,28 +273,23 @@ extension TagFilterViewController: UICollectionViewDataSource {
 
 extension TagFilterViewController: UICollectionViewDropDelegate {
     func collectionView(_ collectionView: UICollectionView, dropSessionDidUpdate session: UIDropSession, withDestinationIndexPath destinationIndexPath: IndexPath?) -> UICollectionViewDropProposal {
-        guard delegate?.currentLibrary.metadataEditable == true,    // allow only when library is editable
-              session.localDragSession != nil,                  // allow only local drag session
-              let destinationIndexPath = destinationIndexPath,
-              destinationIndexPath.row < self.collectionView(collectionView, numberOfItemsInSection: destinationIndexPath.section) else {
-            return UICollectionViewDropProposal(operation: .forbidden)
-        }
-
-        let dragItemsLibraryId = session.items.compactMap({ $0.localObject as? RItem }).compactMap({ $0.libraryId }).first
-        if dragItemsLibraryId == delegate?.currentLibrary.identifier {
-            return UICollectionViewDropProposal(operation: .copy, intent: .insertIntoDestinationIndexPath)
-        }
-
-        return UICollectionViewDropProposal(operation: .forbidden)
+        guard let library = delegate?.currentLibrary,
+              library.metadataEditable,
+              let localContext = session.localDragSession?.localContext as? DragSessionItemsLocalContext,
+              localContext.libraryIdentifier == library.identifier,
+              !localContext.keys.isEmpty,
+              let destinationIndexPath,
+              destinationIndexPath.row < viewModel.state.tags.count
+        else { return UICollectionViewDropProposal(operation: .forbidden) }
+        return UICollectionViewDropProposal(operation: .copy, intent: .insertIntoDestinationIndexPath)
     }
 
     func collectionView(_ collectionView: UICollectionView, performDropWith coordinator: UICollectionViewDropCoordinator) {
-        guard let tag = coordinator.destinationIndexPath.flatMap({ tag(for: $0) }), let libraryId = (coordinator.items.first?.dragItem.localObject as? RItem)?.libraryId else { return }
+        guard let indexPath = coordinator.destinationIndexPath, let tag = tag(for: indexPath) else { return }
         switch coordinator.proposal.operation {
         case .copy:
-            dragDropController.keys(from: coordinator.items.map({ $0.dragItem })) { [weak viewModel] keys in
-                viewModel?.process(action: .assignTag(name: tag.tag.name, toItemKeys: keys, libraryId: libraryId))
-            }
+            guard let localContext = coordinator.session.localDragSession?.localContext as? DragSessionItemsLocalContext, !localContext.keys.isEmpty else { break }
+            viewModel.process(action: .assignTag(name: tag.tag.name, toItemKeys: localContext.keys, libraryId: localContext.libraryIdentifier))
 
         default:
             break

Original file line number	Diff line number	Diff line change
`@@ -24,6 +24,7 @@ private struct StyleData {`
`24`	`24`	`}`
`25`	`25`	`}`
`26`	`26`
	`27`	`+@preconcurrency`
`27`	`28`	`class CitationController: NSObject {`
`28`	`29`	`struct Session: Hashable {`
`29`	`30`	`let id = UUID()`