From 082fd2286275a8cdfaf51fa48ddfbae07e8ad3b9 Mon Sep 17 00:00:00 2001 From: Robert Hunt <rhunt@gitlab.com> Date: Fri, 10 Jun 2022 13:02:36 +0000 Subject: [PATCH] Added intents.json to act as a multiplier to the emojis score When the emoji are retrieved, it will apply the intents multiplier to the score if one exists. This will either decrease or increase the score to force it to float to the top or the bottom of the pile. When a multiplier doesn't exist the code multiplies by 1. Updated the awards logic to do the same thing but in jQuery. Changelog: added MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/88407 --- app/assets/javascripts/awards_handler.js | 40 ++++++- app/assets/javascripts/emoji/constants.js | 2 + app/assets/javascripts/emoji/index.js | 18 +-- app/assets/javascripts/emoji/utils.js | 8 ++ app/assets/javascripts/gfm_auto_complete.js | 2 +- doc/development/fe_guide/emojis.md | 4 + fixtures/emojis/intents.json | 16 +++ spec/frontend/__helpers__/emoji.js | 10 ++ spec/frontend/awards_handler_spec.js | 29 +++++ spec/frontend/emoji/index_spec.js | 115 +++++++++++++------- spec/frontend/emoji/utils_spec.js | 15 +++ 11 files changed, 205 insertions(+), 54 deletions(-) create mode 100644 app/assets/javascripts/emoji/utils.js create mode 100644 fixtures/emojis/intents.json create mode 100644 spec/frontend/emoji/utils_spec.js diff --git a/app/assets/javascripts/awards_handler.js b/app/assets/javascripts/awards_handler.js index aa735df7da5a9..a030797c69836 100644 --- a/app/assets/javascripts/awards_handler.js +++ b/app/assets/javascripts/awards_handler.js @@ -3,9 +3,9 @@ import { GlBreakpointInstance as bp } from '@gitlab/ui/dist/utils'; import $ from 'jquery'; import { uniq } from 'lodash'; +import { getEmojiScoreWithIntent } from '~/emoji/utils'; import { getCookie, setCookie, scrollToElement } from '~/lib/utils/common_utils'; import * as Emoji from '~/emoji'; - import { dispose, fixTitle } from '~/tooltips'; import createFlash from './flash'; import axios from './lib/utils/axios_utils'; @@ -559,13 +559,45 @@ export class AwardsHandler { } } + getEmojiScore(emojis, value) { + const elem = $(value).find('[data-name]').get(0); + const emoji = emojis.filter((x) => x.emoji.name === elem.dataset.name)[0]; + elem.dataset.score = emoji.score; + + return emoji.score; + } + + sortEmojiElements(emojis, $elements) { + const scores = new WeakMap(); + + return $elements.sort((a, b) => { + let aScore = scores.get(a); + let bScore = scores.get(b); + + if (!aScore) { + aScore = this.getEmojiScore(emojis, a); + scores.set(a, aScore); + } + + if (!bScore) { + bScore = this.getEmojiScore(emojis, b); + scores.set(b, bScore); + } + + return aScore - bScore; + }); + } + findMatchingEmojiElements(query) { - const emojiMatches = this.emoji.searchEmoji(query).map((x) => x.emoji.name); + const matchingEmoji = this.emoji + .searchEmoji(query) + .map((x) => ({ ...x, score: getEmojiScoreWithIntent(x.emoji.name, x.score) })); + const matchingEmojiNames = matchingEmoji.map((x) => x.emoji.name); const $emojiElements = $('.emoji-menu-list:not(.frequent-emojis) [data-name]'); const $matchingElements = $emojiElements.filter( - (i, elm) => emojiMatches.indexOf(elm.dataset.name) >= 0, + (i, elm) => matchingEmojiNames.indexOf(elm.dataset.name) >= 0, ); - return $matchingElements.closest('li').clone(); + return this.sortEmojiElements(matchingEmoji, $matchingElements.closest('li').clone()); } /* showMenuElement and hideMenuElement are performance optimizations. We use diff --git a/app/assets/javascripts/emoji/constants.js b/app/assets/javascripts/emoji/constants.js index a6eb42565612d..7970a93209529 100644 --- a/app/assets/javascripts/emoji/constants.js +++ b/app/assets/javascripts/emoji/constants.js @@ -19,3 +19,5 @@ export const CATEGORY_ROW_HEIGHT = 37; export const CACHE_VERSION_KEY = 'gl-emoji-map-version'; export const CACHE_KEY = 'gl-emoji-map'; + +export const NEUTRAL_INTENT_MULTIPLIER = 1; diff --git a/app/assets/javascripts/emoji/index.js b/app/assets/javascripts/emoji/index.js index 4fdcdcc1b04fe..b9392fabcbdc9 100644 --- a/app/assets/javascripts/emoji/index.js +++ b/app/assets/javascripts/emoji/index.js @@ -2,6 +2,7 @@ import { escape, minBy } from 'lodash'; import emojiRegexFactory from 'emoji-regex'; import emojiAliases from 'emojis/aliases.json'; import { setAttributes } from '~/lib/utils/dom_utils'; +import { getEmojiScoreWithIntent } from '~/emoji/utils'; import AccessorUtilities from '../lib/utils/accessor'; import axios from '../lib/utils/axios_utils'; import { CACHE_KEY, CACHE_VERSION_KEY, CATEGORY_ICON_MAP, FREQUENTLY_USED_KEY } from './constants'; @@ -144,6 +145,11 @@ function getNameMatch(emoji, query) { return null; } +// Sort emoji by emoji score falling back to a string comparison +export function sortEmoji(a, b) { + return a.score - b.score || a.fieldValue.localeCompare(b.fieldValue); +} + export function searchEmoji(query) { const lowercaseQuery = query ? `${query}`.toLowerCase() : ''; @@ -156,16 +162,14 @@ export function searchEmoji(query) { getDescriptionMatch(emoji, lowercaseQuery), getAliasMatch(emoji, matchingAliases), getNameMatch(emoji, lowercaseQuery), - ].filter(Boolean); + ] + .filter(Boolean) + .map((x) => ({ ...x, score: getEmojiScoreWithIntent(x.emoji.name, x.score) })); return minBy(matches, (x) => x.score); }) - .filter(Boolean); -} - -export function sortEmoji(items) { - // Sort results by index of and string comparison - return [...items].sort((a, b) => a.score - b.score || a.fieldValue.localeCompare(b.fieldValue)); + .filter(Boolean) + .sort(sortEmoji); } export const CATEGORY_NAMES = Object.keys(CATEGORY_ICON_MAP); diff --git a/app/assets/javascripts/emoji/utils.js b/app/assets/javascripts/emoji/utils.js new file mode 100644 index 0000000000000..eb3dcea73c017 --- /dev/null +++ b/app/assets/javascripts/emoji/utils.js @@ -0,0 +1,8 @@ +import emojiIntents from 'emojis/intents.json'; +import { NEUTRAL_INTENT_MULTIPLIER } from '~/emoji/constants'; + +export function getEmojiScoreWithIntent(emojiName, baseScore) { + const intentMultiplier = emojiIntents[emojiName] || NEUTRAL_INTENT_MULTIPLIER; + + return 2 ** baseScore * intentMultiplier; +} diff --git a/app/assets/javascripts/gfm_auto_complete.js b/app/assets/javascripts/gfm_auto_complete.js index 146255df31f59..d4dafbdc94fdd 100644 --- a/app/assets/javascripts/gfm_auto_complete.js +++ b/app/assets/javascripts/gfm_auto_complete.js @@ -897,7 +897,7 @@ GfmAutoComplete.Emoji = { return Emoji.searchEmoji(query); }, sorter(items) { - return Emoji.sortEmoji(items); + return items.sort(Emoji.sortEmoji); }, }; // Team Members diff --git a/doc/development/fe_guide/emojis.md b/doc/development/fe_guide/emojis.md index 7ef88c5ca1971..3c7fc20440b77 100644 --- a/doc/development/fe_guide/emojis.md +++ b/doc/development/fe_guide/emojis.md @@ -24,6 +24,10 @@ when your platform does not support it. 1. Ensure new sprite sheets generated for 1x and 2x - `app/assets/images/emoji.png` - `app/assets/images/emoji@2x.png` + 1. Update `fixtures/emojis/intents.json` with any new emoji that we would like to highlight as having positive or negative intent. + - Positive intent should be set to `0.5`. + - Neutral intent can be set to `1`. This is applied to all emoji automatically so there is no need to set this explicitly. + - Negative intent should be set to `1.5`. 1. Ensure you see new individual images copied into `app/assets/images/emoji/` 1. Ensure you can see the new emojis and their aliases in the GitLab Flavored Markdown (GLFM) Autocomplete 1. Ensure you can see the new emojis and their aliases in the award emoji menu diff --git a/fixtures/emojis/intents.json b/fixtures/emojis/intents.json new file mode 100644 index 0000000000000..8e8b02aaddb58 --- /dev/null +++ b/fixtures/emojis/intents.json @@ -0,0 +1,16 @@ +{ + "thumbsdown": 1.5, + "thumbsdown_tone1": 1.5, + "thumbsdown_tone2": 1.5, + "thumbsdown_tone3": 1.5, + "thumbsdown_tone4": 1.5, + "thumbsdown_tone5": 1.5, + "thumbsup": 0.5, + "thumbsup_tone1": 0.5, + "thumbsup_tone2": 0.5, + "thumbsup_tone3": 0.5, + "thumbsup_tone4": 0.5, + "thumbsup_tone5": 0.5, + "slight_frown": 1.5, + "slight_smile": 0.5 +} diff --git a/spec/frontend/__helpers__/emoji.js b/spec/frontend/__helpers__/emoji.js index 014a7854024bd..6c9291bdc8f64 100644 --- a/spec/frontend/__helpers__/emoji.js +++ b/spec/frontend/__helpers__/emoji.js @@ -58,6 +58,16 @@ export const validEmoji = { unicodeVersion: '6.0', description: 'because it contains multiple zero width joiners', }, + thumbsup: { + moji: 'ðŸ‘', + unicodeVersion: '6.0', + description: 'thumbs up sign', + }, + thumbsdown: { + moji: '👎', + description: 'thumbs down sign', + unicodeVersion: '6.0', + }, }; export const invalidEmoji = { diff --git a/spec/frontend/awards_handler_spec.js b/spec/frontend/awards_handler_spec.js index 5d65774561532..b14bc5122b926 100644 --- a/spec/frontend/awards_handler_spec.js +++ b/spec/frontend/awards_handler_spec.js @@ -57,6 +57,18 @@ describe('AwardsHandler', () => { d: 'white question mark ornament', u: '6.0', }, + thumbsup: { + c: 'people', + e: 'ðŸ‘', + d: 'thumbs up sign', + u: '6.0', + }, + thumbsdown: { + c: 'people', + e: '👎', + d: 'thumbs down sign', + u: '6.0', + }, }; const openAndWaitForEmojiMenu = (sel = '.js-add-award') => { @@ -296,6 +308,23 @@ describe('AwardsHandler', () => { awardsHandler.searchEmojis('👼'); expect($('[data-name=angel]').is(':visible')).toBe(true); }); + + it('should show positive intent emoji first', async () => { + await openAndWaitForEmojiMenu(); + + awardsHandler.searchEmojis('thumb'); + + const $menu = $('.emoji-menu'); + const $thumbsUpItem = $menu.find('[data-name=thumbsup]'); + const $thumbsDownItem = $menu.find('[data-name=thumbsdown]'); + + expect($thumbsUpItem.is(':visible')).toBe(true); + expect($thumbsDownItem.is(':visible')).toBe(true); + + expect($thumbsUpItem.parents('.emoji-menu-list-item').index()).toBeLessThan( + $thumbsDownItem.parents('.emoji-menu-list-item').index(), + ); + }); }); describe('emoji menu', () => { diff --git a/spec/frontend/emoji/index_spec.js b/spec/frontend/emoji/index_spec.js index cc03758649603..dc8f50e0e4bdb 100644 --- a/spec/frontend/emoji/index_spec.js +++ b/spec/frontend/emoji/index_spec.js @@ -24,6 +24,7 @@ import isEmojiUnicodeSupported, { isHorceRacingSkinToneComboEmoji, isPersonZwjEmoji, } from '~/emoji/support/is_emoji_unicode_supported'; +import { NEUTRAL_INTENT_MULTIPLIER } from '~/emoji/constants'; const emptySupportMap = { personZwj: false, @@ -436,14 +437,28 @@ describe('emoji', () => { it.each([undefined, null, ''])("should return all emoji when the input is '%s'", (input) => { const search = searchEmoji(input); - const expected = Object.keys(validEmoji).map((name) => { - return { - emoji: mockEmojiData[name], - field: 'd', - fieldValue: mockEmojiData[name].d, - score: 0, - }; - }); + const expected = Object.keys(validEmoji) + .map((name) => { + let score = NEUTRAL_INTENT_MULTIPLIER; + + // Positive intent value retrieved from ~/emoji/intents.json + if (name === 'thumbsup') { + score = 0.5; + } + + // Negative intent value retrieved from ~/emoji/intents.json + if (name === 'thumbsdown') { + score = 1.5; + } + + return { + emoji: mockEmojiData[name], + field: 'd', + fieldValue: mockEmojiData[name].d, + score, + }; + }) + .sort(sortEmoji); expect(search).toEqual(expected); }); @@ -457,7 +472,7 @@ describe('emoji', () => { name: 'atom', field: 'e', fieldValue: 'atom', - score: 0, + score: NEUTRAL_INTENT_MULTIPLIER, }, ], ], @@ -469,7 +484,7 @@ describe('emoji', () => { name: 'atom', field: 'alias', fieldValue: 'atom_symbol', - score: 4, + score: 16, }, ], ], @@ -481,7 +496,7 @@ describe('emoji', () => { name: 'atom', field: 'alias', fieldValue: 'atom_symbol', - score: 0, + score: NEUTRAL_INTENT_MULTIPLIER, }, ], ], @@ -509,7 +524,7 @@ describe('emoji', () => { { name: 'atom', field: 'd', - score: 0, + score: NEUTRAL_INTENT_MULTIPLIER, }, ], ], @@ -521,7 +536,7 @@ describe('emoji', () => { { name: 'atom', field: 'd', - score: 0, + score: NEUTRAL_INTENT_MULTIPLIER, }, ], ], @@ -533,7 +548,7 @@ describe('emoji', () => { { name: 'grey_question', field: 'name', - score: 5, + score: 32, }, ], ], @@ -544,7 +559,7 @@ describe('emoji', () => { { name: 'grey_question', field: 'd', - score: 24, + score: 16777216, }, ], ], @@ -552,15 +567,15 @@ describe('emoji', () => { 'searching with query "heart"', 'heart', [ - { - name: 'black_heart', - field: 'd', - score: 6, - }, { name: 'heart', field: 'name', - score: 0, + score: NEUTRAL_INTENT_MULTIPLIER, + }, + { + name: 'black_heart', + field: 'd', + score: 64, }, ], ], @@ -568,15 +583,15 @@ describe('emoji', () => { 'searching with query "HEART"', 'HEART', [ - { - name: 'black_heart', - field: 'd', - score: 6, - }, { name: 'heart', field: 'name', - score: 0, + score: NEUTRAL_INTENT_MULTIPLIER, + }, + { + name: 'black_heart', + field: 'd', + score: 64, }, ], ], @@ -584,15 +599,31 @@ describe('emoji', () => { 'searching with query "star"', 'star', [ + { + name: 'star', + field: 'name', + score: NEUTRAL_INTENT_MULTIPLIER, + }, { name: 'custard', field: 'd', - score: 2, + score: 4, + }, + ], + ], + [ + 'searching for emoji with intentions assigned', + 'thumbs', + [ + { + name: 'thumbsup', + field: 'd', + score: 0.5, }, { - name: 'star', - field: 'name', - score: 0, + name: 'thumbsdown', + field: 'd', + score: 1.5, }, ], ], @@ -619,10 +650,10 @@ describe('emoji', () => { [ { score: 10, fieldValue: '', emoji: { name: 'a' } }, { score: 5, fieldValue: '', emoji: { name: 'b' } }, - { score: 0, fieldValue: '', emoji: { name: 'c' } }, + { score: 1, fieldValue: '', emoji: { name: 'c' } }, ], [ - { score: 0, fieldValue: '', emoji: { name: 'c' } }, + { score: 1, fieldValue: '', emoji: { name: 'c' } }, { score: 5, fieldValue: '', emoji: { name: 'b' } }, { score: 10, fieldValue: '', emoji: { name: 'a' } }, ], @@ -630,25 +661,25 @@ describe('emoji', () => { [ 'should correctly sort by fieldValue', [ - { score: 0, fieldValue: 'y', emoji: { name: 'b' } }, - { score: 0, fieldValue: 'x', emoji: { name: 'a' } }, - { score: 0, fieldValue: 'z', emoji: { name: 'c' } }, + { score: 1, fieldValue: 'y', emoji: { name: 'b' } }, + { score: 1, fieldValue: 'x', emoji: { name: 'a' } }, + { score: 1, fieldValue: 'z', emoji: { name: 'c' } }, ], [ - { score: 0, fieldValue: 'x', emoji: { name: 'a' } }, - { score: 0, fieldValue: 'y', emoji: { name: 'b' } }, - { score: 0, fieldValue: 'z', emoji: { name: 'c' } }, + { score: 1, fieldValue: 'x', emoji: { name: 'a' } }, + { score: 1, fieldValue: 'y', emoji: { name: 'b' } }, + { score: 1, fieldValue: 'z', emoji: { name: 'c' } }, ], ], [ 'should correctly sort by score and then by fieldValue (in order)', [ { score: 5, fieldValue: 'y', emoji: { name: 'c' } }, - { score: 0, fieldValue: 'z', emoji: { name: 'a' } }, + { score: 1, fieldValue: 'z', emoji: { name: 'a' } }, { score: 5, fieldValue: 'x', emoji: { name: 'b' } }, ], [ - { score: 0, fieldValue: 'z', emoji: { name: 'a' } }, + { score: 1, fieldValue: 'z', emoji: { name: 'a' } }, { score: 5, fieldValue: 'x', emoji: { name: 'b' } }, { score: 5, fieldValue: 'y', emoji: { name: 'c' } }, ], @@ -656,7 +687,7 @@ describe('emoji', () => { ]; it.each(testCases)('%s', (_, scoredItems, expected) => { - expect(sortEmoji(scoredItems)).toEqual(expected); + expect(scoredItems.sort(sortEmoji)).toEqual(expected); }); }); }); diff --git a/spec/frontend/emoji/utils_spec.js b/spec/frontend/emoji/utils_spec.js new file mode 100644 index 0000000000000..397388ca0aecc --- /dev/null +++ b/spec/frontend/emoji/utils_spec.js @@ -0,0 +1,15 @@ +import { getEmojiScoreWithIntent } from '~/emoji/utils'; + +describe('Utils', () => { + describe('getEmojiScoreWithIntent', () => { + it.each` + emojiName | baseScore | finalScore + ${'thumbsup'} | ${1} | ${1} + ${'thumbsdown'} | ${1} | ${3} + ${'neutralemoji'} | ${1} | ${2} + ${'zerobaseemoji'} | ${0} | ${1} + `('returns the correct score for $emojiName', ({ emojiName, baseScore, finalScore }) => { + expect(getEmojiScoreWithIntent(emojiName, baseScore)).toBe(finalScore); + }); + }); +}); -- GitLab