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 Rest of UI for Workspace Chats #7957

Merged
merged 46 commits into from
Mar 11, 2022
Merged
Show file tree
Hide file tree
Changes from 31 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
f759195
Add worksapce subtitle for policyExpenseChats
Feb 28, 2022
b7e1989
Use Localize to get correct translation in other languages
Feb 28, 2022
33562ec
Merge branch 'main' of github.com:Expensify/App into amal-workspace-c…
Mar 1, 2022
25db051
Merge branch 'marco-avatarsWorkspaceChats' of github.com:Expensify/Ap…
Mar 1, 2022
a63a86c
Make subtitle for policyExpenseChats visible
Mar 1, 2022
b9685a6
Fix ambiguous boolean statement
Mar 2, 2022
364ca06
Move shouldIncludeParticipants to ReportWelcomeText instead
Mar 2, 2022
d6ebc16
Rename to isChatRoom to be more clear
Mar 2, 2022
9c3eb78
Remove console logs
Mar 2, 2022
88cadd5
Restructure to allow for three options
Mar 2, 2022
9f27699
Use policy name for report welcome text
Mar 2, 2022
7aab36f
Add spanish translation for policyExpenseChat welcome text
Mar 3, 2022
2068333
fix spacing
Mar 3, 2022
8dcf8b3
Add missing space
Mar 3, 2022
d73bb90
use lugar where it makes more sense than sitio
Mar 3, 2022
fed389b
fix import casing
Mar 3, 2022
9af0796
Merge branch 'marco-avatarsWorkspaceChats' of github.com:Expensify/Ap…
Mar 3, 2022
7a7cf3c
Point policyExpenseChats to the reportDetails page
Mar 3, 2022
5030078
Fix reportdetails view for chatrooms and policyExpenseChats
Mar 3, 2022
5c0b09b
Remove unused style
Mar 3, 2022
983b3d2
Fix comment
Mar 3, 2022
f4e9f9a
Ensure ownerEmail is always included in policyExpenseChats
Mar 3, 2022
765aac2
style fixes
Mar 3, 2022
77b2c7b
Merge branch 'main' of github.com:Expensify/App into amal-workspace-c…
Mar 4, 2022
af6d506
Add settings page for policyExpenseChats
Mar 7, 2022
75a255e
Fix title policyExpenseChat members page
Mar 7, 2022
3916197
Fix navigation for members page
Mar 7, 2022
4489e89
style fix
Mar 7, 2022
95143d4
Merge branch 'main' of github.com:Expensify/App into amal-workspace-c…
Mar 8, 2022
beea726
Merge branch 'main' of github.com:Expensify/App into amal-workspace-c…
Mar 9, 2022
ed45672
resolve conflict
Mar 9, 2022
7352c5b
Merge branch 'main' of github.com:Expensify/App into amal-workspace-c…
Mar 10, 2022
6d54630
Clean up proptypes
Mar 10, 2022
6d7befc
Remove self-explanatory comments
Mar 10, 2022
e399654
clean up menuitems
Mar 10, 2022
69ea420
Merge branch 'main' of github.com:Expensify/App into amal-workspace-c…
Mar 10, 2022
2c56b09
Be explicit on which types of chats can access menuItems
Mar 10, 2022
c95f4b7
Use push instead of concat
Mar 10, 2022
3e08a89
add comment about why we don't include self in participants
Mar 10, 2022
b00430c
Clean up comments
Mar 10, 2022
a512f7a
Style fixes
Mar 10, 2022
a792b8a
simplify particpants collecting
Mar 10, 2022
9420c26
Better proptype organization
Mar 10, 2022
2176ddf
Replace unkown policy with better message
Mar 10, 2022
8b713fe
Simplify logic and avoid using ternary and unifying separate data int…
Mar 10, 2022
84a02ed
Add missing import
Mar 10, 2022
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
112 changes: 73 additions & 39 deletions src/components/ReportWelcomeText.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import styles from '../styles/styles';
import Text from './Text';
import withLocalize, {withLocalizePropTypes} from './withLocalize';
import compose from '../libs/compose';
import * as ReportUtils from '../libs/reportUtils';
import * as OptionsListUtils from '../libs/OptionsListUtils';
import ONYXKEYS from '../ONYXKEYS';
import CONST from '../CONST';
Expand All @@ -26,24 +27,34 @@ const personalDetailsPropTypes = PropTypes.shape({
});

