Don't reload collections list when sources are modified

Just update the row that changed, moving it if necessary.

chrome/content/zotero/xpcom/collectionTreeView.js

@@ -287,20 +287,30 @@ Zotero.CollectionTreeView.prototype.notify = Zotero.Promise.coroutine(function*
 		return;
 	}
 	
-	if (action == 'refresh' && type == 'trash') {
+	//
-		// libraryID is passed as parameter to 'refresh'
+	// Actions that don't change the selection
-		let deleted = yield Zotero.Items.getDeleted(ids[0], true);
+	//
-		this._trashNotEmpty[ids[0]] = !!deleted.length;
-		return;
-	}
-	
 	if (action == 'redraw') {
 		this._treebox.invalidate();
 		return;
 	}
+	if (action == 'refresh') {
+		// If trash is refreshed, we probably need to update the icon from full to empty
+		if (type == 'trash') {
+			// libraryID is passed as parameter to 'refresh'
+			let deleted = yield Zotero.Items.getDeleted(ids[0], true);
+			this._trashNotEmpty[ids[0]] = !!deleted.length;
+			let row = this.getRowIndexByID("T" + ids[0]);
+			this._treebox.invalidateRow(row);
+		}
+		return;
+	}
 	
+	//
+	// Actions that can change the selection
+	//
+	var currentTreeRow = this.getRow(this.selection.currentIndex);
 	this.selection.selectEventsSuppressed = true;
