commit 820755e152117cffb9b7202ecbb7f3bb9780f5b9
parent 1a4b7121d3b4cc1020a376c37751fa97ea9cf563
Author: Dan Stillman <dstillman@zotero.org>
Date: Fri, 24 Mar 2017 05:18:55 -0400
Rework libraryTreeView event handling
Changes `libraryTreeView::addEventListener('load')` and similar to
`libraryTreeView::onLoad.addListener(listener, once)`, etc. `once` is an
optional boolean that, when true, causes the listener to fire once and
then be removed. This is implicit for 'load'.
'load' maintains its special behavior of running immediately if the
treeview has already been loaded.
Also adds `waitForLoad()` and `waitForSelect()` functions that return
promises on event completion, since most uses of those events were just
resolving deferreds.
Diffstat:
10 files changed, 105 insertions(+), 99 deletions(-)
diff --git a/chrome/content/zotero/integration/addCitationDialog.js b/chrome/content/zotero/integration/addCitationDialog.js
@@ -661,11 +661,9 @@ var Zotero_Citation_Dialog = new function () {
_multipleSourceButton.disabled = false;
}
} else {
- collectionsView.addEventListener('load', Zotero.Promise.coroutine(function* () {
+ collectionsView.onLoad.addListener(Zotero.Promise.coroutine(function* () {
if (itemsView) {
- var deferred = Zotero.Promise.defer();
- itemsView.addEventListener('load', () => deferred.resolve());
- yield deferred.promise;
+ yield itemsView.waitForLoad();
_acceptButton.disabled = !itemsView.getSelectedItems().length;
}
}));
diff --git a/chrome/content/zotero/selectItemsDialog.js b/chrome/content/zotero/selectItemsDialog.js
@@ -49,9 +49,7 @@ var doLoad = Zotero.Promise.coroutine(function* () {
collectionsView.hideSources = ['duplicates', 'trash', 'feeds'];
document.getElementById('zotero-collections-tree').view = collectionsView;
- var deferred = Zotero.Promise.defer();
- collectionsView.addEventListener('load', () => deferred.resolve());
- yield deferred.promise;
+ yield collectionsView.waitForLoad();
connectionSelectedDeferred = Zotero.Promise.defer();
yield connectionSelectedDeferred.promise;
@@ -93,9 +91,7 @@ var onCollectionSelected = Zotero.Promise.coroutine(function* ()
// Create items list and wait for it to load
itemsView = new Zotero.ItemTreeView(collectionTreeRow, (window.arguments[1] ? true : false));
document.getElementById('zotero-items-tree').view = itemsView;
- var deferred = Zotero.Promise.defer();
- itemsView.addEventListener('load', () => deferred.resolve());
- yield deferred.promise;
+ yield itemsView.waitForLoad();
clearItemsPaneMessage();
diff --git a/chrome/content/zotero/xpcom/collectionTreeView.js b/chrome/content/zotero/xpcom/collectionTreeView.js
@@ -120,7 +120,7 @@ Zotero.CollectionTreeView.prototype.setTree = Zotero.Promise.coroutine(function*
}
this.selection.selectEventsSuppressed = false;
- yield this._runListeners('load');
+ yield this.runListeners('load');
this._initialized = true;
}
catch (e) {
@@ -301,10 +301,9 @@ Zotero.CollectionTreeView.prototype.selectWait = Zotero.Promise.method(function
if (this.selection.currentIndex == row) {
return;
};
- var deferred = Zotero.Promise.defer();
- this.addEventListener('select', () => deferred.resolve());
+ var promise = this.waitForSelect();
this.selection.select(row);
- return deferred.promise;
+ return promise;
});
@@ -516,10 +515,9 @@ Zotero.CollectionTreeView.prototype.notify = Zotero.Promise.coroutine(function*
this._rememberScrollPosition(scrollPosition);
- var deferred = Zotero.Promise.defer();
- this.addEventListener('select', () => deferred.resolve());
+ var promise = this.waitForSelect();
this.selection.selectEventsSuppressed = false;
- return deferred.promise;
+ return promise;
});
/**
@@ -1180,9 +1178,7 @@ Zotero.CollectionTreeView.prototype.selectItem = Zotero.Promise.coroutine(functi
return false;
}
- var deferred = Zotero.Promise.defer();
- this.addEventListener('load', () => deferred.resolve());
- yield deferred.promise;
+ yield this.waitForLoad();
var currentLibraryID = this.getSelectedLibraryID();
// If in a different library
@@ -1198,9 +1194,7 @@ Zotero.CollectionTreeView.prototype.selectItem = Zotero.Promise.coroutine(functi
var itemsView = this.selectedTreeRow.itemTreeView;
- deferred = Zotero.Promise.defer();
- itemsView.addEventListener('load', () => deferred.resolve());
- yield deferred.promise;
+ yield itemsView.waitForLoad();
var selected = yield itemsView.selectItem(itemID, expand);
if (selected) {
@@ -1217,9 +1211,7 @@ Zotero.CollectionTreeView.prototype.selectItem = Zotero.Promise.coroutine(functi
}
itemsView = this.selectedTreeRow.itemTreeView;
- deferred = Zotero.Promise.defer();
- itemsView.addEventListener('load', () => deferred.resolve());
- yield deferred.promise;
+ yield itemsView.waitForLoad();
return itemsView.selectItem(itemID, expand);
});
diff --git a/chrome/content/zotero/xpcom/itemTreeView.js b/chrome/content/zotero/xpcom/itemTreeView.js
@@ -276,7 +276,7 @@ Zotero.ItemTreeView.prototype.setTree = Zotero.Promise.coroutine(function* (tree
Zotero.debug("Set tree for items view " + this.id + " in " + (Date.now() - start) + " ms");
this._initialized = true;
- yield this._runListeners('load');
+ yield this.runListeners('load');
}
catch (e) {
Zotero.debug(e, 1);
@@ -953,14 +953,14 @@ Zotero.ItemTreeView.prototype.notify = Zotero.Promise.coroutine(function* (actio
}*/
//this._treebox.endUpdateBatch();
+ let selectPromise;
if (madeChanges) {
- var deferred = Zotero.Promise.defer();
- this.addEventListener('select', () => deferred.resolve());
+ selectPromise = this.waitForSelect();
}
this.selection.selectEventsSuppressed = false;
if (madeChanges) {
Zotero.debug("Yielding for select promise"); // TEMP
- return deferred.promise;
+ return selectPromise;
}
});
@@ -1735,12 +1735,12 @@ Zotero.ItemTreeView.prototype.selectItem = Zotero.Promise.coroutine(function* (i
// 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.
+ let promise;
if (this.selection.selectEventsSuppressed) {
this.selection.select(row);
}
else {
- var deferred = Zotero.Promise.defer();
- this.addEventListener('select', () => deferred.resolve());
+ promise = this.waitForSelect();
this.selection.select(row);
}
@@ -1750,8 +1750,8 @@ Zotero.ItemTreeView.prototype.selectItem = Zotero.Promise.coroutine(function* (i
}
this.selection.select(row);
- if (deferred) {
- yield deferred.promise;
+ if (promise) {
+ yield promise;
}
this.betterEnsureRowIsVisible(row, parentRow);
diff --git a/chrome/content/zotero/xpcom/libraryTreeView.js b/chrome/content/zotero/xpcom/libraryTreeView.js
@@ -25,15 +25,28 @@
Zotero.LibraryTreeView = function () {
this._initialized = false;
- this._listeners = {
- load: [],
- select: []
- };
+ this._listeners = {};
this._rows = [];
this._rowMap = {};
this.id = Zotero.Utilities.randomString();
Zotero.debug("Creating " + this.type + "s view with id " + this.id);
+
+ //
+ // Create .on(Load|Select).addListener() methods
+ //
+ var _createEventBinding = function (event, alwaysOnce) {
+ return alwaysOnce
+ ? {
+ addListener: listener => this._addListener(event, listener, true)
+ }
+ : {
+ addListener: (listener, once) => this._addListener(event, listener, once)
+ };
+ }.bind(this);
+
+ this.onLoad = _createEventBinding('load', true);
+ this.onSelect = _createEventBinding('select');
};
Zotero.LibraryTreeView.prototype = {
@@ -41,31 +54,56 @@ Zotero.LibraryTreeView.prototype = {
return this._initialized;
},
- addEventListener: function(event, listener) {
- if (event == 'load') {
- // If already initialized run now
- if (this._initialized) {
- listener.call(this);
- }
- else {
- this._listeners[event].push(listener);
+
+ addEventListener: function (event, listener) {
+ Zotero.logError("Zotero.LibraryTreeView::addEventListener() is deprecated");
+ this.addListener(event, listener);
+ },
+
+
+ waitForLoad: function () {
+ return this._waitForEvent('load');
+ },
+
+
+ waitForSelect: function () {
+ return this._waitForEvent('select');
+ },
+
+
+ runListeners: Zotero.Promise.coroutine(function* (event) {
+ //Zotero.debug(`Calling ${event} listeners on ${this.type} tree ${this.id}`);
+ if (!this._listeners[event]) return;
+ for (let [listener, once] of this._listeners[event].entries()) {
+ yield Zotero.Promise.resolve(listener.call(this));
+ if (once) {
+ this._listeners[event].delete(listener);
}
}
+ }),
+
+
+ _addListener: function(event, listener, once) {
+ // If already initialized run now
+ if (event == 'load' && this._initialized) {
+ listener.call(this);
+ }
else {
if (!this._listeners[event]) {
- this._listeners[event] = [];
+ this._listeners[event] = new Map();
}
- this._listeners[event].push(listener);
+ this._listeners[event].set(listener, once);
}
},
- _runListeners: Zotero.Promise.coroutine(function* (event) {
- if (!this._listeners[event]) return;
- var listener;
- while (listener = this._listeners[event].shift()) {
- yield Zotero.Promise.resolve(listener.call(this));
+ _waitForEvent: Zotero.Promise.coroutine(function* (event) {
+ if (event == 'load' && this._initialized) {
+ return;
}
+ return new Zotero.Promise((resolve, reject) => {
+ this._addListener(event, () => resolve(), true);
+ });
}),
@@ -141,7 +179,7 @@ Zotero.LibraryTreeView.prototype = {
},
- onSelect: function () {
+ runSelectListeners: function () {
return this._runListeners('select');
},
diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js
@@ -402,11 +402,11 @@ var ZoteroPane = new function()
// TODO: Remove now that no tab mode?
var containerWindow = window;
if(containerWindow.zoteroSavedCollectionSelection) {
- this.collectionsView.addEventListener('load', Zotero.Promise.coroutine(function* () {
+ this.collectionsView.onLoad.addListener(Zotero.Promise.coroutine(function* () {
yield this.collectionsView.selectByID(containerWindow.zoteroSavedCollectionSelection);
if (containerWindow.zoteroSavedItemSelection) {
- this.itemsView.addEventListener('load', function () {
+ this.itemsView.onLoad.addListener(function () {
this.itemsView.rememberSelection(containerWindow.zoteroSavedItemSelection);
delete containerWindow.zoteroSavedItemSelection;
}.bind(this));
@@ -987,8 +987,7 @@ var ZoteroPane = new function()
var cv = this.collectionsView;
- var deferred = Zotero.Promise.defer();
- cv.addEventListener('select', () => deferred.resolve());
+ var promise = cv.waitForSelect();
var selectedRow = cv.selection.currentIndex;
yield cv.refresh();
@@ -1004,7 +1003,7 @@ var ZoteroPane = new function()
this.collectionsView.selection.selectEventsSuppressed = false;
- return deferred.promise;
+ return promise;
});
@@ -1199,11 +1198,10 @@ var ZoteroPane = new function()
// Wait for existing items view to finish loading before unloading it
//
// TODO: Cancel loading
- let deferred = Zotero.Promise.defer();
- this.itemsView.addEventListener('load', () => deferred.resolve());
- if (deferred.promise.isPending()) {
+ let promise = this.itemsView.waitForLoad();
+ if (promise.isPending()) {
Zotero.debug("Waiting for items view " + this.itemsView.id + " to finish loading");
- yield deferred.promise;
+ yield promise;
}
this.itemsView.unregister();
@@ -1240,7 +1238,7 @@ var ZoteroPane = new function()
Zotero.Prefs.clear('lastViewedFolder');
ZoteroPane_Local.displayErrorMessage();
};
- this.itemsView.addEventListener('load', () => this.setTagScope());
+ this.itemsView.onLoad.addListener(() => this.setTagScope());
if (this.tagSelectorShown()) {
let tagSelector = document.getElementById('zotero-tag-selector')
let handler = function () {
@@ -1250,7 +1248,7 @@ var ZoteroPane = new function()
tagSelector.addEventListener('refresh', handler);
}
else {
- this.itemsView.addEventListener('load', () => Zotero.uiIsReady());
+ this.itemsView.onLoad.addListener(() => Zotero.uiIsReady());
}
// If item data not yet loaded for library, load it now.
@@ -1320,7 +1318,7 @@ var ZoteroPane = new function()
Zotero.Prefs.set('lastViewedFolder', collectionTreeRow.id);
}, this)
.finally(function () {
- return this.collectionsView.onSelect();
+ return this.collectionsView.runListeners('select');
}.bind(this));
};
@@ -1387,12 +1385,9 @@ var ZoteroPane = new function()
// Don't select item until items list has loaded
//
// This avoids an error if New Item is used while the pane is first loading.
- var deferred = Zotero.Promise.defer();
- this.itemsView.addEventListener('load', function () {
- deferred.resolve();
- });
- if (deferred.promise.isPending()) {
- yield deferred.promise;
+ var promise = this.itemsView.waitForLoad();
+ if (promise.isPending()) {
+ yield promise;
}
if (!this.itemsView || !this.itemsView.selection) {
@@ -1598,7 +1593,7 @@ var ZoteroPane = new function()
return true;
}.bind(this))()
.finally(function () {
- return this.itemsView.onSelect();
+ return this.itemsView.runListeners('select');
}.bind(this));
}
@@ -2462,9 +2457,7 @@ var ZoteroPane = new function()
// some menu items (e.g., export/createBib/loadReport) to appear gray in the menu at first and
// then turn black once there are items. Pass a flag to prevent an accidental infinite loop.
if (!collectionTreeRow.isHeader() && !this.itemsView.initialized && !noRepeat) {
- this.itemsView.addEventListener('load', function () {
- this.buildCollectionContextMenu(true);
- }.bind(this));
+ this.itemsView.onLoad.addListener(() => this.buildCollectionContextMenu(true));
}
// Set attributes on the menu from the configuration object
diff --git a/test/content/support.js b/test/content/support.js
@@ -190,15 +190,11 @@ var waitForItemsLoad = Zotero.Promise.coroutine(function* (win, collectionRowToS
var zp = win.ZoteroPane;
var cv = zp.collectionsView;
- var deferred = Zotero.Promise.defer();
- cv.addEventListener('load', () => deferred.resolve());
- yield deferred.promise;
+ yield cv.waitForLoad();
if (collectionRowToSelect !== undefined) {
yield cv.selectWait(collectionRowToSelect);
}
- deferred = Zotero.Promise.defer();
- zp.itemsView.addEventListener('load', () => deferred.resolve());
- return deferred.promise;
+ yield zp.itemsView.waitForLoad();
});
var waitForTagSelector = function (win) {
diff --git a/test/tests/advancedSearchTest.js b/test/tests/advancedSearchTest.js
@@ -35,8 +35,7 @@ describe("Advanced Search", function () {
var deferred = Zotero.Promise.defer();
o.search();
var iv = o.itemsView;
- iv.addEventListener('load', () => deferred.resolve());
- yield deferred.promise;
+ yield iv.waitForLoad();
// Check results
assert.equal(iv.rowCount, 1);
diff --git a/test/tests/itemTreeViewTest.js b/test/tests/itemTreeViewTest.js
@@ -566,8 +566,7 @@ describe("Zotero.ItemTreeView", function() {
let view = zp.itemsView;
yield view.selectItem(item3.id, true);
- var deferred = Zotero.Promise.defer();
- view.addEventListener('select', () => deferred.resolve());
+ var promise = view.waitForSelect();
view.drop(view.getRowIndexByID(item2.id), 0, {
dropEffect: 'copy',
@@ -585,7 +584,7 @@ describe("Zotero.ItemTreeView", function() {
mozItemCount: 1
})
- yield deferred.promise;
+ yield promise;
// Old parent should be empty
assert.isFalse(view.isContainerOpen(view.getRowIndexByID(item1.id)));
@@ -606,8 +605,7 @@ describe("Zotero.ItemTreeView", function() {
let view = zp.itemsView;
yield view.selectItem(item3.id, true);
- var deferred = Zotero.Promise.defer();
- view.addEventListener('select', () => deferred.resolve());
+ var promise = view.waitForSelect();
view.drop(view.getRowIndexByID(item1.id), 0, {
dropEffect: 'copy',
@@ -625,7 +623,7 @@ describe("Zotero.ItemTreeView", function() {
mozItemCount: 1
})
- yield deferred.promise;
+ yield promise;
// Old parent should be empty
assert.isFalse(view.isContainerOpen(view.getRowIndexByID(item2.id)));
@@ -640,8 +638,7 @@ describe("Zotero.ItemTreeView", function() {
var file = getTestDataDirectory();
file.append('test.png');
- var deferred = Zotero.Promise.defer();
- itemsView.addEventListener('select', () => deferred.resolve());
+ var promise = itemsView.waitForSelect();
itemsView.drop(0, -1, {
dropEffect: 'copy',
@@ -659,7 +656,7 @@ describe("Zotero.ItemTreeView", function() {
}
})
- yield deferred.promise;
+ yield promise;
var items = itemsView.getSelectedItems();
var path = yield items[0].getFilePathAsync();
assert.equal(
@@ -669,8 +666,7 @@ describe("Zotero.ItemTreeView", function() {
});
it("should create a top-level attachment when a URL is dragged", function* () {
- var deferred = Zotero.Promise.defer();
- itemsView.addEventListener('select', () => deferred.resolve());
+ var promise = itemsView.waitForSelect();
itemsView.drop(0, -1, {
dropEffect: 'copy',
@@ -688,7 +684,7 @@ describe("Zotero.ItemTreeView", function() {
mozItemCount: 1,
})
- yield deferred.promise;
+ yield promise;
var item = itemsView.getSelectedItems()[0];
assert.equal(item.getField('url'), pdfURL);
assert.equal(
diff --git a/test/tests/relatedboxTest.js b/test/tests/relatedboxTest.js
@@ -35,9 +35,7 @@ describe("Related Box", function () {
yield Zotero.Promise.delay(50);
}
while (!view);
- var deferred = Zotero.Promise.defer();
- view.addEventListener('load', () => deferred.resolve());
- yield deferred.promise;
+ yield view.waitForLoad();
// Select the other item
for (let i = 0; i < view.rowCount; i++) {