const propTypes = {
/** Whether it is a default Chat Room */
shouldIncludeParticipants: PropTypes.bool,
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice one, that comment didn't really match the prop name.


/** The report currently being looked at */
report: PropTypes.oneOfType([PropTypes.object]),

/** All of the personal details for everyone */
personalDetails: PropTypes.objectOf(personalDetailsPropTypes).isRequired,

/* Onyx Props */
TomatoToaster marked this conversation as resolved.
Show resolved Hide resolved

/** The policies which the user has access to and which the report could be tied to */
policies: PropTypes.shape({
/** The policy name */
name: PropTypes.string,

/** ID of the policy */
id: PropTypes.string,
TomatoToaster marked this conversation as resolved.
Show resolved Hide resolved
}).isRequired,

...withLocalizePropTypes,
};

const defaultProps = {
report: {},
shouldIncludeParticipants: true,
};

const ReportWelcomeText = (props) => {
const isPolicyExpenseChat = ReportUtils.isPolicyExpenseChat(props.report);
const isChatRoom = ReportUtils.isChatRoom(props.report);
const isDefault = !(isChatRoom || isPolicyExpenseChat);
const participants = lodashGet(props.report, 'participants', []);
const isMultipleParticipant = participants.length > 1;
const displayNamesWithTooltips = _.map(
Expand All @@ -66,46 +77,66 @@ const ReportWelcomeText = (props) => {
};
},
);
const chatUsers = props.shouldIncludeParticipants ? displayNamesWithTooltips : [{displayName: props.report.reportName}];
const chatUsers = isChatRoom ? [{displayName: props.report.reportName}] : displayNamesWithTooltips;
const isResctrictedRoom = lodashGet(props, 'report.visibility', '') === CONST.REPORT.VISIBILITY.RESTRICTED;