-	var savedSelection = this.saveSelection();
 	
 	if (action == 'delete') {
 		var selectedIndex = this.selection.count ? this.selection.currentIndex : 0;
@@ -356,25 +366,45 @@ Zotero.CollectionTreeView.prototype.notify = Zotero.Promise.coroutine(function*
 	else if(action == 'move')
 	{
 		yield this.reload();
+		yield this.restoreSelection(currentTreeRow);
+	}
+	else if (action == 'modify') {
+		let row;
+		let id = ids[0];
 		
-		// Open the new parent collection if closed
+		switch (type) {
-		for (var i=0; i<ids.length; i++) {
+		case 'collection':
-			var collection = yield Zotero.Collections.getAsync(ids[i]);
+			row = this.getRowIndexByID("C" + id);
-			var parentID = collection.parentID;
+			if (row !== false) {
-			if (parentID && this._rowMap["C" + parentID] &&
+				// TODO: Only move if name changed
-					!this.isContainerOpen(this._rowMap["C" + parentID])) {
+				let reopen = this.isContainerOpen(row);
-				yield this.toggleOpenState(this._rowMap["C" + parentID]);
+				if (reopen) {
+					this._closeContainer(row);
 				}
+				this._removeRow(row);
+				yield this._addSortedRow('collection', id);
+				if (reopen) {
+					yield this.toggleOpenState(row);
 				}
+				yield this.restoreSelection(currentTreeRow);
+			}
+			break;
 		
-		this.rememberSelection(savedSelection);
+		case 'search':
+			row = this.getRowIndexByID("S" + id);
+			if (row !== false) {
+				// TODO: Only move if name changed
+				this._removeRow(row);
+				yield this._addSortedRow('search', id);
+				yield this.restoreSelection(currentTreeRow);
 			}
-	else if (action == 'modify' || action == 'refresh') {
+			break;
-		if (type != 'bucket'
+		
-				&& (type != 'publications' || this.selectedTreeRow.isPublications())) {
+		default:
 			yield this.reload();
+			yield this.restoreSelection(currentTreeRow);
+			break;
 		}
-		this.rememberSelection(savedSelection);
 	}
 	else if(action == 'add')
 	{
@@ -388,11 +418,6 @@ Zotero.CollectionTreeView.prototype.notify = Zotero.Promise.coroutine(function*
 			case 'search':
 				yield this._addSortedRow(type, id);
 				
-				if (Zotero.suppressUIUpdates) {
-					this.rememberSelection(savedSelection);
-					break;
-				}
-				
 				if (selectRow) {
 					if (type == 'collection') {
 						yield this.selectCollection(id);
@@ -407,7 +432,8 @@ Zotero.CollectionTreeView.prototype.notify = Zotero.Promise.coroutine(function*
 			case 'group':
 				yield this.reload();
 				// Groups can only be created during sync
-				this.rememberSelection(savedSelection);
+				let libraryID = Zotero.Groups.getLibraryIDFromGroupID(id);
+				yield this.selectByID("L" + libraryID);
 				break;
 		}
 	}
@@ -422,6 +448,10 @@ Zotero.CollectionTreeView.prototype.notify = Zotero.Promise.coroutine(function*
  * Add a row in the appropriate place
  *
  * This only adds a row if it would be visible without opening any containers
+ *
+ * @param {String} objectType
+ * @param {Integer} id
+ * @return {Integer|false} - Index at which the row was added, or false if it wasn't added
  */
 Zotero.CollectionTreeView.prototype._addSortedRow = Zotero.Promise.coroutine(function* (objectType, id) {
 	let beforeRow;
@@ -456,6 +486,7 @@ Zotero.CollectionTreeView.prototype._addSortedRow = Zotero.Promise.coroutine(fun
 		else {
 			// Get all collections at the same level that don't have a different parent
 			startRow++;
+			loop:
 			for (let i = startRow; i < this.rowCount; i++) {
 				let treeRow = this.getRow(i);
 				beforeRow = i;
@@ -474,8 +505,8 @@ Zotero.CollectionTreeView.prototype._addSortedRow = Zotero.Promise.coroutine(fun
 					// Fast forward through subcollections
 					while (rowLevel > level) {
 						beforeRow = ++i;
-						if (i == this.rowCount) {
+						if (i == this.rowCount || !this.getRow(i).isCollection()) {
-							break;
+							break loop;
 						}
 						treeRow = this.getRow(i);
 						rowLevel = this.getLevel(i);
@@ -533,7 +564,7 @@ Zotero.CollectionTreeView.prototype._addSortedRow = Zotero.Promise.coroutine(fun
 			beforeRow
 		);
 	}
-	return true;
+	return beforeRow;
 });
 
 
@@ -763,7 +794,7 @@ Zotero.CollectionTreeView.prototype.setCellText = function (row, col, val) {
 	}
 	var treeRow = this.getRow(row);
 	treeRow.ref.name = val;
-	treeRow.ref.save();
+	treeRow.ref.saveTx();
 }
 
 
@@ -877,6 +908,13 @@ Zotero.CollectionTreeView.prototype.selectByID = Zotero.Promise.coroutine(functi
 });
 
 
+Zotero.CollectionTreeView.prototype.restoreSelection = Zotero.Promise.coroutine(function* (collectionTreeRow) {
+	yield this.selectByID(collectionTreeRow.id);
+	// Swap back in the previous tree row to avoid reselection and subsequent items view refresh
+	this._rows[this.selection.currentIndex] = collectionTreeRow;
+})
+
+
 /**
  * @param	{Integer}		libraryID		Library to select
  */
@@ -1172,37 +1210,6 @@ Zotero.CollectionTreeView.prototype.getSelectedCollection = function(asID) {
 }
 
 
-/*
- *  Saves the ids of the currently selected item for later
- */
-Zotero.CollectionTreeView.prototype.saveSelection = function()
-{
-	for (var i=0, len=this.rowCount; i<len; i++) {
-		if (this.selection.isSelected(i)) {
-			var treeRow = this.getRow(i);
-			var id = treeRow.id;
-			if (id) {
-				return id;
-			}
-			else {
-				break;
-			}
-		}
-	}
-	return false;
-}
-
-/*
- *  Sets the selection based on saved selection ids (see above)
- */
-Zotero.CollectionTreeView.prototype.rememberSelection = Zotero.Promise.coroutine(function* (selection)
-{
-	if (selection && this._rowMap[selection] != 'undefined') {
-		this.selection.select(this._rowMap[selection]);
-	}
-});
-
-
 /**
  * Creates mapping of item group ids to tree rows
  */

chrome/content/zotero/xpcom/libraryTreeView.js

@@ -138,7 +138,7 @@ Zotero.LibraryTreeView.prototype = {
 		
 		var lastRow = row == this.rowCount - 1;
 		if (lastRow && this.selection.isSelected(row)) {
-			// Deslect removed row
+			// Deselect removed row
 			this.selection.toggleSelect(row);
 			// If no other rows selected, select row before
 			if (this.selection.count == 0 && row !== 0) {

chrome/content/zotero/zoteroPane.js

@@ -1282,6 +1282,10 @@ var ZoteroPane = new function()
 			}
 			
 			var collectionTreeRow = this.getCollectionTreeRow();
+			// I don't think this happens in normal usage, but it can happen during tests
+			if (!collectionTreeRow) {
+				return false;
+			}
 			
 			// Single item selected
 			if (this.itemsView.selection.count == 1 && this.itemsView.selection.currentIndex != -1)

test/tests/collectionTreeViewTest.js

@@ -1,11 +1,12 @@
 "use strict";
 
 describe("Zotero.CollectionTreeView", function() {
-	var win, cv;
+	var win, zp, cv;
 	
 	before(function* () {
 		win = yield loadZoteroPane();
-		cv = win.ZoteroPane.collectionsView;
+		zp = win.ZoteroPane;
+		cv = zp.collectionsView;
 	});
 	beforeEach(function () {
 		// TODO: Add a selectCollection() function and select a collection instead?
@@ -137,7 +138,7 @@ describe("Zotero.CollectionTreeView", function() {
 			assert.equal(cv.getSelectedLibraryID(), Zotero.Libraries.userLibraryID);
 		});
 		
-		it("should reselect a selected modified collection", function* () {
+		it("should maintain selection on a selected modified collection", function* () {
 			// Create collection
 			var collection = new Zotero.Collection;
 			collection.name = "Reselect on modify";
@@ -154,6 +155,56 @@ describe("Zotero.CollectionTreeView", function() {
 			assert.equal(selected, id);
 		});
 		
+		it("should re-sort a modified collection", function* () {
+			var prefix = Zotero.Utilities.randomString() + " ";
+			var collectionA = yield createDataObject('collection', { name: prefix + "A" });
+			var collectionB = yield createDataObject('collection', { name: prefix + "B" });
+			
+			var aRow = cv.getRowIndexByID("C" + collectionA.id);
+			var aRowOriginal = aRow;
+			var bRow = cv.getRowIndexByID("C" + collectionB.id);
+			assert.equal(bRow, aRow + 1);
+			
+			collectionA.name = prefix + "C";
+			yield collectionA.saveTx();
+			
+			var aRow = cv.getRowIndexByID("C" + collectionA.id);
+			var bRow = cv.getRowIndexByID("C" + collectionB.id);
+			assert.equal(bRow, aRowOriginal);
+			assert.equal(aRow, bRow + 1);
+		})
+		
+		it("should re-sort a modified search", function* () {
+			var prefix = Zotero.Utilities.randomString() + " ";
+			var searchA = yield createDataObject('search', { name: prefix + "A" });
+			var searchB = yield createDataObject('search', { name: prefix + "B" });
+			
+			var aRow = cv.getRowIndexByID("S" + searchA.id);
+			var aRowOriginal = aRow;
+			var bRow = cv.getRowIndexByID("S" + searchB.id);
+			assert.equal(bRow, aRow + 1);
+			
+			searchA.name = prefix + "C";
+			yield searchA.saveTx();
+			
+			var aRow = cv.getRowIndexByID("S" + searchA.id);
+			var bRow = cv.getRowIndexByID("S" + searchB.id);
+			assert.equal(bRow, aRowOriginal);
+			assert.equal(aRow, bRow + 1);
+		})
+		
+		it("shouldn't refresh the items list when a collection is modified", function* () {
+			var collection = yield createDataObject('collection');
+			yield waitForItemsLoad(win);
+			var itemsView = zp.itemsView;
+			
+			collection.name = "New Name";
+			yield collection.saveTx();
+			
+			yield waitForItemsLoad(win);
+			assert.equal(zp.itemsView, itemsView);
+		})
+		
 		it("should add a saved search after collections", function* () {
 			var collection = new Zotero.Collection;
 			collection.name = "Test";