From 0546a11fd99b18fb2be8cfdd308260ef15deefb2 Mon Sep 17 00:00:00 2001 From: grimhilt <107760093+grimhilt@users.noreply.github.com> Date: Mon, 27 Feb 2023 18:09:15 +0100 Subject: [PATCH] Sort short/exact emoji matches before longer incomplete matches (#10212) * apply sort for exact match * add tests for emoji provider * apply filter in the emoji picker * add tests * revert cypress version * put correct copyright * fix eslint * fix eslint * add type * fix cypress test * fix tsc types issues * add forgotten space... --------- Co-authored-by: grimhilt Co-authored-by: David Baker --- src/autocomplete/EmojiProvider.tsx | 26 +++++++++++--- .../views/emojipicker/EmojiPicker.tsx | 22 +++++++++++- test/autocomplete/EmojiProvider-test.ts | 32 +++++++++++------ .../views/emojipicker/EmojiPicker-test.tsx | 34 +++++++++++++++++++ 4 files changed, 97 insertions(+), 17 deletions(-) create mode 100644 test/components/views/emojipicker/EmojiPicker-test.tsx diff --git a/src/autocomplete/EmojiProvider.tsx b/src/autocomplete/EmojiProvider.tsx index b434eb6e573..bad4477768e 100644 --- a/src/autocomplete/EmojiProvider.tsx +++ b/src/autocomplete/EmojiProvider.tsx @@ -19,7 +19,7 @@ limitations under the License. */ import React from "react"; -import { uniq, sortBy, ListIteratee } from "lodash"; +import { uniq, sortBy, uniqBy, ListIteratee } from "lodash"; import EMOTICON_REGEX from "emojibase-regex/emoticon"; import { Room } from "matrix-js-sdk/src/models/room"; @@ -118,7 +118,7 @@ export default class EmojiProvider extends AutocompleteProvider { // Do second match with shouldMatchWordsOnly in order to match against 'name' completions = completions.concat(this.nameMatcher.match(matchedString)); - let sorters: ListIteratee[] = []; + const sorters: ListIteratee[] = []; // make sure that emoticons come first sorters.push((c) => score(matchedString, c.emoji.emoticon || "")); @@ -140,11 +140,27 @@ export default class EmojiProvider extends AutocompleteProvider { completions = completions.slice(0, LIMIT); // Do a second sort to place emoji matching with frequently used one on top - sorters = []; + const recentlyUsedAutocomplete: ISortedEmoji[] = []; this.recentlyUsed.forEach((emoji) => { - sorters.push((c) => score(emoji.shortcodes[0], c.emoji.shortcodes[0])); + if (emoji.shortcodes[0].indexOf(trimmedMatch) === 0) { + recentlyUsedAutocomplete.push({ emoji: emoji, _orderBy: 0 }); + } }); - completions = sortBy(uniq(completions), sorters); + + //if there is an exact shortcode match in the frequently used emojis, it goes before everything + for (let i = 0; i < recentlyUsedAutocomplete.length; i++) { + if (recentlyUsedAutocomplete[i].emoji.shortcodes[0] === trimmedMatch) { + const exactMatchEmoji = recentlyUsedAutocomplete[i]; + for (let j = i; j > 0; j--) { + recentlyUsedAutocomplete[j] = recentlyUsedAutocomplete[j - 1]; + } + recentlyUsedAutocomplete[0] = exactMatchEmoji; + break; + } + } + + completions = recentlyUsedAutocomplete.concat(completions); + completions = uniqBy(completions, "emoji"); return completions.map((c) => ({ completion: c.emoji.unicode, diff --git a/src/components/views/emojipicker/EmojiPicker.tsx b/src/components/views/emojipicker/EmojiPicker.tsx index 1d8c2336966..50bae75b922 100644 --- a/src/components/views/emojipicker/EmojiPicker.tsx +++ b/src/components/views/emojipicker/EmojiPicker.tsx @@ -194,10 +194,30 @@ class EmojiPicker extends React.Component { emojis = cat.id === "recent" ? this.recentlyUsed : DATA_BY_CATEGORY[cat.id]; } emojis = emojis.filter((emoji) => this.emojiMatchesFilter(emoji, lcFilter)); + emojis = emojis.sort((a, b) => { + const indexA = a.shortcodes[0].indexOf(lcFilter); + const indexB = b.shortcodes[0].indexOf(lcFilter); + + // Prioritize emojis containing the filter in its shortcode + if (indexA == -1 || indexB == -1) { + return indexB - indexA; + } + + // If both emojis start with the filter + // put the shorter emoji first + if (indexA == 0 && indexB == 0) { + return a.shortcodes[0].length - b.shortcodes[0].length; + } + + // Prioritize emojis starting with the filter + return indexA - indexB; + }); this.memoizedDataByCategory[cat.id] = emojis; cat.enabled = emojis.length > 0; // The setState below doesn't re-render the header and we already have the refs for updateVisibility, so... - cat.ref.current.disabled = !cat.enabled; + if (cat.ref.current) { + cat.ref.current.disabled = !cat.enabled; + } } this.setState({ filter }); // Header underlines need to be updated, but updating requires knowing diff --git a/test/autocomplete/EmojiProvider-test.ts b/test/autocomplete/EmojiProvider-test.ts index ac5e40d041b..c3f3c7fc353 100644 --- a/test/autocomplete/EmojiProvider-test.ts +++ b/test/autocomplete/EmojiProvider-test.ts @@ -69,20 +69,30 @@ describe("EmojiProvider", function () { }, ); - it("Returns correct autocompletion based on recently used emoji", async function () { + it("Recently used emojis are correctly sorted", async function () { add("😘"); //kissing_heart - add("😘"); - add("😚"); //kissing_closed_eyes - const emojiProvider = new EmojiProvider(null!); + add("💗"); //heartpulse + add("💗"); //heartpulse + add("😍"); //heart_eyes - let completionsList = await emojiProvider.getCompletions(":kis", { beginning: true, end: 3, start: 3 }); - expect(completionsList[0].component!.props.title).toEqual(":kissing_heart:"); - expect(completionsList[1].component!.props.title).toEqual(":kissing_closed_eyes:"); + const ep = new EmojiProvider(testRoom); + const completionsList = await ep.getCompletions(":heart", { beginning: true, start: 0, end: 6 }); + expect(completionsList[0]?.component?.props.title).toEqual(":heartpulse:"); + expect(completionsList[1]?.component?.props.title).toEqual(":heart_eyes:"); + }); - completionsList = await emojiProvider.getCompletions(":kissing_c", { beginning: true, end: 3, start: 3 }); - expect(completionsList[0].component!.props.title).toEqual(":kissing_closed_eyes:"); + it("Exact match in recently used takes the lead", async function () { + add("😘"); //kissing_heart + add("💗"); //heartpulse + add("💗"); //heartpulse + add("😍"); //heart_eyes + + add("❤️"); //heart + const ep = new EmojiProvider(testRoom); + const completionsList = await ep.getCompletions(":heart", { beginning: true, start: 0, end: 6 }); - completionsList = await emojiProvider.getCompletions(":so", { beginning: true, end: 2, start: 2 }); - expect(completionsList[0].component!.props.title).toEqual(":sob:"); + expect(completionsList[0]?.component?.props.title).toEqual(":heart:"); + expect(completionsList[1]?.component?.props.title).toEqual(":heartpulse:"); + expect(completionsList[2]?.component?.props.title).toEqual(":heart_eyes:"); }); }); diff --git a/test/components/views/emojipicker/EmojiPicker-test.tsx b/test/components/views/emojipicker/EmojiPicker-test.tsx new file mode 100644 index 00000000000..efd09825a9c --- /dev/null +++ b/test/components/views/emojipicker/EmojiPicker-test.tsx @@ -0,0 +1,34 @@ +/* +Copyright 2023 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import EmojiPicker from "../../../../src/components/views/emojipicker/EmojiPicker"; +import { stubClient } from "../../../test-utils"; + +describe("EmojiPicker", function () { + stubClient(); + + it("sort emojis by shortcode and size", function () { + const ep = new EmojiPicker({ onChoose: (str: String) => false }); + + //@ts-ignore private access + ep.onChangeFilter("heart"); + + //@ts-ignore private access + expect(ep.memoizedDataByCategory["people"][0].shortcodes[0]).toEqual("heart"); + //@ts-ignore private access + expect(ep.memoizedDataByCategory["people"][1].shortcodes[0]).toEqual("heartbeat"); + }); +});