-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Move Leave button into a row of the Report Details page #41823
Changes from 10 commits
f245c2f
43334c4
746ad28
ca4e3ad
d1814c1
7c630c8
0aa0fb9
564b82f
6c53db0
8a9c211
f6202cc
6bdf55c
37e90c7
7d6fe50
1c8f9d3
2436d71
ec49893
bf1dac5
0b0126e
ab5a203
25a6964
d09d4d7
19c22bf
9f957f5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
import {useRoute} from '@react-navigation/native'; | ||
import type {StackScreenProps} from '@react-navigation/stack'; | ||
import React, {useEffect, useMemo} from 'react'; | ||
import React, {useCallback, useEffect, useMemo, useState} from 'react'; | ||
import {View} from 'react-native'; | ||
import type {OnyxCollection, OnyxEntry} from 'react-native-onyx'; | ||
import {withOnyx} from 'react-native-onyx'; | ||
|
@@ -29,6 +29,7 @@ import * as OptionsListUtils from '@libs/OptionsListUtils'; | |
import * as PolicyUtils from '@libs/PolicyUtils'; | ||
import * as ReportUtils from '@libs/ReportUtils'; | ||
import * as Report from '@userActions/Report'; | ||
import ConfirmModal from '@src/components/ConfirmModal'; | ||
import CONST from '@src/CONST'; | ||
import type {TranslationPaths} from '@src/languages/types'; | ||
import ONYXKEYS from '@src/ONYXKEYS'; | ||
|
@@ -49,6 +50,7 @@ type ReportDetailsPageMenuItem = { | |
action: () => void; | ||
brickRoadIndicator?: ValueOf<typeof CONST.BRICK_ROAD_INDICATOR_STATUS>; | ||
subtitle?: number; | ||
shouldShowRightIcon?: boolean; | ||
}; | ||
|
||
type ReportDetailsPageOnyxProps = { | ||
|
@@ -65,10 +67,12 @@ function ReportDetailsPage({policies, report, session, personalDetails}: ReportD | |
const {isOffline} = useNetwork(); | ||
const styles = useThemeStyles(); | ||
const route = useRoute(); | ||
const [isLastMemberLeavingGroupModalVisible, setIsLastMemberLeavingGroupModalVisible] = useState(false); | ||
const policy = useMemo(() => policies?.[`${ONYXKEYS.COLLECTION.POLICY}${report?.policyID ?? ''}`], [policies, report?.policyID]); | ||
const isPolicyAdmin = useMemo(() => PolicyUtils.isPolicyAdmin(policy ?? null), [policy]); | ||
const isPolicyEmployee = useMemo(() => PolicyUtils.isPolicyEmployee(report?.policyID ?? '', policies), [report?.policyID, policies]); | ||
const shouldUseFullTitle = useMemo(() => ReportUtils.shouldUseFullTitleToDisplay(report), [report]); | ||
const isPolicyExpenseChat = useMemo(() => ReportUtils.isPolicyExpenseChat(report), [report]); | ||
const isChatRoom = useMemo(() => ReportUtils.isChatRoom(report), [report]); | ||
const isUserCreatedPolicyRoom = useMemo(() => ReportUtils.isUserCreatedPolicyRoom(report), [report]); | ||
const isDefaultRoom = useMemo(() => ReportUtils.isDefaultRoom(report), [report]); | ||
|
@@ -113,23 +117,22 @@ function ReportDetailsPage({policies, report, session, personalDetails}: ReportD | |
Report.getReportPrivateNote(report?.reportID ?? ''); | ||
}, [report?.reportID, isOffline, isPrivateNotesFetchTriggered, isSelfDM]); | ||
|
||
const leaveChat = useCallback(() => { | ||
if (isChatRoom) { | ||
const isWorkspaceMemberLeavingWorkspaceRoom = (report.visibility === CONST.REPORT.VISIBILITY.RESTRICTED || isPolicyExpenseChat) && isPolicyEmployee; | ||
Report.leaveRoom(report.reportID, isWorkspaceMemberLeavingWorkspaceRoom); | ||
return; | ||
} | ||
Report.leaveGroupChat(report.reportID); | ||
}, [isChatRoom, isPolicyEmployee, isPolicyExpenseChat, report.reportID, report.visibility]); | ||
|
||
const menuItems: ReportDetailsPageMenuItem[] = useMemo(() => { | ||
const items: ReportDetailsPageMenuItem[] = []; | ||
|
||
if (isSelfDM) { | ||
return []; | ||
} | ||
|
||
if (!isGroupDMChat) { | ||
items.push({ | ||
key: CONST.REPORT_DETAILS_MENU_ITEM.SHARE_CODE, | ||
translationKey: 'common.shareCode', | ||
icon: Expensicons.QrCode, | ||
isAnonymousAction: true, | ||
action: () => Navigation.navigate(ROUTES.REPORT_WITH_ID_DETAILS_SHARE_CODE.getRoute(report?.reportID ?? '')), | ||
}); | ||
} | ||
|
||
if (isArchivedRoom) { | ||
return items; | ||
} | ||
|
@@ -151,6 +154,7 @@ function ReportDetailsPage({policies, report, session, personalDetails}: ReportD | |
icon: Expensicons.Users, | ||
subtitle: activeChatMembers.length, | ||
isAnonymousAction: false, | ||
shouldShowRightIcon: true, | ||
action: () => { | ||
if (isUserCreatedPolicyRoom || isChatThread) { | ||
Navigation.navigate(ROUTES.ROOM_MEMBERS.getRoute(report?.reportID ?? '')); | ||
|
@@ -159,15 +163,13 @@ function ReportDetailsPage({policies, report, session, personalDetails}: ReportD | |
} | ||
}, | ||
}); | ||
} else if ( | ||
(isUserCreatedPolicyRoom && (!participants.length || !isPolicyEmployee)) || | ||
((isDefaultRoom || ReportUtils.isPolicyExpenseChat(report)) && isChatThread && !isPolicyEmployee) | ||
) { | ||
} else if ((isUserCreatedPolicyRoom && (!participants.length || !isPolicyEmployee)) || ((isDefaultRoom || isPolicyExpenseChat) && isChatThread && !isPolicyEmployee)) { | ||
items.push({ | ||
key: CONST.REPORT_DETAILS_MENU_ITEM.INVITE, | ||
translationKey: 'common.invite', | ||
icon: Expensicons.Users, | ||
isAnonymousAction: false, | ||
shouldShowRightIcon: true, | ||
action: () => { | ||
Navigation.navigate(ROUTES.ROOM_INVITE.getRoute(report?.reportID ?? '')); | ||
}, | ||
|
@@ -179,6 +181,7 @@ function ReportDetailsPage({policies, report, session, personalDetails}: ReportD | |
translationKey: 'common.settings', | ||
icon: Expensicons.Gear, | ||
isAnonymousAction: false, | ||
shouldShowRightIcon: true, | ||
action: () => { | ||
Navigation.navigate(ROUTES.REPORT_SETTINGS.getRoute(report?.reportID ?? '')); | ||
}, | ||
|
@@ -191,15 +194,32 @@ function ReportDetailsPage({policies, report, session, personalDetails}: ReportD | |
translationKey: 'privateNotes.title', | ||
icon: Expensicons.Pencil, | ||
isAnonymousAction: false, | ||
shouldShowRightIcon: true, | ||
action: () => ReportUtils.navigateToPrivateNotes(report, session), | ||
brickRoadIndicator: Report.hasErrorInPrivateNotes(report) ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined, | ||
}); | ||
} | ||
|
||
if ((isGroupChat || (isChatRoom && ReportUtils.canLeaveChat(report, policy ?? null))) && !isThread) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree with you at this point There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree, but sort of out of scope for the original ticket maybe. I believe there is a separate project to make this change. |
||
items.push({ | ||
key: CONST.REPORT_DETAILS_MENU_ITEM.LEAVE_ROOM, | ||
translationKey: 'common.leave', | ||
icon: Expensicons.Exit, | ||
isAnonymousAction: true, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Coming from #43404: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmmm There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for confirmation |
||
action: () => { | ||
if (Object.keys(report?.participants ?? {}).length === 1 && isGroupChat) { | ||
setIsLastMemberLeavingGroupModalVisible(true); | ||
return; | ||
} | ||
|
||
leaveChat(); | ||
}, | ||
}); | ||
} | ||
|
||
return items; | ||
}, [ | ||
isSelfDM, | ||
isGroupDMChat, | ||
isArchivedRoom, | ||
isGroupChat, | ||
isDefaultRoom, | ||
|
@@ -208,10 +228,15 @@ function ReportDetailsPage({policies, report, session, personalDetails}: ReportD | |
isUserCreatedPolicyRoom, | ||
participants.length, | ||
report, | ||
isPolicyExpenseChat, | ||
isMoneyRequestReport, | ||
isInvoiceReport, | ||
isThread, | ||
isChatRoom, | ||
policy, | ||
activeChatMembers.length, | ||
session, | ||
leaveChat, | ||
]); | ||
|
||
const displayNamesWithTooltips = useMemo(() => { | ||
|
@@ -258,10 +283,7 @@ function ReportDetailsPage({policies, report, session, personalDetails}: ReportD | |
/> | ||
); | ||
|
||
const reportName = | ||
ReportUtils.isDeprecatedGroupDM(report) || ReportUtils.isGroupChat(report) | ||
? ReportUtils.getGroupChatName(undefined, false, report.reportID ?? '') | ||
: ReportUtils.getReportName(report); | ||
const reportName = ReportUtils.isDeprecatedGroupDM(report) || isGroupChat ? ReportUtils.getGroupChatName(undefined, false, report.reportID ?? '') : ReportUtils.getReportName(report); | ||
return ( | ||
<ScreenWrapper testID={ReportDetailsPage.displayName}> | ||
<FullPageNotFoundView shouldShow={isEmptyObject(report)}> | ||
|
@@ -320,7 +342,10 @@ function ReportDetailsPage({policies, report, session, personalDetails}: ReportD | |
</View> | ||
</View> | ||
{shouldShowReportDescription && ( | ||
<OfflineWithFeedback pendingAction={report.pendingFields?.description}> | ||
<OfflineWithFeedback | ||
pendingAction={report.pendingFields?.description} | ||
style={styles.mb5} | ||
> | ||
<MenuItemWithTopDescription | ||
shouldShowRightIcon={canEditReportDescription} | ||
interactive={canEditReportDescription} | ||
|
@@ -332,7 +357,7 @@ function ReportDetailsPage({policies, report, session, personalDetails}: ReportD | |
/> | ||
</OfflineWithFeedback> | ||
)} | ||
{isGroupChat && <ChatDetailsQuickActionsBar report={report} />} | ||
{!isGroupDMChat && <ChatDetailsQuickActionsBar report={report} />} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can pin a Group DM chat, but not share it. They are getting deprecated soon. But, I think this is more or less incorrect logic and we should be able to remove the |
||
{menuItems.map((item) => { | ||
const brickRoadIndicator = | ||
ReportUtils.hasReportNameError(report) && item.key === CONST.REPORT_DETAILS_MENU_ITEM.SETTINGS ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined; | ||
|
@@ -344,12 +369,25 @@ function ReportDetailsPage({policies, report, session, personalDetails}: ReportD | |
icon={item.icon} | ||
onPress={item.action} | ||
isAnonymousAction={item.isAnonymousAction} | ||
shouldShowRightIcon | ||
shouldShowRightIcon={item.shouldShowRightIcon} | ||
brickRoadIndicator={brickRoadIndicator ?? item.brickRoadIndicator} | ||
/> | ||
); | ||
})} | ||
</ScrollView> | ||
<ConfirmModal | ||
danger | ||
title={translate('groupChat.lastMemberTitle')} | ||
isVisible={isLastMemberLeavingGroupModalVisible} | ||
onConfirm={() => { | ||
setIsLastMemberLeavingGroupModalVisible(false); | ||
Report.leaveGroupChat(report.reportID); | ||
}} | ||
onCancel={() => setIsLastMemberLeavingGroupModalVisible(false)} | ||
prompt={translate('groupChat.lastMemberWarning')} | ||
confirmText={translate('common.leave')} | ||
cancelText={translate('common.cancel')} | ||
/> | ||
</FullPageNotFoundView> | ||
</ScreenWrapper> | ||
); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well, we need to add back the
if (!isGroupDMChat)
to this part now... I thought that was clear from my last comment sorry if not.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, sorry
Perhaps I misunderstood
But I fixed it
And I updated the styles for one button inside
ActionsBar
following this comment#40256 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you mind posting a screen shot of this change? I want to make sure it's what @shawnborton expects. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something like this if we have one button
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we're working on a component for this that has all of that baked in. @grgia does that sound right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check this PR here: #41972
Just want to make sure we aren't duplicating work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shawnborton thanks for noticing! It looks like this PR will conflict with #41972
cc @grgia
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the heads up @shawnborton
I think we're good here though. We just need to fix some conflicts now @ZhenjaHorbach. I've instructed @cdOut to pause the next PR they mentioned since we don't really need it.
@grgia might want to do a quick scan to make sure there aren't any other improvements with open issues that snuck into that design doc, but don't think there are a ton. They should all be in the #vip-split project and categorized by Group Chats.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated the PR and made changes to use the new implementation for ActionBar
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although we have TS issue in main
#42798