From 7cf90974679c9c2edc5d1dededdba3c3eb9351d3 Mon Sep 17 00:00:00 2001 From: abaevbog Date: Tue, 15 Oct 2024 03:34:42 -0700 Subject: [PATCH] vpat 16: context menu as a drag-drop alternative to move/copy collections (#4420) - Added menuitems to move collections within the same library and to copy collections - "Move to" only displays collections within the current library - "Copy to" displays all libraries, if more than one library exists. If there is only one library, top-level collections from "My Library" are displayed. - while copying within the same library, create copies of all collections and add items into them, without actually duplicating items - while copying between different libraries, items will be duplicated, the same way it is done when collections are dragged and dropped in another library --- chrome/content/zotero/collectionTree.jsx | 510 +++++++++++++---------- chrome/content/zotero/zoteroPane.js | 193 ++++++++- chrome/content/zotero/zoteroPane.xhtml | 10 +- chrome/locale/en-US/zotero/zotero.ftl | 4 + test/tests/zoteroPaneTest.js | 140 +++++++ 5 files changed, 628 insertions(+), 229 deletions(-) diff --git a/chrome/content/zotero/collectionTree.jsx b/chrome/content/zotero/collectionTree.jsx index c664238728..2524ed1267 100644 --- a/chrome/content/zotero/collectionTree.jsx +++ b/chrome/content/zotero/collectionTree.jsx @@ -1796,7 +1796,276 @@ var CollectionTree = class CollectionTree extends LibraryTree { } return true; } + + /** + * Copy a given item into another library. Used when we need to create a copy of a collection + * in another library if collection is drag-dropped into a group it is not a part of. + */ + async _copyItem({ item, targetLibraryID, targetTreeRow, options }) { + // Check if there's already a copy of this item in the library + var linkedItem = await item.getLinkedItem(targetLibraryID, true); + if (linkedItem) { + // If linked item is in the trash, undelete it and remove it from collections + // (since it shouldn't be restored to previous collections) + if (linkedItem.deleted) { + linkedItem.setCollections(); + linkedItem.deleted = false; + await linkedItem.save({ + skipSelect: true + }); + } + return linkedItem.id; + + /* + // TODO: support tags, related, attachments, etc. + + // Overlay source item fields on unsaved clone of linked item + var newItem = item.clone(false, linkedItem.clone(true)); + newItem.setField('dateAdded', item.dateAdded); + newItem.setField('dateModified', item.dateModified); + + var diff = newItem.diff(linkedItem, false, ["dateAdded", "dateModified"]); + if (!diff) { + // Check if creators changed + var creatorsChanged = false; + + var creators = item.getCreators(); + var linkedCreators = linkedItem.getCreators(); + if (creators.length != linkedCreators.length) { + Zotero.debug('Creators have changed'); + creatorsChanged = true; + } + else { + for (var i=0; i { + var collections = [{ + id: collection.id, + children: collection.getDescendents(true), + type: 'collection' + }]; + + var addItems = new Map(); + await this._copyCollections({ + descendents: collections, + parentID: targetCollectionID, + addItems, + targetLibraryID, + targetTreeRow, + copyOptions + }); + for (let [collectionID, items] of addItems.entries()) { + let collection = await Zotero.Collections.getAsync(collectionID); + await collection.addItems(items); + } + + // TODO: add subcollections and subitems, if they don't already exist, + // and display a warning if any of the subcollections already exist + }); + } + async onDrop(event, index) { const treeRow = this.getRow(index); this._dropRow = null; @@ -1809,7 +2078,6 @@ var CollectionTree = class CollectionTree extends LibraryTree { || !(await this.canDropCheckAsync(row, orient, dataTransfer))) { return false; } - var dragData = Zotero.DragDrop.getDataFromDataTransfer(dataTransfer); if (!dragData) { Zotero.debug("No drag data"); @@ -1828,237 +2096,20 @@ var CollectionTree = class CollectionTree extends LibraryTree { childFileAttachments: Zotero.Prefs.get('groups.copyChildFileAttachments'), annotations: Zotero.Prefs.get('groups.copyAnnotations'), }; - var copyItem = async function (item, targetLibraryID, options) { - var targetLibraryType = Zotero.Libraries.get(targetLibraryID).libraryType; - - // Check if there's already a copy of this item in the library - var linkedItem = await item.getLinkedItem(targetLibraryID, true); - if (linkedItem) { - // If linked item is in the trash, undelete it and remove it from collections - // (since it shouldn't be restored to previous collections) - if (linkedItem.deleted) { - linkedItem.setCollections(); - linkedItem.deleted = false; - await linkedItem.save({ - skipSelect: true - }); - } - return linkedItem.id; - - /* - // TODO: support tags, related, attachments, etc. - - // Overlay source item fields on unsaved clone of linked item - var newItem = item.clone(false, linkedItem.clone(true)); - newItem.setField('dateAdded', item.dateAdded); - newItem.setField('dateModified', item.dateModified); - - var diff = newItem.diff(linkedItem, false, ["dateAdded", "dateModified"]); - if (!diff) { - // Check if creators changed - var creatorsChanged = false; - - var creators = item.getCreators(); - var linkedCreators = linkedItem.getCreators(); - if (creators.length != linkedCreators.length) { - Zotero.debug('Creators have changed'); - creatorsChanged = true; - } - else { - for (var i=0; i { for (let item of chunk) { - var id = await copyItem(item, targetLibraryID, copyOptions) + var id = await this._copyItem({ + item, + targetLibraryID, + targetTreeRow, + options: copyOptions + }); // Standalone attachments might not get copied if (!id) { continue; @@ -2147,7 +2203,7 @@ var CollectionTree = class CollectionTree extends LibraryTree { newIDs.push(id); } }); - } + }.bind(this) ); if (toReconcile.length) { diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js index afbad7613e..877ba71310 100644 --- a/chrome/content/zotero/zoteroPane.js +++ b/chrome/content/zotero/zoteroPane.js @@ -2565,6 +2565,55 @@ var ZoteroPane = new function() } }); + // Move selected collection to specified target collection or library. + // Target has to be in the same library as the currently selected collection. + this.moveCollection = async (target) => { + let selected = this.getSelectedCollection(); + if (!selected) return; + + if (target.libraryID !== selected.libraryID) { + throw new Error("Moving collections is only possible within the same library."); + } + if (target instanceof Zotero.Library) { + selected.parentID = null; + } + else { + selected.parentID = target.id; + } + + await selected.saveTx(); + }; + + // Copy selected collection into another collection or library. + // Partially, a replication of drag-drop mechanism from CollectionTree.onDrop. + this.copyCollection = async (target) => { + let selected = this.getSelectedCollection(); + if (!selected) return; + + let targetTreeRowID = `L${target.libraryID}`; + if (target instanceof Zotero.Collection) { + targetTreeRowID = `C${target.id}`; + // Make sure the row is actually visible + await ZoteroPane.collectionsView.expandToCollection(target.id); + } + let targetTreeRowIndex = ZoteroPane.collectionsView.getRowIndexByID(targetTreeRowID); + let targetTreeRow = ZoteroPane.collectionsView.getRow(targetTreeRowIndex); + let copyOptions = { + tags: Zotero.Prefs.get('groups.copyTags'), + childNotes: Zotero.Prefs.get('groups.copyChildNotes'), + childLinks: Zotero.Prefs.get('groups.copyChildLinks'), + childFileAttachments: Zotero.Prefs.get('groups.copyChildFileAttachments'), + annotations: Zotero.Prefs.get('groups.copyAnnotations'), + }; + ZoteroPane.collectionsView.executeCollectionCopy({ + collection: selected, + targetCollectionID: target instanceof Zotero.Collection ? target.id : null, + targetLibraryID: target.libraryID, + targetTreeRow, + copyOptions + }); + }; + this.toggleSelectedItemsRead = Zotero.Promise.coroutine(function* () { yield Zotero.FeedItems.toggleReadByID(this.getSelectedItems(true)); }); @@ -3179,6 +3228,12 @@ var ZoteroPane = new function() id: "editSelectedCollection", oncommand: () => this.editSelectedCollection() }, + { + id: "moveCollection", + }, + { + id: "copyCollection" + }, { id: "duplicate", oncommand: () => this.duplicateSelectedCollection() @@ -3297,6 +3352,8 @@ var ZoteroPane = new function() 'newSubcollection', 'sep2', 'editSelectedCollection', + 'moveCollection', + 'copyCollection', 'deleteCollection', 'deleteCollectionAndItems', 'sep3', @@ -3316,6 +3373,8 @@ var ZoteroPane = new function() // Adjust labels document.l10n.setAttributes(m.editSelectedCollection, 'collections-menu-rename-collection'); + document.l10n.setAttributes(m.moveCollection, 'collections-menu-move-collection'); + document.l10n.setAttributes(m.copyCollection, 'collections-menu-copy-collection'); m.deleteCollection.setAttribute('label', Zotero.getString('pane.collections.menu.delete.collection')); m.deleteCollectionAndItems.setAttribute('label', Zotero.getString('pane.collections.menu.delete.collectionAndItems')); @@ -3978,7 +4037,139 @@ var ZoteroPane = new function() }); - this.buildAddToCollectionMenu = function (event) { + // Build a menu to move or copy a collection into another collection and library. + // Alternative to dropping collection into another collection or group + this.buildMoveCollectionMenu = function (event) { + if (event.target !== event.currentTarget) return; + let popup = event.target; + popup.replaceChildren(); + + let selected = this.getSelectedCollection(); + + // Add current library at the top to be able to move collections into it + let library = Zotero.Libraries.get(ZoteroPane.getSelectedLibraryID()); + let libraryMenuItem = document.createXULElement("menuitem"); + libraryMenuItem.setAttribute("label", library.name); + libraryMenuItem.setAttribute("image", library.treeViewImage); + libraryMenuItem.setAttribute("value", library.treeViewID); + libraryMenuItem.addEventListener("command", (event) => { + if (event.target.tagName == 'menuitem') { + this.moveCollection(library); + event.stopPropagation(); + } + }); + // Disable for already top-level collections + libraryMenuItem.disabled = !selected.parentID; + popup.appendChild(libraryMenuItem); + popup.appendChild(document.createXULElement("menuseparator")); + + // Build menus for each top-level collection of this library + let collections = Zotero.Collections.getByLibrary(this.getSelectedLibraryID()); + for (let col of collections) { + let menuItem = Zotero.Utilities.Internal.createMenuForTarget( + col, + popup, + null, + (event, collection) => { + if (event.target.tagName == 'menuitem') { + this.moveCollection(collection); + event.stopPropagation(); + } + }, + + (target) => { + // can't move collection into itself, its parent or its children + return selected == target + || selected.parentKey == target.key + || selected.hasDescendent('collection', target.id); + } + ); + popup.append(menuItem); + } + }; + + + this.buildCopyCollectionMenu = function (event) { + if (event.target !== event.currentTarget) return; + let popup = document.getElementById("zotero-copy-collection-popup"); + popup.replaceChildren(); + let selected = this.getSelectedCollection(); + + // Fetch all libraries + let topLevelEntries = Zotero.Libraries.getAll().filter(lib => !(lib instanceof Zotero.Feed)); + + // Check which libraries have collections linked to the selected collection + // and disable their menuitems. Same logic as in CollectionTree.canDropCheckAsync. + let linkedCollectionsExist = {}; + (async () => { + for (let library of topLevelEntries) { + if (library.libraryID == selected.libraryID) continue; + // Check which library has a collection linked to the selected collection + let linkedCollection = await selected.getLinkedCollection(library.libraryID, true); + linkedCollectionsExist[library.libraryID] = linkedCollection; + // Also check which library has collections linked to a subcollection of the selected collection + for (let descendent of selected.getDescendents(false, 'collection')) { + let subcollection = Zotero.Collections.get(descendent.id); + let linkedSubcollection = await subcollection.getLinkedCollection(library.libraryID, true); + if (linkedSubcollection) { + linkedCollectionsExist[library.libraryID] = linkedSubcollection; + } + } + } + // Libraries that have linked collections have their menus disabled + for (let libraryMenuItem of [...popup.childNodes]) { + let menuItemLibID = libraryMenuItem.getAttribute("value").substring(1); + if (linkedCollectionsExist[menuItemLibID]) { + libraryMenuItem.disabled = true; + } + } + })(); + + // If there is only one library, display its collections as top-level menuitems + if (topLevelEntries.length == 1) { + // Manually add My Library menuitem at the top, so one can still copy into it + let myLibrary = topLevelEntries[0]; + let myLibraryMenuItem = document.createXULElement("menuitem"); + myLibraryMenuItem.setAttribute("label", myLibrary.name); + myLibraryMenuItem.setAttribute("image", myLibrary.treeViewImage); + myLibraryMenuItem.setAttribute("value", myLibrary.treeViewID); + myLibraryMenuItem.addEventListener("command", (event) => { + if (event.target.tagName == 'menuitem') { + this.copyCollection(myLibrary); + event.stopPropagation(); + } + }); + popup.appendChild(myLibraryMenuItem); + popup.appendChild(document.createXULElement("menuseparator")); + + // Top-level collections used to construct the menus + topLevelEntries = Zotero.Collections.getByLibrary(topLevelEntries[0].id); + } + + // Build menus for all libraries (or collections) + for (let obj of topLevelEntries) { + let menuItem = Zotero.Utilities.Internal.createMenuForTarget( + obj, + popup, + null, + (event, collection) => { + if (event.target.tagName == 'menuitem') { + this.copyCollection(collection); + event.stopPropagation(); + } + }, + + (target) => { + // can't copy collection into itself or into non-editable groups + return selected == target + || (target instanceof Zotero.Group && !target.editable); + } + ); + popup.append(menuItem); + } + }; + + this.buildAddItemToCollectionMenu = function (event) { if (event.target !== event.currentTarget) return; let popup = event.target; diff --git a/chrome/content/zotero/zoteroPane.xhtml b/chrome/content/zotero/zoteroPane.xhtml index 6495fcd2df..fb7060c7f9 100644 --- a/chrome/content/zotero/zoteroPane.xhtml +++ b/chrome/content/zotero/zoteroPane.xhtml @@ -912,6 +912,14 @@ + + + + + + + + @@ -967,7 +975,7 @@ - + diff --git a/chrome/locale/en-US/zotero/zotero.ftl b/chrome/locale/en-US/zotero/zotero.ftl index 02233ced0a..3e0b0b2bde 100644 --- a/chrome/locale/en-US/zotero/zotero.ftl +++ b/chrome/locale/en-US/zotero/zotero.ftl @@ -96,6 +96,10 @@ collections-menu-rename-collection = .label = Rename Collection collections-menu-edit-saved-search = .label = Edit Saved Search +collections-menu-move-collection = + .label = Move To +collections-menu-copy-collection = + .label = Copy To item-creator-moveDown = .label = Move Down diff --git a/test/tests/zoteroPaneTest.js b/test/tests/zoteroPaneTest.js index 5f0cee9037..22b8658ddb 100644 --- a/test/tests/zoteroPaneTest.js +++ b/test/tests/zoteroPaneTest.js @@ -1796,4 +1796,144 @@ describe("ZoteroPane", function() { assert.equal(attachment.getCollections()[0], collection.id); }); }); + + describe("#copyCollection", function () { + it("should copy collection within the same library", async function () { + let collectionParent = await createDataObject('collection'); + let collectionChild = await createDataObject('collection', { parentID: collectionParent.id }); + let collectionDestination = await createDataObject('collection'); + + let itemOne = await createDataObject('item', { collections: [collectionParent.id] }); + let itemTwo = await createDataObject('item', { collections: [collectionChild.id] }); + + await zp.collectionsView.selectByID("C" + collectionParent.id); + + await zp.copyCollection(collectionDestination); + await waitForNotifierEvent("add", "collection"); + + // Newly created collections have the same names as the original ones + let collectionNames = collectionDestination.getDescendents(false, 'collection').map(col => col.name); + assert.sameMembers(collectionNames, [collectionParent.name, collectionChild.name]); + + // Newly created collections contain the same items + let items = collectionDestination.getDescendents(false, 'item').map(item => item.id); + assert.sameMembers(items, [itemOne.id, itemTwo.id]); + }); + + it("should duplicate top-level collection", async function () { + let collection = await createDataObject('collection'); + let mylibrary = Zotero.Libraries.get(collection.libraryID); + + let itemOne = await createDataObject('item', { collections: [collection.id] }); + + await zp.collectionsView.selectByID("C" + collection.id); + + await zp.copyCollection(mylibrary); + await waitForNotifierEvent("add", "collection"); + + // Find the duplicated collection and make sure it exists + let topLevelCollections = Zotero.Collections.getByLibrary(mylibrary.id); + let newCollection = topLevelCollections.find(col => col.name == collection.name && collection.id !== col.id); + assert.exists(newCollection); + + // Newly created collection contain the same item + let items = newCollection.getDescendents(false, 'item').map(item => item.id); + assert.sameMembers(items, [itemOne.id]); + }); + + it("should copy collection between libraries", async function () { + let groupDestination = await createGroup(); + let groupCollection = await createDataObject('collection', { libraryID: groupDestination.libraryID }); + + let collectionParent = await createDataObject('collection'); + let collectionChild = await createDataObject('collection', { parentID: collectionParent.id }); + + let itemOne = await createDataObject('item', { collections: [collectionParent.id] }); + let itemTwo = await createDataObject('item', { collections: [collectionChild.id] }); + + await zp.collectionsView.selectByID("C" + collectionParent.id); + + await zp.copyCollection(groupCollection); + + await waitForNotifierEvent("add", "collection"); + + // Newly created collections have the same names as the original ones + let collectionNames = groupCollection.getDescendents(false, 'collection').map(col => col.name); + assert.sameMembers(collectionNames, [collectionParent.name, collectionChild.name]); + + // Newly created collections also have copies of items + let items = groupCollection.getDescendents(false, 'item').map(item => Zotero.Items.get(item.id).getDisplayTitle()); + assert.sameMembers(items, [itemOne.getDisplayTitle(), itemTwo.getDisplayTitle()]); + }); + + it("should not allow copying between libraries if there is a linked collection", async function () { + let groupDestination = await createGroup(); + let groupCollection = await createDataObject('collection', { libraryID: groupDestination.libraryID }); + + let collection = await createDataObject('collection'); + let collectionChild = await createDataObject('collection', { parentID: collection.id }); + + await zp.collectionsView.selectByID("C" + collectionChild.id); + + await zp.copyCollection(groupCollection); + + await waitForNotifierEvent("add", "collection"); + + // Collection has been copies + let groupCollections = groupCollection.getDescendents(false, 'collection'); + let newCollectionID = groupCollections.find(col => col.name == collectionChild.name).id; + let newCollection = Zotero.Collections.get(newCollectionID); + assert.exists(newCollection); + + // Right click on the selected collection + await zp.collectionsView.selectByID("C" + collectionChild.id); + zp.buildCopyCollectionMenu({}); + + // Delay for menus to get disabled + await Zotero.Promise.delay(); + let groupMenu = doc.querySelector(`#zotero-copy-collection-popup menu[value="L${groupDestination.libraryID}"]`); + // Menu of the library with linked collection should be disabled + assert.equal(groupMenu.disabled, true); + + // Right click on the parent of the copies collection + await zp.collectionsView.selectByID("C" + collection.id); + zp.buildCopyCollectionMenu({}); + // Delay for menus to get disabled + await Zotero.Promise.delay(); + groupMenu = doc.querySelector(`#zotero-copy-collection-popup menu[value="L${groupDestination.libraryID}"]`); + // Menu of the library with linked sub-collection should be disabled + assert.equal(groupMenu.disabled, true); + }); + }); + describe("#moveCollection", function () { + it("should move collection into another collection of the same library", async function () { + let collection = await createDataObject('collection'); + let collectionDestination = await createDataObject('collection'); + + await zp.collectionsView.selectByID("C" + collection.id); + + let promise = waitForNotifierEvent("modify", "collection"); + await zp.moveCollection(collectionDestination); + await promise; + + // Collection was moved into destination collection + let collectionChildIDs = collectionDestination.getDescendents(false, 'collection').map(col => col.id); + assert.sameMembers(collectionChildIDs, [collection.id]); + }); + it("should make collection a top-level collection", async function () { + let collectionParent = await createDataObject('collection'); + let collectionChild = await createDataObject('collection', { parentID: collectionParent.id }); + let library = Zotero.Libraries.get(collectionChild.libraryID); + + await zp.collectionsView.selectByID("C" + collectionChild.id); + + let promise = waitForNotifierEvent("modify", "collection"); + await zp.moveCollection(library); + await promise; + + // Child collection was pulled from under its parent to become a top-level collection + let topLevelCollections = Zotero.Collections.getByLibrary(library.id); + assert.includeMembers(topLevelCollections, [collectionChild]); + }); + }); })