Group the items list by library

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. The first heading sits flush at the
top; each later one has a gap above it and a hairline below separating it from
its 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.

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);
 	};

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');
 		}

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;

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;

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;

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 () {