return (
<Text style={[styles.mt3, styles.mw100, styles.textAlignCenter]}>
{!props.shouldIncludeParticipants
? (
<>
<Text>
{isResctrictedRoom
? `${props.translate('reportActionsView.beginningOfChatHistoryRestrictedPartOne')}`
: `${props.translate('reportActionsView.beginningOfChatHistoryPrivatePartOne')}`}
</Text>
<Text style={[styles.textStrong]}>
{lodashGet(chatUsers, '[0].displayName', '')}
</Text>
<Text>
{isResctrictedRoom
? `${props.translate('reportActionsView.beginningOfChatHistoryRestrictedPartTwo')}`
: `${props.translate('reportActionsView.beginningOfChatHistoryPrivatePartTwo')}`}
</Text>
</>
) : (
<>
<Text>
{props.translate('reportActionsView.beginningOfChatHistory')}
</Text>
{_.map(chatUsers, ({displayName, pronouns}, index) => (
<Text key={displayName}>
<Text style={[styles.textStrong]}>
{displayName}
</Text>
{!_.isEmpty(pronouns) && <Text>{` (${pronouns})`}</Text>}
{(index === chatUsers.length - 1) && <Text>.</Text>}
{(index === chatUsers.length - 2) && <Text>{` ${props.translate('common.and')} `}</Text>}
{(index < chatUsers.length - 2) && <Text>, </Text>}
{isPolicyExpenseChat && (
<>
<Text>
{props.translate('reportActionsView.beginningOfChatHistoryPolicyExpenseChatPartOne')}
</Text>
<Text style={[styles.textStrong]}>
{/* Use the policyExpenseChat owner's first name or their email if it's undefined or an empty string */}
{lodashGet(props.personalDetails, [props.report.ownerEmail, 'firstName']) || props.report.ownerEmail}
Copy link
Contributor

Choose a reason for hiding this comment

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

Where's the empty string come from (mentioned in the comment)? Should we at least have the firstName or ownerEmail?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yes I mean to say that if the firstName isn't set we should fall back to ownerEmail. I should be saying "Use the policyExpenseChat owner's first name or fall back to their email if it's unavailable". Could be a bit of a self-explanatory comment though, do you think I should just delete it?

Copy link
Contributor

Choose a reason for hiding this comment

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

I feel an overwhelming almost painful impulse to delete comments if they don't explain something the code doesn't explain itself. But meditation is helping a lot with that.

</Text>
<Text>
{props.translate('reportActionsView.beginningOfChatHistoryPolicyExpenseChatPartTwo')}
</Text>
<Text style={[styles.textStrong]}>
{lodashGet(props.policies, [`${ONYXKEYS.COLLECTION.POLICY}${props.report.policyID}`, 'name'], 'Unknown Policy')}
TomatoToaster marked this conversation as resolved.
Show resolved Hide resolved
</Text>
<Text>
{props.translate('reportActionsView.beginningOfChatHistoryPolicyExpenseChatPartThree')}
</Text>
</>
)}
{isChatRoom && (
<>
<Text>
{isResctrictedRoom
? `${props.translate('reportActionsView.beginningOfChatHistoryRestrictedPartOne')}`
: `${props.translate('reportActionsView.beginningOfChatHistoryPrivatePartOne')}`}
</Text>
<Text style={[styles.textStrong]}>
{lodashGet(chatUsers, '[0].displayName', '')}
TomatoToaster marked this conversation as resolved.
Show resolved Hide resolved
</Text>
<Text>
{isResctrictedRoom
? `${props.translate('reportActionsView.beginningOfChatHistoryRestrictedPartTwo')}`
: `${props.translate('reportActionsView.beginningOfChatHistoryPrivatePartTwo')}`}
</Text>
</>
)}
{isDefault && (
<>
<Text>
{props.translate('reportActionsView.beginningOfChatHistory')}
</Text>
{_.map(chatUsers, ({displayName, pronouns}, index) => (
<Text key={displayName}>
<Text style={[styles.textStrong]}>
{displayName}
</Text>
))}
</>
)}
{!_.isEmpty(pronouns) && <Text>{` (${pronouns})`}</Text>}
{(index === chatUsers.length - 1) && <Text>.</Text>}
{(index === chatUsers.length - 2) && <Text>{` ${props.translate('common.and')} `}</Text>}
{(index < chatUsers.length - 2) && <Text>, </Text>}
</Text>
))}
</>
)}
</Text>
);
};
Expand All @@ -120,5 +151,8 @@ export default compose(
personalDetails: {
key: ONYXKEYS.PERSONAL_DETAILS,
},
policies: {
key: ONYXKEYS.COLLECTION.POLICY,
},
}),
)(ReportWelcomeText);
3 changes: 3 additions & 0 deletions src/languages/en.js
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,9 @@ export default {
beginningOfChatHistoryRestrictedPartOne: 'This is the beginning of ',
beginningOfChatHistoryPrivatePartTwo: ' room, invite others by @mentioning them.',
beginningOfChatHistoryRestrictedPartTwo: ', invite others by @mentioning them.',
beginningOfChatHistoryPolicyExpenseChatPartOne: 'Collaboration between ',
beginningOfChatHistoryPolicyExpenseChatPartTwo: ' and ',
beginningOfChatHistoryPolicyExpenseChatPartThree: ' starts here! 🎉 This is the place to chat, request money and settle up.',
TomatoToaster marked this conversation as resolved.
Show resolved Hide resolved
},
reportActionsViewMarkerBadge: {
newMsg: ({count}) => `${count} new message${count > 1 ? 's' : ''}`,
Expand Down
5 changes: 4 additions & 1 deletion src/languages/es.js
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,9 @@ export default {
beginningOfChatHistoryRestrictedPartOne: 'Este es el principio de ',
beginningOfChatHistoryPrivatePartTwo: ', invita a otros @mencionándolos.',
beginningOfChatHistoryRestrictedPartTwo: ', invita a otros @mencionándolos.',
beginningOfChatHistoryPolicyExpenseChatPartOne: '¡La colaboración entre ',
beginningOfChatHistoryPolicyExpenseChatPartTwo: ' y ',
beginningOfChatHistoryPolicyExpenseChatPartThree: ' empieza aquí! :tada: Este es el lugar donde chatear, pedir dinero y pagar.',
},
reportActionsViewMarkerBadge: {
newMsg: ({count}) => `${count} mensaje${count > 1 ? 's' : ''} nuevo${count > 1 ? 's' : ''}`,
Expand Down Expand Up @@ -493,7 +496,7 @@ export default {
connectManually: 'Conectar manualmente',
desktopConnection: 'Para conectarse con Chase, Wells Fargo, Capital One o Bank of America, haga clic aquí para completar este proceso en un navegador.',
yourDataIsSecure: 'Tus datos están seguros',
toGetStarted: 'Añade una cuenta bancaria y emite tarjetas corporativas, reembolsa gastos y cobra y paga facturas, todo desde un mismo sitio.',
toGetStarted: 'Añade una cuenta bancaria y emite tarjetas corporativas, reembolsa gastos y cobra y paga facturas, todo desde un mismo lugar.',
plaidBodyCopy: 'Ofrezca a sus empleados una forma más sencilla de pagar - y recuperar - los gastos de la empresa.',
checkHelpLine: 'Su número de ruta y número de cuenta se pueden encontrar en un cheque de la cuenta bancaria.',
validateAccountError: 'Para terminar de configurar tu cuenta bancaria, debes validar tu cuenta de Expensify. Por favor revisa tu correo electrónico para validar tu cuenta y regresa aquí para continuar.',
Expand Down
6 changes: 1 addition & 5 deletions src/libs/OptionsListUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -210,11 +210,7 @@ function hasReportDraftComment(report) {
* @returns {Array<String>}
*/
function getParticipants(report) {
const participants = [...lodashGet(report, ['participants'], [])];
if (ReportUtils.isPolicyExpenseChat(report) && !_.contains(participants, report.ownerEmail)) {
participants.push(report.ownerEmail);
}
return participants;
return [...lodashGet(report, ['participants'], [])];
Copy link
Contributor

Choose a reason for hiding this comment

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

OK, now that we are not pushing anything into the participants the spread is not necessary? Probably this whole method can go and the one usage can be updated

const logins = getParticipants(report);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yeah this is a good point, now I'm not sure why the spread was there in the first place 😅

}

/**
Expand Down
11 changes: 9 additions & 2 deletions src/libs/actions/Report.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,16 @@ function getUnreadActionCount(report) {
* @param {Object} report
* @return {String[]}
*/
function getParticipantEmailsFromReport({sharedReportList, reportNameValuePairs}) {
function getParticipantEmailsFromReport({sharedReportList, reportNameValuePairs, ownerEmail}) {
const emailArray = _.map(sharedReportList, participant => participant.email);
return (ReportUtils.isChatRoom(reportNameValuePairs) || ReportUtils.isPolicyExpenseChat(reportNameValuePairs)) ? emailArray : _.without(emailArray, currentUserEmail);
if (ReportUtils.isChatRoom(reportNameValuePairs)) {
return emailArray;
}
if (ReportUtils.isPolicyExpenseChat(reportNameValuePairs)) {
// The owner of the policyExpenseChat isn't in the sharedReportsList so they need to be explicitely included.
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
// The owner of the policyExpenseChat isn't in the sharedReportsList so they need to be explicitely included.
// The owner of the policyExpenseChat isn't in the sharedReportsList so they need to be explicitly included.

return [ownerEmail, ...emailArray];
}
return _.without(emailArray, currentUserEmail);
TomatoToaster marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand Down
4 changes: 4 additions & 0 deletions src/libs/reportUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import lodashGet from 'lodash/get';
import Onyx from 'react-native-onyx';
import ONYXKEYS from '../ONYXKEYS';
import CONST from '../CONST';
import * as Localize from './Localize';

let sessionEmail;
Onyx.connect({
Expand Down Expand Up @@ -170,6 +171,9 @@ function getChatRoomSubtitle(report, policiesMap) {
if (isArchivedRoom(report)) {
return report.oldPolicyName;
}
if (isPolicyExpenseChat(report) && report.isOwnPolicyExpenseChat) {
return Localize.translateLocal('workspace.common.workspace');
}
return lodashGet(
policiesMap,
[`${ONYXKEYS.COLLECTION.POLICY}${report.policyID}`, 'name'],
Expand Down
15 changes: 13 additions & 2 deletions src/pages/ReportDetailsPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ class ReportDetailsPage extends Component {
action: () => { Navigation.navigate(ROUTES.getReportParticipantsRoute(props.report.reportID)); },
});

// Chat rooms will allow you to more things than typical chats so they have extra options
// Chat rooms will allow you to do more things than typical chats so they have extra options
TomatoToaster marked this conversation as resolved.
Show resolved Hide resolved
if (ReportUtils.isChatRoom(this.props.report)) {
this.menuItems = this.menuItems.concat([
{
Expand All @@ -100,6 +100,17 @@ class ReportDetailsPage extends Component {
},
]);
}

// Policy Expense Chats will also allow the user to see settings[]
if (ReportUtils.isPolicyExpenseChat(this.props.report)) {
TomatoToaster marked this conversation as resolved.
Show resolved Hide resolved
this.menuItems = this.menuItems.concat([
TomatoToaster marked this conversation as resolved.
Show resolved Hide resolved
{
translationKey: 'common.settings',
icon: Expensicons.Gear,
action: () => { Navigation.navigate(ROUTES.getReportSettingsRoute(props.report.reportID)); },
},
TomatoToaster marked this conversation as resolved.
Show resolved Hide resolved
]);
}
}

render() {
Expand Down Expand Up @@ -146,7 +157,7 @@ class ReportDetailsPage extends Component {
tooltipEnabled
numberOfLines={1}
textStyles={[styles.headerText, styles.mb2, styles.textAlignCenter]}
shouldUseFullTitle={isChatRoom}
shouldUseFullTitle={isChatRoom || isPolicyExpenseChat}
/>
</View>
<Text
Expand Down
4 changes: 2 additions & 2 deletions src/pages/ReportParticipantsPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,10 @@ const ReportParticipantsPage = (props) => {
return (
<ScreenWrapper>
<HeaderWithCloseButton
title={props.translate(ReportUtils.isChatRoom(props.report) ? 'common.members' : 'common.details')}
title={props.translate((ReportUtils.isChatRoom(props.report) || ReportUtils.isPolicyExpenseChat(props.report)) ? 'common.members' : 'common.details')}
onCloseButtonPress={Navigation.dismissModal}
onBackButtonPress={Navigation.goBack}
shouldShowBackButton={ReportUtils.isChatRoom(props.report)}
shouldShowBackButton={ReportUtils.isChatRoom(props.report) || ReportUtils.isPolicyExpenseChat(props.report)}
/>
<View
pointerEvents="box-none"
Expand Down
54 changes: 29 additions & 25 deletions src/pages/ReportSettingsPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,9 @@ class ReportSettingsPage extends Component {
}

render() {
const shouldDisableRename = ReportUtils.isDefaultRoom(this.props.report) || ReportUtils.isArchivedRoom(this.props.report);
const shouldShowRename = !ReportUtils.isPolicyExpenseChat(this.props.report);
const shouldDisableRename = ReportUtils.isDefaultRoom(this.props.report)
|| ReportUtils.isArchivedRoom(this.props.report);
const linkedWorkspace = _.find(this.props.policies, policy => policy.id === this.props.report.policyID);

return (
Expand Down Expand Up @@ -184,33 +186,35 @@ class ReportSettingsPage extends Component {
/>
</View>
</View>
<View style={styles.mt4}>
<Text style={[styles.formLabel]} numberOfLines={1}>
{this.props.translate('newRoomPage.roomName')}
</Text>
<View style={[styles.flexRow, styles.mb1]}>
<View style={[styles.flex3]}>
<RoomNameInput
initialValue={this.state.newRoomName}
policyID={linkedWorkspace && linkedWorkspace.id}
errorText={this.state.errors.newRoomName}
onChangeText={newRoomName => this.clearErrorAndSetValue('newRoomName', newRoomName)}
disabled={shouldDisableRename}
{shouldShowRename && (
<View style={styles.mt4}>
<Text style={[styles.formLabel]} numberOfLines={1}>
{this.props.translate('newRoomPage.roomName')}
</Text>
<View style={[styles.flexRow, styles.mb1]}>
<View style={[styles.flex3]}>
<RoomNameInput
initialValue={this.state.newRoomName}
policyID={linkedWorkspace && linkedWorkspace.id}
errorText={this.state.errors.newRoomName}
onChangeText={newRoomName => this.clearErrorAndSetValue('newRoomName', newRoomName)}
disabled={shouldDisableRename}
/>
</View>
<Button
large
success={!shouldDisableRename}
text={this.props.translate('common.save')}
onPress={this.validateAndRenameReport}
style={[styles.ml2, styles.flex1]}
textStyles={[styles.label]}
innerStyles={[styles.ph5]}
isLoading={this.props.isLoadingRenamePolicyRoom}
isDisabled={shouldDisableRename}
/>
</View>
<Button
large
success={!shouldDisableRename}
text={this.props.translate('common.save')}
onPress={this.validateAndRenameReport}
style={[styles.ml2, styles.flex1]}
textStyles={[styles.label]}
innerStyles={[styles.ph5]}
isLoading={this.props.isLoadingRenamePolicyRoom}
isDisabled={shouldDisableRename}
/>
</View>
</View>
)}
{linkedWorkspace && (
<View style={[styles.mt4]}>
<Text style={[styles.formLabel]} numberOfLines={1}>
Expand Down
4 changes: 2 additions & 2 deletions src/pages/home/HeaderView.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ const HeaderView = (props) => {
>
<Pressable
onPress={() => {
if (isChatRoom) {
if (isChatRoom || isPolicyExpenseChat) {
return Navigation.navigate(ROUTES.getReportDetailsRoute(props.report.reportID));
}
if (participants.length === 1) {
Expand Down Expand Up @@ -158,7 +158,7 @@ const HeaderView = (props) => {
textStyles={[styles.headerText, styles.textNoWrap]}
shouldUseFullTitle={isChatRoom || isPolicyExpenseChat}
/>
{isChatRoom && (
{(isChatRoom || isPolicyExpenseChat) && (
<Text
style={[
styles.sidebarLinkText,
Expand Down
Loading