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

Collection/item tree selection improvements

Browse source 
Wait for the pane's collectionSelected() to finish before returning from
collectionTreeView select methods (e.g., selectLibrary()), and wait for
previous items view to finish loading before creating a new one in
collectionSelected(). This ensures that the items view has been created (though
not loaded) before returning from a select. The tree can still get a bit
confused switching between collections, but I think we're getting closer to
fixing that.

Also switch the items tree to use the same pattern.

This also fixes dragging items to collections (#731).
This commit is contained in:
Dan Stillman 2015-05-22 19:15:21 -04:00
parent a2d4b05064
commit 61cb01b7c2
6 changed files with 297 additions and 190 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -71,7 +71,6 @@ Object.defineProperty(Zotero.CollectionTreeView.prototype, "selectedTreeRow", {
});
/*
* Called by the tree itself
*/
@ -248,6 +247,29 @@ Zotero.CollectionTreeView.prototype.reload = function()
}.bind(this));
}
/**
* Select a row and wait for its items view to be created
*
* Note that this doesn't wait for the items view to be loaded. For that, add a 'load' event
* listener to the items view.
*
* @param {Integer} row
* @return {Promise}
*/
Zotero.CollectionTreeView.prototype.selectWait = Zotero.Promise.method(function (row) {
if (this.selection.selectEventsSuppressed) {
this.selection.select(row);
return;
}
var deferred = Zotero.Promise.defer();
this.addEventListener('select', () => deferred.resolve());
this.selection.select(row);
return deferred.promise;
});
/*
* Called by Zotero.Notifier on any changes to collections in the data layer
*/
@ -389,7 +411,10 @@ Zotero.CollectionTreeView.prototype.notify = Zotero.Promise.coroutine(function*
}
}
var deferred = Zotero.Promise.defer();
this.addEventListener('select', () => deferred.resolve());
this.selection.selectEventsSuppressed = false;
return deferred.promise;
});
/**
@ -823,7 +848,7 @@ Zotero.CollectionTreeView.prototype.selectByID = Zotero.Promise.coroutine(functi
switch (type) {
case 'L':
this.selectLibrary(id);
yield this.selectLibrary(id);
return;
case 'C':
@ -841,14 +866,14 @@ Zotero.CollectionTreeView.prototype.selectByID = Zotero.Promise.coroutine(functi
}
var row = this._rowMap[type + id];
this._treebox.ensureRowIsVisible(row);
this.selection.select(row);
yield this.selectWait(row);
});
/**
* @param {Integer} libraryID Library to select
*/
Zotero.CollectionTreeView.prototype.selectLibrary = function (libraryID) {
Zotero.CollectionTreeView.prototype.selectLibrary = Zotero.Promise.coroutine(function* (libraryID) {
if (Zotero.suppressUIUpdates) {
Zotero.debug("UI updates suppressed -- not changing library selection");
return false;
@ -857,7 +882,7 @@ Zotero.CollectionTreeView.prototype.selectLibrary = function (libraryID) {
// Select local library
if (!libraryID) {
this._treebox.ensureRowIsVisible(0);
this.selection.select(0);
yield this.selectWait(0);
return true;
}
@ -874,12 +899,12 @@ Zotero.CollectionTreeView.prototype.selectLibrary = function (libraryID) {
var row = this._rowMap['L' + libraryID];
if (row !== undefined) {
this._treebox.ensureRowIsVisible(row);
this.selection.select(row);
this.selectWait(row);
return true;
}
return false;
}
});
Zotero.CollectionTreeView.prototype.selectCollection = function (id) {

31
chrome/content/zotero/xpcom/data/collection.js
View file

@ -360,23 +360,22 @@ Zotero.Collection.prototype.addItems = Zotero.Promise.coroutine(function* (itemI
yield this.loadChildItems();
var current = this.getChildItems(true);
return Zotero.DB.executeTransaction(function* () {
for (let i=0; i<itemIDs.length; i++) {
let itemID = itemIDs[i];
if (current && current.indexOf(itemID) != -1) {
Zotero.debug("Item " + itemID + " already a child of collection " + this.id);
continue;
}
let item = yield this.ChildObjects.getAsync(itemID);
yield item.loadCollections();
item.addToCollection(this.id);
yield item.save({
skipDateModifiedUpdate: true
});
Zotero.DB.requireTransaction();
for (let i = 0; i < itemIDs.length; i++) {
let itemID = itemIDs[i];
if (current && current.indexOf(itemID) != -1) {
Zotero.debug("Item " + itemID + " already a child of collection " + this.id);
continue;
}
}.bind(this));
let item = yield this.ChildObjects.getAsync(itemID);
yield item.loadCollections();
item.addToCollection(this.id);
yield item.save({
skipDateModifiedUpdate: true
});
}
yield this.loadChildItems(true);
});

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

@ -68,7 +68,7 @@ Zotero.ItemTreeView.prototype.type = 'item';
Zotero.ItemTreeView.prototype.setTree = Zotero.serial(Zotero.Promise.coroutine(function* (treebox)
{
try {
Zotero.debug("Setting item tree");
Zotero.debug("Setting tree for items view " + this.id);
var start = Date.now();
// Try to set the window document if not yet set
if (treebox && !this._ownerDocument) {
@ -256,7 +256,7 @@ Zotero.ItemTreeView.prototype.setTree = Zotero.serial(Zotero.Promise.coroutine(f
this.collectionTreeRow.itemToSelect = null;
}
Zotero.debug("Set tree in "+(Date.now()-start)+" ms");
Zotero.debug("Set tree for items view " + this.id + " in " + (Date.now() - start) + " ms");
this._initialized = true;
yield this._runListeners('load');
@ -434,7 +434,7 @@ Zotero.ItemTreeView.prototype.notify = Zotero.Promise.coroutine(function* (actio
yield this._refreshPromise;
if (!this._treebox || !this._treebox.treeBody) {
Components.utils.reportError("Treebox didn't exist in itemTreeView.notify()");
Zotero.debug("Treebox didn't exist in itemTreeView.notify()");
return;
}
@ -934,12 +934,13 @@ Zotero.ItemTreeView.prototype.notify = Zotero.Promise.coroutine(function* (actio
//this._treebox.endUpdateBatch();
if (madeChanges) {
var promise = this._getItemSelectedPromise();
var deferred = Zotero.Promise.defer();
this.addEventListener('select', () => deferred.resolve());
}
this.selection.selectEventsSuppressed = false;
if (madeChanges) {
Zotero.debug("Yielding for select promise"); // TEMP
yield promise;
return deferred.promise;
}
});
@ -1630,6 +1631,12 @@ Zotero.ItemTreeView.prototype.selectItem = Zotero.Promise.coroutine(function* (i
return false;
}
var selected = this.getSelectedItems(true);
if (selected.length == 1 && selected[0] == id) {
Zotero.debug("Item " + id + " is already selected");
return;
}
var row = this._rowMap[id];
// Get the row of the parent, if there is one
@ -1674,17 +1681,20 @@ Zotero.ItemTreeView.prototype.selectItem = Zotero.Promise.coroutine(function* (i
row = this._rowMap[id];
}
// This function calls nsITreeSelection.select(), which 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 and 'yield selectItem(itemID)' continues before the
// itembox has been refreshed. To get around this, we make a promise resolver that's
// triggered by itemSelected() when it's done.
if (!this.selection.selectEventsSuppressed) {
var itemSelectedPromise = this._getItemSelectedPromise(id);
// 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
// here, which means that 'yield selectItem(itemID)' continues before the itembox has been
// refreshed. To get around this, we wait for a select event that's triggered by
// itemSelected() when it's done.
if (this.selection.selectEventsSuppressed) {
this.selection.select(row);
}
else {
var deferred = Zotero.Promise.defer();
this.addEventListener('select', () => deferred.resolve());
this.selection.select(row);
}
this.selection.select(row);
// If |expand|, open row if container
if (expand && this.isContainer(row) && !this.isContainerOpen(row)) {
@ -1692,8 +1702,8 @@ Zotero.ItemTreeView.prototype.selectItem = Zotero.Promise.coroutine(function* (i
}
this.selection.select(row);
if (!this.selection.selectEventsSuppressed) {
yield itemSelectedPromise;
if (deferred) {
yield deferred.promise;
}
// We aim for a row 5 below the target row, since ensureRowIsVisible() does
@ -1718,23 +1728,6 @@ Zotero.ItemTreeView.prototype.selectItem = Zotero.Promise.coroutine(function* (i
});
Zotero.ItemTreeView.prototype._getItemSelectedPromise = function (itemID) {
if (itemID) {
var selected = this.getSelectedItems(true);
var alreadySelected = selected.length == 1 && selected[0] == itemID;
if (alreadySelected) {
return Zotero.Promise.resolve();
}
}
return new Zotero.Promise(function () {
this._itemSelectedPromiseResolver = {
resolve: arguments[0],
reject: arguments[1]
};
}.bind(this));
}
/**
* Select multiple top-level items
*

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

@ -26,10 +26,14 @@
Zotero.LibraryTreeView = function () {
this._initialized = false;
this._listeners = {
load: []
load: [],
select: []
};
this._rows = [];
this._rowMap = {};
this.id = Zotero.Utilities.randomString();
Zotero.debug("Creating " + this.type + "s view with id " + this.id);
};
Zotero.LibraryTreeView.prototype = {
@ -43,10 +47,17 @@ Zotero.LibraryTreeView.prototype = {
this._listeners[event].push(listener);
}
}
else {
if (!this._listeners[event]) {
this._listeners[event] = [];
}
this._listeners[event].push(listener);
}
},
_runListeners: Zotero.Promise.coroutine(function* (event) {
if (!this._listeners[event]) return;
var listener;
while (listener = this._listeners[event].shift()) {
yield Zotero.Promise.resolve(listener());
@ -55,13 +66,33 @@ Zotero.LibraryTreeView.prototype = {
/**
* Returns a reference to the tree row at a given row
* Return a reference to the tree row at a given row
*
* @return {Zotero.CollectionTreeRow|Zotero.ItemTreeRow}
*/
getRow: function(row) {
return this._rows[row];
},
/**
* Return the index of the row with a given ID (e.g., "C123" for collection 123)
*
* @param {String} - Row id
* @return {Integer}
*/
getRowByID: function (id) {
var type = id[0];
id = ('' + id).substr(1);
return this._rowMap[type + id];
},
onSelect: function () {
return this._runListeners('select');
},
/**
* Add a tree row to the main array, update the row count, tell the treebox that the row
* count changed, and update the row map

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

@ -1107,110 +1107,127 @@ var ZoteroPane = new function()
});
this.onCollectionSelected = Zotero.Promise.coroutine(function* () {
var collectionTreeRow = this.getCollectionTreeRow();
if (!collectionTreeRow) {
return;
}
if (this.itemsView && this.itemsView.collectionTreeRow == collectionTreeRow) {
Zotero.debug("Collection selection hasn't changed");
return;
}
if (this.itemsView) {
this.itemsView.unregister();
document.getElementById('zotero-items-tree').view = this.itemsView = null;
}
if (this.collectionsView.selection.count != 1) {
return;
}
// Clear quick search and tag selector when switching views
document.getElementById('zotero-tb-search').value = "";
// XBL functions might not yet be available
var tagSelector = document.getElementById('zotero-tag-selector');
if (tagSelector.clearAll) {
tagSelector.clearAll();
}
// Not necessary with seltype="cell", which calls nsITreeView::isSelectable()
/*if (collectionTreeRow.isSeparator()) {
document.getElementById('zotero-items-tree').view = this.itemsView = null;
return;
}*/
collectionTreeRow.setSearch('');
collectionTreeRow.setTags(getTagSelection());
// Enable or disable toolbar icons and menu options as necessary
const disableIfNoEdit = [
"cmd_zotero_newCollection",
"cmd_zotero_newSavedSearch",
"zotero-tb-add",
"cmd_zotero_newItemFromCurrentPage",
"zotero-tb-lookup",
"cmd_zotero_newStandaloneNote",
"zotero-tb-note-add",
"zotero-tb-attachment-add"
];
for(var i=0; i<disableIfNoEdit.length; i++) {
var el = document.getElementById(disableIfNoEdit[i]);
// If a trash is selected, new collection depends on the
// editability of the library
if (collectionTreeRow.isTrash() &&
disableIfNoEdit[i] == 'cmd_zotero_newCollection') {
var overrideEditable = Zotero.Libraries.isEditable(collectionTreeRow.ref.libraryID);
}
else {
var overrideEditable = false;
this.onCollectionSelected = function () {
return Zotero.spawn(function* () {
var collectionTreeRow = this.getCollectionTreeRow();
if (!collectionTreeRow) {
return;
}
if (collectionTreeRow.editable || overrideEditable) {
if(el.hasAttribute("disabled")) el.removeAttribute("disabled");
} else {
el.setAttribute("disabled", "true");
if (this.itemsView && this.itemsView.collectionTreeRow == collectionTreeRow) {
Zotero.debug("Collection selection hasn't changed");
return;
}
}
this.itemsView = new Zotero.ItemTreeView(collectionTreeRow);
this.itemsView.onError = function () {
ZoteroPane_Local.displayErrorMessage();
};
// If any queued load listeners, set them to run when the tree is ready
if (this._listeners.itemsLoaded) {
let listener;
while (listener = this._listeners.itemsLoaded.shift()) {
this.itemsView.addEventListener('load', listener);
if (this.itemsView) {
// Wait for existing items view to finish loading before unloading it
//
// TODO: Cancel loading
let deferred = Zotero.Promise.defer();
this.itemsView.addEventListener('load', function () {
deferred.resolve();
});
if (deferred.promise.isPending()) {
Zotero.debug("Waiting for items view " + this.itemsView.id + " to finish loading");
}
yield deferred.promise;
this.itemsView.unregister();
document.getElementById('zotero-items-tree').view = this.itemsView = null;
}
}
this.itemsView.addEventListener('load', this.setTagScope);
document.getElementById('zotero-items-tree').view = this.itemsView;
// Add events to treecolpicker to update menu before showing/hiding
try {
let treecols = document.getElementById('zotero-items-columns-header');
let treecolpicker = treecols.boxObject.firstChild.nextSibling;
let menupopup = treecolpicker.boxObject.firstChild.nextSibling;
let attr = menupopup.getAttribute('onpopupshowing');
if (attr.indexOf('Zotero') == -1) {
menupopup.setAttribute('onpopupshowing', 'ZoteroPane.itemsView.onColumnPickerShowing(event);')
// Keep whatever else is there
+ ' ' + attr;
menupopup.setAttribute('onpopuphidden', 'ZoteroPane.itemsView.onColumnPickerHidden(event);')
// Keep whatever else is there
+ ' ' + menupopup.getAttribute('onpopuphidden');
if (this.collectionsView.selection.count != 1) {
return;
}
}
catch (e) {
Zotero.debug(e);
}
Zotero.Prefs.set('lastViewedFolder', collectionTreeRow.id);
});
// Clear quick search and tag selector when switching views
document.getElementById('zotero-tb-search').value = "";
// XBL functions might not yet be available
var tagSelector = document.getElementById('zotero-tag-selector');
if (tagSelector.clearAll) {
tagSelector.clearAll();
}
// Not necessary with seltype="cell", which calls nsITreeView::isSelectable()
/*if (collectionTreeRow.isSeparator()) {
document.getElementById('zotero-items-tree').view = this.itemsView = null;
return;
}*/
collectionTreeRow.setSearch('');
collectionTreeRow.setTags(getTagSelection());
// Enable or disable toolbar icons and menu options as necessary
const disableIfNoEdit = [
"cmd_zotero_newCollection",
"cmd_zotero_newSavedSearch",
"zotero-tb-add",
"cmd_zotero_newItemFromCurrentPage",
"zotero-tb-lookup",
"cmd_zotero_newStandaloneNote",
"zotero-tb-note-add",
"zotero-tb-attachment-add"
];
for(var i=0; i<disableIfNoEdit.length; i++) {
var el = document.getElementById(disableIfNoEdit[i]);
// If a trash is selected, new collection depends on the
// editability of the library
if (collectionTreeRow.isTrash() &&
disableIfNoEdit[i] == 'cmd_zotero_newCollection') {
var overrideEditable = Zotero.Libraries.isEditable(collectionTreeRow.ref.libraryID);
}
else {
var overrideEditable = false;
}
if (collectionTreeRow.editable || overrideEditable) {
if(el.hasAttribute("disabled")) el.removeAttribute("disabled");
} else {
el.setAttribute("disabled", "true");
}
}
this.itemsView = new Zotero.ItemTreeView(collectionTreeRow);
this.itemsView.onError = function () {
ZoteroPane_Local.displayErrorMessage();
};
// If any queued load listeners, set them to run when the tree is ready
if (this._listeners.itemsLoaded) {
let listener;
while (listener = this._listeners.itemsLoaded.shift()) {
this.itemsView.addEventListener('load', listener);
}
}
this.itemsView.addEventListener('load', this.setTagScope);
document.getElementById('zotero-items-tree').view = this.itemsView;
// Add events to treecolpicker to update menu before showing/hiding
try {
let treecols = document.getElementById('zotero-items-columns-header');
let treecolpicker = treecols.boxObject.firstChild.nextSibling;
let menupopup = treecolpicker.boxObject.firstChild.nextSibling;
let attr = menupopup.getAttribute('onpopupshowing');
if (attr.indexOf('Zotero') == -1) {
menupopup.setAttribute('onpopupshowing', 'ZoteroPane.itemsView.onColumnPickerShowing(event);')
// Keep whatever else is there
+ ' ' + attr;
menupopup.setAttribute('onpopuphidden', 'ZoteroPane.itemsView.onColumnPickerHidden(event);')
// Keep whatever else is there
+ ' ' + menupopup.getAttribute('onpopuphidden');
}
}
catch (e) {
Zotero.debug(e);
}
Zotero.Prefs.set('lastViewedFolder', collectionTreeRow.id);
}, this)
.finally(function () {
return this.collectionsView.onSelect();
}.bind(this));
};
this.getCollectionTreeRow = function () {
@ -1419,19 +1436,8 @@ var ZoteroPane = new function()
return true;
}, this)
.then(function (result) {
// See note in itemTreeView.js::selectItem()
if (this.itemsView && this.itemsView._itemSelectedPromiseResolver) {
this.itemsView._itemSelectedPromiseResolver.resolve();
}
return result;
}.bind(this))
.catch(function (e) {
Zotero.debug(e, 1);
if (this.itemsView && this.itemsView._itemSelectedPromiseResolver) {
this.itemsView._itemSelectedPromiseResolver.reject(e);
}
throw e;
.finally(function () {
return this.itemsView.onSelect();
}.bind(this));
}
@ -2052,11 +2058,11 @@ var ZoteroPane = new function()
if (!selected) {
if (item.deleted) {
Zotero.debug("Item is deleted; switching to trash");
self.collectionsView.selectTrash(item.libraryID);
yield self.collectionsView.selectTrash(item.libraryID);
}
else {
Zotero.debug("Item was not selected; switching to library");
self.collectionsView.selectLibrary(item.libraryID);
yield self.collectionsView.selectLibrary(item.libraryID);
}
yield self.itemsView.selectItem(itemID, expand);
}
@ -2071,12 +2077,12 @@ var ZoteroPane = new function()
// If in a different library
if (item.libraryID != currentLibraryID) {
Zotero.debug("Library ID differs; switching library");
self.collectionsView.selectLibrary(item.libraryID);
yield self.collectionsView.selectLibrary(item.libraryID);
}
// Force switch to library view
else if (!self.collectionsView.selectedTreeRow.isLibrary() && inLibrary) {
Zotero.debug("Told to select in library; switching to library");
self.collectionsView.selectLibrary(item.libraryID);
yield self.collectionsView.selectLibrary(item.libraryID);
}
}
catch (e) {

83
test/tests/collectionTreeViewTest.js
View file

@ -3,27 +3,31 @@
describe("Zotero.CollectionTreeView", function() {
var win, collectionsView;
// Select library
// TODO: Add a selectCollection() function and select a collection instead
var resetSelection = Zotero.Promise.coroutine(function* () {
yield collectionsView.selectLibrary(Zotero.Libraries.userLibraryID);
yield waitForItemsLoad(win);
assert.equal(collectionsView.getSelectedLibraryID(), Zotero.Libraries.userLibraryID);
});
// Load Zotero pane and select library
before(function* () {
win = yield loadZoteroPane();
collectionsView = win.ZoteroPane.collectionsView;
});
beforeEach(function () {
return resetSelection();
})
after(function () {
win.close();
});
// Select library
// TODO: Add a selectCollection() function and select a collection instead
var resetSelection = function () {
collectionsView.selectLibrary(Zotero.Libraries.userLibraryID);
assert.equal(collectionsView.getSelectedLibraryID(), Zotero.Libraries.userLibraryID);
}
describe("collapse/expand", function () {
it("should close and open My Library repeatedly", function* () {
var libraryID = Zotero.Libraries.userLibraryID;
var cv = collectionsView;
cv.selectLibrary(libraryID);
yield cv.selectLibrary(libraryID);
var row = cv.selection.currentIndex;
cv.collapseLibrary(libraryID);
@ -54,8 +58,6 @@ describe("Zotero.CollectionTreeView", function() {
describe("#notify()", function () {
it("should select a new collection", function* () {
resetSelection();
// Create collection
var collection = new Zotero.Collection;
collection.name = "Select new collection";
@ -67,8 +69,6 @@ describe("Zotero.CollectionTreeView", function() {
});
it("shouldn't select a new collection if skipNotifier is passed", function* () {
resetSelection();
// Create collection with skipNotifier flag
var collection = new Zotero.Collection;
collection.name = "No select on skipNotifier";
@ -81,8 +81,6 @@ describe("Zotero.CollectionTreeView", function() {
});
it("shouldn't select a new collection if skipSelect is passed", function* () {
resetSelection();
// Create collection with skipSelect flag
var collection = new Zotero.Collection;
collection.name = "No select on skipSelect";
@ -100,7 +98,7 @@ describe("Zotero.CollectionTreeView", function() {
collection.name = "No select on modify";
var id = yield collection.saveTx();
resetSelection();
yield resetSelection();
collection.name = "No select on modify 2";
yield collection.saveTx();
@ -157,4 +155,59 @@ describe("Zotero.CollectionTreeView", function() {
}
})
})
describe("#drop()", function () {
it("should add an item to a collection", function* () {
var collection = yield createDataObject('collection', false, {
skipSelect: true
});
var item = yield createDataObject('item', false, {
skipSelect: true
});
var row = collectionsView.getRowByID("C" + collection.id);
// Add observer to wait for collection add
var deferred = Zotero.Promise.defer();
var observerID = Zotero.Notifier.registerObserver({
notify: function (event, type, ids) {
if (type == 'collection-item' && event == 'add'
&& ids[0] == collection.id + "-" + item.id) {
setTimeout(function () {
deferred.resolve();
});
}
}
}, 'collection-item', 'test');
// Simulate a drag and drop
var stub = sinon.stub(Zotero.DragDrop, "getDragTarget");
stub.returns(collectionsView.getRow(row));
collectionsView.drop(row, 0, {
dropEffect: 'copy',
effectAllowed: 'copy',
mozSourceNode: win.document.getElementById('zotero-items-tree'),
types: {
contains: function (type) {
return type == 'zotero/item';
}
},
getData: function (type) {
if (type == 'zotero/item') {
return "" + item.id;
}
}
})
yield deferred.promise;
stub.restore();
Zotero.Notifier.unregisterObserver(observerID);
yield collectionsView.selectCollection(collection.id);
yield waitForItemsLoad(win);
var itemsView = win.ZoteroPane.itemsView
assert.equal(itemsView.rowCount, 1);
var treeRow = itemsView.getRow(0);
assert.equal(treeRow.ref.id, item.id);
})
})
})