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.

Sign up for GitHub

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

Refactor close account page to use Form #12534

Merged
merged 15 commits into from
Nov 18, 2022
Merged

Refactor close account page to use Form #12534

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
cf6e656
refactor to use form
Puneet-here Nov 5, 2022
0bbc47b
use new key
Puneet-here Nov 8, 2022
c8fa465
clean up
Puneet-here Nov 8, 2022
0f40bff
remove unnecessary code
Puneet-here Nov 8, 2022
327241d
fix typo
Puneet-here Nov 8, 2022
b0684e1
add isDangerousAction prop
Puneet-here Nov 8, 2022
83aae95
Merge branch 'main' of https://github.com/Puneet-here/App into form-r…
Puneet-here Nov 8, 2022
85d5fc4
resolve conflicts
Puneet-here Nov 8, 2022
a62334a
use ingresa instead of ingrese
Puneet-here Nov 9, 2022
99e0bd4
add spanish copy
Puneet-here Nov 9, 2022
9a5b602
Merge branch 'form-refactor-close-account-page' of https://github.com…
Puneet-here Nov 9, 2022
be2f120
remove close account error modal and unused code
Puneet-here Nov 15, 2022
93c0564
remove unused onyx prop
Puneet-here Nov 15, 2022
b2b2eb2
clear error on refresh and closing page
Puneet-here Nov 15, 2022
003391b
add userEmailOrPhone inside validate function
Puneet-here Nov 16, 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
4 changes: 1 addition & 3 deletions src/ONYXKEYS.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,6 @@ export default {
// Is Keyboard shortcuts modal open?
IS_SHORTCUTS_MODAL_OPEN: 'isShortcutsModalOpen',

// Data related to user closing their account (loading status and error message)
CLOSE_ACCOUNT: 'closeAccount',

// Stores information about active wallet transfer amount, selectedAccountID, status, etc
WALLET_TRANSFER: 'walletTransfer',

Expand All @@ -176,6 +173,7 @@ export default {
REQUEST_CALL_FORM: 'requestCallForm',
REIMBURSEMENT_ACCOUNT_FORM: 'reimbursementAccount',
WORKSPACE_SETTINGS_FORM: 'workspaceSettingsForm',
CLOSE_ACCOUNT_FORM: 'closeAccount',
Beamanator marked this conversation as resolved.
Show resolved Hide resolved
PROFILE_SETTINGS_FORM: 'profileSettingsForm',
DISPLAY_NAME_FORM: 'displayNameForm',
},
Expand Down
5 changes: 5 additions & 0 deletions src/components/Form.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ const propTypes = {
/** Should the button be enabled when offline */
enabledWhenOffline: PropTypes.bool,

/** Whether the action is dangerous */
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand this - what does it mean for an action to be "dangerous"? 😕

Why was this added to such a low level component..

Copy link
Contributor

Choose a reason for hiding this comment

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

To me, "Dangerous" means if the user clicks the "Submit" button, the action being taken should only be taken with caution. I think it would be helpful if you can recommend a "better solution" or what you'd prefer seeing - I am guessing you are saying you'd like to see a more generic prop name, maybe something having to do with the submit button color?

Copy link
Contributor

Choose a reason for hiding this comment

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

To me, "Dangerous" means if the user clicks the "Submit" button, the action being taken should only be taken with caution.

I am guessing you are saying you'd like to see a more generic prop name

No, I think it's too generic. Which "action" is "dangerous"? It's a Form so the user can take many actions. It looks like this just controls the color of the button. So my suggestion would be something like isSubmitButtonDangerous, isSubmitActionDangerous etc.

Copy link
Contributor

Choose a reason for hiding this comment

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

Aah ok I'm glad we got that clarified, I see what you mean that technically there can be many "actions" taken in a form, though I thought it was a bit self explanatory that the main "action" is submitting the form - but it's good to be clear 👍 I like your suggestions, I think isSubmitActionDangerous is a bit more clear to me - there should only be 1 "submit action" (clicking on the submit button) so I'll go with that - follow-up PR incoming!

Copy link
Contributor

Choose a reason for hiding this comment

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

@marcaaron follow-up PR here: #13981

isDangerousAction: PropTypes.bool,

...withLocalizePropTypes,
};

Expand All @@ -59,6 +62,7 @@ const defaultProps = {
},
draftValues: {},
enabledWhenOffline: false,
isDangerousAction: false,
};

class Form extends React.Component {
Expand Down Expand Up @@ -232,6 +236,7 @@ class Form extends React.Component {
}}
containerStyles={[styles.mh0, styles.mt5]}
enabledWhenOffline={this.props.enabledWhenOffline}
isDangerousAction={this.props.isDangerousAction}
/>
)}
</View>
Expand Down
6 changes: 6 additions & 0 deletions src/components/FormAlertWithSubmitButton.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ const propTypes = {

/** Should the button be enabled when offline */
enabledWhenOffline: PropTypes.bool,

/** Whether the action is dangerous */
isDangerousAction: PropTypes.bool,
};

