From 94ccba45b9b557275e8e9d0498bd8f5336f8f2e5 Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Fri, 15 Mar 2019 12:46:27 -0400
Subject: [PATCH] Avoid unnecessary tag queries (regression from React tag
 selector)

---
 .../content/zotero/containers/tagSelector.jsx | 32 +++++++---
 test/tests/tagSelectorTest.js                 | 58 +++++++++++++++----
 2 files changed, 72 insertions(+), 18 deletions(-)

diff --git a/chrome/content/zotero/containers/tagSelector.jsx b/chrome/content/zotero/containers/tagSelector.jsx
index 0f21ee1494..040942eaa0 100644
--- a/chrome/content/zotero/containers/tagSelector.jsx
+++ b/chrome/content/zotero/containers/tagSelector.jsx
@@ -68,14 +68,11 @@ Zotero.TagSelector = class TagSelectorContainer extends React.Component {
 				default:
 					return;
 			}
-			return this.setState({tags: await this.getTags()});
 		}
-
-		if (type == 'item' || type == 'item-tag') {
-			if (event == 'redraw') {
-				return;
-			}
-			return this.setState({tags: await this.getTags()});
+		
+		// Ignore item events other than 'trash'
+		if (type == 'item' && event != 'trash') {
+			return;
 		}
 		
 		// If a selected tag no longer exists, deselect it
@@ -93,6 +90,27 @@ Zotero.TagSelector = class TagSelectorContainer extends React.Component {
 			}
 			return;
 		}
+		
+		// TODO: Check libraryID for some events to avoid refreshing unnecessarily on sync changes?
+		
+		var newTags = await this.getTags();
+		
+		if (type == 'item-tag' && event == 'remove') {
+			let changed = false;
+			let visibleTags = newTags.map(tag => tag.tag);
+			for (let id of ids) {
+				let tag = extraData[id].tag;
+				if (this.selectedTags.has(tag) && !visibleTags.includes(tag)) {
+					this.selectedTags.delete(tag);
+					changed = true;
+				}
+			}
+			if (changed && typeof(this.props.onSelection) === 'function') {
+				this.props.onSelection(this.selectedTags);
+			}
+		}
+		
+		return this.setState({tags: newTags});
 	}
 	
 	async getTags(tagsInScope, tagColors) {
diff --git a/test/tests/tagSelectorTest.js b/test/tests/tagSelectorTest.js
index 870ef29a2d..6a1d2b956d 100644
--- a/test/tests/tagSelectorTest.js
+++ b/test/tests/tagSelectorTest.js
@@ -413,37 +413,73 @@ describe("Tag Selector", function () {
 			assert.notInclude(getRegularTags(), "A");
 		});
 		
+		it("should deselect a tag when removed from the last item in this view", async function () {
+			var libraryID = Zotero.Libraries.userLibraryID;
+			await selectLibrary(win);
+			
+			var tag1 = Zotero.Utilities.randomString();
+			var tag2 = Zotero.Utilities.randomString();
+			var promise = waitForTagSelector(win);
+			var item1 = await createDataObject('item', { tags: [{ tag: tag1 }] });
+			var item2 = await createDataObject('item', { tags: [{ tag: tag2 }] });
+			await promise;
+			
+			tagSelector.handleTagSelected(tag1);
+			await waitForTagSelector(win);
+			
+			// Tag selector should show the selected tag
+			assert.include(getRegularTags(), tag1);
+			// And not the unselected one
+			assert.notInclude(getRegularTags(), tag2);
+			
+			// Remove tag from item
+			promise = waitForTagSelector(win);
+			item1.removeTag(tag1);
+			await item1.saveTx();
+			await promise;
+			// Notifier item-tag remove triggers #onSelected, which eventually triggers #onItemViewChanged
+			await waitForTagSelector(win);
+			
+			// Removed tag should no longer be shown or selected
+			assert.notInclude(getRegularTags(), tag1);
+			assert.notInclude(Array.from(tagSelector.getTagSelection()), tag1);
+			// Other tags should be shown again
+			assert.include(getRegularTags(), tag2);
+		});
+		
 		it("should deselect a tag when deleted from a library", async function () {
 			var libraryID = Zotero.Libraries.userLibraryID;
 			await selectLibrary(win);
 			
 			var promise = waitForTagSelector(win);
 			
-			await createDataObject('item', { tags: [{ tag: 'A' }] });
-			await createDataObject('item', { tags: [{ tag: 'B' }] });
+			var tag1 = Zotero.Utilities.randomString();
+			var tag2 = Zotero.Utilities.randomString();
+			await createDataObject('item', { tags: [{ tag: tag1 }] });
+			await createDataObject('item', { tags: [{ tag: tag2 }] });
 			await promise;
 			
-			tagSelector.handleTagSelected('A');
+			tagSelector.handleTagSelected(tag1);
 			await waitForTagSelector(win);
 			
 			// Tag selector should show the selected tag
-			assert.include(getRegularTags(), 'A');
+			assert.include(getRegularTags(), tag1);
 			// And not the unselected one
-			assert.notInclude(getRegularTags(), 'B');
+			assert.notInclude(getRegularTags(), tag2);
 			
 			// Remove tag from library
 			promise = waitForTagSelector(win);
-			await Zotero.Tags.removeFromLibrary(libraryID, Zotero.Tags.getID('A'));
-			// notify item-tag remove
+			await Zotero.Tags.removeFromLibrary(libraryID, Zotero.Tags.getID(tag1));
+			// Notifier item-tag remove
 			await promise;
-			// notify tag delete which triggers #onSelected, which eventually triggers #onItemViewChanged
+			// Notifier tag delete triggers #onSelected, which eventually triggers #onItemViewChanged
 			await waitForTagSelector(win);
 			
 			// Deleted tag should no longer be shown or selected
-			assert.notInclude(getRegularTags(), 'A');
-			assert.notInclude(Array.from(tagSelector.getTagSelection()), 'A');
+			assert.notInclude(getRegularTags(), tag1);
+			assert.notInclude(Array.from(tagSelector.getTagSelection()), tag1);
 			// Other tags should be shown again
-			assert.include(getRegularTags(), 'B');
+			assert.include(getRegularTags(), tag2);
 		});
 	});