From b80fa996f52aeb684de8978b0d00f5dbb1ab6af4 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Sat, 13 Jun 2026 19:21:00 -0400 Subject: [PATCH] Group the items list by library (#5954) When the items list contains items from more than one library, group them by library -- in collections-list order, independent of the active sort -- with a section heading above each library's items. Grouping is triggered automatically by an items list spanning more than one library, not the kind of selection behind it, so any future source of multi-library items would be separated the same way. Today the cross-library collection selection is the only such source. --- .../content/zotero/collectionViewItemTree.jsx | 88 ++++++++++++++++- chrome/content/zotero/itemTree.jsx | 1 + chrome/content/zotero/itemTreeRow.js | 43 +++++++++ scss/components/_item-tree.scss | 16 ++++ test/tests/collectionViewItemTreeTest.js | 94 ++++++++++++++++++- test/tests/zoteroPaneTest.js | 20 ++++ 6 files changed, 258 insertions(+), 4 deletions(-) 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 () {