From f932f312ebbc08559b62e90a469781eea4d78a77 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Sat, 9 Mar 2013 02:42:34 -0500 Subject: [PATCH] Overhaul tags box - Improvements to #20, with the tags box switching to a multiline textbox in the style of #164 on a multiline paste or Shift-Return. In the multiline box, Return is a newline and Shift-Return saves - Allow tabbing through tags via keyboard (and keep the last empty textbox open on tab, so you can hold down the tab key to get all the way to the end) - Fix various post-update focusing issues (though the wrong textbox is still selected for some multiline updates via Tab/Shift-Tab) - Make (single-line) tag entering much faster by not reloading the whole tags list and just placing the new tag in the correct sorted position. This could be made even faster with tag selector optimizations. - Allow the Add button to focus when switching to the Tags pane (and the same for the Related pane, for good measure) --- chrome/content/zotero/bindings/relatedbox.xml | 3 +- chrome/content/zotero/bindings/tagsbox.xml | 582 ++++++++++++------ chrome/content/zotero/itemPane.js | 2 +- chrome/content/zotero/xpcom/data/item.js | 28 +- .../skin/default/zotero/bindings/tagsbox.css | 3 + 5 files changed, 406 insertions(+), 212 deletions(-) create mode 100644 chrome/skin/default/zotero/bindings/tagsbox.css diff --git a/chrome/content/zotero/bindings/relatedbox.xml b/chrome/content/zotero/bindings/relatedbox.xml index 975c460666..cba1bfbce6 100644 --- a/chrome/content/zotero/bindings/relatedbox.xml +++ b/chrome/content/zotero/bindings/relatedbox.xml @@ -289,7 +289,8 @@ - diff --git a/chrome/content/zotero/bindings/tagsbox.xml b/chrome/content/zotero/bindings/tagsbox.xml index eeb4772ca8..4a5630ee71 100644 --- a/chrome/content/zotero/bindings/tagsbox.xml +++ b/chrome/content/zotero/bindings/tagsbox.xml @@ -36,6 +36,9 @@ + + false + @@ -74,6 +77,10 @@ @@ -143,11 +150,14 @@ @@ -187,29 +198,12 @@ name = ''; } - if (!tabindex) - { - if (this.id('tagRows').lastChild) - { - tabindex = parseInt(this.id('tagRows').lastChild. - firstChild.nextSibling.getAttribute('ztabindex')) + 1; - } - else { - tabindex = 1; - } + if (!tabindex) { + tabindex = this.id('tagRows').childNodes.length + 1; } var icon = document.createElement("image"); icon.className = "zotero-box-icon"; - var iconFile = 'tag'; - if (type == 0) { - icon.setAttribute('tooltiptext', Zotero.getString('pane.item.tags.icon.user')); - } - else if (type == 1) { - iconFile += '-automatic'; - icon.setAttribute('tooltiptext', Zotero.getString('pane.item.tags.icon.automatic')); - } - icon.setAttribute('src', 'chrome://zotero/skin/' + iconFile + '.png'); // DEBUG: Why won't just this.nextSibling.blur() work? icon.setAttribute('onclick','if (this.nextSibling.inputField){ this.nextSibling.inputField.blur() }'); @@ -220,15 +214,7 @@ var remove = document.createElement("label"); remove.setAttribute('value','-'); remove.setAttribute('class','zotero-clicky zotero-clicky-minus'); - if (tagID) - { - remove.setAttribute('ztabindex', -1); - remove.setAttribute('onclick',"document.getBindingParent(this).remove('"+ tagID +"');"); - } - else - { - remove.setAttribute('disabled', true); - } + remove.setAttribute('tabindex', -1); } var row = document.createElement("row"); @@ -238,19 +224,73 @@ row.appendChild(remove); } - if (tagID) - { - row.setAttribute('id', 'tag-' + tagID); - row.setAttribute('tagType', type); - } + this.updateRow(row, tagObj, tabindex); this.id('tagRows').appendChild(row); + return row; ]]> + + + + + + + + @@ -273,8 +313,6 @@ valueElement.className += ' zotero-clicky'; } - this._tabIndexMaxTagsFields = Math.max(this._tabIndexMaxTagsFields, tabindex); - var firstSpace; if (typeof valueText == 'string') { firstSpace = valueText.indexOf(" "); @@ -308,6 +346,8 @@ + + 1) { + t.setAttribute('multiline', true); + t.setAttribute('rows', rows); + } // Add auto-complete - t.setAttribute('type', 'autocomplete'); - t.setAttribute('autocompletesearch', 'zotero'); - var suffix = itemID ? itemID : ''; - t.setAttribute('autocompletesearchparam', fieldName + '/' + suffix); + else { + t.setAttribute('type', 'autocomplete'); + t.setAttribute('autocompletesearch', 'zotero'); + var suffix = itemID ? itemID : ''; + t.setAttribute('autocompletesearchparam', fieldName + '/' + suffix); + } var box = elem.parentNode; box.replaceChild(t, elem); + t.setAttribute('onblur', "return document.getBindingParent(this).blurHandler(this)"); + t.setAttribute('onkeypress', "return document.getBindingParent(this).handleKeyPress(event)"); + t.setAttribute('onpaste', "return document.getBindingParent(this).handlePaste(event)"); + + this._tabDirection = false; + this._lastTabIndex = tabindex; + // Prevent error when clicking between a changed field // and another -- there's probably a better way if (!t.select) { return; } - t.select(); - t.addEventListener('blur', function () { - document.getBindingParent(this).blurHandler(this); - }, false); - t.setAttribute('onkeypress', "return document.getBindingParent(this).handleKeyPress(event)"); - - this._tabDirection = false; - this._lastTabIndex = tabindex; - return t; ]]> @@ -373,23 +419,53 @@ switch (event.keyCode) { case event.DOM_VK_RETURN: + var multiline = target.getAttribute('multiline'); + var empty = target.value == ""; + if (event.shiftKey) { + if (!multiline) { + var self = this; + setTimeout(function () { + var val = target.value; + if (val !== "") { + val += "\n"; + } + self.makeMultiline(target, val, 6); + }, 0); + return false; + } + // Submit + } + else if (multiline) { + return true; + } + var fieldname = 'tag'; - // Prevent blur on containing textbox - // DEBUG: what happens if this isn't present? - event.preventDefault(); - // If last tag row, create new one - var row = target.parentNode.parentNode; - if (row == row.parentNode.lastChild) { + var row = Zotero.getAncestorByTagName(target, 'row'); + + // If non-empty last row, add new row + if (row == row.parentNode.lastChild && !empty) { + var focusField = true; this._tabDirection = 1; - var lastTag = true; + } + // If empty non-last row, refocus current row + else if (row != row.parentNode.lastChild && empty) { + var focusField = true; + } + // If non-empty non-last row, return focus to items pane + else { + var focusField = false; + this._lastTabIndex = false; } focused.blur(); + if (focusField) { + this._focusField(); + } // Return focus to items pane - if (!lastTag) { + else { var tree = document.getElementById('zotero-items-tree'); if (tree) { tree.focus(); @@ -400,10 +476,16 @@ case event.DOM_VK_ESCAPE: // Reset field to original value - target.value = target.getAttribute('value'); + if (target.getAttribute('multiline')) { + target.value = ""; + } + else { + target.value = target.getAttribute('value'); + } var tagsbox = Zotero.getAncestorByTagName(focused, 'tagsbox'); + this._lastTabIndex = false; focused.blur(); if (tagsbox) { @@ -419,8 +501,17 @@ return false; case event.DOM_VK_TAB: + // If already an empty last row, ignore forward tab + if (target.value == "" && !event.shiftKey) { + var row = Zotero.getAncestorByTagName(target, 'row'); + if (row == row.parentNode.lastChild) { + return false; + } + } + this._tabDirection = event.shiftKey ? -1 : 1; focused.blur(); + this._focusField(); return false; } @@ -429,6 +520,63 @@ + + + + + + + + + + + + + + + + @@ -449,7 +597,7 @@ var fieldName = 'tag'; var tabindex = textbox.getAttribute('ztabindex'); - + var value = textbox.value; var tagsbox = Zotero.getAncestorByTagName(textbox, 'tagsbox'); @@ -468,69 +616,74 @@ var tagArray = value.split(/\r\n?|\n/); if (saveChanges) { - if (id && (tagArray.length < 2)) { + // Modifying existing tag + if (id && tagArray.length < 2) { if (value) { - var origTagIndex = this.item.getTagIndex(id); - var changed = tagsbox.replace(id, value); - if (changed) { - var newTagIndex = this.item.getTagIndex(changed); - if (newTagIndex>origTagIndex) { - if (this._tabDirection == 1) { - this._lastTabIndex--; - } - } - else if (newTagIndex 1) { + var lastTag = row == row.parentNode.lastChild; + + Zotero.DB.beginTransaction(); + + // If old tag isn't in array, remove it + if (id) { + var oldValue = Zotero.Tags.getName(id); + if (tagArray.indexOf(oldValue) == -1) { + this.item.removeTag(id); + } + } + + this.item.addTags(tagArray); + + Zotero.DB.commitTransaction(); + + // TODO: get tab index right + + if (lastTag) { + this._lastTabIndex = this.item.getTags().length; + } + + this.reload(); + return; + } + // Single tag at end else { - //Check for newlines or carriage returns used as delimiters - //in a series of tags added at once. Add each tag - //separately. - if (tagArray.length > 1) { - var extremeTag = false; - var nextTag = false; + id = tagsbox.add(value); + // New tag + if (id) { + // Stay put, since a tag was added above if (this._tabDirection == -1) { - if (this._lastTabIndex == 1) { - extremeTag = true; - } else { - nextTag = row.previousSibling.getAttribute('id').split('-')[1]; - } - } else if (this._tabDirection == 1) { - if (this._lastTabIndex >= this.item.getTags().length) { - extremeTag = true; - } else { - nextTag = row.nextSibling.getAttribute('id').split('-')[1]; - } - } - - id = this.item.addTags(tagArray); - - if (extremeTag) { - if (this._tabDirection == 1) { - this._lastTabIndex = this.item.getTags().length; - } else if (this._tabDirection == -1) { - this._lastTabIndex = 2; - } - } else { - this._lastTabIndex = this.item.getTagIndex(nextTag)+1-this._tabDirection; + this._tabDirection = false; } } + // Already exists else { - id = tagsbox.add(value); - } - if (!id && (this._tabDirection==1)) { - this._lastTabIndex--; + // Go back one, since we'll remove this below + if (this._tabDirection == 1) { + this._lastTabIndex--; + } } } } @@ -541,8 +694,55 @@ tabindex ); - var box = textbox.parentNode; - box.replaceChild(elem, textbox); + var row = textbox.parentNode; + row.replaceChild(elem, textbox); + + this.updateRow(row, Zotero.Tags.get(id), tabindex); + + if (!unchanged) { + // Move row to appropriate place, alphabetically + var collation = Zotero.getLocaleCollation(); + var rows = row.parentNode; + var labels = rows.getElementsByAttribute('fieldname', 'tag'); + + rows.removeChild(row); + var currentTabIndex = elem.getAttribute('ztabindex'); + + var before = null; + var inserted = false; + for (var i=0; i 0) { + labels[i].setAttribute('ztabindex', newTabIndex); + continue; + } + + elem.setAttribute('ztabindex', newTabIndex); + rows.insertBefore(row, labels[i].parentNode); + inserted = true; + + // Adjust last tab index + if (this._tabDirection == -1) { + if (this._lastTabIndex > newTabIndex) { + this._lastTabIndex++; + } + } + else if (this._tabDirection == 1) { + if (this._lastTabIndex < newTabIndex) { + this._lastTabIndex--; + } + } + } + if (!inserted) { + elem.setAttribute('ztabindex', i + 1); + rows.appendChild(row); + } + } } else { // Just remove the row @@ -553,12 +753,6 @@ } catch (e) {} } - - var focusBox = tagsbox; - - if (this._tabDirection) { - this._focusNextField(focusBox, this._lastTabIndex, this._tabDirection == -1); - } ]]> @@ -569,6 +763,7 @@ @@ -648,29 +843,6 @@ - - - - - - - - - - - - - + + + + + + + + + + + + + + - diff --git a/chrome/content/zotero/itemPane.js b/chrome/content/zotero/itemPane.js index d61880e7f3..0e824b0f2b 100644 --- a/chrome/content/zotero/itemPane.js +++ b/chrome/content/zotero/itemPane.js @@ -49,7 +49,7 @@ var ZoteroItemPane = new function() { /* - * Load an item + * Load a top-level item */ this.viewItem = function (item, mode, index) { if (!index) { diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js index 72078fdd96..7f9f828f3b 100644 --- a/chrome/content/zotero/xpcom/data/item.js +++ b/chrome/content/zotero/xpcom/data/item.js @@ -3753,19 +3753,16 @@ Zotero.Item.prototype.addTag = function(name, type) { Zotero.Item.prototype.addTags = function (tags, type) { Zotero.DB.beginTransaction(); try { - var tagIDArray = []; - var tempID = false; + var tagIDs = []; for (var i = 0; i < tags.length; i++) { - tempID = this.addTag(tags[i], type); - if (tempID) { - tagIDArray.push(tempID); + let id = this.addTag(tags[i], type); + if (id) { + tagIDs.push(id); } } - tagIDArray = (tagIDArray.length>0) ? tagIDArray : false; - Zotero.DB.commitTransaction(); - return tagIDArray; + return tagIDs.length > 0 ? tagIDs : false; } catch (e) { Zotero.DB.rollbackTransaction(); @@ -3847,21 +3844,6 @@ Zotero.Item.prototype.getTagIDs = function() { return Zotero.DB.columnQuery(sql, this.id); } -/** -* Return the index of tagID in the list of the item's tags sorted in alphabetical order. -*/ -Zotero.Item.prototype.getTagIndex = function(tagID) { - var tags = this.getTags(); - - for (var i=0;i