From 7a80e183278f01889e5e00ca19d532a2d3231339 Mon Sep 17 00:00:00 2001
From: William Grant <willmgrant@gmail.com>
Date: Fri, 26 Aug 2022 14:39:57 +1000
Subject: [PATCH] fix: improved react popup localisation, review fixes
---
_locales/en/messages.json | 10 +++-
.../message/reactions/Reaction.tsx | 8 +--
.../message/reactions/ReactionPopup.tsx | 59 ++++++++++---------
ts/types/LocalizerKeys.ts | 10 +++-
ts/types/Reaction.ts | 2 +-
ts/util/emoji.ts | 2 +-
ts/util/reactions.ts | 8 +--
ts/util/readableList.ts | 45 --------------
8 files changed, 56 insertions(+), 88 deletions(-)
delete mode 100644 ts/util/readableList.ts
diff --git a/_locales/en/messages.json b/_locales/en/messages.json
index 9625e3ede..79e5a0c0c 100644
--- a/_locales/en/messages.json
+++ b/_locales/en/messages.json
@@ -454,9 +454,13 @@
"hideBanner": "Hide",
"openMessageRequestInboxDescription": "View your Message Request inbox",
"clearAllReactions": "Are you sure you want to clear all $emoji$ ?",
- "reactionTooltip": "reacted with",
"expandedReactionsText": "Show Less",
"reactionNotification": "Reacts to a message with $emoji$",
- "readableListCounterSingular": "other",
- "readableListCounterPlural": "others"
+ "otherSingular": "$number$ other",
+ "otherPlural": "$number$ others",
+ "reactionPopup": "reacted with",
+ "reactionPopupOne": "$name$",
+ "reactionPopupTwo": "$name$ & $name2$",
+ "reactionPopupThree": "$name$, $name2$ & $name3$",
+ "reactionPopupMany": "$name$, $name2$, $name3$ &"
}
diff --git a/ts/components/conversation/message/reactions/Reaction.tsx b/ts/components/conversation/message/reactions/Reaction.tsx
index 0ef46c532..483362ea6 100644
--- a/ts/components/conversation/message/reactions/Reaction.tsx
+++ b/ts/components/conversation/message/reactions/Reaction.tsx
@@ -69,8 +69,8 @@ export const Reaction = (props: ReactionProps): ReactElement => {
handlePopupClick,
} = props;
const reactionsMap = (reactions && Object.fromEntries(reactions)) || {};
- const senders = reactionsMap[emoji].senders || [];
- const count = reactionsMap[emoji].count;
+ const senders = reactionsMap[emoji]?.senders || [];
+ const count = reactionsMap[emoji]?.count;
const showCount = count !== undefined && (count > 1 || inGroup);
const reactionRef = useRef<HTMLDivElement>(null);
@@ -138,8 +138,8 @@ export const Reaction = (props: ReactionProps): ReactElement => {
<ReactionPopup
messageId={messageId}
emoji={popupReaction}
- count={reactionsMap[popupReaction].count}
- senders={reactionsMap[popupReaction].senders}
+ count={reactionsMap[popupReaction]?.count}
+ senders={reactionsMap[popupReaction]?.senders}
tooltipPosition={tooltipPosition}
onClick={() => {
if (handlePopupReaction) {
diff --git a/ts/components/conversation/message/reactions/ReactionPopup.tsx b/ts/components/conversation/message/reactions/ReactionPopup.tsx
index 9a694f670..7ac5235dc 100644
--- a/ts/components/conversation/message/reactions/ReactionPopup.tsx
+++ b/ts/components/conversation/message/reactions/ReactionPopup.tsx
@@ -3,7 +3,6 @@ import styled from 'styled-components';
import { Data } from '../../../../data/data';
import { PubKey } from '../../../../session/types/PubKey';
import { nativeEmojiData } from '../../../../util/emoji';
-import { defaultConjunction, defaultWordLimit, readableList } from '../../../../util/readableList';
export type TipPosition = 'center' | 'left' | 'right';
@@ -61,9 +60,8 @@ const StyledOthers = styled.span`
const generateContactsString = async (
messageId: string,
- senders: Array<string>,
- count: number
-): Promise<string | null> => {
+ senders: Array<string>
+): Promise<Array<string> | null> => {
let results = [];
const message = await Data.getMessageById(messageId);
if (message) {
@@ -80,38 +78,45 @@ const generateContactsString = async (
results = [window.i18n('you'), ...results];
}
if (results && results.length > 0) {
- let contacts = readableList(results);
- // This can only happen in an opengroup
- if (results.length !== count) {
- const [names, others] = contacts.split(`${defaultConjunction} `);
- const [_, suffix] = others.split(' '); // i.e. 3 others
- contacts = `${names} ${defaultConjunction} ${count - defaultWordLimit} ${suffix}`;
- }
- return contacts;
+ return results;
}
}
return null;
};
-const Contacts = (contacts: string) => {
- if (!contacts) {
+const Contacts = (contacts: Array<string>, count: number) => {
+ if (!Boolean(contacts?.length > 0)) {
return;
}
- if (contacts.includes(defaultConjunction) && contacts.includes('other')) {
- const [names, others] = contacts.split(defaultConjunction);
+ const reactors = contacts.length;
+ if (reactors === 1 || reactors === 2 || reactors === 3) {
+ return (
+ <span>
+ {window.i18n(
+ reactors === 1
+ ? 'reactionPopupOne'
+ : reactors === 2
+ ? 'reactionPopupTwo'
+ : 'reactionPopupThree',
+ contacts
+ )}{' '}
+ {window.i18n('reactionPopup')}
+ </span>
+ );
+ } else if (reactors > 3) {
return (
<span>
- {names} & <StyledOthers>{others}</StyledOthers> {window.i18n('reactionTooltip')}
+ {window.i18n('reactionPopupMany', [contacts[0], contacts[1], contacts[3]])}{' '}
+ <StyledOthers>
+ {window.i18n(reactors === 4 ? 'otherSingular' : 'otherPlural', [`${count - 3}`])}
+ </StyledOthers>{' '}
+ {window.i18n('reactionPopup')}
</span>
);
+ } else {
+ return null;
}
-
- return (
- <span>
- {contacts.replace(defaultConjunction, '&')} {window.i18n('reactionTooltip')}
- </span>
- );
};
type Props = {
@@ -126,11 +131,11 @@ type Props = {
export const ReactionPopup = (props: Props): ReactElement => {
const { messageId, emoji, count, senders, tooltipPosition = 'center', onClick } = props;
- const [contacts, setContacts] = useState('');
+ const [contacts, setContacts] = useState<Array<string>>([]);
useEffect(() => {
let isCancelled = false;
- generateContactsString(messageId, senders, count)
+ generateContactsString(messageId, senders)
.then(async results => {
if (isCancelled) {
return;
@@ -148,11 +153,11 @@ export const ReactionPopup = (props: Props): ReactElement => {
return () => {
isCancelled = true;
};
- }, [count, generateContactsString, messageId, senders]);
+ }, [count, messageId, senders]);
return (
<StyledPopupContainer tooltipPosition={tooltipPosition} onClick={onClick}>
- {Contacts(contacts)}
+ {Contacts(contacts, count)}
<StyledEmoji role={'img'} aria-label={nativeEmojiData?.ariaLabels?.[emoji]}>
{emoji}
</StyledEmoji>
diff --git a/ts/types/LocalizerKeys.ts b/ts/types/LocalizerKeys.ts
index 842f10e3e..03e4521b4 100644
--- a/ts/types/LocalizerKeys.ts
+++ b/ts/types/LocalizerKeys.ts
@@ -9,6 +9,7 @@ export type LocalizerKeys =
| 'unblocked'
| 'keepDisabled'
| 'userAddedToModerators'
+ | 'otherSingular'
| 'to'
| 'sent'
| 'requestsPlaceholder'
@@ -39,6 +40,7 @@ export type LocalizerKeys =
| 'autoUpdateLaterButtonLabel'
| 'maximumAttachments'
| 'deviceOnly'
+ | 'reactionPopupTwo'
| 'beginYourSession'
| 'typingIndicatorsSettingDescription'
| 'changePasswordToastDescription'
@@ -140,7 +142,6 @@ export type LocalizerKeys =
| 'banUser'
| 'answeredACall'
| 'sendMessage'
- | 'readableListCounterSingular'
| 'recoveryPhraseRevealMessage'
| 'showRecoveryPhrase'
| 'autoUpdateSettingDescription'
@@ -186,7 +187,6 @@ export type LocalizerKeys =
| 'nameAndMessage'
| 'autoUpdateDownloadedMessage'
| 'onionPathIndicatorTitle'
- | 'readableListCounterPlural'
| 'unknown'
| 'mediaMessage'
| 'addAsModerator'
@@ -253,6 +253,7 @@ export type LocalizerKeys =
| 'setPassword'
| 'editMenuDeleteContact'
| 'hideMenuBarTitle'
+ | 'reactionPopupOne'
| 'imageCaptionIconAlt'
| 'sendRecoveryPhraseTitle'
| 'multipleJoinedTheGroup'
@@ -268,6 +269,7 @@ export type LocalizerKeys =
| 'editMenuRedo'
| 'hideRequestBanner'
| 'changeNicknameMessage'
+ | 'reactionPopupThree'
| 'close'
| 'deleteMessageQuestion'
| 'newMessage'
@@ -296,6 +298,7 @@ export type LocalizerKeys =
| 'timerOption_6_hours_abbreviated'
| 'timerOption_1_week_abbreviated'
| 'timerSetTo'
+ | 'otherPlural'
| 'enable'
| 'notificationSubtitle'
| 'youChangedTheTimer'
@@ -307,6 +310,7 @@ export type LocalizerKeys =
| 'noNameOrMessage'
| 'pinConversationLimitTitle'
| 'noSearchResults'
+ | 'reactionPopup'
| 'changeNickname'
| 'userUnbanned'
| 'respondingToRequestWarning'
@@ -325,7 +329,6 @@ export type LocalizerKeys =
| 'media'
| 'noMembersInThisGroup'
| 'saveLogToDesktop'
- | 'reactionTooltip'
| 'copyErrorAndQuit'
| 'onlyAdminCanRemoveMembers'
| 'passwordTypeError'
@@ -433,6 +436,7 @@ export type LocalizerKeys =
| 'settingsHeader'
| 'autoUpdateNewVersionMessage'
| 'oneNonImageAtATimeToast'
+ | 'reactionPopupMany'
| 'removePasswordTitle'
| 'iAmSure'
| 'selectMessage'
diff --git a/ts/types/Reaction.ts b/ts/types/Reaction.ts
index 617999541..7357e54aa 100644
--- a/ts/types/Reaction.ts
+++ b/ts/types/Reaction.ts
@@ -124,7 +124,7 @@ export type ReactionList = Record<
count: number;
index: number; // relies on reactsIndex in the message model
senders: Array<string>;
- you?: boolean; // whether you are in the senders because sometimes we dont have the full list of senders yet.
+ you?: boolean; // whether we are in the senders because sometimes we dont have the full list of senders yet.
}
>;
diff --git a/ts/util/emoji.ts b/ts/util/emoji.ts
index 1ec255932..2345e34a1 100644
--- a/ts/util/emoji.ts
+++ b/ts/util/emoji.ts
@@ -76,7 +76,7 @@ export function initialiseEmojiData(data: any) {
}
// Synchronous version of Emoji Mart's SearchIndex.search()
-// If you upgrade the package things will probably break
+// If we upgrade the package things will probably break
export function searchSync(query: string, args?: any): Array<any> {
if (!nativeEmojiData) {
window.log.error('No native emoji data found');
diff --git a/ts/util/reactions.ts b/ts/util/reactions.ts
index 60e1c1e0e..fb0943c96 100644
--- a/ts/util/reactions.ts
+++ b/ts/util/reactions.ts
@@ -101,7 +101,7 @@ export const sendMessageReaction = async (messageId: string, emoji: string) => {
let action: Action = Action.REACT;
const reacts = found.get('reacts');
- if (reacts && Object.keys(reacts).includes(emoji) && reacts[emoji].senders.includes(me)) {
+ if (reacts?.[emoji]?.senders?.includes(me)) {
window.log.info('Found matching reaction removing it');
action = Action.REMOVE;
} else {
@@ -179,7 +179,7 @@ export const handleMessageReaction = async (
break;
case Action.REMOVE:
default:
- if (senders && senders.length > 0) {
+ if (senders?.length > 0) {
const sendersIndex = senders.indexOf(sender);
if (sendersIndex >= 0) {
details.senders.splice(sendersIndex, 1);
@@ -240,12 +240,12 @@ export const handleOpenGroupMessageReactions = async (
const you = reactions[key].you || false;
if (you) {
- if (reactions[key].reactors && reactions[key].reactors.length > 0) {
+ if (reactions[key]?.reactors.length > 0) {
const reactorsWithoutMe = reactions[key].reactors.filter(
reactor => !isUsAnySogsFromCache(reactor)
);
- // If you aren't included in the reactors then remove the extra reactor to match with the SOGSReactorsFetchCount.
+ // If we aren't included in the reactors then remove the extra reactor to match with the SOGSReactorsFetchCount.
if (reactorsWithoutMe.length === SOGSReactorsFetchCount) {
reactorsWithoutMe.pop();
}
diff --git a/ts/util/readableList.ts b/ts/util/readableList.ts
deleted file mode 100644
index 81d7549b6..000000000
--- a/ts/util/readableList.ts
+++ /dev/null
@@ -1,45 +0,0 @@
-// we need to use a character that cannot be used as a display name for string manipulation up until we render the UI
-export const defaultConjunction = '\uFFD7';
-export const defaultWordLimit = 3;
-
-export const readableList = (
- arr: Array<string>,
- conjunction: string = defaultConjunction,
- wordLimit: number = defaultWordLimit
-): string => {
- if (arr.length === 0) {
- return '';
- }
-
- const count = arr.length;
- switch (count) {
- case 1:
- return arr[0];
- default:
- let result = '';
- let others = 0;
- for (let i = 0; i < count; i++) {
- if (others === 0 && i === count - 1 && i < wordLimit) {
- result += ` ${conjunction} `;
- } else if (i !== 0 && i < wordLimit) {
- result += ', ';
- } else if (i >= wordLimit) {
- others++;
- }
-
- if (others === 0) {
- result += arr[i];
- }
- }
-
- if (others > 0) {
- result += ` ${conjunction} ${others} ${
- others > 1
- ? window.i18n('readableListCounterPlural')
- : window.i18n('readableListCounterSingular')
- }`;
- }
-
- return result;
- }
-};