diff --git a/chrome/content/zotero/components/tagSelector/tagSelectorList.jsx b/chrome/content/zotero/components/tagSelector/tagSelectorList.jsx index e9d6f826c0..5e88c40099 100644 --- a/chrome/content/zotero/components/tagSelector/tagSelectorList.jsx +++ b/chrome/content/zotero/components/tagSelector/tagSelectorList.jsx @@ -125,7 +125,7 @@ class TagList extends React.PureComponent { forceNewLine = true; } // size of the colored dot + space between the dot and the tag name always sums up to fontSize (e.g., 8px + 3px at 11px fontSize) - const tagColorWidth = (tag.color && !Zotero.Utilities.Internal.isOnlyEmoji(tag.name)) ? this.props.fontSize : 0; + const tagColorWidth = (tag.color && !Zotero.Utilities.Internal.containsEmoji(tag.name)) ? this.props.fontSize : 0; let tagWidth = tagPaddingLeft + Math.min(tag.width, tagMaxWidth) + tagPaddingRight + tagColorWidth; // If first row or cell fits, add to current row if (!forceNewLine && (i == 0 || ((rowX + tagWidth) < (this.props.width - panePaddingRight - this.scrollbarWidth)))) { @@ -175,7 +175,7 @@ class TagList extends React.PureComponent { if (tag.disabled) { className += ' disabled'; } - if (Zotero.Utilities.Internal.isOnlyEmoji(tag.name)) { + if (Zotero.Utilities.Internal.containsEmoji(tag.name)) { className += ' emoji'; } diff --git a/chrome/content/zotero/elements/tagsBox.js b/chrome/content/zotero/elements/tagsBox.js index 312367e2b1..c5579ebec3 100644 --- a/chrome/content/zotero/elements/tagsBox.js +++ b/chrome/content/zotero/elements/tagsBox.js @@ -140,22 +140,10 @@ var tags = this.item.getTags(); - // Sort tags alphabetically - var collation = Zotero.getLocaleCollation(); - tags.sort((a, b) => { - let aTag = a.tag; - let bTag = b.tag; - let aHasColor = this._tagColors.has(aTag); - let bHasColor = this._tagColors.has(bTag); - // Sort colored tags to the top - if (aHasColor && !bHasColor) { - return -1; - } - if (!aHasColor && bHasColor) { - return 1; - } - return collation.compareString(1, aTag, bTag); - }); + + // Sort tags alphabetically with colored tags at the top followed by emoji tags + tags.sort((a, b) => Zotero.Tags.compareTagsOrder(this.item.libraryID, a.tag, b.tag)); + for (let i = 0; i < tags.length; i++) { this.addDynamicRow(tags[i], i + 1); @@ -523,24 +511,18 @@ row = this.addDynamicRow(tagData, false, true); var elem = row.getElementsByAttribute('fieldname', 'tag')[0]; - // Move row to appropriate place, alphabetically - var collation = Zotero.getLocaleCollation(); - var tagEditables = rowsElement.getElementsByAttribute('fieldname', 'tag'); - - var inserted = false; - for (let editable of tagEditables) { - // Sort tags without colors below tags with colors - if (!color && this._tagColors.has(editable.value) - || editable.value && collation.compareString(1, tagName, editable.value) > 0) { - continue; - } - - rowsElement.insertBefore(row, editable.parentNode); - inserted = true; - break; + // Construct what the array of tags would be if this tag was a part of it + let newTagsArray = this.item.getTags(); + newTagsArray.push({ tag: tagName, color: color || null }); + // Sort it with the colored tags on top, followed by emoji tags, followed by everything else + newTagsArray.sort((a, b) => Zotero.Tags.compareTagsOrder(this._item.libraryID, a.tag, b.tag)); + // Find where the new tag should be placed and insert it there + let newTagIndex = newTagsArray.findIndex(tag => tag.tag == tagName); + if (newTagIndex < rowsElement.childNodes.length) { + rowsElement.insertBefore(row, rowsElement.childNodes[newTagIndex]); } - if (!inserted) { - rowsElement.appendChild(row); + else { + rowsElement.append(row); } this.updateCount(this.count + 1); diff --git a/chrome/content/zotero/itemTree.jsx b/chrome/content/zotero/itemTree.jsx index 7d0dd50b8e..2d616699e3 100644 --- a/chrome/content/zotero/itemTree.jsx +++ b/chrome/content/zotero/itemTree.jsx @@ -2801,15 +2801,14 @@ var ItemTree = class ItemTree extends LibraryTree { let tagAriaLabel = ''; let tagSpans = []; - let coloredTags = item.getColoredTags(); + let coloredTags = item.getItemsListTags(); if (coloredTags.length) { let { emoji, colored } = coloredTags.reduce((acc, tag) => { - acc[Zotero.Utilities.Internal.isOnlyEmoji(tag.tag) ? 'emoji' : 'colored'].push(tag); + acc[Zotero.Utilities.Internal.containsEmoji(tag.tag) ? 'emoji' : 'colored'].push(tag); return acc; }, { emoji: [], colored: [] }); - tagSpans.push(...emoji.map(x => this._getTagSwatch(x.tag))); - + // Add colored tags first if (colored.length) { let coloredTagSpans = colored.map(x => this._getTagSwatch(x.tag, x.color)); let coloredTagSpanWrapper = document.createElement('span'); @@ -2817,6 +2816,9 @@ var ItemTree = class ItemTree extends LibraryTree { coloredTagSpanWrapper.append(...coloredTagSpans); tagSpans.push(coloredTagSpanWrapper); } + + // Add emoji tags after + tagSpans.push(...emoji.map(x => this._getTagSwatch(x.tag))); tagAriaLabel = coloredTags.length == 1 ? Zotero.getString('searchConditions.tag') : Zotero.getString('itemFields.tags'); tagAriaLabel += ' ' + coloredTags.map(x => x.tag).join(', ') + '.'; @@ -3864,12 +3866,13 @@ var ItemTree = class ItemTree extends LibraryTree { _getTagSwatch(tag, color) { let span = document.createElement('span'); span.className = 'tag-swatch'; - // If only emoji, display directly + let extractedEmojis = Zotero.Tags.extractEmojiForItemsList(tag); + // If contains emojis, display directly // // TODO: Check for a maximum number of graphemes, which is hard to do // https://stackoverflow.com/a/54369605 - if (Zotero.Utilities.Internal.isOnlyEmoji(tag)) { - span.textContent = tag; + if (extractedEmojis) { + span.textContent = extractedEmojis; span.className += ' emoji'; } // Otherwise display color diff --git a/chrome/content/zotero/xpcom/data/dataObjectUtilities.js b/chrome/content/zotero/xpcom/data/dataObjectUtilities.js index bc4ec601e2..2ba4b6875d 100644 --- a/chrome/content/zotero/xpcom/data/dataObjectUtilities.js +++ b/chrome/content/zotero/xpcom/data/dataObjectUtilities.js @@ -739,7 +739,7 @@ Zotero.DataObjectUtilities = { numNotes: () => 0, isAttachment: () => false, numAttachments: () => false, - getColoredTags: () => false, + getItemsListTags: () => [], isRegularItem: () => false, // Should be false to prevent items dropped into deleted searches getNotes: () => [], getAttachments: () => [], diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js index 9ce1ccfd89..41fdf91349 100644 --- a/chrome/content/zotero/xpcom/data/item.js +++ b/chrome/content/zotero/xpcom/data/item.js @@ -4535,33 +4535,20 @@ Zotero.Item.prototype.getItemTypeIconName = function (skipLinkMode = false) { }; -Zotero.Item.prototype.getTagColors = function () { - Zotero.warn("Zotero.Item::getTagColors() is deprecated -- use Zotero.Item::getColoredTags()"); - return this.getColoredTags().map(x => x.color); -}; - - /** - * Return tags and colors + * Return tags with assigned colors and tags that contain emojis * * @return {Object[]} - Array of object with 'tag' and 'color' properties */ -Zotero.Item.prototype.getColoredTags = function () { +Zotero.Item.prototype.getItemsListTags = function () { var tags = this.getTags(); if (!tags.length) return []; - - let colorData = []; let tagColors = Zotero.Tags.getColors(this.libraryID); - for (let tag of tags) { - let data = tagColors.get(tag.tag); - if (data) { - colorData.push({tag: tag.tag, ...data}); - } - } - return colorData.sort((a, b) => a.position - b.position).map(x => ({ tag: x.tag, color: x.color })); + let colorOrEmojiTags = tags.filter(tag => tagColors.get(tag.tag) || Zotero.Utilities.Internal.containsEmoji(tag.tag)); + colorOrEmojiTags.sort((a, b) => Zotero.Tags.compareTagsOrder(this.libraryID, a.tag, b.tag)); + return colorOrEmojiTags.map(x => ({ tag: x.tag, color: tagColors.get(x.tag)?.color || null })); }; - /** * Compares this item to another * diff --git a/chrome/content/zotero/xpcom/data/tags.js b/chrome/content/zotero/xpcom/data/tags.js index c1d1702924..5a534bc76f 100644 --- a/chrome/content/zotero/xpcom/data/tags.js +++ b/chrome/content/zotero/xpcom/data/tags.js @@ -710,6 +710,10 @@ Zotero.Tags = new function() { else { tagColors.splice(position, 0, newObj); } + _libraryColorsByName[libraryID].set(name, { + color: color, + position: position + }); } if (tagColors.length) { @@ -991,7 +995,35 @@ Zotero.Tags = new function() { ctx.fill(); } } - + + // Return the first sequence of emojis from a string + this.extractEmojiForItemsList = function (str) { + // Split by anything that is not an emoji, Zero Width Joiner, or Variation Selector-16 + // And return first continuous span of emojis + let re = /[^\p{Extended_Pictographic}\u200D\uFE0F]+/gu; + return str.split(re).filter(Boolean)[0] || null; + }; + + // Used as parameter for .sort() method on an array of tags + // Orders colored tags first by their position + // Then order tags with emojis alphabetically. + // Then order all remaining tags alphabetically + this.compareTagsOrder = function (libraryID, tagA, tagB) { + var collation = Zotero.getLocaleCollation(); + let tagColors = this.getColors(libraryID); + let colorForA = tagColors.get(tagA); + let colorForB = tagColors.get(tagB); + if (colorForA && !colorForB) return -1; + if (!colorForA && colorForB) return 1; + if (colorForA && colorForB) { + return colorForA.position - colorForB.position; + } + let emojiForA = Zotero.Utilities.Internal.containsEmoji(tagA); + let emojiForB = Zotero.Utilities.Internal.containsEmoji(tagB); + if (emojiForA && !emojiForB) return -1; + if (!emojiForA && emojiForB) return 1; + return collation.compareString(1, tagA, tagB); + }; /** * Compare two API JSON tag objects diff --git a/chrome/content/zotero/xpcom/utilities_internal.js b/chrome/content/zotero/xpcom/utilities_internal.js index 20464b4f87..eb000e99e3 100644 --- a/chrome/content/zotero/xpcom/utilities_internal.js +++ b/chrome/content/zotero/xpcom/utilities_internal.js @@ -396,10 +396,9 @@ Zotero.Utilities.Internal = { return s; }, - isOnlyEmoji: function (str) { - // Remove emoji, Zero Width Joiner, and Variation Selector-16 and see if anything's left - const re = /\p{Extended_Pictographic}|\u200D|\uFE0F/gu; - return !str.replace(re, ''); + containsEmoji: function (str) { + let re = /\p{Extended_Pictographic}/gu; + return !!str.match(re); }, includesEmoji: function (str) { diff --git a/test/tests/itemTest.js b/test/tests/itemTest.js index 5029628f40..c32ec017fd 100644 --- a/test/tests/itemTest.js +++ b/test/tests/itemTest.js @@ -1849,6 +1849,49 @@ describe("Zotero.Item", function () { assert.sameDeepMembers(tags, [{ tag: 'a' }, { tag: 'b' }]); }) }) + + describe("#getItemsListTags", function() { + it("should return tags with emojis after colored tags", async function () { + var tags = [ + { + tag: "BBB ⭐️⭐️" + }, + { + tag: "ZZZ πŸ‘²" + }, + { + tag: "colored tag two" + }, + { + tag: "AAA πŸ˜€" + }, + { + tag: "colored tag one" + }, + { + tag: "not included" + } + ]; + await Zotero.Tags.setColor(Zotero.Libraries.userLibraryID, "colored tag one", "#990000"); + await Zotero.Tags.setColor(Zotero.Libraries.userLibraryID, "colored tag two", "#FF6666"); + + var item = new Zotero.Item('journalArticle'); + item.setTags(tags); + await item.saveTx(); + + var itemListTags = item.getItemsListTags(); + var expected = [ + { tag: "colored tag one", color: "#990000" }, + { tag: "colored tag two", color: "#FF6666" }, + { tag: "AAA πŸ˜€", color: null }, + { tag: "BBB ⭐️⭐️", color: null }, + { tag: "ZZZ πŸ‘²", color: null }, + ]; + for (let i = 0; i < 5; i++) { + assert.deepEqual(itemListTags[i], expected[i]); + } + }); + }); // // Relations and related items diff --git a/test/tests/tagsTest.js b/test/tests/tagsTest.js index 849a3167eb..e3b580b9d0 100644 --- a/test/tests/tagsTest.js +++ b/test/tests/tagsTest.js @@ -222,4 +222,56 @@ describe("Zotero.Tags", function () { ]); }); }); -}) + + describe("#extractEmojiForItemsList()", function () { + it("should return first emoji span", function () { + assert.equal(Zotero.Tags.extractEmojiForItemsList("🐩🐩🐩 🐩🐩🐩🐩"), "🐩🐩🐩"); + }); + it("should return first emoji span when string doesn't start with emoji", function () { + assert.equal(Zotero.Tags.extractEmojiForItemsList("./'!@#$ 🐩🐩🐩 🐩🐩🐩🐩"), "🐩🐩🐩"); + }); + + it("should return first emoji span for text with an emoji with Variation Selector-16", function () { + assert.equal(Zotero.Tags.extractEmojiForItemsList("Here are ⭐️⭐️⭐️⭐️⭐️"), "⭐️⭐️⭐️⭐️⭐️"); + }); + + it("should return first emoji span for text with an emoji made up of multiple characters with ZWJ", function () { + assert.equal(Zotero.Tags.extractEmojiForItemsList("We are πŸ‘¨β€πŸŒΎπŸ‘¨β€πŸŒΎ. And I am a πŸ‘¨β€πŸ«."), "πŸ‘¨β€πŸŒΎπŸ‘¨β€πŸŒΎ"); + }); + }); + + describe("#compareTagsOrder()", function () { + it('should order colored tags by position and other tags - alphabetically', async function () { + var libraryID = Zotero.Libraries.userLibraryID; + await createDataObject('item', { + tags: [ + { tag: 'one' }, + { tag: 'two', type: 1 }, + { tag: 'three' }, + { tag: 'four', type: 1 }, + { tag: 'five' }, + { tag: 'sixπŸ˜€' }, + { tag: 'sevenπŸ˜€' } + ] + }); + await Zotero.Tags.setColor(libraryID, 'three', '#111111', 0); + await Zotero.Tags.setColor(libraryID, 'four', '#222222', 1); + await Zotero.Tags.setColor(libraryID, 'two', '#222222', 2); + + assert.equal(Zotero.Tags.compareTagsOrder(libraryID, 'three', 'one'), -1, "colored vs ordinary tag -> -1"); + assert.equal(Zotero.Tags.compareTagsOrder(libraryID, 'one', 'three'), 1, "ordinary vs colored -> 1"); + + assert.equal(Zotero.Tags.compareTagsOrder(libraryID, 'three', 'sixπŸ˜€'), -1, "colored vs emoji tag -> -1"); + assert.equal(Zotero.Tags.compareTagsOrder(libraryID, 'sixπŸ˜€', 'three'), 1, "emoji vs colored tag -> 1"); + + assert.equal(Zotero.Tags.compareTagsOrder(libraryID, 'two', 'three'), 2, "colored vs colored => compare their positions"); + + + assert.equal(Zotero.Tags.compareTagsOrder(libraryID, 'one', 'sixπŸ˜€'), 1, "ordinary tag vs tag with emoji -> 1"); + assert.equal(Zotero.Tags.compareTagsOrder(libraryID, 'sixπŸ˜€', 'one'), -1, "tag with emoji vs ordinary tag -> -1"); + + assert.equal(Zotero.Tags.compareTagsOrder(libraryID, 'sixπŸ˜€', 'sevenπŸ˜€'), 1, "both emoji tags -> alphabetical"); + assert.isAbove(Zotero.Tags.compareTagsOrder(libraryID, 'one', 'five'), 0, "ordinary tag vs ordinary tag -> alphabetical"); + }); + }); +}); diff --git a/test/tests/tagsboxTest.js b/test/tests/tagsboxTest.js index 4891a1ceba..b7b8525b90 100644 --- a/test/tests/tagsboxTest.js +++ b/test/tests/tagsboxTest.js @@ -143,4 +143,78 @@ describe("Item Tags Box", function () { assert.equal(rows.length, 0); }) }) + + describe("#render", function() { + it("should render colored tags followed by emoji tags followed by ordinary tags", async function() { + let item = await createDataObject('item', { + tags: [ + { tag: 'A_usual_tag' }, + { tag: 'B_usual_tag' }, + { tag: 'C_emoji_tagπŸ˜€' }, + { tag: 'D_emoji_tagπŸ˜€' }, + { tag: 'E_colored_tag' }, + { tag: 'F_colored_tag' }, + ] + }); + + await Zotero.Tags.setColor(item.libraryID, 'F_colored_tag', '#111111', 0); + await Zotero.Tags.setColor(item.libraryID, 'E_colored_tag', '#222222', 1); + + var tagsbox = doc.querySelector('#zotero-editpane-tags'); + var tagRows = [...tagsbox.querySelectorAll(".row")]; + + // Colored tags sorted first by their position + assert.equal(tagRows[0].querySelector("editable-text").value, "F_colored_tag"); + assert.equal(tagRows[1].querySelector("editable-text").value, "E_colored_tag"); + // Followed by emoji tags sorted alphabetically + assert.equal(tagRows[2].querySelector("editable-text").value, "C_emoji_tagπŸ˜€"); + assert.equal(tagRows[3].querySelector("editable-text").value, "D_emoji_tagπŸ˜€"); + // Followed by remaining tags sorted alphabetically + assert.equal(tagRows[4].querySelector("editable-text").value, "A_usual_tag"); + assert.equal(tagRows[5].querySelector("editable-text").value, "B_usual_tag"); + }); + + it("should add a new tag at the correct position", async function () { + // Create a colored tag that the item does not have + await createDataObject('item', { + tags: [ + { tag: 'a_colored_tag' }, + ] + }); + + // Create item with a lot of tags - colored, emoji and usual + let item = await createDataObject('item', { + tags: [ + { tag: 'AA_usual_tag' }, + { tag: 'BB_usual_tag' }, + { tag: 'CC_emoji_tagπŸ˜€' }, + { tag: 'DD_emoji_tagπŸ˜€' }, + { tag: 'EE_colored_tag' }, + { tag: 'FF_colored_tag' }, + ] + }); + + await Zotero.Tags.setColor(item.libraryID, 'FF_colored_tag', '#111111', 0); + await Zotero.Tags.setColor(item.libraryID, 'EE_colored_tag', '#222222', 1); + await Zotero.Tags.setColor(item.libraryID, 'a_colored_tag', '#222222', 2); + + var tagsbox = doc.querySelector('#zotero-editpane-tags'); + var tagRows; + + // should be added above all usual tags but below colored and emoji + tagsbox.add("a_usual_tag"); + tagRows = [...tagsbox.querySelectorAll(".row")]; + assert.equal(tagRows[4].querySelector("editable-text").value, "a_usual_tag"); + + // should be added below colored tags above all other emoji tags + tagsbox.add("a_emoji_tagπŸ˜€"); + tagRows = [...tagsbox.querySelectorAll(".row")]; + assert.equal(tagRows[2].querySelector("editable-text").value, "a_emoji_tagπŸ˜€"); + + // should be added at the position of the colored tag + tagsbox.add("a_colored_tag"); + tagRows = [...tagsbox.querySelectorAll(".row")]; + assert.equal(tagRows[2].querySelector("editable-text").value, "a_colored_tag"); + }); + }); }) diff --git a/test/tests/utilities_internalTest.js b/test/tests/utilities_internalTest.js index d2f209cb1e..3f3c413c84 100644 --- a/test/tests/utilities_internalTest.js +++ b/test/tests/utilities_internalTest.js @@ -86,25 +86,24 @@ describe("Zotero.Utilities.Internal", function () { }); - describe("#isOnlyEmoji()", function () { - it("should return true for emoji", function () { - assert.isTrue(Zotero.Utilities.Internal.isOnlyEmoji("🐩")); + describe("#containsEmoji()", function () { + it("should return true for text with an emoji", function () { + assert.isTrue(Zotero.Utilities.Internal.containsEmoji("🐩 Hello 🐩")); }); - it("should return true for emoji with text representation that use Variation Selector-16", function () { - assert.isTrue(Zotero.Utilities.Internal.isOnlyEmoji("⭐️")); + it("should return true for text with an emoji with text representation that use Variation Selector-16", function () { + assert.isTrue(Zotero.Utilities.Internal.containsEmoji("This is a ⭐️")); }); - it("should return true for emoji made up of multiple characters with ZWJ", function () { - assert.isTrue(Zotero.Utilities.Internal.isOnlyEmoji("πŸ‘¨β€πŸŒΎ")); + it("should return true for text with an emoji made up of multiple characters with ZWJ", function () { + assert.isTrue(Zotero.Utilities.Internal.containsEmoji("I am a πŸ‘¨β€πŸŒΎ")); }); it("should return false for integer", function () { - assert.isFalse(Zotero.Utilities.Internal.isOnlyEmoji("0")); + assert.isFalse(Zotero.Utilities.Internal.containsEmoji("0")); }); }); - describe("#delayGenerator", function () { var spy;