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

Commit

Permalink
MM-41911 Move normalization of recent emojis to getRecentEmojis (#10321)
Browse files Browse the repository at this point in the history 
* Add type definitions for emoji.jsx and EmojiMap

* MM-41911 Move normalization of recent emojis to getRecentEmojis

* Clarify comment

* Revert swapped argument order

* Fix mock name
  • Loading branch information
@hmhealey
hmhealey committed Jun 16, 2022
1 parent 5cd3fbf commit a041fc2
Show file tree
Hide file tree
Showing 26 changed files with 725 additions and 188 deletions.
4 changes: 2 additions & 2 deletions actions/emoji_actions.js
View file
Expand Up @@ -6,7 +6,7 @@ import {getCustomEmojisByName} from 'mattermost-redux/selectors/entities/emojis'
import {getConfig} from 'mattermost-redux/selectors/entities/general';
import {getCurrentUserId} from 'mattermost-redux/selectors/entities/users';

import {getEmojiMap, getRecentEmojis, getRecentEmojisNames, isCustomEmojiEnabled} from 'selectors/emojis';
import {getEmojiMap, getRecentEmojisData, getRecentEmojisNames, isCustomEmojiEnabled} from 'selectors/emojis';
import {isCustomStatusEnabled, makeGetCustomStatus} from 'selectors/views/custom_status';
import {savePreferences} from 'mattermost-redux/actions/preferences';

Expand Down Expand Up @@ -64,7 +64,7 @@ export function addRecentEmoji(alias) {
return (dispatch, getState) => {
const state = getState();
const currentUserId = getCurrentUserId(state);
const recentEmojis = getRecentEmojis(state);
const recentEmojis = getRecentEmojisData(state);
const emojiMap = getEmojiMap(state);

let name;
Expand Down
14 changes: 7 additions & 7 deletions actions/emoji_actions.test.js
View file
Expand Up @@ -5,7 +5,7 @@ import configureStore from 'redux-mock-store';

import thunk from 'redux-thunk';

import {getRecentEmojis, getEmojiMap} from 'selectors/emojis';
import {getRecentEmojisData, getEmojiMap} from 'selectors/emojis';
import * as EmojiActions from 'actions/emoji_actions';
import * as PreferenceActions from 'mattermost-redux/actions/preferences';

Expand All @@ -21,7 +21,7 @@ const initialState = {
const mockStore = configureStore([thunk]);

jest.mock('selectors/emojis', () => ({
getRecentEmojis: jest.fn(),
getRecentEmojisData: jest.fn(),
getEmojiMap: jest.fn(),
}));

Expand All @@ -36,7 +36,7 @@ describe('Actions.Emojis', () => {
});

test('Emoji alias is stored in recent emojis', async () => {
getRecentEmojis.mockImplementation(() => {
getRecentEmojisData.mockImplementation(() => {
return [];
});

Expand Down Expand Up @@ -69,7 +69,7 @@ describe('Actions.Emojis', () => {
});

test('First alias is stored in recent emojis even if second alias used', async () => {
getRecentEmojis.mockImplementation(() => {
getRecentEmojisData.mockImplementation(() => {
return [];
});

Expand Down Expand Up @@ -102,7 +102,7 @@ describe('Actions.Emojis', () => {
});

test('Invalid emoji are not stored in recents', async () => {
getRecentEmojis.mockImplementation(() => {
getRecentEmojisData.mockImplementation(() => {
return [];
});

Expand All @@ -119,7 +119,7 @@ describe('Actions.Emojis', () => {
});

test('Emoji already present in recent should be bumped on the top', async () => {
getRecentEmojis.mockImplementation(() => {
getRecentEmojisData.mockImplementation(() => {
return [
{name: 'smile', usageCount: 1},
{name: 'grinning', usageCount: 1},
Expand Down Expand Up @@ -186,7 +186,7 @@ describe('Actions.Emojis', () => {
{name: '22', usageCount: 1},
{name: '23', usageCount: 1},
];
getRecentEmojis.mockImplementation(() => {
getRecentEmojisData.mockImplementation(() => {
return recentEmojisList;
});

Expand Down
25 changes: 16 additions & 9 deletions build/emoji/emoji_gen.mjs
View file
Expand Up @@ -285,7 +285,15 @@ fullEmoji.forEach((emoji, index) => {
emoji.image = file;

if (emoji.category !== 'custom') {
emojiFilePositions.set(file, `-${emoji.sheet_x * EMOJI_SIZE_PADDED}px -${emoji.sheet_y * EMOJI_SIZE_PADDED}px;`);
let x = emoji.sheet_x * EMOJI_SIZE_PADDED;
if (x !== 0) {
x += 'px';
}
let y = emoji.sheet_y * EMOJI_SIZE_PADDED;
if (y !== 0) {
y += 'px';
}
emojiFilePositions.set(file, `-${x} -${y};`);
}

emojiImagesByAlias.push(...emoji.short_names.map((alias) => `"${alias}": "${file}"`));
Expand Down Expand Up @@ -342,13 +350,12 @@ export const SkinTranslations = new Map([${skinTranslations.join(', ')}]);
export const ComponentCategory = 'Component';
export const AllEmojiIndicesByCategory = new Map(${JSON.stringify(Array.from(emojiIndicesByCategory))});
const AllEmojiIndicesByCategory = new Map(${JSON.stringify(Array.from(emojiIndicesByCategory))});
export const EmojiIndicesByCategoryAndSkin = new Map([${writeableSkinCategories.join(', ')}]);
export const EmojiIndicesByCategoryNoSkin = new Map(${JSON.stringify(Array.from(emojiIndicesByCategoryNoSkin))});
const EmojiIndicesByCategoryAndSkin = new Map([${writeableSkinCategories.join(', ')}]);
const EmojiIndicesByCategoryNoSkin = new Map(${JSON.stringify(Array.from(emojiIndicesByCategoryNoSkin))});
export const skinCodes = ${JSON.stringify(skinCodes)};
export const EMOJI_DEFAULT_SKIN = '${EMOJI_DEFAULT_SKIN}';
const skinCodes = ${JSON.stringify(skinCodes)};
// Generate the list of indices that belong to each category by an specified skin
function genSkinnedCategories(skin) {
Expand All @@ -364,7 +371,7 @@ function genSkinnedCategories(skin) {
return result;
}
export const getSkinnedCategories = memoize(genSkinnedCategories);
const getSkinnedCategories = memoize(genSkinnedCategories);
export const EmojiIndicesByCategory = new Map([${skinnedCats.join(', ')}]);
`;

Expand Down Expand Up @@ -446,8 +453,8 @@ const cssRules = `
zoom: 0.35;
}
${cssCats.join('\n')};
${cssEmojis.join('\n')};
${cssCats.join('\n')}
${cssEmojis.join('\n')}
`;

// write emoji.jsx
Expand Down
5 changes: 3 additions & 2 deletions components/emoji/add_emoji/add_emoji.test.tsx
View file
Expand Up @@ -8,7 +8,8 @@ import {Team} from '@mattermost/types/teams';
import {CustomEmoji} from '@mattermost/types/emojis';
import {UserProfile} from '@mattermost/types/users';

import EmojiMap from 'utils/emoji_map.js';
import EmojiMap from 'utils/emoji_map';
import {TestHelper} from 'utils/test_helper';

import AddEmoji, {AddEmojiProps} from './add_emoji';

Expand All @@ -24,7 +25,7 @@ const image = 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAEgAAABICAYAAABV7bN

describe('components/emoji/components/AddEmoji', () => {
const baseProps: AddEmojiProps = {
emojiMap: new EmojiMap(new Map<string, unknown>([['mycustomemoji', {}]])),
emojiMap: new EmojiMap(new Map([['mycustomemoji', TestHelper.getCustomEmojiMock({name: 'mycustomemoji'})]])),
team: {
id: 'team-id',
} as Team,
Expand Down
4 changes: 2 additions & 2 deletions components/emoji_picker/components/emoji_picker_skin.tsx
View file
Expand Up @@ -63,7 +63,7 @@ export class EmojiPickerSkin extends React.PureComponent<Props, State> {
extended() {
const choices = skinsList.map((skinPair) => {
const skin = skinPair[1];
const emoji = skinToneEmojis.get(skin);
const emoji = skinToneEmojis.get(skin)!;
const spriteClassName = classNames('emojisprite', `emoji-category-${emoji.category}`, `emoji-${emoji.unified.toLowerCase()}`);

return (
Expand Down Expand Up @@ -104,7 +104,7 @@ export class EmojiPickerSkin extends React.PureComponent<Props, State> {
);
}
collapsed() {
const emoji = skinToneEmojis.get(this.props.userSkinTone);
const emoji = skinToneEmojis.get(this.props.userSkinTone)!;
const spriteClassName = classNames('emojisprite', `emoji-category-${emoji.category}`, `emoji-${emoji.unified.toLowerCase()}`);
const tooltip = (
<Tooltip
Expand Down
2 changes: 1 addition & 1 deletion components/emoji_picker/constants/index.ts
View file
Expand Up @@ -32,7 +32,7 @@ function createCategory(name: EmojiCategory): Category {
name,
id: Emoji.CategoryTranslations.get(name) || '',
className: categoryClass.get(name) || '',
message: Emoji.CategoryMessage.get(name),
message: Emoji.CategoryMessage.get(name)!,
};
}

Expand Down
2 changes: 1 addition & 1 deletion components/emoji_picker/emoji_picker.test.tsx
View file
Expand Up @@ -26,7 +26,7 @@ describe('components/emoji_picker/EmojiPicker', () => {
handleFilterChange: jest.fn(),
customEmojisEnabled: false,
customEmojiPage: 1,
emojiMap: new EmojiMap([]),
emojiMap: new EmojiMap(new Map()),
recentEmojis: [],
userSkinTone: 'default',
currentTeamName: 'testTeam',
Expand Down
2 changes: 1 addition & 1 deletion components/emoji_picker/emoji_picker.tsx
View file
Expand Up @@ -121,7 +121,7 @@ const EmojiPicker = ({
setCategoryOrEmojisRows(updatedCategoryOrEmojisRows);
setEmojiPositionsArray(updatedEmojiPositions);
throttledSearchCustomEmoji.current(filter, customEmojisEnabled);
}, [filter, userSkinTone, shouldRunCreateCategoryAndEmojiRows.current, customEmojisEnabled]);
}, [filter, shouldRunCreateCategoryAndEmojiRows.current, customEmojisEnabled]);

// Hack for getting focus on search input when tab changes to emoji from gifs
useEffect(() => {
Expand Down
29 changes: 7 additions & 22 deletions components/emoji_picker/utils/index.test.ts
View file
Expand Up @@ -192,7 +192,7 @@ describe('getFilteredEmojis', () => {
expect(getFilteredEmojis(allEmojis as any, filter, recentEmojisString, userSkinTone)).toEqual(filteredResults);
});

test('Should filter emojis containning skin tone with user skin tone', () => {
test('Should filter emojis containing skin tone with user skin tone', () => {
const allEmojis = {
thumbsup: thumbsupEmoji,
thumbsupDark: thumbsupEmojiDarkSkin,
Expand All @@ -203,6 +203,8 @@ describe('getFilteredEmojis', () => {
const recentEmojisString: string[] = [];
const userSkinTone = SkinTones.Dark;

// Note that filteredResults doesn't match what will be returned in a real use case because the variants of
// thumbsup will be deduped when using non-test data
const filteredResults = [
thumbsupEmoji,
thumbsupEmojiDarkSkin,
Expand All @@ -211,36 +213,19 @@ describe('getFilteredEmojis', () => {
expect(getFilteredEmojis(allEmojis as any, filter, recentEmojisString, userSkinTone)).toEqual(filteredResults);
});

test('Should filter recent emojis containning skin tone with user skin tone', () => {
test('Should filter recent emojis', () => {
const allEmojis = {
thumbsup: thumbsupEmoji,
thumbsupDark: thumbsupEmojiDarkSkin,
thumbsupLight: thumbsupEmojiLightSkin,
thumbsupMedium: thumbsupEmojiMediumSkin,
};
const filter = 'thumbs';
const recentEmojisString = ['thumbsupMedium', 'thumbsupDark'];
const userSkinTone = SkinTones.Dark;

const filteredResults = [
thumbsupEmojiDarkSkin,
thumbsupEmoji,
];

expect(getFilteredEmojis(allEmojis as any, filter, recentEmojisString, userSkinTone)).toEqual(filteredResults);
});

test('Should filter recent emojis containning skin tone and replace with user skin tone', () => {
const allEmojis = {
thumbsup: thumbsupEmoji,
thumbsupDark: thumbsupEmojiDarkSkin,
thumbsupLight: thumbsupEmojiLightSkin,
thumbsupMedium: thumbsupEmojiMediumSkin,
};
const filter = 'thumbs';
const recentEmojisString = ['thumbsupMedium', 'thumbsupLight'];
const recentEmojisString = ['thumbsupDark'];
const userSkinTone = SkinTones.Dark;

// Note that filteredResults doesn't match what will be returned in a real use case because the variants of
// thumbsup will be deduped when using non-test data
const filteredResults = [
thumbsupEmojiDarkSkin,
thumbsupEmoji,
Expand Down
44 changes: 5 additions & 39 deletions components/emoji_picker/utils/index.ts
View file
Expand Up @@ -16,9 +16,8 @@ import {
EmojiCursor,
} from 'components/emoji_picker/types';

import {EmojiIndicesByCategory, Emojis as EmojisJson, EmojiIndicesByUnicode} from 'utils/emoji.jsx';
import {getSkin, emojiMatchesSkin} from 'utils/emoticons';
import {compareEmojis} from 'utils/emoji_utils';
import {EmojiIndicesByCategory, Emojis as EmojisJson} from 'utils/emoji';
import {compareEmojis, convertEmojiSkinTone, emojiMatchesSkin, getSkin} from 'utils/emoji_utils';
import EmojiMap from 'utils/emoji_map';

import {
Expand All @@ -38,7 +37,7 @@ export function isCategoryHeaderRow(row: CategoryOrEmojiRow): row is CategoryHea
function updateSkinTone(initialEmoji: SystemEmoji, skinTone: string): Emoji {
const initialEmojiSkin = getSkin(initialEmoji);
if (initialEmojiSkin && initialEmojiSkin !== skinTone) {
const emojiWithUpdatedSkinTone = convertEmojiSkinTone(initialEmoji, initialEmojiSkin, skinTone);
const emojiWithUpdatedSkinTone = convertEmojiSkinTone(initialEmoji, skinTone);
if (emojiWithUpdatedSkinTone && emojiWithUpdatedSkinTone.unified) {
return emojiWithUpdatedSkinTone;
}
Expand Down Expand Up @@ -101,47 +100,16 @@ export function getFilteredEmojis(allEmojis: Record<string, Emoji>, filter: stri
return filteredEmojisUserSkinTone;
}

// Note : This function is not an idea implementation, a more better and efficeint way to do this come when we make changes to emoji json.
function convertEmojiSkinTone(emoji: SystemEmoji, emojiSkin: string, skinTone: string) {
let newEmojiId = '';

// If its a default (yellow) emoji, get the skin variation from its property
if (emojiSkin === 'default') {
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
const variation = Object.keys(emoji?.skin_variations).find((skinVariation) => skinVariation.includes(skinTone));
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
newEmojiId = variation ? emoji.skin_variations[variation].unified : emoji.unified;
} else if (skinTone === 'default') {
// If default (yellow) skin is selected, remove the skin code from emoji id
newEmojiId = emoji.unified.replaceAll(/-(1F3FB|1F3FC|1F3FD|1F3FE|1F3FF)/g, '');
} else {
// If non default skin is selected, add the new skin selected code to emoji id
newEmojiId = emoji.unified.replaceAll(/(1F3FB|1F3FC|1F3FD|1F3FE|1F3FF)/g, skinTone);
}

const emojiIndex = EmojiIndicesByUnicode.get(newEmojiId.toLowerCase()) as number;
return EmojisJson[emojiIndex];
}

export function getEmojisByCategory(
function getEmojisByCategory(
allEmojis: Record<string, Emoji>,
category: Category,
categoryName: EmojiCategory,
userSkinTone: string,
): Emoji[] {
const emojiIds = category?.emojiIds ?? [];

if (emojiIds.length === 0) {
return [];
}

// For recent category, perform checks for skin tone uniformity
if (categoryName === 'recent') {
return convertEmojisToUserSkinTone(emojiIds, allEmojis, userSkinTone);
}

// For all other categories, return emojis of the categoryies from allEmojis
return emojiIds.map((emojiId) => allEmojis[emojiId]);
}
Expand All @@ -165,7 +133,7 @@ export function getUpdatedCategoriesAndAllEmojis(
return emojiMap.has(name);
}).
map((name) => {
return emojiMap.get(name);
return emojiMap.get(name)!;
});
} else {
const indices = (EmojiIndicesByCategory.get(userSkinTone) as Map<string, number[]>).get(categoryName) || [];
Expand Down Expand Up @@ -320,8 +288,6 @@ export function createCategoryAndEmojiRows(
const emojis = getEmojisByCategory(
allEmojis,
categories[categoryName as EmojiCategory],
categoryName as EmojiCategory,
userSkinTone,
);

sortedEmojis = [...sortedEmojis, ...emojis];
Expand Down
2 changes: 1 addition & 1 deletion components/markdown/markdown.test.tsx
View file
Expand Up @@ -37,7 +37,7 @@ describe('components/Markdown', () => {
}),
hasImageProxy: false,
minimumHashtagLength: 3,
emojiMap: new EmojiMap([]),
emojiMap: new EmojiMap(new Map()),
metadata: {},
};

Expand Down
3 changes: 2 additions & 1 deletion components/post_view/post_info/index.ts
View file
Expand Up @@ -8,6 +8,7 @@ import {showActionsDropdownPulsatingDot} from 'selectors/actions_menu';
import {setActionsMenuInitialisationState} from 'mattermost-redux/actions/preferences';

import {DispatchFunc, GetStateFunc} from 'mattermost-redux/types/actions';
import {Emoji} from '@mattermost/types/emojis';
import {removePost, ExtendedPost} from 'mattermost-redux/actions/posts';
import {getCurrentTeamId} from 'mattermost-redux/selectors/entities/teams';
import {makeGetCommentCountForPost} from 'mattermost-redux/selectors/entities/posts';
Expand Down Expand Up @@ -60,7 +61,7 @@ function makeMapStateToProps() {
const shortcutReactToLastPostEmittedFrom = getShortcutReactToLastPostEmittedFrom(state);
const showActionsMenuPulsatingDot = showActionsDropdownPulsatingDot(state);

let emojis = [];
let emojis: Emoji[] = [];
const oneClickReactionsEnabled = get(state, Preferences.CATEGORY_DISPLAY_SETTINGS, Preferences.ONE_CLICK_REACTIONS_ENABLED, Preferences.ONE_CLICK_REACTIONS_ENABLED_DEFAULT) === 'true';
if (oneClickReactionsEnabled) {
emojis = getOneClickReactionEmojis(state);
Expand Down
3 changes: 2 additions & 1 deletion components/rhs_comment/index.ts
View file
Expand Up @@ -17,6 +17,7 @@ import {getCurrentTeamId} from 'mattermost-redux/selectors/entities/teams';
import {getUser} from 'mattermost-redux/selectors/entities/users';

import {GenericAction} from 'mattermost-redux/types/actions';
import {Emoji} from '@mattermost/types/emojis';
import {Post} from '@mattermost/types/posts';

import {markPostAsUnread, emitShortcutReactToLastPostFrom} from 'actions/post_actions';
Expand Down Expand Up @@ -65,7 +66,7 @@ function mapStateToProps(state: GlobalState, ownProps: OwnProps) {
const highlightedPostId = getHighlightedPostId(state);
const showActionsMenuPulsatingDot = showActionsDropdownPulsatingDot(state);

let emojis = [];
let emojis: Emoji[] = [];
const oneClickReactionsEnabled = get(state, Preferences.CATEGORY_DISPLAY_SETTINGS, Preferences.ONE_CLICK_REACTIONS_ENABLED, Preferences.ONE_CLICK_REACTIONS_ENABLED_DEFAULT) === 'true';
if (oneClickReactionsEnabled) {
emojis = getOneClickReactionEmojis(state);
Expand Down

0 comments on commit a041fc2

Please sign in to comment.