Skip to content

Commit

Permalink
simplify avatar logic and remove unnecessary calls to getAvatar() fn
Browse files Browse the repository at this point in the history
  • Loading branch information
@Kicu
Kicu committed Apr 15, 2024
1 parent db9e01c commit 441d495
Show file tree
Hide file tree
Showing 23 changed files with 139 additions and 129 deletions.
12 changes: 9 additions & 3 deletions src/components/Avatar.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import useTheme from '@hooks/useTheme';
import useThemeStyles from '@hooks/useThemeStyles';
import * as ReportUtils from '@libs/ReportUtils';
import type {AvatarSource} from '@libs/UserUtils';
import * as UserUtils from '@libs/UserUtils';
import type {AvatarSizeName} from '@styles/utils';
import CONST from '@src/CONST';
import type {AvatarType} from '@src/types/onyx/OnyxCommon';
Expand Down Expand Up @@ -49,6 +50,9 @@ type AvatarProps = {

/** Owner of the avatar. If user, displayName. If workspace, policy name */
name?: string;

/** Optional account id if it's user avatar */
accountID?: number;
};

function Avatar({
Expand All @@ -62,6 +66,7 @@ function Avatar({
fallbackIconTestID = '',
type = CONST.ICON_TYPE_AVATAR,
name = '',
accountID,
}: AvatarProps) {
const theme = useTheme();
const styles = useThemeStyles();
Expand All @@ -75,16 +80,17 @@ function Avatar({
}, [source]);

const isWorkspace = type === CONST.ICON_TYPE_WORKSPACE;
const iconSize = StyleUtils.getAvatarSize(size);
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 || source === Expensicons.FallbackAvatar || !source;
const useFallBackAvatar = imageError || isEmptySource || 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 : source;
const avatarSource = useFallBackAvatar ? fallbackAvatar : UserUtils.getAvatar(source, accountID) ?? Expensicons.FallbackAvatar;

let iconColors;
if (isWorkspace) {
Expand Down
7 changes: 5 additions & 2 deletions src/components/AvatarWithIndicator.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ type AvatarWithIndicatorProps = {
/** URL for the avatar */
source: UserUtils.AvatarSource;

/** account id if it's user avatar */
accountID?: number;

/** To show a tooltip on hover */
tooltipText?: string;

Expand All @@ -23,7 +26,7 @@ type AvatarWithIndicatorProps = {
isLoading?: boolean;
};

function AvatarWithIndicator({source, tooltipText = '', fallbackIcon = Expensicons.FallbackAvatar, isLoading = true}: AvatarWithIndicatorProps) {
function AvatarWithIndicator({source, accountID, tooltipText = '', fallbackIcon = Expensicons.FallbackAvatar, isLoading = true}: AvatarWithIndicatorProps) {
const styles = useThemeStyles();

return (
Expand All @@ -35,7 +38,7 @@ function AvatarWithIndicator({source, tooltipText = '', fallbackIcon = Expensico
<>
<Avatar
size={CONST.AVATAR_SIZE.SMALL}
source={UserUtils.getSmallSizeAvatar(source)}
source={UserUtils.getSmallSizeAvatar(source, accountID)}
fallbackIcon={fallbackIcon}
/>
<Indicator />
Expand Down
2 changes: 2 additions & 0 deletions src/components/MultipleAvatars.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ function MultipleAvatars({
name={icons[0].name}
type={icons[0].type}
fallbackIcon={icons[0].fallbackIcon}
accountID={icons[0].id}
/>
</View>
</UserDetailsTooltip>
Expand Down Expand Up @@ -207,6 +208,7 @@ function MultipleAvatars({
name={icon.name}
type={icon.type}
fallbackIcon={icon.fallbackIcon}
accountID={icon.id}
/>
</View>
</UserDetailsTooltip>
Expand Down
2 changes: 1 addition & 1 deletion src/components/ReportActionItem/TaskView.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ function TaskView({report, shouldShowHorizontalRule, ...props}: TaskViewProps) {
<MenuItem
label={translate('task.assignee')}
title={ReportUtils.getDisplayNameForParticipant(report.managerID)}
icon={OptionsListUtils.getAvatarsForAccountIDs(report.managerID ? [report.managerID] : [], personalDetails)}
icon={OptionsListUtils.getAvatarsForAccountIDs([report.managerID], personalDetails)}
iconType={CONST.ICON_TYPE_AVATAR}
avatarSize={CONST.AVATAR_SIZE.SMALLER}
titleStyle={styles.assigneeTextStyle}
Expand Down
4 changes: 2 additions & 2 deletions src/components/UserDetailsTooltip/BaseUserDetailsTooltip/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import useLocalize from '@hooks/useLocalize';
import useThemeStyles from '@hooks/useThemeStyles';
import * as LocalePhoneNumber from '@libs/LocalePhoneNumber';
import * as ReportUtils from '@libs/ReportUtils';
import * as UserUtils from '@libs/UserUtils';
import CONST from '@src/CONST';

function BaseUserDetailsTooltip({accountID, fallbackUserDetails, icon, delegateAccountID, shiftHorizontal, children}: UserDetailsTooltipProps) {
Expand Down Expand Up @@ -55,10 +54,11 @@ function BaseUserDetailsTooltip({accountID, fallbackUserDetails, icon, delegateA
<View style={styles.emptyAvatar}>
<Avatar
containerStyles={[styles.actionAvatar]}
source={icon?.source ?? UserUtils.getAvatar(userAvatar, userAccountID)}
source={icon?.source ?? userAvatar}
type={icon?.type ?? CONST.ICON_TYPE_AVATAR}
name={icon?.name ?? userLogin}
fallbackIcon={icon?.fallbackIcon}
accountID={userAccountID}
/>
</View>
<Text style={[styles.mt2, styles.textMicroBold, styles.textReactionSenders, styles.textAlignCenter]}>{title}</Text>
Expand Down
59 changes: 27 additions & 32 deletions src/libs/OptionsListUtils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import lodashSet from 'lodash/set';
import lodashSortBy from 'lodash/sortBy';
import Onyx from 'react-native-onyx';
import type {OnyxCollection, OnyxEntry} from 'react-native-onyx';
import {FallbackAvatar} from '@components/Icon/Expensicons';
import type {SelectedTagOption} from '@components/TagPicker';
import CONST from '@src/CONST';
import type {TranslationPaths} from '@src/languages/types';
Expand Down Expand Up @@ -303,17 +304,24 @@ function getAvatarsForAccountIDs(accountIDs: number[], personalDetails: OnyxEntr
Object.entries(defaultValues).forEach((item) => {
reversedDefaultValues[item[1]] = item[0];
});
return accountIDs.map((accountID) => {
const login = reversedDefaultValues[accountID] ?? '';
const userPersonalDetail = personalDetails?.[accountID] ?? {login, accountID, avatar: ''};

return {
id: accountID,
source: UserUtils.getAvatar(userPersonalDetail.avatar, userPersonalDetail.accountID),
type: CONST.ICON_TYPE_AVATAR,
name: userPersonalDetail.login ?? '',
};
});
return accountIDs
.map<OnyxCommon.Icon | undefined>((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);
}

/**
Expand All @@ -332,9 +340,7 @@ function getPersonalDetailsForAccountIDs(accountIDs: number[] | undefined, perso
}
let personalDetail: OnyxEntry<PersonalDetails> = personalDetails[accountID];
if (!personalDetail) {
personalDetail = {
avatar: UserUtils.getDefaultAvatar(cleanAccountID),
} as PersonalDetails;
personalDetail = {} as PersonalDetails;
}

if (cleanAccountID === CONST.ACCOUNT_ID.CONCIERGE) {
Expand Down Expand Up @@ -363,6 +369,7 @@ function getParticipantsOption(participant: ReportUtils.OptionData | Participant
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
const login = detail?.login || participant.login || '';
const displayName = PersonalDetailsUtils.getDisplayNameOrDefault(detail, LocalePhoneNumber.formatPhoneNumber(login));

return {
keyForList: String(detail?.accountID),
login,
Expand All @@ -373,7 +380,7 @@ function getParticipantsOption(participant: ReportUtils.OptionData | Participant
alternateText: LocalePhoneNumber.formatPhoneNumber(login) || displayName,
icons: [
{
source: UserUtils.getAvatar(detail?.avatar ?? '', detail?.accountID ?? -1),
source: detail?.avatar ?? FallbackAvatar,
name: login,
type: CONST.ICON_TYPE_AVATAR,
id: detail?.accountID,
Expand Down Expand Up @@ -757,13 +764,7 @@ function createOption(
// Disabling this line for safeness as nullish coalescing works only if the value is undefined or null
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
result.searchText = getSearchText(report, reportName, personalDetailList, !!result.isChatRoom || !!result.isPolicyExpenseChat, !!result.isThread);
result.icons = ReportUtils.getIcons(
report,
personalDetails,
UserUtils.getAvatar(personalDetail?.avatar ?? '', personalDetail?.accountID),
personalDetail?.login,
personalDetail?.accountID,
);
result.icons = ReportUtils.getIcons(report, personalDetails, personalDetail?.avatar, personalDetail?.login, personalDetail?.accountID);
result.subtitle = subtitle;

return result;
Expand Down Expand Up @@ -1851,7 +1852,6 @@ function getOptions(
[optimisticAccountID]: {
accountID: optimisticAccountID,
login: searchValue,
avatar: UserUtils.getDefaultAvatar(optimisticAccountID),
},
};
userToInvite = createOption([optimisticAccountID], personalDetailsExtended, null, reportActions, {
Expand All @@ -1864,10 +1864,10 @@ function getOptions(
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
userToInvite.alternateText = userToInvite.alternateText || searchValue;

// If user doesn't exist, use a default avatar
// If user doesn't exist, use a fallback avatar
userToInvite.icons = [
{
source: UserUtils.getAvatar('', optimisticAccountID),
source: FallbackAvatar,
name: searchValue,
type: CONST.ICON_TYPE_AVATAR,
},
Expand Down Expand Up @@ -1941,17 +1941,12 @@ 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}] : [];

return {
text: PersonalDetailsUtils.getDisplayNameOrDefault(personalDetail, formattedLogin),
alternateText: formattedLogin || PersonalDetailsUtils.getDisplayNameOrDefault(personalDetail, '', false),
icons: [
{
source: UserUtils.getAvatar(personalDetail.avatar, personalDetail.accountID),
name: personalDetail.login ?? '',
type: CONST.ICON_TYPE_AVATAR,
id: personalDetail.accountID,
},
],
icons,
descriptiveText: amountText ?? '',
login: personalDetail.login ?? '',
accountID: personalDetail.accountID,
Expand Down
29 changes: 15 additions & 14 deletions src/libs/ReportUtils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import Onyx from 'react-native-onyx';
import type {ValueOf} from 'type-fest';
import type {FileObject} from '@components/AttachmentModal';
import * as Expensicons from '@components/Icon/Expensicons';
import {FallbackAvatar} from '@components/Icon/Expensicons';
import * as defaultGroupAvatars from '@components/Icon/GroupDefaultAvatars';
import * as defaultWorkspaceAvatars from '@components/Icon/WorkspaceDefaultAvatars';
import CONST from '@src/CONST';
Expand Down Expand Up @@ -1629,7 +1630,7 @@ function getIconsForParticipants(participants: number[], personalDetails: OnyxCo
const participantsList = participants || [];

for (const accountID of participantsList) {
const avatarSource = UserUtils.getAvatar(personalDetails?.[accountID]?.avatar ?? '', accountID);
const avatarSource = personalDetails?.[accountID]?.avatar ?? FallbackAvatar;
const displayNameLogin = personalDetails?.[accountID]?.displayName ? personalDetails?.[accountID]?.displayName : personalDetails?.[accountID]?.login;
participantDetails.push([accountID, displayNameLogin ?? '', avatarSource, personalDetails?.[accountID]?.fallbackIcon ?? '']);
}
Expand Down Expand Up @@ -1690,12 +1691,12 @@ function getPersonalDetailsForAccountID(accountID: number): Partial<PersonalDeta
if (!accountID) {
return {};
}
return (
allPersonalDetails?.[accountID] ?? {
avatar: UserUtils.getDefaultAvatar(accountID),
isOptimisticPersonalDetail: true,
}
);

const defaultDetails = {
isOptimisticPersonalDetail: true,
};

return allPersonalDetails?.[accountID] ?? defaultDetails;
}

/**
Expand Down Expand Up @@ -1828,7 +1829,7 @@ function getIcons(
const parentReportAction = ReportActionsUtils.getParentReportAction(report);
const workspaceIcon = getWorkspaceIcon(report, policy);
const memberIcon = {
source: UserUtils.getAvatar(personalDetails?.[parentReportAction.actorAccountID ?? -1]?.avatar ?? '', parentReportAction.actorAccountID ?? -1),
source: personalDetails?.[parentReportAction.actorAccountID ?? -1]?.avatar ?? FallbackAvatar,
id: parentReportAction.actorAccountID,
type: CONST.ICON_TYPE_AVATAR,
name: personalDetails?.[parentReportAction.actorAccountID ?? -1]?.displayName ?? '',
Expand All @@ -1844,7 +1845,7 @@ function getIcons(
const actorDisplayName = PersonalDetailsUtils.getDisplayNameOrDefault(allPersonalDetails?.[actorAccountID ?? -1], '', false);
const actorIcon = {
id: actorAccountID,
source: UserUtils.getAvatar(personalDetails?.[actorAccountID ?? -1]?.avatar ?? '', actorAccountID ?? -1),
source: personalDetails?.[actorAccountID ?? -1]?.avatar ?? FallbackAvatar,
name: actorDisplayName,
type: CONST.ICON_TYPE_AVATAR,
fallbackIcon: personalDetails?.[parentReportAction.actorAccountID ?? -1]?.fallbackIcon,
Expand All @@ -1859,7 +1860,7 @@ function getIcons(
if (isTaskReport(report)) {
const ownerIcon = {
id: report?.ownerAccountID,
source: UserUtils.getAvatar(personalDetails?.[report?.ownerAccountID ?? -1]?.avatar ?? '', report?.ownerAccountID ?? -1),
source: personalDetails?.[report?.ownerAccountID ?? -1]?.avatar ?? FallbackAvatar,
type: CONST.ICON_TYPE_AVATAR,
name: personalDetails?.[report?.ownerAccountID ?? -1]?.displayName ?? '',
fallbackIcon: personalDetails?.[report?.ownerAccountID ?? -1]?.fallbackIcon,
Expand Down Expand Up @@ -1891,7 +1892,7 @@ function getIcons(
if (isPolicyExpenseChat(report) || isExpenseReport(report)) {
const workspaceIcon = getWorkspaceIcon(report, policy);
const memberIcon = {
source: UserUtils.getAvatar(personalDetails?.[report?.ownerAccountID ?? -1]?.avatar ?? '', report?.ownerAccountID ?? -1),
source: personalDetails?.[report?.ownerAccountID ?? -1]?.avatar ?? FallbackAvatar,
id: report?.ownerAccountID,
type: CONST.ICON_TYPE_AVATAR,
name: personalDetails?.[report?.ownerAccountID ?? -1]?.displayName ?? '',
Expand All @@ -1901,15 +1902,15 @@ function getIcons(
}
if (isIOUReport(report)) {
const managerIcon = {
source: UserUtils.getAvatar(personalDetails?.[report?.managerID ?? -1]?.avatar ?? '', report?.managerID ?? -1),
source: personalDetails?.[report?.managerID ?? -1]?.avatar ?? FallbackAvatar,
id: report?.managerID,
type: CONST.ICON_TYPE_AVATAR,
name: personalDetails?.[report?.managerID ?? -1]?.displayName ?? '',
fallbackIcon: personalDetails?.[report?.managerID ?? -1]?.fallbackIcon,
};
const ownerIcon = {
id: report?.ownerAccountID,
source: UserUtils.getAvatar(personalDetails?.[report?.ownerAccountID ?? -1]?.avatar ?? '', report?.ownerAccountID ?? -1),
source: personalDetails?.[report?.ownerAccountID ?? -1]?.avatar ?? FallbackAvatar,
type: CONST.ICON_TYPE_AVATAR,
name: personalDetails?.[report?.ownerAccountID ?? -1]?.displayName ?? '',
fallbackIcon: personalDetails?.[report?.ownerAccountID ?? -1]?.fallbackIcon,
Expand Down Expand Up @@ -1955,7 +1956,7 @@ function getDisplayNamesWithTooltips(
const accountID = Number(user?.accountID);
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
const displayName = getDisplayNameForParticipant(accountID, isMultipleParticipantReport, shouldFallbackToHidden, shouldAddCurrentUserPostfix) || user?.login || '';
const avatar = UserUtils.getDefaultAvatar(accountID);
const avatar = user && 'avatar' in user ? user.avatar : FallbackAvatar;

let pronouns = user?.pronouns ?? undefined;
if (pronouns?.startsWith(CONST.PRONOUNS.PREFIX)) {
Expand Down
3 changes: 1 addition & 2 deletions src/libs/SidebarUtils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import * as OptionsListUtils from './OptionsListUtils';
import * as ReportActionsUtils from './ReportActionsUtils';
import * as ReportUtils from './ReportUtils';
import * as TaskUtils from './TaskUtils';
import * as UserUtils from './UserUtils';

const reportActionsByReport: OnyxCollection<ReportActions> = {};
const visibleReportActionItems: ReportActions = {};
Expand Down Expand Up @@ -414,7 +413,7 @@ function getOptionData({
result.subtitle = subtitle;
result.participantsList = participantPersonalDetailList;

result.icons = ReportUtils.getIcons(report, personalDetails, UserUtils.getAvatar(personalDetail?.avatar ?? {}, personalDetail?.accountID), '', -1, policy);
result.icons = ReportUtils.getIcons(report, personalDetails, personalDetail?.avatar, '', -1, policy);
result.searchText = OptionsListUtils.getSearchText(report, reportName, participantPersonalDetailList, result.isChatRoom || result.isPolicyExpenseChat, result.isThread);
result.displayNamesWithTooltips = displayNamesWithTooltips;

Expand Down
Loading

0 comments on commit 441d495

Please sign in to comment.