commit 2bd246e2eaf83e43baa064d0d52ba779c7f99c92
parent cbbdebc5b70fd1bc24cf4ba939ffae8d08ebd017
Author: Dan Stillman <dstillman@zotero.org>
Date: Mon, 25 May 2015 21:22:43 -0400
Fixes #728, Tag selector refreshing
Diffstat:
7 files changed, 268 insertions(+), 52 deletions(-)
diff --git a/chrome/content/zotero/bindings/tagselector.xml b/chrome/content/zotero/bindings/tagselector.xml
@@ -132,7 +132,7 @@
if (!this._scope[tag.tag]) {
this._scope[tag.tag] = [];
}
- this._scope[tag.tag].push(tag.type);
+ this._scope[tag.tag].push(tag.type || 0);
}
}
else {
@@ -174,7 +174,11 @@
<![CDATA[
this._initialized = true;
this.selection = {};
- this._notifierID = Zotero.Notifier.registerObserver(this, ['collection-item', 'item-tag', 'tag', 'setting'], 'tagSelector');
+ this._notifierID = Zotero.Notifier.registerObserver(
+ this,
+ ['collection-item', 'item', 'item-tag', 'tag', 'setting'],
+ 'tagSelector'
+ );
]]>
</body>
</method>
@@ -455,12 +459,32 @@
this.id('no-tags-deck').selectedIndex = 1;
Zotero.debug("Loaded tag selector in " + (new Date - t) + " ms");
+
+ var event = new Event('refresh');
+ this.dispatchEvent(event);
}, this);
]]>
</body>
</method>
+ <method name="getVisible">
+ <body><![CDATA[
+ var tagsBox = this.id('tags-toggle');
+ var labels = tagsBox.getElementsByTagName('label');
+ var visible = [];
+ for (let i = 0; i < labels.length; i++){
+ let label = labels[i];
+ if (label.getAttribute('hidden') != 'true'
+ && label.getAttribute('inScope') == 'true') {
+ visible.push(label.value);
+ }
+ }
+ return visible;
+ ]]></body>
+ </method>
+
+
<method name="getNumSelected">
<body>
<![CDATA[
@@ -522,10 +546,15 @@
}
}
+ // Ignore item events other than 'trash'
+ if (type == 'item' && event != 'trash') {
+ return;
+ }
+
var selectionChanged = false;
// If a selected tag no longer exists, deselect it
- if (event == 'delete' || event == 'modify') {
+ if (event == 'delete' || event == 'trash' || event == 'modify') {
// TODO: necessary, or just use notifier value?
this._tags = yield Zotero.Tags.getAll(this.libraryID, this._types);
diff --git a/chrome/content/zotero/xpcom/collectionTreeView.js b/chrome/content/zotero/xpcom/collectionTreeView.js
@@ -53,7 +53,8 @@ Zotero.CollectionTreeView = function()
'trash',
'bucket'
],
- 'collectionTreeView'
+ 'collectionTreeView',
+ 25
);
this._containerState = {};
this._duplicateLibraries = [];
@@ -2091,7 +2092,7 @@ Zotero.CollectionTreeCache = {
"lastSearch":null,
"lastResults":null,
- "clear": Zotero.Promise.coroutine(function* () {
+ "clear": function () {
this.lastTreeRow = null;
this.lastSearch = null;
if(this.lastTempTable) {
@@ -2102,7 +2103,7 @@ Zotero.CollectionTreeCache = {
}
this.lastTempTable = null;
this.lastResults = null;
- })
+ }
};
Zotero.CollectionTreeRow = function(type, ref, level, isOpen)
@@ -2306,8 +2307,8 @@ Zotero.CollectionTreeRow.prototype.getItems = Zotero.Promise.coroutine(function*
});
Zotero.CollectionTreeRow.prototype.getSearchResults = Zotero.Promise.coroutine(function* (asTempTable) {
- if(Zotero.CollectionTreeCache.lastTreeRow !== this) {
- yield Zotero.CollectionTreeCache.clear();
+ if (Zotero.CollectionTreeCache.lastTreeRow && Zotero.CollectionTreeCache.lastTreeRow !== this) {
+ Zotero.CollectionTreeCache.clear();
}
if(!Zotero.CollectionTreeCache.lastResults) {
@@ -2332,7 +2333,7 @@ Zotero.CollectionTreeRow.prototype.getSearchResults = Zotero.Promise.coroutine(f
*/
Zotero.CollectionTreeRow.prototype.getSearchObject = Zotero.Promise.coroutine(function* () {
if(Zotero.CollectionTreeCache.lastTreeRow !== this) {
- yield Zotero.CollectionTreeCache.clear();
+ Zotero.CollectionTreeCache.clear();
}
if(Zotero.CollectionTreeCache.lastSearch) {
@@ -2417,15 +2418,15 @@ Zotero.CollectionTreeRow.prototype.getChildTags = Zotero.Promise.coroutine(funct
});
-Zotero.CollectionTreeRow.prototype.setSearch = Zotero.Promise.coroutine(function* (searchText) {
- yield Zotero.CollectionTreeCache.clear();
+Zotero.CollectionTreeRow.prototype.setSearch = function (searchText) {
+ Zotero.CollectionTreeCache.clear();
this.searchText = searchText;
-});
+}
-Zotero.CollectionTreeRow.prototype.setTags = Zotero.Promise.coroutine(function* (tags) {
- yield Zotero.CollectionTreeCache.clear();
+Zotero.CollectionTreeRow.prototype.setTags = function (tags) {
+ Zotero.CollectionTreeCache.clear();
this.tags = tags;
-});
+}
/*
* Returns TRUE if saved search, quicksearch or tag filter
diff --git a/chrome/content/zotero/xpcom/itemTreeView.js b/chrome/content/zotero/xpcom/itemTreeView.js
@@ -54,7 +54,10 @@ Zotero.ItemTreeView = function (collectionTreeRow, sourcesOnly) {
this._refreshPromise = Zotero.Promise.resolve();
this._unregisterID = Zotero.Notifier.registerObserver(
- this, ['item', 'collection-item', 'item-tag', 'share-items', 'bucket'], 'itemTreeView'
+ this,
+ ['item', 'collection-item', 'item-tag', 'share-items', 'bucket'],
+ 'itemTreeView',
+ 50
);
}
@@ -277,7 +280,7 @@ Zotero.ItemTreeView.prototype.setTree = Zotero.serial(Zotero.Promise.coroutine(f
* (doesn't call the tree.invalidate methods, etc.)
*/
Zotero.ItemTreeView.prototype.refresh = Zotero.serial(Zotero.Promise.coroutine(function* () {
- Zotero.debug('Refreshing items list');
+ Zotero.debug('Refreshing items list for ' + this.id);
//if(!Zotero.ItemTreeView._haveCachedFields) yield Zotero.Promise.resolve();
var cacheFields = ['title', 'date'];
@@ -455,6 +458,7 @@ Zotero.ItemTreeView.prototype.notify = Zotero.Promise.coroutine(function* (actio
var collectionTreeRow = this.collectionTreeRow;
var madeChanges = false;
+ var refreshed = false;
var sort = false;
var savedSelection = this.getSelectedItems(true);
@@ -495,16 +499,19 @@ Zotero.ItemTreeView.prototype.notify = Zotero.Promise.coroutine(function* (actio
if (type == 'share-items') {
if (collectionTreeRow.isShare()) {
yield this.refresh();
+ refreshed = true;
}
}
else if (type == 'bucket') {
if (collectionTreeRow.isBucket()) {
yield this.refresh();
+ refreshed = true;
}
}
else if (type == 'publications') {
if (collectionTreeRow.isPublications()) {
yield this.refresh();
+ refreshed = true;
}
}
// If refreshing a single item, clear caches and then unselect and reselect row
@@ -567,6 +574,7 @@ Zotero.ItemTreeView.prototype.notify = Zotero.Promise.coroutine(function* (actio
if (collectionTreeRow.isDuplicates()) {
previousRow = this._rowMap[ids[0]];
yield this.refresh();
+ refreshed = true;
madeChanges = true;
sort = true;
}
@@ -630,6 +638,7 @@ Zotero.ItemTreeView.prototype.notify = Zotero.Promise.coroutine(function* (actio
if (collectionTreeRow.isTrash() || collectionTreeRow.isSearch())
{
yield this.refresh();
+ refreshed = true;
madeChanges = true;
sort = true;
}
@@ -741,6 +750,7 @@ Zotero.ItemTreeView.prototype.notify = Zotero.Promise.coroutine(function* (actio
// In some modes, just re-run search
if (collectionTreeRow.isSearch() || collectionTreeRow.isTrash() || collectionTreeRow.isUnfiled()) {
yield this.refresh();
+ refreshed = true;
madeChanges = true;
sort = true;
}
@@ -792,6 +802,11 @@ Zotero.ItemTreeView.prototype.notify = Zotero.Promise.coroutine(function* (actio
if(madeChanges)
{
+ // If we made individual changes, we have to clear the cache
+ if (!refreshed) {
+ Zotero.CollectionTreeCache.clear();
+ }
+
var singleSelect = false;
// If adding a single top-level item and this is the active window, select it
if (action == 'add' && activeWindow) {
@@ -1819,7 +1834,6 @@ Zotero.ItemTreeView.prototype.deleteSelection = Zotero.Promise.coroutine(functio
ids.push(this.getRow(j).id);
}
- Zotero.CollectionTreeCache.clear();
var collectionTreeRow = this.collectionTreeRow;
if (collectionTreeRow.isBucket()) {
@@ -1867,8 +1881,6 @@ Zotero.ItemTreeView.prototype.setFilter = Zotero.Promise.coroutine(function* (ty
//this._treebox.endUpdateBatch();
this.selection.selectEventsSuppressed = false;
-
- yield this._runListeners('load');
});
diff --git a/chrome/content/zotero/xpcom/notifier.js b/chrome/content/zotero/xpcom/notifier.js
@@ -23,6 +23,8 @@
***** END LICENSE BLOCK *****
*/
+"use strict";
+
Zotero.Notifier = new function(){
var _observers = {};
var _disabled = false;
@@ -35,9 +37,6 @@ Zotero.Notifier = new function(){
var _locked = false;
var _queue = [];
- this.registerObserver = registerObserver;
- this.unregisterObserver = unregisterObserver;
- this.untrigger = untrigger;
this.begin = begin;
this.reset = reset;
this.disable = disable;
@@ -45,13 +44,13 @@ Zotero.Notifier = new function(){
this.isEnabled = isEnabled;
- function registerObserver(ref, types, id) {
+ this.registerObserver = function (ref, types, id, priority) {
if (types){
types = Zotero.flattenArguments(types);
for (var i=0; i<types.length; i++){
if (_types.indexOf(types[i]) == -1){
- throw new Error('Invalid type ' + types[i] + ' in registerObserver()');
+ throw new Error("Invalid type '" + types[i] + "'");
}
}
}
@@ -70,16 +69,23 @@ Zotero.Notifier = new function(){
}
while (_observers[hash]);
- Zotero.debug('Registering observer for '
- + (types ? '[' + types.join() + ']' : 'all types')
- + ' in notifier with hash ' + hash + "'", 4);
- _observers[hash] = {ref: ref, types: types};
+ var msg = "Registering observer '" + hash + "' for "
+ + (types ? '[' + types.join() + ']' : 'all types');
+ if (priority) {
+ msg += " with priority " + priority;
+ }
+ Zotero.debug(msg);
+ _observers[hash] = {
+ ref: ref,
+ types: types,
+ priority: priority || false
+ };
return hash;
}
- function unregisterObserver(hash){
- Zotero.debug("Unregistering observer in notifier with hash '" + hash + "'", 4);
- delete _observers[hash];
+ this.unregisterObserver = function (id) {
+ Zotero.debug("Unregistering observer in notifier with id '" + id + "'", 4);
+ delete _observers[id];
}
/**
@@ -116,7 +122,7 @@ Zotero.Notifier = new function(){
Zotero.debug("Notifier.trigger('" + event + "', '" + type + "', " + '[' + ids.join() + '], ' + extraData + ')'
+ (queue ? " queued" : " called " + "[observers: " + Object.keys(_observers).length + "]"));
if (extraData) {
- Zotero.debug("EXTRA DATA:");
+ Zotero.debug("Extra data:");
Zotero.debug(extraData);
}
@@ -138,7 +144,6 @@ Zotero.Notifier = new function(){
// Merge extraData keys
if (extraData) {
- Zotero.debug("ADDING EXTRA DATA");
// If just a single id, extra data can be keyed by id or passed directly
if (ids.length == 1) {
let id = ids[0];
@@ -159,25 +164,24 @@ Zotero.Notifier = new function(){
return true;
}
- for (var i in _observers){
- Zotero.debug("Calling notify() with " + event + "/" + type + " on observer with hash '" + i + "'", 4);
+ var order = _getObserverOrder(type);
+ for (let id of order) {
+ Zotero.debug("Calling notify() with " + event + "/" + type
+ + " on observer with id '" + id + "'", 4);
- if (!_observers[i]) {
+ if (!_observers[id]) {
Zotero.debug("Observer no longer exists");
continue;
}
- // Find observers that handle notifications for this type (or all types)
- if (!_observers[i].types || _observers[i].types.indexOf(type)!=-1){
- // Catch exceptions so all observers get notified even if
- // one throws an error
- try {
- yield Zotero.Promise.resolve(_observers[i].ref.notify(event, type, ids, extraData));
- }
- catch (e) {
- Zotero.debug(e);
- Components.utils.reportError(e);
- }
+ // Catch exceptions so all observers get notified even if
+ // one throws an error
+ try {
+ yield Zotero.Promise.resolve(_observers[id].ref.notify(event, type, ids, extraData));
+ }
+ catch (e) {
+ Zotero.debug(e);
+ Components.utils.reportError(e);
}
}
@@ -185,7 +189,33 @@ Zotero.Notifier = new function(){
});
- function untrigger(event, type, ids) {
+ /**
+ * Get order of observer by priority, with lower numbers having higher priority.
+ * If an observer doesn't have a priority, sort it last.
+ */
+ function _getObserverOrder(type) {
+ var order = [];
+ for (let i in _observers) {
+ // Skip observers that don't handle notifications for this type (or all types)
+ if (_observers[i].types && _observers[i].types.indexOf(type) == -1) {
+ continue;
+ }
+ order.push({
+ id: i,
+ priority: _observers[i].priority || false
+ });
+ }
+ order.sort((a, b) => {
+ if (a.priority === false && b.priority === false) return 0;
+ if (a.priority === false) return 1;
+ if (b.priority === false) return -1;
+ return a.priority - b.priority;
+ });
+ return order.map(o => o.id);
+ }
+
+
+ this.untrigger = function (event, type, ids) {
if (!_inTransaction) {
throw ("Zotero.Notifier.untrigger() called with no active event queue")
}
diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js
@@ -1128,8 +1128,8 @@ var ZoteroPane = new function()
});
if (deferred.promise.isPending()) {
Zotero.debug("Waiting for items view " + this.itemsView.id + " to finish loading");
+ yield deferred.promise;
}
- yield deferred.promise;
this.itemsView.unregister();
document.getElementById('zotero-items-tree').view = this.itemsView = null;
diff --git a/test/tests/collectionTreeViewTest.js b/test/tests/collectionTreeViewTest.js
@@ -3,7 +3,6 @@
describe("Zotero.CollectionTreeView", function() {
var win, collectionsView;
- // Load Zotero pane and select library
before(function* () {
win = yield loadZoteroPane();
collectionsView = win.ZoteroPane.collectionsView;
@@ -140,13 +139,13 @@ describe("Zotero.CollectionTreeView", function() {
var collection = new Zotero.Collection;
collection.name = "Test";
var collectionID = yield collection.saveTx();
- var cv = win.ZoteroPane.collectionsView;
var search = new Zotero.Search;
search.name = "A Test Search";
search.addCondition('title', 'contains', 'test');
var searchID = yield search.saveTx();
+ var cv = win.ZoteroPane.collectionsView;
var collectionRow = cv._rowMap["C" + collectionID];
var searchRow = cv._rowMap["S" + searchID];
var duplicatesRow = cv._rowMap["D" + Zotero.Libraries.userLibraryID];
diff --git a/test/tests/tagSelectorTest.js b/test/tests/tagSelectorTest.js
@@ -0,0 +1,145 @@
+"use strict";
+
+describe("Tag Selector", function () {
+ var win, doc, collectionsView;
+
+ before(function* () {
+ win = yield loadZoteroPane();
+ doc = win.document;
+ collectionsView = win.ZoteroPane.collectionsView;
+
+ // Wait for things to settle
+ yield Zotero.Promise.delay(100);
+ });
+ after(function () {
+ win.close();
+ });
+
+ function waitForTagSelector() {
+ var deferred = Zotero.Promise.defer();
+ var tagSelector = doc.getElementById('zotero-tag-selector');
+ var onRefresh = function (event) {
+ tagSelector.removeEventListener('refresh', onRefresh);
+ deferred.resolve();
+ }
+ tagSelector.addEventListener('refresh', onRefresh);
+ return deferred.promise;
+ }
+
+ describe("#notify()", function () {
+ it("should add a tag when added to an item in the current view", function* () {
+ var promise, tagSelector;
+
+ if (collectionsView.selection.currentIndex != 0) {
+ promise = waitForTagSelector();
+ yield collectionsView.selectLibrary();
+ yield promise;
+ }
+
+ // Add item with tag to library root
+ var item = createUnsavedDataObject('item');
+ item.setTags([
+ {
+ tag: 'A'
+ }
+ ]);
+ promise = waitForTagSelector();
+ yield item.saveTx();
+ yield promise;
+
+ // Tag selector should have at least one tag
+ tagSelector = doc.getElementById('zotero-tag-selector');
+ assert.isAbove(tagSelector.getVisible().length, 0);
+
+ // Add collection
+ promise = waitForTagSelector();
+ var collection = yield createDataObject('collection');
+ yield promise;
+
+ // Tag selector should be empty in new collection
+ tagSelector = doc.getElementById('zotero-tag-selector');
+ assert.equal(tagSelector.getVisible().length, 0);
+
+ // Add item with tag to collection
+ var item = createUnsavedDataObject('item');
+ item.setTags([
+ {
+ tag: 'B'
+ }
+ ]);
+ item.setCollections([collection.id]);
+ promise = waitForTagSelector()
+ yield item.saveTx();
+ yield promise;
+
+ // Tag selector should show the new item's tag
+ tagSelector = doc.getElementById('zotero-tag-selector');
+ assert.equal(tagSelector.getVisible().length, 1);
+ })
+
+ it("should remove a tag when an item is removed from a collection", function* () {
+ // Add collection
+ var promise = waitForTagSelector();
+ var collection = yield createDataObject('collection');
+ yield promise;
+
+ // Add item with tag to collection
+ var item = createUnsavedDataObject('item');
+ item.setTags([
+ {
+ tag: 'A'
+ }
+ ]);
+ item.setCollections([collection.id]);
+ promise = waitForTagSelector()
+ yield item.saveTx();
+ yield promise;
+
+ // Tag selector should show the new item's tag
+ var tagSelector = doc.getElementById('zotero-tag-selector');
+ assert.equal(tagSelector.getVisible().length, 1);
+
+ item.setCollections();
+ promise = waitForTagSelector();
+ yield item.saveTx();
+ yield promise;
+
+ // Tag selector shouldn't show the removed item's tag
+ tagSelector = doc.getElementById('zotero-tag-selector');
+ assert.equal(tagSelector.getVisible().length, 0);
+ })
+
+ it("should remove a tag when an item in a collection is moved to the trash", function* () {
+ // Add collection
+ var promise = waitForTagSelector();
+ var collection = yield createDataObject('collection');
+ yield promise;
+
+ // Add item with tag to collection
+ var item = createUnsavedDataObject('item');
+ item.setTags([
+ {
+ tag: 'A'
+ }
+ ]);
+ item.setCollections([collection.id]);
+ promise = waitForTagSelector()
+ yield item.saveTx();
+ yield promise;
+
+ // Tag selector should show the new item's tag
+ var tagSelector = doc.getElementById('zotero-tag-selector');
+ assert.equal(tagSelector.getVisible().length, 1);
+
+ // Move item to trash
+ item.deleted = true;
+ promise = waitForTagSelector();
+ yield item.saveTx();
+ yield promise;
+
+ // Tag selector shouldn't show the deleted item's tag
+ tagSelector = doc.getElementById('zotero-tag-selector');
+ assert.equal(tagSelector.getVisible().length, 0);
+ })
+ })
+})
