Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use fallback user avatar in cases where the user is unknown to us #41846

Merged
merged 23 commits into from
May 28, 2024
Merged

Use fallback user avatar in cases where the user is unknown to us #41846

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
1839a03
simplify avatar logic and use FallbackAvatar when user is unknown
Kicu May 7, 2024
a8879a1
fix not found Page when opening ProfilePage from workspace members li…
Kicu May 8, 2024
00ca93d
Remove more unnecessary usages of .getAvatar() function
Kicu May 8, 2024
7214e1a
Apply some fixes after review
Kicu May 10, 2024
8b3ac5d
Make accountID optional in AvatarWithImagePicker
Kicu May 10, 2024
f886a6b
Add updates after review
Kicu May 13, 2024
1b09f2e
Merge branch 'main' into kicu/38743-fallback-avatar-fix
Kicu May 13, 2024
9091250
Fix typo after merge conflict
Kicu May 13, 2024
4120f36
Fix lint
Kicu May 14, 2024
ee05310
Remove task assignee avatar workaround
Kicu May 14, 2024
42605f5
Merge branch 'main' into kicu/38743-fallback-avatar-fix
Kicu May 14, 2024
a71d08b
Fix bug after merge conflicts
Kicu May 14, 2024
b549d46
Remove instance of unneeded FallbackAvatar
Kicu May 14, 2024
5f355ae
Update tests for `getDisplayNamesWithTooltips`
Kicu May 14, 2024
456d62f
Merge branch 'main' into kicu/38743-fallback-avatar-fix
Kicu May 16, 2024
0b8a92d
Update Avatar code after workspace avatar styling changes
Kicu May 16, 2024
83f1b5f
Merge branch 'main' into kicu/38743-fallback-avatar-fix
Kicu May 16, 2024
ff2dab5
Merge branch 'main' into kicu/38743-fallback-avatar-fix
Kicu May 17, 2024
d9844a6
Fix wrong WorkspaceAvatar colors on workspace profile page
Kicu May 17, 2024
21c9e86
Rename avatarId variable
Kicu May 20, 2024
0319ead
Merge branch 'main' into kicu/38743-fallback-avatar-fix
Kicu May 23, 2024
b87b586
Add minor comment fixes in avatars
Kicu May 23, 2024
0daa461
Merge branch 'main' into kicu/38743-fallback-avatar-fix
Kicu May 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/components/Avatar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ function Avatar({
const isWorkspace = type === CONST.ICON_TYPE_WORKSPACE;

// if it's user avatar then accountID will be a number
mountiny marked this conversation as resolved.
Show resolved Hide resolved
const source = isWorkspace ? originalSource : UserUtils.getAvatar(originalSource, accountID as number);
const source = isWorkspace ? originalSource : UserUtils.getAvatar(originalSource, avatarID as number);
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';
Expand Down
6 changes: 3 additions & 3 deletions src/components/AvatarWithImagePicker.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ type AvatarWithImagePickerProps = {
source?: AvatarSource;

/** Account id of user for which avatar is displayed */
accountID?: number | string;
avatarID?: number | string;

/** Additional style props */
style?: StyleProp<ViewStyle>;
Expand Down Expand Up @@ -139,7 +139,7 @@ function AvatarWithImagePicker({
errorRowStyles,
onErrorClose = () => {},
source = '',
accountID,
avatarID,
fallbackIcon = Expensicons.FallbackAvatar,
size = CONST.AVATAR_SIZE.DEFAULT,
type = CONST.ICON_TYPE_AVATAR,
Expand Down Expand Up @@ -333,7 +333,7 @@ function AvatarWithImagePicker({
containerStyles={avatarStyle}
imageStyles={[avatarStyle, styles.alignSelfCenter]}
source={source}
accountID={accountID}
avatarID={avatarID}
fallbackIcon={fallbackIcon}
size={size}
type={type}
Expand Down
2 changes: 1 addition & 1 deletion src/components/AvatarWithIndicator.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ function AvatarWithIndicator({source, accountID, tooltipText = '', fallbackIcon
size={CONST.AVATAR_SIZE.SMALL}
source={UserUtils.getSmallSizeAvatar(source, accountID)}
fallbackIcon={fallbackIcon}
accountID={accountID}
avatarID={accountID}
/>
<Indicator />
</>
Expand Down
7 changes: 3 additions & 4 deletions src/components/MenuItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ type MenuItemBaseProps = {
onFocus?: () => void;

/** Optional account id if it's user avatar or policy id if it's workspace avatar */
accountID?: number | string;
avatarID?: number | string;
};

type MenuItemProps = (IconProps | AvatarProps | NoIcon) & MenuItemBaseProps;
Expand Down Expand Up @@ -345,7 +345,7 @@ function MenuItem(
isPaneMenu = false,
shouldPutLeftPaddingWhenNoIcon = false,
onFocus,
accountID,
avatarID,
}: MenuItemProps,
ref: PressableRef,
) {
Expand Down Expand Up @@ -528,7 +528,6 @@ function MenuItem(
imageStyles={[styles.alignSelfCenter]}
size={CONST.AVATAR_SIZE.DEFAULT}
source={icon}
accountID={accountID}
fallbackIcon={fallbackIcon}
name={title}
avatarID={avatarID}
Expand All @@ -539,7 +538,7 @@ function MenuItem(
<Avatar
imageStyles={[styles.alignSelfCenter]}
source={icon}
accountID={accountID}
avatarID={avatarID}
fallbackIcon={fallbackIcon}
size={avatarSize}
/>
Expand Down
3 changes: 1 addition & 2 deletions src/components/UserDetailsTooltip/BaseUserDetailsTooltip/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,10 @@ function BaseUserDetailsTooltip({accountID, fallbackUserDetails, icon, delegateA
<Avatar
containerStyles={[styles.actionAvatar]}
source={icon?.source ?? userAvatar}
accountID={userAccountID}
avatarID={icon?.id ?? userAccountID}
type={icon?.type ?? CONST.ICON_TYPE_AVATAR}
name={icon?.name ?? userLogin}
fallbackIcon={icon?.fallbackIcon}
avatarID={icon?.id}
/>
</View>
<Text style={[styles.mt2, styles.textMicroBold, styles.textReactionSenders, styles.textAlignCenter]}>{title}</Text>
Expand Down
4 changes: 2 additions & 2 deletions src/libs/actions/IOU.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1706,8 +1706,8 @@ function getSendInvoiceInformation(
const receiverLogin = receiverParticipant && 'login' in receiverParticipant && receiverParticipant.login ? receiverParticipant.login : '';
receiver = {
accountID: receiverAccountID,
displayName: LocalePhoneNumber.formatPhoneNumber(receiverParticipant?.login ?? ''),
login: receiverParticipant?.login,
displayName: LocalePhoneNumber.formatPhoneNumber(receiverLogin),
login: receiverLogin,
isOptimisticPersonalDetail: true,
};

Expand Down
2 changes: 1 addition & 1 deletion src/pages/DetailsPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ function DetailsPage({personalDetails, route, session}: DetailsPageProps) {
containerStyles={[styles.avatarLarge, styles.mb3]}
imageStyles={[styles.avatarLarge]}
source={details?.avatar}
accountID={details?.accountID}
avatarID={details?.accountID}
size={CONST.AVATAR_SIZE.LARGE}
fallbackIcon={details?.fallbackIcon}
/>
Expand Down
2 changes: 1 addition & 1 deletion src/pages/ProfilePage.tsx
View file
Open in desktop
Copy link
Contributor

@s77rt s77rt May 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes here seem unrelated. Can you please double check

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're asking about ProfilePage? So the changes are deliberate, and related to fixing #40989.

Basically right now (on main) the decision to show most of the content is based on this line:

const hasMinimumDetails = !isEmptyObject(details.avatar);

(https://github.com/Expensify/App/blob/main/src/pages/ProfilePage.tsx#L154)

But we know that there will be no avatar in case we're offline and rely on optimistic data - as I removed the default avatar from optimistic data almost everywhere.
All the other basic data is there, so I just allow everything to display and the Fallback avatar will be correctly shown.
I also disable clicking on the avatar when it's the fallback displayed

All the other changes are just indentation moved 1 level up. I tested this both in offline and online and it behaves correct I believe.

Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ function ProfilePage({route}: ProfilePageProps) {
containerStyles={[styles.avatarXLarge, styles.mb3]}
imageStyles={[styles.avatarXLarge]}
source={details.avatar}
accountID={accountID}
avatarID={accountID}
size={CONST.AVATAR_SIZE.XLARGE}
fallbackIcon={fallbackIcon}
/>
Expand Down
2 changes: 1 addition & 1 deletion src/pages/ReportDetailsPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ function ReportDetailsPage({policies, report, session, personalDetails}: ReportD
isGroupChat && !isThread ? (
<AvatarWithImagePicker
source={icons[0].source}
accountID={icons[0].id}
avatarID={icons[0].id}
isUsingDefaultAvatar={!report.avatarUrl}
size={CONST.AVATAR_SIZE.XLARGE}
avatarStyle={styles.avatarXLarge}
Expand Down
2 changes: 1 addition & 1 deletion src/pages/ReportParticipantDetailsPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ function ReportParticipantDetails({personalDetails, report, route}: ReportPartic
containerStyles={[styles.avatarXLarge, styles.mv5, styles.noOutline]}
imageStyles={[styles.avatarXLarge]}
source={details.avatar}
accountID={accountID}
avatarID={accountID}
size={CONST.AVATAR_SIZE.XLARGE}
fallbackIcon={fallbackIcon}
/>
Expand Down
2 changes: 1 addition & 1 deletion src/pages/RoomMembersPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ function RoomMembersPage({report, session, policies}: RoomMembersPageProps) {
source: details.avatar ?? FallbackAvatar,
name: details.login ?? '',
type: CONST.ICON_TYPE_AVATAR,
id: Number(accountID),
id: accountID,
},
],
pendingAction: pendingChatMember?.pendingAction,
Expand Down
2 changes: 1 addition & 1 deletion src/pages/home/report/ReportActionItemSingle.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ function ReportActionItemSingle({
source: avatarSource ?? FallbackAvatar,
type: isWorkspaceActor ? CONST.ICON_TYPE_WORKSPACE : CONST.ICON_TYPE_AVATAR,
name: primaryDisplayName ?? '',
id: avatarAccountId,
id: isWorkspaceActor ? report.policyID : avatarAccountId,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NAB. avatarAccountId already takes into account the isWorkspaceActor condition. Change the variable name to avatarId to avoid confusion

};

// Since the display name for a report action message is delivered with the report history as an array of fragments
Expand Down
2 changes: 1 addition & 1 deletion src/pages/settings/InitialSettingsPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ function InitialSettingsPage({session, userWallet, bankAccountList, fundList, wa
<AvatarWithImagePicker
isUsingDefaultAvatar={UserUtils.isDefaultAvatar(currentUserDetails?.avatar ?? '')}
source={avatarURL}
accountID={accountID}
avatarID={accountID}
onImageSelected={PersonalDetails.updateAvatar}
onImageRemoved={PersonalDetails.deleteAvatar}
size={CONST.AVATAR_SIZE.XLARGE}
Expand Down
2 changes: 1 addition & 1 deletion src/pages/workspace/WorkspaceAvatar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ function WorkspaceAvatar({policy, isLoadingApp = true}: WorkspaceAvatarProps) {
source={UserUtils.getFullSizeAvatar(avatarURL, 0)}
onModalClose={Navigation.goBack}
isWorkspaceAvatar
originalFileName={policy?.originalFileName ?? policy?.name ?? ''}
originalFileName={policy?.originalFileName ?? policy?.id}
shouldShowNotFoundPage={!Object.keys(policy ?? {}).length && !isLoadingApp}
isLoading={!Object.keys(policy ?? {}).length && !!isLoadingApp}
maybeIcon
Expand Down
2 changes: 1 addition & 1 deletion src/pages/workspace/WorkspaceProfilePage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ function WorkspaceProfilePage({policy, currencyList = {}, route}: WorkSpaceProfi
fallbackIcon={Expensicons.FallbackWorkspaceAvatar}
size={CONST.AVATAR_SIZE.XLARGE}
name={policyName}
avatarID={policy?.id ?? ''}
avatarID={policy?.id}
type={CONST.ICON_TYPE_WORKSPACE}
/>
),
Expand Down
2 changes: 1 addition & 1 deletion src/pages/workspace/WorkspacesListRow.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ function WorkspacesListRow({
<>
<Avatar
source={ownerDetails.avatar}
accountID={ownerDetails.accountID}
avatarID={ownerDetails.accountID}
size={CONST.AVATAR_SIZE.SMALL}
containerStyles={styles.workspaceOwnerAvatarWrapper}
/>
Expand Down
2 changes: 1 addition & 1 deletion src/pages/workspace/members/WorkspaceMemberDetailsPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ function WorkspaceMemberDetailsPage({personalDetails, policy, route}: WorkspaceM
containerStyles={[styles.avatarXLarge, styles.mv5, styles.noOutline]}
imageStyles={[styles.avatarXLarge]}
source={details.avatar}
accountID={accountID}
avatarID={accountID}
size={CONST.AVATAR_SIZE.XLARGE}
fallbackIcon={fallbackIcon}
/>
Expand Down
Loading