From 6be95b717769756579f6d1a1457f021aa3d4e2ae Mon Sep 17 00:00:00 2001 From: Audric Ackermann <audric@loki.network> Date: Fri, 7 Jul 2023 11:52:44 +0200 Subject: [PATCH 1/4] fix: notif settings in list item convoId from contextprovider --- ts/components/menu/Menu.tsx | 45 +++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 25 deletions(-) diff --git a/ts/components/menu/Menu.tsx b/ts/components/menu/Menu.tsx index d1010df93..695d0acf9 100644 --- a/ts/components/menu/Menu.tsx +++ b/ts/components/menu/Menu.tsx @@ -6,6 +6,7 @@ import { useAvatarPath, useConversationUsername, useHasNickname, + useIsActive, useIsBlinded, useIsBlocked, useIsIncomingRequest, @@ -15,6 +16,7 @@ import { useIsPrivate, useIsPrivateAndFriend, useIsPublic, + useNotificationSetting, useWeAreAdmin, } from '../../hooks/useParamSelector'; import { @@ -35,6 +37,10 @@ import { showUpdateGroupNameByConvoId, unblockConvoById, } from '../../interactions/conversationInteractions'; +import { + ConversationNotificationSetting, + ConversationNotificationSettingType, +} from '../../models/conversationAttributes'; import { getConversationController } from '../../session/conversations'; import { PubKey } from '../../session/types'; import { @@ -43,23 +49,10 @@ import { updateUserDetailsModal, } from '../../state/ducks/modalDialog'; import { getIsMessageSection } from '../../state/selectors/section'; -import { - useSelectedConversationKey, - useSelectedIsActive, - useSelectedIsBlocked, - useSelectedIsKickedFromGroup, - useSelectedIsLeft, - useSelectedIsPrivate, - useSelectedIsPrivateFriend, - useSelectedNotificationSetting, -} from '../../state/selectors/selectedConversation'; +import { useSelectedConversationKey } from '../../state/selectors/selectedConversation'; +import { LocalizerKeys } from '../../types/LocalizerKeys'; import { SessionButtonColor } from '../basic/SessionButton'; import { useConvoIdFromContext } from '../leftpane/conversation-list-item/ConvoIdContext'; -import { - ConversationNotificationSetting, - ConversationNotificationSettingType, -} from '../../models/conversationAttributes'; -import { LocalizerKeys } from '../../types/LocalizerKeys'; /** Menu items standardized */ @@ -556,18 +549,20 @@ export const DeclineAndBlockMsgRequestMenuItem = () => { }; export const NotificationForConvoMenuItem = (): JSX.Element | null => { - const selectedConvoId = useSelectedConversationKey(); + // Note: this item is used in the header and in the list item, so we need to grab the details + // from the convoId from the context itself, not the redux selected state + const convoId = useConvoIdFromContext(); - const currentNotificationSetting = useSelectedNotificationSetting(); - const isBlocked = useSelectedIsBlocked(); - const isActive = useSelectedIsActive(); - const isLeft = useSelectedIsLeft(); - const isKickedFromGroup = useSelectedIsKickedFromGroup(); - const isFriend = useSelectedIsPrivateFriend(); - const isPrivate = useSelectedIsPrivate(); + const currentNotificationSetting = useNotificationSetting(convoId); + const isBlocked = useIsBlocked(convoId); + const isActive = useIsActive(convoId); + const isLeft = useIsLeft(convoId); + const isKickedFromGroup = useIsKickedFromGroup(convoId); + const isFriend = useIsPrivateAndFriend(convoId); + const isPrivate = useIsPrivate(convoId); if ( - !selectedConvoId || + !convoId || isLeft || isKickedFromGroup || isBlocked || @@ -606,7 +601,7 @@ export const NotificationForConvoMenuItem = (): JSX.Element | null => { <Item key={item.value} onClick={async () => { - await setNotificationForConvoId(selectedConvoId, item.value); + await setNotificationForConvoId(convoId, item.value); }} disabled={disabled} > From c6d86d25d8ad4f66b94572a757230b2e45f47841 Mon Sep 17 00:00:00 2001 From: Audric Ackermann <audric@loki.network> Date: Fri, 7 Jul 2023 15:33:03 +0200 Subject: [PATCH 2/4] 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<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> ); 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 ( - <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> + ); + } +); 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<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; - }); -} 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<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()) || []; +} 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<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() { From 8c6f17fc334c67b82b436d1ae32025889cdc7bd7 Mon Sep 17 00:00:00 2001 From: Audric Ackermann <audric@loki.network> Date: Fri, 7 Jul 2023 15:40:03 +0200 Subject: [PATCH 3/4] fix: single sha512 of pubkey per pubkey for avatars --- .../AvatarPlaceHolder/AvatarPlaceHolder.tsx | 18 +++++--- .../AvatarPlaceHolder/ClosedGroupAvatar.tsx | 45 +++++++++---------- ts/hooks/useParamSelector.ts | 2 +- 3 files changed, 34 insertions(+), 31 deletions(-) diff --git a/ts/components/avatar/AvatarPlaceHolder/AvatarPlaceHolder.tsx b/ts/components/avatar/AvatarPlaceHolder/AvatarPlaceHolder.tsx index 826bbe962..42319aa57 100644 --- a/ts/components/avatar/AvatarPlaceHolder/AvatarPlaceHolder.tsx +++ b/ts/components/avatar/AvatarPlaceHolder/AvatarPlaceHolder.tsx @@ -1,6 +1,7 @@ import React, { useEffect, useState } from 'react'; import { COLORS } from '../../../themes/constants/colors'; import { getInitials } from '../../../util/getInitials'; +import { allowOnlyOneAtATime } from '../../../session/utils/Promise'; type Props = { diameter: number; @@ -8,13 +9,15 @@ type Props = { pubkey: string; }; -const sha512FromPubkey = async (pubkey: string): Promise<string> => { - const buf = await crypto.subtle.digest('SHA-512', new TextEncoder().encode(pubkey)); +const sha512FromPubkeyOneAtAtime = async (pubkey: string) => { + return allowOnlyOneAtATime(`sha512FromPubkey-${pubkey}`, async () => { + const buf = await crypto.subtle.digest('SHA-512', new TextEncoder().encode(pubkey)); - // tslint:disable: prefer-template restrict-plus-operands - return Array.prototype.map - .call(new Uint8Array(buf), (x: any) => ('00' + x.toString(16)).slice(-2)) - .join(''); + // tslint:disable: prefer-template restrict-plus-operands + return Array.prototype.map + .call(new Uint8Array(buf), (x: any) => ('00' + x.toString(16)).slice(-2)) + .join(''); + }); }; // do not do this on every avatar, just cache the values so we can reuse them across the app @@ -46,7 +49,8 @@ function useHashBasedOnPubkey(pubkey: string) { } return; } - void sha512FromPubkey(pubkey).then(sha => { + + void sha512FromPubkeyOneAtAtime(pubkey).then(sha => { if (isInProgress) { setIsLoading(false); // Generate the seed simulate the .hashCode as Java diff --git a/ts/components/avatar/AvatarPlaceHolder/ClosedGroupAvatar.tsx b/ts/components/avatar/AvatarPlaceHolder/ClosedGroupAvatar.tsx index 2a11366e9..0979b9528 100644 --- a/ts/components/avatar/AvatarPlaceHolder/ClosedGroupAvatar.tsx +++ b/ts/components/avatar/AvatarPlaceHolder/ClosedGroupAvatar.tsx @@ -45,7 +45,8 @@ function moveUsAtTheEnd(members: Array<string>, us: string) { 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 + // we render at most 2 avatars for closed groups + return { firstMember: usAtTheEndIfNeeded?.[0], secondMember: usAtTheEndIfNeeded?.[1] }; } function useGroupMembersAvatars(convoId: string | undefined) { @@ -60,26 +61,24 @@ function useGroupMembersAvatars(convoId: string | 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] || ''; +export const ClosedGroupAvatar = ({ + convoId, + size, + onAvatarClick, +}: { + size: number; + convoId: string; + onAvatarClick?: () => void; +}) => { + const memberAvatars = useGroupMembersAvatars(convoId); + const avatarsDiameter = getClosedGroupAvatarsSize(size); + const firstMemberId = memberAvatars?.firstMember || ''; + const secondMemberID = memberAvatars?.secondMember || ''; - return ( - <div className="module-avatar__icon-closed"> - <Avatar size={avatarsDiameter} pubkey={firstMemberId} onAvatarClick={onAvatarClick} /> - <Avatar size={avatarsDiameter} pubkey={secondMemberID} onAvatarClick={onAvatarClick} /> - </div> - ); - } -); + return ( + <div className="module-avatar__icon-closed"> + <Avatar size={avatarsDiameter} pubkey={firstMemberId} onAvatarClick={onAvatarClick} /> + <Avatar size={avatarsDiameter} pubkey={secondMemberID} onAvatarClick={onAvatarClick} /> + </div> + ); +}; diff --git a/ts/hooks/useParamSelector.ts b/ts/hooks/useParamSelector.ts index 60c3629e2..74ef78556 100644 --- a/ts/hooks/useParamSelector.ts +++ b/ts/hooks/useParamSelector.ts @@ -288,6 +288,6 @@ export function useSortedGroupMembers(convoId: string | undefined): Array<string if (!convoProps || convoProps.isPrivate || convoProps.isPublic) { return []; } - // we need to close it before being able to call sort on it + // we need to clone the array before being able to call sort() it return compact(convoProps.members?.slice()?.sort()) || []; } From 07616eb674315792f72dd4869b861e3a0ae943ad Mon Sep 17 00:00:00 2001 From: Audric Ackermann <audric@loki.network> Date: Mon, 10 Jul 2023 09:16:15 +0200 Subject: [PATCH 4/4] fix: avatar placeholder instead of grey circle when not enough members --- .../AvatarPlaceHolder/AvatarPlaceHolder.tsx | 19 +++---------------- .../icon/MemberAvatarPlaceHolder.tsx | 15 +++++++++++++++ 2 files changed, 18 insertions(+), 16 deletions(-) create mode 100644 ts/components/icon/MemberAvatarPlaceHolder.tsx diff --git a/ts/components/avatar/AvatarPlaceHolder/AvatarPlaceHolder.tsx b/ts/components/avatar/AvatarPlaceHolder/AvatarPlaceHolder.tsx index 42319aa57..b38780c89 100644 --- a/ts/components/avatar/AvatarPlaceHolder/AvatarPlaceHolder.tsx +++ b/ts/components/avatar/AvatarPlaceHolder/AvatarPlaceHolder.tsx @@ -2,6 +2,7 @@ import React, { useEffect, useState } from 'react'; import { COLORS } from '../../../themes/constants/colors'; import { getInitials } from '../../../util/getInitials'; import { allowOnlyOneAtATime } from '../../../session/utils/Promise'; +import { MemberAvatarPlaceHolder } from '../../icon/MemberAvatarPlaceHolder'; type Props = { diameter: number; @@ -83,22 +84,8 @@ export const AvatarPlaceHolder = (props: Props) => { const rWithoutBorder = diameterWithoutBorder / 2; if (loading || !hash) { - // return grey circle - return ( - <svg viewBox={viewBox}> - <g id="UrTavla"> - <circle - cx={r} - cy={r} - r={rWithoutBorder} - fill="#d2d2d3" - shapeRendering="geometricPrecision" - stroke={'var(--avatar-border-color)'} - strokeWidth="1" - /> - </g> - </svg> - ); + // return avatar placeholder circle + return <MemberAvatarPlaceHolder />; } const initials = getInitials(name); diff --git a/ts/components/icon/MemberAvatarPlaceHolder.tsx b/ts/components/icon/MemberAvatarPlaceHolder.tsx new file mode 100644 index 000000000..3ef483284 --- /dev/null +++ b/ts/components/icon/MemberAvatarPlaceHolder.tsx @@ -0,0 +1,15 @@ +import React from 'react'; +// tslint:disable: no-http-string + +export const MemberAvatarPlaceHolder = () => { + return ( + <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 26 26"> + <circle fill="var(--primary-color)" cx="13" cy="13" r="13" /> + <path + fill="var(--white-color)" + d="M18.9 19.1c-1.5-.9-3-.8-3-.8h-3.6c-2.3 0-3.3 0-4.2.3-.9.3-1.8.8-2.8 2-.5.7-.8 1.4-1 2C6.6 24.7 9.7 26 13 26c3.3 0 6.4-1.3 8.7-3.3-.5-1.6-1.5-2.8-2.8-3.6z" + /> + <ellipse cx="13" cy="10.8" fill="var(--white-color)" rx="5.6" ry="6.1" /> + </svg> + ); +};