fix: improved react popup localisation, review fixes

_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",
+  "otherSingular": "$number$ other",
-  "readableListCounterPlural": "others"
+  "otherPlural": "$number$ others",
+  "reactionPopup": "reacted with",
+  "reactionPopupOne": "$name$",
+  "reactionPopupTwo": "$name$ & $name2$",
+  "reactionPopupThree": "$name$, $name2$ & $name3$",
+  "reactionPopupMany": "$name$, $name2$, $name3$ &"
 }

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 senders = reactionsMap[emoji]?.senders || [];
-  const count = reactionsMap[emoji].count;
+  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}
+          count={reactionsMap[popupReaction]?.count}
-          senders={reactionsMap[popupReaction].senders}
+          senders={reactionsMap[popupReaction]?.senders}
           tooltipPosition={tooltipPosition}
           onClick={() => {
             if (handlePopupReaction) {

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>,
+  senders: Array<string>
-  count: number
+): Promise<Array<string> | null> => {
-): Promise<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);
+      return 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 null;
 };
 
-const Contacts = (contacts: string) => {
+const Contacts = (contacts: Array<string>, count: number) => {
-  if (!contacts) {
+  if (!Boolean(contacts?.length > 0)) {
     return;
   }
 
-  if (contacts.includes(defaultConjunction) && contacts.includes('other')) {
+  const reactors = contacts.length;
-    const [names, others] = contacts.split(defaultConjunction);
+  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>

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'

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.
   }
 >;
 

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');

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();
           }

ts/util/readableList.ts

@@ -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;
-  }
-};