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 Guides call request flow to workspace card configuration screen #4637

Merged
merged 14 commits into from
Aug 18, 2021
3 changes: 2 additions & 1 deletion src/ROUTES.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ export default {
getWorkspacePeopleRoute: policyID => `workspace/${policyID}/people`,
getWorkspaceInviteRoute: policyID => `workspace/${policyID}/invite`,
WORKSPACE_INVITE: 'workspace/:policyID/invite',
REQUEST_CALL: 'request-call',
getRequestCallRoute: taskID => `request-call/${taskID}`,
REQUEST_CALL: 'request-call/:taskID',
getWorkspaceEditorRoute: policyID => `workspace/${policyID}/edit`,
WORKSPACE_EDITOR: 'workspace/:policyID/edit',
VALIDATE_CODE_URL: (accountID, validateCode, exitTo = '') => {
Expand Down
13 changes: 13 additions & 0 deletions src/components/HeaderWithCloseButton.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import Icon from './Icon';
import {Close, Download, BackArrow} from './Icon/Expensicons';
import compose from '../libs/compose';
import withLocalize, {withLocalizePropTypes} from './withLocalize';
import InboxCallButton from './InboxCallButton';

const propTypes = {
/** Title of the Header */
Expand All @@ -32,6 +33,11 @@ const propTypes = {
/** Whether we should show a download button */
shouldShowDownloadButton: PropTypes.bool,

/** Whether weshould show a inbox call button */
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
/** Whether weshould show a inbox call button */
/** Whether we should show a inbox call button */

shouldShowInboxCallButton: PropTypes.bool,

alex-mechler marked this conversation as resolved.
Show resolved Hide resolved
inboxCallTaskID: PropTypes.string,

...withLocalizePropTypes,
};

Expand All @@ -43,6 +49,8 @@ const defaultProps = {
shouldShowBackButton: false,
shouldShowBorderBottom: false,
shouldShowDownloadButton: false,
shouldShowInboxCallButton: false,
inboxCallTaskID: '',
};

const HeaderWithCloseButton = props => (
Expand Down Expand Up @@ -77,6 +85,11 @@ const HeaderWithCloseButton = props => (
)
}

{
props.shouldShowInboxCallButton && <InboxCallButton taskID={props.inboxCallTaskID} />

Copy link
Contributor

Choose a reason for hiding this comment

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

remove the unnecessary line break

}

<TouchableOpacity
onPress={props.onCloseButtonPress}
style={[styles.touchableButtonImage]}
Expand Down
58 changes: 58 additions & 0 deletions src/components/InboxCallButton.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import React from 'react';
import {
View, Pressable,
} from 'react-native';
import PropTypes from 'prop-types';
import Icon from './Icon';
import {Phone} from './Icon/Expensicons';
import styles from '../styles/styles';
import themeColors from '../styles/themes/default';
import withLocalize, {withLocalizePropTypes} from './withLocalize';
import compose from '../libs/compose';
import Navigation from '../libs/Navigation/Navigation';
import ROUTES from '../ROUTES';
import Text from './Text';

const propTypes = {
...withLocalizePropTypes,
taskID: PropTypes.string,
};

const defaultProps = {
taskID: '',
};

const InboxCallButton = props => (
<>
<View
style={[styles.flex1, styles.flexRow, styles.justifyContentCenter, styles.alignItemsCenter]}
>
<Pressable
onPress={() => {
Navigation.navigate(ROUTES.getRequestCallRoute(props.taskID));
}}
style={[styles.button, styles.buttonSmall]}
>
<View style={styles.flexRow}>
<View style={styles.mr1}>
<Icon
src={Phone}
fill={themeColors.heading}
small
/>
</View>
<View>
<Text style={styles.buttonText}>
{props.translate('requestCallPage.needHelp')}
</Text>
</View>
</View>
</Pressable>
</View>
</>
);

InboxCallButton.propTypes = propTypes;
InboxCallButton.defaultProps = defaultProps;
InboxCallButton.displayName = 'InboxCallButton';
export default compose(withLocalize)(InboxCallButton);
12 changes: 7 additions & 5 deletions src/components/VideoChatButtonAndMenu.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,13 @@ import ROUTES from '../ROUTES';
const propTypes = {
...withLocalizePropTypes,
...windowDimensionsPropTypes,
isConcierge: PropTypes.bool,
openInboxCall: 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.

openInboxCall feels like an odd name for a boolean. I feel like this should start with is or should, maybe something like shouldOpenInboxCall or something. Either way, looking at the logic and now understanding it i felt like isConciergeChat is more obvious of a variable to understand whats going on i.e., if not concierge chat, open video menu, else if concierge chat open inbox call to schedule a setup call.

If you'd like to keep inboxCall then maybe add some comments explaining what happens if this variable is true or false.

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, before all the design changes, the new button also used this flow, so the variable name was changed. Since the design changes mean they are two different flows, I will change this back! Good catch

taskID: PropTypes.string,
};

const defaultProps = {
isConcierge: false,
openInboxCall: false,
taskID: '',
};

class VideoChatButtonAndMenu extends Component {
Expand Down Expand Up @@ -98,8 +100,8 @@ class VideoChatButtonAndMenu extends Component {
<Pressable
onPress={() => {
// If this is the Concierge chat, we'll open the modal for requesting a setup call instead
alex-mechler marked this conversation as resolved.
Show resolved Hide resolved
if (this.props.isConcierge) {
Navigation.navigate(ROUTES.REQUEST_CALL);
if (this.props.openInboxCall) {
Navigation.navigate(ROUTES.getRequestCallRoute(this.props.taskID));
return;
}
this.toggleVideoChatMenu();
Expand All @@ -108,7 +110,7 @@ class VideoChatButtonAndMenu extends Component {
>
<Icon
src={Phone}
fill={(this.props.isConcierge || this.state.isVideoChatMenuActive)
fill={(this.props.openInboxCall || this.state.isVideoChatMenuActive)
? themeColors.heading
: themeColors.icon}
/>
Expand Down
1 change: 1 addition & 0 deletions src/languages/en.js
Original file line number Diff line number Diff line change
Expand Up @@ -613,5 +613,6 @@ export default {
growlMessageInvalidPhone: 'That doesn’t look like a valid phone number. Try again with the country code.\ne.g. +15005550006',
growlMessageEmptyName: 'Please provide both a first and last name so our Guides know how to address you!',
growlMessageNoPersonalPolicy: 'I wasn’t able to find a personal policy to associate this Guides call with, please check your connection and try again.',
needHelp: 'Need help?',
},
};
1 change: 1 addition & 0 deletions src/languages/es.js
Original file line number Diff line number Diff line change
Expand Up @@ -615,5 +615,6 @@ export default {
growlMessageInvalidPhone: 'El teléfono no es valido. Intentalo de nuevo agregando el código de país. P. ej.: +15005550006',
growlMessageEmptyName: 'Por favor ingresa tu nombre completo',
growlMessageNoPersonalPolicy: 'No he podido encontrar una póliza personal con la que asociar esta llamada a las Guías, compruebe su conexión e inténtelo de nuevo.',
needHelp: '¿Necesitas ayuda?',
},
};
6 changes: 3 additions & 3 deletions src/libs/actions/Inbox.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import {Inbox_CallUser} from '../API';

function requestConciergeDMCall(policyID, firstName, lastName, phoneNumber) {
function requestInboxCall(taskID, policyID, firstName, lastName, phoneNumber) {
return Inbox_CallUser({
policyID,
firstName,
lastName,
phoneNumber,
taskID: 'NewExpensifyConciergeDM',
taskID,
});
}

// eslint-disable-next-line import/prefer-default-export
export {requestConciergeDMCall};
export {requestInboxCall};
10 changes: 8 additions & 2 deletions src/pages/RequestCallPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import Button from '../components/Button';
import FixedFooter from '../components/FixedFooter';
import CONST from '../CONST';
import Growl from '../libs/Growl';
import {requestConciergeDMCall} from '../libs/actions/Inbox';
import {requestInboxCall} from '../libs/actions/Inbox';
import {fetchOrCreateChatReport} from '../libs/actions/Report';
import personalDetailsPropType from './personalDetailsPropType';
import ExpensiTextInput from '../components/ExpensiTextInput';
Expand Down Expand Up @@ -52,6 +52,12 @@ const propTypes = {
/** The type of the policy */
type: PropTypes.string,
}).isRequired,

route: PropTypes.shape({
alex-mechler marked this conversation as resolved.
Show resolved Hide resolved
params: PropTypes.shape({
taskID: PropTypes.string,
}),
}).isRequired,
};

class RequestCallPage extends Component {
Expand Down Expand Up @@ -83,7 +89,7 @@ class RequestCallPage extends Component {
Growl.error(this.props.translate('requestCallPage.growlMessageNoPersonalPolicy'), 3000);
return;
}
requestConciergeDMCall(personalPolicy.id, this.state.firstName, this.state.lastName, this.state.phoneNumber)
requestInboxCall(this.props.route.params.taskID, personalPolicy.id, this.state.firstName, this.state.lastName, this.state.phoneNumber)
.then((result) => {
this.setState({isLoading: false});
if (result.jsonCode === 200) {
Expand Down
2 changes: 1 addition & 1 deletion src/pages/home/HeaderView.js
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ const HeaderView = (props) => {
{props.report.hasOutstandingIOU && (
<IOUBadge iouReportID={props.report.iouReportID} />
)}
<VideoChatButtonAndMenu isConcierge={isConcierge} />
<VideoChatButtonAndMenu openInboxCall={isConcierge} taskID="ExpensifyCashConciergeDM" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Thoughts on making the 2nd param consistent and more verbose i.e., instead of taskID calling it inboxCallTaskID.
Two reasons why I suggest this,

  1. taskID for VideoChatButtonAndMenu doesn't mean much imo so someone needs to look at the code to know what it is.
  2. Renaming to inboxCallTaskID would keep it consistent to the variable here.

<Pressable
onPress={() => togglePinnedState(props.report)}
style={[styles.touchableButtonImage, styles.mr0]}
Expand Down
2 changes: 2 additions & 0 deletions src/pages/workspace/WorkspaceCardPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,8 @@ const WorkspaceCardPage = ({
onCloseButtonPress={() => Navigation.dismissModal()}
onBackButtonPress={() => Navigation.goBack()}
shouldShowBackButton={isSmallScreenWidth}
shouldShowInboxCallButton
inboxCallTaskID="WorkspaceCompanyCards"
alex-mechler marked this conversation as resolved.
Show resolved Hide resolved
/>
<ScrollView style={[styles.settingsPageBackground]} bounces={false}>
<View style={styles.pageWrapper}>
Expand Down