fix: cleanup closed group avatar logic

ts/components/avatar/Avatar.tsx

@@ -66,21 +66,21 @@ export const CrownIcon = () => {
   );
 };
 
-const NoImage = (
-  props: Pick<Props, 'forcedName' | 'size' | 'pubkey' | 'onAvatarClick'> & {
-    isClosedGroup: boolean;
-  }
-) => {
-  const { forcedName, size, pubkey, isClosedGroup } = props;
-  // if no image but we have conversations set for the group, renders group members avatars
-  if (pubkey && isClosedGroup) {
-    return (
-      <ClosedGroupAvatar size={size} closedGroupId={pubkey} onAvatarClick={props.onAvatarClick} />
-    );
+const NoImage = React.memo(
+  (
+    props: Pick<Props, 'forcedName' | 'size' | 'pubkey' | 'onAvatarClick'> & {
+      isClosedGroup: boolean;
+    }
+  ) => {
+    const { forcedName, size, pubkey, isClosedGroup, onAvatarClick } = props;
+    // if no image but we have conversations set for the group, renders group members avatars
+    if (pubkey && isClosedGroup) {
+      return <ClosedGroupAvatar size={size} convoId={pubkey} onAvatarClick={onAvatarClick} />;
+    }
+
+    return <Identicon size={size} forcedName={forcedName} pubkey={pubkey} />;
   }
-
-  return <Identicon size={size} forcedName={forcedName} pubkey={pubkey} />;
-};
+);
 
 const AvatarImage = (
   props: Pick<Props, 'base64Data' | 'dataTestId'> & {
@@ -111,12 +111,20 @@ const AvatarImage = (
 };
 
 const AvatarInner = (props: Props) => {
-  const { base64Data, size, pubkey, forcedAvatarPath, forcedName, dataTestId } = props;
+  const {
+    base64Data,
+    size,
+    pubkey,
+    forcedAvatarPath,
+    forcedName,
+    dataTestId,
+    onAvatarClick,
+  } = props;
   const [imageBroken, setImageBroken] = useState(false);
 
   const isSelectingMessages = useSelector(isMessageSelectionMode);
 
-  const isClosedGroupAvatar = useIsClosedGroup(pubkey);
+  const isClosedGroup = useIsClosedGroup(pubkey);
   const avatarPath = useAvatarPath(pubkey);
   const name = useConversationUsername(pubkey);
   // contentType is not important
@@ -130,9 +138,9 @@ const AvatarInner = (props: Props) => {
     setImageBroken(true);
   };
 
-  const hasImage = (base64Data || urlToLoad) && !imageBroken && !isClosedGroupAvatar;
+  const hasImage = (base64Data || urlToLoad) && !imageBroken && !isClosedGroup;
 
-  const isClickable = !!props.onAvatarClick;
+  const isClickable = !!onAvatarClick;
   return (
     <div
       className={classNames(
@@ -167,7 +175,13 @@ const AvatarInner = (props: Props) => {
           dataTestId={dataTestId ? `img-${dataTestId}` : undefined}
         />
       ) : (
-        <NoImage {...props} isClosedGroup={isClosedGroupAvatar} />
+        <NoImage
+          pubkey={pubkey}
+          isClosedGroup={isClosedGroup}
+          size={size}
+          forcedName={forcedName}
+          onAvatarClick={onAvatarClick}
+        />
       )}
     </div>
   );

ts/components/avatar/AvatarPlaceHolder/ClosedGroupAvatar.tsx

@@ -1,13 +1,9 @@
 import React from 'react';
-import { useMembersAvatars } from '../../../hooks/useMembersAvatars';
 import { assertUnreachable } from '../../../types/sqlSharedTypes';
 import { Avatar, AvatarSize } from '../Avatar';
-
-type Props = {
-  size: number;
-  closedGroupId: string;
-  onAvatarClick?: () => void;
-};
+import { isEmpty } from 'lodash';
+import { useIsClosedGroup, useSortedGroupMembers } from '../../../hooks/useParamSelector';
+import { UserUtils } from '../../../session/utils';
 
 function getClosedGroupAvatarsSize(size: AvatarSize): AvatarSize {
   // Always use the size directly under the one requested
@@ -29,18 +25,61 @@ function getClosedGroupAvatarsSize(size: AvatarSize): AvatarSize {
   }
 }
 
-export const ClosedGroupAvatar = (props: Props) => {
-  const { closedGroupId, size, onAvatarClick } = props;
-
-  const memberAvatars = useMembersAvatars(closedGroupId);
-  const avatarsDiameter = getClosedGroupAvatarsSize(size);
-  const firstMemberId = memberAvatars?.[0];
-  const secondMemberID = memberAvatars?.[1];
-
-  return (
-    <div className="module-avatar__icon-closed">
-      <Avatar size={avatarsDiameter} pubkey={firstMemberId || ''} onAvatarClick={onAvatarClick} />
-      <Avatar size={avatarsDiameter} pubkey={secondMemberID || ''} onAvatarClick={onAvatarClick} />
-    </div>
-  );
-};
+/**
+ * Move our pubkey at the end of the list if we are in the list of members.
+ * We do this, as we want to
+ * - show 2 other members when there are enough of them,
+ * - show us as the 2nd member when there are only 2 members
+ * - show us first with a grey avatar as second when there are only us in the group.
+ */
+function moveUsAtTheEnd(members: Array<string>, us: string) {
+  const usAt = members.findIndex(val => val === us);
+  if (us && usAt > -1) {
+    // we need to move us at the end of the array
+    const updated = members.filter(m => m !== us);
+    updated.push(us);
+    return updated;
+  }
+  return members;
+}
+
+function sortAndSlice(sortedMembers: Array<string>, us: string) {
+  const usAtTheEndIfNeeded = moveUsAtTheEnd(sortedMembers, us); // make sure we are not one of the first 2 members if there is enough members
+  return usAtTheEndIfNeeded.slice(0, 2); // we render at most 2 avatars for closed groups
+}
+
+function useGroupMembersAvatars(convoId: string | undefined) {
+  const us = UserUtils.getOurPubKeyStrFromCache();
+  const isClosedGroup = useIsClosedGroup(convoId);
+  const sortedMembers = useSortedGroupMembers(convoId);
+
+  if (!convoId || !isClosedGroup || isEmpty(sortedMembers)) {
+    return undefined;
+  }
+
+  return sortAndSlice(sortedMembers, us);
+}
+
+export const ClosedGroupAvatar = React.memo(
+  ({
+    convoId,
+    size,
+    onAvatarClick,
+  }: {
+    size: number;
+    convoId: string;
+    onAvatarClick?: () => void;
+  }) => {
+    const memberAvatars = useGroupMembersAvatars(convoId);
+    const avatarsDiameter = getClosedGroupAvatarsSize(size);
+    const firstMemberId = memberAvatars?.[0] || '';
+    const secondMemberID = memberAvatars?.[1] || '';
+
+    return (
+      <div className="module-avatar__icon-closed">
+        <Avatar size={avatarsDiameter} pubkey={firstMemberId} onAvatarClick={onAvatarClick} />
+        <Avatar size={avatarsDiameter} pubkey={secondMemberID} onAvatarClick={onAvatarClick} />
+      </div>
+    );
+  }
+);

ts/hooks/useMembersAvatars.tsx

@@ -1,40 +0,0 @@
-import { UserUtils } from '../session/utils';
-import * as _ from 'lodash';
-import { useSelector } from 'react-redux';
-import { StateType } from '../state/reducer';
-
-export function useMembersAvatars(closedGroupPubkey: string | undefined) {
-  const ourPrimary = UserUtils.getOurPubKeyStrFromCache();
-
-  return useSelector((state: StateType): Array<string> | undefined => {
-    if (!closedGroupPubkey) {
-      return undefined;
-    }
-    const groupConvo = state.conversations.conversationLookup[closedGroupPubkey];
-
-    if (groupConvo.isPrivate || groupConvo.isPublic) {
-      return undefined;
-    }
-    // this must be a closed group
-    const originalMembers = _.cloneDeep(groupConvo.members);
-    if (!originalMembers || originalMembers.length === 0) {
-      return undefined;
-    }
-    const allMembersSorted = originalMembers.sort((a, b) => (a < b ? -1 : a > b ? 1 : 0));
-
-    // no need to forward more than 2 conversations for rendering the group avatar
-    const usAtTheEndMaxTwo = _.sortBy(allMembersSorted, a => (a === ourPrimary ? 1 : 0)).slice(
-      0,
-      2
-    );
-    const memberConvos = _.compact(
-      usAtTheEndMaxTwo
-        .map(m => state.conversations.conversationLookup[m])
-        .map(m => {
-          return m?.id || undefined;
-        })
-    );
-
-    return memberConvos && memberConvos.length ? memberConvos : undefined;
-  });
-}

ts/hooks/useParamSelector.ts

@@ -1,4 +1,4 @@
-import { isEmpty, isNumber } from 'lodash';
+import { compact, isEmpty, isNumber } from 'lodash';
 import { useSelector } from 'react-redux';
 import {
   hasValidIncomingRequestValues,
@@ -278,3 +278,16 @@ export function useQuoteAuthorName(
 
   return { authorName, isMe };
 }
+
+/**
+ * Get the list of members of a closed group or []
+ * @param convoId the closed group id to extract members from
+ */
+export function useSortedGroupMembers(convoId: string | undefined): Array<string> {
+  const convoProps = useConversationPropsById(convoId);
+  if (!convoProps || convoProps.isPrivate || convoProps.isPublic) {
+    return [];
+  }
+  // we need to close it before being able to call sort on it
+  return compact(convoProps.members?.slice()?.sort()) || [];
+}

ts/state/selectors/selectedConversation.ts

@@ -110,7 +110,7 @@ export const isClosedGroupConversation = (state: StateType): boolean => {
   );
 };
 
-const getGroupMembers = (state: StateType): Array<string> => {
+const getSelectedGroupMembers = (state: StateType): Array<string> => {
   const selected = getSelectedConversation(state);
   if (!selected) {
     return [];
@@ -190,7 +190,7 @@ export function useSelectedisNoteToSelf() {
 }
 
 export function useSelectedMembers() {
-  return useSelector(getGroupMembers);
+  return useSelector(getSelectedGroupMembers);
 }
 
 export function useSelectedSubscriberCount() {