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

Closes #1644, Allow selecting multiple items with zotero://select

Browse source 
This adds selectItems() to ZoteroPane and collectionTreeView and removes
the ancient, unused 'expand' argument to selectItem(), which didn't
really make sense there. It also includes a new
itemTreeView::ensureRowsAreVisible() that tries to scroll to an
appropriate place (or, better yet, not scroll at all) given the
specified rows and page size.
This commit is contained in:
Dan Stillman 2019-02-23 17:28:44 -05:00
parent 4d3625f101
commit 945c413c42
8 changed files with 331 additions and 185 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

24
chrome/content/zotero/xpcom/api.js
View file

@ -97,9 +97,7 @@ Zotero.API = {
}
var s = new Zotero.Search;
if (params.libraryID !== undefined) {
s.addCondition('libraryID', 'is', params.libraryID);
}
s.libraryID = params.libraryID;
if (params.objectKey) {
s.addCondition('key', 'is', params.objectKey);
@ -107,11 +105,12 @@ Zotero.API = {
else if (params.objectID) {
s.addCondition('itemID', 'is', params.objectID);
}
if (params.itemKey) {
for (let i=0; i<params.itemKey.length; i++) {
let itemKey = params.itemKey[i];
s.addCondition('key', 'is', itemKey);
// For performance reasons, add requested item keys instead of filtering after.
// This will need to be changed if other conditions are added to the search.
else if (params.itemKey) {
s.addCondition('joinMode', 'any');
for (let key of params.itemKey) {
s.addCondition('key', 'is', key);
}
}
@ -123,20 +122,19 @@ Zotero.API = {
var ids = yield s.search();
}
let itemKeys = new Set(params.itemKey || []);
if (results) {
// Filter results by item key
if (params.itemKey) {
results = results.filter(function (result) {
return params.itemKey.indexOf(result.key) !== -1;
});
results = results.filter(key => itemKeys.has(key));
}
}
else if (ids) {
// Filter results by item key
if (params.itemKey) {
ids = ids.filter(function (id) {
ids = ids.filter((id) => {
var {libraryID, key} = Zotero.Items.getLibraryAndKeyFromID(id);
return params.itemKey.indexOf(key) !== -1;
return itemKeys.has(key);
});
}
results = yield Zotero.Items.getAsync(ids);

68
chrome/content/zotero/xpcom/collectionTreeView.js
View file

@ -1164,57 +1164,75 @@ Zotero.CollectionTreeView.prototype.selectTrash = Zotero.Promise.coroutine(funct
/**
* Find item in current collection, or, if not there, in a library root, and select it
* Find an item in the current collection, or, if not there, in a library root, and select it
*
* @param {Integer} itemID
* @param {Integer} - itemID
* @param {Boolean} [inLibraryRoot=false] - Always show in library root
* @param {Boolean} [expand=false] - Open item if it's a container
* @return {Boolean} - True if item was found, false if not
* @return {Boolean} - TRUE if the item was selected, FALSE if not
*/
Zotero.CollectionTreeView.prototype.selectItem = Zotero.Promise.coroutine(function* (itemID, inLibraryRoot, expand) {
if (!itemID) {
return false;
Zotero.CollectionTreeView.prototype.selectItem = async function (itemID, inLibraryRoot) {
return !!(await this.selectItems([itemID], inLibraryRoot));
};
/**
* Find items in current collection, or, if not there, in a library root, and select them
*
* @param {Integer[]} itemIDs
* @param {Boolean} [inLibraryRoot=false] - Always show in library root
* @return {Integer} - The number of items selected
*/
Zotero.CollectionTreeView.prototype.selectItems = async function (itemIDs, inLibraryRoot) {
if (!itemIDs.length) {
return 0;
}
var item = yield Zotero.Items.getAsync(itemID);
if (!item) {
return false;
var items = await Zotero.Items.getAsync(itemIDs);
if (!items.length) {
return 0;
}
yield this.waitForLoad();
await this.waitForLoad();
// Check if items from multiple libraries were specified
if (items.length > 1 && new Set(items.map(item => item.libraryID)).size > 1) {
Zotero.debug("Can't select items in multiple libraries", 2);
return 0;
}
var currentLibraryID = this.getSelectedLibraryID();
var libraryID = items[0].libraryID;
// If in a different library
if (item.libraryID != currentLibraryID) {
if (libraryID != currentLibraryID) {
Zotero.debug("Library ID differs; switching library");
yield this.selectLibrary(item.libraryID);
await this.selectLibrary(libraryID);
}
// Force switch to library view
else if (!this.selectedTreeRow.isLibrary() && inLibraryRoot) {
Zotero.debug("Told to select in library; switching to library");
yield this.selectLibrary(item.libraryID);
await this.selectLibrary(libraryID);
}
yield this.itemTreeView.waitForLoad();
await this.itemTreeView.waitForLoad();
var selected = yield this.itemTreeView.selectItem(itemID, expand);
if (selected) {
return true;
var numSelected = await this.itemTreeView.selectItems(itemIDs);
if (numSelected == items.length) {
return numSelected;
}
if (item.deleted) {
// If there's a single item and it's in the trash, switch to that
if (items.length == 1 && items[0].deleted) {
Zotero.debug("Item is deleted; switching to trash");
yield this.selectTrash(item.libraryID);
await this.selectTrash(libraryID);
}
else {
Zotero.debug("Item was not selected; switching to library");
yield this.selectLibrary(item.libraryID);
await this.selectLibrary(libraryID);
}
yield this.itemTreeView.waitForLoad();
return this.itemTreeView.selectItem(itemID, expand);
});
await this.itemTreeView.waitForLoad();
return this.itemTreeView.selectItems(itemIDs);
};
/*

221
chrome/content/zotero/xpcom/itemTreeView.js
View file

@ -259,10 +259,9 @@ Zotero.ItemTreeView.prototype.setTree = async function (treebox) {
this._updateIntroText();
if (this.collectionTreeRow && this.collectionTreeRow.itemToSelect) {
var item = this.collectionTreeRow.itemToSelect;
await this.selectItem(item['id'], item['expand']);
this.collectionTreeRow.itemToSelect = null;
if (this.collectionTreeRow && this.collectionTreeRow.itemsToSelect) {
await this.selectItems(this.collectionTreeRow.itemsToSelect);
this.collectionTreeRow.itemsToSelect = null;
}
Zotero.debug("Set tree for items view " + this.id + " in " + (Date.now() - start) + " ms");
@ -1804,60 +1803,56 @@ Zotero.ItemTreeView.prototype._updateIntroText = function() {
/*
* Select an item
*/
Zotero.ItemTreeView.prototype.selectItem = Zotero.Promise.coroutine(function* (id, expand, noRecurse) {
Zotero.ItemTreeView.prototype.selectItem = async function (id) {
return this.selectItems([id]);
};
Zotero.ItemTreeView.prototype.selectItems = async function (ids, noRecurse) {
if (!ids.length) return 0;
// If no row map, we're probably in the process of switching collections,
// so store the item to select on the item group for later
// so store the items to select on the item group for later
if (!this._rowMap) {
if (this.collectionTreeRow) {
this.collectionTreeRow.itemToSelect = { id: id, expand: expand };
Zotero.debug("_rowMap not yet set; not selecting item");
return false;
this.collectionTreeRow.itemsToSelect = ids;
Zotero.debug("_rowMap not yet set; not selecting items");
return 0;
}
Zotero.debug('Item group not found and no row map in ItemTreeView.selectItem() -- discarding select', 2);
return false;
return 0;
}
var row = this._rowMap[id];
// Get the row of the parent, if there is one
var parentRow = null;
var item = yield Zotero.Items.getAsync(id);
var idsToSelect = [];
for (let id of ids) {
let row = this._rowMap[id];
let item = Zotero.Items.get(id);
// Can't select a deleted item if we're not in the trash
if (item.deleted && !this.collectionTreeRow.isTrash()) {
return false;
continue;
}
var parent = item.parentItemID;
if (parent && this._rowMap[parent] != undefined) {
parentRow = this._rowMap[parent];
}
// Get the row of the parent, if there is one
let parent = item.parentItemID;
let parentRow = parent && this._rowMap[parent];
var selected = this.getSelectedItems(true);
if (selected.length == 1 && selected[0] == id) {
Zotero.debug("Item " + id + " is already selected");
this.betterEnsureRowIsVisible(row, parentRow);
return true;
}
// If row with id not visible, check to see if it's hidden under a parent
if(row == undefined)
{
if (!parent || parentRow === null) {
// If row with id isn't visible, check to see if it's hidden under a parent
if (row == undefined) {
if (!parent || parentRow === undefined) {
// No parent -- it's not here
// Clear the quick search and tag selection and try again (once)
if (!noRecurse && this.window.ZoteroPane) {
let cleared1 = yield this.window.ZoteroPane.clearQuicksearch();
let cleared1 = await this.window.ZoteroPane.clearQuicksearch();
let cleared2 = this.window.ZoteroPane.tagSelector.clearTagSelection();
if (cleared1 || cleared2) {
return this.selectItem(id, expand, true);
return this.selectItems(ids, true);
}
}
Zotero.debug("Could not find row for item; not selecting item");
return false;
Zotero.debug(`Couldn't find row for item ${id} -- not selecting`);
continue;
}
// If parent is already open and we haven't found the item, the child
@ -1866,9 +1861,28 @@ Zotero.ItemTreeView.prototype.selectItem = Zotero.Promise.coroutine(function* (i
// Open the parent
this.toggleOpenState(parentRow);
row = this._rowMap[id];
}
// Since we're opening containers, we still need to reference by id
idsToSelect.push(id);
}
// Now that all items have been expanded, get associated rows
var rowsToSelect = [];
for (let id of idsToSelect) {
let row = this._rowMap[id];
rowsToSelect.push(row);
}
// If items are already selected, just scroll to the top-most one
var selectedRows = new Set(this.getSelectedRowIndexes());
if (rowsToSelect.length == selectedRows.size && rowsToSelect.every(row => selectedRows.has(row))) {
this.ensureRowsAreVisible(rowsToSelect);
return rowsToSelect.length;
}
// Single item
if (rowsToSelect.length == 1) {
// this.selection.select() triggers the <tree>'s 'onselect' attribute, which calls
// ZoteroPane.itemSelected(), which calls ZoteroItemPane.viewItem(), which refreshes the
// itembox. But since the 'onselect' doesn't handle promises, itemSelected() isn't waited for
@ -1877,19 +1891,10 @@ Zotero.ItemTreeView.prototype.selectItem = Zotero.Promise.coroutine(function* (i
// itemSelected() when it's done.
let promise;
try {
if (this.selection.selectEventsSuppressed) {
this.selection.select(row);
}
else {
if (!this.selection.selectEventsSuppressed) {
promise = this.waitForSelect();
this.selection.select(row);
}
// If |expand|, open row if container
if (expand && this.isContainer(row) && !this.isContainerOpen(row)) {
this.toggleOpenState(row);
}
this.selection.select(row);
this.selection.select(rowsToSelect[0]);
}
// Ignore NS_ERROR_UNEXPECTED from nsITreeSelection::select(), apparently when the tree
// disappears before it's called (though I can't reproduce it):
@ -1900,69 +1905,101 @@ Zotero.ItemTreeView.prototype.selectItem = Zotero.Promise.coroutine(function* (i
}
if (promise) {
yield promise;
await promise;
}
this.betterEnsureRowIsVisible(row, parentRow);
return true;
});
/**
* Select multiple top-level items
*
* @param {Integer[]} ids An array of itemIDs
*/
Zotero.ItemTreeView.prototype.selectItems = function(ids) {
if (ids.length == 0) {
return;
}
var rows = [];
for (let id of ids) {
if(this._rowMap[id] !== undefined) rows.push(this._rowMap[id]);
}
rows.sort(function (a, b) {
return a - b;
});
// Multiple items
else {
this.selection.clearSelection();
this.selection.selectEventsSuppressed = true;
var lastStart = 0;
for (var i = 0, len = rows.length; i < len; i++) {
if (i == len - 1 || rows[i + 1] != rows[i] + 1) {
this.selection.rangedSelect(rows[lastStart], rows[i], true);
for (let i = 0, len = rowsToSelect.length; i < len; i++) {
if (i == len - 1 || rowsToSelect[i + 1] != rowsToSelect[i] + 1) {
this.selection.rangedSelect(rowsToSelect[lastStart], rowsToSelect[i], true);
lastStart = i + 1;
}
}
this.selection.selectEventsSuppressed = false;
// TODO: This could probably be improved to try to focus more of the selected rows
this.betterEnsureRowIsVisible(rows[0]);
}
this.ensureRowsAreVisible(rowsToSelect);
Zotero.ItemTreeView.prototype.betterEnsureRowIsVisible = function (row, parentRow = null) {
// We aim for a row 5 below the target row, since ensureRowIsVisible() does
// the bare minimum to get the row in view
for (let v = row + 5; v >= row; v--) {
if (this._rows[v]) {
this._treebox.ensureRowIsVisible(v);
if (this._treebox.getFirstVisibleRow() <= row) {
break;
return rowsToSelect.length;
};
Zotero.ItemTreeView.prototype.ensureRowsAreVisible = function (rows) {
const firstVisibleRow = this._treebox.getFirstVisibleRow();
const pageLength = this._treebox.getPageLength();
const lastVisibleRow = firstVisibleRow + pageLength;
const maxBuffer = 5;
var isRowVisible = function (row) {
return row >= firstVisibleRow && row <= lastVisibleRow;
};
rows = rows.concat();
rows.sort((a, b) => a - b);
var rowsWithParents = [];
for (let row of rows) {
let parent = this.getParentIndex(row);
rowsWithParents.push(parent != -1 ? parent : row);
}
// If all rows are visible, don't change anything
if (rows.every(row => isRowVisible(row))) {
//Zotero.debug("All rows are visible");
return;
}
// If we can fit all parent rows in view, do that
for (let buffer = maxBuffer; buffer >= 0; buffer--) {
if (rowsWithParents[rowsWithParents.length - 1] - rowsWithParents[0] - buffer < pageLength) {
//Zotero.debug(`We can fit all parent rows with buffer ${buffer}`);
this._treebox.scrollToRow(rowsWithParents[0] - buffer);
return;
}
}
// If we can fit all rows in view, do that
for (let buffer = maxBuffer; buffer >= 0; buffer--) {
if (rows[rows.length - 1] - rows[0] - buffer < pageLength) {
//Zotero.debug(`We can fit all rows with buffer ${buffer}`);
this._treebox.scrollToRow(rows[0] - buffer);
return;
}
}
// If more than half of the rows are visible, don't change anything
var visible = 0;
for (let row of rows) {
if (isRowVisible(row)) {
visible++;
}
}
if (visible > rows / 2) {
//Zotero.debug("More than half of rows are visible");
return;
}
// If the first parent row isn't in view and we have enough room, make it visible, trying to
// put it five rows from the top
if (rows[0] != rowsWithParents[0]) {
for (let buffer = maxBuffer; buffer >= 0; buffer--) {
if (rows[0] - rowsWithParents[0] - buffer <= pageLength) {
//Zotero.debug(`Scrolling to first parent minus ${buffer}`);
this._treebox.scrollToRow(rowsWithParents[0] - buffer);
return;
}
}
}
// If the parent row isn't in view and we have enough room, make parent visible
if (parentRow !== null && this._treebox.getFirstVisibleRow() > parentRow) {
if ((row - parentRow) < this._treebox.getPageLength()) {
this._treebox.ensureRowIsVisible(parentRow);
}
}
// Otherwise just put the first row at the top
//Zotero.debug("Scrolling to first row " + Math.max(rows[0] - maxBuffer, 0));
this._treebox.scrollToRow(Math.max(rows[0] - maxBuffer, 0));
};

14
chrome/content/zotero/xpcom/libraryTreeView.js
View file

@ -134,6 +134,20 @@ Zotero.LibraryTreeView.prototype = {
},
getSelectedRowIndexes: function () {
var rows = [];
var start = {};
var end = {};
for (let i = 0, len = this.selection.getRangeCount(); i < len; i++) {
this.selection.getRangeAt(i, start, end);
for (let j = start.value; j <= end.value; j++) {
rows.push(j);
}
}
return rows;
},
/**
* Return an object describing the current scroll position to restore after changes
*

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

@ -2187,13 +2187,21 @@ var ZoteroPane = new function()
});
this.selectItem = Zotero.Promise.coroutine(function* (itemID, inLibraryRoot, expand) {
this.selectItem = async function (itemID, inLibraryRoot) {
if (!itemID) {
return false;
}
return this.selectItems([itemID], inLibraryRoot);
};
var item = yield Zotero.Items.getAsync(itemID);
if (!item) {
this.selectItems = async function (itemIDs, inLibraryRoot) {
if (!itemIDs.length) {
return false;
}
var items = await Zotero.Items.getAsync(itemIDs);
if (!items.length) {
return false;
}
@ -2206,7 +2214,7 @@ var ZoteroPane = new function()
throw new Error("Collections view not loaded");
}
var found = yield this.collectionsView.selectItem(itemID, inLibraryRoot, expand);
var found = await this.collectionsView.selectItems(itemIDs, inLibraryRoot);
// Focus the items pane
if (found) {
@ -2215,7 +2223,7 @@ var ZoteroPane = new function()
// open Zotero pane
this.show();
});
};
this.getSelectedLibraryID = function () {

9
components/zotero-protocol-handler.js
View file

@ -849,6 +849,12 @@ function ZoteroProtocolHandler() {
router.run(path);
Zotero.API.parseParams(params);
if (!params.objectKey && !params.objectID && !params.itemKey) {
Zotero.debug("No objects specified");
return;
}
var results = yield Zotero.API.getResultsFromParams(params);
if (!results.length) {
@ -868,8 +874,7 @@ function ZoteroProtocolHandler() {
return zp.collectionsView.selectCollection(results[0].id);
}
else {
// TODO: Currently only able to select one item
return zp.selectItem(results[0].id);
return zp.selectItems(results.map(x => x.id));
}
}),

14
test/tests/collectionTreeViewTest.js
View file

@ -490,7 +490,19 @@ describe("Zotero.CollectionTreeView", function() {
await cv.selectItem(item.id);
await waitForItemsLoad(win);
assert.equal(cv.selection.currentIndex, 0);
assert.sameMembers(zp.itemsView.getSelectedItems(), [item]);
assert.sameMembers(zp.itemsView.getSelectedItems(true), [item.id]);
});
});
describe("#selectItems()", function () {
it("should switch to library root if at least one item isn't in the current collection", async function () {
var collection = await createDataObject('collection');
var item1 = await createDataObject('item', { collections: [collection.id] });
var item2 = await createDataObject('item');
await cv.selectItems([item1.id, item2.id]);
await waitForItemsLoad(win);
assert.equal(cv.selection.currentIndex, 0);
assert.sameMembers(zp.itemsView.getSelectedItems(true), [item1.id, item2.id]);
});
});

76
test/tests/itemTreeViewTest.js
View file

@ -1,7 +1,9 @@
"use strict";
describe("Zotero.ItemTreeView", function() {
var win, zp, cv, itemsView, existingItemID;
var win, zp, cv, itemsView;
var existingItemID;
var existingItemID2;
// Load Zotero pane and select library
before(function* () {
@ -9,8 +11,10 @@ describe("Zotero.ItemTreeView", function() {
zp = win.ZoteroPane;
cv = zp.collectionsView;
var item = yield createDataObject('item', { setTitle: true });
existingItemID = item.id;
var item1 = yield createDataObject('item', { setTitle: true });
existingItemID = item1.id;
var item2 = yield createDataObject('item');
existingItemID2 = item2.id;
});
beforeEach(function* () {
yield selectLibrary(win);
@ -30,20 +34,70 @@ describe("Zotero.ItemTreeView", function() {
describe("#selectItem()", function () {
/**
* Make sure that selectItem() doesn't hang if the pane's item-select handler is never
* triggered due to the item already being selected
* Don't hang if the pane's item-select handler is never triggered due to the item already
* being selected
*/
it("should return if item is already selected", function* () {
yield itemsView.selectItem(existingItemID);
it("should return if item is already selected", async function () {
var numSelected = await itemsView.selectItem(existingItemID);
assert.equal(numSelected, 1);
var selected = itemsView.getSelectedItems(true);
assert.lengthOf(selected, 1);
assert.equal(selected[0], existingItemID);
yield itemsView.selectItem(existingItemID);
numSelected = await itemsView.selectItem(existingItemID);
assert.equal(numSelected, 1);
selected = itemsView.getSelectedItems(true);
assert.lengthOf(selected, 1);
assert.equal(selected[0], existingItemID);
});
})
});
describe("#selectItems()", function () {
/**
* Don't hang if the pane's item-select handler is never triggered due to the items already
* being selected
*/
it("should return if all items are already selected", async function () {
var itemIDs = [existingItemID, existingItemID2];
var numSelected = await itemsView.selectItems(itemIDs);
assert.equal(numSelected, 2);
var selected = itemsView.getSelectedItems(true);
assert.lengthOf(selected, 2);
assert.sameMembers(selected, itemIDs);
numSelected = await itemsView.selectItems(itemIDs);
assert.equal(numSelected, 2);
selected = itemsView.getSelectedItems(true);
assert.lengthOf(selected, 2);
assert.sameMembers(selected, itemIDs);
});
it("should expand parent items to select children", async function () {
var item1 = await createDataObject('item');
var item2 = await createDataObject('item');
var item3 = await createDataObject('item');
var note1 = await createDataObject('item', { itemType: 'note', parentID: item1.id });
var note2 = await createDataObject('item', { itemType: 'note', parentID: item2.id });
var note3 = await createDataObject('item', { itemType: 'note', parentID: item3.id });
var toSelect = [note1.id, note2.id, note3.id];
itemsView.collapseAllRows();
var numSelected = await itemsView.selectItems(toSelect);
assert.equal(numSelected, 3);
var selected = itemsView.getSelectedItems(true);
assert.lengthOf(selected, 3);
assert.sameMembers(selected, toSelect);
// Again with the ids given in reverse order
itemsView.collapseAllRows();
toSelect = toSelect.reverse();
var numSelected = await itemsView.selectItems(toSelect);
assert.equal(numSelected, 3);
var selected = itemsView.getSelectedItems(true);
assert.lengthOf(selected, 3);
assert.sameMembers(selected, toSelect);
});
});
describe("#getCellText()", function () {
it("should return new value after edit", function* () {
@ -720,7 +774,7 @@ describe("Zotero.ItemTreeView", function() {
var item3 = yield createDataObject('item', { itemType: 'note', parentID: item1.id });
let view = zp.itemsView;
yield view.selectItem(item3.id, true);
yield view.selectItem(item3.id);
var promise = view.waitForSelect();
@ -759,7 +813,7 @@ describe("Zotero.ItemTreeView", function() {
var item3 = yield createDataObject('item', { itemType: 'note', parentID: item2.id });
let view = zp.itemsView;
yield view.selectItem(item3.id, true);
yield view.selectItem(item3.id);
var promise = view.waitForSelect();