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

New Contact methods page linked from Profile page #15039

Merged
merged 18 commits into from
Feb 21, 2023
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
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
2 changes: 2 additions & 0 deletions src/ROUTES.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const IOU_REQUEST_CURRENCY = `${IOU_REQUEST}/currency`;
const IOU_BILL_CURRENCY = `${IOU_BILL}/currency`;
const IOU_SEND_CURRENCY = `${IOU_SEND}/currency`;
const SETTINGS_PERSONAL_DETAILS = 'settings/profile/personal-details';
const SETTINGS_CONTACT_METHODS = 'settings/profile/contact-methods';

export default {
BANK_ACCOUNT: 'bank-account',
Expand Down Expand Up @@ -49,6 +50,7 @@ export default {
SETTINGS_PERSONAL_DETAILS_LEGAL_NAME: `${SETTINGS_PERSONAL_DETAILS}/legal-name`,
SETTINGS_PERSONAL_DETAILS_DATE_OF_BIRTH: `${SETTINGS_PERSONAL_DETAILS}/date-of-birth`,
SETTINGS_PERSONAL_DETAILS_ADDRESS: `${SETTINGS_PERSONAL_DETAILS}/address`,
SETTINGS_CONTACT_METHODS,
NEW_GROUP: 'new/group',
NEW_CHAT: 'new/chat',
REPORT,
Expand Down
4 changes: 4 additions & 0 deletions src/languages/en.js
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,10 @@ export default {
pronouns: 'Pronouns',
isShownOnProfile: 'Your pronouns are shown on your profile.',
},
contacts: {
contactMethod: 'Contact method',
contactMethods: 'Contact methods',
},
pronouns: {
coCos: 'Co / Cos',
eEyEmEir: 'E / Ey / Em / Eir',
Expand Down
4 changes: 4 additions & 0 deletions src/languages/es.js
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,10 @@ export default {
pronouns: 'Pronombres',
isShownOnProfile: 'Tus pronombres se muestran en tu perfil.',
},
contacts: {
contactMethod: 'Método de contacto',
contactMethods: 'Métodos de contacto',
},
pronouns: {
coCos: 'Co / Cos',
eEyEmEir: 'E / Ey / Em / Eir',
Expand Down
9 changes: 8 additions & 1 deletion src/libs/Navigation/AppNavigator/ModalStackNavigators.js
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,14 @@ const SettingsModalStackNavigator = createModalStackNavigator([
},
{
getComponent: () => {
const SettingsAddSecondaryLoginPage = require('../../../pages/settings/AddSecondaryLoginPage').default;
const SettingsContactMethodsPage = require('../../../pages/settings/Profile/Contacts/ContactMethodsPage').default;
return SettingsContactMethodsPage;
},
name: 'Settings_ContactMethods',
},
{
getComponent: () => {
const SettingsAddSecondaryLoginPage = require('../../../pages/settings/Profile/Contacts/AddSecondaryLoginPage').default;
return SettingsAddSecondaryLoginPage;
},
name: 'Settings_Add_Secondary_Login',
Expand Down
4 changes: 4 additions & 0 deletions src/libs/Navigation/linkingConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,10 @@ export default {
path: ROUTES.SETTINGS_APP_DOWNLOAD_LINKS,
exact: true,
},
Settings_ContactMethods: {
path: ROUTES.SETTINGS_CONTACT_METHODS,
exact: true,
},
Settings_Add_Secondary_Login: {
path: ROUTES.SETTINGS_ADD_LOGIN,
},
Expand Down
2 changes: 1 addition & 1 deletion src/libs/actions/User.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ function setSecondaryLoginAndNavigate(login, password) {
}).then((response) => {
if (response.jsonCode === 200) {
Onyx.set(ONYXKEYS.LOGIN_LIST, response.loginList);
Navigation.navigate(ROUTES.SETTINGS_PROFILE);
Navigation.navigate(ROUTES.SETTINGS_CONTACT_METHODS);
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,22 @@ import PropTypes from 'prop-types';
import {View, ScrollView} from 'react-native';
import _ from 'underscore';
import Str from 'expensify-common/lib/str';
import HeaderWithCloseButton from '../../components/HeaderWithCloseButton';
import Navigation from '../../libs/Navigation/Navigation';
import ScreenWrapper from '../../components/ScreenWrapper';
import Text from '../../components/Text';
import styles from '../../styles/styles';
import * as User from '../../libs/actions/User';
import ONYXKEYS from '../../ONYXKEYS';
import Button from '../../components/Button';
import ROUTES from '../../ROUTES';
import CONST from '../../CONST';
import withLocalize, {withLocalizePropTypes} from '../../components/withLocalize';
import compose from '../../libs/compose';
import FixedFooter from '../../components/FixedFooter';
import TextInput from '../../components/TextInput';
import userPropTypes from './userPropTypes';
import * as LoginUtils from '../../libs/LoginUtils';
import HeaderWithCloseButton from '../../../../components/HeaderWithCloseButton';
import Navigation from '../../../../libs/Navigation/Navigation';
import ScreenWrapper from '../../../../components/ScreenWrapper';
import Text from '../../../../components/Text';
import styles from '../../../../styles/styles';
import * as User from '../../../../libs/actions/User';
import ONYXKEYS from '../../../../ONYXKEYS';
import Button from '../../../../components/Button';
import ROUTES from '../../../../ROUTES';
import CONST from '../../../../CONST';
import withLocalize, {withLocalizePropTypes} from '../../../../components/withLocalize';
import compose from '../../../../libs/compose';
import FixedFooter from '../../../../components/FixedFooter';
import TextInput from '../../../../components/TextInput';
import userPropTypes from '../../userPropTypes';
import * as LoginUtils from '../../../../libs/LoginUtils';

const propTypes = {
/* Onyx Props */
Expand Down Expand Up @@ -104,7 +104,7 @@ class AddSecondaryLoginPage extends Component {
? 'addSecondaryLoginPage.addPhoneNumber'
: 'addSecondaryLoginPage.addEmailAddress')}
shouldShowBackButton
onBackButtonPress={() => Navigation.navigate(ROUTES.SETTINGS_PROFILE)}
onBackButtonPress={() => Navigation.navigate(ROUTES.SETTINGS_CONTACT_METHODS)}
onCloseButtonPress={() => Navigation.dismissModal()}
/>
{/* We use keyboardShouldPersistTaps="handled" to prevent the keyboard from being hidden when switching focus on input fields */}
Expand Down
138 changes: 138 additions & 0 deletions src/pages/settings/Profile/Contacts/ContactMethodsPage.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
import Str from 'expensify-common/lib/str';
import lodashGet from 'lodash/get';
import PropTypes from 'prop-types';
import React, {Component} from 'react';
import {ScrollView} from 'react-native-gesture-handler';
import {withOnyx} from 'react-native-onyx';
import _ from 'underscore';
import HeaderWithCloseButton from '../../../../components/HeaderWithCloseButton';
import ScreenWrapper from '../../../../components/ScreenWrapper';
import withCurrentUserPersonalDetails, {withCurrentUserPersonalDetailsDefaultProps, withCurrentUserPersonalDetailsPropTypes} from '../../../../components/withCurrentUserPersonalDetails';
import withLocalize, {withLocalizePropTypes} from '../../../../components/withLocalize';
import CONST from '../../../../CONST';
import compose from '../../../../libs/compose';
import Navigation from '../../../../libs/Navigation/Navigation';
import ONYXKEYS from '../../../../ONYXKEYS';
import ROUTES from '../../../../ROUTES';
import LoginField from './LoginField';

const propTypes = {
/* Onyx Props */

/** Login list for the user that is signed in */
loginList: PropTypes.shape({
/** Value of partner name */
partnerName: PropTypes.string,

/** Phone/Email associated with user */
partnerUserID: PropTypes.string,

/** Date of when login was validated */
validatedDate: PropTypes.string,
}),

...withLocalizePropTypes,
...withCurrentUserPersonalDetailsPropTypes,
};

const defaultProps = {
loginList: {},
...withCurrentUserPersonalDetailsDefaultProps,
};

class ContactMethodsPage extends Component {
constructor(props) {
super(props);

this.state = {
logins: this.getLogins(),
};
Comment on lines +47 to +49
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you know why we copy this to a state instead of just directly calling this.getLogins() on render and use what we get there? synchronizing the prop with the state adds more complexity and in this case doesn't seem to serve any purpose.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Def agreed - i have no idea why we did this in the first place, and if we want to take the time to refactor I def agree it makes sense to just directly use what we get from props and not deal with syncing it with state in componentDidUpdate 👍


this.getLogins = this.getLogins.bind(this);
Copy link
Contributor

Choose a reason for hiding this comment

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

Unnecessary bindings

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see a this.props.loginList in getLogins - we don't need to bind for that?

Copy link
Collaborator

Choose a reason for hiding this comment

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

We don't need to bind here, we need to bind only if we pass a method as the prop to other components.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@Beamanator bump!

Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is just a copy paste and the component will get redone eventually (taken from here), maybe we don't care about this type of polishing. What do you think @Santhosh-Sellavel ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

All right then!

}

componentDidUpdate(prevProps) {
let stateToUpdate = {};

// Recalculate logins if loginList has changed
if (_.keys(this.props.loginList).length !== _.keys(prevProps.loginList).length) {
stateToUpdate = {logins: this.getLogins()};
}

if (_.isEmpty(stateToUpdate)) {
return;
}

// eslint-disable-next-line react/no-did-update-set-state
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should be act on this warning instead of just disable it. I see that we have disable it in other places.

According to the rule documentation, we should do these setStates wrapped in a function:

https://github.com/jsx-eslint/eslint-plugin-react/blob/master/docs/rules/no-did-update-set-state.md

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this.onUpdate(function callback(newName) {

🤢 I feel ya... If we end up getting rid of logins stored in state, we don't have to think about this so I'm going to do nothing for now as we discuss further :D

this.setState(stateToUpdate);
}

/**
* Get the most validated login of each type
*
* @returns {Object}
*/
getLogins() {
return _.reduce(_.values(this.props.loginList), (logins, currentLogin) => {
const type = Str.isSMSLogin(currentLogin.partnerUserID) ? CONST.LOGIN_TYPE.PHONE : CONST.LOGIN_TYPE.EMAIL;
const login = Str.removeSMSDomain(currentLogin.partnerUserID);

// If there's already a login type that's validated and/or currentLogin isn't valid then return early
if ((login !== lodashGet(this.props.currentUserPersonalDetails, 'login')) && !_.isEmpty(logins[type])
&& (logins[type].validatedDate || !currentLogin.validatedDate)) {
return logins;
}
return {
...logins,
[type]: {
...currentLogin,
type,
partnerUserID: Str.removeSMSDomain(currentLogin.partnerUserID),
},
};
}, {
phone: {},
email: {},
});
}

render() {
return (
<ScreenWrapper>
<HeaderWithCloseButton
title={this.props.translate('contacts.contactMethods')}
shouldShowBackButton
onBackButtonPress={() => Navigation.navigate(ROUTES.SETTINGS_PROFILE)}
onCloseButtonPress={() => Navigation.dismissModal(true)}
/>
<ScrollView>
<LoginField
label={this.props.translate('profilePage.emailAddress')}
type="email"
login={this.state.logins.email}
defaultValue={this.state.logins.email}
/>
<LoginField
label={this.props.translate('common.phoneNumber')}
type="phone"
login={this.state.logins.phone}
defaultValue={this.state.logins.phone}
/>
</ScrollView>
</ScreenWrapper>
);
}
}

ContactMethodsPage.propTypes = propTypes;
ContactMethodsPage.defaultProps = defaultProps;

export default compose(
withLocalize,
withCurrentUserPersonalDetails,
withOnyx({
loginList: {
key: ONYXKEYS.LOGIN_LIST,
},
}),
)(ContactMethodsPage);
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
import React, {Component} from 'react';
import {View} from 'react-native';
import PropTypes from 'prop-types';
import Text from '../../../components/Text';
import styles from '../../../styles/styles';
import themeColors from '../../../styles/themes/default';
import * as Expensicons from '../../../components/Icon/Expensicons';
import Icon from '../../../components/Icon';
import ROUTES from '../../../ROUTES';
import CONST from '../../../CONST';
import Navigation from '../../../libs/Navigation/Navigation';
import * as User from '../../../libs/actions/User';
import withLocalize, {withLocalizePropTypes} from '../../../components/withLocalize';
import Button from '../../../components/Button';
import MenuItemWithTopDescription from '../../../components/MenuItemWithTopDescription';
import Text from '../../../../components/Text';
import styles from '../../../../styles/styles';
import themeColors from '../../../../styles/themes/default';
import * as Expensicons from '../../../../components/Icon/Expensicons';
import Icon from '../../../../components/Icon';
import ROUTES from '../../../../ROUTES';
import CONST from '../../../../CONST';
import Navigation from '../../../../libs/Navigation/Navigation';
import * as User from '../../../../libs/actions/User';
import withLocalize, {withLocalizePropTypes} from '../../../../components/withLocalize';
import Button from '../../../../components/Button';
import MenuItemWithTopDescription from '../../../../components/MenuItemWithTopDescription';

const propTypes = {
/** Label to display on login form */
Expand Down
Loading