From 3acceaa4859d8d53245ee8645ee0e7f6a1984e04 Mon Sep 17 00:00:00 2001 From: sahil Date: Tue, 15 Feb 2022 03:45:54 +0530 Subject: [PATCH 1/9] refactor checkboxWithLabel --- src/components/Checkbox.js | 12 +++++-- src/components/CheckboxWithLabel.js | 33 +++++++++++++++---- src/components/InlineErrorText.js | 6 +++- .../ReimbursementAccount/ACHContractStep.js | 1 - .../ReimbursementAccount/BankAccountStep.js | 2 +- src/pages/ReimbursementAccount/CompanyStep.js | 1 - .../ReimbursementAccount/RequestorStep.js | 1 - .../settings/Payments/AddDebitCardPage.js | 1 - src/styles/utilities/spacing.js | 4 +++ 9 files changed, 45 insertions(+), 16 deletions(-) diff --git a/src/components/Checkbox.js b/src/components/Checkbox.js index 81412da8d7c3..3308578dbefb 100644 --- a/src/components/Checkbox.js +++ b/src/components/Checkbox.js @@ -13,27 +13,33 @@ const propTypes = { onPress: PropTypes.func.isRequired, /** Should the input be styled for errors */ - hasError: PropTypes.bool, + errorText: PropTypes.string, /** Should the input be disabled */ disabled: PropTypes.bool, + + /** A ref to forward to the Pressable */ + forwardedRef: PropTypes.oneOfType([PropTypes.object]), + }; const defaultProps = { - hasError: false, + errorText: '', disabled: false, + forwardedRef: undefined, }; const Checkbox = props => ( props.onPress(!props.isChecked)} + ref={props.forwardedRef} > diff --git a/src/components/CheckboxWithLabel.js b/src/components/CheckboxWithLabel.js index 623bbce3af4a..05ce60c1da7f 100644 --- a/src/components/CheckboxWithLabel.js +++ b/src/components/CheckboxWithLabel.js @@ -6,6 +6,7 @@ import styles from '../styles/styles'; import Checkbox from './Checkbox'; import Text from './Text'; import InlineErrorText from './InlineErrorText'; +import * as FormUtils from '../libs/FormUtils'; const propTypes = { /** Whether the checkbox is checked */ @@ -23,22 +24,36 @@ const propTypes = { /** Component to display for label */ LabelComponent: PropTypes.func, - /** Should the input be styled for errors */ - hasError: PropTypes.bool, /** Error text to display */ errorText: PropTypes.string, + + /** Indicates that the input is being used with the Form component */ + isFormInput: PropTypes.bool, + + /** + * The ID used to uniquely identify the input + * + * @param {Object} props - props passed to the input + * @returns {Object} - returns an Error object if isFormInput is supplied but inputID is falsey or not a string + */ + inputID: props => FormUtils.getInputIDPropTypes(props), + + /** Saves a draft of the input value when used in a form */ + shouldSaveDraft: PropTypes.bool, }; const defaultProps = { + isFormInput: false, + inputID: undefined, style: [], label: undefined, LabelComponent: undefined, - hasError: false, errorText: '', + shouldSaveDraft: false, }; -const CheckboxWithLabel = (props) => { +const CheckboxWithLabel = React.forwardRef((props, ref) => { const LabelComponent = props.LabelComponent; const defaultStyles = [styles.flexRow, styles.alignItemsCenter]; const wrapperStyles = _.isArray(props.style) ? [...defaultStyles, ...props.style] : [...defaultStyles, props.style]; @@ -53,7 +68,11 @@ const CheckboxWithLabel = (props) => { isChecked={props.isChecked} onPress={() => props.onPress(!props.isChecked)} label={props.label} - hasError={props.hasError} + errorText={props.errorText} + forwardedRef={ref} + isFormInput={props.isFormInput} + inputID={props.inputID} + shouldSaveDraft={props.shouldSaveDraft} /> props.onPress(!props.isChecked)} @@ -76,13 +95,13 @@ const CheckboxWithLabel = (props) => { {!_.isEmpty(props.errorText) && ( - + {props.errorText} )} ); -}; +}); CheckboxWithLabel.propTypes = propTypes; CheckboxWithLabel.defaultProps = defaultProps; diff --git a/src/components/InlineErrorText.js b/src/components/InlineErrorText.js index 90f6838e19bc..d77a5c4aacee 100644 --- a/src/components/InlineErrorText.js +++ b/src/components/InlineErrorText.js @@ -7,10 +7,14 @@ import Text from './Text'; const propTypes = { /** Text to display */ children: PropTypes.string, + + /** Styling for inline error text */ + styles: PropTypes.arrayOf(PropTypes.object), }; const defaultProps = { children: 'Error', + styles: [], }; const InlineErrorText = (props) => { @@ -19,7 +23,7 @@ const InlineErrorText = (props) => { } return ( - {props.children} + {props.children} ); }; diff --git a/src/pages/ReimbursementAccount/ACHContractStep.js b/src/pages/ReimbursementAccount/ACHContractStep.js index 565f7b9fe51a..d6140f125e90 100644 --- a/src/pages/ReimbursementAccount/ACHContractStep.js +++ b/src/pages/ReimbursementAccount/ACHContractStep.js @@ -279,7 +279,6 @@ class ACHContractStep extends React.Component { {this.props.translate('beneficialOwnersStep.certifyTrueAndAccurate')} )} errorText={this.getErrorText('certifyTrueInformation')} - hasError={this.getErrors().certifyTrueInformation} /> diff --git a/src/pages/ReimbursementAccount/BankAccountStep.js b/src/pages/ReimbursementAccount/BankAccountStep.js index a6216dc67224..d5aa3dd3aa6c 100644 --- a/src/pages/ReimbursementAccount/BankAccountStep.js +++ b/src/pages/ReimbursementAccount/BankAccountStep.js @@ -312,7 +312,7 @@ class BankAccountStep extends React.Component { )} - hasError={this.getErrors().hasAcceptedTerms} + errorText={this.getErrors().hasAcceptedTerms ? this.props.translate('addDebitCardPage.error.acceptedTerms') : ''} /> )} diff --git a/src/pages/ReimbursementAccount/CompanyStep.js b/src/pages/ReimbursementAccount/CompanyStep.js index 60cad42f1f2e..31847224d9dc 100644 --- a/src/pages/ReimbursementAccount/CompanyStep.js +++ b/src/pages/ReimbursementAccount/CompanyStep.js @@ -315,7 +315,6 @@ class CompanyStep extends React.Component { )} style={[styles.mt4]} errorText={this.getErrorText('hasNoConnectionToCannabis')} - hasError={this.getErrors().hasNoConnectionToCannabis} /> diff --git a/src/pages/ReimbursementAccount/RequestorStep.js b/src/pages/ReimbursementAccount/RequestorStep.js index ced2bc4c02e0..c7a503c28537 100644 --- a/src/pages/ReimbursementAccount/RequestorStep.js +++ b/src/pages/ReimbursementAccount/RequestorStep.js @@ -227,7 +227,6 @@ class RequestorStep extends React.Component { )} style={[styles.mt4]} - hasError={Boolean(this.getErrors().isControllingOfficer)} errorText={this.getErrors().isControllingOfficer ? this.props.translate('requestorStep.isControllingOfficerError') : ''} /> diff --git a/src/pages/settings/Payments/AddDebitCardPage.js b/src/pages/settings/Payments/AddDebitCardPage.js index dc0f308b5526..119cef75e1f7 100644 --- a/src/pages/settings/Payments/AddDebitCardPage.js +++ b/src/pages/settings/Payments/AddDebitCardPage.js @@ -306,7 +306,6 @@ class DebitCardPage extends Component { )} style={[styles.mt4, styles.mb4]} errorText={this.getErrorText('acceptedTerms')} - hasError={Boolean(this.state.errors.acceptedTerms)} /> {!_.isEmpty(this.props.addDebitCardForm.error) && ( diff --git a/src/styles/utilities/spacing.js b/src/styles/utilities/spacing.js index 4e18848fd18f..ec15095f671e 100644 --- a/src/styles/utilities/spacing.js +++ b/src/styles/utilities/spacing.js @@ -109,6 +109,10 @@ export default { marginLeft: 20, }, + ml8: { + marginLeft: 32, + }, + mln5: { marginLeft: -20, }, From 2589aea32077aa459f2403824d3261c2f92b5c3f Mon Sep 17 00:00:00 2001 From: sahil Date: Tue, 15 Feb 2022 03:46:22 +0530 Subject: [PATCH 2/9] add checkboxWithLabel to form stories --- src/stories/Form.stories.js | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/src/stories/Form.stories.js b/src/stories/Form.stories.js index f8b0a9b8cbe3..1309427018de 100644 --- a/src/stories/Form.stories.js +++ b/src/stories/Form.stories.js @@ -4,6 +4,9 @@ import TextInput from '../components/TextInput'; import Form from '../components/Form'; import * as FormActions from '../libs/actions/FormActions'; import styles from '../styles/styles'; +import CheckboxWithLabel from '../components/CheckboxWithLabel'; +import Text from '../components/Text'; + /** * We use the Component Story Format for writing stories. Follow the docs here: @@ -13,7 +16,7 @@ import styles from '../styles/styles'; const story = { title: 'Components/Form', component: Form, - subcomponents: {TextInput}, + subcomponents: {TextInput, CheckboxWithLabel}, }; const Template = (args) => { @@ -39,6 +42,18 @@ const Template = (args) => { containerStyles={[styles.mt4]} isFormInput /> + {}} + isFormInput + shouldSaveDraft + LabelComponent={() => ( + I accept the Expensify Terms of Service + )} + errorText={args.validate(args.draftValues).checkbox} + /> ); }; @@ -61,6 +76,9 @@ const defaultArgs = { if (!values.accountNumber) { errors.accountNumber = 'Please enter an account number'; } + if (!values.checkbox) { + errors.checkbox = 'You must accept the Terms of Service to continue'; + } return errors; }, onSubmit: (values) => { @@ -76,13 +94,14 @@ const defaultArgs = { draftValues: { routingNumber: '00001', accountNumber: '1111222233331111', + checkbox: true, }, }; Default.args = defaultArgs; Loading.args = {...defaultArgs, formState: {isSubmitting: true}}; ServerError.args = {...defaultArgs, formState: {isSubmitting: false, serverErrorMessage: 'There was an unexpected error. Please try again later.'}}; -InputError.args = {...defaultArgs, draftValues: {routingNumber: '', accountNumber: ''}}; +InputError.args = {...defaultArgs, draftValues: {routingNumber: '', accountNumber: '', checkbox: false}}; export default story; export { From 143d19a5377fdef23110481b5964fdc464222d85 Mon Sep 17 00:00:00 2001 From: sahil Date: Thu, 17 Feb 2022 03:10:56 +0530 Subject: [PATCH 3/9] improvement --- src/components/CheckboxWithLabel.js | 14 ++++++++++++-- src/stories/Form.stories.js | 12 +++++++----- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/components/CheckboxWithLabel.js b/src/components/CheckboxWithLabel.js index 05ce60c1da7f..fde124f48ca6 100644 --- a/src/components/CheckboxWithLabel.js +++ b/src/components/CheckboxWithLabel.js @@ -15,6 +15,9 @@ const propTypes = { /** Called when the checkbox or label is pressed */ onPress: PropTypes.func.isRequired, + /** Called when the checkbox or label is pressed */ + onChange: PropTypes.func, + /** Container styles */ style: PropTypes.oneOfType([PropTypes.arrayOf(PropTypes.object), PropTypes.object]), @@ -44,6 +47,7 @@ const propTypes = { }; const defaultProps = { + onChange: () => {}, isFormInput: false, inputID: undefined, style: [], @@ -66,7 +70,10 @@ const CheckboxWithLabel = React.forwardRef((props, ref) => { props.onPress(!props.isChecked)} + onPress={() => { + props.onPress(!props.isChecked); + props.onChange(!props.isChecked); + }} label={props.label} errorText={props.errorText} forwardedRef={ref} @@ -75,7 +82,10 @@ const CheckboxWithLabel = React.forwardRef((props, ref) => { shouldSaveDraft={props.shouldSaveDraft} /> props.onPress(!props.isChecked)} + onPress={() => { + props.onPress(!props.isChecked); + props.onChange(!props.isChecked); + }} style={[ styles.ml3, styles.pr2, diff --git a/src/stories/Form.stories.js b/src/stories/Form.stories.js index 1309427018de..9610923a9c15 100644 --- a/src/stories/Form.stories.js +++ b/src/stories/Form.stories.js @@ -1,4 +1,4 @@ -import React from 'react'; +import React, {useState} from 'react'; import {View} from 'react-native'; import TextInput from '../components/TextInput'; import Form from '../components/Form'; @@ -20,6 +20,8 @@ const story = { }; const Template = (args) => { + const [isChecked, setIsChecked] = useState(args.draftValues.checkbox); + // Form consumes data from Onyx, so we initialize Onyx with the necessary data here FormActions.setIsSubmitting(args.formID, args.formState.isSubmitting); FormActions.setServerErrorMessage(args.formID, args.formState.serverErrorMessage); @@ -44,15 +46,15 @@ const Template = (args) => { /> {}} + onPress={() => { setIsChecked(prev => !prev); }} isFormInput shouldSaveDraft LabelComponent={() => ( I accept the Expensify Terms of Service )} - errorText={args.validate(args.draftValues).checkbox} /> ); @@ -94,7 +96,7 @@ const defaultArgs = { draftValues: { routingNumber: '00001', accountNumber: '1111222233331111', - checkbox: true, + checkbox: false, }, }; From 1b9605f54be19476a611d3395abb862be0df9fbc Mon Sep 17 00:00:00 2001 From: sahil Date: Tue, 1 Mar 2022 21:22:47 +0530 Subject: [PATCH 4/9] create function for onPress --- src/components/CheckboxWithLabel.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/components/CheckboxWithLabel.js b/src/components/CheckboxWithLabel.js index fde124f48ca6..01585357af00 100644 --- a/src/components/CheckboxWithLabel.js +++ b/src/components/CheckboxWithLabel.js @@ -62,6 +62,11 @@ const CheckboxWithLabel = React.forwardRef((props, ref) => { const defaultStyles = [styles.flexRow, styles.alignItemsCenter]; const wrapperStyles = _.isArray(props.style) ? [...defaultStyles, ...props.style] : [...defaultStyles, props.style]; + const onCheckboxPress = () => { + props.onPress(!props.isChecked); + props.onChange(!props.isChecked); + }; + if (!props.label && !LabelComponent) { throw new Error('Must provide at least label or LabelComponent prop'); } @@ -70,10 +75,7 @@ const CheckboxWithLabel = React.forwardRef((props, ref) => { { - props.onPress(!props.isChecked); - props.onChange(!props.isChecked); - }} + onPress={onCheckboxPress} label={props.label} errorText={props.errorText} forwardedRef={ref} From 046fdcdc536b4ff207e88d9069c3c16141b61717 Mon Sep 17 00:00:00 2001 From: sahil Date: Tue, 1 Mar 2022 21:27:36 +0530 Subject: [PATCH 5/9] change type --- src/components/Checkbox.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/components/Checkbox.js b/src/components/Checkbox.js index 3308578dbefb..6bc25d527a77 100644 --- a/src/components/Checkbox.js +++ b/src/components/Checkbox.js @@ -19,8 +19,10 @@ const propTypes = { disabled: PropTypes.bool, /** A ref to forward to the Pressable */ - forwardedRef: PropTypes.oneOfType([PropTypes.object]), - + forwardedRef: PropTypes.oneOfType([ + PropTypes.func, + PropTypes.shape({current: PropTypes.instanceOf(React.Component)}), + ]), }; const defaultProps = { From 801fa9cb46eae1cbcb5e96aadac20755de51dc99 Mon Sep 17 00:00:00 2001 From: sahil Date: Tue, 1 Mar 2022 21:52:28 +0530 Subject: [PATCH 6/9] remove extra line --- src/stories/Form.stories.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/stories/Form.stories.js b/src/stories/Form.stories.js index 95d6ba8ab115..d07219239c7d 100644 --- a/src/stories/Form.stories.js +++ b/src/stories/Form.stories.js @@ -8,7 +8,6 @@ import styles from '../styles/styles'; import CheckboxWithLabel from '../components/CheckboxWithLabel'; import Text from '../components/Text'; - /** * We use the Component Story Format for writing stories. Follow the docs here: * From 714d1e5fca2bd6d21552adafa03a79eb20d59d27 Mon Sep 17 00:00:00 2001 From: sahil Date: Wed, 2 Mar 2022 16:52:39 +0530 Subject: [PATCH 7/9] pass onCheckboxPress in TouchableOpacity --- src/components/CheckboxWithLabel.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/components/CheckboxWithLabel.js b/src/components/CheckboxWithLabel.js index 01585357af00..0424ec3584f6 100644 --- a/src/components/CheckboxWithLabel.js +++ b/src/components/CheckboxWithLabel.js @@ -84,10 +84,7 @@ const CheckboxWithLabel = React.forwardRef((props, ref) => { shouldSaveDraft={props.shouldSaveDraft} /> { - props.onPress(!props.isChecked); - props.onChange(!props.isChecked); - }} + onPress={onCheckboxPress} style={[ styles.ml3, styles.pr2, From 963593a5db42d455881cd3d189106144024888c8 Mon Sep 17 00:00:00 2001 From: sahil Date: Tue, 8 Mar 2022 03:55:46 +0530 Subject: [PATCH 8/9] change function name --- src/components/CheckboxWithLabel.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/components/CheckboxWithLabel.js b/src/components/CheckboxWithLabel.js index 0424ec3584f6..9c44f2c341eb 100644 --- a/src/components/CheckboxWithLabel.js +++ b/src/components/CheckboxWithLabel.js @@ -62,10 +62,10 @@ const CheckboxWithLabel = React.forwardRef((props, ref) => { const defaultStyles = [styles.flexRow, styles.alignItemsCenter]; const wrapperStyles = _.isArray(props.style) ? [...defaultStyles, ...props.style] : [...defaultStyles, props.style]; - const onCheckboxPress = () => { + function toggleCheckbox() { props.onPress(!props.isChecked); props.onChange(!props.isChecked); - }; + } if (!props.label && !LabelComponent) { throw new Error('Must provide at least label or LabelComponent prop'); @@ -75,7 +75,7 @@ const CheckboxWithLabel = React.forwardRef((props, ref) => { { shouldSaveDraft={props.shouldSaveDraft} /> Date: Thu, 10 Mar 2022 04:13:19 +0530 Subject: [PATCH 9/9] use hasError --- src/components/Checkbox.js | 6 +++--- src/components/CheckboxWithLabel.js | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/components/Checkbox.js b/src/components/Checkbox.js index 6bc25d527a77..0a9f758cf90f 100644 --- a/src/components/Checkbox.js +++ b/src/components/Checkbox.js @@ -13,7 +13,7 @@ const propTypes = { onPress: PropTypes.func.isRequired, /** Should the input be styled for errors */ - errorText: PropTypes.string, + hasError: PropTypes.bool, /** Should the input be disabled */ disabled: PropTypes.bool, @@ -26,7 +26,7 @@ const propTypes = { }; const defaultProps = { - errorText: '', + hasError: false, disabled: false, forwardedRef: undefined, }; @@ -41,7 +41,7 @@ const Checkbox = props => ( style={[ styles.checkboxContainer, props.isChecked && styles.checkedContainer, - props.errorText && styles.borderColorDanger, + props.hasError && styles.borderColorDanger, props.disabled && styles.cursorDisabled, ]} > diff --git a/src/components/CheckboxWithLabel.js b/src/components/CheckboxWithLabel.js index 9c44f2c341eb..73847cbd3d97 100644 --- a/src/components/CheckboxWithLabel.js +++ b/src/components/CheckboxWithLabel.js @@ -77,7 +77,7 @@ const CheckboxWithLabel = React.forwardRef((props, ref) => { isChecked={props.isChecked} onPress={toggleCheckbox} label={props.label} - errorText={props.errorText} + hasError={Boolean(props.errorText)} forwardedRef={ref} isFormInput={props.isFormInput} inputID={props.inputID}