From 9a765dd0b99bbd95511d23c1765a5a32eb1d3bfd Mon Sep 17 00:00:00 2001 From: Bogdan Abaev Date: Mon, 9 Dec 2024 10:42:13 -0800 Subject: [PATCH 1/3] unify collection canDropCheck with context menu - move collectionTree canDropCheck and canDropCheckAsync to Zotero.Collection::canMoveToTarget and Zotero.Collection::canMoveToTargetAsync per https://github.com/zotero/zotero/pull/4420#issuecomment-2406579899 That way, we share the logic to determine if collection row can be dropped onto another row, as well as to disable/enable context menus. - allow to copy a collection to another library if there already is a linked collection but it is in trash. Fixes: #4862 --- chrome/content/zotero/collectionTree.jsx | 44 +------------ .../content/zotero/xpcom/data/collection.js | 66 +++++++++++++++++++ chrome/content/zotero/zoteroPane.js | 57 +++++----------- test/tests/zoteroPaneTest.js | 41 +++++++++++- 4 files changed, 124 insertions(+), 84 deletions(-) diff --git a/chrome/content/zotero/collectionTree.jsx b/chrome/content/zotero/collectionTree.jsx index c1d47d7009..524cafa249 100644 --- a/chrome/content/zotero/collectionTree.jsx +++ b/chrome/content/zotero/collectionTree.jsx @@ -1777,27 +1777,9 @@ var CollectionTree = class CollectionTree extends LibraryTree { return true; } else if (dataType == 'zotero/collection') { - if (!treeRow.isLibrary(true) && !treeRow.isCollection()) { - return false; - } - let draggedCollectionID = data[0]; let draggedCollection = Zotero.Collections.get(draggedCollectionID); - - // Dragging within same library - if (treeRow.ref.libraryID == draggedCollection.libraryID) { - // Collections cannot be dropped on themselves - if (draggedCollectionID == treeRow.ref.id) { - return false; - } - - // Nor in their children - if (draggedCollection.hasDescendent('collection', treeRow.ref.id)) { - return false; - } - } - - return true; + return draggedCollection.canMoveToTarget(treeRow.ref, { debug: true }); } } return false; @@ -1871,28 +1853,8 @@ var CollectionTree = class CollectionTree extends LibraryTree { else if (dataType == 'zotero/collection') { let draggedCollectionID = data[0]; let draggedCollection = Zotero.Collections.get(draggedCollectionID); - - // Dragging a collection to a different library - if (treeRow.ref.libraryID != draggedCollection.libraryID) { - // Disallow if linked collection already exists - if (await draggedCollection.getLinkedCollection(treeRow.ref.libraryID, true)) { - Zotero.debug("Linked collection already exists in library"); - return false; - } - - let descendents = draggedCollection.getDescendents(false, 'collection'); - for (let descendent of descendents) { - descendent = Zotero.Collections.get(descendent.id); - // Disallow if linked collection already exists for any subcollections - // - // If this is allowed in the future for the root collection, - // need to allow drag only to root - if (await descendent.getLinkedCollection(treeRow.ref.libraryID, true)) { - Zotero.debug("Linked subcollection already exists in library"); - return false; - } - } - } + let canMove = await draggedCollection.canMoveToTargetAsync(treeRow.ref, { debug: true }); + return canMove; } } return true; diff --git a/chrome/content/zotero/xpcom/data/collection.js b/chrome/content/zotero/xpcom/data/collection.js index 2c7b4f9f2b..b27314b45e 100644 --- a/chrome/content/zotero/xpcom/data/collection.js +++ b/chrome/content/zotero/xpcom/data/collection.js @@ -987,3 +987,69 @@ Zotero.Collection.prototype._unregisterChildItem = function (itemID) { this._childItems.delete(itemID); } } + +Zotero.Collection.prototype.canMoveToTarget = function (target, options = {}) { + let logMessage = (message) => { + if (options.debug) { + Zotero.debug(message); + } + }; + if (!(target instanceof Zotero.Library || target instanceof Zotero.Collection)) { + logMessage("Can only add collection to another collection or group"); + return false; + } + let targetLibrary = target instanceof Zotero.Library + ? target + : Zotero.Libraries.get(target.libraryID); + if (!targetLibrary.editable) { + logMessage("Not editable target library."); + return false; + } + if (target instanceof Zotero.Collection) { + if (target.id == this.id) { + logMessage("Cannot add collection to itself"); + return false; + } + if (this.hasDescendent('collection', target.id)) { + logMessage("Cannot add collection to its descendent"); + return false; + } + if (this.parentID == target.id) { + logMessage("Cannot add collection to its direct parent"); + return false; + } + } + + return true; +}; + +Zotero.Collection.prototype.canMoveToTargetAsync = async function (target, options = {}) { + let logMessage = (message) => { + if (options.debug) { + Zotero.debug(message); + } + }; + if (!this.canMoveToTarget(target, options)) return false; + + if (target.libraryID !== this.libraryID) { + let linkedCollection = await this.getLinkedCollection(target.libraryID, true); + if (linkedCollection && !linkedCollection.deleted) { + logMessage(`Linked collection already exists in library ${target.libraryID}`); + return false; + } + let descendents = this.getDescendents(false, 'collection'); + for (let descendent of descendents) { + descendent = Zotero.Collections.get(descendent.id); + // Disallow if linked collection already exists for any subcollections + // + // If this is allowed in the future for the root collection, + // need to allow drag only to root + let linkedSubcollection = await descendent.getLinkedCollection(target.libraryID, true); + if (linkedSubcollection && !linkedSubcollection.deleted) { + logMessage("Linked subcollection already exists in library"); + return false; + } + } + } + return true; +}; diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js index 2a3ae4294f..e55d9c0327 100644 --- a/chrome/content/zotero/zoteroPane.js +++ b/chrome/content/zotero/zoteroPane.js @@ -4276,20 +4276,14 @@ var ZoteroPane = new function () { 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); - } + target => !selected.canMoveToTarget(target) ); popup.append(menuItem); } }; - this.buildCopyCollectionMenu = function (event) { + this.buildCopyCollectionMenu = Zotero.Utilities.Internal.serial(async function (event) { if (event.target !== event.currentTarget) return; let popup = document.getElementById("zotero-copy-collection-popup"); popup.replaceChildren(); @@ -4298,33 +4292,6 @@ var ZoteroPane = new function () { // 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 @@ -4359,16 +4326,22 @@ var ZoteroPane = new function () { event.stopPropagation(); } }, - - (target) => { - // can't copy collection into itself or into non-editable groups - return selected == target - || (target instanceof Zotero.Group && !target.editable); - } + target => !selected.canMoveToTarget(target) ); popup.append(menuItem); } - }; + + // Disable libraries to which collection cannot be copied + if (topLevelEntries[0] instanceof Zotero.Library) { + for (let entry of topLevelEntries) { + let canCopy = await selected.canMoveToTargetAsync(entry); + if (!canCopy) { + let menuitem = popup.querySelector(`[value="L${entry.libraryID}"]`); + menuitem.disabled = true; + } + } + } + }); this.buildAddItemToCollectionMenu = function (event, items = this.getSelectedItems()) { if (event.target !== event.currentTarget) return; diff --git a/test/tests/zoteroPaneTest.js b/test/tests/zoteroPaneTest.js index 8a0a6070f1..d33cc03520 100644 --- a/test/tests/zoteroPaneTest.js +++ b/test/tests/zoteroPaneTest.js @@ -1825,7 +1825,7 @@ describe("ZoteroPane", function () { await waitForNotifierEvent("add", "collection"); - // Collection has been copies + // Collection has been copied let groupCollections = groupCollection.getDescendents(false, 'collection'); let newCollectionID = groupCollections.find(col => col.name == collectionChild.name).id; let newCollection = Zotero.Collections.get(newCollectionID); @@ -1873,6 +1873,45 @@ describe("ZoteroPane", function () { // Copied collection is a top-level collection assert.notOk(newCollection.parentID); }); + + it("should allow copying between libraries if there is a linked collection but it is in trash", 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 copied + let newCollection = zp.getSelectedCollection(); + assert.equal(newCollection.libraryID, groupCollection.libraryID); + assert.equal(newCollection.name, collectionChild.name); + + // Send new collection to trash + newCollection.deleted = true; + await newCollection.saveTx(); + + // Right click on the selected collection + await zp.collectionsView.selectByID("C" + collectionChild.id); + await zp.buildCopyCollectionMenu({}); + 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 enabled + assert.equal(groupMenu.disabled, false); + + // Copy collection again + await zp.copyCollection(groupCollection); + await waitForNotifierEvent("add", "collection"); + let anotherNewCollection = zp.getSelectedCollection(); + // Newly created collection is in the group + assert.equal(anotherNewCollection.libraryID, groupCollection.libraryID); + }); }); describe("#moveCollection", function () { it("should move collection into another collection of the same library", async function () { From db71a75b131d6f5044ed297a6380fd31bfa12d49 Mon Sep 17 00:00:00 2001 From: Bogdan Abaev Date: Thu, 19 Mar 2026 15:04:55 -0700 Subject: [PATCH 2/3] select collection from long list via dialog If the user has too many collections to comfortably navigate via context menu, do not render the collection hierarchy via menus, and instead have the user select the collection via dialog that has a full collectionTree. Two hidden prefs determine when the dialog will appear: - collectionsMenu.maxEntriesTotal (default 5000) - max collections in specified libraries - collectionsMenu.maxEntriesAtOneLevel (default 100) - max collections on any single level (e.g max top level collections) If there are more collections than maxEntriesTotal or one collection has more than maxEntriesAtOneLevel children, the dialog will be used. It also takes care of some performance issues caused by trying to render too many collections. Fixes: #5693 --- chrome/content/zotero/collectionTree.jsx | 66 +++++--- chrome/content/zotero/newCollectionDialog.js | 37 ++++- .../content/zotero/newCollectionDialog.xhtml | 5 +- .../content/zotero/selectCollectionDialog.js | 120 ++++++++++++++ .../zotero/selectCollectionDialog.xhtml | 77 +++++++++ .../content/zotero/xpcom/data/collection.js | 13 ++ .../content/zotero/xpcom/data/collections.js | 35 ++++ chrome/content/zotero/zoteroPane.js | 154 +++++++++++++----- chrome/content/zotero/zoteroPane.xhtml | 3 + chrome/locale/en-US/zotero/zotero.ftl | 6 + defaults/preferences/zotero.js | 2 + scss/_zotero.scss | 1 + scss/components/_menu.scss | 1 + scss/components/_newCollectionDialog.scss | 20 +++ scss/components/_selectCollectionDialog.scss | 48 ++++++ test/tests/zoteroPaneTest.js | 145 +++++++++++++++++ 16 files changed, 673 insertions(+), 60 deletions(-) create mode 100644 chrome/content/zotero/selectCollectionDialog.js create mode 100644 chrome/content/zotero/selectCollectionDialog.xhtml create mode 100644 scss/components/_selectCollectionDialog.scss diff --git a/chrome/content/zotero/collectionTree.jsx b/chrome/content/zotero/collectionTree.jsx index 524cafa249..aefd3b1215 100644 --- a/chrome/content/zotero/collectionTree.jsx +++ b/chrome/content/zotero/collectionTree.jsx @@ -1851,6 +1851,8 @@ var CollectionTree = class CollectionTree extends LibraryTree { } } else if (dataType == 'zotero/collection') { + // Can drop collections into trash to delete it + if (treeRow.isTrash()) return true; let draggedCollectionID = data[0]; let draggedCollection = Zotero.Collections.get(draggedCollectionID); let canMove = await draggedCollection.canMoveToTargetAsync(treeRow.ref, { debug: true }); @@ -2687,6 +2689,36 @@ var CollectionTree = class CollectionTree extends LibraryTree { } } + clearCollectionSearch(searchFieldSelector) { + let collectionsSearchField = document.querySelector(searchFieldSelector); + // If empty filter - just focus the collectionTree + if (collectionsSearchField.value.length == 0) { + return document.getElementById("collection-tree"); + } + // Clear the search field and focus collection tree + if (collectionsSearchField.value.length) { + collectionsSearchField.value = ''; + this.setFilter("", true); + } + return null; + } + + focusCollectionTree(searchFieldSelector) { + let collectionsSearchField = document.querySelector(searchFieldSelector); + // Prevent Enter/Tab pressed before the filtering ran from doing anything + if (!this.filterEquals(collectionsSearchField.value)) { + return null; + } + // If the current row passes the filter, make sure it is visible and focus collectionTree + if (this.focusedRowMatchesFilter()) { + this.ensureRowIsVisible(this.selection.focused); + return document.getElementById('collection-tree'); + } + // Otherwise, focus the first row passing the filter + this.focusFirstMatchingRow(false); + return null; + } + /** * Creates an extra hidden row to keep focus on it when a currently focused row does not match the filter. @@ -2724,17 +2756,6 @@ var CollectionTree = class CollectionTree extends LibraryTree { return this._filter === ""; } - clearFilter() { - // Clear the search field - if (collectionsSearchField.value.length) { - collectionsSearchField.value = ''; - ZoteroPane.handleCollectionSearchInput(); - return null; - } - // If the search field is empty, focus the collection tree - return document.getElementById('collection-tree'); - } - /** * Select and focus the first row matching the collection filter. If it is a child of a collapsed * container(s), the container(s) on the way will be toggled open. @@ -2940,7 +2961,8 @@ var CollectionTree = class CollectionTree extends LibraryTree { && this._virtualCollectionLibraries.duplicates[libraryID] !== false; var showUnfiled = this.props.hideSources.indexOf('unfiled') == -1 && this._virtualCollectionLibraries.unfiled?.[libraryID] !== false; - var showRecentlyRead = this._virtualCollectionLibraries.recentlyRead?.[libraryID] !== false; + var showRecentlyRead = this.props.hideSources.indexOf('recentlyRead') == -1 + && this._virtualCollectionLibraries.recentlyRead?.[libraryID] !== false; var showRetracted = this.props.hideSources.indexOf('retracted') == -1 && this._virtualCollectionLibraries.retracted?.[libraryID] !== false && Zotero.Retractions.libraryHasRetractedItems(libraryID); @@ -2948,6 +2970,7 @@ var CollectionTree = class CollectionTree extends LibraryTree { && this._virtualCollectionLibraries.publications?.[libraryID] !== false && libraryID == Zotero.Libraries.userLibraryID; var showTrash = this.props.hideSources.indexOf('trash') == -1; + var showSavedSearches = this.props.hideSources.indexOf('searches') == -1; } else { var savedSearches = []; @@ -2957,6 +2980,7 @@ var CollectionTree = class CollectionTree extends LibraryTree { var showRetracted = false; var showPublications = false; var showTrash = false; + var showSavedSearches = false; } // If not a manual open and either the library is set to be collapsed or this is a collection that isn't explicitly opened, @@ -3013,14 +3037,16 @@ var CollectionTree = class CollectionTree extends LibraryTree { } // Add searches - for (var i = 0, len = savedSearches.length; i < len; i++) { - // Skip searches in trash - if (savedSearches[i].deleted) continue; - // Skip searches not matching the filter - if (!this._includedInTree(savedSearches[i])) continue; - rows.splice(row + 1 + newRows, 0, - new Zotero.CollectionTreeRow(this, 'search', savedSearches[i], level + 1)); - newRows++; + if (showSavedSearches) { + for (let i = 0; i < savedSearches.length; i++) { + // Skip searches in trash + if (savedSearches[i].deleted) continue; + // Skip searches not matching the filter + if (!this._includedInTree(savedSearches[i])) continue; + rows.splice(row + 1 + newRows, 0, + new Zotero.CollectionTreeRow(this, 'search', savedSearches[i], level + 1)); + newRows++; + } } if (showPublications && this._isFilterEmpty()) { diff --git a/chrome/content/zotero/newCollectionDialog.js b/chrome/content/zotero/newCollectionDialog.js index d956642b82..1887269a00 100644 --- a/chrome/content/zotero/newCollectionDialog.js +++ b/chrome/content/zotero/newCollectionDialog.js @@ -23,6 +23,8 @@ ***** END LICENSE BLOCK ***** */ +import CollectionTree from 'zotero/collectionTree'; + var Zotero_New_Collection_Dialog = { _handleLoad() { let io = window.arguments[0]; @@ -34,7 +36,20 @@ var Zotero_New_Collection_Dialog = { this._libraryID = io.libraryID; this._parentCollectionID = io.parentCollectionID; - this._updateMenu(); + + // depending on how many collections there are, display the dropdown + // or the entire collection tree + if (Zotero.Collections.canListCollectionsInMenu(this._libraryID)) { + document.getElementById("create-in").hidden = false; + this._updateMenu(); + } + else { + document.getElementById("zotero-collections-tree-container").hidden = false; + setTimeout(() => { + window.moveToAlertPosition(); + this._initCollectionTree(); + }); + } }, _handleAccept() { @@ -45,6 +60,26 @@ var Zotero_New_Collection_Dialog = { }; }, + async _initCollectionTree() { + this._collectionsView = await CollectionTree.init(document.getElementById('zotero-collections-tree'), { + onSelectionChange: () => { + let selectedCollection = this._collectionsView.getSelectedCollection(); + this._parentCollectionID = selectedCollection ? selectedCollection.id : null; + }, + filterLibraryIDs: [this._libraryID], + hideSources: ['duplicates', 'trash', 'feeds', 'unfiled', 'retracted', 'publications', 'searches', 'recentlyRead'] + }); + + await this._collectionsView.makeVisible(); + + if (this._parentCollectionID) { + await this._collectionsView.selectByID('C' + this._parentCollectionID); + } + else { + await this._collectionsView.selectByID('L' + this._libraryID); + } + }, + _updateMenu() { let createInField = document.querySelector('#create-in'); let menupopup = createInField.firstElementChild; diff --git a/chrome/content/zotero/newCollectionDialog.xhtml b/chrome/content/zotero/newCollectionDialog.xhtml index 5d62da35a3..a38358cae8 100644 --- a/chrome/content/zotero/newCollectionDialog.xhtml +++ b/chrome/content/zotero/newCollectionDialog.xhtml @@ -65,9 +65,12 @@ diff --git a/chrome/content/zotero/selectCollectionDialog.js b/chrome/content/zotero/selectCollectionDialog.js new file mode 100644 index 0000000000..b2c52f2919 --- /dev/null +++ b/chrome/content/zotero/selectCollectionDialog.js @@ -0,0 +1,120 @@ +/* + ***** BEGIN LICENSE BLOCK ***** + + Copyright © 2009 Center for History and New Media + George Mason University, Fairfax, Virginia, USA + http://zotero.org + + This file is part of Zotero. + + Zotero is free software: you can redistribute it and/or modify + it under the terms of the GNU Affero General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + Zotero is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU Affero General Public License for more details. + + You should have received a copy of the GNU Affero General Public License + along with Zotero. If not, see . + + ***** END LICENSE BLOCK ***** +*/ + +import CollectionTree from 'zotero/collectionTree'; + +var collectionsView; +var loaded; +var io; + +var doLoad = async function () { + // Set font size from pref + var sbc = document.getElementById('zotero-select-collection-container'); + Zotero.UIProperties.registerRoot(sbc); + + io = window.arguments[0]; + if (io.wrappedJSObject) io = io.wrappedJSObject; + + let filterLibraryIDs = io.filterLibraryIDs || false; + collectionsView = await CollectionTree.init(document.getElementById('zotero-collections-tree'), { + onSelectionChange: async () => { + let selectedRow = collectionsView.getRow(collectionsView.selection.focused); + let canSelect = await io.canSelect(selectedRow.ref); + document.getElementById("select-collection-dialog").getButton("accept").disabled = !canSelect; + }, + filterLibraryIDs, + hideSources: ['duplicates', 'trash', 'feeds', 'unfiled', 'retracted', 'publications', 'searches', 'recentlyRead'], + onActivate: () => { + document.querySelector('dialog').acceptDialog(); + } + }); + + await collectionsView.makeVisible(); + + if (io.currentCollectionID) { + await collectionsView.selectByID('C' + io.currentCollectionID); + } + + document.addEventListener('dialogaccept', doAccept); + + // Setup keyboard navigation + let collectionTree = document.getElementById('collection-tree'); + let searchField = document.getElementById('zotero-collections-search'); + + searchField.addEventListener('keydown', (event) => { + if (event.key === 'Escape') { + _handleCollectionFilterEscape(event); + } + else if (event.key === 'Enter' || (event.key === 'Tab' && !event.shiftKey)) { + _handleCollectionFilterEnter(event); + } + }); + + collectionTree.addEventListener('keydown', (event) => { + if (event.key === 'Escape') { + _handleCollectionFilterEscape(event); + } + }); + + // Used in tests + loaded = true; +}; + +function doUnload() +{ + collectionsView.unregister(); + + io.deferred && io.deferred.resolve(); +} + +function handleCollectionSearchInput() { + let collectionsSearchField = document.getElementById("zotero-collections-search"); + collectionsView.setFilter(collectionsSearchField.value); +} + +function doAccept() { + let selected = collectionsView.getRow(collectionsView.selection.focused); + io.dataOut = selected.ref; +} + +function _handleCollectionFilterEnter(event) { + let result = collectionsView.focusCollectionTree("#zotero-collections-search"); + if (result) { + result.focus(); + event.preventDefault(); + event.stopPropagation(); + } +} + +function _handleCollectionFilterEscape(event) { + if (!document.getElementById("zotero-collections-search").value) return; + + let result = collectionsView.clearCollectionSearch("#zotero-collections-search"); + if (result) { + result.focus(); + } + event.preventDefault(); + event.stopPropagation(); +} diff --git a/chrome/content/zotero/selectCollectionDialog.xhtml b/chrome/content/zotero/selectCollectionDialog.xhtml new file mode 100644 index 0000000000..2421c35fb3 --- /dev/null +++ b/chrome/content/zotero/selectCollectionDialog.xhtml @@ -0,0 +1,77 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/chrome/content/zotero/xpcom/data/collection.js b/chrome/content/zotero/xpcom/data/collection.js index b27314b45e..a416e6edd9 100644 --- a/chrome/content/zotero/xpcom/data/collection.js +++ b/chrome/content/zotero/xpcom/data/collection.js @@ -1053,3 +1053,16 @@ Zotero.Collection.prototype.canMoveToTargetAsync = async function (target, optio } return true; }; + +// Determine which libraries the collection can be copied to +Zotero.Collection.prototype.getLibrariesForCopying = async function () { + let libraries = Zotero.Libraries.getAll().filter(lib => !(lib instanceof Zotero.Feed)); + let librariesWhereOneCanCopy = []; + for (let library of libraries) { + let canCopy = await this.canMoveToTargetAsync(library); + if (canCopy) { + librariesWhereOneCanCopy.push(library); + } + } + return librariesWhereOneCanCopy; +}; diff --git a/chrome/content/zotero/xpcom/data/collections.js b/chrome/content/zotero/xpcom/data/collections.js index e38e29527d..6e679f3e07 100644 --- a/chrome/content/zotero/xpcom/data/collections.js +++ b/chrome/content/zotero/xpcom/data/collections.js @@ -85,6 +85,41 @@ Zotero.Collections = function () { return _getByContainer(null, parentCollectionID, recursive, includeTrashed); } + + /** + * Determine if the user has too many collections in a library to list them all in a menu. + * @param {Integer[]} libraryIDs - ids of libraries whose collections will be counted. + * If not provided, will check all libraries. + * @returns {Boolean} true if the number of collections exceeds the limits set in preferences. + */ + this.canListCollectionsInMenu = function (libraryIDs) { + if (!Array.isArray(libraryIDs)) { + libraryIDs = [libraryIDs]; + } + let maxEntriesTotal = Zotero.Prefs.get("collectionsMenu.maxEntriesTotal"); + let maxEntriesAtOneLevel = Zotero.Prefs.get("collectionsMenu.maxEntriesAtOneLevel"); + let totalCollectionsCount = 0; + for (let libraryID of libraryIDs) { + let allCollections = Zotero.Collections.getByLibrary(libraryID, true, false); + totalCollectionsCount += allCollections.length; + // Too many top-level collections in this library + let topLevelCollections = Zotero.Collections.getByLibrary(libraryID); + if (topLevelCollections.length > maxEntriesAtOneLevel) { + return false; + } + // Any single collection has too many children + for (let collection of allCollections) { + if (collection.getChildCollections().length > maxEntriesAtOneLevel) { + return false; + } + } + } + // Too many total collections across all libraries + if (totalCollectionsCount > maxEntriesTotal) { + return false; + } + return true; + }; var _getByContainer = function (libraryID, parentID, recursive, includeTrashed) { let children = []; diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js index e55d9c0327..cf1f972a7b 100644 --- a/chrome/content/zotero/zoteroPane.js +++ b/chrome/content/zotero/zoteroPane.js @@ -313,33 +313,15 @@ var ZoteroPane = new function () { moveFocus(actionsMap, event, true); }); - let collectionsSearchField = document.getElementById("zotero-collections-search"); let clearCollectionSearch = () => { - // If empty filter - just focus the collectionTree - if (collectionsSearchField.value.length == 0) { - return document.getElementById("collection-tree"); + let result = ZoteroPane.collectionsView.clearCollectionSearch("#zotero-collections-search"); + if (result === null) { + ZoteroPane.hideCollectionSearch(); } - // Clear the search field and focus collection tree - if (collectionsSearchField.value.length) { - collectionsSearchField.value = ''; - ZoteroPane.collectionsView.setFilter("", true); - } - ZoteroPane.hideCollectionSearch(); - return null; + return result; }; let focusCollectionTree = () => { - // Prevent Enter/Tab pressed before the filtering ran from doing anything - if (!ZoteroPane.collectionsView.filterEquals(collectionsSearchField.value)) { - return null; - } - // If the current row passes the filter, make sure it is visible and focus collectionTree - if (ZoteroPane.collectionsView.focusedRowMatchesFilter()) { - ZoteroPane.collectionsView.ensureRowIsVisible(ZoteroPane.collectionsView.selection.focused); - return document.getElementById('collection-tree'); - } - // Otherwise, focus the first row passing the filter - ZoteroPane.collectionsView.focusFirstMatchingRow(false); - return null; + return ZoteroPane.collectionsView.focusCollectionTree("#zotero-collections-search"); }; collectionTreeToolbar.addEventListener("keydown", (event) => { let actionsMap = { @@ -2664,6 +2646,26 @@ var ZoteroPane = new function () { await selected.saveTx(); }; + this.moveCollectionViaDialog = async function () { + let selected = this.getSelectedCollection(); + if (!selected) return; + + let io = { + dataIn: null, + dataOut: null, + filterLibraryIDs: [selected.libraryID], + currentCollectionID: selected.id, + canSelect: async (target) => { + return selected.canMoveToTargetAsync(target); + } + }; + window.openDialog('chrome://zotero/content/selectCollectionDialog.xhtml', '', + 'chrome,modal,centerscreen,resizable=no', io); + + if (!io.dataOut) return; + await this.moveCollection(io.dataOut); + }; + // Copy selected collection into another collection or library. // Partially, a replication of drag-drop mechanism from CollectionTree.onDrop. this.copyCollection = async (target) => { @@ -2694,6 +2696,28 @@ var ZoteroPane = new function () { }); }; + this.copyCollectionViaDialog = async function () { + let selected = this.getSelectedCollection(); + if (!selected) return; + + let librariesWhereOneCanCopy = await selected.getLibrariesForCopying(); + + let io = { + dataIn: null, + dataOut: null, + filterLibraryIDs: librariesWhereOneCanCopy.map(lib => lib.libraryID), + currentCollectionID: selected.id, + canSelect: async (collection) => { + return selected.canMoveToTargetAsync(collection); + } + }; + window.openDialog('chrome://zotero/content/selectCollectionDialog.xhtml', '', + 'chrome,modal,centerscreen,resizable=no', io); + + if (!io.dataOut) return; + await this.copyCollection(io.dataOut); + }; + this.toggleSelectedItemsRead = async function () { await Zotero.FeedItems.toggleReadByID(this.getSelectedItems(true)); }; @@ -3367,9 +3391,17 @@ var ZoteroPane = new function () { { id: "moveCollection", }, + { + id: "moveCollectionDialog", + oncommand: () => this.moveCollectionViaDialog() + }, { id: "copyCollection" }, + { + id: "copyCollectionDialog", + oncommand: () => this.copyCollectionViaDialog() + }, { id: "duplicate", oncommand: () => this.duplicateSelectedCollection() @@ -3488,8 +3520,6 @@ var ZoteroPane = new function () { 'newSubcollection', 'sep2', 'editSelectedCollection', - 'moveCollection', - 'copyCollection', 'deleteCollection', 'deleteCollectionAndItems', 'sep3', @@ -3497,6 +3527,16 @@ var ZoteroPane = new function () { 'createBibCollection', 'loadReport' ]; + + // Decide if collections can be listed as menuitems for move/copy menus + // or (if there are too many collections), collections will be selected in the dialog + let librariesWhereOneCanCopy = await collectionTreeRow.ref.getLibrariesForCopying(); + if (Zotero.Collections.canListCollectionsInMenu(librariesWhereOneCanCopy.map(lib => lib.libraryID))) { + show.push('moveCollection', 'copyCollection'); + } + else { + show.push('moveCollectionDialog', 'copyCollectionDialog'); + } if (!this.itemsView.rowCount) { disable = ['createBibCollection', 'loadReport']; @@ -3729,6 +3769,7 @@ var ZoteroPane = new function () { 'toggleRead', 'changeParentItem', 'addToCollection', + 'addToCollectionDialog', 'removeItems', 'duplicateAndConvert', 'duplicateItem', @@ -4140,8 +4181,15 @@ var ZoteroPane = new function () { && collectionTreeRow.editable && Zotero.Items.keepTopLevel(items).every(item => item.isTopLevelItem()) ) { - menu.childNodes[m.addToCollection].setAttribute('label', Zotero.getString('pane.items.menu.addToCollection')); - show.add(m.addToCollection); + // List collections if possible. If there are too many of them, collection + // will be selected in the dialog. + if (Zotero.Collections.canListCollectionsInMenu([items[0].libraryID])) { + menu.childNodes[m.addToCollection].setAttribute('label', Zotero.getString('pane.items.menu.addToCollection')); + show.add(m.addToCollection); + } + else { + show.add(m.addToCollectionDialog); + } } // Remove from collection @@ -4316,6 +4364,20 @@ var ZoteroPane = new function () { // Build menus for all libraries (or collections) for (let obj of topLevelEntries) { + // For libraries that cannot be copied to, just add a disabled stub + if (obj instanceof Zotero.Library) { + let canCopy = await selected.canMoveToTargetAsync(obj); + if (!canCopy) { + let menuItem = document.createXULElement("menu"); + menuItem.setAttribute("label", obj.name); + menuItem.setAttribute("image", obj.treeViewImage); + menuItem.setAttribute("value", obj.treeViewID); + menuItem.classList.add('menu-iconic'); + menuItem.disabled = true; + popup.append(menuItem); + continue; + } + } let menuItem = Zotero.Utilities.Internal.createMenuForTarget( obj, popup, @@ -4330,17 +4392,6 @@ var ZoteroPane = new function () { ); popup.append(menuItem); } - - // Disable libraries to which collection cannot be copied - if (topLevelEntries[0] instanceof Zotero.Library) { - for (let entry of topLevelEntries) { - let canCopy = await selected.canMoveToTargetAsync(entry); - if (!canCopy) { - let menuitem = popup.querySelector(`[value="L${entry.libraryID}"]`); - menuitem.disabled = true; - } - } - } }); this.buildAddItemToCollectionMenu = function (event, items = this.getSelectedItems()) { @@ -4387,6 +4438,33 @@ var ZoteroPane = new function () { separator.hidden = !collections.length; }; + this.addToCollectionViaDialog = async function (event, items = this.getSelectedItems()) { + items = Zotero.Items.keepTopLevel(items); + if (!items.length) return; + + let libraryID = items[0].libraryID; + if (items.some(item => item.libraryID !== libraryID)) return; + + let selectedCollection = ZoteroPane.collectionsView.getSelectedCollection(); + let io = { + dataIn: null, + dataOut: null, + filterLibraryIDs: [libraryID], + currentCollectionID: selectedCollection?.id, + canSelect: (collection) => { + if (!(collection instanceof Zotero.Collection)) return false; + if (selectedCollection && collection.id === selectedCollection.id) return false; + if (items.every(item => collection.hasItem(item))) return false; + return true; + } + }; + window.openDialog('chrome://zotero/content/selectCollectionDialog.xhtml', '', + 'chrome,modal,centerscreen,resizable=no', io); + + if (!io.dataOut) return; + await this.addItemsToCollection(items, io.dataOut); + }; + this.addItemsToCollection = async function (items, collection, createNew = false) { items = Zotero.Items.keepTopLevel(items); diff --git a/chrome/content/zotero/zoteroPane.xhtml b/chrome/content/zotero/zoteroPane.xhtml index e345f06214..3effe3e02e 100644 --- a/chrome/content/zotero/zoteroPane.xhtml +++ b/chrome/content/zotero/zoteroPane.xhtml @@ -947,10 +947,12 @@ + + @@ -1008,6 +1010,7 @@ + diff --git a/chrome/locale/en-US/zotero/zotero.ftl b/chrome/locale/en-US/zotero/zotero.ftl index 9b0e3535d6..76e1e5dee6 100644 --- a/chrome/locale/en-US/zotero/zotero.ftl +++ b/chrome/locale/en-US/zotero/zotero.ftl @@ -189,6 +189,10 @@ collections-menu-move-collection = .label = Move To collections-menu-copy-collection = .label = Copy To +collections-menu-move-collection-dialog = + .label = Move To… +collections-menu-copy-collection-dialog = + .label = Copy To… item-creator-moveDown = .label = Move Down @@ -230,6 +234,8 @@ item-menu-change-parent-item = .label = Change Parent Item… item-menu-relate-items = .label = Relate Items +item-menu-add-to-collection-dialog = + .label = Add to Collection… view-online = View Online item-menu-option-view-online = diff --git a/defaults/preferences/zotero.js b/defaults/preferences/zotero.js index 986c569b81..551781debe 100644 --- a/defaults/preferences/zotero.js +++ b/defaults/preferences/zotero.js @@ -32,6 +32,8 @@ pref("extensions.zotero.hideContextAnnotationRows", true); pref("extensions.zotero.fontSize", "1.00"); pref("extensions.zotero.layout", "standard"); pref("extensions.zotero.recursiveCollections", false); +pref("extensions.zotero.collectionsMenu.maxEntriesTotal", 5000); +pref("extensions.zotero.collectionsMenu.maxEntriesAtOneLevel", 100); pref("extensions.zotero.autoRecognizeFiles", true); pref("extensions.zotero.autoRenameFiles", true); pref("extensions.zotero.autoRenameFiles.linked", false); diff --git a/scss/_zotero.scss b/scss/_zotero.scss index de4f1af786..5db27887be 100644 --- a/scss/_zotero.scss +++ b/scss/_zotero.scss @@ -60,6 +60,7 @@ @import "components/search"; @import "components/select"; @import "components/selectItemsDialog"; +@import "components/selectCollectionDialog"; @import "components/spinner"; @import "components/syncButtonTooltip"; @import "components/tabBar"; diff --git a/scss/components/_menu.scss b/scss/components/_menu.scss index 6d50a7acbe..76b7df9fbb 100644 --- a/scss/components/_menu.scss +++ b/scss/components/_menu.scss @@ -58,6 +58,7 @@ $menu-icons: ( rename-from-parent: "rename", create-note-from-annotations: "light-dark:note-annotation", add-to-collection: "new-collection", + add-to-collection-dialog: "new-collection", new-tab: "new-tab", new-window: "new-window", pin: "pin", diff --git a/scss/components/_newCollectionDialog.scss b/scss/components/_newCollectionDialog.scss index 9e251b1395..d898ad0f73 100644 --- a/scss/components/_newCollectionDialog.scss +++ b/scss/components/_newCollectionDialog.scss @@ -6,4 +6,24 @@ menulist::part(icon) { margin-inline-end: 3px; } + + [hidden="true"] { + display: none !important; + } + + #zotero-collections-tree-container { + min-width: 300px; + min-height: 300px; + max-height: 400px; + margin-top: 8px; + + .virtualized-table-body { + padding: 8px; + } + } + + #zotero-collections-tree { + border-radius: 5px; + background: var(--material-sidepane); + } } diff --git a/scss/components/_selectCollectionDialog.scss b/scss/components/_selectCollectionDialog.scss new file mode 100644 index 0000000000..e71853b741 --- /dev/null +++ b/scss/components/_selectCollectionDialog.scss @@ -0,0 +1,48 @@ +#zotero-select-collection-dialog { + display: flex; + min-width: 400px; + min-height: 450px; +} + +#select-collection-dialog { + appearance: none; + background: var(--material-background); + display: flex; + flex-direction: column; + -moz-window-dragging: drag; + height: 100%; + + #zotero-select-collection-container { + display: flex; + flex-direction: column; + flex: 1; + min-height: 100%; + -moz-window-dragging: no-drag; + margin: -15px -15px 0 -15px; + + #search-toolbar { + margin: 10px; + @media (-moz-platform: windows) { + margin: 20px 15px 10px 15px; + } + #zotero-collections-search { + max-width: 100%; + width: 100%; + } + } + } + + #zotero-collections-tree-container { + min-width: 400px; + flex: 1; + background: var(--material-sidepane); + + .virtualized-table-body { + padding: 16px 8px; + } + } + + #zotero-collections-tree { + background: var(--material-sidepane); + } +} diff --git a/test/tests/zoteroPaneTest.js b/test/tests/zoteroPaneTest.js index d33cc03520..0fdbb92efa 100644 --- a/test/tests/zoteroPaneTest.js +++ b/test/tests/zoteroPaneTest.js @@ -1110,6 +1110,76 @@ describe("ZoteroPane", function () { var restoreMenuItem = menu.querySelector('.zotero-menuitem-restore-to-library'); assert.isTrue(restoreMenuItem.disabled); }); + + it("should list collections when not too many", async function () { + Zotero.Prefs.set('collectionsMenu.maxEntriesTotal', 1000); + + let collection = await createDataObject('collection'); + let item = await createDataObject('item'); + + await zp.selectItems([item.id]); + await zp.buildItemContextMenu(); + + let menu = win.document.getElementById('zotero-itemmenu'); + menu.openPopup(); + // The menu (not menuitem) variant should be shown + let addToCollectionMenu = menu.querySelector('.zotero-menuitem-add-to-collection'); + assert.isFalse(addToCollectionMenu.hidden); + // The dialog variant should be hidden + let addToCollectionDialog = menu.querySelector('.zotero-menuitem-add-to-collection-dialog'); + assert.isTrue(addToCollectionDialog.hidden); + + // Open the menupopup so it gets populated + let popup = addToCollectionMenu.querySelector('menupopup'); + popup.openPopup(); + + // Find the menuitem for our collection and click it + let collectionMenuItem = popup.querySelector(`[value="C${collection.id}"]`); + assert.ok(collectionMenuItem); + collectionMenuItem.doCommand(); + popup.hidePopup(); + await waitForNotifierEvent("modify", "item"); + + // Verify item was added to the collection + assert.isTrue(collection.hasItem(item.id)); + }); + + it("should not list collections if too many", async function () { + let collection = await createDataObject('collection'); + let item = await createDataObject('item'); + + Zotero.Prefs.set('collectionsMenu.maxEntriesTotal', 0); + + await zp.selectItems([item.id]); + await zp.buildItemContextMenu(); + + let menu = win.document.getElementById('zotero-itemmenu'); + // The menu variant should be hidden + let addToCollectionMenu = menu.querySelector('.zotero-menuitem-add-to-collection'); + assert.isTrue(addToCollectionMenu.hidden); + // The dialog menuitem should be shown + let addToCollectionDialog = menu.querySelector('.zotero-menuitem-add-to-collection-dialog'); + assert.isFalse(addToCollectionDialog.hidden); + + // Set up listener for the dialog before triggering it + waitForWindow('chrome://zotero/content/selectCollectionDialog.xhtml', async (selectWin) => { + // Wait for the dialog to finish loading + do { + await Zotero.Promise.delay(50); + } + while (!selectWin.loaded); + await selectWin.collectionsView.makeVisible(); + // Select the target collection + await selectWin.collectionsView.selectByID('C' + collection.id); + selectWin.document.querySelector('dialog').acceptDialog(); + }); + await zp.addToCollectionViaDialog(); + + // Verify item was added to the collection + assert.isTrue(collection.hasItem(item.id)); + + Zotero.Prefs.set('collectionsMenu.maxEntriesTotal', 1000); + }); }); describe("#restoreSelectedItems()", function () { @@ -1913,6 +1983,81 @@ describe("ZoteroPane", function () { assert.equal(anotherNewCollection.libraryID, groupCollection.libraryID); }); }); + describe("#buildCollectionContextMenu()", function () { + it("should list collections for copy when not too many", async function () { + Zotero.Prefs.set('collectionsMenu.maxEntriesTotal', 1000); + + let collectionA = await createDataObject('collection'); + let collectionB = await createDataObject('collection'); + + await select(win, collectionA); + await zp.buildCollectionContextMenu(); + + let menu = doc.getElementById('zotero-collectionmenu'); + menu.openPopup(); + + // The menu (not menuitem) variant for copy should be shown + let copyCollectionMenu = menu.querySelector('#copyCollection'); + assert.isFalse(copyCollectionMenu.hidden); + // The dialog variant should be hidden + let copyCollectionDialog = menu.querySelector('#copyCollectionDialog'); + assert.isTrue(copyCollectionDialog.hidden); + + // Open the menupopup so it gets populated + let popup = doc.getElementById('zotero-copy-collection-popup'); + popup.openPopup(); + + // Find the menuitem for collection B and click it + let collectionMenuItem = popup.querySelector(`[value="C${collectionB.id}"]`); + assert.ok(collectionMenuItem); + collectionMenuItem.doCommand(); + popup.hidePopup(); + await waitForNotifierEvent("add", "collection"); + + // Verify collection A was copied into collection B + let childNames = collectionB.getDescendents(false, 'collection').map(col => col.name); + assert.include(childNames, collectionA.name); + }); + + it("should not list collections for copy if too many", async function () { + let collectionA = await createDataObject('collection'); + let collectionB = await createDataObject('collection'); + + Zotero.Prefs.set('collectionsMenu.maxEntriesTotal', 0); + + await select(win, collectionA); + await zp.buildCollectionContextMenu(); + + let menu = doc.getElementById('zotero-collectionmenu'); + + // The menu variant for copy should be hidden + let copyCollectionMenu = menu.querySelector('#copyCollection'); + assert.isTrue(copyCollectionMenu.hidden); + // The dialog menuitem should be shown + let copyCollectionDialog = menu.querySelector('#copyCollectionDialog'); + assert.isFalse(copyCollectionDialog.hidden); + + // Set up listener for the dialog before triggering it + waitForWindow('chrome://zotero/content/selectCollectionDialog.xhtml', async (selectWin) => { + do { + await Zotero.Promise.delay(50); + } + while (!selectWin.loaded); + await selectWin.collectionsView.makeVisible(); + // Select the target collection + await selectWin.collectionsView.selectByID('C' + collectionB.id); + selectWin.document.querySelector('dialog').acceptDialog(); + }); + await zp.copyCollectionViaDialog(); + await waitForNotifierEvent("add", "collection"); + + // Verify collection A was copied into collection B + let childNames = collectionB.getDescendents(false, 'collection').map(col => col.name); + assert.include(childNames, collectionA.name); + + Zotero.Prefs.set('collectionsMenu.maxEntriesTotal', 1000); + }); + }); describe("#moveCollection", function () { it("should move collection into another collection of the same library", async function () { let collection = await createDataObject('collection'); From 9501f7c8f123f16cce30d7e97003175a3ee95b18 Mon Sep 17 00:00:00 2001 From: Bogdan Abaev Date: Thu, 19 Mar 2026 18:12:41 -0700 Subject: [PATCH 3/3] test fix --- test/tests/zoteroPaneTest.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/tests/zoteroPaneTest.js b/test/tests/zoteroPaneTest.js index 0fdbb92efa..ccc4ef0323 100644 --- a/test/tests/zoteroPaneTest.js +++ b/test/tests/zoteroPaneTest.js @@ -2006,6 +2006,8 @@ describe("ZoteroPane", function () { // Open the menupopup so it gets populated let popup = doc.getElementById('zotero-copy-collection-popup'); popup.openPopup(); + // Wait a moment to populate the popup + await Zotero.Promise.delay(); // Find the menuitem for collection B and click it let collectionMenuItem = popup.querySelector(`[value="C${collectionB.id}"]`);