-
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
[Group Chats] Add remaining features #39757
Changes from 22 commits
9dba74f
b35fccb
bf96f01
c14f10b
66a220a
2e3e6bb
b041c73
9ed73cd
35bbf1b
9aa8f74
b7caed0
d8c6956
459bef4
5a0e7eb
e6a047d
699cfc7
8791b81
fc84939
639693b
790d3ca
b734506
8118ed1
7ac3859
61afba6
cd7ccc0
c7d89f2
98058a5
08fedfc
46c75fd
d37e029
259355f
0b56abe
a6d8ca4
09c2fd9
896f0c7
38c5934
640f90b
6c59b94
0289227
d7837d8
14aaaf1
dea4cbf
46ea41a
5dae3d1
2bdf2c8
17ad9c5
99cfe87
aae648b
fab964f
6664cfb
51ab0c2
e2663cb
c7d2d36
e202c7b
67f2986
bd0f05e
2dda3f1
9488c3b
0dd6b90
3241bbd
21fd1d6
ef4286d
51052e0
9b10486
f049283
dec226e
1dfdc64
6a74c9d
faed7df
72b4884
9568bc3
9c2ab05
def5c54
feaf901
d27b691
ba3e768
1efda68
4ee60b5
76e0d92
bc0c380
b4248e4
2acb986
69e0de5
e57ad31
01b032a
6f3c030
84e631c
351f318
d054329
22f6d15
41b3924
85454e6
06eb201
c479153
e391d7d
ae4ed17
2440183
f5970a9
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 |
---|---|---|
|
@@ -186,6 +186,7 @@ const ROUTES = { | |
NEW: 'new', | ||
NEW_CHAT: 'new/chat', | ||
NEW_CHAT_CONFIRM: 'new/chat/confirm', | ||
NEW_CHAT_EDIT_NAME: 'new/chat/confirm/name/edit', | ||
NEW_ROOM: 'new/room', | ||
|
||
REPORT: 'r', | ||
|
@@ -222,6 +223,18 @@ const ROUTES = { | |
route: 'r/:reportID/participants', | ||
getRoute: (reportID: string) => `r/${reportID}/participants` as const, | ||
}, | ||
REPORT_PARTICIPANTS_INVITE: { | ||
route: 'r/:reportID/participants/invite', | ||
getRoute: (reportID: string) => `r/${reportID}/participants/invite` as const, | ||
}, | ||
REPORT_PARTICIPANTS_DETAILS: { | ||
route: 'r/:reportID/participants/:accountID', | ||
getRoute: (reportID: string, accountID: number, backTo?: string) => getUrlWithBackToParam(`r/${reportID}/participants/${accountID}`, backTo), | ||
}, | ||
REPORT_PARTICIPANTS_ROLE_SELECTION: { | ||
route: 'r/:reportID/participants/:accountID/role', | ||
getRoute: (reportID: string, accountID: number, backTo?: string) => getUrlWithBackToParam(`r/${reportID}/participants/${accountID}/role`, backTo), | ||
}, | ||
REPORT_WITH_ID_DETAILS: { | ||
route: 'r/:reportID/details', | ||
getRoute: (reportID: string, backTo?: string) => getUrlWithBackToParam(`r/${reportID}/details`, backTo), | ||
|
@@ -234,6 +247,10 @@ const ROUTES = { | |
route: 'r/:reportID/settings/room-name', | ||
getRoute: (reportID: string) => `r/${reportID}/settings/room-name` as const, | ||
}, | ||
REPORT_SETTINGS_GROUP_NAME: { | ||
route: 'r/:reportID/settings/group-name', | ||
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. Unsure if there is a better url for this. Maybe 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. Yeah, I'd think it might be simpler to combine 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. Nice. That feels worth exploring as a follow up. Updating the Group name is a lot looser than updating the Room name. Gonna leave this for now I think. |
||
getRoute: (reportID: string) => `r/${reportID}/settings/group-name` as const, | ||
}, | ||
REPORT_SETTINGS_NOTIFICATION_PREFERENCES: { | ||
route: 'r/:reportID/settings/notification-preferences', | ||
getRoute: (reportID: string) => `r/${reportID}/settings/notification-preferences` as const, | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -117,6 +117,9 @@ type AvatarWithImagePickerProps = { | |||||
|
||||||
/** Allows to open an image without Attachment Picker. */ | ||||||
enablePreview?: boolean; | ||||||
|
||||||
/** Hard disables the "View photo" option */ | ||||||
disableViewPhoto?: boolean; | ||||||
marcaaron marked this conversation as resolved.
Show resolved
Hide resolved
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.
Suggested change
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. Yeah. It's also enabled by default. We are going to remove this as a very fast follower so I updated it to |
||||||
}; | ||||||
|
||||||
function AvatarWithImagePicker({ | ||||||
|
@@ -144,6 +147,7 @@ function AvatarWithImagePicker({ | |||||
disabled = false, | ||||||
onViewPhotoPress, | ||||||
enablePreview = false, | ||||||
disableViewPhoto = false, | ||||||
}: AvatarWithImagePickerProps) { | ||||||
const theme = useTheme(); | ||||||
const styles = useThemeStyles(); | ||||||
|
@@ -355,7 +359,7 @@ function AvatarWithImagePicker({ | |||||
const menuItems = createMenuItems(openPicker); | ||||||
|
||||||
// If the current avatar isn't a default avatar, allow the "View Photo" option | ||||||
marcaaron marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
if (!isUsingDefaultAvatar) { | ||||||
if (!disableViewPhoto && !isUsingDefaultAvatar) { | ||||||
menuItems.push({ | ||||||
icon: Expensicons.Eye, | ||||||
text: translate('avatarWithImagePicker.viewPhoto'), | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
import React from 'react'; | ||
import {View} from 'react-native'; | ||
import * as Report from '../libs/actions/Report'; | ||
import Button from './Button'; | ||
import * as Expensicons from './Icon/Expensicons'; | ||
|
||
function ChatActionsBar({report}) { | ||
const isPinned = !!report.isPinned; | ||
return ( | ||
<View style={{flexDirection: 'row', paddingHorizontal: 14, marginBottom: 20}}> | ||
<View style={{flex: 1, paddingHorizontal: 6}}> | ||
marcaaron marked this conversation as resolved.
Show resolved
Hide resolved
|
||
<Button | ||
onPress={() => Report.leaveGroupChat(report.reportID)} | ||
icon={Expensicons.Exit} | ||
style={{flex: 1}} | ||
text="Leave" | ||
marcaaron marked this conversation as resolved.
Show resolved
Hide resolved
|
||
/> | ||
</View> | ||
<View style={{flex: 1, paddingHorizontal: 6}}> | ||
<Button | ||
onPress={() => Report.togglePinnedState(report.reportID, isPinned)} | ||
icon={Expensicons.Pin} | ||
style={{flex: 1}} | ||
text={isPinned ? 'Unpin' : 'Pin'} | ||
/> | ||
</View> | ||
</View> | ||
); | ||
} | ||
|
||
marcaaron marked this conversation as resolved.
Show resolved
Hide resolved
|
||
export default ChatActionsBar; |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,5 +1,5 @@ | ||||||
import type {ReactNode} from 'react'; | ||||||
import React from 'react'; | ||||||
import React, {useMemo} from 'react'; | ||||||
import type {StyleProp, TextStyle, ViewStyle} from 'react-native'; | ||||||
import {View} from 'react-native'; | ||||||
import useThemeStyles from '@hooks/useThemeStyles'; | ||||||
|
@@ -21,13 +21,35 @@ type HeaderProps = { | |||||
|
||||||
/** Additional header container styles */ | ||||||
containerStyles?: StyleProp<ViewStyle>; | ||||||
|
||||||
/** Whether to show the subtitle on the top or bottom */ | ||||||
subtitleOnTop?: boolean; | ||||||
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.
Suggested change
|
||||||
}; | ||||||
|
||||||
function Header({title = '', subtitle = '', textStyles = [], containerStyles = [], shouldShowEnvironmentBadge = false}: HeaderProps) { | ||||||
function Header({title = '', subtitle = '', textStyles = [], containerStyles = [], shouldShowEnvironmentBadge = false, subtitleOnTop = true}: HeaderProps) { | ||||||
marcaaron marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
const styles = useThemeStyles(); | ||||||
const renderedSubtitle = useMemo( | ||||||
() => ( | ||||||
<> | ||||||
{/* If there's no subtitle then display a fragment to avoid an empty space which moves the main title */} | ||||||
{typeof subtitle === 'string' | ||||||
? Boolean(subtitle) && ( | ||||||
<Text | ||||||
style={[styles.mutedTextLabel, styles.pre]} | ||||||
numberOfLines={1} | ||||||
> | ||||||
{subtitle} | ||||||
</Text> | ||||||
) | ||||||
: subtitle} | ||||||
</> | ||||||
), | ||||||
[subtitle, styles], | ||||||
); | ||||||
return ( | ||||||
<View style={[styles.flex1, styles.flexRow, containerStyles]}> | ||||||
<View style={styles.mw100}> | ||||||
{subtitleOnTop && renderedSubtitle} | ||||||
{typeof title === 'string' | ||||||
? Boolean(title) && ( | ||||||
<Text | ||||||
|
@@ -38,17 +60,7 @@ function Header({title = '', subtitle = '', textStyles = [], containerStyles = [ | |||||
</Text> | ||||||
) | ||||||
: title} | ||||||
{/* If there's no subtitle then display a fragment to avoid an empty space which moves the main title */} | ||||||
{typeof subtitle === 'string' | ||||||
? Boolean(subtitle) && ( | ||||||
<Text | ||||||
style={[styles.mutedTextLabel, styles.pre]} | ||||||
numberOfLines={1} | ||||||
> | ||||||
{subtitle} | ||||||
</Text> | ||||||
) | ||||||
: subtitle} | ||||||
{!subtitleOnTop && renderedSubtitle} | ||||||
</View> | ||||||
{shouldShowEnvironmentBadge && <EnvironmentBadge />} | ||||||
</View> | ||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -129,6 +129,9 @@ type HeaderWithBackButtonProps = Partial<ChildrenProps> & { | |||||
|
||||||
/** Additional styles to add to the component */ | ||||||
style?: StyleProp<ViewStyle>; | ||||||
|
||||||
/** Flips the subtitle with the title */ | ||||||
subtitleOnTop?: boolean; | ||||||
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.
Suggested change
|
||||||
}; | ||||||
|
||||||
export type {ThreeDotsMenuItem}; | ||||||
|
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.
question: why does this route not use
getUrlWithBackToParam
, whileDETAILS
andROLE_SELECTION
do?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.
Great question. I don't have a great understanding of
backTo
or how that works. I removed it from the routes you mentioned. If we find out we need them we can add them later.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 it's mostly for the sake of the browser back button working as expected, but last I checked it wasn't really working after the "ideal nav" changes a couple months ago.
Whatever the case, more polish than blocker
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.
Ok I'll move this into the "to check later" pile. I am eager to get this merged soon-ish.