const defaultProps = {
Expand All @@ -45,6 +48,7 @@ const defaultProps = {
isLoading: false,
onFixTheErrorsLinkPressed: () => {},
enabledWhenOffline: false,
isDangerousAction: false,
};

const FormAlertWithSubmitButton = props => (
Expand All @@ -61,6 +65,7 @@ const FormAlertWithSubmitButton = props => (
isDisabled
text={props.buttonText}
style={[styles.mb3]}
danger={props.isDangerousAction}
/>
) : (
<Button
Expand All @@ -70,6 +75,7 @@ const FormAlertWithSubmitButton = props => (
onPress={props.onSubmit}
isDisabled={props.isDisabled}
isLoading={props.isLoading}
danger={props.isDangerousAction}
/>
))}
</FormAlertWrapper>
Expand Down
1 change: 1 addition & 0 deletions src/languages/en.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,7 @@ export default {
defaultContact: 'Default contact method:',
okayGotIt: 'Okay, Got it',
closeAccountError: 'Unable to close account',
enterYourDefaultContactMethod: 'Please enter your default contact method to close your account.',
},
passwordPage: {
changePassword: 'Change password',
Expand Down
1 change: 1 addition & 0 deletions src/languages/es.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,7 @@ export default {
defaultContact: 'Método de contacto predeterminado:',
okayGotIt: 'Ok, entendido',
closeAccountError: 'No se pudo cerrar tu cuenta',
enterYourDefaultContactMethod: 'Please enter your default contact method to close your account.',
},
passwordPage: {
changePassword: 'Cambiar contraseña',
Expand Down
4 changes: 2 additions & 2 deletions src/libs/actions/CloseAccount.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@ import CONST from '../../CONST';
* Clear CloseAccount error message to hide modal
*/
function clearError() {
Onyx.merge(ONYXKEYS.CLOSE_ACCOUNT, {error: ''});
Onyx.merge(ONYXKEYS.FORMS.CLOSE_ACCOUNT_FORM, {error: ''});
}

/**
* Set default Onyx data
*/
function setDefaultData() {
Onyx.merge(ONYXKEYS.CLOSE_ACCOUNT, {...CONST.DEFAULT_CLOSE_ACCOUNT_DATA});
Onyx.merge(ONYXKEYS.FORMS.CLOSE_ACCOUNT_FORM, {...CONST.DEFAULT_CLOSE_ACCOUNT_DATA});
}

export {
Expand Down
4 changes: 2 additions & 2 deletions src/libs/actions/User.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,14 +75,14 @@ function closeAccount(message) {
optimisticData: [
{
onyxMethod: CONST.ONYX.METHOD.MERGE,
key: ONYXKEYS.CLOSE_ACCOUNT,
key: ONYXKEYS.FORMS.CLOSE_ACCOUNT_FORM,
value: {isLoading: true},
},
],
failureData: [
{
onyxMethod: CONST.ONYX.METHOD.MERGE,
key: ONYXKEYS.CLOSE_ACCOUNT,
key: ONYXKEYS.FORMS.CLOSE_ACCOUNT_FORM,
value: {isLoading: false},
},
],
Expand Down
69 changes: 30 additions & 39 deletions src/pages/settings/Security/CloseAccountPage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import React, {Component} from 'react';
import {Linking, ScrollView, View} from 'react-native';
import {Linking, View} from 'react-native';
import {withOnyx} from 'react-native-onyx';
import PropTypes from 'prop-types';
import Str from 'expensify-common/lib/str';
import _ from 'underscore';
import HeaderWithCloseButton from '../../../components/HeaderWithCloseButton';
import Navigation from '../../../libs/Navigation/Navigation';
import ROUTES from '../../../ROUTES';
Expand All @@ -11,16 +12,13 @@ import compose from '../../../libs/compose';
import styles from '../../../styles/styles';
import ScreenWrapper from '../../../components/ScreenWrapper';
import TextInput from '../../../components/TextInput';
import Button from '../../../components/Button';
import Text from '../../../components/Text';
import ConfirmModal from '../../../components/ConfirmModal';
import withLocalize, {withLocalizePropTypes} from '../../../components/withLocalize';
import withWindowDimensions, {windowDimensionsPropTypes} from '../../../components/withWindowDimensions';
import * as CloseAccount from '../../../libs/actions/CloseAccount';
import ONYXKEYS from '../../../ONYXKEYS';
import OfflineIndicator from '../../../components/OfflineIndicator';
import {withNetwork} from '../../../components/OnyxProvider';
import networkPropTypes from '../../../components/networkPropTypes';
import Form from '../../../components/Form';

const propTypes = {
/** Onyx Props */
Expand All @@ -29,9 +27,6 @@ const propTypes = {
closeAccount: PropTypes.shape({
/** Error message if previous attempt to close account was unsuccessful */
error: PropTypes.string,

/** Is account currently being closed? */
isLoading: PropTypes.bool,
}),

/** Session of currently logged in user */
Expand All @@ -40,25 +35,33 @@ const propTypes = {
email: PropTypes.string.isRequired,
}).isRequired,

/** Information about the network */
network: networkPropTypes.isRequired,

...windowDimensionsPropTypes,
...withLocalizePropTypes,
};

const defaultProps = {
closeAccount: {error: '', isLoading: false},
closeAccount: {error: ''},
};

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

this.state = {
reasonForLeaving: '',
phoneOrEmail: '',
};
this.onSubmit = this.onSubmit.bind(this);
this.validate = this.validate.bind(this);
Puneet-here marked this conversation as resolved.
Show resolved Hide resolved
}

onSubmit(values) {
User.closeAccount(values.reasonForLeaving);
}

validate(values, userEmailOrPhone) {
const errors = {};

if (_.isEmpty(values.phoneOrEmail) || userEmailOrPhone.toLowerCase() !== values.phoneOrEmail.toLowerCase()) {
errors.phoneOrEmail = this.props.translate('closeAccountPage.enterYourDefaultContactMethod');
}
return errors;
}

render() {
Expand All @@ -71,21 +74,21 @@ class CloseAccountPage extends Component {
onBackButtonPress={() => Navigation.navigate(ROUTES.SETTINGS_SECURITY)}
onCloseButtonPress={() => Navigation.dismissModal(true)}
/>
<ScrollView
contentContainerStyle={[
styles.flexGrow1,
styles.flexColumn,
styles.p5,
]}
<Form
formID={ONYXKEYS.FORMS.CLOSE_ACCOUNT_FORM}
validate={values => this.validate(values, userEmailOrPhone)}
Puneet-here marked this conversation as resolved.
Show resolved Hide resolved
onSubmit={this.onSubmit}
submitButtonText={this.props.translate('closeAccountPage.closeAccount')}
style={[styles.flexGrow1, styles.mh5]}
isDangerousAction
>
<View style={[styles.flexGrow1]}>
<Text>{this.props.translate('closeAccountPage.reasonForLeavingPrompt')}</Text>
<TextInput
inputID="reasonForLeaving"
multiline
numberOfLines={6}
textAlignVertical="top"
value={this.state.reasonForLeaving}
onChangeText={reasonForLeaving => this.setState({reasonForLeaving})}
label={this.props.translate('closeAccountPage.enterMessageHere')}
containerStyles={[styles.mt5, styles.closeAccountMessageInput]}
/>
Expand All @@ -104,24 +107,13 @@ class CloseAccountPage extends Component {
{userEmailOrPhone}
</Text>
<TextInput
inputID="phoneOrEmail"
autoCapitalize="none"
value={this.state.phoneOrEmail}
onChangeText={phoneOrEmail => this.setState({phoneOrEmail: phoneOrEmail.toLowerCase()})}
label={this.props.translate('closeAccountPage.enterDefaultContact')}
containerStyles={[styles.mt5]}
/>
</View>
<Button
danger
text={this.props.translate('closeAccountPage.closeAccount')}
isLoading={this.props.closeAccount.isLoading}
onPress={() => User.closeAccount(this.state.reasonForLeaving)}
isDisabled={Str.removeSMSDomain(userEmailOrPhone).toLowerCase() !== this.state.phoneOrEmail.toLowerCase() || this.props.network.isOffline}
style={[styles.mt5]}
/>
{!this.props.isSmallScreenWidth
&& <OfflineIndicator containerStyles={[styles.mt2]} />}
</ScrollView>
</Form>
<ConfirmModal
title={this.props.translate('closeAccountPage.closeAccountError')}
success
Expand Down Expand Up @@ -155,10 +147,9 @@ CloseAccountPage.defaultProps = defaultProps;
export default compose(
withLocalize,
withWindowDimensions,
withNetwork(),
withOnyx({
closeAccount: {
key: ONYXKEYS.CLOSE_ACCOUNT,
key: ONYXKEYS.FORMS.CLOSE_ACCOUNT_FORM,
},
session: {
key: ONYXKEYS.SESSION,
Expand Down
2 changes: 1 addition & 1 deletion src/pages/signin/LoginForm.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ LoginForm.defaultProps = defaultProps;
export default compose(
withOnyx({
account: {key: ONYXKEYS.ACCOUNT},
closeAccount: {key: ONYXKEYS.CLOSE_ACCOUNT},
closeAccount: {key: ONYXKEYS.FORMS.CLOSE_ACCOUNT_FORM},
}),
withWindowDimensions,
withLocalize,
Expand Down