Skip to content

Static emoji autosuggestion #14686

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
5b1f23f
squash all commits
perunt Feb 16, 2023
4054f29
Merge branch 'main' of https://github.com/margelo/expensify-app-fork …
perunt Feb 16, 2023
d247dcc
remove space
perunt Feb 16, 2023
bd49ca0
update preferredSkinToneIndex
perunt Feb 23, 2023
d6fc1a9
Merge branch 'main' of https://github.com/margelo/expensify-app-fork …
perunt Feb 23, 2023
a80e5ff
add isExcludedTextAreaNodes to ArrowKeyFocusManager
perunt Feb 23, 2023
4cb3733
text centering
perunt Feb 23, 2023
0702b6e
move getColoredBackgroundStyle to StyleUtils
perunt Feb 28, 2023
3011816
remove space after merge
perunt Feb 28, 2023
6ad9af4
shouldExcludeTextAreaNodes
perunt Mar 1, 2023
95ebe2d
Merge branch 'main' of https://github.com/margelo/expensify-app-fork …
perunt Mar 1, 2023
74a3a68
fix conflicts after merge
perunt Mar 1, 2023
18e20de
Merge branch 'main' of https://github.com/Expensify/App into feature/…
perunt Mar 1, 2023
72dc2c3
remove colors
perunt Mar 1, 2023
25f287b
lint
perunt Mar 1, 2023
6f71301
changed structure
perunt Mar 2, 2023
0f8771b
update PropTypes values
perunt Mar 3, 2023
683bf67
merge
perunt Mar 3, 2023
b8b3a0d
use const for keyboard shortcuts
perunt Mar 6, 2023
26713f3
Merge branch 'main' of https://github.com/margelo/expensify-app-fork …
perunt Mar 6, 2023
6a074f1
use Boolean for shouldShowReportRecipientLocalTime
perunt Mar 7, 2023
f998dd1
Merge branch 'main' of https://github.com/Expensify/App into feature/…
perunt Mar 7, 2023
5c8d245
add keyboard shortcut TAB
perunt Mar 7, 2023
180c6df
Merge branch 'main' of https://github.com/Expensify/App into feature/…
perunt Mar 8, 2023
65604cb
call updateComment with debounce
perunt Mar 8, 2023
a10e829
Merge branch 'main' of https://github.com/margelo/expensify-app-fork …
perunt Mar 8, 2023
b7fc7e5
fix
perunt Mar 8, 2023
8a5e8fa
avoid anonymous function
perunt Mar 8, 2023
2f1befd
tests of getStyledTextArray
perunt Mar 9, 2023
ca9f306
clean code
perunt Mar 9, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions src/CONST.js
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,11 @@ const CONST = {
shortcutKey: 'ArrowDown',
modifiers: [],
},
TAB: {
descriptionKey: null,
shortcutKey: 'Tab',
modifiers: [],
},
},
KEYBOARD_SHORTCUT_KEY_DISPLAY_NAME: {
CONTROL: 'CTRL',
Expand Down Expand Up @@ -564,6 +569,14 @@ const CONST = {
NON_NATIVE_EMOJI_PICKER_LIST_HEIGHT: 256,
EMOJI_PICKER_ITEM_HEIGHT: 32,
EMOJI_PICKER_HEADER_HEIGHT: 32,
RECIPIENT_LOCAL_TIME_HEIGHT: 25,
EMOJI_SUGGESTER: {
SUGGESTER_PADDING: 6,
ITEM_HEIGHT: 36,
SMALL_CONTAINER_HEIGHT_FACTOR: 2.5,
MIN_AMOUNT_OF_ITEMS: 3,
MAX_AMOUNT_OF_ITEMS: 5,
},
COMPOSER_MAX_HEIGHT: 125,
CHAT_FOOTER_MIN_HEIGHT: 65,
CHAT_SKELETON_VIEW: {
Expand Down Expand Up @@ -829,6 +842,11 @@ const CONST = {
AFTER_FIRST_LINE_BREAK: /\n.*/g,
CODE_2FA: /^\d{6}$/,
ATTACHMENT_ID: /chat-attachments\/(\d+)/,
HAS_COLON_ONLY_AT_THE_BEGINNING: /^:[^:]+$/,
NEW_LINE_OR_WHITE_SPACE: /[\n\s]/g,

// Define the regular expression pattern to match a string starting with a colon and ending with a space or newline character
EMOJI_REPLACER: /^:[^\n\r]+?(?=$|\s)/,
MERGED_ACCOUNT_PREFIX: /^(MERGED_\d+@)/,
},

Expand Down Expand Up @@ -1003,6 +1021,8 @@ const CONST = {
CHAT_ATTACHMENT_TOKEN_KEY: 'X-Chat-Attachment-Token',

USA_COUNTRY_NAME,
SPACE_LENGTH: 1,
SPACE: 1,
Copy link
Contributor

Choose a reason for hiding this comment

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

This caused a regression #19289. In our world SPACE is still and not 1 😅.

Copy link
Contributor

Choose a reason for hiding this comment

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

whoooops 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

😄

ALL_COUNTRIES: {
AC: 'Ascension Island',
AD: 'Andorra',
Expand Down
8 changes: 6 additions & 2 deletions src/components/ArrowKeyFocusManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,14 @@ const propTypes = {

/** A callback executed when the focused input changes. */
onFocusedIndexChanged: PropTypes.func.isRequired,

/** If this value is true, then we exclude TextArea Node. */
shouldExcludeTextAreaNodes: PropTypes.bool,
};

const defaultProps = {
disabledIndexes: [],
shouldExcludeTextAreaNodes: true,
};

class ArrowKeyFocusManager extends Component {
Expand All @@ -48,7 +52,7 @@ class ArrowKeyFocusManager extends Component {
}

this.props.onFocusedIndexChanged(newFocusedIndex);
}, arrowUpConfig.descriptionKey, arrowUpConfig.modifiers, true, false, 0, true, ['TEXTAREA']);
}, arrowUpConfig.descriptionKey, arrowUpConfig.modifiers, true, false, 0, true, [this.props.shouldExcludeTextAreaNodes && 'TEXTAREA']);

this.unsubscribeArrowDownKey = KeyboardShortcut.subscribe(arrowDownConfig.shortcutKey, () => {
if (this.props.maxIndex < 0) {
Expand All @@ -66,7 +70,7 @@ class ArrowKeyFocusManager extends Component {
}

this.props.onFocusedIndexChanged(newFocusedIndex);
}, arrowDownConfig.descriptionKey, arrowDownConfig.modifiers, true, false, 0, true, ['TEXTAREA']);
}, arrowDownConfig.descriptionKey, arrowDownConfig.modifiers, true, false, 0, true, [this.props.shouldExcludeTextAreaNodes && 'TEXTAREA']);
}

