diff --git a/chrome/content/zotero/collectionViewItemTree.jsx b/chrome/content/zotero/collectionViewItemTree.jsx index ec423c0380..1614c3acb9 100644 --- a/chrome/content/zotero/collectionViewItemTree.jsx +++ b/chrome/content/zotero/collectionViewItemTree.jsx @@ -43,6 +43,7 @@ const React = require('react'); const ReactDOM = require('react-dom'); const ItemTree = require('zotero/itemTree'); const { ItemTreeRowProvider } = ItemTree; +const { LibraryHeaderItemTreeRow } = require('zotero/itemTreeRow'); const { OS } = ChromeUtils.importESModule("chrome://zotero/content/osfile.mjs"); const { ZOTERO_CONFIG } = ChromeUtils.importESModule('resource://zotero/config.mjs'); @@ -109,6 +110,47 @@ class CollectionViewItemTreeRowProvider extends ItemTreeRowProvider { return this.collectionTreeRow?.searchText.length > 0; } + /** + * When showing multiple libraries, group rows by library in collections-list + * order -- independent of the active sort direction -- with each library's + * header row pinned above its items + */ + _compareRows(a, b) { + if (this._groupedByLibrary) { + let rankA = this._libraryOrder.get(a.ref.libraryID) ?? Infinity; + let rankB = this._libraryOrder.get(b.ref.libraryID) ?? Infinity; + if (rankA != rankB) { + return rankA - rankB; + } + let aHeader = a.type == 'library-header'; + let bHeader = b.type == 'library-header'; + if (aHeader || bHeader) { + return aHeader == bHeader ? 0 : (aHeader ? -1 : 1); + } + } + return super._compareRows(a, b); + } + + /** + * Insert a library header row above each library's group of items. + * Rows must already be sorted with library as the primary grouping. + */ + _insertLibraryHeaders() { + let newRows = []; + let lastLibraryID = null; + for (let row of this._rows) { + if (row.type == 'library-header') { + continue; + } + if (row.level == 0 && row.ref.libraryID !== lastLibraryID) { + lastLibraryID = row.ref.libraryID; + newRows.push(new LibraryHeaderItemTreeRow(Zotero.Libraries.get(lastLibraryID))); + } + newRows.push(row); + } + this._rows = newRows; + } + /** * Set new collectionTreeRows and refresh items. * This handles the data/model logic; UI orchestration stays in ItemTree. @@ -140,6 +182,19 @@ class CollectionViewItemTreeRowProvider extends ItemTreeRowProvider { } this.collectionTreeRows = collectionTreeRows; + // When the selection spans multiple libraries, group items by library in + // collections-list order (the order of the selected rows), with a header + // row above each library's items + this._libraryOrder = new Map(); + for (let row of collectionTreeRows) { + let libraryID = row.ref.libraryID; + if (libraryID !== undefined && !this._libraryOrder.has(libraryID)) { + this._libraryOrder.set(libraryID, this._libraryOrder.size); + } + } + this._groupedByLibrary = !collectionTreeRows[0].isFeedsOrFeed() + && this._libraryOrder.size > 1; + // Set ID based on visibilityGroup const visibilityGroup = collectionTreeRows[0].visibilityGroup || 'default'; let treeID = "item-tree-" + this.itemTree.props.id + "-" + visibilityGroup; @@ -259,6 +314,10 @@ class CollectionViewItemTreeRowProvider extends ItemTreeRowProvider { var skipChildren; for (let i = 0; i < this._rows.length; i++) { let row = this._rows[i]; + // Don't copy library header rows -- they're reinserted after sorting + if (row.type == 'library-header') { + continue; + } // Top-level items if (row.level == 0) { // A top-level attachment moved into a parent. Don't copy, it will be added @@ -336,7 +395,10 @@ class CollectionViewItemTreeRowProvider extends ItemTreeRowProvider { this._rows = newRows; this.refreshRowMap(); await this.itemTree._ensureSortContextReady(); - this._sort(options.forceSortAll ? null : [...addedItemIDs]); + // In grouped mode, always sort everything: a partial sort doesn't compare + // pre-existing rows against each other, so library grouping wouldn't be + // applied to rows carried over from the previous view + this._sort(options.forceSortAll || this._groupedByLibrary ? null : [...addedItemIDs]); // Toggle all open containers closed and open to refresh child items var t = new Date(); @@ -347,7 +409,12 @@ class CollectionViewItemTreeRowProvider extends ItemTreeRowProvider { } this.refreshRowMap(); Zotero.debug(`Refreshed open parents in ${new Date() - t} ms`); - + + if (this._groupedByLibrary) { + this._insertLibraryHeaders(); + this.refreshRowMap(); + } + this._searchMode = newSearchMode; this._searchItemIDs = newSearchItemIDs; // items matching the search this.itemTree.invalidateRowCache(true); @@ -536,7 +603,14 @@ class CollectionViewItemTreeRowProvider extends ItemTreeRowProvider { return; } - if ((action == 'remove' && !collectionTreeRows.some(row => row.isLibrary(true))) + // In grouped (multi-library) mode, handle removals with a full refresh, since + // incremental row removal would leave the header row of an emptied library group + if (this._groupedByLibrary && ['remove', 'delete', 'trash'].includes(action)) { + this.itemTree.invalidateRowCache(ids); + refresh = true; + madeChanges = true; + } + else if ((action == 'remove' && !collectionTreeRows.some(row => row.isLibrary(true))) || action == 'delete' || action == 'trash' || (action == 'removeDuplicatesMaster' && collectionTreeRow.isDuplicates())) { // Since a remove involves shifting of rows, we have to do it in order, @@ -1022,6 +1096,10 @@ class CollectionViewItemTree extends ItemTree { * @returns {Boolean} */ isSelectable(index, selectAll=false) { + // Library header rows are never selectable + if (this.getRow(index)?.type == 'library-header') { + return false; + } // Every listed item is selectable individually. There are exceptions // for select-all selections. if (!selectAll) return true; @@ -1088,6 +1166,10 @@ class CollectionViewItemTree extends ItemTree { * Start a drag using HTML 5 Drag and Drop */ onDragStart(event, index) { + if (this.getRow(index)?.type == 'library-header') { + event.preventDefault(); + return false; + } Zotero.DragDrop.currentDragSource = this.collectionTreeRow; return super.onDragStart(event, index); }; diff --git a/chrome/content/zotero/itemTree.jsx b/chrome/content/zotero/itemTree.jsx index a0ee9a3920..ab08b68911 100644 --- a/chrome/content/zotero/itemTree.jsx +++ b/chrome/content/zotero/itemTree.jsx @@ -2188,6 +2188,7 @@ var ItemTree = class ItemTree extends LibraryTree { div.classList.toggle('first-highlighted', this._highlightedRows.has(rowData.id) && !this._highlightedRows.has(prevRowID)); div.classList.toggle('last-highlighted', this._highlightedRows.has(rowData.id) && !this._highlightedRows.has(nextRowID)); div.classList.toggle('annotation-row', row.type === 'annotation'); + div.classList.toggle('library-header-row', row.type === 'library-header'); if (row.type !== 'annotation') { div.classList.remove('tight'); } diff --git a/chrome/content/zotero/itemTreeRow.js b/chrome/content/zotero/itemTreeRow.js index 09099c1548..a8c765311f 100644 --- a/chrome/content/zotero/itemTreeRow.js +++ b/chrome/content/zotero/itemTreeRow.js @@ -631,6 +631,48 @@ class SearchItemTreeRow extends ItemTreeRow { * Dispatch order: Collection, Search, annotation item, file attachment item, * generic Zotero.Item, and finally the base ItemTreeRow fallback. */ +/** + * Non-selectable section header row shown above each library's items when the + * items list displays a multi-library selection. Wraps a Zotero.Library. + */ +class LibraryHeaderItemTreeRow extends ItemTreeRow { + constructor(library) { + super(library, 0, false); + } + + get type() { + return 'library-header'; + } + + getDisplayTitle() { + return Zotero.Libraries.getName(this.ref.libraryID); + } + + getField(field) { + if (field == 'title') { + return this.getDisplayTitle(); + } + return ''; + } + + getIcon() { + let icon = getCSSIcon(this.ref.libraryType == 'group' ? 'library-group' : 'library'); + icon.classList.add('icon-item-type'); + return icon; + } + + renderRow(div, _index, _columns, _rowData, _renderCtx) { + // Single cell with the library icon and name, spanning the row + let span = document.createElement('span'); + span.className = 'cell primary library-header'; + let textSpan = document.createElement('span'); + textSpan.className = 'cell-text'; + textSpan.textContent = this.getDisplayTitle(); + span.append(this.getIcon(), textSpan); + div.appendChild(span); + } +} + ItemTreeRow.create = function (ref, level, isOpen) { if (ref instanceof Zotero.Collection) return new CollectionItemTreeRow(ref, level, isOpen); if (ref instanceof Zotero.Search) return new SearchItemTreeRow(ref, level, isOpen); @@ -646,3 +688,4 @@ module.exports.FileItemTreeRow = FileItemTreeRow; module.exports.AnnotationItemTreeRow = AnnotationItemTreeRow; module.exports.CollectionItemTreeRow = CollectionItemTreeRow; module.exports.SearchItemTreeRow = SearchItemTreeRow; +module.exports.LibraryHeaderItemTreeRow = LibraryHeaderItemTreeRow; diff --git a/scss/components/_item-tree.scss b/scss/components/_item-tree.scss index e432c1df5a..c3e5ac5eb6 100644 --- a/scss/components/_item-tree.scss +++ b/scss/components/_item-tree.scss @@ -215,6 +215,22 @@ } } + // Section header above each library's items in a cross-library selection. + .library-header-row { + .cell.library-header { + display: flex; + align-items: center; + padding-bottom: 3px; + font-weight: 600; + color: var(--fill-secondary); + + .icon-item-type { + margin-inline-end: 6px; + opacity: 0.8; + } + } + } + .annotation-row { .cell { font-size: $font-size-small; diff --git a/test/tests/collectionViewItemTreeTest.js b/test/tests/collectionViewItemTreeTest.js index a1306b1f57..57fb4f293e 100644 --- a/test/tests/collectionViewItemTreeTest.js +++ b/test/tests/collectionViewItemTreeTest.js @@ -2648,7 +2648,99 @@ describe("CollectionViewItemTree", function () { } }); }); - + + describe("Library grouping", function () { + async function selectMultipleCollections(collections) { + await cv.selectByID("C" + collections[0].id); + await waitForItemsLoad(win); + for (let i = 1; i < collections.length; i++) { + cv.selection.toggleSelect(cv.getRowIndexByID("C" + collections[i].id)); + } + await zp.onCollectionSelected(); + await zp.itemsView.waitForLoad(); + } + + it("should group items by library with headers in collections-list order", async function () { + let group = await createGroup(); + let collection1 = await createDataObject('collection'); + let collection2 = await createDataObject('collection', { libraryID: group.libraryID }); + // Reverse-alphabetical across the library boundary, so title sorting + // alone would put the group item first + let item1 = await createDataObject('item', { title: "ZZZ", collections: [collection1.id] }); + let item2 = await createDataObject( + 'item', + { libraryID: group.libraryID, title: "AAA", collections: [collection2.id] } + ); + + await cv.expandLibrary(group.libraryID); + await selectMultipleCollections([collection1, collection2]); + + let view = zp.itemsView; + let userHeaderRow = view.getRowIndexByID("L" + Zotero.Libraries.userLibraryID); + let groupHeaderRow = view.getRowIndexByID("L" + group.libraryID); + let item1Row = view.getRowIndexByID(item1.id); + let item2Row = view.getRowIndexByID(item2.id); + + assert.isNumber(userHeaderRow, "User library header should be shown"); + assert.isNumber(groupHeaderRow, "Group library header should be shown"); + assert.isBelow(userHeaderRow, groupHeaderRow, + "User library group should come first"); + assert.isAbove(item1Row, userHeaderRow); + assert.isBelow(item1Row, groupHeaderRow, + "User library item should be in the user library group despite sorting after the group item"); + assert.isAbove(item2Row, groupHeaderRow); + + // Header rows aren't selectable + assert.isFalse(view.isSelectable(userHeaderRow)); + + await selectLibrary(win); + await group.eraseTx(); + }); + + it("shouldn't group when all items are in a single library", async function () { + let collection1 = await createDataObject('collection'); + let collection2 = await createDataObject('collection'); + let item1 = await createDataObject('item', { collections: [collection1.id] }); + let item2 = await createDataObject('item', { collections: [collection2.id] }); + + await selectMultipleCollections([collection1, collection2]); + + let view = zp.itemsView; + assert.isFalse(view.rowProvider._groupedByLibrary); + assert.isFalse(view.getRowIndexByID("L" + Zotero.Libraries.userLibraryID), + "No library header should be shown for a single-library selection"); + assert.isNumber(view.getRowIndexByID(item1.id)); + assert.isNumber(view.getRowIndexByID(item2.id)); + + await selectLibrary(win); + }); + + it("shouldn't group feeds by library, even across feed libraries", async function () { + let feed1 = await createFeed(); + let feed2 = await createFeed(); + let feedItem1 = await createDataObject('feedItem', { libraryID: feed1.libraryID }); + let feedItem2 = await createDataObject('feedItem', { libraryID: feed2.libraryID }); + + // Select both feeds (each is its own feed library) + await cv.selectByID(feed1.treeViewID); + await waitForItemsLoad(win); + cv.selection.toggleSelect(cv.getRowIndexByID(feed2.treeViewID)); + await zp.onCollectionSelected(); + await zp.itemsView.waitForLoad(); + + let view = zp.itemsView; + // The selection spans two feed libraries, but feeds are never grouped + assert.isFalse(view.rowProvider._groupedByLibrary); + assert.isFalse(view.getRowIndexByID("L" + feed1.libraryID), + "No library header should be shown for a feeds selection"); + assert.isNumber(view.getRowIndexByID(feedItem1.id)); + assert.isNumber(view.getRowIndexByID(feedItem2.id)); + + await selectLibrary(win); + await clearFeeds(); + }); + }); + describe("#setFilter()", function () { it("should refresh when search filter value changes", async function () { let rowProvider = itemsView.rowProvider; diff --git a/test/tests/zoteroPaneTest.js b/test/tests/zoteroPaneTest.js index 70f108bcf2..58d29239f2 100644 --- a/test/tests/zoteroPaneTest.js +++ b/test/tests/zoteroPaneTest.js @@ -2245,6 +2245,26 @@ describe("ZoteroPane", function () { } assert.equal(count, 1, "Item in both collections should appear only once"); }); + + it("should group items by library for a cross-library selection", async function () { + let group = await createGroup(); + let collection1 = await createDataObject('collection'); + let collection2 = await createDataObject('collection', { libraryID: group.libraryID }); + await createDataObject('item', { collections: [collection1.id] }); + await createDataObject('item', { libraryID: group.libraryID, collections: [collection2.id] }); + + await zp.collectionsView.expandLibrary(group.libraryID); + await selectMultipleCollections([collection1, collection2]); + + // The detailed grouping behavior (ordering, header heights, gating) is + // covered in collectionViewItemTreeTest; here just confirm that a + // cross-library selection produces a library-grouped view + assert.isNumber(zp.itemsView.getRowIndexByID("L" + Zotero.Libraries.userLibraryID), + "Cross-library selection should show a library header"); + + await selectLibrary(win); + await group.eraseTx(); + }); }); describe("#newItem()", function () {