[GH-11437] Fix thumbs emoji sorting logic for mobile (#3633)

* [GH-11437] Fix thumbs emoji sorting logic for mobile

* Add unit tests

* Sort emojis in selector and handle string emojis

* Remove redundant serverVersion logic

* Fix typo in app/utils/emoji_utils.test.js

Co-Authored-By: Saturnino Abril <saturnino.abril@gmail.com>
This commit is contained in:
Nev Angelova 2019-12-08 14:10:56 +01:00 committed by Elias Nahum
parent 0b3ba08030
commit f6addb491b
6 changed files with 183 additions and 45 deletions

View file

@ -1,7 +1,7 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
import React, {Component} from 'react';
import React, {PureComponent} from 'react';
import PropTypes from 'prop-types';
import {
FlatList,
@ -10,8 +10,6 @@ import {
View,
} from 'react-native';
import {isMinimumServerVersion} from 'mattermost-redux/utils/helpers';
import AutocompleteDivider from 'app/components/autocomplete/autocomplete_divider';
import Emoji from 'app/components/emoji';
import TouchableWithFeedback from 'app/components/touchable_with_feedback';
@ -22,7 +20,7 @@ import {makeStyleSheetFromTheme} from 'app/utils/theme';
const EMOJI_REGEX = /(^|\s|^\+|^-)(:([^:\s]*))$/i;
const EMOJI_REGEX_WITHOUT_PREFIX = /\B(:([^:\s]*))$/i;
export default class EmojiSuggestion extends Component {
export default class EmojiSuggestion extends PureComponent {
static propTypes = {
actions: PropTypes.shape({
addReactionToLatestPost: PropTypes.func.isRequired,
@ -38,7 +36,6 @@ export default class EmojiSuggestion extends Component {
onResultCountChange: PropTypes.func.isRequired,
rootId: PropTypes.string,
value: PropTypes.string,
serverVersion: PropTypes.string,
nestedScrollEnabled: PropTypes.bool,
};
@ -80,35 +77,15 @@ export default class EmojiSuggestion extends Component {
const oldMatchTerm = this.matchTerm;
this.matchTerm = match[3] || '';
// If we're server version 4.7 or higher
if (isMinimumServerVersion(this.props.serverVersion, 4, 7)) {
if (this.matchTerm !== oldMatchTerm && this.matchTerm.length) {
this.props.actions.autocompleteCustomEmojis(this.matchTerm);
return;
}
if (this.matchTerm.length) {
this.handleFuzzySearch(this.matchTerm, nextProps);
} else {
const initialEmojis = [...nextProps.emojis];
initialEmojis.splice(0, 300);
const data = initialEmojis.sort();
this.setEmojiData(data);
}
if (this.matchTerm !== oldMatchTerm && this.matchTerm.length) {
this.props.actions.autocompleteCustomEmojis(this.matchTerm);
return;
}
// If we're server version 4.6 or lower
if (this.matchTerm !== oldMatchTerm) {
if (this.matchTerm.length) {
this.handleFuzzySearch(this.matchTerm, nextProps);
} else if (!this.matchTerm.length) {
const initialEmojis = [...nextProps.emojis];
initialEmojis.splice(0, 300);
const data = initialEmojis.sort();
this.setEmojiData(data);
} else {
this.setEmojiData(nextProps.emojis);
}
}

View file

@ -11,6 +11,7 @@ import {autocompleteCustomEmojis} from 'mattermost-redux/actions/emojis';
import {addReactionToLatestPost} from 'app/actions/views/emoji';
import {getTheme} from 'mattermost-redux/selectors/entities/preferences';
import {EmojiIndicesByAlias} from 'app/utils/emojis';
import {compareEmojis} from 'app/utils/emoji_utils';
import EmojiSuggestion from './emoji_suggestion';
import Fuse from 'fuse.js';
@ -23,7 +24,7 @@ const getEmojisByName = createSelector(
emoticons.add(key);
}
return Array.from(emoticons);
return Array.from(emoticons).sort(compareEmojis);
}
);
@ -45,7 +46,6 @@ function mapStateToProps(state) {
fuse,
emojis,
theme: getTheme(state),
serverVersion: state.entities.general.serverVersion,
};
}

View file

@ -16,8 +16,6 @@ import {
import FontAwesomeIcon from 'react-native-vector-icons/FontAwesome';
import sectionListGetItemLayout from 'react-native-section-list-get-item-layout';
import {isMinimumServerVersion} from 'mattermost-redux/utils/helpers';
import Emoji from 'app/components/emoji';
import FormattedText from 'app/components/formatted_text';
import {DeviceTypes} from 'app/constants';
@ -50,7 +48,6 @@ export default class EmojiPicker extends PureComponent {
fuse: PropTypes.object.isRequired,
isLandscape: PropTypes.bool.isRequired,
onEmojiPress: PropTypes.func,
serverVersion: PropTypes.string,
theme: PropTypes.object.isRequired,
actions: PropTypes.shape({
getCustomEmojis: PropTypes.func.isRequired,
@ -87,13 +84,13 @@ export default class EmojiPicker extends PureComponent {
filteredEmojis: [],
searchTerm: '',
currentSectionIndex: 0,
missingPages: isMinimumServerVersion(this.props.serverVersion, 4, 7),
missingPages: true,
};
}
componentWillReceiveProps(nextProps) {
componentDidUpdate(prevProps) {
this.rebuildEmojis = false;
if (this.props.deviceWidth !== nextProps.deviceWidth) {
if (this.props.deviceWidth !== prevProps.deviceWidth) {
this.rebuildEmojis = true;
if (this.searchBarRef) {
@ -101,7 +98,7 @@ export default class EmojiPicker extends PureComponent {
}
}
if (this.props.emojis !== nextProps.emojis) {
if (this.props.emojis !== prevProps.emojis) {
this.rebuildEmojis = true;
this.setRebuiltEmojis();
}
@ -190,9 +187,6 @@ export default class EmojiPicker extends PureComponent {
clearTimeout(this.searchTermTimeout);
const timeout = searchTerm ? 100 : 0;
this.searchTermTimeout = setTimeout(async () => {
if (isMinimumServerVersion(this.props.serverVersion, 4, 7)) {
await this.props.actions.searchCustomEmojis(searchTerm);
}
const filteredEmojis = this.searchEmojis(searchTerm);
this.setState({
filteredEmojis,
@ -308,7 +302,7 @@ export default class EmojiPicker extends PureComponent {
};
loadMoreCustomEmojis = async () => {
if (!this.props.customEmojisEnabled || !isMinimumServerVersion(this.props.serverVersion, 4, 7)) {
if (!this.props.customEmojisEnabled) {
return;
}

View file

@ -165,7 +165,6 @@ function mapStateToProps(state) {
theme: getTheme(state),
customEmojisEnabled: getConfig(state).EnableCustomEmoji === 'true',
customEmojiPage: state.views.emoji.emojiPickerCustomPage,
serverVersion: state.entities.general.serverVersion,
};
}

View file

@ -113,3 +113,42 @@ export function getEmojiByName(emojiName) {
return null;
}
// Since there is no shared logic between the web and mobile app
// this is copied from the webapp as custom sorting logic for emojis
const defaultComparisonRule = (aName, bName) => {
return aName.localeCompare(bName);
};
const thumbsDownComparisonRule = (other) =>
(other === 'thumbsup' || other === '+1' ? 1 : 0);
const thumbsUpComparisonRule = (other) => (other === 'thumbsdown' || other === '-1' ? -1 : 0);
const customComparisonRules = {
thumbsdown: thumbsDownComparisonRule,
'-1': thumbsDownComparisonRule,
thumbsup: thumbsUpComparisonRule,
'+1': thumbsUpComparisonRule,
};
export function compareEmojis(emojiA, emojiB, searchedName) {
const aName = emojiA.name || (emojiA.aliases ? emojiA.aliases[0] : emojiA);
const bName = emojiB.name || (emojiB.aliases ? emojiB.aliases[0] : emojiB);
// Have the emojis that contain the search appear first
const aPrefix = aName.startsWith(searchedName);
const bPrefix = bName.startsWith(searchedName);
if (aPrefix === bPrefix) {
if (customComparisonRules[aName]) {
return customComparisonRules[aName](bName) || defaultComparisonRule(aName, bName);
}
return defaultComparisonRule(aName, bName, searchedName);
} else if (aPrefix) {
return -1;
}
return 1;
}

View file

@ -1,7 +1,7 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
import {doesMatchNamedEmoji, hasEmojisOnly} from './emoji_utils';
import {doesMatchNamedEmoji, hasEmojisOnly, compareEmojis} from './emoji_utils';
describe('hasEmojisOnly with named emojis', () => {
const testCases = [{
@ -283,3 +283,132 @@ describe('doesMatchNamedEmoji', () => {
});
}
});
describe('compareEmojis', () => {
test('should sort an array of emojis alphabetically', () => {
const goatEmoji = {
name: 'goat',
};
const dashEmoji = {
name: 'dash',
};
const smileEmoji = {
name: 'smile',
};
const emojiArray = [goatEmoji, dashEmoji, smileEmoji];
emojiArray.sort((a, b) => compareEmojis(a, b));
expect(emojiArray).toEqual([dashEmoji, goatEmoji, smileEmoji]);
});
test('should have partial matched emoji first', () => {
const goatEmoji = {
name: 'goat',
};
const dashEmoji = {
name: 'dash',
};
const smileEmoji = {
name: 'smile',
};
const emojiArray = [goatEmoji, dashEmoji, smileEmoji];
emojiArray.sort((a, b) => compareEmojis(a, b, 'smi'));
expect(emojiArray).toEqual([smileEmoji, dashEmoji, goatEmoji]);
});
test('should be able to sort on aliases', () => {
const goatEmoji = {
aliases: [':goat:'],
};
const dashEmoji = {
aliases: [':dash:'],
};
const smileEmoji = {
aliases: [':smile:'],
};
const emojiArray = [goatEmoji, dashEmoji, smileEmoji];
emojiArray.sort((a, b) => compareEmojis(a, b));
expect(emojiArray).toEqual([dashEmoji, goatEmoji, smileEmoji]);
});
test('special case for thumbsup emoji should sort it before thumbsdown by aliases', () => {
const thumbsUpEmoji = {
aliases: ['+1'],
};
const thumbsDownEmoji = {
aliases: ['-1'],
};
const smileEmoji = {
aliases: ['smile'],
};
const emojiArray = [thumbsDownEmoji, thumbsUpEmoji, smileEmoji];
emojiArray.sort((a, b) => compareEmojis(a, b));
expect(emojiArray).toEqual([thumbsUpEmoji, thumbsDownEmoji, smileEmoji]);
});
test('special case for thumbsup emoji should sort it before thumbsdown by names', () => {
const thumbsUpEmoji = {
name: 'thumbsup',
};
const thumbsDownEmoji = {
name: 'thumbsdown',
};
const smileEmoji = {
name: 'smile',
};
const emojiArray = [thumbsDownEmoji, thumbsUpEmoji, smileEmoji];
emojiArray.sort((a, b) => compareEmojis(a, b));
expect(emojiArray).toEqual([smileEmoji, thumbsUpEmoji, thumbsDownEmoji]);
});
test('special case for thumbsup emoji should sort it when emoji is matched', () => {
const thumbsUpEmoji = {
name: 'thumbsup',
};
const thumbsDownEmoji = {
name: 'thumbsdown',
};
const smileEmoji = {
name: 'smile',
};
const emojiArray = [thumbsDownEmoji, thumbsUpEmoji, smileEmoji];
emojiArray.sort((a, b) => compareEmojis(a, b, 'thumb'));
expect(emojiArray).toEqual([thumbsUpEmoji, thumbsDownEmoji, smileEmoji]);
});
test('it compares emojis when emojis are strings', () => {
const thumbsUpEmoji = 'thumbsup';
const thumbsDownEmoji = 'thumbsdown';
const smileEmoji = {
name: 'smile',
};
const emojiArray = [thumbsDownEmoji, thumbsUpEmoji, smileEmoji];
emojiArray.sort((a, b) => compareEmojis(a, b, 'thumb'));
expect(emojiArray).toEqual([thumbsUpEmoji, thumbsDownEmoji, smileEmoji]);
});
});