componentWillUnmount() {
Expand Down
141 changes: 141 additions & 0 deletions src/components/EmojiSuggestions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
import React from 'react';
import {View, Pressable} from 'react-native';
import PropTypes from 'prop-types';
import _ from 'underscore';

// We take FlatList from this package to properly handle the scrolling of EmojiSuggestions in chats since one scroll is nested inside another
import {FlatList} from 'react-native-gesture-handler';
import styles from '../styles/styles';
import * as StyleUtils from '../styles/StyleUtils';
import * as EmojiUtils from '../libs/EmojiUtils';
import Text from './Text';
import CONST from '../CONST';
import getStyledTextArray from '../libs/GetStyledTextArray';

const propTypes = {
/** The index of the highlighted emoji */
highlightedEmojiIndex: PropTypes.number,

/** Array of suggested emoji */
emojis: PropTypes.arrayOf(PropTypes.shape({
/** The emoji code */
code: PropTypes.string,

/** The name of the emoji */
name: PropTypes.string,
})).isRequired,

/** Fired when the user selects an emoji */
onSelect: PropTypes.func.isRequired,

/** Emoji prefix that follows the colon */
prefix: PropTypes.string.isRequired,

/** Show that we can use large emoji picker.
* Depending on available space and whether the input is expanded, we can have a small or large emoji suggester.
* When this value is false, the suggester will have a height of 2.5 items. When this value is true, the height can be up to 5 items. */
isEmojiPickerLarge: PropTypes.bool.isRequired,

/** Show that we should include ReportRecipientLocalTime view height */
shouldIncludeReportRecipientLocalTimeHeight: PropTypes.bool.isRequired,

/** Stores user's preferred skin tone */
preferredSkinToneIndex: PropTypes.number.isRequired,
};

const defaultProps = {
highlightedEmojiIndex: 0,
};

/**
* @param {Number} numRows
* @param {Boolean} isEmojiPickerLarge
* @returns {Number}
*/
const measureHeightOfEmojiRows = (numRows, isEmojiPickerLarge) => {
if (isEmojiPickerLarge) {
return numRows * CONST.EMOJI_SUGGESTER.ITEM_HEIGHT;
}
if (numRows > 2) {
// on small screens, we display a scrollable window with a height of 2.5 items, indicating that there are more items available beyond what is currently visible
return CONST.EMOJI_SUGGESTER.SMALL_CONTAINER_HEIGHT_FACTOR * CONST.EMOJI_SUGGESTER.ITEM_HEIGHT;
}
return numRows * CONST.EMOJI_SUGGESTER.ITEM_HEIGHT;
};

/**
* Create unique keys for each emoji item
* @param {Object} item
* @param {Number} index
* @returns {String}
*/
const keyExtractor = (item, index) => `${item.name}+${index}}`;

const EmojiSuggestions = (props) => {
/**
* Render a suggestion menu item component.
* @param {Object} params.item
* @param {Number} params.index
* @returns {JSX.Element}
*/
const renderSuggestionMenuItem = ({item, index}) => {
const styledTextArray = getStyledTextArray(item.name, props.prefix);

return (
<Pressable
style={({hovered}) => StyleUtils.getEmojiSuggestionItemStyle(
props.highlightedEmojiIndex,
CONST.EMOJI_SUGGESTER.ITEM_HEIGHT,
hovered,
index,
)}
onMouseDown={e => e.preventDefault()}
Copy link
Member

Choose a reason for hiding this comment

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

Hi! I'm coming from #16485

onMouseDown={e => e.preventDefault()} prevents the focus from moving and the keyboard from closing. But unfortunately, when we long press the emoji element, the mousedown event is not fired on mWeb Android.

Causing the bug - #16485

The mWeb Android behavior is well demonstrated here.

onPress={() => props.onSelect(index)}
>
<View style={styles.emojiSuggestionContainer}>
<Text style={styles.emojiSuggestionsEmoji}>{EmojiUtils.getEmojiCodeWithSkinColor(item, props.preferredSkinToneIndex)}</Text>
<Text style={styles.emojiSuggestionsText}>
:
{_.map(styledTextArray, ({text, isColored}, i) => (
<Text key={`${text}+${i}`} style={StyleUtils.getColoredBackgroundStyle(isColored)}>
{text}
</Text>
))}
:
</Text>
</View>
</Pressable>
);
};

const rowHeight = measureHeightOfEmojiRows(
props.emojis.length,
props.isEmojiPickerLarge,
);

return (
<View
style={[
styles.emojiSuggestionsContainer,
StyleUtils.getEmojiSuggestionContainerStyle(
rowHeight,
props.shouldIncludeReportRecipientLocalTimeHeight,
),
]}
>
<FlatList
keyboardShouldPersistTaps="handled"
data={props.emojis}
renderItem={renderSuggestionMenuItem}
keyExtractor={keyExtractor}
style={{height: rowHeight}}
/>
</View>
);
};

EmojiSuggestions.propTypes = propTypes;
EmojiSuggestions.defaultProps = defaultProps;
EmojiSuggestions.displayName = 'EmojiSuggestions';

export default EmojiSuggestions;
17 changes: 17 additions & 0 deletions src/libs/EmojiUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,22 @@ function suggestEmojis(text, limit = 5) {
return [];
}

/**
* Given an emoji item object, return an emoji code based on its type.
*
* @param {Object} item
* @param {Number} preferredSkinToneIndex
* @returns {String}
*/
const getEmojiCodeWithSkinColor = (item, preferredSkinToneIndex) => {
const {code, types} = item;
if (types && types[preferredSkinToneIndex]) {
return types[preferredSkinToneIndex];
}

return code;
};

export {
getHeaderEmojis,
mergeEmojisWithFrequentlyUsedEmojis,
Expand All @@ -255,4 +271,5 @@ export {
replaceEmojis,
suggestEmojis,
trimEmojiUnicode,
getEmojiCodeWithSkinColor,
};
36 changes: 36 additions & 0 deletions src/libs/GetStyledTextArray.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/**
* Render a suggestion menu item component.
* @param {String} name
* @param {String} prefix
* @returns {Array}
*/
const getStyledTextArray = (name, prefix) => {
const texts = [];
const prefixLocation = name.search(prefix);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is another issue when you type this emoji :+1: the app will crash, that’s because /+1/ isn’t a valid regular expression, It should have been escaped with Str.escapeForRegEx

    const prefixLocation = name.search(Str.escapeForRegExp(prefix));
Screen.Recording.2023-03-11.at.12.29.12.AM.mov


if (prefixLocation === 0 && prefix.length === name.length) {
texts.push({text: prefix, isColored: true});
Copy link
Contributor

Choose a reason for hiding this comment

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

text should be name instead of prefix.
Later, this caused minor regression - #32749

} else if (prefixLocation === 0 && prefix.length !== name.length) {
texts.push(
{text: name.slice(0, prefix.length), isColored: true},
{text: name.slice(prefix.length), isColored: false},
);
} else if (prefixLocation > 0 && prefix.length !== name.length) {
texts.push(
{text: name.slice(0, prefixLocation), isColored: false},
{
text: name.slice(prefixLocation, prefixLocation + prefix.length),
isColored: true,
},
{
text: name.slice(prefixLocation + prefix.length),
isColored: false,
},
);
} else {
texts.push({text: name, isColored: false});
}
return texts;
};

export default getStyledTextArray;
Loading