Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Resolve emoji autocomplete not being temporally consistent #8086

Merged
merged 13 commits into from
Apr 14, 2022
Merged
28 changes: 23 additions & 5 deletions src/autocomplete/EmojiProvider.tsx
Original file line number Diff line number Diff line change
@@ -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 <[email protected]>

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
@@ -33,9 +34,13 @@ import { TimelineRenderingType } from '../contexts/RoomContext';

const LIMIT = 20;

// The delimiter used to start and end emoji shortcodes.
const EMOJI_DELIMITER = ':';

// Match for ascii-style ";-)" emoticons or ":wink:" shortcodes provided by emojibase
// anchored to only match from the start of parts otherwise it'll show emoji suggestions whilst typing matrix IDs
const EMOJI_REGEX = new RegExp('(' + EMOTICON_REGEX.source + '|(?:^|\\s):[+-\\w]*:?)$', 'g');
const EMOJI_SHORTNAME_REGEX = `(?${EMOJI_DELIMITER}^|\\s)${EMOJI_DELIMITER}[+-\\w]*${EMOJI_DELIMITER}?`;
const EMOJI_REGEX = new RegExp('(' + EMOTICON_REGEX.source + '|' + EMOJI_SHORTNAME_REGEX + ')$', 'g');

interface ISortedEmoji {
emoji: IEmoji;
@@ -62,6 +67,19 @@ function score(query, space) {
}
}

function delimiterTrimmed(string: string): string {
// Trim off leading and potentially trailing `:` to correctly
// match the emoji data as they exist in emojibase.
let returned = string;
if (string[0] === EMOJI_DELIMITER) {
returned = returned.substring(1);
}
if (returned[returned.length - 1] === EMOJI_DELIMITER) {
returned = returned.slice(0, -1);
}
return returned;
}

export default class EmojiProvider extends AutocompleteProvider {
matcher: QueryMatcher<ISortedEmoji>;
nameMatcher: QueryMatcher<ISortedEmoji>;
@@ -70,7 +88,7 @@ export default class EmojiProvider extends AutocompleteProvider {
super({ commandRegex: EMOJI_REGEX, renderingType });
this.matcher = new QueryMatcher<ISortedEmoji>(SORTED_EMOJI, {
keys: [],
funcs: [o => o.emoji.shortcodes.map(s => `:${s}:`)],
funcs: [o => o.emoji.shortcodes.map(s => `${EMOJI_DELIMITER}${s}${EMOJI_DELIMITER}`)],
Copy link
Contributor

@MadLittleMods MadLittleMods Mar 31, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fix does work for the most part to at least not switch the emoji from underneath you 👍 and still shows you the exact match.

But does still have some inconsistency with/without the final : with the results within the list itself. Not the end of the world. Just noting a small caveat.

emoji without with final :
:smile: Screen Shot 2022-03-31 at 3 47 46 PM Screen Shot 2022-03-31 at 3 47 55 PM
:hand: Screen Shot 2022-03-31 at 3 46 53 PM Screen Shot 2022-03-31 at 3 47 03 PM
:grinning: . .

Copy link
Contributor Author

@commonlawfeature commonlawfeature Mar 31, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, agreed. I did not have time to look into the other inconsistencies, as they appear to be related to either the search or the scoring. This was just a quick win to get some testing around this area, and fix a particularly annoying bug. I am happy to keep looking into the other inconsistencies as well, but I don't think that those fixes, will be in the same area.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@commonlawfeature Fixing the main problem is a good iteration 👍 Leave the other stuff for another PR/iteration

// For matching against ascii equivalents
shouldMatchWordsOnly: false,
});
@@ -107,9 +125,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 `:`
// then sort by max score of all shortcodes, trim off the `EMOJI_DELIMITER`
sorters.push(c => Math.min(
...c.emoji.shortcodes.map(s => score(matchedString.substring(1), s)),
...c.emoji.shortcodes.map(s => score(delimiterTrimmed(matchedString), s)),
));
// If the matchedString is not empty, sort by length of shortcode. Example:
// matchedString = ":bookmark"
@@ -124,7 +142,7 @@ export default class EmojiProvider extends AutocompleteProvider {
completions = completions.map(c => ({
completion: c.emoji.unicode,
component: (
<PillCompletion title={`:${c.emoji.shortcodes[0]}:`} aria-label={c.emoji.unicode}>
<PillCompletion title={`${EMOJI_DELIMITER}${c.emoji.shortcodes[0]}${EMOJI_DELIMITER}`} aria-label={c.emoji.unicode}>
<span>{ c.emoji.unicode }</span>
</PillCompletion>
),
64 changes: 64 additions & 0 deletions test/autocomplete/EmojiProvider-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
Copyright 2022 Ryan Browne <[email protected]>

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';

const EMOJI_SHORTNAMES = [
':+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_SHORTNAME = [
{ emojiShortcode: ':o', expectedEmoji: '⭕️' },
];

describe('EmojiProvider', function() {
it.each(EMOJI_SHORTNAMES)('Returns consistent results after final colon %s', async function(emojiShortcode) {
const ep = new EmojiProvider('test-room');
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_SHORTNAME,
)("Returns correct results after final colon %s", async ({ emojiShortcode, expectedEmoji }) => {
const ep = new EmojiProvider('test-room');
const range = { "beginning": true, "start": 0, "end": 3 };
const completions = await ep.getCompletions(emojiShortcode + ':', range);

expect(completions[0].completion).toEqual(expectedEmoji);
});
});