scriptoriumadmin/zotero
1
0
Fork 0
mirror of https://github.com/zotero/zotero.git synced 2026-10-06 02:50:03 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

Show library-aware section headers in the item tree for multi-row selections

Browse source 
https://github.com/zotero/zotero/pull/5954#issuecomment-4724330966
This commit is contained in:
Dan Stillman 2026-06-17 14:32:22 -04:00
parent 7d6bf376c3
commit 167045d000
6 changed files with 307 additions and 76 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

126
chrome/content/zotero/collectionViewItemTree.jsx
View file

@ -43,7 +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 { LibraryHeaderItemTreeRow, SpacerItemTreeRow } = require('zotero/itemTreeRow');
const { OS } = ChromeUtils.importESModule("chrome://zotero/content/osfile.mjs");
const { ZOTERO_CONFIG } = ChromeUtils.importESModule('resource://zotero/config.mjs');
@ -140,23 +140,80 @@ class CollectionViewItemTreeRowProvider extends ItemTreeRowProvider {
let lastLibraryID = null;
let seenFirstHeader = false;
for (let row of this._rows) {
if (row.type == 'library-header') {
if (row.type == 'library-header' || row.type == 'spacer') {
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;
let library = Zotero.Libraries.get(lastLibraryID);
// Every header except the first gets a blank spacer row above it, for one
// row of whitespace separating its section from the previous library's
if (seenFirstHeader) {
newRows.push(new SpacerItemTreeRow(library));
}
seenFirstHeader = true;
newRows.push(header);
let { label, iconName } = this._sectionHeaders.get(lastLibraryID) || {};
newRows.push(new LibraryHeaderItemTreeRow(library, label, iconName));
}
newRows.push(row);
}
this._rows = newRows;
}
/**
* Compute the section header label (and, for single-library selections, icon) for
* each library in the selection. Returns a Map of libraryID -> { label, iconName }.
*
* Across libraries, each header is the library name plus what's selected in it
* ("My Library (2 collections selected)"); within a single library, one header
* summarizes the selection ("2 collections selected"). Labels are resolved here (and
* cached on the rows) so the sticky header can render them synchronously.
*/
async _computeSectionHeaders(collectionTreeRows) {
let grouped = this._groupedByLibrary;
// Group the selected rows by library, preserving selection order
let byLibrary = new Map();
for (let row of collectionTreeRows) {
let libraryID = row.ref.libraryID;
if (!byLibrary.has(libraryID)) {
byLibrary.set(libraryID, []);
}
byLibrary.get(libraryID).push(row);
}
let entries = [];
for (let [libraryID, rows] of byLibrary) {
let count = rows.length;
let library = Zotero.Libraries.getName(libraryID);
let id;
if (rows.every(row => row.isRecentlyRead())) {
id = grouped ? 'items-section-library-recently-read' : null;
}
else if (rows.every(row => row.isLibrary(true))) {
id = grouped ? 'items-section-library' : null;
}
else if (rows.every(row => row.isCollection())) {
id = grouped ? 'items-section-library-collections' : 'items-section-collections-selected';
}
else if (rows.every(row => row.isSearch())) {
id = grouped ? 'items-section-library-searches' : 'items-section-searches-selected';
}
else {
id = grouped ? 'items-section-library-sources' : 'items-section-sources-selected';
}
// Cross-library headers use the library icon; the single-library summary
// header has no icon, to set it apart from a sticky library header
entries.push({ libraryID, id, args: { library, count }, iconName: grouped ? undefined : null });
}
let labels = await document.l10n.formatValues(
entries.map(e => (e.id ? { id: e.id, args: e.args } : { id: 'items-section-library', args: e.args }))
);
let map = new Map();
entries.forEach((e, i) => {
map.set(e.libraryID, { label: labels[i], iconName: e.iconName });
});
return map;
}
/**
* Set new collectionTreeRows and refresh items.
* This handles the data/model logic; UI orchestration stays in ItemTree.
@ -200,6 +257,14 @@ class CollectionViewItemTreeRowProvider extends ItemTreeRowProvider {
}
this._groupedByLibrary = !collectionTreeRows[0].isFeedsOrFeed()
&& this._libraryOrder.size > 1;
// A section header is shown above each library's items whenever more than one row
// is selected (a single header for a single-library selection; one per library
// across libraries). Feeds are never grouped or headed.
this._showSectionHeaders = !collectionTreeRows[0].isFeedsOrFeed()
&& collectionTreeRows.length > 1;
this._sectionHeaders = this._showSectionHeaders
? await this._computeSectionHeaders(collectionTreeRows)
: new Map();
// Set ID based on visibilityGroup
const visibilityGroup = collectionTreeRows[0].visibilityGroup || 'default';
@ -311,8 +376,8 @@ 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') {
// Don't copy library header or spacer rows -- they're reinserted after sorting
if (row.type == 'library-header' || row.type == 'spacer') {
continue;
}
// Top-level items
@ -407,7 +472,7 @@ class CollectionViewItemTreeRowProvider extends ItemTreeRowProvider {
this.refreshRowMap();
Zotero.debug(`Refreshed open parents in ${new Date() - t} ms`);
if (this._groupedByLibrary) {
if (this._showSectionHeaders) {
this._insertLibraryHeaders();
this.refreshRowMap();
}
@ -1040,10 +1105,6 @@ 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();
@ -1051,35 +1112,6 @@ 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.
@ -1126,8 +1158,9 @@ class CollectionViewItemTree extends ItemTree {
* @returns {Boolean}
*/
isSelectable(index, selectAll=false) {
// Library header rows are never selectable
if (this.getRow(index)?.type == 'library-header') {
// Library header and spacer rows are never selectable
let type = this.getRow(index)?.type;
if (type == 'library-header' || type == 'spacer') {
return false;
}
// Every listed item is selectable individually. There are exceptions
@ -1196,7 +1229,8 @@ class CollectionViewItemTree extends ItemTree {
* Start a drag using HTML 5 Drag and Drop
*/
onDragStart(event, index) {
if (this.getRow(index)?.type == 'library-header') {
let type = this.getRow(index)?.type;
if (type == 'library-header' || type == 'spacer') {
event.preventDefault();
return false;
}

5
chrome/content/zotero/itemTree.jsx
View file

@ -1424,6 +1424,9 @@ var ItemTree = class ItemTree extends LibraryTree {
multiSelect: this.props.multiSelect,
stickySectionHeaders: true,
isSectionHeader: index => this.getRow(index)?.type == 'library-header',
onSelectionChange: this._handleSelectionChange.bind(this),
isSelectable: this.isSelectable.bind(this),
getParentIndex: this.getParentIndex.bind(this),
@ -2189,7 +2192,7 @@ var ItemTree = class ItemTree extends LibraryTree {
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);
div.classList.toggle('spacer-row', row.type === 'spacer');
if (row.type !== 'annotation') {
div.classList.remove('tight');
}

55
chrome/content/zotero/itemTreeRow.js
View file

@ -636,8 +636,13 @@ class SearchItemTreeRow extends ItemTreeRow {
* items list displays a multi-library selection. Wraps a Zotero.Library.
*/
class LibraryHeaderItemTreeRow extends ItemTreeRow {
constructor(library) {
constructor(library, label, iconName) {
super(library, 0, false);
// Label and icon are computed from the selection (see
// CollectionViewItemTreeRow._computeSectionHeaders); fall back to the library's
// own name/icon when not provided
this._label = label;
this._iconName = iconName;
}
get type() {
@ -645,7 +650,7 @@ class LibraryHeaderItemTreeRow extends ItemTreeRow {
}
getDisplayTitle() {
return Zotero.Libraries.getName(this.ref.libraryID);
return this._label ?? Zotero.Libraries.getName(this.ref.libraryID);
}
getField(field) {
@ -656,23 +661,62 @@ class LibraryHeaderItemTreeRow extends ItemTreeRow {
}
getIcon() {
let icon = getCSSIcon(this.ref.libraryType == 'group' ? 'library-group' : 'library');
// An explicit null icon name means render no icon (e.g. the single-library
// summary header), distinct from undefined (use the library's own icon)
if (this._iconName === null) {
return null;
}
let iconName = this._iconName
?? (this.ref.libraryType == 'group' ? 'library-group' : 'library');
let icon = getCSSIcon(iconName);
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
// Single cell with an optional icon and the label, 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);
let icon = this.getIcon();
if (icon) {
span.append(icon, textSpan);
}
else {
// No icon, but reserve its space so the label still lines up with item titles
let spacer = document.createElement('span');
spacer.className = 'library-header-icon-spacer';
span.append(spacer, textSpan);
}
div.appendChild(span);
}
}
/**
* A blank, non-selectable row providing one row of whitespace above a library header
* (see CollectionViewItemTreeRow._insertLibraryHeaders). Kept separate from the header
* row so the header stays a uniform height and pins flush to the top when sticky.
*/
class SpacerItemTreeRow extends ItemTreeRow {
constructor(library) {
super(library, 0, false, 'spacer-' + library.libraryID);
}
get type() {
return 'spacer';
}
getField() {
return '';
}
renderRow(_div, _index, _columns, _rowData, _renderCtx) {
// Intentionally empty -- the row's height alone provides the whitespace
}
}
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);
@ -688,4 +732,5 @@ module.exports.FileItemTreeRow = FileItemTreeRow;
module.exports.AnnotationItemTreeRow = AnnotationItemTreeRow;
module.exports.CollectionItemTreeRow = CollectionItemTreeRow;
module.exports.SearchItemTreeRow = SearchItemTreeRow;
module.exports.SpacerItemTreeRow = SpacerItemTreeRow;
module.exports.LibraryHeaderItemTreeRow = LibraryHeaderItemTreeRow;

35
chrome/locale/en-US/zotero/zotero.ftl
View file

@ -195,6 +195,41 @@ collections-menu-show-recently-read =
.label = Show { recently-read }
item-menu-remove-from-recently-read =
.label = Remove from { recently-read }…
# Item list section headers for a multiple-row collection-tree selection within one library
items-section-collections-selected =
{ $count ->
[one] { $count } collection selected
*[other] { $count } collections selected
}
items-section-searches-selected =
{ $count ->
[one] { $count } saved search selected
*[other] { $count } saved searches selected
}
items-section-sources-selected =
{ $count ->
[one] { $count } source selected
*[other] { $count } sources selected
}
# Item list section headers for a selection spanning multiple libraries, one per library
items-section-library-collections =
{ $count ->
[one] { $library } ({ $count } collection selected)
*[other] { $library } ({ $count } collections selected)
}
items-section-library-searches =
{ $count ->
[one] { $library } ({ $count } saved search selected)
*[other] { $library } ({ $count } saved searches selected)
}
items-section-library-sources =
{ $count ->
[one] { $library } ({ $count } source selected)
*[other] { $library } ({ $count } sources selected)
}
items-section-library-recently-read = { $library } ({ recently-read })
items-section-library = { $library }
collections-menu-rename =
.label = Rename
edit-saved-search = Edit Saved Search

52
scss/components/_item-tree.scss
View file

@ -25,6 +25,14 @@
padding-inline-start: 8px;
padding-inline-end: calc(8px + var(--scrollbar-width, 0px));
box-sizing: border-box;
// A clear divider below the column headers (the default faint border is lost
// against the white section-header/spacer rows below it). The header already
// carries a second 1px line via ::after, so drop that one to avoid doubling.
border-bottom: var(--material-panedivider);
&::after {
border-bottom: none;
}
.cell.hasAttachment,
.cell.numNotes {
@ -75,7 +83,8 @@
.virtualized-table, .drag-image-container {
.row {
&.odd:not(.selected) {
// Header and spacer rows are always the pane background, never striped
&.odd:not(.selected):not(.library-header-row):not(.spacer-row) {
background-color: var(--material-stripe);
}
@ -215,17 +224,14 @@
}
}
// 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.
// Section header above each library's items in a cross-library selection. These
// rows are pinned to the top while their section is scrolled (see the sticky
// section header support in virtualized-table), so they're a uniform height with
// top-aligned content. The background matches the pane so the items below stripe
// against it; square corners keep the on-scroll hairline from curving at the edges.
.library-header-row {
border-bottom: var(--material-panedivider);
&.section-gap {
align-items: flex-end;
}
background-color: var(--material-background);
border-radius: 0;
.cell.library-header {
display: flex;
@ -235,12 +241,34 @@
color: var(--fill-secondary);
.icon-item-type {
margin-inline-end: 6px;
margin-inline-end: 4px;
opacity: 0.8;
}
// Stands in for a missing icon (single-library summary header) so the
// label still aligns with the item titles below it
.library-header-icon-spacer {
width: 16px;
margin-inline-end: 4px;
flex-shrink: 0;
}
}
}
// The divider under a section header shows only while its section is scrolled
// under the pinned copy (see the "stuck" state in virtualized-table). Drawn as a
// box-shadow rather than a border so toggling it doesn't shift the row's content.
.virtualized-table-sticky-section-header.stuck .library-header-row {
box-shadow: inset 0 -1px 0 var(--color-panedivider);
}
// Blank whitespace row above each library header (except the first); matches the
// pane background so it reads as empty space rather than a striped row
.spacer-row {
background-color: var(--material-background);
pointer-events: none;
}
.annotation-row {
.cell {
font-size: $font-size-small;

110
test/tests/collectionViewItemTreeTest.js
View file

@ -2582,6 +2582,12 @@ describe("CollectionViewItemTree", function () {
});
describe("Library grouping", function () {
// Fluent wraps interpolated values in bidi isolation marks; strip them for
// plain-text comparisons
function stripBidi(str) {
return str.replace(/[⁦-⁩]/g, '');
}
async function selectMultipleCollections(collections) {
await cv.selectByID("C" + collections[0].id);
await waitForItemsLoad(win);
@ -2625,21 +2631,99 @@ describe("CollectionViewItemTree", function () {
// 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");
// Each cross-library header is the library name plus what's selected in it
assert.equal(stripBidi(view.getRow(groupHeaderRow).getDisplayTitle()),
Zotero.Libraries.getName(group.libraryID) + " (1 collection selected)");
// A blank spacer row sits above every header except the first, for whitespace
// separating the sections
assert.notEqual(view.getRow(userHeaderRow - 1)?.type, 'spacer',
"No spacer above the first header");
assert.equal(view.getRow(groupHeaderRow - 1).type, 'spacer',
"Spacer row above a later header");
assert.isFalse(view.isSelectable(groupHeaderRow - 1),
"Spacer rows aren't selectable");
await selectLibrary(win);
await group.eraseTx();
});
it("shouldn't group when all items are in a single library", async function () {
it("should pin the section header of the library scrolled to the top", 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] });
// Enough group items below the group header that it can be scrolled to the top
await Zotero.DB.executeTransaction(async function () {
for (let i = 0; i < 60; i++) {
let item = createUnsavedDataObject(
'item', { libraryID: group.libraryID, collections: [collection2.id] }
);
await item.save();
}
});
await cv.expandLibrary(group.libraryID);
await selectMultipleCollections([collection1, collection2]);
let view = zp.itemsView;
let tree = view.tree;
let body = tree._jsWindow.targetElement;
let groupHeaderRow = view.getRowIndexByID("L" + group.libraryID);
// At the top of the list, the first (user library) header is pinned
body.scrollTop = 0;
tree._updateStickySectionHeader();
assert.include(tree._stickyHeader.textContent,
Zotero.Libraries.getName(Zotero.Libraries.userLibraryID));
// The pinned header lines up horizontally with the real header row
let userHeaderRow = view.getRowIndexByID("L" + Zotero.Libraries.userLibraryID);
let realIcon = tree._jsWindow.getElementByIndex(userHeaderRow).querySelector('.icon-item-type');
let stickyIcon = tree._stickyHeaderContent.querySelector('.icon-item-type');
assert.equal(
stickyIcon.getBoundingClientRect().left,
realIcon.getBoundingClientRect().left,
"Pinned header icon should align with the real header icon"
);
// Scrolling the group header to the top pins the group library header instead
body.scrollTop = tree._jsWindow._getItemPosition(groupHeaderRow);
tree._updateStickySectionHeader();
assert.include(tree._stickyHeader.textContent, Zotero.Libraries.getName(group.libraryID));
await selectLibrary(win);
await group.eraseTx();
});
it("should restart row striping at each section header", async function () {
let group = await createGroup();
let collection1 = await createDataObject('collection');
let collection2 = await createDataObject('collection', { libraryID: group.libraryID });
// Two user-library items so the group's first item falls on an odd absolute index
await createDataObject('item', { collections: [collection1.id] });
await createDataObject('item', { collections: [collection1.id] });
await createDataObject('item', { libraryID: group.libraryID, collections: [collection2.id] });
await cv.expandLibrary(group.libraryID);
await selectMultipleCollections([collection1, collection2]);
let view = zp.itemsView;
let tree = view.tree;
let groupHeaderRow = view.getRowIndexByID("L" + group.libraryID);
let firstGroupItemRow = groupHeaderRow + 1;
// The group's first item is at an odd absolute index, but striping restarts at
// the header, so it gets the unstriped (even) shade
assert.equal(firstGroupItemRow % 2, 1, "Setup: first group item at an odd index");
let elem = tree._jsWindow.getElementByIndex(firstGroupItemRow);
assert.isTrue(elem.classList.contains('even'), "First item of a section is unstriped");
assert.isFalse(elem.classList.contains('odd'));
await selectLibrary(win);
await group.eraseTx();
});
it("should show one summary header but not group for a single-library multi-selection", async function () {
let collection1 = await createDataObject('collection');
let collection2 = await createDataObject('collection');
let item1 = await createDataObject('item', { collections: [collection1.id] });
@ -2648,9 +2732,11 @@ describe("CollectionViewItemTree", function () {
await selectMultipleCollections([collection1, collection2]);
let view = zp.itemsView;
// One library -> a single summary header, but not grouped into sections
assert.isFalse(view.rowProvider._groupedByLibrary);
assert.isFalse(view.getRowIndexByID("L" + Zotero.Libraries.userLibraryID),
"No library header should be shown for a single-library selection");
let headerRow = view.getRowIndexByID("L" + Zotero.Libraries.userLibraryID);
assert.isNumber(headerRow, "A summary header should be shown");
assert.equal(stripBidi(view.getRow(headerRow).getDisplayTitle()), "2 collections selected");
assert.isNumber(view.getRowIndexByID(item1.id));
assert.isNumber(view.getRowIndexByID(item2.id));