From ecab0e578547d1ff0eea01182620b22c6e0b2a7c Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Mon, 25 Dec 2006 07:06:46 +0000 Subject: [PATCH] Closes #470, Add tag add/modification/delete targets to Notifier Closes #471, Tag selector should update when tags are added/removed Tag Selector overhaul: - Right-click to rename/delete tags globally - Filter tags to only those associated with currently visible items, with a Display All checkbox to show others in gray -- scope list set via new callback mechanism in the items tree - Drag and drop items onto tags to batch assign - Tag Notifier events, currently unused (tag selector currently just refreshes on all item events, since doing granular tag updates is considerably more complicated) - Performance improvements, offset by the new features that make it slower There should probably be an option to use either an ANY or an ALL search in the tag selector... (It's ALL by default now.) New methods: - Zotero.hasValues(obj) -- return true if an object (/associative array) has at least one value, false if not - Zotero.Item.addTagByID() - Zotero.Item.hasTag() - Zotero.Tags.getAllWithinSearch(search) - Zotero.Tags.rename(tagID, tag) - Zotero.Tags.remove(tagID) - ItemTreeView.addCallback() - ItemTreeView.setFilter('search'|'tags', data) -- replaces searchText() - CollectionTreeView.getSearchObject() -- search object used to generate the items list - CollectionTreeView.getChildTags() --- .../content/zotero/bindings/tagselector.xml | 364 +++++++++++++++--- chrome/content/zotero/overlay.js | 47 ++- chrome/content/zotero/selectItemsDialog.js | 2 +- .../zotero/xpcom/collectionTreeView.js | 25 +- chrome/content/zotero/xpcom/data_access.js | 127 +++++- chrome/content/zotero/xpcom/itemTreeView.js | 39 +- chrome/content/zotero/xpcom/notifier.js | 19 +- chrome/content/zotero/xpcom/zotero.js | 13 + chrome/locale/en-US/zotero/zotero.dtd | 3 + chrome/locale/en-US/zotero/zotero.properties | 5 + .../default/zotero/bindings/tagselector.css | 38 +- 11 files changed, 583 insertions(+), 99 deletions(-) diff --git a/chrome/content/zotero/bindings/tagselector.xml b/chrome/content/zotero/bindings/tagselector.xml index 6f8cd52821..1f5b6b7f62 100644 --- a/chrome/content/zotero/bindings/tagselector.xml +++ b/chrome/content/zotero/bindings/tagselector.xml @@ -33,117 +33,285 @@ + false + false + null + null + false + null + + + + + + + + + false + null + + + + + + + + + + + + + + + + - + - - + + - + + + + + + + + + + + + + + + + // If not in filter, hide + if (this._hasFilter && !this._filter[tagID]) { + //Zotero.debug(1); + labels[i].setAttribute('hidden', true); + } + else if (this.filterToScope) { + if (this._hasScope && this.scope[tagID]) { + //Zotero.debug(2); + labels[i].setAttribute('inScope', true); + labels[i].setAttribute('hidden', false); + } + else { + //Zotero.debug(3); + labels[i].setAttribute('hidden', true); + labels[i].setAttribute('inScope', false); + } + } + // Display all + else { + if (this._hasScope && this.scope[tagID]) { + //Zotero.debug(4); + labels[i].setAttribute('inScope', true); + } + else { + //Zotero.debug(5); + labels[i].setAttribute('inScope', false); + } + + labels[i].setAttribute('hidden', false); + } + } + ]]> + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -197,14 +459,22 @@ + + + + + + + + oncommand="this.parentNode.parentNode.parentNode.handleKeyPress(); event.stopPropagation()" + onkeypress="if (event.keyCode == event.DOM_VK_ESCAPE) { this.parentNode.parentNode.parentNode.handleKeyPress(true); }"> @@ -213,7 +483,7 @@ + oncommand="this.parentNode.parentNode.parentNode.clearAll();"/> diff --git a/chrome/content/zotero/overlay.js b/chrome/content/zotero/overlay.js index 8782ae642f..e00acd3933 100644 --- a/chrome/content/zotero/overlay.js +++ b/chrome/content/zotero/overlay.js @@ -148,6 +148,9 @@ var ZoteroPane = new function() */ function onUnload() { + var tagSelector = document.getElementById('zotero-tag-selector'); + tagSelector.unregister(); + collectionsView.unregister(); if(itemsView) itemsView.unregister(); @@ -310,14 +313,18 @@ var ZoteroPane = new function() function toggleTagSelector(){ var tagSelector = document.getElementById('zotero-tag-selector'); var collapsed = tagSelector.getAttribute('collapsed')=='true'; - // If hiding, clear selection - if (!collapsed){ - tagSelector.init(); - } tagSelector.setAttribute('collapsed', !collapsed); + // If showing, set scope to items in current view + // and focus filter textbox if (collapsed) { + tagSelector.init(); + _setTagScope(); tagSelector.focusTextbox(); } + // If hiding, clear selection + else { + tagSelector.uninit(); + } } @@ -327,18 +334,25 @@ var ZoteroPane = new function() } + /* + * Sets the tag filter on the items view + */ function updateTagFilter(){ - if (itemsView) - { - itemsView.unregister(); - } - - if (collectionsView){ - var itemgroup = collectionsView._getItemAtRow(collectionsView.selection.currentIndex); - itemgroup.setTags(getTagSelection()); - - itemsView = new Zotero.ItemTreeView(itemgroup); - document.getElementById('zotero-items-tree').view = itemsView; + itemsView.setFilter('tags', getTagSelection()); + } + + + /* + * Set the tags scope to the items in the current view + * + * Passed to the items tree to trigger on changes + */ + function _setTagScope() { + var itemgroup = collectionsView._getItemAtRow(collectionsView.selection.currentIndex); + var tagSelector = document.getElementById('zotero-tag-selector'); + if (tagSelector.getAttribute('collapsed') == 'false') { + Zotero.debug('Updating tag selector with current tags'); + tagSelector.scope = itemgroup.getChildTags(); } } @@ -357,6 +371,7 @@ var ZoteroPane = new function() itemgroup.setTags(getTagSelection()); itemsView = new Zotero.ItemTreeView(itemgroup); + itemsView.addCallback(_setTagScope); document.getElementById('zotero-items-tree').view = itemsView; itemsView.selection.clearSelection(); } @@ -632,7 +647,7 @@ var ZoteroPane = new function() if(itemsView) { var searchVal = document.getElementById('zotero-tb-search').value; - itemsView.searchText(searchVal); + itemsView.setFilter('search', searchVal); document.getElementById('zotero-tb-search-cancel').hidden = searchVal == ""; } diff --git a/chrome/content/zotero/selectItemsDialog.js b/chrome/content/zotero/selectItemsDialog.js index 036230124f..922af8bbc1 100644 --- a/chrome/content/zotero/selectItemsDialog.js +++ b/chrome/content/zotero/selectItemsDialog.js @@ -72,7 +72,7 @@ function onSearch() if(itemsView) { var searchVal = document.getElementById('zotero-tb-search').value; - itemsView.searchText(searchVal); + itemsView.setFilter('search', searchVal); document.getElementById('zotero-tb-search-cancel').hidden = searchVal == ""; } diff --git a/chrome/content/zotero/xpcom/collectionTreeView.js b/chrome/content/zotero/xpcom/collectionTreeView.js index 85732753ae..f1707f5de6 100644 --- a/chrome/content/zotero/xpcom/collectionTreeView.js +++ b/chrome/content/zotero/xpcom/collectionTreeView.js @@ -678,6 +678,18 @@ Zotero.ItemGroup.prototype.getName = function() Zotero.ItemGroup.prototype.getChildItems = function() { + var s = this.getSearchObject(); + var ids = s.search(); + return Zotero.Items.get(ids); +} + + +/* + * Returns the search object for the currently display + * + * This accounts for the collection, saved search, quicksearch, tags, etc. + */ +Zotero.ItemGroup.prototype.getSearchObject = function() { var s = new Zotero.Search(); if (this.searchText){ @@ -716,10 +728,19 @@ Zotero.ItemGroup.prototype.getChildItems = function() } } - var ids = s.search(); - return Zotero.Items.get(ids); + return s; } + +/* + * Returns all the tags used by items in the current view + */ +Zotero.ItemGroup.prototype.getChildTags = function() { + var s = this.getSearchObject(); + return Zotero.Tags.getAllWithinSearch(s); +} + + Zotero.ItemGroup.prototype.setSearch = function(searchText) { this.searchText = searchText; diff --git a/chrome/content/zotero/xpcom/data_access.js b/chrome/content/zotero/xpcom/data_access.js index 97fd8aafeb..cc3c239442 100644 --- a/chrome/content/zotero/xpcom/data_access.js +++ b/chrome/content/zotero/xpcom/data_access.js @@ -1528,11 +1528,11 @@ Zotero.Item.prototype.getBestSnapshot = function(){ // Zotero.Item.prototype.addTag = function(tag){ if (!this.getID()){ - this.save(); + throw ('Cannot add tag to unsaved item in Item.addTag()'); } if (!tag){ - Zotero.debug('Not saving empty tag', 2); + Zotero.debug('Not saving empty tag in Item.addTag()', 2); return false; } @@ -1541,11 +1541,41 @@ Zotero.Item.prototype.addTag = function(tag){ if (!tagID){ var tagID = Zotero.Tags.add(tag); } + try { + var result = this.addTagByID(tagID); + Zotero.DB.commitTransaction(); + } + catch (e) { + Zotero.DB.rollbackTransaction(); + throw (e); + } + + return result ? tagID : false; +} + + +Zotero.Item.prototype.addTagByID = function(tagID) { + if (!this.getID()) { + throw ('Cannot add tag to unsaved item in Item.addTagByID()'); + } + + if (!tagID) { + Zotero.debug('Not saving nonexistent tag in Item.addTagByID()', 2); + return false; + } + + var sql = "SELECT COUNT(*) FROM tags WHERE tagID = ?"; + var count = !!Zotero.DB.valueQuery(sql, tagID); + + if (!count) { + throw ('Cannot add invalid tag id ' + tagID + ' in Item.addTagByID()'); + } + + Zotero.DB.beginTransaction(); // If INSERT OR IGNORE gave us affected rows, we wouldn't need this... - var sql = "SELECT COUNT(*) FROM itemTags WHERE itemID=? AND tagID=?"; - var exists = Zotero.DB.valueQuery(sql, [this.getID(), tagID]); - if (exists){ + if (this.hasTag(tagID)) { + Zotero.debug('Item ' + this.getID() + ' already has tag ' + tagID + ' in Item.addTagByID()'); Zotero.DB.commitTransaction(); return false; } @@ -1554,12 +1584,14 @@ Zotero.Item.prototype.addTag = function(tag){ Zotero.DB.query(sql, [this.getID(), tagID]); Zotero.DB.commitTransaction(); + Zotero.Notifier.trigger('modify', 'item', this.getID()); - if (!Zotero.DB.transactionInProgress()){ - Zotero.Notifier.trigger('modify', 'item', this.getID()); - } - - return tagID; + return true; +} + +Zotero.Item.prototype.hasTag = function(tagID) { + var sql = "SELECT COUNT(*) FROM itemTags WHERE itemID=? AND tagID=?"; + return !!Zotero.DB.valueQuery(sql, [this.getID(), tagID]); } Zotero.Item.prototype.getTags = function(){ @@ -1609,10 +1641,7 @@ Zotero.Item.prototype.removeTag = function(tagID){ Zotero.DB.query(sql, [this.getID(), tagID]); Zotero.Tags.purge(); Zotero.DB.commitTransaction(); - - if (!Zotero.DB.transactionInProgress()){ - Zotero.Notifier.trigger('modify', 'item', this.getID()); - } + Zotero.Notifier.trigger('modify', 'item', this.getID()); } @@ -3010,8 +3039,12 @@ Zotero.Tags = new function(){ this.getName = getName; this.getID = getID; this.getAll = getAll; + this.getAllWithinSearch = getAllWithinSearch; + this.getTagItems = getTagItems; this.search = search; this.add = add; + this.rename = rename; + this.remove = remove; this.purge = purge; /* @@ -3067,6 +3100,30 @@ Zotero.Tags = new function(){ } + /* + * Get all tags within the items of a Zotero.Search object + */ + function getAllWithinSearch(search) { + var searchSQL = search.getSQL(); + var searchParams = search.getSQLParams(); + + var sql = "SELECT DISTINCT tagID, tag FROM itemTags NATURAL JOIN tags " + + "WHERE itemID IN (" + searchSQL + ") ORDER BY tag COLLATE NOCASE"; + var tags = Zotero.DB.query(sql, searchParams); + var indexed = {}; + for (var i in tags){ + indexed[tags[i]['tagID']] = tags[i]['tag']; + } + return indexed; + } + + + function getTagItems(tagID) { + var sql = "SELECT itemID FROM itemTags WHERE tagID=?"; + return Zotero.DB.columnQuery(sql, tagID); + } + + function search(str){ var sql = 'SELECT tagID, tag FROM tags WHERE tag LIKE ? ' + 'ORDER BY tag COLLATE NOCASE'; @@ -3094,10 +3151,50 @@ Zotero.Tags = new function(){ Zotero.DB.query(sql, [{int: rnd}, {string: tag}]); Zotero.DB.commitTransaction(); + Zotero.Notifier.trigger('add', 'tag', rnd); return rnd; } + function rename(tagID, tag) { + Zotero.debug('Renaming tag', 4); + + Zotero.DB.beginTransaction(); + var sql = "UPDATE tags SET tag=? WHERE tagID=?"; + Zotero.DB.query(sql, [{string: tag}, {int: tagID}]); + + var itemIDs = this.getTagItems(tagID); + + delete _tags[_tagsByID[tagID]]; + delete _tagsByID[tagID]; + + Zotero.DB.commitTransaction(); + + Zotero.Notifier.trigger('modify', 'item', itemIDs); + Zotero.Notifier.trigger('modify', 'tag', tagID); + } + + + function remove(tagID) { + Zotero.DB.beginTransaction(); + + var sql = "SELECT itemID FROM itemTags WHERE tagID=?"; + var items = Zotero.DB.columnQuery(sql, tagID); + + if (!items) { + return; + } + + var sql = "DELETE FROM itemTags WHERE tagID=?"; + Zotero.DB.query(sql, tagID); + Zotero.Notifier.trigger('modify', 'item', items) + + this.purge(); + Zotero.DB.commitTransaction(); + return; + } + + /* * Delete obsolete tags from database and clear internal array entries * @@ -3123,6 +3220,8 @@ Zotero.Tags = new function(){ + '(SELECT tagID FROM itemTags);'; var result = Zotero.DB.query(sql); + Zotero.Notifier.trigger('delete', 'tag', toDelete); + return toDelete; } } diff --git a/chrome/content/zotero/xpcom/itemTreeView.js b/chrome/content/zotero/xpcom/itemTreeView.js index 830e2bf58d..f568b8e4e7 100644 --- a/chrome/content/zotero/xpcom/itemTreeView.js +++ b/chrome/content/zotero/xpcom/itemTreeView.js @@ -36,12 +36,27 @@ Zotero.ItemTreeView = function(itemGroup, sourcesOnly) this._itemGroup = itemGroup; this._sourcesOnly = sourcesOnly; + this._callbacks = []; + this._treebox = null; this.refresh(); this._unregisterID = Zotero.Notifier.registerObserver(this, 'item'); } + +Zotero.ItemTreeView.prototype.addCallback = function(callback) { + this._callbacks.push(callback); +} + + +Zotero.ItemTreeView.prototype._runCallbacks = function() { + for each(var cb in this._callbacks) { + cb(); + } +} + + /* * Called by the tree itself */ @@ -59,8 +74,12 @@ Zotero.ItemTreeView.prototype.setTree = function(treebox) { this.sort(); } + + //Zotero.debug('Running callbacks in itemTreeView.setTree()', 4); + this._runCallbacks(); } + /* * Reload the rows from the data access methods * (doesn't call the tree.invalidate methods, etc.) @@ -629,15 +648,24 @@ Zotero.ItemTreeView.prototype.deleteSelection = function(eraseChildren, force) this._treebox.endUpdateBatch(); } + /* - * Set the search filter on the view + * Set the tags filter on the view */ -Zotero.ItemTreeView.prototype.searchText = function(search) -{ +Zotero.ItemTreeView.prototype.setFilter = function(type, data) { this.selection.selectEventsSuppressed = true; var savedSelection = this.saveSelection(); - this._itemGroup.setSearch(search); + switch (type) { + case 'search': + this._itemGroup.setSearch(data); + break; + case 'tags': + this._itemGroup.setTags(data); + break; + default: + throw ('Invalid filter type in setFilter'); + } var oldCount = this.rowCount; this.refresh(); this._treebox.rowCountChanged(0,this.rowCount-oldCount); @@ -647,8 +675,11 @@ Zotero.ItemTreeView.prototype.searchText = function(search) this.rememberSelection(savedSelection); this.selection.selectEventsSuppressed = false; this._treebox.invalidate(); + //Zotero.debug('Running callbacks in itemTreeView.setFilter()', 4); + this._runCallbacks(); } + /* * Called by various view functions to show a row * diff --git a/chrome/content/zotero/xpcom/notifier.js b/chrome/content/zotero/xpcom/notifier.js index 267f8f1f11..38eb375d82 100644 --- a/chrome/content/zotero/xpcom/notifier.js +++ b/chrome/content/zotero/xpcom/notifier.js @@ -23,7 +23,7 @@ Zotero.Notifier = new function(){ var _observers = new Zotero.Hash(); var _disabled = false; - var _types = ['collection', 'search', 'item']; + var _types = ['collection', 'search', 'item', 'tag']; var _inTransaction; var _locked = false; var _queue = []; @@ -69,7 +69,7 @@ Zotero.Notifier = new function(){ while (_observers.get(hash)); Zotero.debug('Registering observer for ' - + (types ? '[' + types.join() + ']' : ' all types') + + (types ? '[' + types.join() + ']' : 'all types') + ' in notifier with hash ' + hash + "'", 4); _observers.set(hash, {ref: ref, types: types}); return hash; @@ -114,9 +114,18 @@ Zotero.Notifier = new function(){ * type - 'collection', 'search', 'item' * ids - single id or array of ids * - * c = collection, s = search, i = item + * c = collection, s = search, i = item, t = tag * - * New events and types should be added to the order arrays in commit() + * + * Notes: + * + * - add-item is currently used for both item creation and adding an + * existing item to a collection + * + * - If event queuing is on, events will not fire until commit() is called + * unless _force_ is true. + * + * - New events and types should be added to the order arrays in commit() **/ function trigger(event, type, ids, force){ if (_disabled){ @@ -206,7 +215,7 @@ Zotero.Notifier = new function(){ function sorter(a, b) { return order.indexOf(a) - order.indexOf(b); } - var order = ['collection', 'search', 'items']; + var order = ['collection', 'search', 'items', 'tags']; _queue.sort(); var order = ['add', 'modify', 'remove', 'move', 'delete']; diff --git a/chrome/content/zotero/xpcom/zotero.js b/chrome/content/zotero/xpcom/zotero.js index 1797cbfe27..2549366470 100644 --- a/chrome/content/zotero/xpcom/zotero.js +++ b/chrome/content/zotero/xpcom/zotero.js @@ -57,6 +57,7 @@ var Zotero = new function(){ this.inArray = inArray; this.arraySearch = arraySearch; this.arrayToHash = arrayToHash; + this.hasValues = hasValues; this.randomString = randomString; this.getRandomID = getRandomID; this.moveToUnique = moveToUnique; @@ -470,6 +471,18 @@ var Zotero = new function(){ } + /* + * Returns true if an object (or associative array) has at least one value + */ + function hasValues(obj) { + for (var i in obj) { + return true; + } + + return false; + } + + /** * Generate a random string of length 'len' (defaults to 8) **/ diff --git a/chrome/locale/en-US/zotero/zotero.dtd b/chrome/locale/en-US/zotero/zotero.dtd index 373d91f8e6..61ce672596 100644 --- a/chrome/locale/en-US/zotero/zotero.dtd +++ b/chrome/locale/en-US/zotero/zotero.dtd @@ -56,9 +56,12 @@ + + + diff --git a/chrome/locale/en-US/zotero/zotero.properties b/chrome/locale/en-US/zotero/zotero.properties index ccef0cc716..b0371958f3 100644 --- a/chrome/locale/en-US/zotero/zotero.properties +++ b/chrome/locale/en-US/zotero/zotero.properties @@ -17,6 +17,11 @@ pane.collections.menu.createBib.savedSearch = Create Bibliography From Saved Se pane.collections.menu.generateReport.collection = Generate Report from Collection... pane.collections.menu.generateReport.savedSearch = Generate Report from Saved Search... +pane.tagSelector.rename.title = Please enter a new name for this tag. +pane.tagSelector.rename.message = The tag will be changed in all associated items. +pane.tagSelector.delete.title = Are you sure you want to delete this tag? +pane.tagSelector.delete.message = The tag will be removed from all items. + pane.items.delete = Are you sure you want to delete the selected item? pane.items.delete.multiple = Are you sure you want to delete the selected items? pane.items.delete.title = Delete diff --git a/chrome/skin/default/zotero/bindings/tagselector.css b/chrome/skin/default/zotero/bindings/tagselector.css index 18f1d97231..84161b612f 100644 --- a/chrome/skin/default/zotero/bindings/tagselector.css +++ b/chrome/skin/default/zotero/bindings/tagselector.css @@ -1,9 +1,3 @@ -@namespace url("http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"); -@namespace html url("http://www.w3.org/1999/xhtml"); - - -/* Tag selector */ - /* Don't focus filter textbox if pane isn't open */ zoterotagselector[collapsed=true] tags-search { @@ -19,22 +13,45 @@ groupbox #tags-toggle { overflow: auto; - margin-bottom: 10px; display: block; /* allow labels to wrap instead of all being in one line */ } +checkbox +{ + margin: .75em 0 .4em; +} + label { - margin-right: 5px; - padding: 2px 4px; + margin-right: .2em; + padding: .15em .25em; -moz-user-focus: ignore; } -label[selected=true] +label[selected="true"] { background: #a9c6f0 !important; } +/* Visible out-of-scope tags should be grey */ +label[inScope="false"] +{ + color: #666 !important; +} + +/* Don't display clicky effect to out-of-scope icons */ +label.zotero-clicky[inScope="false"]:hover, +label.zotero-clicky[inScope="false"]:active +{ + background: inherit !important; +} + +label[draggedOver="true"] +{ + color: white !important; + background: #666; +} + hbox { -moz-box-align: baseline; @@ -49,6 +66,7 @@ hbox list-style-image: url('chrome://zotero/skin/search-cancel.png'); } +/* Bottom buttons */ toolbarbutton.zotero-clicky { margin:3px 5px;