From 1568e08055c508b35454d20f13fd92d5873e3e6e Mon Sep 17 00:00:00 2001 From: Elias Nahum Date: Tue, 26 May 2020 16:03:11 -0400 Subject: [PATCH] MM-25470 Fix emoji autocomplete results (#4344) * Fix emoji autocomplete results * Rename selectors with "select" prefix --- .../emoji_suggestion.test.js.snap | 3057 +++++ .../emoji_suggestion/emoji_suggestion.js | 81 +- .../emoji_suggestion/emoji_suggestion.test.js | 86 + .../autocomplete/emoji_suggestion/index.js | 36 +- .../__snapshots__/emoji_picker.test.js.snap | 10091 +++++++++++++++- .../emoji_picker/emoji_picker.test.js | 42 +- .../emoji_picker/emoji_picker_base.js | 21 +- app/components/emoji_picker/index.js | 140 +- app/selectors/emojis.js | 131 + 9 files changed, 13472 insertions(+), 213 deletions(-) create mode 100644 app/components/autocomplete/emoji_suggestion/__snapshots__/emoji_suggestion.test.js.snap create mode 100644 app/components/autocomplete/emoji_suggestion/emoji_suggestion.test.js create mode 100644 app/selectors/emojis.js diff --git a/app/components/autocomplete/emoji_suggestion/__snapshots__/emoji_suggestion.test.js.snap b/app/components/autocomplete/emoji_suggestion/__snapshots__/emoji_suggestion.test.js.snap new file mode 100644 index 000000000..9d6be4bd6 --- /dev/null +++ b/app/components/autocomplete/emoji_suggestion/__snapshots__/emoji_suggestion.test.js.snap @@ -0,0 +1,3057 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`components/autocomplete/emoji_suggestion should match snapshot 1`] = `null`; + +exports[`components/autocomplete/emoji_suggestion should match snapshot 2`] = ` + +`; diff --git a/app/components/autocomplete/emoji_suggestion/emoji_suggestion.js b/app/components/autocomplete/emoji_suggestion/emoji_suggestion.js index 6c778d2f4..a0149be97 100644 --- a/app/components/autocomplete/emoji_suggestion/emoji_suggestion.js +++ b/app/components/autocomplete/emoji_suggestion/emoji_suggestion.js @@ -9,7 +9,6 @@ import { Text, View, } from 'react-native'; -import Fuse from 'fuse.js'; import AutocompleteDivider from '@components/autocomplete/autocomplete_divider'; import Emoji from '@components/emoji'; @@ -21,15 +20,6 @@ import {makeStyleSheetFromTheme} from '@utils/theme'; const EMOJI_REGEX = /(^|\s|^\+|^-)(:([^:\s]*))$/i; const EMOJI_REGEX_WITHOUT_PREFIX = /\B(:([^:\s]*))$/i; -const options = { - shouldSort: true, - threshold: 0.3, - location: 0, - distance: 100, - minMatchCharLength: 2, - maxPatternLength: 32, -}; - export default class EmojiSuggestion extends PureComponent { static propTypes = { actions: PropTypes.shape({ @@ -39,6 +29,7 @@ export default class EmojiSuggestion extends PureComponent { cursorPosition: PropTypes.number, customEmojisEnabled: PropTypes.bool, emojis: PropTypes.array.isRequired, + fuse: PropTypes.object.isRequired, isSearch: PropTypes.bool, maxListHeight: PropTypes.number, theme: PropTypes.object.isRequired, @@ -63,23 +54,16 @@ export default class EmojiSuggestion extends PureComponent { super(props); this.matchTerm = ''; - const list = props.emojis || []; - this.fuse = new Fuse(list, options); } - componentDidUpdate(prevProps) { + componentDidUpdate() { if (this.props.isSearch) { return; } - const {cursorPosition, emojis, value} = this.props; + const {cursorPosition, value} = this.props; const match = value.substring(0, cursorPosition).match(EMOJI_REGEX); - if (prevProps.emojis !== emojis) { - const list = emojis || []; - this.fuse = new Fuse(list, options); - } - if (!match || this.state.emojiComplete) { this.resetAutocomplete(); return; @@ -91,7 +75,7 @@ export default class EmojiSuggestion extends PureComponent { if (this.props.customEmojisEnabled) { this.props.actions.autocompleteCustomEmojis(this.matchTerm); } - this.searchEmoji(this.matchTerm); + this.searchEmojis(this.matchTerm); } } @@ -145,17 +129,6 @@ export default class EmojiSuggestion extends PureComponent { getItemLayout = ({index}) => ({length: 40, offset: 40 * index, index}) - handleFuzzySearch = (matchTerm) => { - const {emojis} = this.props; - - clearTimeout(this.searchTermTimeout); - this.searchTermTimeout = setTimeout(() => { - const results = this.fuse.search(matchTerm.toLowerCase()).map((r) => r.refIndex); - const data = results.map((index) => emojis[index]); - this.setEmojiData(data, matchTerm); - }, 100); - }; - keyExtractor = (item) => item; renderItem = ({item}) => { @@ -188,26 +161,38 @@ export default class EmojiSuggestion extends PureComponent { this.props.onResultCountChange(0); } - searchEmoji = (matchTerm) => { - if (matchTerm.length) { - this.handleFuzzySearch(matchTerm); - } else { - this.setEmojiData(this.props.emojis); - } - } + searchEmojis = (searchTerm) => { + const {emojis, fuse} = this.props; - setEmojiData = (data, matchTerm = null) => { let sorter = compareEmojis; - if (matchTerm) { - sorter = (a, b) => compareEmojis(a, b, matchTerm); + if (searchTerm.trim().length) { + const searchTermLowerCase = searchTerm.toLowerCase(); + + sorter = (a, b) => compareEmojis(a, b, searchTermLowerCase); + clearTimeout(this.searchTermTimeout); + + this.searchTermTimeout = setTimeout(() => { + const fuzz = fuse.search(searchTerm); + const results = fuzz.reduce((values, r) => { + const v = r.matches[0]?.value; + if (v) { + values.push(v); + } + + return values; + }, []); + const data = results.sort(sorter); + this.setState({ + active: data.length > 0, + dataSource: data, + }); + }, 100); + } else { + this.setState({ + active: emojis.length > 0, + dataSource: emojis.sort(sorter), + }); } - - this.setState({ - active: data.length > 0, - dataSource: data.sort(sorter), - }); - - this.props.onResultCountChange(data.length); }; render() { diff --git a/app/components/autocomplete/emoji_suggestion/emoji_suggestion.test.js b/app/components/autocomplete/emoji_suggestion/emoji_suggestion.test.js new file mode 100644 index 000000000..d80439ce1 --- /dev/null +++ b/app/components/autocomplete/emoji_suggestion/emoji_suggestion.test.js @@ -0,0 +1,86 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import React from 'react'; +import Fuse from 'fuse.js'; + +import Preferences from '@mm-redux/constants/preferences'; +import {selectEmojisByName} from '@selectors/emojis'; +import initialState from '@store/initial_state'; +import {shallowWithIntl} from 'test/intl-test-helper'; + +import EmojiSuggestion from './emoji_suggestion'; + +jest.useFakeTimers(); + +describe('components/autocomplete/emoji_suggestion', () => { + const state = { + ...initialState, + views: { + recentEmojis: [], + }, + }; + const emojis = selectEmojisByName(state); + const options = { + shouldSort: false, + threshold: 0.3, + location: 0, + distance: 10, + includeMatches: true, + findAllMatches: true, + }; + const fuse = new Fuse(emojis, options); + const baseProps = { + actions: { + addReactionToLatestPost: jest.fn(), + autocompleteCustomEmojis: jest.fn(), + }, + cursorPosition: 0, + customEmojisEnabled: false, + emojis, + fuse, + isSearch: false, + theme: Preferences.THEMES.default, + onChangeText: jest.fn(), + onResultCountChange: jest.fn(), + rootId: '', + value: '', + nestedScrollEnabled: false, + }; + + test('should match snapshot', () => { + const wrapper = shallowWithIntl(); + expect(wrapper.getElement()).toMatchSnapshot(); + + wrapper.setProps({cursorPosition: 1, value: ':1'}); + expect(wrapper.getElement()).toMatchSnapshot(); + }); + + test('searchEmojis should return the right values on fuse', () => { + const output1 = ['100', '1234', '1st_place_medal', '+1', '-1', 'u7121']; + const output2 = ['+1']; + const output3 = ['-1']; + + const wrapper = shallowWithIntl(); + wrapper.instance().searchEmojis(''); + expect(wrapper.state('dataSource')).toEqual(baseProps.emojis); + + wrapper.instance().searchEmojis('1'); + jest.runAllTimers(); + setTimeout(() => { + expect(wrapper.state('dataSource')).toEqual(output1); + }, 100); + + wrapper.instance().searchEmojis('+'); + jest.runAllTimers(); + setTimeout(() => { + expect(wrapper.state('dataSource')).toEqual(output2); + }, 100); + + wrapper.instance().searchEmojis('-'); + jest.runAllTimers(); + setTimeout(() => { + expect(wrapper.state('dataSource')).toEqual(output3); + }, 100); + }); +}); \ No newline at end of file diff --git a/app/components/autocomplete/emoji_suggestion/index.js b/app/components/autocomplete/emoji_suggestion/index.js index 1081cde92..f490b3d3b 100644 --- a/app/components/autocomplete/emoji_suggestion/index.js +++ b/app/components/autocomplete/emoji_suggestion/index.js @@ -3,34 +3,32 @@ import {connect} from 'react-redux'; import {bindActionCreators} from 'redux'; +import Fuse from 'fuse.js'; -import {getCustomEmojisByName} from '@mm-redux/selectors/entities/emojis'; -import {getConfig} from '@mm-redux/selectors/entities/general'; +import {addReactionToLatestPost} from '@actions/views/emoji'; import {autocompleteCustomEmojis} from '@mm-redux/actions/emojis'; -import {createIdsSelector} from '@mm-redux/utils/helpers'; - -import {addReactionToLatestPost} from 'app/actions/views/emoji'; +import {getConfig} from '@mm-redux/selectors/entities/general'; import {getTheme} from '@mm-redux/selectors/entities/preferences'; -import {EmojiIndicesByAlias} from 'app/utils/emojis'; +import {selectEmojisByName} from '@selectors/emojis'; import EmojiSuggestion from './emoji_suggestion'; -const getEmojisByName = createIdsSelector( - getCustomEmojisByName, - (customEmojis) => { - const emoticons = new Set(); - for (const [key] of [...EmojiIndicesByAlias.entries(), ...customEmojis.entries()]) { - emoticons.add(key); - } - - return Array.from(emoticons); - }, -); - function mapStateToProps(state) { - const emojis = getEmojisByName(state); + const emojis = selectEmojisByName(state); + const options = { + shouldSort: false, + threshold: 0.3, + location: 0, + distance: 10, + includeMatches: true, + findAllMatches: true, + }; + + const list = emojis.length ? emojis : []; + const fuse = new Fuse(list, options); return { + fuse, emojis, customEmojisEnabled: getConfig(state).EnableCustomEmoji === 'true', theme: getTheme(state), diff --git a/app/components/emoji_picker/__snapshots__/emoji_picker.test.js.snap b/app/components/emoji_picker/__snapshots__/emoji_picker.test.js.snap index 93c1f8167..52e7ff0e5 100644 --- a/app/components/emoji_picker/__snapshots__/emoji_picker.test.js.snap +++ b/app/components/emoji_picker/__snapshots__/emoji_picker.test.js.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`components/emoji_picker/EmojiPicker should match snapshot 1`] = ` +exports[`components/emoji_picker/emoji_picker.ios should match snapshot 1`] = ` + > + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/app/components/emoji_picker/emoji_picker.test.js b/app/components/emoji_picker/emoji_picker.test.js index 2cf800492..c5a6bc296 100644 --- a/app/components/emoji_picker/emoji_picker.test.js +++ b/app/components/emoji_picker/emoji_picker.test.js @@ -2,14 +2,35 @@ // See LICENSE.txt for license information. import React from 'react'; +import Fuse from 'fuse.js'; import Preferences from '@mm-redux/constants/preferences'; - +import {selectEmojisByName, selectEmojisBySection} from '@selectors/emojis'; +import initialState from '@store/initial_state'; import {shallowWithIntl} from 'test/intl-test-helper'; + import {filterEmojiSearchInput} from './emoji_picker_base'; import EmojiPicker from './emoji_picker.ios'; -describe('components/emoji_picker/EmojiPicker', () => { +describe('components/emoji_picker/emoji_picker.ios', () => { + const state = { + ...initialState, + views: { + recentEmojis: [], + }, + }; + const emojis = selectEmojisByName(state); + const emojisBySection = selectEmojisBySection(state); + const options = { + shouldSort: false, + threshold: 0.3, + location: 0, + distance: 10, + includeMatches: true, + findAllMatches: true, + }; + const fuse = new Fuse(emojis, options); + const baseProps = { actions: { getCustomEmojis: jest.fn(), @@ -19,9 +40,9 @@ describe('components/emoji_picker/EmojiPicker', () => { customEmojisEnabled: false, customEmojiPage: 200, deviceWidth: 400, - emojis: [], - emojisBySection: [], - fuse: {}, + emojis, + emojisBySection, + fuse, isLandscape: false, theme: Preferences.THEMES.default, }; @@ -48,6 +69,15 @@ describe('components/emoji_picker/EmojiPicker', () => { expect(wrapper.getElement()).toMatchSnapshot(); }); + test('searchEmojis should return the right values on fuse', () => { + const input = '1'; + const output = ['100', '1234', '1st_place_medal', '+1', '-1', 'u7121']; + + const wrapper = shallowWithIntl(); + const result = wrapper.instance().searchEmojis(input); + expect(result).toEqual(output); + }); + test('should set rebuildEmojis to true when deviceWidth changes', () => { const wrapper = shallowWithIntl(); const instance = wrapper.instance(); @@ -84,7 +114,7 @@ describe('components/emoji_picker/EmojiPicker', () => { const setRebuiltEmojis = jest.spyOn(instance, 'setRebuiltEmojis'); setRebuiltEmojis(searchBarAnimationComplete); - expect(instance.setState).toHaveBeenCalledWith({emojis: []}); + expect(instance.setState).toHaveBeenCalledTimes(1); expect(instance.rebuildEmojis).toBe(false); }); diff --git a/app/components/emoji_picker/emoji_picker_base.js b/app/components/emoji_picker/emoji_picker_base.js index b54b37c6b..93a06261e 100644 --- a/app/components/emoji_picker/emoji_picker_base.js +++ b/app/components/emoji_picker/emoji_picker_base.js @@ -203,16 +203,24 @@ export default class EmojiPicker extends PureComponent { }; searchEmojis = (searchTerm) => { - const {emojis, fuse} = this.props; + const {fuse} = this.props; const searchTermLowerCase = searchTerm.toLowerCase(); if (!searchTerm) { return []; } - const results = fuse.search(searchTermLowerCase).map((r) => r.refIndex); - const sorter = (a, b) => compareEmojis(a, b, searchTerm); - const data = results.map((index) => emojis[index]).sort(sorter); + const sorter = (a, b) => compareEmojis(a, b, searchTermLowerCase); + const fuzz = fuse.search(searchTermLowerCase); + const results = fuzz.reduce((values, r) => { + const v = r.matches[0]?.value; + if (v) { + values.push(v); + } + + return values; + }, []); + const data = results.sort(sorter); return data; }; @@ -295,6 +303,7 @@ export default class EmojiPicker extends PureComponent { @@ -476,6 +485,10 @@ export const getStyleSheetFromTheme = makeStyleSheetFromTheme((theme) => { backgroundColor: theme.centerChannelBg, flex: 1, }, + emojiText: { + color: '#000', + fontWeight: 'bold', + }, flatList: { flex: 1, backgroundColor: theme.centerChannelBg, diff --git a/app/components/emoji_picker/index.js b/app/components/emoji_picker/index.js index f9d54a02a..0456f2571 100644 --- a/app/components/emoji_picker/index.js +++ b/app/components/emoji_picker/index.js @@ -3,154 +3,28 @@ import {connect} from 'react-redux'; import {bindActionCreators} from 'redux'; -import {createSelector} from 'reselect'; import Fuse from 'fuse.js'; import {incrementEmojiPickerPage} from '@actions/views/emoji'; import {getCustomEmojis} from '@mm-redux/actions/emojis'; import {getTheme} from '@mm-redux/selectors/entities/preferences'; -import {getCustomEmojisByName} from '@mm-redux/selectors/entities/emojis'; import {getConfig} from '@mm-redux/selectors/entities/general'; -import {createIdsSelector} from '@mm-redux/utils/helpers'; import {getDimensions, isLandscape} from '@selectors/device'; -import {BuiltInEmojis, CategoryNames, Emojis, EmojiIndicesByAlias, EmojiIndicesByCategory} from '@utils/emojis'; -import {t} from '@utils/i18n'; +import {selectEmojisByName, selectEmojisBySection} from '@selectors/emojis'; import EmojiPicker from './emoji_picker'; -const categoryToI18n = { - activity: { - id: t('mobile.emoji_picker.activity'), - defaultMessage: 'ACTIVITY', - icon: 'futbol-o', - }, - custom: { - id: t('mobile.emoji_picker.custom'), - defaultMessage: 'CUSTOM', - icon: 'at', - }, - flags: { - id: t('mobile.emoji_picker.flags'), - defaultMessage: 'FLAGS', - icon: 'flag-o', - }, - foods: { - id: t('mobile.emoji_picker.foods'), - defaultMessage: 'FOODS', - icon: 'cutlery', - }, - nature: { - id: t('mobile.emoji_picker.nature'), - defaultMessage: 'NATURE', - icon: 'leaf', - }, - objects: { - id: t('mobile.emoji_picker.objects'), - defaultMessage: 'OBJECTS', - icon: 'lightbulb-o', - }, - people: { - id: t('mobile.emoji_picker.people'), - defaultMessage: 'PEOPLE', - icon: 'smile-o', - }, - places: { - id: t('mobile.emoji_picker.places'), - defaultMessage: 'PLACES', - icon: 'plane', - }, - recent: { - id: t('mobile.emoji_picker.recent'), - defaultMessage: 'RECENTLY USED', - icon: 'clock-o', - }, - symbols: { - id: t('mobile.emoji_picker.symbols'), - defaultMessage: 'SYMBOLS', - icon: 'heart-o', - }, -}; - -function fillEmoji(indice) { - const emoji = Emojis[indice]; - return { - name: emoji.aliases[0], - aliases: emoji.aliases, - }; -} - -const getEmojisBySection = createSelector( - getCustomEmojisByName, - (state) => state.views.recentEmojis, - (customEmojis, recentEmojis) => { - const emoticons = CategoryNames.filter((name) => name !== 'custom').map((category) => { - const items = EmojiIndicesByCategory.get(category).map(fillEmoji); - - const section = { - ...categoryToI18n[category], - key: category, - data: items, - }; - - return section; - }); - - const customEmojiItems = []; - BuiltInEmojis.forEach((emoji) => { - customEmojiItems.push({ - name: emoji, - }); - }); - - for (const [key] of customEmojis) { - customEmojiItems.push({ - name: key, - }); - } - - emoticons.push({ - ...categoryToI18n.custom, - key: 'custom', - data: customEmojiItems, - }); - - if (recentEmojis.length) { - const items = recentEmojis.map((emoji) => ({name: emoji})); - - emoticons.unshift({ - ...categoryToI18n.recent, - key: 'recent', - data: items, - }); - } - - return emoticons; - }, -); - -const getEmojisByName = createIdsSelector( - getCustomEmojisByName, - (customEmojis) => { - const emoticons = new Set(); - for (const [key] of [...EmojiIndicesByAlias.entries(), ...customEmojis.entries()]) { - emoticons.add(key); - } - - return Array.from(emoticons); - }, -); - function mapStateToProps(state) { - const emojisBySection = getEmojisBySection(state); - const emojis = getEmojisByName(state); + const emojisBySection = selectEmojisBySection(state); + const emojis = selectEmojisByName(state); const {deviceWidth} = getDimensions(state); const options = { - shouldSort: true, + shouldSort: false, threshold: 0.3, location: 0, - distance: 100, - minMatchCharLength: 2, - maxPatternLength: 32, + distance: 10, + includeMatches: true, + findAllMatches: true, }; const list = emojis.length ? emojis : []; diff --git a/app/selectors/emojis.js b/app/selectors/emojis.js new file mode 100644 index 000000000..a8fe0e8cc --- /dev/null +++ b/app/selectors/emojis.js @@ -0,0 +1,131 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import {createSelector} from 'reselect'; + +import {t} from '@utils/i18n'; +import {getCustomEmojisByName as selectCustomEmojisByName} from '@mm-redux/selectors/entities/emojis'; +import {createIdsSelector} from '@mm-redux/utils/helpers'; +import {BuiltInEmojis, CategoryNames, Emojis, EmojiIndicesByAlias, EmojiIndicesByCategory} from '@utils/emojis'; + +const categoryToI18n = { + activity: { + id: t('mobile.emoji_picker.activity'), + defaultMessage: 'ACTIVITY', + icon: 'futbol-o', + }, + custom: { + id: t('mobile.emoji_picker.custom'), + defaultMessage: 'CUSTOM', + icon: 'at', + }, + flags: { + id: t('mobile.emoji_picker.flags'), + defaultMessage: 'FLAGS', + icon: 'flag-o', + }, + foods: { + id: t('mobile.emoji_picker.foods'), + defaultMessage: 'FOODS', + icon: 'cutlery', + }, + nature: { + id: t('mobile.emoji_picker.nature'), + defaultMessage: 'NATURE', + icon: 'leaf', + }, + objects: { + id: t('mobile.emoji_picker.objects'), + defaultMessage: 'OBJECTS', + icon: 'lightbulb-o', + }, + people: { + id: t('mobile.emoji_picker.people'), + defaultMessage: 'PEOPLE', + icon: 'smile-o', + }, + places: { + id: t('mobile.emoji_picker.places'), + defaultMessage: 'PLACES', + icon: 'plane', + }, + recent: { + id: t('mobile.emoji_picker.recent'), + defaultMessage: 'RECENTLY USED', + icon: 'clock-o', + }, + symbols: { + id: t('mobile.emoji_picker.symbols'), + defaultMessage: 'SYMBOLS', + icon: 'heart-o', + }, +}; + +function fillEmoji(indice) { + const emoji = Emojis[indice]; + return { + name: emoji.aliases[0], + aliases: emoji.aliases, + }; +} + +export const selectEmojisByName = createIdsSelector( + selectCustomEmojisByName, + (customEmojis) => { + const emoticons = new Set(); + for (const [key] of [...EmojiIndicesByAlias.entries(), ...customEmojis.entries()]) { + emoticons.add(key); + } + + return Array.from(emoticons); + }, +); + +export const selectEmojisBySection = createSelector( + selectCustomEmojisByName, + (state) => state.views.recentEmojis, + (customEmojis, recentEmojis) => { + const emoticons = CategoryNames.filter((name) => name !== 'custom').map((category) => { + const items = EmojiIndicesByCategory.get(category).map(fillEmoji); + + const section = { + ...categoryToI18n[category], + key: category, + data: items, + }; + + return section; + }); + + const customEmojiItems = []; + BuiltInEmojis.forEach((emoji) => { + customEmojiItems.push({ + name: emoji, + }); + }); + + for (const [key] of customEmojis) { + customEmojiItems.push({ + name: key, + }); + } + + emoticons.push({ + ...categoryToI18n.custom, + key: 'custom', + data: customEmojiItems, + }); + + if (recentEmojis.length) { + const items = recentEmojis.map((emoji) => ({name: emoji})); + + emoticons.unshift({ + ...categoryToI18n.recent, + key: 'recent', + data: items, + }); + } + + return emoticons; + }, +);