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

Add support for viewing group avatar photo #48757

Merged
merged 14 commits into from
Sep 17, 2024
Merged

Add support for viewing group avatar photo #48757

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
1c93560
Add support for viewing group avatar photo
tsa321 Sep 7, 2024
755a304
Merge main and resolve conflict
tsa321 Sep 9, 2024
e2e2093
Fix some issues
tsa321 Sep 9, 2024
8e37922
Revert the use of interaction manager when navigate
tsa321 Sep 9, 2024
631b494
Use useOnyx instad of withOnyx
tsa321 Sep 10, 2024
58249b7
Revert some changes
tsa321 Sep 11, 2024
bcfb52a
Merge branch 'main' into groupAvatar
tsa321 Sep 11, 2024
e0ad3ff
Revert newgroupchatdraft onyx types
tsa321 Sep 11, 2024
f27a9d4
Fix typo
tsa321 Sep 11, 2024
0638ed5
Simplify code
tsa321 Sep 11, 2024
f9137e9
useOnyx for other onyx entries instead of withonyx
tsa321 Sep 16, 2024
d6020af
move comment
tsa321 Sep 16, 2024
6bc1eb1
Merge main and resolve conflict
tsa321 Sep 16, 2024
1dce9ac
Using fullsize avatar
tsa321 Sep 16, 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
6 changes: 6 additions & 0 deletions src/libs/ReportUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,7 @@ type OptimisticChatReport = Pick<
| 'description'
| 'writeCapability'
| 'avatarUrl'
| 'avatarFileName'
| 'invoiceReceiver'
| 'isHidden'
> & {
Expand Down Expand Up @@ -5060,6 +5061,7 @@ function buildOptimisticChatReport(
parentReportID = '',
description = '',
avatarUrl = '',
avatarFileName = '',
optimisticReportID = '',
): OptimisticChatReport {
const isWorkspaceChatType = chatType && isWorkspaceChat(chatType);
Expand Down Expand Up @@ -5100,6 +5102,7 @@ function buildOptimisticChatReport(
description,
writeCapability,
avatarUrl,
avatarFileName,
};

if (chatType === CONST.REPORT.CHAT_TYPE.INVOICE) {
Expand All @@ -5117,6 +5120,7 @@ function buildOptimisticGroupChatReport(
participantAccountIDs: number[],
reportName: string,
avatarUri: string,
avatarFilename: string,
optimisticReportID?: string,
notificationPreference?: NotificationPreference,
) {
Expand All @@ -5135,6 +5139,7 @@ function buildOptimisticGroupChatReport(
undefined,
undefined,
avatarUri,
avatarFilename,
optimisticReportID,
);
}
Expand Down Expand Up @@ -5544,6 +5549,7 @@ function buildOptimisticWorkspaceChats(policyID: string, policyName: string, exp
undefined,
undefined,
undefined,
undefined,
expenseReportId,
);
const expenseChatReportID = expenseChatData.reportID;
Expand Down
16 changes: 13 additions & 3 deletions src/libs/actions/Report.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -697,7 +697,8 @@ function updateGroupChatAvatar(reportID: string, file?: File | CustomRNImageMani
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`,
value: {
avatarUrl: file?.uri ?? '',
avatarUrl: file ? file?.uri ?? '' : null,
avatarFileName: file ? file?.name ?? '' : null,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
avatarFileName: file ? file?.name ?? '' : null,
avatarFileName: file?.name ?? '',

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we set avatarFileName to an empty string when file is undefined, the avatarFileName will remain an empty string. For example, when removing the avatar, the avatarFileName will stay as an empty string, but it should be set to null to effectively remove the avatarFileName.

To test this, go to the avatar picker’s popover menu and select "Remove Avatar." Then, check the Onyx value of avatarFileName. While the avatarUrl will be removed, the avatarFileName will remain an empty string.

Copy link
Contributor

Choose a reason for hiding this comment

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

While the avatarUrl will be removed, the avatarFileName will remain an empty string

How is that the url is being removed but the filename is not? given that both fallback to an empty string, or is it because the former is cleared from BE onyx response?

Either way, let's set both to null

pendingFields: {
avatar: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE,
},
Expand All @@ -708,12 +709,14 @@ function updateGroupChatAvatar(reportID: string, file?: File | CustomRNImageMani
},
];

const fetchedReport = ReportConnection.getAllReports()?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`];
const failureData: OnyxUpdate[] = [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`,
value: {
avatarUrl: ReportConnection.getAllReports()?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`]?.avatarUrl ?? null,
avatarUrl: fetchedReport?.avatarUrl ?? null,
avatarFileName: fetchedReport?.avatarFileName ?? null,
pendingFields: {
avatar: null,
},
Expand Down Expand Up @@ -1011,7 +1014,14 @@ function navigateToAndOpenReport(
if (isEmptyObject(chat)) {
if (isGroupChat) {
// If we are creating a group chat then participantAccountIDs is expected to contain currentUserAccountID
newChat = ReportUtils.buildOptimisticGroupChatReport(participantAccountIDs, reportName ?? '', avatarUri ?? '', optimisticReportID, CONST.REPORT.NOTIFICATION_PREFERENCE.ALWAYS);
newChat = ReportUtils.buildOptimisticGroupChatReport(
participantAccountIDs,
reportName ?? '',
avatarUri ?? '',
avatarFile?.name ?? '',
optimisticReportID,
CONST.REPORT.NOTIFICATION_PREFERENCE.ALWAYS,
);
} else {
newChat = ReportUtils.buildOptimisticChatReport(
[...participantAccountIDs, currentUserAccountID],
Expand Down
33 changes: 25 additions & 8 deletions src/pages/ReportAvatar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type {StackScreenProps} from '@react-navigation/stack';
import React from 'react';
import type {OnyxCollection, OnyxEntry} from 'react-native-onyx';
import React, {useMemo} from 'react';
import {withOnyx} from 'react-native-onyx';
import type {OnyxCollection, OnyxEntry} from 'react-native-onyx';
import AttachmentModal from '@components/AttachmentModal';
import Navigation from '@libs/Navigation/Navigation';
import type {AuthScreensParamList} from '@libs/Navigation/types';
Expand All @@ -23,21 +23,38 @@ type ReportAvatarProps = ReportAvatarOnyxProps & StackScreenProps<AuthScreensPar
function ReportAvatar({report = {} as Report, route, policies, isLoadingApp = true}: ReportAvatarProps) {
Copy link
Contributor

Choose a reason for hiding this comment

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

While at it, can you please migrate the withOnyx props to use the hook instead useOnyx

const policyID = route.params.policyID ?? '-1';
const policy = policies?.[`${ONYXKEYS.COLLECTION.POLICY}${policyID}`];
const policyName = ReportUtils.getPolicyName(report, false, policy);
const avatarURL = ReportUtils.getWorkspaceAvatar(report);

const attachment = useMemo(() => {
if (ReportUtils.isGroupChat(report) && !ReportUtils.isThread(report)) {
return {
source: report?.avatarUrl ? UserUtils.getFullSizeAvatar(report.avatarUrl, 0) : ReportUtils.getDefaultGroupAvatar(report?.reportID ?? ''),
Copy link
Contributor

Choose a reason for hiding this comment

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

report?.avatarUrl should be enough as you are nut supposed to be able to access this page if report does not have a custom avatar

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 can still access it using a URL, for example: dev.new.expensify.com:8082/r/12345678/avatar.

headerTitle: ReportUtils.getReportName(report),
originalFileName: report?.avatarFileName ?? '',
isWorkspaceAvatar: false,
};
}

return {
source: UserUtils.getFullSizeAvatar(ReportUtils.getWorkspaceAvatar(report), 0),
headerTitle: ReportUtils.getPolicyName(report, false, policy),
originalFileName: policy?.originalFileName ?? policy?.id ?? report?.policyID ?? '',
isWorkspaceAvatar: true,
};
// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
}, [report, policy]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we still need to disable eslint here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok


return (
<AttachmentModal
headerTitle={policyName}
headerTitle={attachment.headerTitle}
defaultOpen
source={UserUtils.getFullSizeAvatar(avatarURL, 0)}
source={attachment.source}
onModalClose={() => {
Navigation.goBack(ROUTES.REPORT_WITH_ID_DETAILS.getRoute(report?.reportID ?? '-1'));
}}
isWorkspaceAvatar
isWorkspaceAvatar={attachment.isWorkspaceAvatar}
maybeIcon
// In the case of default workspace avatar, originalFileName prop takes policyID as value to get the color of the avatar
Copy link
Contributor

Choose a reason for hiding this comment

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

Move this comment to where we set originalFileName for the workspace related reports

originalFileName={policy?.originalFileName ?? policy?.id ?? report?.policyID}
originalFileName={attachment.originalFileName}
shouldShowNotFoundPage={!report?.reportID && !isLoadingApp}
isLoading={(!report?.reportID || !policy?.id) && !!isLoadingApp}
/>
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 @@ -506,7 +506,7 @@ function ReportDetailsPage({policies, report, session, personalDetails}: ReportD
isUsingDefaultAvatar={!report.avatarUrl}
size={CONST.AVATAR_SIZE.XLARGE}
avatarStyle={styles.avatarXLarge}
shouldDisableViewPhoto
onViewPhotoPress={() => Navigation.navigate(ROUTES.REPORT_AVATAR.getRoute(report.reportID ?? '-1'))}
onImageRemoved={() => {
// Calling this without a file will remove the avatar
Report.updateGroupChatAvatar(report.reportID ?? '');
Expand Down
2 changes: 2 additions & 0 deletions src/pages/home/ReportScreen.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ function ReportScreen({route, currentReportID = '', navigation}: ReportScreenPro
isOptimisticReport: reportOnyx?.isOptimisticReport,
lastMentionedTime: reportOnyx?.lastMentionedTime,
avatarUrl: reportOnyx?.avatarUrl,
avatarFileName: reportOnyx?.avatarFileName,
permissions,
invoiceReceiver: reportOnyx?.invoiceReceiver,
policyAvatar: reportOnyx?.policyAvatar,
Expand Down Expand Up @@ -248,6 +249,7 @@ function ReportScreen({route, currentReportID = '', navigation}: ReportScreenPro
reportOnyx?.isOptimisticReport,
reportOnyx?.lastMentionedTime,
reportOnyx?.avatarUrl,
reportOnyx?.avatarFileName,
permissions,
reportOnyx?.invoiceReceiver,
reportOnyx?.policyAvatar,
Expand Down
3 changes: 3 additions & 0 deletions src/types/onyx/Report.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,9 @@ type Report = OnyxCommon.OnyxValueWithOfflineFeedback<
/** Collection of report permissions granted to the current user */
permissions?: Array<ValueOf<typeof CONST.REPORT.PERMISSIONS>>;

/** The filename of the avatar */
avatarFileName?: string;

Copy link
Contributor

Choose a reason for hiding this comment

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

NAB. Can you move this to be below avatarUrl

/** The trip data for a trip room */
tripData?: {
/** The start date of a trip */
Expand Down
Loading