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.

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 full screen Group Chat custom avatars #41586

Merged
merged 38 commits into from
Jul 19, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
7ce2242
enable view photo for group chat
nexarvo May 3, 2024
b12d3ef
create variable for filename
nexarvo May 6, 2024
349ea43
Merge branch 'main' into feat/39850
nexarvo May 12, 2024
5b6a7c6
add support for new chat view photo
nexarvo May 17, 2024
66d8edc
revert removing extension
nexarvo May 20, 2024
7d122da
use file name for new group chat draft
nexarvo May 20, 2024
91f73c8
add newGroupChat query param for report avatar
nexarvo May 23, 2024
20294f6
clean code
nexarvo May 23, 2024
c9993e8
Merge branch 'main' into feat/39850
nexarvo May 24, 2024
e4ae8b4
remove shouldDisableViewPhoto param
nexarvo May 24, 2024
b26a166
add: conditional for route depending on isNewGroupChat
nexarvo May 24, 2024
1e29cab
add: parser to convert isNewGroupChat to boolean
nexarvo May 24, 2024
0b0da6c
update: made isNewGroupChat query param optional
nexarvo May 24, 2024
10eade0
refactor: clean code
nexarvo May 24, 2024
232e787
fix: lint issues
nexarvo May 24, 2024
8ff95be
fix: lint issues
nexarvo May 24, 2024
284d1f0
fix: minor logic issues
nexarvo May 27, 2024
3bf6bcf
fix: shouldShowNotFoundPage condition
nexarvo May 29, 2024
2992240
Merge branch 'main' into feat/39850
nexarvo May 29, 2024
a4315b7
Merge branch 'main' into feat/39850
nexarvo Jun 4, 2024
f6f582c
fix: handle edge cases for group chat name
nexarvo Jun 4, 2024
cf3f942
fix: report title edge cases
nexarvo Jun 6, 2024
b91c010
fix: changes WIP
nexarvo Jun 6, 2024
9c5b15e
Merge branch 'main' into feat/39850
nexarvo Jun 10, 2024
c0db9ef
fix: remove policy from not found
nexarvo Jun 10, 2024
6d6953f
fix: lint issues
nexarvo Jun 10, 2024
9d4a66f
add: originalFileName to report onyx
nexarvo Jun 12, 2024
38d7f5b
fix: null checks for title
nexarvo Jun 12, 2024
530df19
fix: lint issues
nexarvo Jun 12, 2024
bc18cf6
add: handle offline behaviour for view photo
nexarvo Jul 4, 2024
55b8487
Merge branch 'main' into feat/39850
nexarvo Jul 4, 2024
db65135
add: isNewGroupChat to types after merge main
nexarvo Jul 4, 2024
86fb5e7
fix: view photo when chat created offline(optimistic)
nexarvo Jul 7, 2024
12d1da7
fix: lint issues
nexarvo Jul 7, 2024
3695b6b
Merge branch 'main' into feat/39850
nexarvo Jul 17, 2024
16ac2ff
Merge branch 'main' into feat/39850
nexarvo Jul 17, 2024
f2e6e7b
making a single getAllReports call and merging main
nexarvo Jul 17, 2024
1c3e7db
verify parameters of buildOptimisticChatReport
nexarvo Jul 18, 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: 3 additions & 3 deletions src/pages/NewChatConfirmPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -117,17 +117,17 @@ function NewChatConfirmPage({newGroupDraft, allPersonalDetails}: NewChatConfirmP
source={stashedLocalAvatarImage ?? ReportUtils.getDefaultGroupAvatar(optimisticReportID.current)}
onImageSelected={(image) => {
fileRef.current = image;
Report.setGroupDraft({avatarUri: image?.uri ?? ''});
Report.setGroupDraft({avatarUri: image?.uri ?? '', originalFileName: image?.name});
}}
onImageRemoved={() => {
fileRef.current = undefined;
Report.setGroupDraft({avatarUri: null});
Report.setGroupDraft({avatarUri: null, originalFileName: null});
}}
size={CONST.AVATAR_SIZE.XLARGE}
avatarStyle={styles.avatarXLarge}
shouldDisableViewPhoto
nexarvo marked this conversation as resolved.
Show resolved Hide resolved
nexarvo marked this conversation as resolved.
Show resolved Hide resolved
editIcon={Expensicons.Camera}
editIconStyle={styles.smallEditIconAccount}
onViewPhotoPress={() => Navigation.navigate(ROUTES.REPORT_AVATAR.getRoute(optimisticReportID.current))}
/>
</View>
<MenuItemWithTopDescription
Expand Down
24 changes: 17 additions & 7 deletions src/pages/ReportAvatar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,33 +10,40 @@ import * as UserUtils from '@libs/UserUtils';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import type SCREENS from '@src/SCREENS';
import type {Policy, Report} from '@src/types/onyx';
import type {NewGroupChatDraft, Policy, Report} from '@src/types/onyx';

type ReportAvatarOnyxProps = {
report: OnyxEntry<Report>;
isLoadingApp: OnyxEntry<boolean>;
policies: OnyxCollection<Policy>;
groupChatDraft: OnyxEntry<NewGroupChatDraft>;
};

type ReportAvatarProps = ReportAvatarOnyxProps & StackScreenProps<AuthScreensParamList, typeof SCREENS.REPORT_AVATAR>;

function ReportAvatar({report = {} as Report, policies, isLoadingApp = true}: ReportAvatarProps) {
function ReportAvatar({report = {} as Report, policies, isLoadingApp = true, groupChatDraft}: ReportAvatarProps) {
const policy = policies?.[`${ONYXKEYS.COLLECTION.POLICY}${report?.policyID ?? '0'}`];
const policyName = ReportUtils.getPolicyName(report, false, policy);
const avatarURL = ReportUtils.getWorkspaceAvatar(report);
const title = policy ? ReportUtils.getPolicyName(report, false, policy) : report?.reportName;
let avatarURL = policy ? ReportUtils.getWorkspaceAvatar(report) : report?.avatarUrl;
let fileName = policy?.originalFileName ?? title;

if (!avatarURL && groupChatDraft?.avatarUri && groupChatDraft?.originalFileName) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This fallback logic will not work correctly in cases where we'd expect to see a not found page i.e. we will always fallback to the draft avatar. Can you instead add a query params that determines if we want to use the group chat draft?

avatarURL = groupChatDraft.avatarUri ?? null;
fileName = groupChatDraft.originalFileName ?? null;
}

return (
<AttachmentModal
headerTitle={policyName}
headerTitle={title}
defaultOpen
source={UserUtils.getFullSizeAvatar(avatarURL, 0)}
onModalClose={() => {
Navigation.goBack(ROUTES.REPORT_WITH_ID_DETAILS.getRoute(report?.reportID ?? ''));
}}
isWorkspaceAvatar
maybeIcon
originalFileName={policy?.originalFileName ?? policyName}
shouldShowNotFoundPage={!report?.reportID && !isLoadingApp}
originalFileName={fileName}
shouldShowNotFoundPage={!report?.reportID && !groupChatDraft && !isLoadingApp}
Copy link
Contributor

Choose a reason for hiding this comment

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

Related to the above ^. Since groupChatDraft will pretty much always be set we will not see the not found page in any case. Please update the logic accordingly

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 remove ! groupChatDraft from above condition then for groupChatDraft it always return not found page.

isLoading={(!report?.reportID || !policy?.id) && !!isLoadingApp}
/>
);
Expand All @@ -54,4 +61,7 @@ export default withOnyx<ReportAvatarProps, ReportAvatarOnyxProps>({
policies: {
key: ONYXKEYS.COLLECTION.POLICY,
},
groupChatDraft: {
key: ONYXKEYS.NEW_GROUP_CHAT_DRAFT,
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's make this use the useOnyx hook instead. It should help avoid loading the data from onyx if we are not going to use it. Related to the point above ^

})(ReportAvatar);
2 changes: 1 addition & 1 deletion src/pages/ReportDetailsPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,6 @@ function ReportDetailsPage({policies, report, session, personalDetails}: ReportD
isUsingDefaultAvatar={!report.avatarUrl}
size={CONST.AVATAR_SIZE.XLARGE}
avatarStyle={styles.avatarXLarge}
shouldDisableViewPhoto
onImageRemoved={() => {
// Calling this without a file will remove the avatar
Report.updateGroupChatAvatar(report.reportID ?? '');
Expand All @@ -250,6 +249,7 @@ function ReportDetailsPage({policies, report, session, personalDetails}: ReportD
errors={report.errorFields?.avatar ?? null}
errorRowStyles={styles.mt6}
onErrorClose={() => Report.clearAvatarErrors(report.reportID ?? '')}
onViewPhotoPress={() => Navigation.navigate(ROUTES.REPORT_AVATAR.getRoute(report.reportID))}
/>
) : (
<RoomHeaderAvatars
Expand Down
1 change: 1 addition & 0 deletions src/types/onyx/NewGroupChatDraft.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ type NewGroupChatDraft = {
participants: SelectedParticipant[];
reportName: string | null;
avatarUri: string | null;
originalFileName: string | null;
};
export type {SelectedParticipant};
export default NewGroupChatDraft;
Loading