diff --git a/chrome/content/zotero/collectionViewItemTree.jsx b/chrome/content/zotero/collectionViewItemTree.jsx index a960bc430e..dad021e11e 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,53 @@ 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; + let seenFirstHeader = false; + for (let row of this._rows) { + if (row.type == 'library-header') { + continue; + } + if (row.level == 0 && row.ref.libraryID !== lastLibraryID) { + lastLibraryID = row.ref.libraryID; + let header = new LibraryHeaderItemTreeRow(Zotero.Libraries.get(lastLibraryID)); + // Every header except the first gets a gap above it separating it from + // the previous library's items (see _updateLibraryHeaderHeights and CSS) + header.hasSectionGap = seenFirstHeader; + seenFirstHeader = true; + newRows.push(header); + } + 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 +188,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; @@ -250,6 +311,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 @@ -327,7 +392,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(); @@ -338,7 +406,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); @@ -527,7 +600,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, @@ -960,6 +1040,10 @@ class CollectionViewItemTree extends ItemTree { } async handleRowModelUpdate(rows, options = {}) { + // Update header row heights before super renders: super invalidates the + // windowed list, and _renderItem reads the custom-height map at render time, + // so the map and offsets have to be set first for the taller rows to paint + this._updateLibraryHeaderHeights(); const completed = await super.handleRowModelUpdate(rows, options); if (completed) { await this._updateIntroText(); @@ -967,6 +1051,35 @@ class CollectionViewItemTree extends ItemTree { return completed; } + /** + * Each library header after the first is given extra height so that, with the + * header content bottom-aligned (see _item-tree.scss), there's a gap above it + * separating it from the previous library's items. The first header gets no extra + * height (no gap at the top of the list). The windowed list positions every row at + * index * baseRowHeight unless an override is given, so feed it the current header + * indices on each model update. + */ + _updateLibraryHeaderHeights() { + if (!this.tree?._jsWindow || !this.tree._rowHeight) { + return; + } + // _groupedByLibrary and the row array live on the row provider, not the tree + let provider = this.rowProvider; + let customRowHeights = []; + if (provider._groupedByLibrary) { + // Headers with a section gap are made taller; bottom-aligned content (CSS) + // turns the extra height into a gap above the heading + let headerHeight = Math.round(this.tree._rowHeight * 1.5); + let rows = provider._rows; + for (let i = 0; i < rows.length; i++) { + if (rows[i].type == 'library-header' && rows[i].hasSectionGap) { + customRowHeights.push([i, headerHeight]); + } + } + } + this.tree.updateCustomRowHeights(customRowHeights); + } + async notify(action, type, ids, extraData) { // If a collection with subcollections is deleted/restored, ids will include subcollections // though they are not showing in itemTree. @@ -1013,6 +1126,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; @@ -1079,6 +1196,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 8300e68ea7..114e50e0e8 100644 --- a/chrome/content/zotero/itemTree.jsx +++ b/chrome/content/zotero/itemTree.jsx @@ -2188,6 +2188,8 @@ 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'); + div.classList.toggle('section-gap', !!row.hasSectionGap); 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..683ead15ac 100644 --- a/scss/components/_item-tree.scss +++ b/scss/components/_item-tree.scss @@ -215,6 +215,32 @@ } } + // Section header above each library's items in a cross-library selection. + // A hairline below the heading separates it from its own items. The first + // heading sits flush at the top; every later heading carries extra height + // (via customRowHeights) that, with bottom-aligned content, becomes a gap + // above it separating it from the previous library's items. + .library-header-row { + border-bottom: var(--material-panedivider); + + &.section-gap { + align-items: flex-end; + } + + .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 6db0d466c0..51bc5aad9e 100644 --- a/test/tests/collectionViewItemTreeTest.js +++ b/test/tests/collectionViewItemTreeTest.js @@ -2580,7 +2580,109 @@ 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)); + + // The first library header sits flush at the top (default height); each + // later header is taller, giving a gap above it that separates sections + let tree = view.tree; + assert.notProperty(tree._customRowHeightMap, String(userHeaderRow), + "First library header should use the default height (no gap at top)"); + assert.isAbove(tree._customRowHeightMap[groupHeaderRow], tree._rowHeight, + "Later library headers should be taller, for a gap above the heading"); + assert.notProperty(tree._customRowHeightMap, String(item1Row), + "Item rows should use the default height"); + + 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 5755e4e85d..a27fcbf3d0 100644 --- a/test/tests/zoteroPaneTest.js +++ b/test/tests/zoteroPaneTest.js @@ -2196,6 +2196,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 () {