Add fixes after review and always generate icons property

Expensify · Apr 16, 2024 · 6b3528a · 6b3528a
1 parent c8fd46b
commit 6b3528a
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 55 deletions.
diff --git a/src/components/Avatar.tsx b/src/components/Avatar.tsx
@@ -56,7 +56,7 @@ type AvatarProps = {
 };
 
 function Avatar({
-    source,
+    source: originalSource,
     imageStyles,
     iconAdditionalStyles,
     containerStyles,
@@ -77,20 +77,20 @@ function Avatar({
 
     useEffect(() => {
         setImageError(false);
-    }, [source]);
+    }, [originalSource]);
 
     const isWorkspace = type === CONST.ICON_TYPE_WORKSPACE;
-    const isEmptySource = !source;
 
     const iconSize = StyleUtils.getAvatarSize(size);
     const imageStyle: StyleProp<ImageStyle> = [StyleUtils.getAvatarStyle(size), imageStyles, styles.noBorderRadius];
     const iconStyle = imageStyles ? [StyleUtils.getAvatarStyle(size), styles.bgTransparent, imageStyles] : undefined;
 
     // We pass the color styles down to the SVG for the workspace and fallback avatar.
-    const useFallBackAvatar = imageError || isEmptySource || source === Expensicons.FallbackAvatar;
+    const source = isWorkspace ? originalSource : UserUtils.getAvatar(originalSource, accountID);
+    const useFallBackAvatar = imageError || !source || source === Expensicons.FallbackAvatar;
     const fallbackAvatar = isWorkspace ? ReportUtils.getDefaultWorkspaceAvatar(name) : fallbackIcon || Expensicons.FallbackAvatar;
     const fallbackAvatarTestID = isWorkspace ? ReportUtils.getDefaultWorkspaceAvatarTestID(name) : fallbackIconTestID || 'SvgFallbackAvatar Icon';
-    const avatarSource = useFallBackAvatar ? fallbackAvatar : UserUtils.getAvatar(source, accountID) ?? Expensicons.FallbackAvatar;
+    const avatarSource = useFallBackAvatar ? fallbackAvatar : source;
 
     let iconColors;
     if (isWorkspace) {

diff --git a/src/libs/OptionsListUtils.ts b/src/libs/OptionsListUtils.ts
@@ -305,23 +305,17 @@ function getAvatarsForAccountIDs(accountIDs: number[], personalDetails: OnyxEntr
         reversedDefaultValues[item[1]] = item[0];
     });
 
-    return accountIDs
-        .map<OnyxCommon.Icon | undefined>((accountID) => {
-            const login = reversedDefaultValues[accountID] ?? '';
-            const userPersonalDetail = personalDetails?.[accountID] ?? {login, accountID, avatar: ''};
+    return accountIDs.map((accountID) => {
+        const login = reversedDefaultValues[accountID] ?? '';
+        const userPersonalDetail = personalDetails?.[accountID] ?? {login, accountID, avatar: ''};
 
-            if (!userPersonalDetail.avatar) {
-                return;
-            }
-
-            return {
-                id: accountID,
-                source: userPersonalDetail.avatar,
-                type: CONST.ICON_TYPE_AVATAR,
-                name: userPersonalDetail.login ?? '',
-            };
-        })
-        .filter((icon): icon is OnyxCommon.Icon => icon !== undefined);
+        return {
+            id: accountID,
+            source: userPersonalDetail.avatar ?? FallbackAvatar,
+            type: CONST.ICON_TYPE_AVATAR,
+            name: userPersonalDetail.login ?? '',
+        };
+    });
 }
 
 /**
@@ -1941,7 +1935,7 @@ function getShareLogOptions(options: OptionList, searchValue = '', betas: Beta[]
  */
 function getIOUConfirmationOptionsFromPayeePersonalDetail(personalDetail: PersonalDetails | EmptyObject, amountText?: string): PayeePersonalDetails {
     const formattedLogin = LocalePhoneNumber.formatPhoneNumber(personalDetail.login ?? '');
-    const icons = personalDetail.avatar ? [{source: personalDetail.avatar, name: personalDetail.login ?? '', type: CONST.ICON_TYPE_AVATAR, id: personalDetail.accountID}] : [];
+    const icons = [{source: personalDetail.avatar ?? FallbackAvatar, name: personalDetail.login ?? '', type: CONST.ICON_TYPE_AVATAR, id: personalDetail.accountID}];
 
     return {
         text: PersonalDetailsUtils.getDisplayNameOrDefault(personalDetail, formattedLogin),

diff --git a/src/pages/home/report/ReactionList/BaseReactionList.tsx b/src/pages/home/report/ReactionList/BaseReactionList.tsx
@@ -3,6 +3,7 @@ import Str from 'expensify-common/lib/str';
 import React from 'react';
 import {FlatList} from 'react-native';
 import type {FlatListProps} from 'react-native';
+import {FallbackAvatar} from '@components/Icon/Expensicons';
 import OptionRow from '@components/OptionRow';
 import useThemeStyles from '@hooks/useThemeStyles';
 import useWindowDimensions from '@hooks/useWindowDimensions';
@@ -53,38 +54,33 @@ function BaseReactionList({hasUserReacted = false, users, isVisible = false, emo
      * so that the sticky headers function properly
      *
      */
-    const renderItem: FlatListProps<PersonalDetails>['renderItem'] = ({item}) => {
-        const icons = item.avatar
-            ? [
-                  {
-                      id: item.accountID,
-                      source: item.avatar,
-                      name: item.login ?? '',
-                      type: CONST.ICON_TYPE_AVATAR,
-                  },
-              ]
-            : [];
-        return (
-            <OptionRow
-                boldStyle
-                style={{maxWidth: variables.mobileResponsiveWidthBreakpoint}}
-                hoverStyle={hoveredComponentBG}
-                onSelectRow={() => {
-                    onClose?.();
+    const renderItem: FlatListProps<PersonalDetails>['renderItem'] = ({item}) => (
+        <OptionRow
+            boldStyle
+            style={{maxWidth: variables.mobileResponsiveWidthBreakpoint}}
+            hoverStyle={hoveredComponentBG}
+            onSelectRow={() => {
+                onClose?.();
 
-                    Navigation.navigate(ROUTES.PROFILE.getRoute(item.accountID));
-                }}
-                option={{
-                    reportID: String(item.accountID),
-                    text: Str.removeSMSDomain(item.displayName ?? ''),
-                    alternateText: Str.removeSMSDomain(item.login ?? ''),
-                    participantsList: [item],
-                    icons,
-                    keyForList: item.login ?? String(item.accountID),
-                }}
-            />
-        );
-    };
+                Navigation.navigate(ROUTES.PROFILE.getRoute(item.accountID));
+            }}
+            option={{
+                reportID: String(item.accountID),
+                text: Str.removeSMSDomain(item.displayName ?? ''),
+                alternateText: Str.removeSMSDomain(item.login ?? ''),
+                participantsList: [item],
+                icons: [
+                    {
+                        id: item.accountID,
+                        source: item.avatar ?? FallbackAvatar,
+                        name: item.login ?? '',
+                        type: CONST.ICON_TYPE_AVATAR,
+                    },
+                ],
+                keyForList: item.login ?? String(item.accountID),
+            }}
+        />
+    );
 
     return (
         <>

diff --git a/src/pages/home/report/ReportActionItemSingle.tsx b/src/pages/home/report/ReportActionItemSingle.tsx
@@ -101,7 +101,7 @@ function ReportActionItemSingle({
         actorHint = `${delegateDisplayName} (${translate('reportAction.asCopilot')} ${displayName})`;
         displayName = actorHint;
         avatarSource = delegateDetails?.avatar;
-        avatarAccountId = Number(action.delegateAccountID);
+        avatarAccountId = action.delegateAccountID;
     }
 
     // If this is a report preview, display names and avatars of both people involved
@@ -133,7 +133,7 @@ function ReportActionItemSingle({
         source: avatarSource ?? FallbackAvatar,
         type: isWorkspaceActor ? CONST.ICON_TYPE_WORKSPACE : CONST.ICON_TYPE_AVATAR,
         name: primaryDisplayName ?? '',
-        id: isWorkspaceActor ? undefined : avatarAccountId,
+        id: avatarAccountId,
     };
 
     // Since the display name for a report action message is delivered with the report history as an array of fragments