Merge pull request #8682 from Expensify/cmartins-renameOnChangeCheckbox

Clean up CheckboxWithLabel refactor
Expensify · Apr 20, 2022 · 971566b · 971566b
2 parents f1e4f96 + 187aa68
commit 971566b
Show file tree

Hide file tree

Showing 9 changed files with 35 additions and 29 deletions.
diff --git a/src/components/CheckboxWithLabel.js b/src/components/CheckboxWithLabel.js
@@ -10,13 +10,10 @@ import * as FormUtils from '../libs/FormUtils';
 
 const propTypes = {
     /** Whether the checkbox is checked */
-    isChecked: PropTypes.bool.isRequired,
+    isChecked: PropTypes.bool,
 
     /** Called when the checkbox or label is pressed */
-    onPress: PropTypes.func.isRequired,
-
-    /** Called when the checkbox or label is pressed */
-    onChange: PropTypes.func,
+    onInputChange: PropTypes.func.isRequired,
 
     /** Container styles */
     style: PropTypes.oneOfType([PropTypes.arrayOf(PropTypes.object), PropTypes.object]),
@@ -33,6 +30,12 @@ const propTypes = {
     /** Indicates that the input is being used with the Form component */
     isFormInput: PropTypes.bool,
 
+    /** The default value for the checkbox */
+    defaultValue: PropTypes.bool,
+
+    /** React ref being forwarded to the Checkbox input */
+    forwardedRef: PropTypes.func,
+
     /**
      * The ID used to uniquely identify the input
      *
@@ -46,24 +49,29 @@ const propTypes = {
 };
 
 const defaultProps = {
-    onChange: () => {},
     isFormInput: false,
     inputID: undefined,
     style: [],
     label: undefined,
     LabelComponent: undefined,
     errorText: '',
     shouldSaveDraft: false,
+    isChecked: false,
+    defaultValue: false,
+    forwardedRef: () => {},
 };
 
-const CheckboxWithLabel = React.forwardRef((props, ref) => {
+const CheckboxWithLabel = (props) => {
     const LabelComponent = props.LabelComponent;
     const defaultStyles = [styles.flexRow, styles.alignItemsCenter];
     const wrapperStyles = _.isArray(props.style) ? [...defaultStyles, ...props.style] : [...defaultStyles, props.style];
 
+    // We keep track of the checkbox "state" in a local variable so that this component has an uncontrolled input interface
+    let isChecked = props.defaultValue ? props.defaultValue : props.isChecked;
+
     function toggleCheckbox() {
-        props.onPress(!props.isChecked);
-        props.onChange(!props.isChecked);
+        props.onInputChange(!isChecked);
+        isChecked = !isChecked;
     }
 
     if (!props.label && !LabelComponent) {
@@ -73,11 +81,11 @@ const CheckboxWithLabel = React.forwardRef((props, ref) => {
         <>
             <View style={wrapperStyles}>
                 <Checkbox
-                    isChecked={props.isChecked}
+                    isChecked={isChecked}
                     onPress={toggleCheckbox}
                     label={props.label}
                     hasError={Boolean(props.errorText)}
-                    forwardedRef={ref}
+                    forwardedRef={props.forwardedRef}
                     isFormInput={props.isFormInput}
                     inputID={props.inputID}
                     shouldSaveDraft={props.shouldSaveDraft}
@@ -107,10 +115,13 @@ const CheckboxWithLabel = React.forwardRef((props, ref) => {
             </InlineErrorText>
         </>
     );
-});
+};
 
 CheckboxWithLabel.propTypes = propTypes;
 CheckboxWithLabel.defaultProps = defaultProps;
 CheckboxWithLabel.displayName = 'CheckboxWithLabel';
 
-export default CheckboxWithLabel;
+export default React.forwardRef((props, ref) => (
+    // eslint-disable-next-line react/jsx-props-no-spreading
+    <CheckboxWithLabel {...props} forwardedRef={ref} />
+));
diff --git a/src/pages/EnablePayments/TermsStep.js b/src/pages/EnablePayments/TermsStep.js
@@ -76,7 +76,7 @@ class TermsStep extends React.Component {
                     <CheckboxWithLabel
                         style={[styles.mb4, styles.mt4]}
                         isChecked={this.state.hasAcceptedDisclosure}
-                        onPress={this.toggleDisclosure}
+                        onInputChange={this.toggleDisclosure}
                         LabelComponent={() => (
                             <Text>
                                 {`${this.props.translate('termsStep.haveReadAndAgree')}`}
@@ -88,7 +88,7 @@ class TermsStep extends React.Component {
                     />
                     <CheckboxWithLabel
                         isChecked={this.state.hasAcceptedPrivacyPolicyAndWalletAgreement}
-                        onPress={this.togglePrivacyPolicy}
+                        onInputChange={this.togglePrivacyPolicy}
                         LabelComponent={() => (
                             <>
                                 <Text>

diff --git a/src/pages/ReimbursementAccount/ACHContractStep.js b/src/pages/ReimbursementAccount/ACHContractStep.js
@@ -185,7 +185,7 @@ class ACHContractStep extends React.Component {
                     <CheckboxWithLabel
                         style={[styles.mb2]}
                         isChecked={this.state.ownsMoreThan25Percent}
-                        onPress={() => this.toggleCheckbox('ownsMoreThan25Percent')}
+                        onInputChange={() => this.toggleCheckbox('ownsMoreThan25Percent')}
                         LabelComponent={() => (
                             <Text>
                                 {this.props.translate('beneficialOwnersStep.iOwnMoreThan25Percent')}
@@ -196,7 +196,7 @@ class ACHContractStep extends React.Component {
                     <CheckboxWithLabel
                         style={[styles.mb2]}
                         isChecked={this.state.hasOtherBeneficialOwners}
-                        onPress={() => {
+                        onInputChange={() => {
                             this.setState((prevState) => {
                                 const hasOtherBeneficialOwners = !prevState.hasOtherBeneficialOwners;
                                 const newState = {
@@ -259,7 +259,7 @@ class ACHContractStep extends React.Component {
                     <CheckboxWithLabel
                         style={[styles.mt4]}
                         isChecked={this.state.acceptTermsAndConditions}
-                        onPress={() => this.toggleCheckbox('acceptTermsAndConditions')}
+                        onInputChange={() => this.toggleCheckbox('acceptTermsAndConditions')}
                         LabelComponent={() => (
                             <View style={[styles.flexRow]}>
                                 <Text>{this.props.translate('common.iAcceptThe')}</Text>
@@ -274,7 +274,7 @@ class ACHContractStep extends React.Component {
                     <CheckboxWithLabel
                         style={[styles.mt4]}
                         isChecked={this.state.certifyTrueInformation}
-                        onPress={() => this.toggleCheckbox('certifyTrueInformation')}
+                        onInputChange={() => this.toggleCheckbox('certifyTrueInformation')}
                         LabelComponent={() => (
                             <Text>{this.props.translate('beneficialOwnersStep.certifyTrueAndAccurate')}</Text>
                         )}

diff --git a/src/pages/ReimbursementAccount/BankAccountStep.js b/src/pages/ReimbursementAccount/BankAccountStep.js
@@ -302,7 +302,7 @@ class BankAccountStep extends React.Component {
                         <CheckboxWithLabel
                             style={styles.mt4}
                             isChecked={this.state.hasAcceptedTerms}
-                            onPress={this.toggleTerms}
+                            onInputChange={this.toggleTerms}
                             LabelComponent={() => (
                                 <View style={[styles.flexRow, styles.alignItemsCenter]}>
                                     <Text>

diff --git a/src/pages/ReimbursementAccount/CompanyStep.js b/src/pages/ReimbursementAccount/CompanyStep.js
@@ -294,7 +294,7 @@ class CompanyStep extends React.Component {
                     </View>
                     <CheckboxWithLabel
                         isChecked={this.state.hasNoConnectionToCannabis}
-                        onPress={() => {
+                        onInputChange={() => {
                             this.setState((prevState) => {
                                 const newState = {hasNoConnectionToCannabis: !prevState.hasNoConnectionToCannabis};
                                 BankAccounts.updateReimbursementAccountDraft(newState);

diff --git a/src/pages/ReimbursementAccount/RequestorStep.js b/src/pages/ReimbursementAccount/RequestorStep.js
@@ -213,7 +213,7 @@ class RequestorStep extends React.Component {
                         />
                         <CheckboxWithLabel
                             isChecked={this.state.isControllingOfficer}
-                            onPress={() => {
+                            onInputChange={() => {
                                 this.setState((prevState) => {
                                     const newState = {isControllingOfficer: !prevState.isControllingOfficer};
                                     BankAccounts.updateReimbursementAccountDraft(newState);

diff --git a/src/pages/settings/Payments/AddDebitCardPage.js b/src/pages/settings/Payments/AddDebitCardPage.js
@@ -318,7 +318,7 @@ class DebitCardPage extends Component {
                             </View>
                             <CheckboxWithLabel
                                 isChecked={this.state.acceptedTerms}
-                                onPress={() => {
+                                onInputChange={() => {
                                     this.setState(prevState => ({
                                         acceptedTerms: !prevState.acceptedTerms,
                                         errors: {

diff --git a/src/pages/settings/Profile/ProfilePage.js b/src/pages/settings/Profile/ProfilePage.js
@@ -298,7 +298,7 @@ class ProfilePage extends Component {
                         <CheckboxWithLabel
                             label={this.props.translate('profilePage.setMyTimezoneAutomatically')}
                             isChecked={this.state.isAutomaticTimezone}
-                            onPress={this.setAutomaticTimezone}
+                            onInputChange={this.setAutomaticTimezone}
                         />
                     </ScrollView>
                     <FixedFooter>

diff --git a/src/stories/Form.stories.js b/src/stories/Form.stories.js
@@ -26,8 +26,6 @@ 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);
@@ -101,10 +99,7 @@ const Template = (args) => {
             />
             <CheckboxWithLabel
                 inputID="checkbox"
-                isChecked={isChecked}
-                defaultValue={isChecked}
                 style={[styles.mb4, styles.mt5]}
-                onPress={() => { setIsChecked(prev => !prev); }}
                 isFormInput
                 shouldSaveDraft
                 LabelComponent={() => (