From 08a2d81d6b531ac0f06051d247965d43a5f26c8e Mon Sep 17 00:00:00 2001 From: commonlawfeature <40127608+commonlawfeature@users.noreply.github.com> Date: Thu, 14 Apr 2022 13:31:50 -0700 Subject: [PATCH] Resolve emoji autocomplete not being temporally consistent (#8086) * Adds a test to demonstrate the issue with emoji autocomplete reported in https://github.com/vector-im/element-web/issues/19302. Signed-off-by: Ryan Browne * Trim trailing `:` when checking for autocompletes for emoji. Closes https://github.com/vector-im/element-web/issues/19302 Signed-off-by: Ryan Browne * Move all references to the emoji delimiter character to reference a constant. Signed-off-by: Ryan Browne * Revert "Move all references to the emoji delimiter character to reference a constant." This reverts commit ac09e71e4c6151e35d21f612c9b329ead2a381f1. Signed-off-by: Ryan Browne * Rename variable. Signed-off-by: Ryan Browne * Make the test file a .js file. Signed-off-by: Ryan Browne * Update quotes to match style and make a valid stubbed room. Signed-off-by: Ryan Browne * Fix variable name and test reporting. Signed-off-by: Ryan Browne * Use str.replace with a regex. Signed-off-by: Ryan Browne * Use an improved regex that does not have have to iterate through the entire string, and can just backtrack at most the last 2 characters. Signed-off-by: Ryan Browne * Revert "Use an improved regex that does not have have to iterate through the entire string, and can just backtrack at most the last 2 characters." This regex is very efficient, but requires a specific form of the emoji shortcode that it is not clear is within our control. This is a restriction that is not required by the technicalities of solving the bug this PR is attempting to fix. (It requires that an emoji shortcode end with a colon.) This reverts commit 220cb0efb8de247158c11daf9170464a57cc3af2. Signed-off-by: Ryan Browne Co-authored-by: Ryan Browne --- src/autocomplete/EmojiProvider.tsx | 11 +++- test/autocomplete/EmojiProvider-test.ts | 67 +++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-) create mode 100644 test/autocomplete/EmojiProvider-test.ts diff --git a/src/autocomplete/EmojiProvider.tsx b/src/autocomplete/EmojiProvider.tsx index 55d58a8953..4c051a7126 100644 --- a/src/autocomplete/EmojiProvider.tsx +++ b/src/autocomplete/EmojiProvider.tsx @@ -3,6 +3,7 @@ Copyright 2016 Aviral Dasgupta Copyright 2017 Vector Creations Ltd Copyright 2017, 2018 New Vector Ltd Copyright 2019 The Matrix.org Foundation C.I.C. +Copyright 2022 Ryan Browne Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -62,6 +63,13 @@ function score(query, space) { } } +function colonsTrimmed(str: string): string { + // Trim off leading and potentially trailing `:` to correctly match the emoji data as they exist in emojibase. + // Notes: The regex is pinned to the start and end of the string so that we can use the lazy-capturing `*?` matcher. + // It needs to be lazy so that the trailing `:` is not captured in the replacement group, if it exists. + return str.replace(/^:(.*?):?$/, "$1"); +} + export default class EmojiProvider extends AutocompleteProvider { matcher: QueryMatcher; nameMatcher: QueryMatcher; @@ -108,8 +116,9 @@ export default class EmojiProvider extends AutocompleteProvider { // then sort by score (Infinity if matchedString not in shortcode) sorters.push(c => score(matchedString, c.emoji.shortcodes[0])); // then sort by max score of all shortcodes, trim off the `:` + const trimmedMatch = colonsTrimmed(matchedString); sorters.push(c => Math.min( - ...c.emoji.shortcodes.map(s => score(matchedString.substring(1), s)), + ...c.emoji.shortcodes.map(s => score(trimmedMatch, s)), )); // If the matchedString is not empty, sort by length of shortcode. Example: // matchedString = ":bookmark" diff --git a/test/autocomplete/EmojiProvider-test.ts b/test/autocomplete/EmojiProvider-test.ts new file mode 100644 index 0000000000..a64312fa29 --- /dev/null +++ b/test/autocomplete/EmojiProvider-test.ts @@ -0,0 +1,67 @@ +/* +Copyright 2022 Ryan Browne + +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 EmojiProvider from '../../src/autocomplete/EmojiProvider'; +import { mkStubRoom } from '../test-utils/test-utils'; + +const EMOJI_SHORTCODES = [ + ":+1", + ":heart", + ":grinning", + ":hand", + ":man", + ":sweat", + ":monkey", + ":boat", + ":mailbox", + ":cop", + ":bow", + ":kiss", + ":golf", +]; + +// Some emoji shortcodes are too short and do not actually trigger autocompletion until the ending `:`. +// This means that we cannot compare their autocompletion before and after the ending `:` and have +// to simply assert that the final completion with the colon is the exact emoji. +const TOO_SHORT_EMOJI_SHORTCODE = [ + { emojiShortcode: ":o", expectedEmoji: "⭕️" }, +]; + +describe('EmojiProvider', function() { + const testRoom = mkStubRoom(undefined, undefined, undefined); + + it.each(EMOJI_SHORTCODES)('Returns consistent results after final colon %s', async function(emojiShortcode) { + const ep = new EmojiProvider(testRoom); + const range = { "beginning": true, "start": 0, "end": 3 }; + const completionsBeforeColon = await ep.getCompletions(emojiShortcode, range); + const completionsAfterColon = await ep.getCompletions(emojiShortcode + ':', range); + + const firstCompletionWithoutColon = completionsBeforeColon[0].completion; + const firstCompletionWithColon = completionsAfterColon[0].completion; + + expect(firstCompletionWithoutColon).toEqual(firstCompletionWithColon); + }); + + it.each( + TOO_SHORT_EMOJI_SHORTCODE, + )('Returns correct results after final colon $emojiShortcode', async ({ emojiShortcode, expectedEmoji }) => { + const ep = new EmojiProvider(testRoom); + const range = { "beginning": true, "start": 0, "end": 3 }; + const completions = await ep.getCompletions(emojiShortcode + ':', range); + + expect(completions[0].completion).toEqual(expectedEmoji); + }); +});