From c6d86d25d8ad4f66b94572a757230b2e45f47841 Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Fri, 7 Jul 2023 15:33:03 +0200 Subject: [PATCH] fix: cleanup closed group avatar logic --- ts/components/avatar/Avatar.tsx | 52 +++++++----- .../AvatarPlaceHolder/ClosedGroupAvatar.tsx | 83 ++++++++++++++----- ts/hooks/useMembersAvatars.tsx | 40 --------- ts/hooks/useParamSelector.ts | 15 +++- ts/state/selectors/selectedConversation.ts | 4 +- 5 files changed, 110 insertions(+), 84 deletions(-) delete mode 100644 ts/hooks/useMembersAvatars.tsx diff --git a/ts/components/avatar/Avatar.tsx b/ts/components/avatar/Avatar.tsx index e85ee2a89..c5f1a7dff 100644 --- a/ts/components/avatar/Avatar.tsx +++ b/ts/components/avatar/Avatar.tsx @@ -66,21 +66,21 @@ export const CrownIcon = () => { ); }; -const NoImage = ( - props: Pick & { - 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 ( - - ); +const NoImage = React.memo( + ( + props: Pick & { + 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 ; + } + + return ; } - - return ; -}; +); const AvatarImage = ( props: Pick & { @@ -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 (
{ dataTestId={dataTestId ? `img-${dataTestId}` : undefined} /> ) : ( - + )}
); diff --git a/ts/components/avatar/AvatarPlaceHolder/ClosedGroupAvatar.tsx b/ts/components/avatar/AvatarPlaceHolder/ClosedGroupAvatar.tsx index ba7525f07..2a11366e9 100644 --- a/ts/components/avatar/AvatarPlaceHolder/ClosedGroupAvatar.tsx +++ b/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 ( -
- - -
- ); -}; +/** + * 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, 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, 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 ( +
+ + +
+ ); + } +); diff --git a/ts/hooks/useMembersAvatars.tsx b/ts/hooks/useMembersAvatars.tsx deleted file mode 100644 index 4daa649d5..000000000 --- a/ts/hooks/useMembersAvatars.tsx +++ /dev/null @@ -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 | 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; - }); -} diff --git a/ts/hooks/useParamSelector.ts b/ts/hooks/useParamSelector.ts index a46bc4776..60c3629e2 100644 --- a/ts/hooks/useParamSelector.ts +++ b/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 { + 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()) || []; +} diff --git a/ts/state/selectors/selectedConversation.ts b/ts/state/selectors/selectedConversation.ts index 31523151f..de5563aec 100644 --- a/ts/state/selectors/selectedConversation.ts +++ b/ts/state/selectors/selectedConversation.ts @@ -110,7 +110,7 @@ export const isClosedGroupConversation = (state: StateType): boolean => { ); }; -const getGroupMembers = (state: StateType): Array => { +const getSelectedGroupMembers = (state: StateType): Array => { 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() {