mirrors/zotero
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages 1 Wiki Activity Actions

Add getSelectedObjects() and limit getSelectedItems() to items

Browse source 
This adds an explicit function on ZoteroPane and the item tree for
getting objects -- including collections and searches in the trash --
and limits `getSelectedItems()` to returning actual items from the
selection. We shim various item properties on the collections and
searches in the trash, but code that's getting the selection should be
explicit about what it wants. Outside of the trash, they're equivalent,
except `getSelectedObjects()` does have an `asIDs` mode.

This switches to using getSelectedObjects() in various places and fixes
collections in the trash appearing as belonging to My Publications when
using the collections-containing-an-item highlight [1].

This also adds support for highlighting the parent collections of
collections in the trash.

[1] https://forums.zotero.org/discussion/115449/zotero-7-beta-deleted-collection-appears-as-belonging-to-my-publications
This commit is contained in:
Dan Stillman 2024-06-23 04:56:19 -04:00
parent a612c1227e
commit c384fef867
4 changed files with 137 additions and 59 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -440,7 +440,7 @@ var ItemTree = class ItemTree extends LibraryTree {
var refreshed = false;
var sort = false;
var savedSelection = this.getSelectedItems(true);
var savedSelection = this.getSelectedObjects();
var previousFirstSelectedRow = this._rowMap[
// 'collection-item' ids are in the form <collectionID>-<itemID>
// 'item' events are just integers
@ -467,7 +467,7 @@ var ItemTree = class ItemTree extends LibraryTree {
}
}
// If refreshing a single item, clear caches and then deselect and reselect row
else if (savedSelection.length == 1 && savedSelection[0] == ids[0]) {
else if (savedSelection.length == 1 && savedSelection[0].id == ids[0]) {
let id = ids[0];
let row = this._rowMap[id];
delete this._rowCache[id];
@ -802,7 +802,7 @@ var ItemTree = class ItemTree extends LibraryTree {
}
// If single item is selected and was modified
else if (action == 'modify' && ids.length == 1 &&
savedSelection.length == 1 && savedSelection[0] == ids[0]) {
savedSelection.length == 1 && savedSelection[0].id == ids[0]) {
if (activeWindow) {
await this.selectItem(ids[0]);
reselect = true;
@ -818,7 +818,7 @@ var ItemTree = class ItemTree extends LibraryTree {
|| action == 'trash'
|| action == 'delete'
|| action == 'removeDuplicatesMaster')
&& savedSelection.some(id => this.getRowIndexByID(id) === false)) {
&& savedSelection.some(o => this.getRowIndexByID(o.id) === false)) {
// In duplicates view, select the next set on delete
if (collectionTreeRow.isDuplicates()) {
if (this._rows[previousFirstSelectedRow]) {
@ -1087,7 +1087,7 @@ var ItemTree = class ItemTree extends LibraryTree {
if (this.selection) {
this.selection.selectEventsSuppressed = true;
}
const selection = this.getSelectedItems(true);
const selection = this.getSelectedObjects();
await this.refresh();
clearItemsPaneMessage && this.clearItemsPaneMessage();
await new Promise((resolve) => {
@ -1451,7 +1451,7 @@ var ItemTree = class ItemTree extends LibraryTree {
return collation.compareString(1, fieldA, fieldB);
}
var savedSelection = this.getSelectedItems(true);
var savedSelection = this.getSelectedObjects();
// Save open state and close containers before sorting
var openItemIDs = this._saveOpenState(true);
@ -1586,7 +1586,7 @@ var ItemTree = class ItemTree extends LibraryTree {
return;
}
if (!skipRowMapRefresh) {
var savedSelection = this.getSelectedItems(true);
var savedSelection = this.getSelectedObjects();
}
var count = 0;
@ -1650,7 +1650,7 @@ var ItemTree = class ItemTree extends LibraryTree {
return;
}
var savedSelection = this.getSelectedItems(true);
var savedSelection = this.getSelectedObjects();
for (var i=0; i<this.rowCount; i++) {
var id = this.getRow(i).ref.id;
if (searchParentIDs.has(id) && this.isContainer(i) && !this.isContainerOpen(i)) {
@ -1663,7 +1663,7 @@ var ItemTree = class ItemTree extends LibraryTree {
expandAllRows() {
this.selection.selectEventsSuppressed = true;
var selectedItems = this.getSelectedItems(true);
var selectedItems = this.getSelectedObjects();
for (var i=0; i<this.rowCount; i++) {
if (this.isContainer(i) && !this.isContainerOpen(i)) {
this.toggleOpenState(i, true);
@ -1678,7 +1678,7 @@ var ItemTree = class ItemTree extends LibraryTree {
collapseAllRows() {
this.selection.selectEventsSuppressed = true;
const selectedItems = this.getSelectedItems(true);
const selectedItems = this.getSelectedObjects();
for (var i=0; i<this.rowCount; i++) {
if (this.isContainer(i)) {
this._closeContainer(i, true);
@ -1693,7 +1693,7 @@ var ItemTree = class ItemTree extends LibraryTree {
expandSelectedRows() {
this.selection.selectEventsSuppressed = true;
const selectedItems = this.getSelectedItems(true);
const selectedItems = this.getSelectedObjects();
// Reverse sort so we don't mess up indices of subsequent
// items when expanding
const indices = Array.from(this.selection.selected).sort((a, b) => b - a);
@ -1711,7 +1711,7 @@ var ItemTree = class ItemTree extends LibraryTree {
collapseSelectedRows() {
this.selection.selectEventsSuppressed = true;
const selectedItems = this.getSelectedItems(true);
const selectedItems = this.getSelectedObjects();
// Reverse sort and so we don't mess up indices of subsequent
// items when collapsing
const indices = Array.from(this.selection.selected).sort((a, b) => b - a);
@ -1825,21 +1825,27 @@ var ItemTree = class ItemTree extends LibraryTree {
}
}
getSelectedItems(asIDs) {
var items = this.selection ? Array.from(this.selection.selected) : [];
items = items.filter(index => index < this._rows.length);
/**
* Get selected objects, including collections and searches in the trash
*/
getSelectedObjects() {
var indexes = this.selection ? Array.from(this.selection.selected) : [];
indexes = indexes.filter(index => index < this._rows.length);
try {
if (asIDs) return items.map(index => this.getRow(index).ref.treeViewID);
return items.map(index => this.getRow(index).ref);
} catch (e) {
Zotero.debug(items);
return indexes.map(index => this.getRow(index).ref);
}
catch (e) {
Zotero.debug(indexes);
throw e;
}
}
saveSelection() {
Zotero.debug("ItemTree::saveSelection() is deprecated -- use getSelectedItems(true)");
return this.getSelectedItems(true);
/**
* Get selected items, omitting collections and searches in the trash
*/
getSelectedItems(asIDs) {
var items = this.getSelectedObjects().filter(x => x.isItem());
return asIDs ? items.map(x => x.id) : items;
}
/**
@ -3127,7 +3133,7 @@ var ItemTree = class ItemTree extends LibraryTree {
if (!this.isContainerOpen(index)) return;
if (!skipRowMapRefresh) {
var savedSelection = this.getSelectedItems(true);
var savedSelection = this.getSelectedObjects();
}
var count = 0;
@ -3663,12 +3669,12 @@ var ItemTree = class ItemTree extends LibraryTree {
}).bind(this);
try {
for (let i = 0; i < selection.length; i++) {
if (this._rowMap[selection[i]] != null) {
toggleSelect(selection[i]);
if (this._rowMap[selection[i].treeViewID] != null) {
toggleSelect(selection[i].treeViewID);
}
// Try the parent
else {
var item = Zotero.Items.get(selection[i]);
let item = selection[i];
if (!item) {
continue;
}
@ -3682,7 +3688,7 @@ var ItemTree = class ItemTree extends LibraryTree {
if (expandCollapsedParents) {
await this._closeContainer(this._rowMap[parent]);
await this.toggleOpenState(this._rowMap[parent]);
toggleSelect(selection[i]);
toggleSelect(selection[i].treeViewID);
}
else {
!this.selection.isSelected(this._rowMap[parent]) &&

88
chrome/content/zotero/zoteroPane.js
View file

@ -47,7 +47,6 @@ var ZoteroPane = new function()
this.handleKeyDown = handleKeyDown;
this.captureKeyDown = captureKeyDown;
this.handleKeyUp = handleKeyUp;
this.setHighlightedRowsCallback = setHighlightedRowsCallback;
this.handleKeyPress = handleKeyPress;
this.getSelectedCollection = getSelectedCollection;
this.getSelectedSavedSearch = getSelectedSavedSearch;
@ -1102,7 +1101,7 @@ var ZoteroPane = new function()
createInstance(Components.interfaces.nsITimer);
// {} implements nsITimerCallback
this.highlightTimer.initWithCallback({
notify: ZoteroPane_Local.setHighlightedRowsCallback
notify: () => this._setHighlightedRowsCallback()
}, 225, Components.interfaces.nsITimer.TYPE_ONE_SHOT);
}
// If anything but Ctlr/Options was pressed, most likely a different shortcut using Ctlr/Options
@ -1227,26 +1226,40 @@ var ZoteroPane = new function()
* Highlights collections containing selected items on Ctrl (Win) or
* Option/Alt (Mac/Linux) press
*/
function setHighlightedRowsCallback() {
var itemIDs = ZoteroPane_Local.getSelectedItems(true);
// If no items or an unreasonable number, don't try
if (!itemIDs || !itemIDs.length || itemIDs.length > 100) return;
this._setHighlightedRowsCallback = async function () {
var objects = this.getSelectedObjects();
Zotero.Promise.coroutine(function* () {
var collectionIDs = yield Zotero.Collections.getCollectionsContainingItems(itemIDs, true);
var ids = collectionIDs.map(id => "C" + id);
var userLibraryID = Zotero.Libraries.userLibraryID;
var allInPublications = Zotero.Items.get(itemIDs).every((item) => {
return item.libraryID == userLibraryID && item.inPublications;
})
if (allInPublications) {
ids.push("P" + Zotero.Libraries.userLibraryID);
// If no items or an unreasonable number, don't try
if (!objects.length || objects.length > 100) return;
var collections = objects.filter(o => o.isCollection());
var items = objects.filter(o => o.isItem());
// Get parent collections of collections
var toHighlight = [];
for (let collection of collections) {
if (collection.parentID) {
toHighlight.push(collection.parentID);
}
if (ids.length) {
ZoteroPane_Local.collectionsView.setHighlightedRows(ids);
}
})();
}
}
// Get collections containing items
toHighlight.push(...await Zotero.Collections.getCollectionsContainingItems(
items.map(x => x.id),
true
));
var treeViewIDs = toHighlight.map(id => 'C' + id);
var userLibraryID = Zotero.Libraries.userLibraryID;
// If no collections selected and every item is in My Publications, highlight that
var allInPublications = !collections.length && items.every((item) => {
return item.libraryID == userLibraryID && item.inPublications;
});
if (allInPublications) {
treeViewIDs.push("P" + Zotero.Libraries.userLibraryID);
}
if (treeViewIDs.length) {
await this.collectionsView.setHighlightedRows(treeViewIDs);
}
};
function handleKeyPress(event) {
@ -1916,7 +1929,7 @@ var ZoteroPane = new function()
return false;
}
var selectedItems = this.itemsView.getSelectedItems();
var selectedItems = this.itemsView.getSelectedObjects();
// Display buttons at top of item pane depending on context. This needs to run even if the
// selection hasn't changed, because the selected items might have been modified.
@ -2383,7 +2396,7 @@ var ZoteroPane = new function()
return false;
}
return this.getSelectedItems().some(item => item.deleted);
return this.getSelectedObjects().some(o => o.deleted);
};
@ -2391,12 +2404,12 @@ var ZoteroPane = new function()
* @return {Promise}
*/
this.restoreSelectedItems = async function () {
let selectedIDs = this.getSelectedItems(true);
if (!selectedIDs.length) {
let selectedObjects = this.getSelectedObjects();
if (!selectedObjects.length) {
return;
}
let isSelected = itemOrID => (itemOrID.treeViewID ? selectedIDs.includes(itemOrID.treeViewID) : selectedIDs.includes(itemOrID));
let isSelected = object => selectedObjects.includes(object);
await Zotero.DB.executeTransaction(async () => {
for (let row = 0; row < this.itemsView.rowCount; row++) {
@ -2406,7 +2419,7 @@ var ZoteroPane = new function()
}
let parent = this.itemsView.getRow(row).ref;
let children = [];
let childIDs = [];
let subcollections = [];
if (parent.isCollection()) {
// If the restored item is a collection, restore its subcollections too
@ -2415,17 +2428,22 @@ var ZoteroPane = new function()
}
}
else {
if (!parent.isNote()) children.push(...parent.getNotes(true));
if (!parent.isAttachment()) children.push(...parent.getAttachments(true));
if (!parent.isNote()) {
childIDs.push(...parent.getNotes(true));
}
if (!parent.isAttachment()) {
childIDs.push(...parent.getAttachments(true));
}
}
let childItems = Zotero.Items.get(childIDs);
if (isSelected(parent)) {
if (parent.deleted) {
parent.deleted = false;
await parent.save();
}
let noneSelected = !children.some(isSelected);
let allChildren = Zotero.Items.get(children).concat(Zotero.Collections.get(subcollections));
let noneSelected = !childItems.some(isSelected);
let allChildren = childItems.concat(Zotero.Collections.get(subcollections));
for (let child of allChildren) {
if ((noneSelected || isSelected(child)) && child.deleted) {
child.deleted = false;
@ -2434,7 +2452,7 @@ var ZoteroPane = new function()
}
}
else {
for (let child of Zotero.Items.get(children)) {
for (let child of childItems) {
if (isSelected(child) && child.deleted) {
child.deleted = false;
await child.save();
@ -2994,7 +3012,13 @@ var ZoteroPane = new function()
return this.collectionsView.getSelectedGroup(asID);
}
this.getSelectedObjects = function () {
if (!this.itemsView) return [];
return this.itemsView.getSelectedObjects();
};
/*
* Return an array of Item objects for selected items
*

23
test/tests/itemTreeTest.js
View file

@ -1544,4 +1544,27 @@ describe("Zotero.ItemTree", function() {
assert.equal(OS.Path.basename(path), originalFileName);
});
});
describe("#_restoreSelection()", function () {
it("should reselect collection in trash", async function () {
var userLibraryID = Zotero.Libraries.userLibraryID;
var collection = await createDataObject('collection', { deleted: true });
var item = await createDataObject('item', { deleted: true });
await cv.selectByID("T" + userLibraryID);
await waitForItemsLoad(win);
var collectionRow = zp.itemsView.getRowIndexByID(collection.treeViewID)
var itemRow = zp.itemsView.getRowIndexByID(item.id)
zp.itemsView.selection.toggleSelect(collectionRow);
zp.itemsView.selection.toggleSelect(itemRow);
var selection = zp.itemsView.getSelectedObjects();
assert.lengthOf(selection, 2);
zp.itemsView.selection.clearSelection();
assert.lengthOf(zp.itemsView.getSelectedObjects(), 0);
zp.itemsView._restoreSelection(selection);
assert.lengthOf(zp.itemsView.getSelectedObjects(), 2);
});
});
})

25
test/tests/zoteroPaneTest.js
View file

@ -15,6 +15,31 @@ describe("ZoteroPane", function() {
win.close();
});
describe("#_setHighlightedRowsCallback()", function () {
it("should highlight parent collection of collection in trash", async function () {
var collection1 = await createDataObject('collection');
var collection2 = await createDataObject('collection', { parentID: collection1.id, deleted: true });
var userLibraryID = Zotero.Libraries.userLibraryID;
await zp.collectionsView.selectByID('T' + userLibraryID);
await waitForItemsLoad(win);
var row = zp.itemsView.getRowIndexByID(collection2.treeViewID);
zp.itemsView.selection.select(row);
var spy = sinon.spy(zp.collectionsView, 'setHighlightedRows');
await zp._setHighlightedRowsCallback();
assert.sameMembers(spy.getCall(0).args[0], ['C1']);
var rows = win.document.querySelectorAll('.highlighted');
assert.lengthOf(rows, 1);
zp.collectionsView.setHighlightedRows();
spy.restore();
});
});
describe("#newItem", function () {
it("should create an item and focus the title field", function* () {
yield zp.newItem(Zotero.ItemTypes.getID('book'), {}, null, true);