From d9cee322cd36e398392d5ad475c5a68aae483182 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Thu, 28 Mar 2019 05:19:41 -0400 Subject: [PATCH] Tag selector performance overhaul - Use react-virtualized to render tags on demand, reducing the number of DOM elements from potentially tens of thousands to <100. This requires tags to be absolutely positioned, so sizing and positioning need to be precomputed rather than relying on CSS. - Avoid unnecessary refreshes, speed up tag retrieval, and optimize sorting - Debounce reflowing when resizing tag selector Also: - Scroll to top when changing collections - Allow tags to take up full width of tag selector without truncation Closes #1649 Closes #281 --- .babelrc | 1 + chrome/content/zotero/components/search.jsx | 4 +- .../zotero/components/tag-selector.jsx | 37 +- .../components/tag-selector/tag-list.jsx | 213 ++++++-- .../content/zotero/containers/tagSelector.jsx | 488 ++++++++++++++---- .../content/zotero/xpcom/collectionTreeRow.js | 12 +- chrome/content/zotero/xpcom/data/item.js | 6 +- chrome/content/zotero/xpcom/data/tags.js | 70 ++- chrome/content/zotero/zoteroPane.js | 31 +- chrome/content/zotero/zoteroPane.xul | 8 +- .../skin/default/zotero/bindings/tagsbox.css | 4 + package-lock.json | 46 ++ package.json | 1 + resource/react-virtualized.js | 1 + scripts/babel-worker.js | 16 +- scripts/config.js | 2 + scss/components/_tag-selector.scss | 45 +- test/tests/tagSelectorTest.js | 244 +++++++-- 18 files changed, 959 insertions(+), 270 deletions(-) create mode 120000 resource/react-virtualized.js diff --git a/.babelrc b/.babelrc index 376b4a8264..c6c0b2e3aa 100644 --- a/.babelrc +++ b/.babelrc @@ -9,6 +9,7 @@ "chrome/content/zotero/xpcom/citeproc.js", "resource/react.js", "resource/react-dom.js", + "resource/react-virtualized.js", "test/resource/*.js" ], "plugins": [ diff --git a/chrome/content/zotero/components/search.jsx b/chrome/content/zotero/components/search.jsx index 872d301da3..5e448afbfe 100644 --- a/chrome/content/zotero/components/search.jsx +++ b/chrome/content/zotero/components/search.jsx @@ -54,17 +54,17 @@ class Search extends React.PureComponent { } focus() { - this.inputRef.focus(); + this.inputRef.current.focus(); } render() { return (
{this.state.immediateValue !== '' diff --git a/chrome/content/zotero/components/tag-selector.jsx b/chrome/content/zotero/components/tag-selector.jsx index bf415fa671..55287e3c34 100644 --- a/chrome/content/zotero/components/tag-selector.jsx +++ b/chrome/content/zotero/components/tag-selector.jsx @@ -3,21 +3,30 @@ const React = require('react'); const PropTypes = require('prop-types'); const TagList = require('./tag-selector/tag-list'); -const Input = require('./form/input'); const { Button } = require('./button'); const { IconTagSelectorMenu } = require('./icons'); const Search = require('./search'); -class TagSelector extends React.Component { +class TagSelector extends React.PureComponent { render() { return (
- +
); } - + render() { - const totalTagCount = this.props.tags.length; - var tagList = ( -
    - { - [...Array(totalTagCount).keys()].map(index => this.renderTag(index)) - } -
- ); + Zotero.debug("Rendering tag list"); + const tagCount = this.props.tags.length; + + var tagList; if (!this.props.loaded) { tagList = (
); - } else if (totalTagCount == 0) { + } + else if (tagCount == 0) { tagList = (
); } + else { + // Scroll to top if more than one tag was removed + if (tagCount < this.prevTagCount - 1) { + this.scrollToTopOnNextUpdate = true; + } + this.prevTagCount = tagCount; + this.updatePositions(); + tagList = ( + + ); + } + return ( -
{ this.container = ref }}> +
{tagList}
- ) - + ); } + + static propTypes = { + tags: PropTypes.arrayOf(PropTypes.shape({ + name: PropTypes.string, + selected: PropTypes.bool, + color: PropTypes.string, + disabled: PropTypes.bool, + width: PropTypes.number + })), + dragObserver: PropTypes.shape({ + onDragOver: PropTypes.func, + onDragExit: PropTypes.func, + onDrop: PropTypes.func + }), + onSelect: PropTypes.func, + onTagContext: PropTypes.func, + loaded: PropTypes.bool, + width: PropTypes.number.isRequired, + height: PropTypes.number.isRequired, + fontSize: PropTypes.number.isRequired, + }; } module.exports = TagList; diff --git a/chrome/content/zotero/containers/tagSelector.jsx b/chrome/content/zotero/containers/tagSelector.jsx index 26f5983f5b..8604429a44 100644 --- a/chrome/content/zotero/containers/tagSelector.jsx +++ b/chrome/content/zotero/containers/tagSelector.jsx @@ -1,24 +1,22 @@ /* global Zotero: false */ 'use strict'; -(function() { - const React = require('react'); const ReactDOM = require('react-dom'); +const PropTypes = require('prop-types'); const { IntlProvider } = require('react-intl'); const TagSelector = require('components/tag-selector.js'); -const noop = Promise.resolve(); const defaults = { tagColors: new Map(), tags: [], + scope: null, showAutomatic: Zotero.Prefs.get('tagSelector.showAutomatic'), searchString: '', - inScope: new Set(), loaded: false }; const { Cc, Ci } = require('chrome'); -Zotero.TagSelector = class TagSelectorContainer extends React.Component { +Zotero.TagSelector = class TagSelectorContainer extends React.PureComponent { constructor(props) { super(props); this._notifierID = Zotero.Notifier.registerObserver( @@ -26,31 +24,57 @@ Zotero.TagSelector = class TagSelectorContainer extends React.Component { ['collection-item', 'item', 'item-tag', 'tag', 'setting'], 'tagSelector' ); - this.displayAllTags = Zotero.Prefs.get('tagSelector.displayAllTags'); - this.selectedTags = new Set(); - this.state = defaults; + Zotero.Prefs.registerObserver('fontSize', this.handleFontChange.bind(this)); + + this.tagListRef = React.createRef(); this.searchBoxRef = React.createRef(); + + this.displayAllTags = Zotero.Prefs.get('tagSelector.displayAllTags'); + // Not stored in state to avoid an unnecessary refresh. Instead, when a tag is selected, we + // trigger the selection handler, which updates the visible items, which triggers + // onItemViewChanged(), which triggers a refresh with the new tags. + this.selectedTags = new Set(); + this.widths = new Map(); + this.widthsBold = new Map(); + + this.state = { + ...defaults, + ...this.getContainerDimensions(), + ...this.getFontInfo() + }; } focusTextbox() { - this.searchBoxRef.focus(); + this.searchBoxRef.current.focus(); + } + + componentDidUpdate(_prevProps, _prevState) { + Zotero.debug("Tag selector updated"); + + // If we changed collections, scroll to top + if (this.collectionTreeRow && this.collectionTreeRow.id != this.prevTreeViewID) { + this.tagListRef.current.scrollToTop(); + this.prevTreeViewID = this.collectionTreeRow.id; + } } // Update trigger #1 (triggered by ZoteroPane) - async onItemViewChanged({collectionTreeRow, libraryID, tagsInScope}) { + async onItemViewChanged({ collectionTreeRow, libraryID }) { Zotero.debug('Updating tag selector from current view'); - this.collectionTreeRow = collectionTreeRow || this.collectionTreeRow; - - let newState = {loaded: true}; - - if (!this.state.tagColors.length && libraryID && this.libraryID != libraryID) { - newState.tagColors = Zotero.Tags.getColors(libraryID); - } + var prevLibraryID = this.libraryID; + this.collectionTreeRow = collectionTreeRow; this.libraryID = libraryID; - newState.tags = await this.getTags(tagsInScope, - this.state.tagColors.length ? this.state.tagColors : newState.tagColors); + var newState = { + loaded: true + }; + if (prevLibraryID != libraryID) { + newState.tagColors = Zotero.Tags.getColors(libraryID); + } + var { tags, scope } = await this.getTagsAndScope(); + newState.tags = tags; + newState.scope = scope; this.setState(newState); } @@ -59,13 +83,13 @@ Zotero.TagSelector = class TagSelectorContainer extends React.Component { if (type === 'setting') { if (ids.some(val => val.split('/')[1] == 'tagColors')) { Zotero.debug("Updating tag selector after tag color change"); - let tagColors = Zotero.Tags.getColors(this.libraryID); - this.state.tagColors = tagColors; - this.setState({tagColors, tags: await this.getTags(null, tagColors)}); + this.setState({ + tagColors: Zotero.Tags.getColors(this.libraryID) + }); } return; } - + // Ignore anything other than deletes in duplicates view if (this.collectionTreeRow && this.collectionTreeRow.isDuplicates()) { switch (event) { @@ -83,107 +107,346 @@ Zotero.TagSelector = class TagSelectorContainer extends React.Component { return; } - // If a selected tag no longer exists, deselect it - if (type == 'tag' && (event == 'modify' || event == 'delete')) { - let changed = false; - for (let id of ids) { - let tag = extraData[id].old.tag; - if (this.selectedTags.has(tag)) { - this.selectedTags.delete(tag); - changed = true; - } - } - if (changed && typeof(this.props.onSelection) === 'function') { - this.props.onSelection(this.selectedTags); - } + // Ignore tag deletions, which are handled by 'item-tag' 'remove' + if (type == 'tag') { return; } - // TODO: Check libraryID for some events to avoid refreshing unnecessarily on sync changes? - Zotero.debug("Updating tag selector after tag change"); - var newTags = await this.getTags(); - if (type == 'item-tag' && event == 'remove') { - let changed = false; - let visibleTags = newTags.map(tag => tag.tag); + if (type == 'item-tag' && ['add', 'remove'].includes(event)) { + let changedTagsInScope = []; + let changedTagsInView = []; + // Group tags by tag type for lookup + let tagsByType = new Map(); for (let id of ids) { - let tag = extraData[id].tag; - if (this.selectedTags.has(tag) && !visibleTags.includes(tag)) { - this.selectedTags.delete(tag); - changed = true; + let [_, tagID] = id.split('-'); + let type = extraData[id].type; + let typeTags = tagsByType.get(type); + if (!typeTags) { + typeTags = []; + tagsByType.set(type, typeTags); + } + typeTags.push(parseInt(tagID)); + } + // Check tags for each tag type to see if they're in view/scope + for (let [type, tagIDs] of tagsByType) { + changedTagsInScope.push(...await this.collectionTreeRow.getTags([type], tagIDs)); + if (this.displayAllTags) { + changedTagsInView.push( + ...await Zotero.Tags.getAllWithin({ libraryID: this.libraryID, tagIDs }) + ); } } - if (changed && typeof(this.props.onSelection) === 'function') { - this.props.onSelection(this.selectedTags); + if (!this.displayAllTags) { + changedTagsInView = changedTagsInScope; + } + changedTagsInScope = new Set(changedTagsInScope.map(tag => tag.tag)); + + if (event == 'add') { + this.sortTags(changedTagsInView); + if (!changedTagsInView.length) { + return; + } + this.setState((state, _props) => { + // Insert sorted + var newTags = [...state.tags]; + var newScope = state.scope ? new Set(state.scope) : new Set(); + var scopeChanged = false; + var start = 0; + var collation = Zotero.getLocaleCollation(); + for (let tag of changedTagsInView) { + let name = tag.tag; + let added = false; + for (let i = start; i < newTags.length; i++) { + start++; + let cmp = collation.compareString(1, newTags[i].tag, name); + // Skip tag if it already exists + if (cmp == 0) { + added = true; + break; + } + if (cmp > 0) { + newTags.splice(i, 0, tag); + added = true; + break; + } + } + if (!added) { + newTags.push(tag); + } + + if (changedTagsInScope.has(name) && !newScope.has(name)) { + newScope.add(name); + scopeChanged = true; + } + } + + var newState = { + tags: newTags + }; + if (scopeChanged) { + newState.scope = newScope; + } + return newState; + }); + return; + } + else if (event == 'remove') { + changedTagsInView = new Set(changedTagsInView.map(tag => tag.tag)); + + this.setState((state, props) => { + var previousTags = new Set(state.tags.map(tag => tag.tag)); + var tagsToRemove = new Set(); + var newScope; + var selectionChanged = false; + for (let id of ids) { + let name = extraData[id].tag; + let removed = false; + + // If tag was shown previously and shouldn't be anymore, remove from view + if (previousTags.has(name) && !changedTagsInView.has(name)) { + tagsToRemove.add(name); + removed = true; + } + + // Remove from scope if there is one + if (state.scope && state.scope.has(name) && !changedTagsInScope.has(name)) { + if (!newScope) { + newScope = new Set(state.scope); + } + newScope.delete(name); + removed = true; + } + + // Removed from either view or scope + if (removed) { + // Deselect if selected + if (this.selectedTags.has(name)) { + this.selectedTags.delete(name); + selectionChanged = true; + } + + // If removing a tag from view, clear its cached width. It might still + // be in this or another library, but if so we'll just recalculate its + // width the next time it's needed. + this.widths.delete(name); + this.widthsBold.delete(name); + } + } + if (selectionChanged && typeof props.onSelection == 'function') { + props.onSelection(this.selectedTags); + } + var newState = {}; + if (tagsToRemove.size) { + newState.tags = state.tags.filter(tag => !tagsToRemove.has(tag.tag)); + } + if (newScope) { + newState.scope = newScope; + } + return newState; + }); + return; } } - return this.setState({tags: newTags}); + this.setState(await this.getTagsAndScope()); } - async getTags(tagsInScope, tagColors) { - if (!tagsInScope) { - tagsInScope = await this.collectionTreeRow.getChildTags(); - } - this.inScope = new Set(tagsInScope.map(t => t.tag)); - let tags; + async getTagsAndScope() { + var tags = await this.collectionTreeRow.getTags(); + // The scope is all visible tags, not all tags in the library + var scope = new Set(tags.map(t => t.tag)); if (this.displayAllTags) { - tags = await Zotero.Tags.getAll(this.libraryID, [0, 1]); - } else { - tags = tagsInScope + tags = await Zotero.Tags.getAll(this.libraryID); } - tagColors = tagColors || this.state.tagColors; - - // Add colored tags that aren't already real tags - let regularTags = new Set(tags.map(tag => tag.tag)); - let coloredTags = Array.from(tagColors.keys()); - - coloredTags.filter(ct => !regularTags.has(ct)).forEach(x => - tags.push(Zotero.Tags.cleanData({ tag: x })) - ); - - // Sort by name (except for colored tags, which sort by assigned number key) - tags.sort(function (a, b) { - let aColored = tagColors.get(a.tag); - let bColored = tagColors.get(b.tag); - if (aColored && !bColored) return -1; - if (!aColored && bColored) return 1; - if (aColored && bColored) { - return aColored.position - bColored.position; + // If tags haven't changed, return previous array without sorting again + if (this.state.tags.length == tags.length) { + let prevTags = new Set(this.state.tags.map(tag => tag.tag)); + let same = true; + for (let tag of tags) { + if (!prevTags.has(tag.tag)) { + same = false; + break; + } } - - return Zotero.getLocaleCollation().compareString(1, a.tag, b.tag); - }); + if (same) { + Zotero.debug("Tags haven't changed"); + return { + tags: this.state.tags, + scope + }; + } + } - return tags; + this.sortTags(tags); + return { tags, scope }; } - + + sortTags(tags) { + var d = new Date(); + var collation = Zotero.Intl.collation; + tags.sort(function (a, b) { + return collation.compareString(1, a.tag, b.tag); + }); + Zotero.debug(`Sorted tags in ${new Date() - d} ms`); + } + + getContainerDimensions() { + var container = document.getElementById(this.props.container); + return { + width: container.clientWidth, + height: container.clientHeight + }; + } + + handleResize() { + //Zotero.debug("Resizing tag selector"); + var { width, height } = this.getContainerDimensions(); + this.setState({ width, height }); + } + + getFontInfo() { + var elem = document.createElementNS("http://www.w3.org/1999/xhtml", "div"); + elem.className = 'tag-selector-item'; + elem.style.position = 'absolute'; + elem.style.opacity = 0; + var container = document.getElementById(this.props.container); + container.appendChild(elem); + var style = window.getComputedStyle(elem); + var props = { + fontSize: style.getPropertyValue('font-size'), + fontFamily: style.getPropertyValue('font-family') + }; + container.removeChild(elem); + return props; + } + + /** + * Recompute tag widths based on the current font settings + */ + handleFontChange() { + this.widths.clear(); + this.widthsBold.clear(); + this.setState({ + ...this.getFontInfo() + }); + } + + /** + * Uses canvas.measureText to compute and return the width of the given text of given font in pixels. + * + * @param {String} text The text to be rendered. + * @param {String} font The css font descriptor that text is to be rendered with (e.g. "bold 14px verdana"). + * + * @see https://stackoverflow.com/questions/118241/calculate-text-width-with-javascript/21015393#21015393 + */ + getTextWidth(text, font) { + // re-use canvas object for better performance + var canvas = this.canvas || (this.canvas = document.createElementNS("http://www.w3.org/1999/xhtml", "canvas")); + var context = canvas.getContext("2d"); + context.font = font; + // Add a little more to make sure we don't crop + var metrics = context.measureText(text); + return Math.ceil(metrics.width); + } + + getWidth(name) { + var num = 0; + var font = this.state.fontSize + ' ' + this.state.fontFamily; + // Colored tags are shown in bold, which results in a different width + var fontBold = 'bold ' + font; + let hasColor = this.state.tagColors.has(name); + let widths = hasColor ? this.widthsBold : this.widths; + let width = widths.get(name); + if (width === undefined) { + //Zotero.debug(`Calculating ${hasColor ? 'bold ' : ''}width for tag '${name}'`); + width = this.getTextWidth(name, hasColor ? fontBold : font); + widths.set(name, width); + } + return width; + } + render() { - let tags = this.state.tags; + Zotero.debug("Rendering tag selector"); + var tags = this.state.tags; + var tagColors = this.state.tagColors; + if (!this.state.showAutomatic) { - tags = tags.filter(t => t.type != 1).map(t => t.tag); + tags = tags.filter(t => t.type != 1); } // Remove duplicates from auto and manual tags else { - tags = Array.from(new Set(tags.map(t => t.tag))); + let seen = new Set(); + let newTags = []; + for (let tag of tags) { + if (!seen.has(tag.tag)) { + newTags.push(tag); + seen.add(tag.tag); + } + } + tags = newTags; } + + // Extract colored tags + var coloredTags = []; + for (let i = 0; i < tags.length; i++) { + if (tagColors.has(tags[i].tag)) { + coloredTags.push(...tags.splice(i, 1)); + i--; + } + } + + // Add colored tags that aren't already real tags + var extractedColoredTags = new Set(coloredTags.map(tag => tag.tag)); + [...tagColors.keys()] + .filter(tag => !extractedColoredTags.has(tag)) + .forEach(tag => coloredTags.push(Zotero.Tags.cleanData({ tag }))); + + // Sort colored tags and place at beginning + coloredTags.sort((a, b) => { + return tagColors.get(a.tag).position - tagColors.get(b.tag).position; + }); + tags = coloredTags.concat(tags); + + // Filter if (this.state.searchString) { let lcStr = this.state.searchString.toLowerCase(); - tags = tags.filter(tag => tag.toLowerCase().includes(lcStr)); + tags = tags.filter(tag => tag.tag.toLowerCase().includes(lcStr)); } - tags = tags.map((name) => { - return { + + // Prepare tag objects for list component + //var d = new Date(); + var inTagColors = true; + tags = tags.map((tag) => { + let name = tag.tag; + tag = { name, - selected: this.selectedTags.has(name), - color: this.state.tagColors.has(name) ? this.state.tagColors.get(name).color : '', - disabled: !this.inScope.has(name) + width: tag.width + }; + if (this.selectedTags.has(name)) { + tag.selected = true; } - }); + if (inTagColors && tagColors.has(name)) { + tag.color = tagColors.get(name).color; + } + else { + inTagColors = false; + } + // If we're not displaying all tags, we only need to check the scope for colored tags, + // since everything else will be in scope + if ((this.displayAllTags || inTagColors) && !this.state.scope.has(name)) { + tag.disabled = true; + } + tag.width = this.getWidth(name); + return tag; + }); + //Zotero.debug(`Prepared tags in ${new Date() - d} ms`); return {} : this.handleTagSelected} @@ -191,6 +454,9 @@ Zotero.TagSelector = class TagSelectorContainer extends React.Component { onSearch={this.handleSearch} onSettings={this.handleSettings.bind(this)} loaded={this.state.loaded} + width={this.state.width} + height={this.state.height} + fontSize={parseInt(this.state.fontSize.replace('px', ''))} />; } @@ -198,13 +464,6 @@ Zotero.TagSelector = class TagSelectorContainer extends React.Component { this.state.viewOnly != (mode == 'view') && this.setState({viewOnly: mode == 'view'}); } - uninit() { - ReactDOM.unmountComponentAtNode(this.domEl); - if (this._notifierID) { - Zotero.Notifier.unregisterObserver(this._notifierID); - } - } - handleTagContext = (tag, ev) => { let tagContextMenu = document.getElementById('tag-menu'); ev.preventDefault(); @@ -244,7 +503,7 @@ Zotero.TagSelector = class TagSelectorContainer extends React.Component { var elem = event.target; // Ignore drops not on tags - if (elem.localName != 'li') { + if (!elem.classList.contains('tag-selector-item')) { return; } @@ -262,7 +521,7 @@ Zotero.TagSelector = class TagSelectorContainer extends React.Component { var elem = event.target; // Ignore drops not on tags - if (elem.localName != 'li') { + if (!elem.classList.contains('tag-selector-item')) { return; } @@ -346,8 +605,8 @@ Zotero.TagSelector = class TagSelectorContainer extends React.Component { let selectedTags = this.selectedTags; if (selectedTags.has(this.contextTag.name)) { - var wasSelected = true; selectedTags.delete(this.contextTag.name); + selectedTags.add(newName.value); } if (Zotero.Tags.getID(this.contextTag.name)) { @@ -363,11 +622,6 @@ Zotero.TagSelector = class TagSelectorContainer extends React.Component { await Zotero.Tags.setColor(this.libraryID, this.contextTag.name, false); await Zotero.Tags.setColor(this.libraryID, newName.value, color.color); } - - if (wasSelected) { - selectedTags.add(newName.value); - } - this.setState({tags: await this.getTags()}) } async openDeletePrompt() { @@ -391,15 +645,13 @@ Zotero.TagSelector = class TagSelectorContainer extends React.Component { else { await Zotero.Tags.setColor(this.libraryID, this.contextTag.name, false); } - - this.setState({tags: await this.getTags()}); } async toggleDisplayAllTags(newValue) { newValue = typeof(newValue) === 'undefined' ? !this.displayAllTags : newValue; Zotero.Prefs.set('tagSelector.displayAllTags', newValue); this.displayAllTags = newValue; - this.setState({tags: await this.getTags()}); + this.setState(await this.getTagsAndScope()); } toggleShowAutomatic(newValue) { @@ -474,5 +726,19 @@ Zotero.TagSelector = class TagSelectorContainer extends React.Component { ref.domEl = domEl; return ref; } -} -})(); + + uninit() { + ReactDOM.unmountComponentAtNode(this.domEl); + if (this._notifierID) { + Zotero.Notifier.unregisterObserver(this._notifierID); + } + if (this._prefObserverID) { + Zotero.Prefs.unregisterObserver('fontSize', this.handleFontChange.bind(this)); + } + } + + static propTypes = { + container: PropTypes.string.isRequired, + onSelection: PropTypes.func.isRequired, + }; +}; diff --git a/chrome/content/zotero/xpcom/collectionTreeRow.js b/chrome/content/zotero/xpcom/collectionTreeRow.js index 07068df974..d86bc5ab27 100644 --- a/chrome/content/zotero/xpcom/collectionTreeRow.js +++ b/chrome/content/zotero/xpcom/collectionTreeRow.js @@ -366,13 +366,17 @@ Zotero.CollectionTreeRow.prototype.getSearchObject = Zotero.Promise.coroutine(fu return s2; }); +Zotero.CollectionTreeRow.prototype.getChildTags = function () { + Zotero.warn("Zotero.CollectionTreeRow::getChildTags() is deprecated -- use getTags() instead"); + return this.getTags(); +}; /** * Returns all the tags used by items in the current view * * @return {Promise} */ -Zotero.CollectionTreeRow.prototype.getChildTags = Zotero.Promise.coroutine(function* () { +Zotero.CollectionTreeRow.prototype.getTags = async function (types, tagIDs) { switch (this.type) { // TODO: implement? case 'share': @@ -381,9 +385,9 @@ Zotero.CollectionTreeRow.prototype.getChildTags = Zotero.Promise.coroutine(funct case 'bucket': return []; } - var results = yield this.getSearchResults(true); - return Zotero.Tags.getAllWithinSearchResults(results); -}); + var results = await this.getSearchResults(true); + return Zotero.Tags.getAllWithin({ tmpTable: results, types, tagIDs }); +}; Zotero.CollectionTreeRow.prototype.setSearch = function (searchText) { diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js index 6880bf5997..1a8932606a 100644 --- a/chrome/content/zotero/xpcom/data/item.js +++ b/chrome/content/zotero/xpcom/data/item.js @@ -1794,12 +1794,14 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) { for (let i=0; i} A promise for an array containing tag objects in API JSON format * [{ { tag: "foo" }, { tag: "bar", type: 1 }] */ - this.getAll = Zotero.Promise.coroutine(function* (libraryID, types) { - var sql = "SELECT DISTINCT name AS tag, type FROM tags " - + "JOIN itemTags USING (tagID) JOIN items USING (itemID) WHERE libraryID=?"; - var params = [libraryID]; - if (types) { - sql += " AND type IN (" + types.join() + ")"; - } - var rows = yield Zotero.DB.queryAsync(sql, params); - return rows.map((row) => this.cleanData(row)); - }); + this.getAll = async function (libraryID, types) { + return this.getAllWithin({ libraryID, types }); + }; /** * Get all tags within the items of a temporary table of search results * - * @param {String} tmpTable Temporary table with items to use - * @param {Array} [types] Array of tag types to fetch - * @return {Promise} Promise for object with tag data in API JSON format, keyed by tagID + * @param {Object} + * @param {Object.Number} libraryID + * @param {Object.String} tmpTable - Temporary table with items to use + * @param {Object.Number[]} [types] - Array of tag types to fetch + * @param {Object.Number[]} [tagIDs] - Array of tagIDs to limit the result to + * @return {Promise} - Promise for an array of tag objects in API JSON format */ - this.getAllWithinSearchResults = Zotero.Promise.coroutine(function* (tmpTable, types) { - var sql = "SELECT DISTINCT name AS tag, type FROM itemTags " - + "JOIN tags USING (tagID) WHERE itemID IN " - + "(SELECT itemID FROM " + tmpTable + ") "; - if (types) { - sql += "AND type IN (" + types.join() + ") "; + this.getAllWithin = async function ({ libraryID, tmpTable, types, tagIDs }) { + // mozStorage/Proxy are slow, so get in a single column + var sql = "SELECT DISTINCT tagID || ':' || type FROM itemTags " + + "JOIN tags USING (tagID) "; + var params = []; + if (libraryID) { + sql += "JOIN items USING (itemID) WHERE libraryID = ? "; + params.push(libraryID); } - var rows = yield Zotero.DB.queryAsync(sql); - return rows.map((row) => this.cleanData(row)); - }); + else { + sql += "WHERE 1 "; + } + if (tmpTable) { + if (libraryID) { + throw new Error("tmpTable and libraryID are mutually exclusive"); + } + sql += "AND itemID IN (SELECT itemID FROM " + tmpTable + ") "; + } + if (types && types.length) { + sql += "AND type IN (" + new Array(types.length).fill('?').join(', ') + ") "; + params.push(...types); + } + if (tagIDs) { + sql += "AND tagID IN (" + new Array(tagIDs.length).fill('?').join(', ') + ") "; + params.push(...tagIDs); + } + // Not a perfect locale sort, but speeds up the sort in the tag selector later without any + // discernible performance cost + sql += "ORDER BY name COLLATE NOCASE"; + var rows = await Zotero.DB.columnQueryAsync(sql, params); + return rows.map((row) => { + var [tagID, type] = row.split(':'); + return this.cleanData({ + tag: Zotero.Tags.getName(parseInt(tagID)), + type: type + }); + }); + }; /** diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js index 6c9bb8d9b5..2687c6d8c0 100644 --- a/chrome/content/zotero/zoteroPane.js +++ b/chrome/content/zotero/zoteroPane.js @@ -105,7 +105,7 @@ var ZoteroPane = new function() this.updateWindow(); this.updateToolbarPosition(); }); - window.setTimeout(ZoteroPane_Local.updateToolbarPosition, 0); + window.setTimeout(this.updateToolbarPosition.bind(this), 0); Zotero.updateQuickSearchBox(document); @@ -1103,13 +1103,21 @@ var ZoteroPane = new function() this.tagSelector = Zotero.TagSelector.init( document.getElementById('zotero-tag-selector'), { - onSelection: this.updateTagFilter.bind(this) + container: 'zotero-tag-selector-container', + onSelection: this.updateTagFilter.bind(this), } ); } }; + this.handleTagSelectorResize = Zotero.Utilities.debounce(function() { + if (this.tagSelectorShown()) { + this.tagSelector.handleResize(); + } + }, 100); + + /* * Sets the tag filter on the items view */ @@ -1120,7 +1128,7 @@ var ZoteroPane = new function() }); - this.toggleTagSelector = Zotero.Promise.coroutine(function* () { + this.toggleTagSelector = function () { var container = document.getElementById('zotero-tag-selector-container'); var showing = container.getAttribute('collapsed') == 'true'; container.setAttribute('collapsed', !showing); @@ -1129,14 +1137,15 @@ var ZoteroPane = new function() // and focus filter textbox if (showing) { this.initTagSelector(); - yield this.setTagScope(); ZoteroPane.tagSelector.focusTextbox(); + this.setTagScope(); } // If hiding, clear selection else { ZoteroPane.tagSelector.uninit(); + ZoteroPane.tagSelector = null; } - }); + }; this.tagSelectorShown = function () { @@ -1153,7 +1162,7 @@ var ZoteroPane = new function() * * Passed to the items tree to trigger on changes */ - this.setTagScope = async function () { + this.setTagScope = function () { var collectionTreeRow = self.getCollectionTreeRow(); if (self.tagSelectorShown()) { if (collectionTreeRow.editable) { @@ -1163,9 +1172,8 @@ var ZoteroPane = new function() ZoteroPane_Local.tagSelector.setMode('view'); } ZoteroPane_Local.tagSelector.onItemViewChanged({ - collectionTreeRow, libraryID: collectionTreeRow.ref.libraryID, - tagsInScope: await collectionTreeRow.getChildTags() + collectionTreeRow }); } }; @@ -4849,8 +4857,9 @@ var ZoteroPane = new function() var itemToolbar = document.getElementById("zotero-item-toolbar"); var tagSelector = document.getElementById("zotero-tag-selector"); - collectionsToolbar.style.width = collectionsPane.boxObject.width + 'px'; - tagSelector.style.maxWidth = collectionsPane.boxObject.width + 'px'; + var collectionsPaneWidth = collectionsPane.boxObject.width + 'px'; + collectionsToolbar.style.width = collectionsPaneWidth; + tagSelector.style.maxWidth = collectionsPaneWidth; if (stackedLayout || itemPane.collapsed) { // The itemsToolbar and itemToolbar share the same space, and it seems best to use some flex attribute from right (because there might be other icons appearing or vanishing). @@ -4876,6 +4885,8 @@ var ZoteroPane = new function() // Allow item pane to shrink to available height in stacked mode, but don't expand to be too // wide when there's no persisted width in non-stacked mode itemPane.setAttribute("flex", stackedLayout ? 1 : 0); + + this.handleTagSelectorResize(); } /** diff --git a/chrome/content/zotero/zoteroPane.xul b/chrome/content/zotero/zoteroPane.xul index 590fa490d2..66809b8895 100644 --- a/chrome/content/zotero/zoteroPane.xul +++ b/chrome/content/zotero/zoteroPane.xul @@ -306,8 +306,12 @@ ondragover="return ZoteroPane_Local.collectionsView.onDragOver(event)" ondrop="return ZoteroPane_Local.collectionsView.onDrop(event)"/> - +